Browse Source

CLDC-1100: Add a Customer Support user role (#454)

* A support role exists that can see all case logs

* Support role requires 2FA

* Support user sees 2FA code screen on login

* Use email for OTP code

* Ensure resend paths work

* Support user can see additional organisation columns on logs page

* Simpler test

* Remove spec description spaces
pull/457/head
baarkerlounger 3 years ago committed by GitHub
parent
commit
0e9f7d12d2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 18
      Gemfile.lock
  2. 6
      app/controllers/auth/passwords_controller.rb
  3. 4
      app/controllers/auth/sessions_controller.rb
  4. 6
      app/models/admin_user.rb
  5. 36
      app/models/user.rb
  6. 15
      app/services/sms.rb
  7. 12
      app/views/case_logs/_log_list.html.erb
  8. 4
      app/views/devise/two_factor_authentication/resend.html.erb
  9. 9
      app/views/devise/two_factor_authentication/show.html.erb
  10. 8
      config/routes.rb
  11. 15
      db/migrate/20220406093139_two_factor_authentication_add_to_user.rb
  12. 8
      db/schema.rb
  13. 3
      spec/factories/user.rb
  14. 31
      spec/features/admin_panel_spec.rb
  15. 7
      spec/features/auth/user_lockout_spec.rb
  16. 186
      spec/features/user_spec.rb
  17. 17
      spec/models/user_spec.rb
  18. 80
      spec/requests/auth/passwords_controller_spec.rb
  19. 159
      spec/requests/case_logs_controller_spec.rb

18
Gemfile.lock

@ -12,7 +12,7 @@ GIT
GIT GIT
remote: https://github.com/baarkerlounger/two_factor_authentication.git remote: https://github.com/baarkerlounger/two_factor_authentication.git
revision: c2237dedb89b1fc53101cec536e57912049c5412 revision: afb91d5ffabbdb79ca29645749ef625f7e3a76ea
specs: specs:
two_factor_authentication (2.2.0) two_factor_authentication (2.2.0)
devise devise
@ -105,7 +105,7 @@ GEM
ruby2_keywords (>= 0.0.2, < 1.0) ruby2_keywords (>= 0.0.2, < 1.0)
ast (2.4.2) ast (2.4.2)
aws-eventstream (1.2.0) aws-eventstream (1.2.0)
aws-partitions (1.571.0) aws-partitions (1.573.0)
aws-sdk-core (3.130.0) aws-sdk-core (3.130.0)
aws-eventstream (~> 1, >= 1.0.2) aws-eventstream (~> 1, >= 1.0.2)
aws-partitions (~> 1, >= 1.525.0) aws-partitions (~> 1, >= 1.525.0)
@ -160,7 +160,7 @@ GEM
railties (>= 3.2) railties (>= 3.2)
encryptor (3.0.0) encryptor (3.0.0)
erubi (1.10.0) erubi (1.10.0)
excon (0.92.1) excon (0.92.2)
factory_bot (6.2.1) factory_bot (6.2.1)
activesupport (>= 5.0.0) activesupport (>= 5.0.0)
factory_bot_rails (6.2.0) factory_bot_rails (6.2.0)
@ -196,7 +196,6 @@ GEM
railties (>= 5.2, < 7.1) railties (>= 5.2, < 7.1)
responders (>= 2, < 4) responders (>= 2, < 4)
iniparse (1.5.0) iniparse (1.5.0)
io-wait (0.2.1)
jmespath (1.6.1) jmespath (1.6.1)
jquery-rails (4.4.0) jquery-rails (4.4.0)
rails-dom-testing (>= 1, < 3) rails-dom-testing (>= 1, < 3)
@ -222,7 +221,7 @@ GEM
listen (3.7.1) listen (3.7.1)
rb-fsevent (~> 0.10, >= 0.10.3) rb-fsevent (~> 0.10, >= 0.10.3)
rb-inotify (~> 0.9, >= 0.9.10) rb-inotify (~> 0.9, >= 0.9.10)
loofah (2.15.0) loofah (2.16.0)
crass (~> 1.0.2) crass (~> 1.0.2)
nokogiri (>= 1.5.9) nokogiri (>= 1.5.9)
mail (2.7.1) mail (2.7.1)
@ -232,7 +231,7 @@ GEM
method_source (1.0.0) method_source (1.0.0)
mini_mime (1.1.2) mini_mime (1.1.2)
minitest (5.15.0) minitest (5.15.0)
msgpack (1.4.5) msgpack (1.5.0)
net-imap (0.2.3) net-imap (0.2.3)
digest digest
net-protocol net-protocol
@ -241,8 +240,7 @@ GEM
digest digest
net-protocol net-protocol
timeout timeout
net-protocol (0.1.2) net-protocol (0.1.3)
io-wait
timeout timeout
net-smtp (0.3.1) net-smtp (0.3.1)
digest digest
@ -273,7 +271,7 @@ GEM
parallel (1.22.1) parallel (1.22.1)
parser (3.1.1.0) parser (3.1.1.0)
ast (~> 2.4.1) ast (~> 2.4.1)
pg (1.3.4) pg (1.3.5)
postcodes_io (0.4.0) postcodes_io (0.4.0)
excon (~> 0.39) excon (~> 0.39)
propshaft (0.6.4) propshaft (0.6.4)
@ -352,7 +350,7 @@ GEM
rspec-expectations (3.11.0) rspec-expectations (3.11.0)
diff-lcs (>= 1.2.0, < 2.0) diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.11.0) rspec-support (~> 3.11.0)
rspec-mocks (3.11.0) rspec-mocks (3.11.1)
diff-lcs (>= 1.2.0, < 2.0) diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.11.0) rspec-support (~> 3.11.0)
rspec-rails (5.1.1) rspec-rails (5.1.1)

