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 neededmain
Matt Jankowski
8 years ago
committed by
Eugen
parent
6af21daac9
commit
67dea31b0f
@ -0,0 +1,43 @@
|
|||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
module Settings
|
||||||
|
module TwoFactorAuthentication
|
||||||
|
class ConfirmationsController < ApplicationController
|
||||||
|
layout 'admin'
|
||||||
|
|
||||||
|
before_action :authenticate_user!
|
||||||
|
|
||||||
|
def new
|
||||||
|
prepare_two_factor_form
|
||||||
|
end
|
||||||
|
|
||||||
|
def create
|
||||||
|
if current_user.validate_and_consume_otp!(confirmation_params[:code])
|
||||||
|
flash[:notice] = I18n.t('two_factor_authentication.enabled_success')
|
||||||
|
|
||||||
|
current_user.otp_required_for_login = true
|
||||||
|
@recovery_codes = current_user.generate_otp_backup_codes!
|
||||||
|
current_user.save!
|
||||||
|
|
||||||
|
render 'settings/two_factor_authentication/recovery_codes/index'
|
||||||
|
else
|
||||||
|
flash.now[:alert] = I18n.t('two_factor_authentication.wrong_code')
|
||||||
|
prepare_two_factor_form
|
||||||
|
render :new
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def confirmation_params
|
||||||
|
params.require(:form_two_factor_confirmation).permit(:code)
|
||||||
|
end
|
||||||
|
|
||||||
|
def prepare_two_factor_form
|
||||||
|
@confirmation = Form::TwoFactorConfirmation.new
|
||||||
|
@provision_url = current_user.otp_provisioning_uri(current_user.email, issuer: Rails.configuration.x.local_domain)
|
||||||
|
@qrcode = RQRCode::QRCode.new(@provision_url)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
@ -0,0 +1,18 @@
|
|||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
module Settings
|
||||||
|
module TwoFactorAuthentication
|
||||||
|
class RecoveryCodesController < ApplicationController
|
||||||
|
layout 'admin'
|
||||||
|
|
||||||
|
before_action :authenticate_user!
|
||||||
|
|
||||||
|
def create
|
||||||
|
@recovery_codes = current_user.generate_otp_backup_codes!
|
||||||
|
current_user.save!
|
||||||
|
flash[:notice] = I18n.t('two_factor_authentication.recovery_codes_regenerated')
|
||||||
|
render :index
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
@ -0,0 +1,30 @@
|
|||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
module Settings
|
||||||
|
class TwoFactorAuthenticationsController < ApplicationController
|
||||||
|
layout 'admin'
|
||||||
|
|
||||||
|
before_action :authenticate_user!
|
||||||
|
before_action :verify_otp_required, only: [:create]
|
||||||
|
|
||||||
|
def show; end
|
||||||
|
|
||||||
|
def create
|
||||||
|
current_user.otp_secret = User.generate_otp_secret(32)
|
||||||
|
current_user.save!
|
||||||
|
redirect_to new_settings_two_factor_authentication_confirmation_path
|
||||||
|
end
|
||||||
|
|
||||||
|
def destroy
|
||||||
|
current_user.otp_required_for_login = false
|
||||||
|
current_user.save!
|
||||||
|
redirect_to settings_two_factor_authentication_path
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def verify_otp_required
|
||||||
|
redirect_to settings_two_factor_authentication_path if current_user.otp_required_for_login?
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
@ -1,56 +0,0 @@
|
|||||||
# frozen_string_literal: true
|
|
||||||
|
|
||||||
class Settings::TwoFactorAuthsController < ApplicationController
|
|
||||||
layout 'admin'
|
|
||||||
|
|
||||||
before_action :authenticate_user!
|
|
||||||
|
|
||||||
def show; end
|
|
||||||
|
|
||||||
def new
|
|
||||||
redirect_to settings_two_factor_auth_path if current_user.otp_required_for_login
|
|
||||||
|
|
||||||
@confirmation = Form::TwoFactorConfirmation.new
|
|
||||||
current_user.otp_secret = User.generate_otp_secret(32)
|
|
||||||
current_user.save!
|
|
||||||
set_qr_code
|
|
||||||
end
|
|
||||||
|
|
||||||
def create
|
|
||||||
if current_user.validate_and_consume_otp!(confirmation_params[:code])
|
|
||||||
current_user.otp_required_for_login = true
|
|
||||||
@codes = current_user.generate_otp_backup_codes!
|
|
||||||
current_user.save!
|
|
||||||
flash[:notice] = I18n.t('two_factor_auth.enabled_success')
|
|
||||||
else
|
|
||||||
@confirmation = Form::TwoFactorConfirmation.new
|
|
||||||
set_qr_code
|
|
||||||
flash.now[:alert] = I18n.t('two_factor_auth.wrong_code')
|
|
||||||
render :new
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
def recovery_codes
|
|
||||||
@codes = current_user.generate_otp_backup_codes!
|
|
||||||
current_user.save!
|
|
||||||
flash[:notice] = I18n.t('two_factor_auth.recovery_codes_regenerated')
|
|
||||||
end
|
|
||||||
|
|
||||||
def disable
|
|
||||||
current_user.otp_required_for_login = false
|
|
||||||
current_user.save!
|
|
||||||
|
|
||||||
redirect_to settings_two_factor_auth_path
|
|
||||||
end
|
|
||||||
|
|
||||||
private
|
|
||||||
|
|
||||||
def set_qr_code
|
|
||||||
@provision_url = current_user.otp_provisioning_uri(current_user.email, issuer: Rails.configuration.x.local_domain)
|
|
||||||
@qrcode = RQRCode::QRCode.new(@provision_url)
|
|
||||||
end
|
|
||||||
|
|
||||||
def confirmation_params
|
|
||||||
params.require(:form_two_factor_confirmation).permit(:code)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
@ -0,0 +1,17 @@
|
|||||||
|
- content_for :page_title do
|
||||||
|
= t('settings.two_factor_authentication')
|
||||||
|
|
||||||
|
= simple_form_for @confirmation, url: settings_two_factor_authentication_confirmation_path, method: :post do |f|
|
||||||
|
%p.hint= t('two_factor_authentication.instructions_html')
|
||||||
|
|
||||||
|
.qr-wrapper
|
||||||
|
.qr-code= raw @qrcode.as_svg(padding: 0, module_size: 4)
|
||||||
|
|
||||||
|
.qr-alternative
|
||||||
|
%p.hint= t('two_factor_authentication.manual_instructions')
|
||||||
|
%samp.qr-alternative__code= current_user.otp_secret.scan(/.{4}/).join(' ')
|
||||||
|
|
||||||
|
= f.input :code, hint: t('two_factor_authentication.code_hint'), placeholder: t('simple_form.labels.defaults.otp_attempt')
|
||||||
|
|
||||||
|
.actions
|
||||||
|
= f.button :button, t('two_factor_authentication.enable'), type: :submit
|
||||||
@ -0,0 +1,9 @@
|
|||||||
|
- content_for :page_title do
|
||||||
|
= t('settings.two_factor_authentication')
|
||||||
|
|
||||||
|
%p.hint= t('two_factor_authentication.recovery_instructions')
|
||||||
|
|
||||||
|
%ol.recovery-codes
|
||||||
|
- @recovery_codes.each do |code|
|
||||||
|
%li<
|
||||||
|
%samp= code
|
||||||
@ -0,0 +1,26 @@
|
|||||||
|
- content_for :page_title do
|
||||||
|
= t('settings.two_factor_authentication')
|
||||||
|
|
||||||
|
.simple_form
|
||||||
|
%p.hint
|
||||||
|
= t('two_factor_authentication.description_html')
|
||||||
|
|
||||||
|
- if current_user.otp_required_for_login
|
||||||
|
= link_to t('two_factor_authentication.disable'),
|
||||||
|
settings_two_factor_authentication_path,
|
||||||
|
data: { method: :delete },
|
||||||
|
class: 'block-button'
|
||||||
|
- else
|
||||||
|
= link_to t('two_factor_authentication.setup'),
|
||||||
|
settings_two_factor_authentication_path,
|
||||||
|
data: { method: :post },
|
||||||
|
class: 'block-button'
|
||||||
|
|
||||||
|
- if current_user.otp_required_for_login
|
||||||
|
.simple_form
|
||||||
|
%p.hint
|
||||||
|
= t('two_factor_authentication.lost_recovery_codes')
|
||||||
|
= link_to t('two_factor_authentication.generate_recovery_codes'),
|
||||||
|
settings_two_factor_authentication_recovery_codes_path,
|
||||||
|
data: { method: :post },
|
||||||
|
class: 'block-button'
|
||||||
@ -1,6 +0,0 @@
|
|||||||
%p.hint= t('two_factor_auth.recovery_instructions')
|
|
||||||
|
|
||||||
%ol.recovery-codes
|
|
||||||
- recovery_codes.each do |code|
|
|
||||||
%li
|
|
||||||
%samp= code
|
|
||||||
@ -1,4 +0,0 @@
|
|||||||
- content_for :page_title do
|
|
||||||
= t('settings.two_factor_auth')
|
|
||||||
|
|
||||||
= render 'recovery_codes', recovery_codes: @codes
|
|
||||||
@ -1,17 +0,0 @@
|
|||||||
- content_for :page_title do
|
|
||||||
= t('settings.two_factor_auth')
|
|
||||||
|
|
||||||
= simple_form_for @confirmation, url: settings_two_factor_auth_path, method: :post do |f|
|
|
||||||
%p.hint= t('two_factor_auth.instructions_html')
|
|
||||||
|
|
||||||
.qr-wrapper
|
|
||||||
.qr-code= raw @qrcode.as_svg(padding: 0, module_size: 4)
|
|
||||||
|
|
||||||
.qr-alternative
|
|
||||||
%p.hint= t('two_factor_auth.manual_instructions')
|
|
||||||
%samp.qr-alternative__code= current_user.otp_secret.scan(/.{4}/).join(' ')
|
|
||||||
|
|
||||||
= f.input :code, hint: t('two_factor_auth.code_hint'), placeholder: t('simple_form.labels.defaults.otp_attempt')
|
|
||||||
|
|
||||||
.actions
|
|
||||||
= f.button :button, t('two_factor_auth.enable'), type: :submit
|
|
||||||
@ -1,4 +0,0 @@
|
|||||||
- content_for :page_title do
|
|
||||||
= t('settings.two_factor_auth')
|
|
||||||
|
|
||||||
= render 'recovery_codes', recovery_codes: @codes
|
|
||||||
@ -1,17 +0,0 @@
|
|||||||
- content_for :page_title do
|
|
||||||
= t('settings.two_factor_auth')
|
|
||||||
|
|
||||||
.simple_form
|
|
||||||
%p.hint= t('two_factor_auth.description_html')
|
|
||||||
|
|
||||||
- if current_user.otp_required_for_login
|
|
||||||
= link_to t('two_factor_auth.disable'), disable_settings_two_factor_auth_path, data: { method: 'POST' }, class: 'block-button'
|
|
||||||
- else
|
|
||||||
= link_to t('two_factor_auth.setup'), new_settings_two_factor_auth_path, class: 'block-button'
|
|
||||||
|
|
||||||
- if current_user.otp_required_for_login
|
|
||||||
%p
|
|
||||||
|
|
||||||
.simple_form
|
|
||||||
%p.hint= t('two_factor_auth.lost_recovery_codes')
|
|
||||||
= link_to t('two_factor_auth.generate_recovery_codes'), recovery_codes_settings_two_factor_auth_path, data: { method: 'POST' }, class: 'block-button'
|
|
||||||
@ -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
|
||||||
@ -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
|
||||||
@ -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
|
||||||
Loading…
Reference in new issue