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:
parent
a19aec0f48
commit
b6b19419e2
5 changed files with 37 additions and 10 deletions
|
@ -10,7 +10,6 @@ class Auth::PasswordsController < Devise::PasswordsController
|
||||||
super do |resource|
|
super do |resource|
|
||||||
if resource.errors.empty?
|
if resource.errors.empty?
|
||||||
resource.session_activations.destroy_all
|
resource.session_activations.destroy_all
|
||||||
resource.forget_me!
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -1,7 +1,6 @@
|
||||||
# frozen_string_literal: true
|
# frozen_string_literal: true
|
||||||
|
|
||||||
class Auth::RegistrationsController < Devise::RegistrationsController
|
class Auth::RegistrationsController < Devise::RegistrationsController
|
||||||
include Devise::Controllers::Rememberable
|
|
||||||
include RegistrationSpamConcern
|
include RegistrationSpamConcern
|
||||||
|
|
||||||
layout :determine_layout
|
layout :determine_layout
|
||||||
|
@ -30,8 +29,6 @@ class Auth::RegistrationsController < Devise::RegistrationsController
|
||||||
super do |resource|
|
super do |resource|
|
||||||
if resource.saved_change_to_encrypted_password?
|
if resource.saved_change_to_encrypted_password?
|
||||||
resource.clear_other_sessions(current_session.session_id)
|
resource.clear_other_sessions(current_session.session_id)
|
||||||
resource.forget_me!
|
|
||||||
remember_me(resource)
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -1,8 +1,6 @@
|
||||||
# frozen_string_literal: true
|
# frozen_string_literal: true
|
||||||
|
|
||||||
class Auth::SessionsController < Devise::SessionsController
|
class Auth::SessionsController < Devise::SessionsController
|
||||||
include Devise::Controllers::Rememberable
|
|
||||||
|
|
||||||
layout 'auth'
|
layout 'auth'
|
||||||
|
|
||||||
skip_before_action :require_no_authentication, only: [:create]
|
skip_before_action :require_no_authentication, only: [:create]
|
||||||
|
|
|
@ -63,7 +63,7 @@ class User < ApplicationRecord
|
||||||
devise :two_factor_backupable,
|
devise :two_factor_backupable,
|
||||||
otp_number_of_backup_codes: 10
|
otp_number_of_backup_codes: 10
|
||||||
|
|
||||||
devise :registerable, :recoverable, :rememberable, :validatable,
|
devise :registerable, :recoverable, :validatable,
|
||||||
:confirmable
|
:confirmable
|
||||||
|
|
||||||
include Omniauthable
|
include Omniauthable
|
||||||
|
|
|
@ -1,3 +1,5 @@
|
||||||
|
require 'devise/strategies/authenticatable'
|
||||||
|
|
||||||
Warden::Manager.after_set_user except: :fetch do |user, warden|
|
Warden::Manager.after_set_user except: :fetch do |user, warden|
|
||||||
if user.session_active?(warden.cookies.signed['_session_id'] || warden.raw_session['auth_id'])
|
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']
|
session_id = warden.cookies.signed['_session_id'] || warden.raw_session['auth_id']
|
||||||
|
@ -72,17 +74,48 @@ module Devise
|
||||||
mattr_accessor :ldap_uid_conversion_replace
|
mattr_accessor :ldap_uid_conversion_replace
|
||||||
@@ldap_uid_conversion_replace = nil
|
@@ldap_uid_conversion_replace = nil
|
||||||
|
|
||||||
class Strategies::PamAuthenticatable
|
module Strategies
|
||||||
def valid?
|
class PamAuthenticatable
|
||||||
super && ::Devise.pam_authentication
|
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
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
Warden::Strategies.add(:session_activation_rememberable, Devise::Strategies::SessionActivationRememberable)
|
||||||
|
|
||||||
Devise.setup do |config|
|
Devise.setup do |config|
|
||||||
config.warden do |manager|
|
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_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 :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_authenticatable
|
||||||
manager.default_strategies(scope: :user).unshift :two_factor_backupable
|
manager.default_strategies(scope: :user).unshift :two_factor_backupable
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in a new issue