From a9707e8cf70487a52efbe43ffe72c9e995f5cdea Mon Sep 17 00:00:00 2001 From: George Thomas Date: Wed, 27 Feb 2019 13:11:14 +0530 Subject: Rewrite `if:` argument in before_action and alike when `only:` is also used Closes #55564 This is first discovered in #54739 (comment 122609857) that if both if: and only: are used in a before_action or after_action or alike, if: is completely ignored. --- app/controllers/sessions_controller.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'app/controllers/sessions_controller.rb') diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index a841859621e..7604b31467a 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -13,18 +13,17 @@ class SessionsController < Devise::SessionsController prepend_before_action :check_initial_setup, only: [:new] prepend_before_action :authenticate_with_two_factor, - if: :two_factor_enabled?, only: [:create] + if: -> { action_name == 'create' && two_factor_enabled? } prepend_before_action :check_captcha, only: [:create] prepend_before_action :store_redirect_uri, only: [:new] prepend_before_action :ldap_servers, only: [:new, :create] prepend_before_action :require_no_authentication_without_flash, only: [:new, :create] - prepend_before_action :ensure_password_authentication_enabled!, if: :password_based_login?, only: [:create] + prepend_before_action :ensure_password_authentication_enabled!, if: -> { action_name == 'create' && password_based_login? } before_action :auto_sign_in_with_provider, only: [:new] before_action :load_recaptcha - after_action :log_failed_login, only: [:new], if: :failed_login? - + after_action :log_failed_login, if: -> { action_name == 'new' && failed_login? } helper_method :captcha_enabled? CAPTCHA_HEADER = 'X-GitLab-Show-Login-Captcha'.freeze -- cgit v1.2.1 From 929b403d21308cb7843aa474bfba599345b706b4 Mon Sep 17 00:00:00 2001 From: Imre Farkas Date: Fri, 26 Jul 2019 07:05:50 +0000 Subject: Ensure Warden triggers after_authentication callback By not triggering the callback: - ActiveSession lookup keys are not cleaned - Devise also misses its hook related to session cleanup --- app/controllers/sessions_controller.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'app/controllers/sessions_controller.rb') diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 7604b31467a..1880bead3ee 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -26,6 +26,17 @@ class SessionsController < Devise::SessionsController after_action :log_failed_login, if: -> { action_name == 'new' && failed_login? } helper_method :captcha_enabled? + # protect_from_forgery is already prepended in ApplicationController but + # authenticate_with_two_factor which signs in the user is prepended before + # that here. + # We need to make sure CSRF token is verified before authenticating the user + # because Devise.clean_up_csrf_token_on_authentication is set to true by + # default to avoid CSRF token fixation attacks. Authenticating the user first + # would cause the CSRF token to be cleared and then + # RequestForgeryProtection#verify_authenticity_token would fail because of + # token mismatch. + protect_from_forgery with: :exception, prepend: true + CAPTCHA_HEADER = 'X-GitLab-Show-Login-Captcha'.freeze def new -- cgit v1.2.1