From 4b9881e31edfa1103b4a52f2ee75b861f6f41a29 Mon Sep 17 00:00:00 2001 From: aignatov-bio Date: Wed, 1 Jul 2020 11:07:33 +0200 Subject: [PATCH] Add 2fa to user settings page --- Gemfile | 1 + Gemfile.lock | 6 +++ .../javascripts/users/registrations/edit.js | 17 ++++++++- .../users/registrations_controller.rb | 26 +++++++++++++ app/models/user.rb | 12 +++--- .../users/registrations/_2fa_modal.html.erb | 30 ++++++++++++++- app/views/users/registrations/edit.html.erb | 6 ++- config/locales/en.yml | 9 +++++ config/routes.rb | 4 ++ spec/models/user_spec.rb | 38 ------------------- 10 files changed, 102 insertions(+), 47 deletions(-) diff --git a/Gemfile b/Gemfile index 7cf883a0b..cacc6aa73 100644 --- a/Gemfile +++ b/Gemfile @@ -76,6 +76,7 @@ gem 'rails_autolink', '~> 1.1', '>= 1.1.6' gem 'rgl' # Graph framework for project diagram calculations gem 'roo', '~> 2.8.2' # Spreadsheet parser gem 'rotp' +gem 'rqrcode' # QR code generator gem 'rubyzip' gem 'scenic', '~> 1.4' gem 'sdoc', '~> 1.0', group: :doc diff --git a/Gemfile.lock b/Gemfile.lock index 0f2a73a0b..599242fc6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -186,6 +186,7 @@ GEM activesupport childprocess (1.0.1) rake (< 13.0) + chunky_png (1.3.11) coderay (1.1.2) coffee-rails (5.0.0) coffee-script (>= 2.2.0) @@ -471,6 +472,10 @@ GEM rubyzip (>= 1.2.1, < 2.0.0) rotp (6.0.0) addressable (~> 2.7) + rqrcode (1.1.2) + chunky_png (~> 1.0) + rqrcode_core (~> 0.1) + rqrcode_core (0.1.2) rspec-core (3.8.2) rspec-support (~> 3.8.0) rspec-expectations (3.8.4) @@ -671,6 +676,7 @@ DEPENDENCIES rgl roo (~> 2.8.2) rotp + rqrcode rspec-rails (>= 4.0.0.beta2) rubocop (>= 0.75.0) rubocop-performance diff --git a/app/assets/javascripts/users/registrations/edit.js b/app/assets/javascripts/users/registrations/edit.js index 7d02da59b..388db117b 100644 --- a/app/assets/javascripts/users/registrations/edit.js +++ b/app/assets/javascripts/users/registrations/edit.js @@ -80,7 +80,22 @@ $fileInput[0].value = ''; }); - $('#twoFactorAuthentication').click(function() { + $('#twoFactorAuthenticationDisable').click(function() { $('#twoFactorAuthenticationModal').modal('show'); }); + + $('#twoFactorAuthenticationEnable').click(function() { + $.get(this.dataset.qrCodeUrl, function(result) { + $('#twoFactorAuthenticationModal .qr-code').html(result.qr_code); + $('#twoFactorAuthenticationModal').modal('show'); + }); + }); + + $('#twoFactorAuthenticationModal .2fa-enable-form').on('ajax:error', function(e, data) { + $(this).find('.submit-code-field').addClass('error').attr('data-error-text', data.responseJSON.error); + }); + + $('#twoFactorAuthenticationModal .2fa-disable-form').on('ajax:error', function(e, data) { + $(this).find('.password-field').addClass('error').attr('data-error-text', data.responseJSON.error); + }); }()); diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index ea2fac89e..67801ccaf 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -182,6 +182,32 @@ class Users::RegistrationsController < Devise::RegistrationsController end end + def two_factor_enable + totp = ROTP::TOTP.new(current_user.otp_secret, issuer: 'SciNote') + if totp.verify(params[:submit_code], drift_behind: 10) + current_user.update!(two_factor_auth_enabled: true) + redirect_to edit_user_registration_path + else + render json: { error: t('users.registrations.edit.2fa_errors.wrong_submit_code') }, status: :unprocessable_entity + end + end + + def two_factor_disable + if current_user.valid_password?(params[:password]) + current_user.update!(two_factor_auth_enabled: false, otp_secret: nil) + redirect_to edit_user_registration_path + else + render json: { error: t('users.registrations.edit.2fa_errors.wrong_password') }, status: :forbidden + end + end + + def two_factor_qr_code + current_user.ensure_2fa_token + qr_code_url = ROTP::TOTP.new(current_user.otp_secret, issuer: 'SciNote').provisioning_uri(current_user.email) + qr_code = RQRCode::QRCode.new(qr_code_url) + render json: { qr_code: qr_code.as_svg } + end + protected # Called upon creating User (before .save). Permits parameters and extracts diff --git a/app/models/user.rb b/app/models/user.rb index 5b94c7b7f..eed3623c0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -286,8 +286,6 @@ class User < ApplicationRecord foreign_key: :resource_owner_id, dependent: :delete_all - before_save :ensure_2fa_token, if: ->(user) { user.changed.include?('two_factor_auth_enabled') } - before_create :assign_2fa_token before_destroy :destroy_notifications def name @@ -630,6 +628,13 @@ class User < ApplicationRecord totp.verify(otp, drift_behind: 10) end + def ensure_2fa_token + return if otp_secret + + assign_2fa_token + save! + end + protected def confirmation_required? @@ -672,7 +677,4 @@ class User < ApplicationRecord self.otp_secret = ROTP::Base32.random end - def ensure_2fa_token - assign_2fa_token unless otp_secret - end end diff --git a/app/views/users/registrations/_2fa_modal.html.erb b/app/views/users/registrations/_2fa_modal.html.erb index fea5db397..9b07d15bc 100644 --- a/app/views/users/registrations/_2fa_modal.html.erb +++ b/app/views/users/registrations/_2fa_modal.html.erb @@ -3,9 +3,35 @@
-

