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

3 files changed, 14 insertions, 3 deletions

diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb
index 4926062f9ca..8c8f0b3a22e 100644
--- a/app/controllers/concerns/authenticates_with_two_factor.rb
+++ b/app/controllers/concerns/authenticates_with_two_factor.rb
@@ -55,7 +55,7 @@ module AuthenticatesWithTwoFactor
 
       remember_me(user) if user_params[:remember_me] == '1'
       user.save!
-      sign_in(user, message: :two_factor_authenticated)
+      sign_in(user, message: :two_factor_authenticated, event: :authentication)
     else
       user.increment_failed_attempts!
       Gitlab::AppLogger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=OTP")
@@ -72,7 +72,7 @@ module AuthenticatesWithTwoFactor
       session.delete(:challenge)
 
       remember_me(user) if user_params[:remember_me] == '1'
-      sign_in(user, message: :two_factor_authenticated)
+      sign_in(user, message: :two_factor_authenticated, event: :authentication)
     else
       user.increment_failed_attempts!
       Gitlab::AppLogger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=U2F")
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index 2a8dd997d04..b1efa767154 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -139,7 +139,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
       if user.two_factor_enabled? && !auth_user.bypass_two_factor?
         prompt_for_two_factor(user)
       else
-        sign_in_and_redirect(user)
+        sign_in_and_redirect(user, event: :authentication)
       end
     else
       fail_login(user)
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