Skip to content

Commit

Permalink
feature: allow users to remove authenticator app from 2FA methods
Browse files Browse the repository at this point in the history
  • Loading branch information
santiagorodriguez96 committed Sep 11, 2023
1 parent 09265f2 commit e883e34
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 1 deletion.
1 change: 1 addition & 0 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ RSpec/AnyInstance:
- 'spec/controllers/api/v1/media_controller_spec.rb'
- 'spec/controllers/auth/sessions_controller_spec.rb'
- 'spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb'
- 'spec/controllers/settings/two_factor_authentication/otp_authentication_controller_spec.rb'
- 'spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb'
- 'spec/controllers/settings/two_factor_authentication/webauthn_credentials_controller_spec.rb'
- 'spec/lib/request_spec.rb'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ def create
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
Expand Down
9 changes: 9 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,15 @@ def two_factor_enabled?
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,22 @@
- if current_user.otp_enabled?
%td
= table_link_to 'pencil', t('two_factor_authentication.edit'), settings_otp_authentication_path, method: :post
%td
= table_link_to 'times', t('otp_authentication.remove'), settings_otp_authentication_path, method: :delete, data: { confirm: t('otp_authentication.remove_confirmation') }
- else
%td
= table_link_to 'plus', t('two_factor_authentication.add'), settings_otp_authentication_path
%td
%tr
%td= t('two_factor_authentication.webauthn')
- if current_user.webauthn_enabled?
%td
= table_link_to 'pencil', t('two_factor_authentication.edit'), settings_webauthn_credentials_path, method: :get
%td
- else
%td
= table_link_to 'key', t('two_factor_authentication.add'), new_settings_webauthn_credential_path, method: :get
%td

- if current_user.two_factor_enabled?
%hr.spacer/
Expand Down
2 changes: 2 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1497,6 +1497,8 @@ en:
enable: Enable
instructions_html: "<strong>Scan this QR code into Google Authenticator or a similar TOTP app on your phone</strong>. From now on, that app will generate tokens that you will have to enter when logging in."
manual_instructions: 'If you can''t scan the QR code and need to enter it manually, here is the plain-text secret:'
remove: Remove
remove_confirmation: Are you sure you want to remove your authenticator app from your two-factor authentication methods?
setup: Set up
wrong_code: The entered code was invalid! Are server time and device time correct?
pagination:
Expand Down
2 changes: 1 addition & 1 deletion config/routes/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
end
end

resource :otp_authentication, only: [:show, :create], controller: 'two_factor_authentication/otp_authentication'
resource :otp_authentication, only: [:show, :create, :destroy], controller: 'two_factor_authentication/otp_authentication'

resources :webauthn_credentials, only: [:index, :new, :create, :destroy],
path: 'security_keys',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,4 +119,32 @@
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

it 'calls #disable_otp_login! for the current user' do
expect_any_instance_of(User).to receive(:disable_otp_login!)

delete :destroy
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
46 changes: 46 additions & 0 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,52 @@
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)
Expand Down

0 comments on commit e883e34

Please sign in to comment.