+

<%= t("users.registrations.edit.2fa_modal.title") %>

-
+ <% if current_user.two_factor_auth_enabled? %> + <%= form_with(url: users_2fa_disable_path, method: "post", class: "2fa-disable-form") do %> +
+
+ <%= label_tag :password, t("users.registrations.edit.2fa_modal.password_label") %> + <%= password_field_tag :password, '', class: "sci-input-field" %> +
+
+
+ <%= button_tag t("users.registrations.edit.2fa_modal.submit_button"), type: 'submit', class: "btn btn-danger" %> +
+ <% end %> + <% else %> + <%= form_with(url: users_2fa_enable_path, method: "post", class: "2fa-enable-form") do %> +
+
+
+ <%= label_tag :submit_code, t("users.registrations.edit.2fa_modal.submit_code_label") %> + <%= text_field_tag :submit_code, '', class: "sci-input-field" %> +
+
+
+ <%= button_tag t("users.registrations.edit.2fa_modal.submit_button"), type: 'submit', class: "btn btn-primary" %> +
+ <% end %> + <% end %> +
diff --git a/app/views/users/registrations/edit.html.erb b/app/views/users/registrations/edit.html.erb index aca9cef15..2fe957e58 100644 --- a/app/views/users/registrations/edit.html.erb +++ b/app/views/users/registrations/edit.html.erb @@ -142,7 +142,11 @@ <% end %> - + <% if current_user.two_factor_auth_enabled? %> + + <% else %> + + <% end %>
diff --git a/config/locales/en.yml b/config/locales/en.yml index 8b1cfce2a..f235c0db7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1600,6 +1600,15 @@ en: new_password_label: "New password" new_password_2_label: "New password confirmation" 2fa_button: "Enable two-factor authentication" + 2fa_button_disable: "Disable two-factor authentication" + 2fa_modal: + title: 'Two-factor authentication' + password_label: 'Password' + submit_code_label: 'Submit code' + submit_button: 'Submit' + 2fa_errors: + wrong_submit_code: 'Not correct code' + wrong_password: 'Not correct password' new: head_title: "Sign up" team_name_label: "Team name" diff --git a/config/routes.rb b/config/routes.rb index c5c38009e..24f9fb9d5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -697,6 +697,10 @@ Rails.application.routes.draw do get 'users/sign_up_provider' => 'users/registrations#new_with_provider' post 'users/authenticate_with_two_factor' => 'users/sessions#authenticate_with_two_factor' post 'users/complete_sign_up_provider' => 'users/registrations#create_with_provider' + + post 'users/2fa_enable' => 'users/registrations#two_factor_enable' + post 'users/2fa_disable' => 'users/registrations#two_factor_disable' + get 'users/2fa_qr_code' => 'users/registrations#two_factor_qr_code' end namespace :api, defaults: { format: 'json' } do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ae6bbf1c4..ff51c9e7b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -332,44 +332,6 @@ describe User, type: :model do it { is_expected.to have_many(:system_notifications) } end - describe 'Callbacks' do - describe 'after_create' do - it 'sets token' do - user = create :user - - expect(user.otp_secret).to be_kind_of String - end - end - - describe 'before_save' do - let(:user) { create :user } - - context 'when changing twofa_enabled' do - context 'when user does not have otp_secret' do - it 'sets token before save' do - user.update_column(:otp_secret, nil) - - expect { user.update(two_factor_auth_enabled: true) }.to(change { user.otp_secret }) - end - end - - context 'when user does have otp_secret' do - it 'does not set new token before save' do - expect { user.update(two_factor_auth_enabled: true) }.not_to(change { user.otp_secret }) - end - end - end - - context 'when changing not twofa_enabled and user does have otp_secret' do - it 'does not set token before save' do - user.update_column(:otp_secret, nil) - - expect { user.update(name: 'SomeNewName') }.not_to(change { user.otp_secret }) - end - end - end - end - describe 'valid_otp?' do let(:user) { create :user } before do