6
app/controllers/auth/passwords_controller.rb

@ -58,7 +58,7 @@ protected
end end
def password_update_flash_message def password_update_flash_message
resource_class == AdminUser ? :updated_2FA : :updated resource.need_two_factor_authentication?(request) ? :updated_2FA : :updated
end end
def resource_class_name def resource_class_name
@ -71,9 +71,9 @@ protected
def after_resetting_password_path_for(resource) def after_resetting_password_path_for(resource)
if Devise.sign_in_after_reset_password if Devise.sign_in_after_reset_password
if resource_class == AdminUser if resource.need_two_factor_authentication?(request)
resource.send_new_otp resource.send_new_otp
admin_user_two_factor_authentication_path send("#{resource_name}_two_factor_authentication_path")
else else
after_sign_in_path_for(resource) after_sign_in_path_for(resource)
end end

4
app/controllers/auth/sessions_controller.rb

@ -29,8 +29,8 @@ private
end end
def after_sign_in_path_for(resource) def after_sign_in_path_for(resource)
if resource_class == AdminUser if resource.need_two_factor_authentication?(request)
admin_user_two_factor_authentication_path send("#{resource_name}_two_factor_authentication_path")
else else
params.dig(resource_class_name, "start").present? ? case_logs_path : super params.dig(resource_class_name, "start").present? ? case_logs_path : super
end end

6
app/models/admin_user.rb

@ -21,13 +21,13 @@ class AdminUser < ApplicationRecord
validates :phone, presence: true, numericality: true 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 RESET_PASSWORD_TEMPLATE_ID = "fbb2d415-b9b1-4507-ba0a-6e542fa3504d".freeze
def send_two_factor_authentication_code(code) def send_two_factor_authentication_code(code)
template_id = MFA_SMS_TEMPLATE_ID template_id = MFA_TEMPLATE_ID
personalisation = { otp: code } personalisation = { otp: code }
Sms.send(phone, template_id, personalisation) DeviseNotifyMailer.new.send_email(email, template_id, personalisation)
end end
def reset_password_notify_template def reset_password_notify_template

