From 9acdb166e8871632f592bfcd2386dfc288d81a07 Mon Sep 17 00:00:00 2001 From: Eugen Date: Sat, 8 Apr 2017 22:20:08 +0200 Subject: Fix #795, fix #704, fix #835 - 2FA requires confirmation to be enabled (#1278) * Fix #795, fix #704, fix #835 - 2FA requires confirmation to be enabled TOTP secret is not shown again after 2FA is enabled * Clean up --- app/assets/stylesheets/forms.scss | 19 +++++++++++ .../settings/two_factor_auths_controller.rb | 39 +++++++++++++++++----- app/models/form/two_factor_confirmation.rb | 7 ++++ app/views/settings/two_factor_auths/new.html.haml | 17 ++++++++++ app/views/settings/two_factor_auths/show.html.haml | 13 ++------ 5 files changed, 76 insertions(+), 19 deletions(-) create mode 100644 app/models/form/two_factor_confirmation.rb create mode 100644 app/views/settings/two_factor_auths/new.html.haml (limited to 'app') diff --git a/app/assets/stylesheets/forms.scss b/app/assets/stylesheets/forms.scss index ceccc14cd..2e3a4f147 100644 --- a/app/assets/stylesheets/forms.scss +++ b/app/assets/stylesheets/forms.scss @@ -25,6 +25,10 @@ code { margin-bottom: 15px; } + strong { + font-weight: 500; + } + .label_input { display: flex; @@ -224,7 +228,12 @@ code { } } +.qr-wrapper { + display: flex; +} + .qr-code { + flex: 0 0 auto; background: #fff; padding: 4px; margin-bottom: 20px; @@ -236,3 +245,13 @@ code { margin: 0; } } + +.qr-alternative { + margin-left: 10px; + color: $color3; + + samp { + display: block; + font-size: 14px; + } +} diff --git a/app/controllers/settings/two_factor_auths_controller.rb b/app/controllers/settings/two_factor_auths_controller.rb index cfee92391..203d1fc46 100644 --- a/app/controllers/settings/two_factor_auths_controller.rb +++ b/app/controllers/settings/two_factor_auths_controller.rb @@ -5,19 +5,29 @@ class Settings::TwoFactorAuthsController < ApplicationController before_action :authenticate_user! - def show - return unless current_user.otp_required_for_login + def show; end - @provision_url = current_user.otp_provisioning_uri(current_user.email, issuer: Rails.configuration.x.local_domain) - @qrcode = RQRCode::QRCode.new(@provision_url) - end + def new + redirect_to settings_two_factor_auth_path if current_user.otp_required_for_login - def enable - current_user.otp_required_for_login = true - current_user.otp_secret = User.generate_otp_secret + @confirmation = Form::TwoFactorConfirmation.new + current_user.otp_secret = User.generate_otp_secret(32) current_user.save! + set_qr_code + end - redirect_to settings_two_factor_auth_path + def create + if current_user.validate_and_consume_otp!(confirmation_params[:code]) + current_user.otp_required_for_login = true + current_user.save! + + redirect_to settings_two_factor_auth_path, 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 action: :new + end end def disable @@ -26,4 +36,15 @@ class Settings::TwoFactorAuthsController < ApplicationController 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 diff --git a/app/models/form/two_factor_confirmation.rb b/app/models/form/two_factor_confirmation.rb new file mode 100644 index 000000000..b8cf76d05 --- /dev/null +++ b/app/models/form/two_factor_confirmation.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class Form::TwoFactorConfirmation + include ActiveModel::Model + + attr_accessor :code +end diff --git a/app/views/settings/two_factor_auths/new.html.haml b/app/views/settings/two_factor_auths/new.html.haml new file mode 100644 index 000000000..5bae743ef --- /dev/null +++ b/app/views/settings/two_factor_auths/new.html.haml @@ -0,0 +1,17 @@ +- 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 diff --git a/app/views/settings/two_factor_auths/show.html.haml b/app/views/settings/two_factor_auths/show.html.haml index 87bfadc69..047fe0c54 100644 --- a/app/views/settings/two_factor_auths/show.html.haml +++ b/app/views/settings/two_factor_auths/show.html.haml @@ -2,16 +2,9 @@ = t('settings.two_factor_auth') .simple_form - - if current_user.otp_required_for_login - %p.hint= t('two_factor_auth.instructions_html') - - .qr-code= raw @qrcode.as_svg(padding: 0, module_size: 5) - - %p.hint= t('two_factor_auth.plaintext_secret_html', secret: current_user.otp_secret) - - %p.hint= t('two_factor_auth.warning') + %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 - %p.hint= t('two_factor_auth.description_html') - = link_to t('two_factor_auth.enable'), enable_settings_two_factor_auth_path, data: { method: 'POST' }, class: 'block-button' + = link_to t('two_factor_auth.setup'), new_settings_two_factor_auth_path, class: 'block-button' -- cgit