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

2 files changed, 29 insertions, 10 deletions

diff --git a/spec/features/oauth_login_spec.rb b/spec/features/oauth_login_spec.rb
index 86331728f88..54705cd51b0 100644
--- a/spec/features/oauth_login_spec.rb
+++ b/spec/features/oauth_login_spec.rb
@@ -34,6 +34,7 @@ describe 'OAuth Login', :js, :allow_forgery_protection do
 
       before do
         stub_omniauth_config(provider)
+        expect(ActiveSession).to receive(:cleanup).with(user).at_least(:once).and_call_original
       end
 
       context 'when two-factor authentication is disabled' do
diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb
index efba303033b..1ea88010c89 100644
--- a/spec/features/users/login_spec.rb
+++ b/spec/features/users/login_spec.rb
@@ -132,7 +132,6 @@ describe 'Login' do
       it 'does not show a "You are already signed in." error message' do
         expect(authentication_metrics)
           .to increment(:user_authenticated_counter)
-          .and increment(:user_session_override_counter)
           .and increment(:user_two_factor_authenticated_counter)
 
         enter_code(user.current_otp)
@@ -144,7 +143,6 @@ describe 'Login' do
         it 'allows login with valid code' do
           expect(authentication_metrics)
             .to increment(:user_authenticated_counter)
-            .and increment(:user_session_override_counter)
             .and increment(:user_two_factor_authenticated_counter)
 
           enter_code(user.current_otp)
@@ -170,7 +168,6 @@ describe 'Login' do
         it 'allows login with invalid code, then valid code' do
           expect(authentication_metrics)
             .to increment(:user_authenticated_counter)
-            .and increment(:user_session_override_counter)
             .and increment(:user_two_factor_authenticated_counter)
 
           enter_code('foo')
@@ -179,6 +176,15 @@ describe 'Login' do
           enter_code(user.current_otp)
           expect(current_path).to eq root_path
         end
+
+        it 'triggers ActiveSession.cleanup for the user' do
+          expect(authentication_metrics)
+            .to increment(:user_authenticated_counter)
+            .and increment(:user_two_factor_authenticated_counter)
+          expect(ActiveSession).to receive(:cleanup).with(user).once.and_call_original
+
+          enter_code(user.current_otp)
+        end
       end
 
       context 'using backup code' do
@@ -195,7 +201,6 @@ describe 'Login' do
           it 'allows login' do
             expect(authentication_metrics)
               .to increment(:user_authenticated_counter)
-              .and increment(:user_session_override_counter)
               .and increment(:user_two_factor_authenticated_counter)
 
             enter_code(codes.sample)
@@ -206,7 +211,6 @@ describe 'Login' do
           it 'invalidates the used code' do
             expect(authentication_metrics)
               .to increment(:user_authenticated_counter)
-              .and increment(:user_session_override_counter)
               .and increment(:user_two_factor_authenticated_counter)
 
             expect { enter_code(codes.sample) }
@@ -216,7 +220,6 @@ describe 'Login' do
           it 'invalidates backup codes twice in a row' do
             expect(authentication_metrics)
               .to increment(:user_authenticated_counter).twice
-              .and increment(:user_session_override_counter).twice
               .and increment(:user_two_factor_authenticated_counter).twice
               .and increment(:user_session_destroyed_counter)
 
@@ -230,6 +233,15 @@ describe 'Login' do
             expect { enter_code(codes.sample) }
               .to change { user.reload.otp_backup_codes.size }.by(-1)
           end
+
+          it 'triggers ActiveSession.cleanup for the user' do
+            expect(authentication_metrics)
+              .to increment(:user_authenticated_counter)
+                    .and increment(:user_two_factor_authenticated_counter)
+            expect(ActiveSession).to receive(:cleanup).with(user).once.and_call_original
+
+            enter_code(codes.sample)
+          end
         end
 
         context 'with invalid code' do
@@ -274,7 +286,7 @@ describe 'Login' do
 
           expect(authentication_metrics)
             .to increment(:user_authenticated_counter)
-            .and increment(:user_session_override_counter)
+          expect(ActiveSession).to receive(:cleanup).with(user).once.and_call_original
 
           sign_in_using_saml!
 
@@ -287,8 +299,8 @@ describe 'Login' do
         it 'shows 2FA prompt after OAuth login' do
           expect(authentication_metrics)
             .to increment(:user_authenticated_counter)
-            .and increment(:user_session_override_counter)
             .and increment(:user_two_factor_authenticated_counter)
+          expect(ActiveSession).to receive(:cleanup).with(user).once.and_call_original
 
           sign_in_using_saml!
 
@@ -329,6 +341,14 @@ describe 'Login' do
 
         expect(page).not_to have_content(I18n.t('devise.failure.already_authenticated'))
       end
+
+      it 'triggers ActiveSession.cleanup for the user' do
+        expect(authentication_metrics)
+          .to increment(:user_authenticated_counter)
+        expect(ActiveSession).to receive(:cleanup).with(user).once.and_call_original
+
+        gitlab_sign_in(user)
+      end
     end
 
     context 'with invalid username and password' do
@@ -649,7 +669,6 @@ describe 'Login' do
         it 'asks the user to accept the terms' do
           expect(authentication_metrics)
             .to increment(:user_authenticated_counter)
-            .and increment(:user_session_override_counter)
             .and increment(:user_two_factor_authenticated_counter)
 
           visit new_user_session_path
@@ -708,7 +727,6 @@ describe 'Login' do
       it 'asks the user to accept the terms before setting an email' do
         expect(authentication_metrics)
           .to increment(:user_authenticated_counter)
-          .and increment(:user_session_override_counter)
 
         gitlab_sign_in_via('saml', user, 'my-uid')