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 --- app/controllers/auth/sessions_controller.rb | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'app/controllers/auth/sessions_controller.rb') diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 9c73b39e2..7afd09e10 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -58,16 +58,20 @@ class Auth::SessionsController < Devise::SessionsController protected def find_user - if session[:attempt_user_id] + if user_params[:email].present? + find_user_from_params + elsif session[:attempt_user_id] User.find_by(id: session[:attempt_user_id]) - else - user = User.authenticate_with_ldap(user_params) if Devise.ldap_authentication - user ||= User.authenticate_with_pam(user_params) if Devise.pam_authentication - user ||= User.find_for_authentication(email: user_params[:email]) - user end end + def find_user_from_params + user = User.authenticate_with_ldap(user_params) if Devise.ldap_authentication + user ||= User.authenticate_with_pam(user_params) if Devise.pam_authentication + user ||= User.find_for_authentication(email: user_params[:email]) + user + end + def user_params params.require(:user).permit(:email, :password, :otp_attempt, :sign_in_token_attempt, credential: {}) end -- cgit From 7283a5d3b94b655172744996ffa43ec80aff0e08 Mon Sep 17 00:00:00 2001 From: Truong Nguyen Date: Thu, 26 Aug 2021 23:51:22 +0900 Subject: Explicitly set userVerification to discoraged (#16545) --- app/controllers/auth/sessions_controller.rb | 5 ++++- .../two_factor_authentication/webauthn_credentials_controller.rb | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) (limited to 'app/controllers/auth/sessions_controller.rb') diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 7afd09e10..2c3d510cb 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -45,7 +45,10 @@ class Auth::SessionsController < Devise::SessionsController user = find_user if user&.webauthn_enabled? - options_for_get = WebAuthn::Credential.options_for_get(allow: user.webauthn_credentials.pluck(:external_id)) + options_for_get = WebAuthn::Credential.options_for_get( + allow: user.webauthn_credentials.pluck(:external_id), + user_verification: 'discouraged' + ) session[:webauthn_challenge] = options_for_get.challenge diff --git a/app/controllers/settings/two_factor_authentication/webauthn_credentials_controller.rb b/app/controllers/settings/two_factor_authentication/webauthn_credentials_controller.rb index 1c557092b..a50d30f06 100644 --- a/app/controllers/settings/two_factor_authentication/webauthn_credentials_controller.rb +++ b/app/controllers/settings/two_factor_authentication/webauthn_credentials_controller.rb @@ -21,7 +21,8 @@ module Settings display_name: current_user.account.username, id: current_user.webauthn_id, }, - exclude: current_user.webauthn_credentials.pluck(:external_id) + exclude: current_user.webauthn_credentials.pluck(:external_id), + authenticator_selection: { user_verification: 'discouraged' } ) session[:webauthn_challenge] = options_for_create.challenge -- cgit