From 7880671f3527b80de891053beaeae4a8a22c8c58 Mon Sep 17 00:00:00 2001 From: Kaylee Date: Tue, 2 May 2017 20:07:12 +0100 Subject: [PATCH] Add option to disable two factor auth in admin accounts panel. (#2584) * Add option to disable two factor auth in admin accounts panel. Closes #2578 * Add @mjankowski's suggestions. * Moves destroy actions behind User#disable_two_factor! * Adds spec coverage for Admin:TwoFactorAuthenticationsController and User#disable_two_factor! --- .../two_factor_authentications_controller.rb | 18 ++++++++++++++++++ app/models/user.rb | 6 ++++++ app/views/admin/accounts/show.html.haml | 2 ++ config/locales/en.yml | 1 + config/routes.rb | 4 ++++ ...o_factor_authentications_controller_spec.rb | 17 +++++++++++++++++ spec/models/user_spec.rb | 14 ++++++++++++++ 7 files changed, 62 insertions(+) create mode 100644 app/controllers/admin/two_factor_authentications_controller.rb create mode 100644 spec/controllers/admin/two_factor_authentications_controller_spec.rb diff --git a/app/controllers/admin/two_factor_authentications_controller.rb b/app/controllers/admin/two_factor_authentications_controller.rb new file mode 100644 index 0000000000..69c08f6059 --- /dev/null +++ b/app/controllers/admin/two_factor_authentications_controller.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Admin + class TwoFactorAuthenticationsController < BaseController + before_action :set_user + + def destroy + @user.disable_two_factor! + redirect_to admin_accounts_path + end + + private + + def set_user + @user = User.find(params[:user_id]) + end + end +end diff --git a/app/models/user.rb b/app/models/user.rb index f6e080d4e5..f8e8a2efa2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -56,6 +56,12 @@ class User < ApplicationRecord confirmed_at.present? end + def disable_two_factor! + self.otp_required_for_login = false + otp_backup_codes&.clear + save! + end + def send_devise_notification(notification, *args) devise_mailer.send(notification, self, *args).deliver_later end diff --git a/app/views/admin/accounts/show.html.haml b/app/views/admin/accounts/show.html.haml index 0b3348960b..1a9bd2c483 100644 --- a/app/views/admin/accounts/show.html.haml +++ b/app/views/admin/accounts/show.html.haml @@ -70,6 +70,8 @@ - if @account.local? %div{ style: 'float: right' } = link_to t('admin.accounts.reset_password'), admin_account_reset_path(@account.id), method: :create, class: 'button' + - if @account.user&.otp_required_for_login? + = link_to t('admin.accounts.disable_two_factor_authentication'), admin_user_two_factor_authentication_path(@account.user.id), method: :delete, class: 'button' %div{ style: 'float: left' } - if @account.silenced? diff --git a/config/locales/en.yml b/config/locales/en.yml index ccf231cd8f..26ecac7bcd 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -84,6 +84,7 @@ en: public: Public push_subscription_expires: PuSH subscription expires reset_password: Reset password + disable_two_factor_authentication: Disable 2FA salmon_url: Salmon URL show: created_reports: Reports created by this account diff --git a/config/routes.rb b/config/routes.rb index 9adaffcafd..1492f99fbd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -89,6 +89,10 @@ Rails.application.routes.draw do resource :suspension, only: [:create, :destroy] resource :confirmation, only: [:create] end + + resources :users, only: [] do + resource :two_factor_authentication, only: [:destroy] + end end get '/admin', to: redirect('/admin/settings', status: 302) diff --git a/spec/controllers/admin/two_factor_authentications_controller_spec.rb b/spec/controllers/admin/two_factor_authentications_controller_spec.rb new file mode 100644 index 0000000000..69f26039a0 --- /dev/null +++ b/spec/controllers/admin/two_factor_authentications_controller_spec.rb @@ -0,0 +1,17 @@ +require 'rails_helper' + +describe Admin::TwoFactorAuthenticationsController do + render_views + + let(:user) { Fabricate(:user) } + before do + sign_in Fabricate(:user, admin: true), scope: :user + end + + describe 'DELETE #destroy' do + it 'redirects to admin accounts page' do + delete :destroy, params: { user_id: user.id } + expect(response).to redirect_to(admin_accounts_path) + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a86bf4ece6..fffd92e3d8 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -126,6 +126,20 @@ RSpec.describe User, type: :model do end end + describe '#disable_two_factor!' do + it 'sets otp_required_for_login to false' do + user = Fabricate.build(:user, otp_required_for_login: true) + user.disable_two_factor! + expect(user.otp_required_for_login).to be false + end + + it 'clears otp_backup_codes' do + user = Fabricate.build(:user, otp_backup_codes: %w[dummy dummy]) + user.disable_two_factor! + expect(user.otp_backup_codes.empty?).to be true + end + end + describe 'whitelist' do around(:each) do |example| old_whitelist = Rails.configuration.x.email_whitelist