Merge branch 'sh-fix-otp-backup-cherry-pick' into 'master'

Merge branch 'sh-fix-otp-backup-invalidation-10-5' into 'master'

See merge request gitlab/gitlabhq!2349

3 files changed, 18 insertions, 0 deletions

diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb
index db8c362f125..2753f83c3cf 100644
--- a/app/controllers/concerns/authenticates_with_two_factor.rb
+++ b/app/controllers/concerns/authenticates_with_two_factor.rb
@@ -56,6 +56,7 @@ module AuthenticatesWithTwoFactor
       session.delete(:otp_user_id)
 
       remember_me(user) if user_params[:remember_me] == '1'
+      user.save!
       sign_in(user)
     else
       user.increment_failed_attempts!
diff --git a/changelogs/unreleased/sh-fix-otp-backup-code-invalidation.yml b/changelogs/unreleased/sh-fix-otp-backup-code-invalidation.yml
new file mode 100644
index 00000000000..cedb09c9a7a
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-otp-backup-code-invalidation.yml
@@ -0,0 +1,5 @@
+---
+title: Ensure that OTP backup codes are always invalidated
+merge_request:
+author:
+type: security
diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb
index 6ef235cf870..bc75dc5d19b 100644
--- a/spec/features/users/login_spec.rb
+++ b/spec/features/users/login_spec.rb
@@ -145,6 +145,18 @@ feature 'Login' do
             expect { enter_code(codes.sample) }
               .to change { user.reload.otp_backup_codes.size }.by(-1)
           end
+
+          it 'invalidates backup codes twice in a row' do
+            random_code = codes.delete(codes.sample)
+            expect { enter_code(random_code) }
+              .to change { user.reload.otp_backup_codes.size }.by(-1)
+
+            gitlab_sign_out
+            gitlab_sign_in(user)
+
+            expect { enter_code(codes.sample) }
+              .to change { user.reload.otp_backup_codes.size }.by(-1)
+          end
         end
 
         context 'with invalid code' do