Automatic merge of gitlab-org/gitlab-ce master

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
   #

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

changelogs/unreleased/ce-remove-already-signed-in.yml

+---
+title: Remove "You are already signed in" banner
+merge_request: 27377
+author:
+type: other

doc/development/contributing/issue_workflow.md

@@ -340,13 +340,14 @@ addressed.
 
 In order to track things that can be improved in GitLab's codebase,
 we use the ~"technical debt" label in [GitLab's issue tracker][ce-tracker].
-For user experience improvements, we use the ~"UX debt" label.
+For missed user experience requirements, we use the ~"UX debt" label.
 
 These labels should be added to issues that describe things that can be improved,
 shortcuts that have been taken, features that need additional attention, and all
 other things that have been left behind due to high velocity of development.
 For example, code that needs refactoring should use the ~"technical debt" label,
-user experience refinements should use the ~"UX debt" label.
+something that didn't ship according to our Design System guidelines should
+use the ~"UX debt" label.
 
 Everyone can create an issue, though you may need to ask for adding a specific
 label, if you do not have permissions to do it by yourself. Additional labels

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