diff --git a/app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb b/app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb index ca8d46afe48..0d2f43abb23 100644 --- a/app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb +++ b/app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb @@ -20,6 +20,12 @@ module Settings redirect_to new_settings_two_factor_authentication_confirmation_path end + def destroy + current_user.disable_otp_login! + + redirect_to settings_two_factor_authentication_methods_path + end + private def verify_otp_not_enabled diff --git a/app/models/user.rb b/app/models/user.rb index 8e0785e7fdd..304291efb0c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -262,6 +262,15 @@ class User < ApplicationRecord otp_required_for_login? || webauthn_credentials.any? end + def disable_otp_login! + return unless otp_required_for_login? + + self.otp_required_for_login = false + self.otp_secret = nil + + save! + end + def disable_two_factor! self.otp_required_for_login = false self.otp_secret = nil diff --git a/app/views/settings/two_factor_authentication_methods/index.html.haml b/app/views/settings/two_factor_authentication_methods/index.html.haml index cf2113132f6..e0c7fa7fd93 100644 --- a/app/views/settings/two_factor_authentication_methods/index.html.haml +++ b/app/views/settings/two_factor_authentication_methods/index.html.haml @@ -23,6 +23,7 @@ - if current_user.otp_enabled? %td = table_link_to 'edit', t('two_factor_authentication.edit'), settings_otp_authentication_path, method: :get + = table_link_to 'delete', t('otp_authentication.delete'), settings_otp_authentication_path, method: :delete, data: { confirm: t('otp_authentication.delete_confirmation') } - else %td = table_link_to 'add', t('two_factor_authentication.add'), settings_otp_authentication_path, method: :get diff --git a/config/locales/en.yml b/config/locales/en.yml index c93c8f246cf..98b54a8a778 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1716,6 +1716,8 @@ en: unit: '' otp_authentication: code_hint: Enter the code generated by your authenticator app to confirm + delete: Delete + delete_confirmation: Are you sure you want to delete your authenticator app from your two-factor authentication methods? description_html: If you enable two-factor authentication using an authenticator app, logging in will require you to be in possession of your phone, which will generate tokens for you to enter. enable: Enable instructions_html: "Scan this QR code into Google Authenticator or a similar TOTP app on your phone. From now on, that app will generate tokens that you will have to enter when logging in." diff --git a/config/routes/settings.rb b/config/routes/settings.rb index cefa24316db..8d595858654 100644 --- a/config/routes/settings.rb +++ b/config/routes/settings.rb @@ -38,7 +38,7 @@ namespace :settings do end scope module: :two_factor_authentication do - resource :otp_authentication, only: [:show, :create], controller: :otp_authentication + resource :otp_authentication, only: [:show, :create, :destroy], controller: :otp_authentication resources :webauthn_credentials, only: [:index, :new, :create, :destroy], path: 'security_keys' do collection do diff --git a/spec/controllers/settings/two_factor_authentication/otp_authentication_controller_spec.rb b/spec/controllers/settings/two_factor_authentication/otp_authentication_controller_spec.rb index a03c4a4adb2..87863574261 100644 --- a/spec/controllers/settings/two_factor_authentication/otp_authentication_controller_spec.rb +++ b/spec/controllers/settings/two_factor_authentication/otp_authentication_controller_spec.rb @@ -96,4 +96,26 @@ RSpec.describe Settings::TwoFactorAuthentication::OtpAuthenticationController do end end end + + describe 'GET #destroy' do + context 'when signed in' do + before do + sign_in user, scope: :user + end + + it 'redirects to two factor authentication methods list page' do + delete :destroy + + expect(response).to redirect_to settings_two_factor_authentication_methods_path + end + end + + context 'when not signed in' do + it 'redirects to login' do + delete :destroy + + expect(response).to redirect_to new_user_session_path + end + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a9ab15a956e..cb9440d9cbd 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -235,6 +235,52 @@ RSpec.describe User do end end + describe '#disable_otp_login!' do + describe 'when user has OTP enabled' do + let(:user) do + Fabricate( + :user, + otp_required_for_login: true, + otp_secret: 'oldotpcode' + ) + end + + it 'saves false for otp_required_for_login' do + user.disable_otp_login! + + expect(user.reload.otp_required_for_login).to be false + end + + it 'saves nil for otp_secret' do + user.disable_otp_login! + + expect(user.reload.otp_secret).to be_nil + end + end + + describe 'when user does not have OTP enabled' do + let(:user) do + Fabricate( + :user, + otp_required_for_login: false, + otp_secret: nil + ) + end + + it 'does not change for otp_required_for_login' do + user.disable_otp_login! + + expect(user.reload.otp_required_for_login).to be false + end + + it 'does not change for otp_secret' do + user.disable_otp_login! + + expect(user.reload.otp_secret).to be_nil + end + end + end + describe '#disable_two_factor!' do it 'saves false for otp_required_for_login' do user = Fabricate.build(:user, otp_required_for_login: true)