From 78a1cbbafe411fa149bb06a3d5b147517a6d65d4 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Wed, 6 Apr 2022 13:36:13 +0100 Subject: [PATCH] Support user sees 2FA code screen on login --- Gemfile.lock | 18 ++++++++---------- app/controllers/auth/passwords_controller.rb | 4 ++-- app/models/user.rb | 6 ++++-- .../two_factor_authentication/show.html.erb | 6 +++--- config/routes.rb | 6 ++++++ spec/features/admin_panel_spec.rb | 10 +++++----- spec/features/user_spec.rb | 18 ++++++++++++++++++ spec/models/user_spec.rb | 4 ++-- 8 files changed, 48 insertions(+), 24 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 05f784cd2..6eff85756 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -12,7 +12,7 @@ GIT GIT remote: https://github.com/baarkerlounger/two_factor_authentication.git - revision: c2237dedb89b1fc53101cec536e57912049c5412 + revision: afb91d5ffabbdb79ca29645749ef625f7e3a76ea specs: two_factor_authentication (2.2.0) devise @@ -105,7 +105,7 @@ GEM ruby2_keywords (>= 0.0.2, < 1.0) ast (2.4.2) aws-eventstream (1.2.0) - aws-partitions (1.571.0) + aws-partitions (1.573.0) aws-sdk-core (3.130.0) aws-eventstream (~> 1, >= 1.0.2) aws-partitions (~> 1, >= 1.525.0) @@ -160,7 +160,7 @@ GEM railties (>= 3.2) encryptor (3.0.0) erubi (1.10.0) - excon (0.92.1) + excon (0.92.2) factory_bot (6.2.1) activesupport (>= 5.0.0) factory_bot_rails (6.2.0) @@ -196,7 +196,6 @@ GEM railties (>= 5.2, < 7.1) responders (>= 2, < 4) iniparse (1.5.0) - io-wait (0.2.1) jmespath (1.6.1) jquery-rails (4.4.0) rails-dom-testing (>= 1, < 3) @@ -222,7 +221,7 @@ GEM listen (3.7.1) rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) - loofah (2.15.0) + loofah (2.16.0) crass (~> 1.0.2) nokogiri (>= 1.5.9) mail (2.7.1) @@ -232,7 +231,7 @@ GEM method_source (1.0.0) mini_mime (1.1.2) minitest (5.15.0) - msgpack (1.4.5) + msgpack (1.5.0) net-imap (0.2.3) digest net-protocol @@ -241,8 +240,7 @@ GEM digest net-protocol timeout - net-protocol (0.1.2) - io-wait + net-protocol (0.1.3) timeout net-smtp (0.3.1) digest @@ -273,7 +271,7 @@ GEM parallel (1.22.1) parser (3.1.1.0) ast (~> 2.4.1) - pg (1.3.4) + pg (1.3.5) postcodes_io (0.4.0) excon (~> 0.39) propshaft (0.6.4) @@ -352,7 +350,7 @@ GEM rspec-expectations (3.11.0) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.11.0) - rspec-mocks (3.11.0) + rspec-mocks (3.11.1) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.11.0) rspec-rails (5.1.1) diff --git a/app/controllers/auth/passwords_controller.rb b/app/controllers/auth/passwords_controller.rb index 1105c0475..df00f1767 100644 --- a/app/controllers/auth/passwords_controller.rb +++ b/app/controllers/auth/passwords_controller.rb @@ -58,7 +58,7 @@ protected end def password_update_flash_message - resource_class == AdminUser ? :updated_2FA : :updated + resource.need_two_factor_authentication?(request) ? :updated_2FA : :updated end def resource_class_name @@ -71,7 +71,7 @@ protected def after_resetting_password_path_for(resource) if Devise.sign_in_after_reset_password - if resource_class == AdminUser + if resource.need_two_factor_authentication?(request) resource.send_new_otp admin_user_two_factor_authentication_path else diff --git a/app/models/user.rb b/app/models/user.rb index aee03000b..5c44bf679 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -27,7 +27,7 @@ class User < ApplicationRecord data_accessor: 0, data_provider: 1, data_coordinator: 2, - support: 99 + support: 99, }.freeze enum role: ROLES @@ -71,7 +71,9 @@ class User < ApplicationRecord update!(is_dpo: true) end - def need_two_factor_authentication? + def need_two_factor_authentication?(_request) support? end + + def send_two_factor_authentication_code(code); end end diff --git a/app/views/devise/two_factor_authentication/show.html.erb b/app/views/devise/two_factor_authentication/show.html.erb index 0966af5b3..8b2886f2f 100644 --- a/app/views/devise/two_factor_authentication/show.html.erb +++ b/app/views/devise/two_factor_authentication/show.html.erb @@ -1,4 +1,4 @@ -<% content_for :title, "Check your phone" %> +<% content_for :title, "Check your email" %> <%= form_with(model: resource, url: "/admin/two-factor-authentication", html: { method: :put }) do |f| %>
@@ -9,7 +9,7 @@ <%= content_for(:title) %> -

