Merge branch 'ce-remove-already-signed-in' into 'master'

Remove "You are already signed in" banner

See merge request gitlab-org/gitlab-ce!27377

4 files changed, 30 insertions, 10 deletions

diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb
index d5c4712bd78..4926062f9ca 100644
--- a/app/controllers/concerns/authenticates_with_two_factor.rb
+++ b/app/controllers/concerns/authenticates_with_two_factor.rb
@@ -8,13 +8,6 @@
 module AuthenticatesWithTwoFactor
   extend ActiveSupport::Concern
 
-  included do
-    # This action comes from DeviseController, but because we call `sign_in`
-    # manually, not skipping this action would cause a "You are already signed
-    # in." error message to be shown upon successful login.
-    skip_before_action :require_no_authentication, only: [:create], raise: false
-  end
-
   # Store the user's ID in the session for later retrieval and render the
   # two factor code prompt
   #
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 6943795e8ac..6fea61cf45d 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -8,6 +8,8 @@ class SessionsController < Devise::SessionsController
   include Recaptcha::Verify
 
   skip_before_action :check_two_factor_requirement, only: [:destroy]
+  # replaced with :require_no_authentication_without_flash
+  skip_before_action :require_no_authentication, only: [:new, :create]
 
   prepend_before_action :check_initial_setup, only: [:new]
   prepend_before_action :authenticate_with_two_factor,
@@ -15,6 +17,8 @@ class SessionsController < Devise::SessionsController
   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]
+
   before_action :auto_sign_in_with_provider, only: [:new]
   before_action :load_recaptcha
 
@@ -54,6 +58,14 @@ class SessionsController < Devise::SessionsController
 
   private
 
+  def require_no_authentication_without_flash
+    require_no_authentication
+
+    if flash[:alert] == I18n.t('devise.failure.already_authenticated')
+      flash[:alert] = nil
+    end
+  end
+
   def captcha_enabled?
     request.headers[CAPTCHA_HEADER] && Gitlab::Recaptcha.enabled?
   end
diff --git a/changelogs/unreleased/ce-remove-already-signed-in.yml b/changelogs/unreleased/ce-remove-already-signed-in.yml
new file mode 100644
index 00000000000..70bed136ced
--- /dev/null
+++ b/changelogs/unreleased/ce-remove-already-signed-in.yml
@@ -0,0 +1,5 @@
+---
+title: Remove "You are already signed in" banner
+merge_request: 27377
+author:
+type: other
diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb
index 9d5780d29b0..efba303033b 100644
--- a/spec/features/users/login_spec.rb
+++ b/spec/features/users/login_spec.rb
@@ -137,7 +137,7 @@ describe 'Login' do
 
         enter_code(user.current_otp)
 
-        expect(page).not_to have_content('You are already signed in.')
+        expect(page).not_to have_content(I18n.t('devise.failure.already_authenticated'))
       end
 
       context 'using one-time code' do
@@ -317,7 +317,17 @@ describe 'Login' do
         gitlab_sign_in(user)
 
         expect(current_path).to eq root_path
-        expect(page).not_to have_content('You are already signed in.')
+        expect(page).not_to have_content(I18n.t('devise.failure.already_authenticated'))
+      end
+
+      it 'does not show already signed in message when opening sign in page after login' do
+        expect(authentication_metrics)
+          .to increment(:user_authenticated_counter)
+
+        gitlab_sign_in(user)
+        visit new_user_session_path
+
+        expect(page).not_to have_content(I18n.t('devise.failure.already_authenticated'))
       end
     end
 
@@ -579,7 +589,7 @@ describe 'Login' do
       click_button 'Accept terms'
 
       expect(current_path).to eq(root_path)
-      expect(page).not_to have_content('You are already signed in.')
+      expect(page).not_to have_content(I18n.t('devise.failure.already_authenticated'))
     end
 
     it 'does not ask for terms when the user already accepted them' do