Fix reviving revoked sessions and invalidating login (#16943)
Up until now, we have used Devise's Rememberable mechanism to re-log users after the end of their browser sessions. This mechanism relies on a signed cookie containing a token. That token was stored on the user's record, meaning it was shared across all logged in browsers, meaning truly revoking a browser's ability to auto-log-in involves revoking the token itself, and revoking access from *all* logged-in browsers. We had a session mechanism that dynamically checks whether a user's session has been disabled, and would log out the user if so. However, this would only clear a session being actively used, and a new one could be respawned with the `remember_user_token` cookie. In practice, this caused two issues: - sessions could be revived after being closed from /auth/edit (security issue) - auto-log-in would be disabled for *all* browsers after logging out from one of them This PR removes the `remember_token` mechanism and treats the `_session_id` cookie/token as a browser-specific `remember_token`, fixing both issues.
This commit is contained in:
		| @@ -10,7 +10,6 @@ class Auth::PasswordsController < Devise::PasswordsController | ||||
|     super do |resource| | ||||
|       if resource.errors.empty? | ||||
|         resource.session_activations.destroy_all | ||||
|         resource.forget_me! | ||||
|       end | ||||
|     end | ||||
|   end | ||||
|   | ||||
| @@ -1,7 +1,6 @@ | ||||
| # frozen_string_literal: true | ||||
|  | ||||
| class Auth::RegistrationsController < Devise::RegistrationsController | ||||
|   include Devise::Controllers::Rememberable | ||||
|   include RegistrationSpamConcern | ||||
|  | ||||
|   layout :determine_layout | ||||
| @@ -30,8 +29,6 @@ class Auth::RegistrationsController < Devise::RegistrationsController | ||||
|     super do |resource| | ||||
|       if resource.saved_change_to_encrypted_password? | ||||
|         resource.clear_other_sessions(current_session.session_id) | ||||
|         resource.forget_me! | ||||
|         remember_me(resource) | ||||
|       end | ||||
|     end | ||||
|   end | ||||
|   | ||||
| @@ -1,8 +1,6 @@ | ||||
| # frozen_string_literal: true | ||||
|  | ||||
| class Auth::SessionsController < Devise::SessionsController | ||||
|   include Devise::Controllers::Rememberable | ||||
|  | ||||
|   layout 'auth' | ||||
|  | ||||
|   skip_before_action :require_no_authentication, only: [:create] | ||||
| @@ -150,7 +148,6 @@ class Auth::SessionsController < Devise::SessionsController | ||||
|     clear_attempt_from_session | ||||
|  | ||||
|     user.update_sign_in!(request, new_sign_in: true) | ||||
|     remember_me(user) | ||||
|     sign_in(user) | ||||
|     flash.delete(:notice) | ||||
|  | ||||
|   | ||||
| @@ -64,7 +64,7 @@ class User < ApplicationRecord | ||||
|   devise :two_factor_backupable, | ||||
|          otp_number_of_backup_codes: 10 | ||||
|  | ||||
|   devise :registerable, :recoverable, :rememberable, :validatable, | ||||
|   devise :registerable, :recoverable, :validatable, | ||||
|          :confirmable | ||||
|  | ||||
|   include Omniauthable | ||||
|   | ||||
| @@ -1,3 +1,5 @@ | ||||
| require 'devise/strategies/authenticatable' | ||||
|  | ||||
| Warden::Manager.after_set_user except: :fetch do |user, warden| | ||||
|   if user.session_active?(warden.cookies.signed['_session_id'] || warden.raw_session['auth_id']) | ||||
|     session_id = warden.cookies.signed['_session_id'] || warden.raw_session['auth_id'] | ||||
| @@ -72,17 +74,48 @@ module Devise | ||||
|   mattr_accessor :ldap_uid_conversion_replace | ||||
|   @@ldap_uid_conversion_replace = nil | ||||
|  | ||||
|   class Strategies::PamAuthenticatable | ||||
|     def valid? | ||||
|       super && ::Devise.pam_authentication | ||||
|   module Strategies | ||||
|     class PamAuthenticatable | ||||
|       def valid? | ||||
|         super && ::Devise.pam_authentication | ||||
|       end | ||||
|     end | ||||
|  | ||||
|     class SessionActivationRememberable < Authenticatable | ||||
|       def valid? | ||||
|         @session_cookie = nil | ||||
|         session_cookie.present? | ||||
|       end | ||||
|  | ||||
|       def authenticate! | ||||
|         resource = SessionActivation.find_by(session_id: session_cookie)&.user | ||||
|  | ||||
|         unless resource | ||||
|           cookies.delete('_session_id') | ||||
|           return pass | ||||
|         end | ||||
|  | ||||
|         if validate(resource) | ||||
|           success!(resource) | ||||
|         end | ||||
|       end | ||||
|  | ||||
|       private | ||||
|  | ||||
|       def session_cookie | ||||
|         @session_cookie ||= cookies.signed['_session_id'] | ||||
|       end | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  | ||||
| Warden::Strategies.add(:session_activation_rememberable, Devise::Strategies::SessionActivationRememberable) | ||||
|  | ||||
| Devise.setup do |config| | ||||
|   config.warden do |manager| | ||||
|     manager.default_strategies(scope: :user).unshift :two_factor_ldap_authenticatable if Devise.ldap_authentication | ||||
|     manager.default_strategies(scope: :user).unshift :two_factor_pam_authenticatable  if Devise.pam_authentication | ||||
|     manager.default_strategies(scope: :user).unshift :session_activation_rememberable | ||||
|     manager.default_strategies(scope: :user).unshift :two_factor_authenticatable | ||||
|     manager.default_strategies(scope: :user).unshift :two_factor_backupable | ||||
|   end | ||||
|   | ||||
		Reference in New Issue
	
	Block a user