We’ve sent you a text message with a security code.

+

We’ve sent you an email with a security code.

<%= f.govuk_number_field :code, label: { text: "Security code", size: "m" }, @@ -24,5 +24,5 @@ <% end %>

- <%= govuk_link_to "Not received a text message?", admin_two_factor_authentication_resend_path %> + <%= govuk_link_to "Not received an email?", admin_two_factor_authentication_resend_path %>

diff --git a/config/routes.rb b/config/routes.rb index 591f9cad5..ac126d996 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -14,6 +14,7 @@ Rails.application.routes.draw do sign_in: "sign-in", sign_out: "sign-out", two_factor_authentication: "two-factor-authentication", + two_factor_authentication_resend_code: "resend-code", }, sign_out_via: %i[get], } @@ -27,15 +28,20 @@ Rails.application.routes.draw do controllers: { passwords: "auth/passwords", sessions: "auth/sessions", + two_factor_authentication: "auth/two_factor_authentication", }, path_names: { sign_in: "sign-in", sign_out: "sign-out", + two_factor_authentication: "two-factor-authentication", + two_factor_authentication_resend_code: "resend-code", }, + sign_out_via: %i[get], } devise_scope :user do get "account/password/reset-confirmation", to: "auth/passwords#reset_confirmation" + get "account/two-factor-authentication/resend", to: "auth/two_factor_authentication#show_resend" put "account", to: "users#update" end diff --git a/spec/features/admin_panel_spec.rb b/spec/features/admin_panel_spec.rb index 3536e1c08..dcbd4ab80 100644 --- a/spec/features/admin_panel_spec.rb +++ b/spec/features/admin_panel_spec.rb @@ -42,7 +42,7 @@ RSpec.describe "Admin Panel" do admin.update!(direct_otp_sent_at: 16.minutes.ago) fill_in("code", with: otp) click_button("Submit") - expect(page).to have_content("Check your phone") + expect(page).to have_content("Check your email") expect(page).to have_http_status(:unprocessable_entity) expect(page).to have_title("Error") expect(page).to have_selector("#error-summary-title") @@ -58,7 +58,7 @@ RSpec.describe "Admin Panel" do click_button("Sign in") fill_in("code", with: otp) click_button("Submit") - expect(page).to have_content("Check your phone") + expect(page).to have_content("Check your email") expect(page).to have_http_status(:unprocessable_entity) expect(page).to have_title("Error") expect(page).to have_selector("#error-summary-title") @@ -74,12 +74,12 @@ RSpec.describe "Admin Panel" do end it "displays the resend view" do - click_link("Not received a text message?") + click_link("Not received an email?") expect(page).to have_button("Resend security code") end it "send a new OTP code and redirects back to the 2FA view" do - click_link("Not received a text message?") + click_link("Not received an email?") expect { click_button("Resend security code") }.to(change { admin.reload.direct_otp }) expect(page).to have_current_path("/admin/two-factor-authentication") end @@ -102,7 +102,7 @@ RSpec.describe "Admin Panel" do fill_in("admin_user[email]", with: admin.email) fill_in("admin_user[password]", with: admin.password) click_button("Sign in") - expect(page).to have_content("Check your phone") + expect(page).to have_content("Check your email") end end diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index d7326dd7e..f7fb1f39b 100644 --- a/spec/features/user_spec.rb +++ b/spec/features/user_spec.rb @@ -324,4 +324,22 @@ RSpec.describe "User Features" do end end end + + context "when the user is a customer support person" do + context "when they are logging in" do + let!(:support_user) { FactoryBot.create(:user, :support, last_sign_in_at: Time.zone.now) } + + before do + visit("/logs") + fill_in("user[email]", with: support_user.email) + fill_in("user[password]", with: "pAssword1") + click_button("Sign in") + end + + it "asks for a 2FA code" do + expect(page).to have_content("We’ve sent you an email with a security code.") + expect(page).to have_field("user[code]") + end + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b8d92eb9d..e8d0008a9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -66,7 +66,7 @@ RSpec.describe User, type: :model do end it "does not require 2FA" do - expect(user.need_two_factor_authentication?).to be false + expect(user.need_two_factor_authentication?(nil)).to be false end context "when the user is a Customer Support person" do @@ -78,7 +78,7 @@ RSpec.describe User, type: :model do end it "requires 2FA" do - expect(user.need_two_factor_authentication?).to be true + expect(user.need_two_factor_authentication?(nil)).to be true end end end