36
app/models/user.rb

@ -2,7 +2,7 @@ class User < ApplicationRecord
# Include default devise modules. Others available are: # Include default devise modules. Others available are:
# :confirmable, :timeoutable and :omniauthable # :confirmable, :timeoutable and :omniauthable
devise :database_authenticatable, :recoverable, :rememberable, :validatable, devise :database_authenticatable, :recoverable, :rememberable, :validatable,
:trackable, :lockable :trackable, :lockable, :two_factor_authenticatable
belongs_to :organisation belongs_to :organisation
has_many :owned_case_logs, through: :organisation has_many :owned_case_logs, through: :organisation
@ -21,16 +21,23 @@ class User < ApplicationRecord
sign_in_count sign_in_count
updated_at] updated_at]
has_one_time_password(encrypted: true)
ROLES = { ROLES = {
data_accessor: 0, data_accessor: 0,
data_provider: 1, data_provider: 1,
data_coordinator: 2, data_coordinator: 2,
support: 99,
}.freeze }.freeze
enum role: ROLES enum role: ROLES
def case_logs def case_logs
CaseLog.for_organisation(organisation) if support?
CaseLog.all
else
CaseLog.for_organisation(organisation)
end
end end
def completed_case_logs def completed_case_logs
@ -41,13 +48,6 @@ class User < ApplicationRecord
case_logs.not_completed case_logs.not_completed
end 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? def is_key_contact?
is_key_contact is_key_contact
end end
@ -63,4 +63,22 @@ class User < ApplicationRecord
def is_data_protection_officer! def is_data_protection_officer!
update!(is_dpo: true) update!(is_dpo: true)
end 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 end

15
app/services/sms.rb

@ -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

12
app/views/case_logs/_log_list.html.erb

@ -15,6 +15,10 @@
<th class="govuk-table__header" scope="col">Tenancy starts</th> <th class="govuk-table__header" scope="col">Tenancy starts</th>
<th class="govuk-table__header" scope="col">Log created</th> <th class="govuk-table__header" scope="col">Log created</th>
<th class="govuk-table__header" scope="col">Completed</th> <th class="govuk-table__header" scope="col">Completed</th>
<% if current_user.support? %>
<th class="govuk-table__header" scope="col">Owning organisation</th>
<th class="govuk-table__header" scope="col">Managing organisation</th>
<% end %>
</tr> </tr>
</thead> </thead>
<tbody class="govuk-table__body"> <tbody class="govuk-table__body">
@ -41,6 +45,14 @@
text: log.status.humanize text: log.status.humanize
) %> ) %>
</td> </td>
<% if current_user.support? %>
<td class="govuk-table__cell">
<%= log.owning_organisation.name %>
</td>
<td class="govuk-table__cell">
<%= log.managing_organisation.name %>
</td>
<% end %>
</tr> </tr>
<% end %> <% end %>
</tbody> </tbody>

4
app/views/devise/two_factor_authentication/resend.html.erb

@ -7,7 +7,7 @@
) %> ) %>
<% end %> <% end %>
<%= form_with(url: resend_code_admin_user_two_factor_authentication_path, html: { method: :get }) do |f| %> <%= form_with(url: send("resend_code_#{resource_name}_two_factor_authentication_path"), html: { method: :get }) do |f| %>
<div class="govuk-grid-row"> <div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds"> <div class="govuk-grid-column-two-thirds">
@ -15,7 +15,7 @@
<%= content_for(:title) %> <%= content_for(:title) %>
</h1> </h1>
<p class="govuk-body">Text messages sometimes take a few minutes to arrive. If you do not receive the text message, you can request a new one.</p> <p class="govuk-body">Emails sometimes take a few minutes to arrive. If you do not receive the email, you can request a new one.</p>
<%= f.govuk_submit "Resend security code" %> <%= f.govuk_submit "Resend security code" %>
</div> </div>

