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..3ce603ade 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,9 +71,9 @@ 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 + send("#{resource_name}_two_factor_authentication_path") else after_sign_in_path_for(resource) end diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 95654e520..ad65de5e0 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -29,8 +29,8 @@ private end def after_sign_in_path_for(resource) - if resource_class == AdminUser - admin_user_two_factor_authentication_path + if resource.need_two_factor_authentication?(request) + send("#{resource_name}_two_factor_authentication_path") else params.dig(resource_class_name, "start").present? ? case_logs_path : super end diff --git a/app/models/admin_user.rb b/app/models/admin_user.rb index dbb8fa196..7fe671e05 100644 --- a/app/models/admin_user.rb +++ b/app/models/admin_user.rb @@ -21,13 +21,13 @@ class AdminUser < ApplicationRecord validates :phone, presence: true, numericality: true - MFA_SMS_TEMPLATE_ID = "bf309d93-804e-4f95-b1f4-bd513c48ecb0".freeze + MFA_TEMPLATE_ID = "6bdf5ee1-8e01-4be1-b1f9-747061d8a24c".freeze RESET_PASSWORD_TEMPLATE_ID = "fbb2d415-b9b1-4507-ba0a-6e542fa3504d".freeze def send_two_factor_authentication_code(code) - template_id = MFA_SMS_TEMPLATE_ID + template_id = MFA_TEMPLATE_ID personalisation = { otp: code } - Sms.send(phone, template_id, personalisation) + DeviseNotifyMailer.new.send_email(email, template_id, personalisation) end def reset_password_notify_template diff --git a/app/models/user.rb b/app/models/user.rb index 159be7e70..69d63f3ac 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -2,7 +2,7 @@ class User < ApplicationRecord # Include default devise modules. Others available are: # :confirmable, :timeoutable and :omniauthable devise :database_authenticatable, :recoverable, :rememberable, :validatable, - :trackable, :lockable + :trackable, :lockable, :two_factor_authenticatable belongs_to :organisation has_many :owned_case_logs, through: :organisation @@ -21,16 +21,23 @@ class User < ApplicationRecord sign_in_count updated_at] + has_one_time_password(encrypted: true) + ROLES = { data_accessor: 0, data_provider: 1, data_coordinator: 2, + support: 99, }.freeze enum role: ROLES def case_logs - CaseLog.for_organisation(organisation) + if support? + CaseLog.all + else + CaseLog.for_organisation(organisation) + end end def completed_case_logs @@ -41,13 +48,6 @@ class User < ApplicationRecord case_logs.not_completed end - RESET_PASSWORD_TEMPLATE_ID = "2c410c19-80a7-481c-a531-2bcb3264f8e6".freeze - SET_PASSWORD_TEMPLATE_ID = "257460a6-6616-4640-a3f9-17c3d73d9e91".freeze - - def reset_password_notify_template - last_sign_in_at ? RESET_PASSWORD_TEMPLATE_ID : SET_PASSWORD_TEMPLATE_ID - end - def is_key_contact? is_key_contact end @@ -63,4 +63,22 @@ class User < ApplicationRecord def is_data_protection_officer! update!(is_dpo: true) end + + MFA_TEMPLATE_ID = "6bdf5ee1-8e01-4be1-b1f9-747061d8a24c".freeze + RESET_PASSWORD_TEMPLATE_ID = "2c410c19-80a7-481c-a531-2bcb3264f8e6".freeze + SET_PASSWORD_TEMPLATE_ID = "257460a6-6616-4640-a3f9-17c3d73d9e91".freeze + + def reset_password_notify_template + last_sign_in_at ? RESET_PASSWORD_TEMPLATE_ID : SET_PASSWORD_TEMPLATE_ID + end + + def need_two_factor_authentication?(_request) + support? + end + + def send_two_factor_authentication_code(code) + template_id = MFA_TEMPLATE_ID + personalisation = { otp: code } + DeviseNotifyMailer.new.send_email(email, template_id, personalisation) + end end diff --git a/app/services/sms.rb b/app/services/sms.rb deleted file mode 100644 index aaf223f1c..000000000 --- a/app/services/sms.rb +++ /dev/null @@ -1,15 +0,0 @@ -require "notifications/client" - -class Sms - def self.notify_client - Notifications::Client.new(ENV["GOVUK_NOTIFY_API_KEY"]) - end - - def self.send(phone_number, template_id, args) - notify_client.send_sms( - phone_number:, - template_id:, - personalisation: args, - ) - end -end diff --git a/app/views/case_logs/_log_list.html.erb b/app/views/case_logs/_log_list.html.erb index e728a5385..3cb30289f 100644 --- a/app/views/case_logs/_log_list.html.erb +++ b/app/views/case_logs/_log_list.html.erb @@ -15,6 +15,10 @@
Text messages sometimes take a few minutes to arrive. If you do not receive the text message, you can request a new one.
+Emails sometimes take a few minutes to arrive. If you do not receive the email, you can request a new one.
<%= f.govuk_submit "Resend security code" %>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 +25,5 @@ <% end %>- <%= govuk_link_to "Not received a text message?", admin_two_factor_authentication_resend_path %> + <%= govuk_link_to "Not received an email?", send("#{resource_name}_two_factor_authentication_resend_path") %>
diff --git a/config/routes.rb b/config/routes.rb index 591f9cad5..4cb364878 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -14,12 +14,13 @@ 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], } devise_scope :admin_user do - get "admin/two-factor-authentication/resend", to: "auth/two_factor_authentication#show_resend" + get "admin/two-factor-authentication/resend", to: "auth/two_factor_authentication#show_resend", as: "admin_user_two_factor_authentication_resend" end devise_for :users, { @@ -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", as: "user_two_factor_authentication_resend" put "account", to: "users#update" end diff --git a/db/migrate/20220406093139_two_factor_authentication_add_to_user.rb b/db/migrate/20220406093139_two_factor_authentication_add_to_user.rb new file mode 100644 index 000000000..49a88ac68 --- /dev/null +++ b/db/migrate/20220406093139_two_factor_authentication_add_to_user.rb @@ -0,0 +1,15 @@ +class TwoFactorAuthenticationAddToUser < ActiveRecord::Migration[7.0] + def change + change_table :users, bulk: true do |t| + t.column :second_factor_attempts_count, :integer, default: 0 + t.column :encrypted_otp_secret_key, :string + t.column :encrypted_otp_secret_key_iv, :string + t.column :encrypted_otp_secret_key_salt, :string + t.column :direct_otp, :string + t.column :direct_otp_sent_at, :datetime + t.column :totp_timestamp, :timestamp + + t.index :encrypted_otp_secret_key, unique: true + end + end +end diff --git a/db/schema.rb b/db/schema.rb index c1ca092bb..61368691c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -324,7 +324,15 @@ ActiveRecord::Schema[7.0].define(version: 202202071123100) do t.boolean "is_dpo", default: false t.boolean "is_key_contact", default: false t.string "phone" + t.integer "second_factor_attempts_count", default: 0 + t.string "encrypted_otp_secret_key" + t.string "encrypted_otp_secret_key_iv" + t.string "encrypted_otp_secret_key_salt" + t.string "direct_otp" + t.datetime "direct_otp_sent_at", precision: nil + t.datetime "totp_timestamp", precision: nil t.index ["email"], name: "index_users_on_email", unique: true + t.index ["encrypted_otp_secret_key"], name: "index_users_on_encrypted_otp_secret_key", unique: true t.index ["organisation_id"], name: "index_users_on_organisation_id" t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true t.index ["unlock_token"], name: "index_users_on_unlock_token", unique: true diff --git a/spec/factories/user.rb b/spec/factories/user.rb index 0de22af97..9b8edb23f 100644 --- a/spec/factories/user.rb +++ b/spec/factories/user.rb @@ -11,6 +11,9 @@ FactoryBot.define do trait :data_protection_officer do is_dpo { true } end + trait :support do + role { "support" } + end created_at { Time.zone.now } updated_at { Time.zone.now } end diff --git a/spec/features/admin_panel_spec.rb b/spec/features/admin_panel_spec.rb index 3536e1c08..9ade961ed 100644 --- a/spec/features/admin_panel_spec.rb +++ b/spec/features/admin_panel_spec.rb @@ -2,13 +2,15 @@ require "rails_helper" RSpec.describe "Admin Panel" do let!(:admin) { FactoryBot.create(:admin_user) } + let(:devise_notify_mailer) { DeviseNotifyMailer.new } let(:notify_client) { instance_double(Notifications::Client) } - let(:mfa_template_id) { AdminUser::MFA_SMS_TEMPLATE_ID } + let(:mfa_template_id) { AdminUser::MFA_TEMPLATE_ID } let(:otp) { "999111" } before do - allow(Sms).to receive(:notify_client).and_return(notify_client) - allow(notify_client).to receive(:send_sms).and_return(true) + allow(DeviseNotifyMailer).to receive(:new).and_return(devise_notify_mailer) + allow(devise_notify_mailer).to receive(:notify_client).and_return(notify_client) + allow(notify_client).to receive(:send_email).and_return(true) end it "shows the admin sign in page" do @@ -26,8 +28,12 @@ RSpec.describe "Admin Panel" do end it "authenticates successfully" do - expect(notify_client).to receive(:send_sms).with( - hash_including(phone_number: admin.phone, template_id: mfa_template_id), + expect(notify_client).to receive(:send_email).with( + { + email_address: admin.email, + template_id: mfa_template_id, + personalisation: { otp: }, + }, ) click_button("Sign in") fill_in("code", with: otp) @@ -42,7 +48,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 +64,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 +80,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,20 +108,15 @@ 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 context "when the admin has forgotten their password" do let!(:admin_user) { FactoryBot.create(:admin_user, last_sign_in_at: Time.zone.now) } - let(:notify_client) { instance_double(Notifications::Client) } let(:reset_password_token) { "MCDH5y6Km-U7CFPgAMVS" } - let(:devise_notify_mailer) { DeviseNotifyMailer.new } before do - allow(DeviseNotifyMailer).to receive(:new).and_return(devise_notify_mailer) - allow(devise_notify_mailer).to receive(:notify_client).and_return(notify_client) - allow(notify_client).to receive(:send_email).and_return(true) allow(Devise.token_generator).to receive(:generate).and_return(reset_password_token) end diff --git a/spec/features/auth/user_lockout_spec.rb b/spec/features/auth/user_lockout_spec.rb index a426982ba..c1ba131df 100644 --- a/spec/features/auth/user_lockout_spec.rb +++ b/spec/features/auth/user_lockout_spec.rb @@ -48,9 +48,12 @@ RSpec.describe "User Lockout" do end context "when login-in with the right admin password and incorrect 2FA token up to a maximum number of attempts" do + let(:devise_notify_mailer) { DeviseNotifyMailer.new } + before do - allow(Sms).to receive(:notify_client).and_return(notify_client) - allow(notify_client).to receive(:send_sms).and_return(true) + allow(DeviseNotifyMailer).to receive(:new).and_return(devise_notify_mailer) + allow(devise_notify_mailer).to receive(:notify_client).and_return(notify_client) + allow(notify_client).to receive(:send_email).and_return(true) visit("/admin/sign-in") fill_in("admin_user[email]", with: admin.email) diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index d7326dd7e..5cd035278 100644 --- a/spec/features/user_spec.rb +++ b/spec/features/user_spec.rb @@ -15,7 +15,7 @@ RSpec.describe "User Features" do end context "when the user navigates to case logs" do - it " is required to log in" do + it "is required to log in" do visit("/logs") expect(page).to have_current_path("/account/sign-in") expect(page).to have_content("Sign in to your account to submit CORE data") @@ -26,7 +26,7 @@ RSpec.describe "User Features" do expect(page).to have_no_content("You need to sign in or sign up before continuing.") end - it " is redirected to case logs after signing in" do + it "is redirected to case logs after signing in" do visit("/logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") @@ -34,7 +34,7 @@ RSpec.describe "User Features" do expect(page).to have_current_path("/logs") end - it " can log out again", js: true do + it "can log out again", js: true do visit("/logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") @@ -44,7 +44,7 @@ RSpec.describe "User Features" do expect(page).to have_content("Start now") end - it " can log out again with js disabled" do + it "can log out again with js disabled" do visit("/logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") @@ -56,13 +56,13 @@ RSpec.describe "User Features" do end context "when the user has forgotten their password" do - it " is redirected to the reset password page when they click the reset password link" do + it "is redirected to the reset password page when they click the reset password link" do visit("/logs") click_link("reset your password") expect(page).to have_current_path("/account/password/new") end - it " is shown an error message if they submit without entering an email address" do + it "is shown an error message if they submit without entering an email address" do visit("/account/password/new") click_button("Send email") expect(page).to have_selector("#error-summary-title") @@ -70,7 +70,7 @@ RSpec.describe "User Features" do expect(page).to have_title("Error") end - it " is shown an error message if they submit an invalid email address" do + it "is shown an error message if they submit an invalid email address" do visit("/account/password/new") fill_in("user[email]", with: "thisisn'tanemail") click_button("Send email") @@ -79,28 +79,28 @@ RSpec.describe "User Features" do expect(page).to have_title("Error") end - it " is redirected to check your email page after submitting an email on the reset password page" do + it "is redirected to check your email page after submitting an email on the reset password page" do visit("/account/password/new") fill_in("user[email]", with: user.email) click_button("Send email") expect(page).to have_content("Check your email") end - it " is shown their email on the password reset confirmation page" do + it "is shown their email on the password reset confirmation page" do visit("/account/password/new") fill_in("user[email]", with: user.email) click_button("Send email") expect(page).to have_content(user.email) end - it " is shown the reset password confirmation page even if their email doesn't exist in the system" do + it "is shown the reset password confirmation page even if their email doesn't exist in the system" do visit("/account/password/new") fill_in("user[email]", with: "idontexist@example.com") click_button("Send email") expect(page).to have_current_path("/account/password/reset-confirmation?email=idontexist%40example.com") end - it " is sent a reset password email via Notify" do + it "is sent a reset password email via Notify" do expect(notify_client).to receive(:send_email).with( { email_address: user.email, @@ -324,4 +324,168 @@ RSpec.describe "User Features" do end end end + + context "when the user is a customer support person" do + let(:support_user) { FactoryBot.create(:user, :support, last_sign_in_at: Time.zone.now) } + let(:devise_notify_mailer) { DeviseNotifyMailer.new } + let(:notify_client) { instance_double(Notifications::Client) } + let(:mfa_template_id) { User::MFA_TEMPLATE_ID } + let(:otp) { "999111" } + + before do + allow(DeviseNotifyMailer).to receive(:new).and_return(devise_notify_mailer) + allow(devise_notify_mailer).to receive(:notify_client).and_return(notify_client) + allow(notify_client).to receive(:send_email).and_return(true) + visit("/logs") + fill_in("user[email]", with: support_user.email) + fill_in("user[password]", with: "pAssword1") + end + + context "when they are logging in" do + before do + allow(SecureRandom).to receive(:random_number).and_return(otp) + end + + it "shows the 2FA code screen" do + click_button("Sign in") + expect(page).to have_content("We’ve sent you an email with a security code.") + expect(page).to have_field("user[code]") + end + + it "sends a 2FA code by email" do + expect(notify_client).to receive(:send_email).with( + { + email_address: support_user.email, + template_id: mfa_template_id, + personalisation: { otp: }, + }, + ) + click_button("Sign in") + end + end + + context "with a valid 2FA code" do + before do + allow(SecureRandom).to receive(:random_number).and_return(otp) + end + + it "authenticates successfully" do + click_button("Sign in") + fill_in("code", with: otp) + click_button("Submit") + expect(page).to have_content("Logs") + expect(page).to have_content("Two factor authentication successful.") + end + + context "but it is more than 15 minutes old" do + it "does not authenticate successfully" do + click_button("Sign in") + support_user.update!(direct_otp_sent_at: 16.minutes.ago) + fill_in("code", with: otp) + click_button("Submit") + 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") + end + end + end + + context "with an invalid 2FA code" do + it "does not authenticate successfully" do + click_button("Sign in") + fill_in("code", with: otp) + click_button("Submit") + 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") + end + end + + context "when the 2FA code needs to be resent" do + before do + click_button("Sign in") + end + + it "displays the resend view" do + 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 an email?") + expect { click_button("Resend security code") }.to(change { support_user.reload.direct_otp }) + expect(page).to have_current_path("/account/two-factor-authentication") + end + end + + context "when signing in and out again" do + before do + allow(SecureRandom).to receive(:random_number).and_return(otp) + end + + it "requires the 2FA code on each login" do + click_button("Sign in") + fill_in("code", with: otp) + click_button("Submit") + click_link("Sign out") + visit("/logs") + fill_in("user[email]", with: support_user.email) + fill_in("user[password]", with: "pAssword1") + click_button("Sign in") + expect(page).to have_content("Check your email") + end + end + + context "when they have forgotten their password" do + let(:reset_password_token) { "MCDH5y6Km-U7CFPgAMVS" } + + before do + allow(Devise.token_generator).to receive(:generate).and_return(reset_password_token) + allow(DeviseNotifyMailer).to receive(:new).and_return(devise_notify_mailer) + allow(devise_notify_mailer).to receive(:notify_client).and_return(notify_client) + allow(notify_client).to receive(:send_email).and_return(true) + end + + it "is redirected to the reset password page when they click the reset password link" do + visit("/account/sign-in") + click_link("reset your password") + expect(page).to have_current_path("/account/password/new") + end + + it "is shown an error message if they submit without entering an email address" do + visit("/account/password/new") + click_button("Send email") + expect(page).to have_selector("#error-summary-title") + expect(page).to have_selector("#user-email-field-error") + expect(page).to have_title("Error") + end + + it "is redirected to login page after reset email is sent" do + visit("/account/password/new") + fill_in("user[email]", with: support_user.email) + click_button("Send email") + expect(page).to have_content("Check your email") + end + + it "is sent a reset password email via Notify" do + expect(notify_client).to receive(:send_email).with( + { + email_address: support_user.email, + template_id: support_user.reset_password_notify_template, + personalisation: { + name: support_user.name, + email: support_user.email, + organisation: support_user.organisation.name, + link: "http://localhost:3000/account/password/edit?reset_password_token=#{reset_password_token}", + }, + }, + ) + visit("/account/password/new") + fill_in("user[email]", with: support_user.email) + click_button("Send email") + end + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 99d27529a..e8d0008a9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -64,6 +64,23 @@ RSpec.describe User, type: :model do expect { user.is_data_protection_officer! } .to change { user.reload.is_data_protection_officer? }.from(false).to(true) end + + it "does not require 2FA" do + expect(user.need_two_factor_authentication?(nil)).to be false + end + + context "when the user is a Customer Support person" do + let(:user) { FactoryBot.create(:user, :support) } + let!(:other_orgs_log) { FactoryBot.create(:case_log) } + + it "has access to logs from all organisations" do + expect(user.case_logs.to_a).to eq([owned_case_log, managed_case_log, other_orgs_log]) + end + + it "requires 2FA" do + expect(user.need_two_factor_authentication?(nil)).to be true + end + end end describe "paper trail" do diff --git a/spec/requests/auth/passwords_controller_spec.rb b/spec/requests/auth/passwords_controller_spec.rb index 0243f75e2..1ba617598 100644 --- a/spec/requests/auth/passwords_controller_spec.rb +++ b/spec/requests/auth/passwords_controller_spec.rb @@ -80,8 +80,8 @@ RSpec.describe Auth::PasswordsController, type: :request do let(:new_value) { "new-password" } before do - allow(Sms).to receive(:notify_client).and_return(notify_client) - allow(notify_client).to receive(:send_sms).and_return(true) + allow(DeviseNotifyMailer).to receive(:notify_client).and_return(notify_client) + allow(notify_client).to receive(:send_email).and_return(true) end it "renders the user edit password view" do @@ -137,11 +137,83 @@ RSpec.describe Auth::PasswordsController, type: :request do expect(response).to redirect_to("/admin/two-factor-authentication") end - it "triggers an SMS" do - expect(notify_client).to receive(:send_sms) + it "triggers an email" do + expect(notify_client).to receive(:send_email) put "/admin/password", headers: headers, params: params end end end end + + context "when a customer support user" do + let(:support_user) { FactoryBot.create(:user, :support) } + + describe "reset password" do + let(:new_value) { "new-password" } + + before do + allow(DeviseNotifyMailer).to receive(:notify_client).and_return(notify_client) + allow(notify_client).to receive(:send_email).and_return(true) + end + + it "renders the user edit password view" do + _raw, enc = Devise.token_generator.generate(User, :reset_password_token) + get "/account/password/edit?reset_password_token=#{enc}" + expect(page).to have_css("h1", text: "Reset your password") + end + + context "when passwords entered don't match" do + let(:raw) { support_user.send_reset_password_instructions } + let(:params) do + { + id: support_user.id, + user: { + password: new_value, + password_confirmation: "something_else", + reset_password_token: raw, + }, + } + end + + it "shows an error" do + put "/account/password", headers: headers, params: params + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content("doesn't match Password") + end + end + + context "when passwords is reset" do + let(:raw) { support_user.send_reset_password_instructions } + let(:params) do + { + id: support_user.id, + user: { + password: new_value, + password_confirmation: new_value, + reset_password_token: raw, + }, + } + end + + it "updates the password" do + expect { + put "/account/password", headers: headers, params: params + support_user.reload + }.to change(support_user, :encrypted_password) + end + + it "sends you to the 2FA page and does not allow bypassing 2FA code" do + put "/account/password", headers: headers, params: params + expect(response).to redirect_to("/account/two-factor-authentication") + get "/logs", headers: headers + expect(response).to redirect_to("/account/two-factor-authentication") + end + + it "triggers an email" do + expect(notify_client).to receive(:send_email) + put "/account/password", headers: headers, params: params + end + end + end + end end diff --git a/spec/requests/case_logs_controller_spec.rb b/spec/requests/case_logs_controller_spec.rb index 7adad985b..0526ee7ed 100644 --- a/spec/requests/case_logs_controller_spec.rb +++ b/spec/requests/case_logs_controller_spec.rb @@ -159,106 +159,129 @@ RSpec.describe CaseLogsController, type: :request do context "when displaying a collection of logs" do let(:headers) { { "Accept" => "text/html" } } - before do - sign_in user - end + context "when the user is a customer support user" do + let(:user) { FactoryBot.create(:user, :support) } - context "when there are less than 20 logs" do before do - get "/logs", headers: headers, params: {} + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user end - it "shows a table of logs" do - expect(CGI.unescape_html(response.body)).to match(/