From 9acdb166e8871632f592bfcd2386dfc288d81a07 Mon Sep 17 00:00:00 2001 From: Eugen Date: Sat, 8 Apr 2017 22:20:08 +0200 Subject: [PATCH] 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 ++++ .../settings/two_factor_auths/new.html.haml | 17 ++++++++ .../settings/two_factor_auths/show.html.haml | 13 ++----- config/locales/en.yml | 16 +++++--- config/navigation.rb | 2 +- config/routes.rb | 3 +- 8 files changed, 88 insertions(+), 28 deletions(-) create mode 100644 app/models/form/two_factor_confirmation.rb create mode 100644 app/views/settings/two_factor_auths/new.html.haml diff --git a/app/assets/stylesheets/forms.scss b/app/assets/stylesheets/forms.scss index ceccc14cd2..2e3a4f147f 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 cfee92391a..203d1fc46b 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 0000000000..b8cf76d058 --- /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 0000000000..5bae743ef6 --- /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 87bfadc692..047fe0c544 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' diff --git a/config/locales/en.yml b/config/locales/en.yml index aa3a732f96..118798ba16 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -94,6 +94,10 @@ en: following: Following list upload: Upload landing_strip_html: %{name} is a user on %{domain}. You can follow them or interact with them if you have an account anywhere in the fediverse. If you don't, you can sign up here. + media_attachments: + validations: + images_and_video: Cannot attach a video to a status that already contains images + too_many: Cannot attach more than 4 files notification_mailer: digest: body: 'Here is a brief summary of what you missed on %{instance} since your last visit on %{since}:' @@ -152,18 +156,18 @@ en: formats: default: "%b %d, %Y, %H:%M" two_factor_auth: + code_hint: Enter the code generated by your authenticator app to confirm description_html: If you enable two-factor authentication, logging in will require you to be in possession of your phone, which will generate tokens for you to enter. disable: Disable enable: Enable - instructions_html: "Scan this QR code into Google Authenticator or a similiar app on your phone. From now on, that app will generate tokens that you will have to enter when logging in." - plaintext_secret_html: 'Plain-text secret: %{secret}' + enabled_success: Two-factor authentication successfully enabled + instructions_html: "Scan this QR code into Google Authenticator or a similiar TOTP app on your phone. 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:' + setup: Set up warning: If you cannot configure an authenticator app right now, you should click "disable" or you won't be able to login. + wrong_code: The entered code was invalid! Are server time and device time correct? users: invalid_email: The e-mail address is invalid invalid_otp_token: Invalid two-factor code will_paginate: page_gap: "…" - media_attachments: - validations: - too_many: Cannot attach more than 4 files - images_and_video: Cannot attach a video to a status that already contains images diff --git a/config/navigation.rb b/config/navigation.rb index c6b7b97678..b92b872023 100644 --- a/config/navigation.rb +++ b/config/navigation.rb @@ -8,7 +8,7 @@ SimpleNavigation::Configuration.run do |navigation| settings.item :profile, safe_join([fa_icon('user fw'), t('settings.edit_profile')]), settings_profile_url settings.item :preferences, safe_join([fa_icon('sliders fw'), t('settings.preferences')]), settings_preferences_url settings.item :password, safe_join([fa_icon('cog fw'), t('auth.change_password')]), edit_user_registration_url - settings.item :two_factor_auth, safe_join([fa_icon('mobile fw'), t('settings.two_factor_auth')]), settings_two_factor_auth_url + settings.item :two_factor_auth, safe_join([fa_icon('mobile fw'), t('settings.two_factor_auth')]), settings_two_factor_auth_url, highlights_on: %r{/settings/two_factor_auth} settings.item :import, safe_join([fa_icon('cloud-upload fw'), t('settings.import')]), settings_import_url settings.item :export, safe_join([fa_icon('cloud-download fw'), t('settings.export')]), settings_export_url settings.item :authorized_apps, safe_join([fa_icon('list fw'), t('settings.authorized_apps')]), oauth_authorized_applications_url diff --git a/config/routes.rb b/config/routes.rb index 315ad5da57..9cbecf0779 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -60,9 +60,8 @@ Rails.application.routes.draw do end end - resource :two_factor_auth, only: [:show] do + resource :two_factor_auth, only: [:show, :new, :create] do member do - post :enable post :disable end end