maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

[deploy] Move deletion of legacy cookie higher in omniauth callback (#10190)

Browse source 
* Move deletion of legacy cookie higher in omniauth callback

* Add specific date to hotfix TODO note

* Fix inverted logic that looked for wrong cookie domain
This commit is contained in:
Vaidehi Joshi 2020-09-03 13:17:34 -07:00 • committed by GitHub
parent d0d3c2a2ab
commit d7d29c1bb2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 17 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
app/controllers/omniauth_callbacks_controller.rb
View file

@ -34,6 +34,12 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
private
def callback_for(provider)
# Deleting the session cookie with the legacy app domain, which does NOT include a preceding dot.
# This should fix the double cookie scenario.
# TODO: this code is a hotfix, we should remove it after 09/18/2020.
legacy_cookie_domain = Rails.env.production? ? ApplicationConfig["APP_DOMAIN"] : nil
cookies.delete(ApplicationConfig["SESSION_KEY"], domain: legacy_cookie_domain)
auth_payload = request.env["omniauth.auth"]
cta_variant = request.env["omniauth.params"]["state"].to_s
@ -44,15 +50,6 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
)
if user_persisted_and_valid?
# Deleting the session cookie with the previous app domain, the one without the leading dot.
# This should fix the double cookie scenario
# NOTE: this code is a hotfix, and shall be removed soon (around 2 weeks from deployment)
domain = Rails.env.production? ? ApplicationConfig["APP_DOMAIN"] : nil
if domain&.starts_with?(".")
domain_without_leading_dot = domain.gsub(/\A./, "")
cookies.delete(ApplicationConfig["SESSION_KEY"], domain: domain_without_leading_dot)
end
# Devise's Omniauthable does not automatically remember users
# see <https://github.com/heartcombo/devise/wiki/Omniauthable,-sign-out-action-and-rememberable>
remember_me(@user)

23
app/controllers/sessions_controller.rb
View file

@ -6,29 +6,28 @@ class SessionsController < Devise::SessionsController
end
def destroy
delete_legacy_cookie
# Let's say goodbye to all the cookies when someone signs out.
cookies.clear(domain: cookie_domain)
delete_legacy_cookie
super
end
private
def cookie_domain
def legacy_cookie_domain
Rails.env.production? ? ApplicationConfig["APP_DOMAIN"] : nil
end
# NOTE: this code is a hotfix, and shall be removed soon (around 2 weeks from deployment)
def cookie_domain
".#{legacy_cookie_domain}"
end
# TODO: this code is a hotfix, we should remove it after 09/18/2020.
def delete_legacy_cookie
domain = cookie_domain
return unless domain&.starts_with?(".")
# Deleting the session cookie with the previous app domain, the one without the leading dot.
# This should fix the double cookie scenario
domain_without_leading_dot = domain.gsub(/\A./, "")
cookies.delete(ApplicationConfig["SESSION_KEY"], domain: domain_without_leading_dot)
# Deleting the session cookie with the legacy app domain, which does NOT include a preceding dot.
# This should fix the double cookie scenario.
cookies.delete(ApplicationConfig["SESSION_KEY"], domain: legacy_cookie_domain)
end
end