9
app/views/devise/two_factor_authentication/show.html.erb

@ -1,6 +1,7 @@
<% 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| %> <% url_prefix = resource_name == :user ? "account" : "admin" %>
<%= form_with(model: resource, url: "/#{url_prefix}/two-factor-authentication", html: { method: :put }) do |f| %>
<div class="govuk-grid-row"> <div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds"> <div class="govuk-grid-column-two-thirds">
<%= f.govuk_error_summary %> <%= f.govuk_error_summary %>
@ -9,7 +10,7 @@
<%= content_for(:title) %> <%= content_for(:title) %>
</h1> </h1>
<p class="govuk-body">We’ve sent you a text message with a security code.</p> <p class="govuk-body">We’ve sent you an email with a security code.</p>
<%= f.govuk_number_field :code, <%= f.govuk_number_field :code,
label: { text: "Security code", size: "m" }, label: { text: "Security code", size: "m" },
@ -24,5 +25,5 @@
<% end %> <% end %>
<p class="govuk-body"> <p class="govuk-body">
<%= 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") %>
</p> </p>

8
config/routes.rb

@ -14,12 +14,13 @@ Rails.application.routes.draw do
sign_in: "sign-in", sign_in: "sign-in",
sign_out: "sign-out", sign_out: "sign-out",
two_factor_authentication: "two-factor-authentication", two_factor_authentication: "two-factor-authentication",
two_factor_authentication_resend_code: "resend-code",
}, },
sign_out_via: %i[get], sign_out_via: %i[get],
} }
devise_scope :admin_user do 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 end
devise_for :users, { devise_for :users, {
@ -27,15 +28,20 @@ Rails.application.routes.draw do
controllers: { controllers: {
passwords: "auth/passwords", passwords: "auth/passwords",
sessions: "auth/sessions", sessions: "auth/sessions",
two_factor_authentication: "auth/two_factor_authentication",
}, },
path_names: { path_names: {
sign_in: "sign-in", sign_in: "sign-in",
sign_out: "sign-out", 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 devise_scope :user do
get "account/password/reset-confirmation", to: "auth/passwords#reset_confirmation" 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" put "account", to: "users#update"
end end

15
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

8
db/schema.rb

@ -324,7 +324,15 @@ ActiveRecord::Schema[7.0].define(version: 202202071123100) do
t.boolean "is_dpo", default: false t.boolean "is_dpo", default: false
t.boolean "is_key_contact", default: false t.boolean "is_key_contact", default: false
t.string "phone" 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 ["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 ["organisation_id"], name: "index_users_on_organisation_id"
t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true 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 t.index ["unlock_token"], name: "index_users_on_unlock_token", unique: true

3
spec/factories/user.rb

@ -11,6 +11,9 @@ FactoryBot.define do
trait :data_protection_officer do trait :data_protection_officer do
is_dpo { true } is_dpo { true }
end end
trait :support do
role { "support" }
end
created_at { Time.zone.now } created_at { Time.zone.now }
updated_at { Time.zone.now } updated_at { Time.zone.now }
end end

31
spec/features/admin_panel_spec.rb

@ -2,13 +2,15 @@ require "rails_helper"
RSpec.describe "Admin Panel" do RSpec.describe "Admin Panel" do
let!(:admin) { FactoryBot.create(:admin_user) } let!(:admin) { FactoryBot.create(:admin_user) }
let(:devise_notify_mailer) { DeviseNotifyMailer.new }
let(:notify_client) { instance_double(Notifications::Client) } 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" } let(:otp) { "999111" }
before do before do
allow(Sms).to receive(:notify_client).and_return(notify_client) allow(DeviseNotifyMailer).to receive(:new).and_return(devise_notify_mailer)
allow(notify_client).to receive(:send_sms).and_return(true) allow(devise_notify_mailer).to receive(:notify_client).and_return(notify_client)
allow(notify_client).to receive(:send_email).and_return(true)
end end
it "shows the admin sign in page" do it "shows the admin sign in page" do
@ -26,8 +28,12 @@ RSpec.describe "Admin Panel" do
end end
it "authenticates successfully" do it "authenticates successfully" do
expect(notify_client).to receive(:send_sms).with( expect(notify_client).to receive(:send_email).with(
hash_including(phone_number: admin.phone, template_id: mfa_template_id), {
email_address: admin.email,
template_id: mfa_template_id,
personalisation: { otp: },
},
) )
click_button("Sign in") click_button("Sign in")
fill_in("code", with: otp) fill_in("code", with: otp)
@ -42,7 +48,7 @@ RSpec.describe "Admin Panel" do
admin.update!(direct_otp_sent_at: 16.minutes.ago) admin.update!(direct_otp_sent_at: 16.minutes.ago)
fill_in("code", with: otp) fill_in("code", with: otp)
click_button("Submit") 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_http_status(:unprocessable_entity)
expect(page).to have_title("Error") expect(page).to have_title("Error")
expect(page).to have_selector("#error-summary-title") expect(page).to have_selector("#error-summary-title")
@ -58,7 +64,7 @@ RSpec.describe "Admin Panel" do
click_button("Sign in") click_button("Sign in")
fill_in("code", with: otp) fill_in("code", with: otp)
click_button("Submit") 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_http_status(:unprocessable_entity)
expect(page).to have_title("Error") expect(page).to have_title("Error")
expect(page).to have_selector("#error-summary-title") expect(page).to have_selector("#error-summary-title")
@ -74,12 +80,12 @@ RSpec.describe "Admin Panel" do
end end
it "displays the resend view" do 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") expect(page).to have_button("Resend security code")
end end
it "send a new OTP code and redirects back to the 2FA view" do 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 { click_button("Resend security code") }.to(change { admin.reload.direct_otp })
expect(page).to have_current_path("/admin/two-factor-authentication") expect(page).to have_current_path("/admin/two-factor-authentication")
end end
@ -102,20 +108,15 @@ RSpec.describe "Admin Panel" do
fill_in("admin_user[email]", with: admin.email) fill_in("admin_user[email]", with: admin.email)
fill_in("admin_user[password]", with: admin.password) fill_in("admin_user[password]", with: admin.password)
click_button("Sign in") click_button("Sign in")
expect(page).to have_content("Check your phone") expect(page).to have_content("Check your email")
end end
end end
context "when the admin has forgotten their password" do context "when the admin has forgotten their password" do
let!(:admin_user) { FactoryBot.create(:admin_user, last_sign_in_at: Time.zone.now) } 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(:reset_password_token) { "MCDH5y6Km-U7CFPgAMVS" }
let(:devise_notify_mailer) { DeviseNotifyMailer.new }
before do 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) allow(Devise.token_generator).to receive(:generate).and_return(reset_password_token)
end end

7
spec/features/auth/user_lockout_spec.rb

@ -48,9 +48,12 @@ RSpec.describe "User Lockout" do
end end
context "when login-in with the right admin password and incorrect 2FA token up to a maximum number of attempts" do 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 before do
allow(Sms).to receive(:notify_client).and_return(notify_client) allow(DeviseNotifyMailer).to receive(:new).and_return(devise_notify_mailer)
allow(notify_client).to receive(:send_sms).and_return(true) 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") visit("/admin/sign-in")
fill_in("admin_user[email]", with: admin.email) fill_in("admin_user[email]", with: admin.email)

186
spec/features/user_spec.rb

@ -15,7 +15,7 @@ RSpec.describe "User Features" do
end end
context "when the user navigates to case logs" do 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") visit("/logs")
expect(page).to have_current_path("/account/sign-in") expect(page).to have_current_path("/account/sign-in")
expect(page).to have_content("Sign in to your account to submit CORE data") 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.") expect(page).to have_no_content("You need to sign in or sign up before continuing.")
end end
it " is redirected to case logs after signing in" do it "is redirected to case logs after signing in" do
visit("/logs") visit("/logs")
fill_in("user[email]", with: user.email) fill_in("user[email]", with: user.email)
fill_in("user[password]", with: "pAssword1") fill_in("user[password]", with: "pAssword1")
@ -34,7 +34,7 @@ RSpec.describe "User Features" do
expect(page).to have_current_path("/logs") expect(page).to have_current_path("/logs")
end end
it " can log out again", js: true do it "can log out again", js: true do
visit("/logs") visit("/logs")
fill_in("user[email]", with: user.email) fill_in("user[email]", with: user.email)
fill_in("user[password]", with: "pAssword1") fill_in("user[password]", with: "pAssword1")
@ -44,7 +44,7 @@ RSpec.describe "User Features" do
expect(page).to have_content("Start now") expect(page).to have_content("Start now")
end end
it " can log out again with js disabled" do it "can log out again with js disabled" do
visit("/logs") visit("/logs")
fill_in("user[email]", with: user.email) fill_in("user[email]", with: user.email)
fill_in("user[password]", with: "pAssword1") fill_in("user[password]", with: "pAssword1")
@ -56,13 +56,13 @@ RSpec.describe "User Features" do
end end
context "when the user has forgotten their password" do 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") visit("/logs")
click_link("reset your password") click_link("reset your password")
expect(page).to have_current_path("/account/password/new") expect(page).to have_current_path("/account/password/new")
end 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") visit("/account/password/new")
click_button("Send email") click_button("Send email")
expect(page).to have_selector("#error-summary-title") expect(page).to have_selector("#error-summary-title")
@ -70,7 +70,7 @@ RSpec.describe "User Features" do
expect(page).to have_title("Error") expect(page).to have_title("Error")
end 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") visit("/account/password/new")
fill_in("user[email]", with: "thisisn'tanemail") fill_in("user[email]", with: "thisisn'tanemail")
click_button("Send email") click_button("Send email")
@ -79,28 +79,28 @@ RSpec.describe "User Features" do
expect(page).to have_title("Error") expect(page).to have_title("Error")
end 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") visit("/account/password/new")
fill_in("user[email]", with: user.email) fill_in("user[email]", with: user.email)
click_button("Send email") click_button("Send email")
expect(page).to have_content("Check your email") expect(page).to have_content("Check your email")
end 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") visit("/account/password/new")
fill_in("user[email]", with: user.email) fill_in("user[email]", with: user.email)
click_button("Send email") click_button("Send email")
expect(page).to have_content(user.email) expect(page).to have_content(user.email)
end 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") visit("/account/password/new")
fill_in("user[email]", with: "idontexist@example.com") fill_in("user[email]", with: "idontexist@example.com")
click_button("Send email") click_button("Send email")
expect(page).to have_current_path("/account/password/reset-confirmation?email=idontexist%40example.com") expect(page).to have_current_path("/account/password/reset-confirmation?email=idontexist%40example.com")
end 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( expect(notify_client).to receive(:send_email).with(
{ {
email_address: user.email, email_address: user.email,
@ -324,4 +324,168 @@ RSpec.describe "User Features" do
end end
end 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 end

17
spec/models/user_spec.rb

@ -64,6 +64,23 @@ RSpec.describe User, type: :model do
expect { user.is_data_protection_officer! } expect { user.is_data_protection_officer! }
.to change { user.reload.is_data_protection_officer? }.from(false).to(true) .to change { user.reload.is_data_protection_officer? }.from(false).to(true)
end 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 end
describe "paper trail" do describe "paper trail" do

80
spec/requests/auth/passwords_controller_spec.rb

@ -80,8 +80,8 @@ RSpec.describe Auth::PasswordsController, type: :request do
let(:new_value) { "new-password" } let(:new_value) { "new-password" }
before do before do
allow(Sms).to receive(:notify_client).and_return(notify_client) allow(DeviseNotifyMailer).to receive(:notify_client).and_return(notify_client)
allow(notify_client).to receive(:send_sms).and_return(true) allow(notify_client).to receive(:send_email).and_return(true)
end end
it "renders the user edit password view" do 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") expect(response).to redirect_to("/admin/two-factor-authentication")
end end
it "triggers an SMS" do it "triggers an email" do
expect(notify_client).to receive(:send_sms) expect(notify_client).to receive(:send_email)
put "/admin/password", headers: headers, params: params put "/admin/password", headers: headers, params: params
end end
end end
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 end

159
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 context "when displaying a collection of logs" do
let(:headers) { { "Accept" => "text/html" } } let(:headers) { { "Accept" => "text/html" } }
before do context "when the user is a customer support user" do
sign_in user let(:user) { FactoryBot.create(:user, :support) }
end
context "when there are less than 20 logs" do
before do before do
get "/logs", headers: headers, params: {} allow(user).to receive(:need_two_factor_authentication?).and_return(false)
sign_in user
end end
it "shows a table of logs" do it "does have organisation columns" do
expect(CGI.unescape_html(response.body)).to match(/<table class="govuk-table">/) get "/logs", headers: headers, params: {}
expect(CGI.unescape_html(response.body)).to match(/logs/) expect(page).to have_content("Owning organisation")
end expect(page).to have_content("Managing organisation")
it "only shows case logs for your organisation" do
expected_case_row_log = "<a class=\"govuk-link\" href=\"/logs/#{case_log.id}\">#{case_log.id}</a>"
unauthorized_case_row_log = "<a class=\"govuk-link\" href=\"/logs/#{unauthorized_case_log.id}\">#{unauthorized_case_log.id}</a>"
expect(CGI.unescape_html(response.body)).to include(expected_case_row_log)
expect(CGI.unescape_html(response.body)).not_to include(unauthorized_case_row_log)
end
it "shows the formatted created at date for each log" do
formatted_date = case_log.created_at.to_formatted_s(:govuk_date)
expect(CGI.unescape_html(response.body)).to include(formatted_date)
end end
end
it "shows the log's status" do context "when the user is not a customer support user" do
expect(CGI.unescape_html(response.body)).to include(case_log.status.humanize) before do
sign_in user
end end
it "shows the total log count" do it "does not have organisation columns" do
expect(CGI.unescape_html(response.body)).to match("<strong>1</strong> total logs") get "/logs", headers: headers, params: {}
expect(page).not_to have_content("Owning organisation")
expect(page).not_to have_content("Managing organisation")
end end
it "does not show the pagination links" do context "when there are less than 20 logs" do
expect(page).not_to have_link("Previous") before do
expect(page).not_to have_link("Next") get "/logs", headers: headers, params: {}
end end
it "does not show the pagination result line" do it "shows a table of logs" do
expect(CGI.unescape_html(response.body)).not_to match("Showing <b>1</b> to <b>20</b> of <b>26</b> logs") expect(CGI.unescape_html(response.body)).to match(/<table class="govuk-table">/)
end expect(CGI.unescape_html(response.body)).to match(/logs/)
end
it "does not have pagination in the title" do it "only shows case logs for your organisation" do
expect(page).to have_title("Logs") expected_case_row_log = "<a class=\"govuk-link\" href=\"/logs/#{case_log.id}\">#{case_log.id}</a>"
end unauthorized_case_row_log = "<a class=\"govuk-link\" href=\"/logs/#{unauthorized_case_log.id}\">#{unauthorized_case_log.id}</a>"
expect(CGI.unescape_html(response.body)).to include(expected_case_row_log)
expect(CGI.unescape_html(response.body)).not_to include(unauthorized_case_row_log)
end
it "shows the download csv link" do it "shows the formatted created at date for each log" do
expect(page).to have_link("Download (CSV)", href: "/logs.csv") formatted_date = case_log.created_at.to_formatted_s(:govuk_date)
end expect(CGI.unescape_html(response.body)).to include(formatted_date)
end end
context "when there are more than 20 logs" do it "shows the log's status" do
before do expect(CGI.unescape_html(response.body)).to include(case_log.status.humanize)
FactoryBot.create_list(:case_log, 25, owning_organisation: organisation, managing_organisation: organisation) end
end
context "when on the first page" do it "shows the total log count" do
before do expect(CGI.unescape_html(response.body)).to match("<strong>1</strong> total logs")
get "/logs", headers: headers, params: {}
end end
it "has pagination links" do it "does not show the pagination links" do
expect(page).to have_content("Previous")
expect(page).not_to have_link("Previous") expect(page).not_to have_link("Previous")
expect(page).to have_content("Next") expect(page).not_to have_link("Next")
expect(page).to have_link("Next")
end end
it "shows which logs are being shown on the current page" do it "does not show the pagination result line" do
expect(CGI.unescape_html(response.body)).to match("Showing <b>1</b> to <b>20</b> of <b>26</b> logs") expect(CGI.unescape_html(response.body)).not_to match("Showing <b>1</b> to <b>20</b> of <b>26</b> logs")
end end
it "has pagination in the title" do it "does not have pagination in the title" do
expect(page).to have_title("Logs (page 1 of 2)") expect(page).to have_title("Logs")
end
it "shows the download csv link" do
expect(page).to have_link("Download (CSV)", href: "/logs.csv")
end end
end end
context "when on the second page" do context "when there are more than 20 logs" do
before do before do
get "/logs?page=2", headers: headers, params: {} FactoryBot.create_list(:case_log, 25, owning_organisation: organisation, managing_organisation: organisation)
end end
it "shows the total log count" do context "when on the first page" do
expect(CGI.unescape_html(response.body)).to match("<strong>26</strong> total logs") before do
end get "/logs", headers: headers, params: {}
end
it "has pagination links" do it "has pagination links" do
expect(page).to have_content("Previous") expect(page).to have_content("Previous")
expect(page).to have_link("Previous") expect(page).not_to have_link("Previous")
expect(page).to have_content("Next") expect(page).to have_content("Next")
expect(page).not_to have_link("Next") expect(page).to have_link("Next")
end end
it "shows which logs are being shown on the current page" do it "shows which logs are being shown on the current page" do
expect(CGI.unescape_html(response.body)).to match("Showing <b>21</b> to <b>26</b> of <b>26</b> logs") expect(CGI.unescape_html(response.body)).to match("Showing <b>1</b> to <b>20</b> of <b>26</b> logs")
end
it "has pagination in the title" do
expect(page).to have_title("Logs (page 1 of 2)")
end
end end
it "has pagination in the title" do context "when on the second page" do
expect(page).to have_title("Logs (page 2 of 2)") before do
get "/logs?page=2", headers: headers, params: {}
end
it "shows the total log count" do
expect(CGI.unescape_html(response.body)).to match("<strong>26</strong> total logs")
end
it "has pagination links" do
expect(page).to have_content("Previous")
expect(page).to have_link("Previous")
expect(page).to have_content("Next")
expect(page).not_to have_link("Next")
end
it "shows which logs are being shown on the current page" do
expect(CGI.unescape_html(response.body)).to match("Showing <b>21</b> to <b>26</b> of <b>26</b> logs")
end
it "has pagination in the title" do
expect(page).to have_title("Logs (page 2 of 2)")
end
end end
end end
end end

Loading…
Cancel
Save