From 94bcf453219da73015cc977835717516b9dc0a67 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 25 Aug 2021 22:52:41 +0200 Subject: Fix authentication failures after going halfway through a sign-in attempt (#16607) * Add tests * Add security-related tests My first (unpublished) attempt at fixing the issues introduced (extremely hard-to-exploit) security vulnerabilities, addressing them in a test. * Fix authentication failures after going halfway through a sign-in attempt * Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious --- .../sign_in_token_authentication_concern.rb | 20 ++++++++++++-------- .../concerns/two_factor_authentication_concern.rb | 22 +++++++++++++--------- 2 files changed, 25 insertions(+), 17 deletions(-) (limited to 'app/controllers/concerns') diff --git a/app/controllers/concerns/sign_in_token_authentication_concern.rb b/app/controllers/concerns/sign_in_token_authentication_concern.rb index cbee84569..384c5c50c 100644 --- a/app/controllers/concerns/sign_in_token_authentication_concern.rb +++ b/app/controllers/concerns/sign_in_token_authentication_concern.rb @@ -16,14 +16,18 @@ module SignInTokenAuthenticationConcern end def authenticate_with_sign_in_token - user = self.resource = find_user - - if user.present? && session[:attempt_user_id].present? && session[:attempt_user_updated_at] != user.updated_at.to_s - restart_session - elsif user_params.key?(:sign_in_token_attempt) && session[:attempt_user_id] - authenticate_with_sign_in_token_attempt(user) - elsif user.present? && user.external_or_valid_password?(user_params[:password]) - prompt_for_sign_in_token(user) + if user_params[:email].present? + user = self.resource = find_user_from_params + prompt_for_sign_in_token(user) if user&.external_or_valid_password?(user_params[:password]) + elsif session[:attempt_user_id] + user = self.resource = User.find_by(id: session[:attempt_user_id]) + return if user.nil? + + if session[:attempt_user_updated_at] != user.updated_at.to_s + restart_session + elsif user_params.key?(:sign_in_token_attempt) + authenticate_with_sign_in_token_attempt(user) + end end end diff --git a/app/controllers/concerns/two_factor_authentication_concern.rb b/app/controllers/concerns/two_factor_authentication_concern.rb index 909ab7717..2583d324b 100644 --- a/app/controllers/concerns/two_factor_authentication_concern.rb +++ b/app/controllers/concerns/two_factor_authentication_concern.rb @@ -35,16 +35,20 @@ module TwoFactorAuthenticationConcern end def authenticate_with_two_factor - user = self.resource = find_user + if user_params[:email].present? + user = self.resource = find_user_from_params + prompt_for_two_factor(user) if user&.external_or_valid_password?(user_params[:password]) + elsif session[:attempt_user_id] + user = self.resource = User.find_by(id: session[:attempt_user_id]) + return if user.nil? - if user.present? && session[:attempt_user_id].present? && session[:attempt_user_updated_at] != user.updated_at.to_s - restart_session - elsif user.webauthn_enabled? && user_params.key?(:credential) && session[:attempt_user_id] - authenticate_with_two_factor_via_webauthn(user) - elsif user_params.key?(:otp_attempt) && session[:attempt_user_id] - authenticate_with_two_factor_via_otp(user) - elsif user.present? && user.external_or_valid_password?(user_params[:password]) - prompt_for_two_factor(user) + if session[:attempt_user_updated_at] != user.updated_at.to_s + restart_session + elsif user.webauthn_enabled? && user_params.key?(:credential) + authenticate_with_two_factor_via_webauthn(user) + elsif user_params.key?(:otp_attempt) + authenticate_with_two_factor_via_otp(user) + end end end -- cgit