From 67dea31b0f83eb711bbd49a7b893d04ca16c56fa Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Fri, 21 Apr 2017 22:23:17 -0400 Subject: 2FA controller cleanup (#2296) * Add spec coverage for settings/two_factor_auth area * extract setup method for qr code * Move otp required check to before action * Merge method only used once * Remove duplicate view * Consolidate creation of @codes for backup * Move settings/2fq#recovery_codes to settings/recovery_codes#create * Rename settings/two_factor_auth#disable to #destroy * Add coverage for the otp required path on 2fa#show * Clean up the recovery codes list styles * Move settings/two_factor_auth to settings/two_factor_authentication * Reorganize the settings two factor auth area Updated to use a flow like: - settings/two_factor_authentication goes to a #show view which has a button either enable or disable 2fa on the account - the disable button turns off the otp requirement for the user - the enable button cycles the user secret and redirects to a confirmation page - the confirmation page is a #new view which shows the QR code for user - that page posts to #create which verifies the code, and creates the recovery codes - that create action shares a view with a recovery codes controller which can be used separately to reset codes if needed --- .../confirmations_controller_spec.rb | 46 +++++++++++++++ .../recovery_codes_controller_spec.rb | 25 ++++++++ .../two_factor_authentications_controller_spec.rb | 66 ++++++++++++++++++++++ 3 files changed, 137 insertions(+) create mode 100644 spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb create mode 100644 spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb create mode 100644 spec/controllers/settings/two_factor_authentications_controller_spec.rb (limited to 'spec/controllers/settings') diff --git a/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb b/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb new file mode 100644 index 000000000..bf555078e --- /dev/null +++ b/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Settings::TwoFactorAuthentication::ConfirmationsController do + render_views + + let(:user) { Fabricate(:user) } + before do + user.otp_secret = User.generate_otp_secret(32) + user.save! + + sign_in user, scope: :user + end + + describe 'GET #new' do + it 'returns http success' do + get :new + + expect(response).to have_http_status(:success) + expect(response).to render_template(:new) + end + end + + describe 'POST #create' do + describe 'when creation succeeds' do + it 'renders page with success' do + allow_any_instance_of(User).to receive(:validate_and_consume_otp!).with('123456').and_return(true) + + post :create, params: { form_two_factor_confirmation: { code: '123456' } } + expect(response).to have_http_status(:success) + expect(response).to render_template('settings/two_factor_authentication/recovery_codes/index') + end + end + + describe 'when creation fails' do + it 'renders the new view' do + allow_any_instance_of(User).to receive(:validate_and_consume_otp!).with('123456').and_return(false) + + post :create, params: { form_two_factor_confirmation: { code: '123456' } } + expect(response).to have_http_status(:success) + expect(response).to render_template(:new) + end + end + end +end diff --git a/spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb b/spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb new file mode 100644 index 000000000..3d6f5faab --- /dev/null +++ b/spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Settings::TwoFactorAuthentication::RecoveryCodesController do + render_views + + let(:user) { Fabricate(:user) } + before do + sign_in user, scope: :user + end + + describe 'POST #create' do + it 'updates the codes and shows them on a view' do + before = user.otp_backup_codes + + post :create + user.reload + + expect(user.otp_backup_codes).not_to eq(before) + expect(response).to have_http_status(:success) + expect(response).to render_template(:index) + end + end +end diff --git a/spec/controllers/settings/two_factor_authentications_controller_spec.rb b/spec/controllers/settings/two_factor_authentications_controller_spec.rb new file mode 100644 index 000000000..25d7a928d --- /dev/null +++ b/spec/controllers/settings/two_factor_authentications_controller_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Settings::TwoFactorAuthenticationsController do + render_views + + let(:user) { Fabricate(:user) } + before do + sign_in user, scope: :user + end + + describe 'GET #show' do + describe 'when user requires otp for login already' do + it 'returns http success' do + user.update(otp_required_for_login: true) + get :show + + expect(response).to have_http_status(:success) + end + end + + describe 'when user does not require otp for login' do + it 'returns http success' do + user.update(otp_required_for_login: false) + get :show + + expect(response).to have_http_status(:success) + end + end + end + + describe 'POST #create' do + describe 'when user requires otp for login already' do + it 'redirects to show page' do + user.update(otp_required_for_login: true) + post :create + + expect(response).to redirect_to(settings_two_factor_authentication_path) + end + end + + describe 'when creation succeeds' do + it 'updates user secret' do + before = user.otp_secret + post :create + + expect(user.reload.otp_secret).not_to eq(before) + expect(response).to redirect_to(new_settings_two_factor_authentication_confirmation_path) + end + end + end + + describe 'POST #destroy' do + before do + user.update(otp_required_for_login: true) + end + it 'turns off otp requirement' do + post :destroy + + expect(response).to redirect_to(settings_two_factor_authentication_path) + user.reload + expect(user.otp_required_for_login).to eq(false) + end + end +end -- cgit