From 326644216e29dced9110df4132b2f1c412b0ab13 Mon Sep 17 00:00:00 2001 From: baarkerlounger <5101747+baarkerlounger@users.noreply.github.com> Date: Thu, 19 May 2022 11:33:52 +0100 Subject: [PATCH] Confirmable (#580) * Confirmable * Remove obsolete rake task * Skip confirmation for inactive users * Send beta onboarding template if migrated from Softwire * Default controller * Use correct link * Redirect confirmation to set password * Confirm account within 3 days * Only redirect to set password if not previously set * Rubocop * Confirm factory bot users * Set password condition * Changing email requires reconfirming * No need to explicitly trigger email, devise does that for us now * Remove flash banner * Mock notify * Mock in the right spec * Test redirect and text * User is confirmable * Rubocop * Redirect to url so we don't bypass authenticity token * Update content * Add test for resend invite flow * Update link to resend confirmation email * Rename password reset resend confirmation partial * Expired link error page * Remove resend confirmation link * Update seed * Expory contact * Time zone Co-authored-by: Paul Robert Lloyd --- .../auth/confirmations_controller.rb | 20 ++++++++ app/controllers/auth/passwords_controller.rb | 3 +- app/controllers/users_controller.rb | 1 - app/mailers/devise_notify_mailer.rb | 31 +++++++++---- app/models/user.rb | 25 ++++++++-- .../devise/confirmations/expired.html.erb | 11 +++++ app/views/devise/confirmations/new.html.erb | 33 +++++++++---- .../devise/passwords/reset_password.html.erb | 2 +- .../reset_resend_confirmation.html.erb} | 0 app/views/devise/shared/_links.html.erb | 2 +- config/initializers/devise.rb | 2 +- config/locales/devise.en.yml | 6 +-- config/locales/en.yml | 3 ++ config/routes.rb | 1 + .../20220517093906_add_confirmable_users.rb | 11 +++++ db/schema.rb | 5 ++ db/seeds.rb | 3 ++ lib/tasks/onboarding_emails.rake | 22 --------- .../admin/users_controller_spec.rb | 5 ++ spec/factories/user.rb | 2 + spec/features/organisation_spec.rb | 8 ++-- spec/lib/tasks/onboarding_emails_spec.rb | 41 ----------------- spec/models/user_spec.rb | 12 +++++ .../auth/confirmations_controller_spec.rb | 46 +++++++++++++++++++ .../auth/passwords_controller_spec.rb | 2 +- spec/requests/users_controller_spec.rb | 12 ++--- .../imports/user_import_service_spec.rb | 8 ++++ 27 files changed, 214 insertions(+), 103 deletions(-) create mode 100644 app/controllers/auth/confirmations_controller.rb create mode 100644 app/views/devise/confirmations/expired.html.erb rename app/views/devise/{confirmations/reset.html.erb => passwords/reset_resend_confirmation.html.erb} (100%) create mode 100644 db/migrate/20220517093906_add_confirmable_users.rb delete mode 100644 lib/tasks/onboarding_emails.rake delete mode 100644 spec/lib/tasks/onboarding_emails_spec.rb create mode 100644 spec/requests/auth/confirmations_controller_spec.rb diff --git a/app/controllers/auth/confirmations_controller.rb b/app/controllers/auth/confirmations_controller.rb new file mode 100644 index 000000000..4c8a4746c --- /dev/null +++ b/app/controllers/auth/confirmations_controller.rb @@ -0,0 +1,20 @@ +class Auth::ConfirmationsController < Devise::ConfirmationsController + # GET /resource/confirmation?confirmation_token=abcdef + def show + self.resource = resource_class.confirm_by_token(params[:confirmation_token]) + yield resource if block_given? + + if resource.errors.empty? + if resource.sign_in_count.zero? + token = resource.send(:set_reset_password_token) + redirect_to "#{edit_user_password_url}?reset_password_token=#{token}&confirmation=true" + else + respond_with_navigational(resource) { redirect_to after_confirmation_path_for(resource_name, resource) } + end + elsif resource.errors.map(&:type).include?(:confirmation_period_expired) + render "devise/confirmations/expired" + else + respond_with_navigational(resource.errors, status: :unprocessable_entity) { render :new } + end + end +end diff --git a/app/controllers/auth/passwords_controller.rb b/app/controllers/auth/passwords_controller.rb index 3ce603ade..c26529157 100644 --- a/app/controllers/auth/passwords_controller.rb +++ b/app/controllers/auth/passwords_controller.rb @@ -11,7 +11,7 @@ class Auth::PasswordsController < Devise::PasswordsController resource.errors.add :email, I18n.t("validations.email.invalid") render "devise/passwords/new", status: :unprocessable_entity else - render "devise/confirmations/reset" + render "devise/passwords/reset_resend_confirmation" end end @@ -24,6 +24,7 @@ class Auth::PasswordsController < Devise::PasswordsController def edit super + @confirmation = params["confirmation"] render "devise/passwords/reset_password" end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index f3ba40f1d..1bd7c4c7e 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -62,7 +62,6 @@ class UsersController < ApplicationController else user = User.create(user_params.merge(org_params).merge(password_params)) if user.persisted? - user.send_reset_password_instructions redirect_to created_user_redirect_path else @resource.errors.add :email, I18n.t("validations.email.taken") diff --git a/app/mailers/devise_notify_mailer.rb b/app/mailers/devise_notify_mailer.rb index 3d44cbec2..cd5500d86 100644 --- a/app/mailers/devise_notify_mailer.rb +++ b/app/mailers/devise_notify_mailer.rb @@ -13,21 +13,34 @@ class DeviseNotifyMailer < Devise::Mailer ) end - def reset_password_instructions(record, token, _opts = {}) - url = public_send("edit_#{record.class.name.underscore}_password_url") - personalisation = { + def personalisation(record, token, url) + { name: record.name || record.email, email: record.email, organisation: record.respond_to?(:organisation) ? record.organisation.name : "", - link: "#{url}?reset_password_token=#{token}", + link: "#{url}#{token}", } - send_email(record.email, record.reset_password_notify_template, personalisation) end - # def confirmation_instructions(record, token, _opts = {}) - # super - # end - # + def reset_password_instructions(record, token, _opts = {}) + base = public_send("edit_#{record.class.name.underscore}_password_url") + url = "#{base}?reset_password_token=" + send_email( + record.email, + record.reset_password_notify_template, + personalisation(record, token, url), + ) + end + + def confirmation_instructions(record, token, _opts = {}) + url = "#{user_confirmation_url}?confirmation_token=" + send_email( + record.email, + record.confirmable_template, + personalisation(record, token, url), + ) + end + # def unlock_instructions(record, token, opts = {}) # super # end diff --git a/app/models/user.rb b/app/models/user.rb index c458f80cd..364b85351 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,8 +1,8 @@ class User < ApplicationRecord # Include default devise modules. Others available are: - # :confirmable, :timeoutable and :omniauthable + # :omniauthable devise :database_authenticatable, :recoverable, :rememberable, :validatable, - :trackable, :lockable, :two_factor_authenticatable + :trackable, :lockable, :two_factor_authenticatable, :confirmable, :timeoutable belongs_to :organisation has_many :owned_case_logs, through: :organisation @@ -66,10 +66,27 @@ class User < ApplicationRecord 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 + CONFIRMABLE_TEMPLATE_ID = "257460a6-6616-4640-a3f9-17c3d73d9e91".freeze + BETA_ONBOARDING_TEMPLATE_ID = "b48bc2cd-5887-4611-8296-d0ab3ed0e7fd".freeze def reset_password_notify_template - last_sign_in_at ? RESET_PASSWORD_TEMPLATE_ID : SET_PASSWORD_TEMPLATE_ID + RESET_PASSWORD_TEMPLATE_ID + end + + def confirmable_template + if was_migrated_from_softwire? + BETA_ONBOARDING_TEMPLATE_ID + else + CONFIRMABLE_TEMPLATE_ID + end + end + + def was_migrated_from_softwire? + old_user_id.present? + end + + def skip_confirmation! + !active? end def need_two_factor_authentication?(_request) diff --git a/app/views/devise/confirmations/expired.html.erb b/app/views/devise/confirmations/expired.html.erb new file mode 100644 index 000000000..5a27eee4d --- /dev/null +++ b/app/views/devise/confirmations/expired.html.erb @@ -0,0 +1,11 @@ +<% content_for :title, "Your invitation link has expired" %> + +
+
+

+ <%= content_for(:title) %> +

+ +

Contact the helpdesk to request a new one.

+
+
diff --git a/app/views/devise/confirmations/new.html.erb b/app/views/devise/confirmations/new.html.erb index fe57a4a06..1b9ca13ea 100644 --- a/app/views/devise/confirmations/new.html.erb +++ b/app/views/devise/confirmations/new.html.erb @@ -1,15 +1,32 @@ -

Resend confirmation instructions

+<% content_for :title, "Resend invitation link" %> + +<% content_for :before_content do %> + <%= govuk_back_link( + text: "Back", + href: :back, + ) %> +<% end %> <%= form_for(resource, as: resource_name, url: confirmation_path(resource_name), html: { method: :post }) do |f| %> - <%= render "devise/shared/error_messages", resource: resource %> +
+
+ <%= f.govuk_error_summary %> + +

+ <%= content_for(:title) %> +

+ +

Enter your email address to get a new invitation link.

- <%= f.govuk_email_field :email, - label: { text: "Email address" }, - autocomplete: "email", - spellcheck: "false", - value: (resource.pending_reconfirmation? ? resource.unconfirmed_email : resource.email) %> + <%= f.govuk_email_field :email, + label: { text: "Email address" }, + autocomplete: "email", + spellcheck: "false", + value: (resource.pending_reconfirmation? ? resource.unconfirmed_email : resource.email) %> - <%= f.govuk_submit "Resend confirmation instructions" %> + <%= f.govuk_submit "Send email" %> +
+
<% end %> <%= render "devise/shared/links" %> diff --git a/app/views/devise/passwords/reset_password.html.erb b/app/views/devise/passwords/reset_password.html.erb index 04353c7b2..307876386 100644 --- a/app/views/devise/passwords/reset_password.html.erb +++ b/app/views/devise/passwords/reset_password.html.erb @@ -1,4 +1,4 @@ -<% content_for :title, "Reset your password" %> +<% content_for :title, @confirmation ? I18n.t("user.create_password") : I18n.t("user.reset_password") %> <% content_for :before_content do %> <%= govuk_back_link( diff --git a/app/views/devise/confirmations/reset.html.erb b/app/views/devise/passwords/reset_resend_confirmation.html.erb similarity index 100% rename from app/views/devise/confirmations/reset.html.erb rename to app/views/devise/passwords/reset_resend_confirmation.html.erb diff --git a/app/views/devise/shared/_links.html.erb b/app/views/devise/shared/_links.html.erb index 05035e08b..e463f5deb 100644 --- a/app/views/devise/shared/_links.html.erb +++ b/app/views/devise/shared/_links.html.erb @@ -11,7 +11,7 @@ <% end %> <%- if devise_mapping.confirmable? && controller_name != 'confirmations' %> - <%= govuk_link_to "Didn’t receive confirmation instructions?", new_confirmation_path(resource_name) %>
+ <% end %> <%- if devise_mapping.lockable? && resource_class.unlock_strategy_enabled?(:email) && controller_name != 'unlocks' %> diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 9f59dd22a..9e59169e6 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -152,7 +152,7 @@ Devise.setup do |config| # their account can't be confirmed with the token any more. # Default is nil, meaning there is no restriction on how long a user can take # before confirming their account. - # config.confirm_within = 3.days + config.confirm_within = 3.days # If true, requires any email changes to be confirmed (exactly the same way as # initial account confirmation) to be applied. Requires additional unconfirmed_email diff --git a/config/locales/devise.en.yml b/config/locales/devise.en.yml index a0a6ad4ff..707e8cffa 100644 --- a/config/locales/devise.en.yml +++ b/config/locales/devise.en.yml @@ -56,10 +56,10 @@ en: unlocked: "Your account has been successfully unlocked. Sign in to continue." errors: messages: - already_confirmed: "has already been confirmed. Sign in." + already_confirmed: "Email has already been confirmed. Sign in." blank: "can’t be blank" - confirmation: "doesn’t match new password" - confirmation_period_expired: "needs to be confirmed within %{period}. Request a new account." + confirmation: "Password confirmation doesn’t match new password" + confirmation_period_expired: "Email needs to be confirmed within %{period}. Request a new link below." expired: "Token has expired. Request a new token." not_found: "was not found" not_locked: "has not been locked" diff --git a/config/locales/en.yml b/config/locales/en.yml index 9df3a1083..367a11658 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -34,6 +34,9 @@ en: feedback_form: "https://forms.office.com/Pages/ResponsePage.aspx?id=EGg0v32c3kOociSi7zmVqC4YDsCJ3llAvEZelBFBLUBURFVUTzFDTUJPQlM4M0laTE5DTlNFSjJBQi4u" organisation: updated: "Organisation details updated" + user: + create_password: "Create a password to finish setting up your account" + reset_password: "Reset your password" validations: other_field_missing: "If %{main_field_label} is other then %{other_field_label} must be provided" diff --git a/config/routes.rb b/config/routes.rb index 3c8995519..f8269ed12 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -26,6 +26,7 @@ Rails.application.routes.draw do devise_for :users, { path: :account, controllers: { + confirmations: "auth/confirmations", passwords: "auth/passwords", sessions: "auth/sessions", two_factor_authentication: "auth/two_factor_authentication", diff --git a/db/migrate/20220517093906_add_confirmable_users.rb b/db/migrate/20220517093906_add_confirmable_users.rb new file mode 100644 index 000000000..46a050266 --- /dev/null +++ b/db/migrate/20220517093906_add_confirmable_users.rb @@ -0,0 +1,11 @@ +class AddConfirmableUsers < ActiveRecord::Migration[7.0] + def change + change_table :users, bulk: true do |t| + t.column :confirmation_token, :string + t.column :confirmed_at, :datetime + t.column :confirmation_sent_at, :datetime + t.string :unconfirmed_email + end + add_index :users, :confirmation_token, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index a65c0df64..0fa555d87 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -343,6 +343,11 @@ ActiveRecord::Schema[7.0].define(version: 2022_05_18_115438) do t.datetime "direct_otp_sent_at", precision: nil t.datetime "totp_timestamp", precision: nil t.boolean "active", default: true + t.string "confirmation_token" + t.datetime "confirmed_at", precision: nil + t.datetime "confirmation_sent_at", precision: nil + t.string "unconfirmed_email" + t.index ["confirmation_token"], name: "index_users_on_confirmation_token", 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" diff --git a/db/seeds.rb b/db/seeds.rb index 222d605fd..79f65f84c 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -31,6 +31,7 @@ if Rails.env.development? && User.count.zero? password: "password", organisation: org, role: "data_provider", + confirmed_at: Time.zone.now, ) User.create!( @@ -38,6 +39,7 @@ if Rails.env.development? && User.count.zero? password: "password", organisation: org, role: "data_coordinator", + confirmed_at: Time.zone.now, ) User.create!( @@ -45,6 +47,7 @@ if Rails.env.development? && User.count.zero? password: "password", organisation: org, role: "support", + confirmed_at: Time.zone.now, ) pp "Seeded 3 dummy users" diff --git a/lib/tasks/onboarding_emails.rake b/lib/tasks/onboarding_emails.rake deleted file mode 100644 index 6610358db..000000000 --- a/lib/tasks/onboarding_emails.rake +++ /dev/null @@ -1,22 +0,0 @@ -namespace :onboarding_emails do - desc "Send onboarding emails to private beta users" - task :send, %i[organisation_id] => :environment do |_task, args| - organisation_id = args[:organisation_id] - host = ENV["APP_HOST"] - raise "Organisation id must be provided" unless organisation_id - raise "Host is not set" unless host - - organisation = Organisation.find(organisation_id) - raise "Organisation #{organisation_id} does not exist" unless organisation - - organisation.users.each do |user| - next unless URI::MailTo::EMAIL_REGEXP.match?(user.email) - - onboarding_template_id = "b48bc2cd-5887-4611-8296-d0ab3ed0e7fd".freeze - token = user.send(:set_reset_password_token) - url = "#{host}/account/password/edit?reset_password_token=#{token}" - personalisation = { name: user.name || user.email, link: url } - DeviseNotifyMailer.new.send_email(user.email, onboarding_template_id, personalisation) - end - end -end diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 71e572493..4ed4ab8ed 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -9,8 +9,13 @@ describe Admin::UsersController, type: :controller do let(:resource_title) { "Users" } let(:valid_session) { {} } let!(:admin_user) { FactoryBot.create(:admin_user) } + let(:notify_client) { instance_double(Notifications::Client) } + 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) sign_in admin_user end diff --git a/spec/factories/user.rb b/spec/factories/user.rb index f5d9e7769..e021e0727 100644 --- a/spec/factories/user.rb +++ b/spec/factories/user.rb @@ -15,6 +15,8 @@ FactoryBot.define do trait :support do role { "support" } end + sign_in_count { 5 } + confirmed_at { Time.zone.now } created_at { Time.zone.now } updated_at { Time.zone.now } end diff --git a/spec/features/organisation_spec.rb b/spec/features/organisation_spec.rb index ddf38c72e..ef54ff2a2 100644 --- a/spec/features/organisation_spec.rb +++ b/spec/features/organisation_spec.rb @@ -5,15 +5,15 @@ RSpec.describe "User Features" do include Helpers let(:organisation) { user.organisation } let(:org_id) { organisation.id } - let(:set_password_template_id) { User::SET_PASSWORD_TEMPLATE_ID } + let(:set_password_template_id) { User::CONFIRMABLE_TEMPLATE_ID } let(:notify_client) { instance_double(Notifications::Client) } - let(:reset_password_token) { "MCDH5y6Km-U7CFPgAMVS" } + let(:confirmation_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(Devise.token_generator).to receive(:generate).and_return(reset_password_token) + allow(Devise).to receive(:friendly_token).and_return(confirmation_token) allow(notify_client).to receive(:send_email).and_return(true) sign_in user end @@ -55,7 +55,7 @@ RSpec.describe "User Features" do name: "New User", email: "new_user@example.com", organisation: organisation.name, - link: "http://localhost:3000/account/password/edit?reset_password_token=#{reset_password_token}", + link: "http://localhost:3000/account/confirmation?confirmation_token=#{confirmation_token}", }, }, ) diff --git a/spec/lib/tasks/onboarding_emails_spec.rb b/spec/lib/tasks/onboarding_emails_spec.rb deleted file mode 100644 index a1f2809b1..000000000 --- a/spec/lib/tasks/onboarding_emails_spec.rb +++ /dev/null @@ -1,41 +0,0 @@ -require "rails_helper" -require "rake" - -describe "rake onboarding_emails:send", type: task do - subject(:task) { Rake::Task["onboarding_emails:send"] } - - context "when onboarding a new organisation to private beta" do - let!(:user) { FactoryBot.create(:user) } - let(:notify_client) { instance_double(Notifications::Client) } - let(:devise_notify_mailer) { DeviseNotifyMailer.new } - let(:reset_password_token) { "MCDH5y6Km-U7CFPgAMVS" } - let(:host) { "http://localhost:3000" } - - before do - Rake.application.rake_require("tasks/onboarding_emails") - Rake::Task.define_task(:environment) - task.reenable - 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(ENV).to receive(:[]) - allow(ENV).to receive(:[]).with("APP_HOST").and_return(host) - end - - it "can send the onboarding emails" do - expect(notify_client).to receive(:send_email).with( - { - email_address: user.email, - template_id: "b48bc2cd-5887-4611-8296-d0ab3ed0e7fd", - personalisation: { - name: user.name, - link: "#{host}/account/password/edit?reset_password_token=#{reset_password_token}", - }, - }, - ) - - task.invoke(user.organisation.id) - end - end -end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9dff75fe3..b7d421fef 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -73,6 +73,18 @@ RSpec.describe User, type: :model do expect(user.need_two_factor_authentication?(nil)).to be false end + it "is confirmable" do + allow(DeviseNotifyMailer).to receive(:confirmation_instructions).and_return(OpenStruct.new(deliver: true)) + expect(DeviseNotifyMailer).to receive(:confirmation_instructions).once + described_class.create!( + name: "unconfirmed_user", + email: "unconfirmed_user@example.com", + password: "password123", + organisation: other_organisation, + role: "data_provider", + ) + end + context "when the user is a data provider" do it "cannot assign roles" do expect(user.assignable_roles).to eq({}) diff --git a/spec/requests/auth/confirmations_controller_spec.rb b/spec/requests/auth/confirmations_controller_spec.rb new file mode 100644 index 000000000..1cecf0447 --- /dev/null +++ b/spec/requests/auth/confirmations_controller_spec.rb @@ -0,0 +1,46 @@ +require "rails_helper" +require_relative "../../support/devise" + +RSpec.describe Auth::ConfirmationsController, type: :request do + let(:page) { Capybara::Node::Simple.new(response.body) } + let(:notify_client) { instance_double(Notifications::Client) } + let(:devise_notify_mailer) { DeviseNotifyMailer.new } + let(:user) { FactoryBot.create(:user, :data_provider, sign_in_count: 0, confirmed_at: nil) } + + 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) + end + + context "when a confirmation link is clicked by a new user" do + before do + user.send_confirmation_instructions + get "/account/confirmation?confirmation_token=#{user.confirmation_token}" + end + + it "marks the user as confirmed" do + expect(user.reload.confirmed_at).to be_a(Time) + end + + it "redirects to the set password page" do + follow_redirect! + expect(page).to have_content(I18n.t("user.create_password")) + end + end + + context "when the token has expired" do + let(:period) { Devise::TimeInflector.time_ago_in_words(User.confirm_within.ago) } + + before do + user.send_confirmation_instructions + allow(User).to receive(:find_first_by_auth_conditions).and_return(user) + allow(user).to receive(:confirmation_period_expired?).and_return(true) + get "/account/confirmation?confirmation_token=#{user.confirmation_token}" + end + + it "shows the error page" do + expect(page).to have_content("Your invitation link has expired") + end + end +end diff --git a/spec/requests/auth/passwords_controller_spec.rb b/spec/requests/auth/passwords_controller_spec.rb index 3bb285244..0615e8257 100644 --- a/spec/requests/auth/passwords_controller_spec.rb +++ b/spec/requests/auth/passwords_controller_spec.rb @@ -87,7 +87,7 @@ RSpec.describe Auth::PasswordsController, type: :request do it "renders the user edit password view" do _raw, enc = Devise.token_generator.generate(AdminUser, :reset_password_token) get "/admin/password/edit?reset_password_token=#{enc}" - expect(page).to have_css("h1", text: "Reset your password") + expect(page).to have_css("h1", text: I18n.t("user.reset_password")) end context "when passwords entered don't match" do diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 9380fe624..679cc02c6 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -255,7 +255,7 @@ RSpec.describe UsersController, type: :request do it "allows changing email but not dpo or key_contact" do user.reload - expect(user.email).to eq(new_email) + expect(user.unconfirmed_email).to eq(new_email) expect(user.is_data_protection_officer?).to be false expect(user.is_key_contact?).to be false end @@ -539,7 +539,7 @@ RSpec.describe UsersController, type: :request do it "allows changing email and dpo" do user.reload - expect(user.email).to eq(new_email) + expect(user.unconfirmed_email).to eq(new_email) expect(user.is_data_protection_officer?).to be true expect(user.is_key_contact?).to be true end @@ -586,7 +586,7 @@ RSpec.describe UsersController, type: :request do it "allows changing email, dpo, key_contact" do patch "/users/#{other_user.id}", headers: headers, params: params other_user.reload - expect(other_user.email).to eq(new_email) + expect(other_user.unconfirmed_email).to eq(new_email) expect(other_user.is_data_protection_officer?).to be true expect(other_user.is_key_contact?).to be true end @@ -971,7 +971,7 @@ RSpec.describe UsersController, type: :request do it "allows changing email and dpo" do user.reload - expect(user.email).to eq(new_email) + expect(user.unconfirmed_email).to eq(new_email) expect(user.is_data_protection_officer?).to be true expect(user.is_key_contact?).to be true end @@ -1018,7 +1018,7 @@ RSpec.describe UsersController, type: :request do it "allows changing email, dpo, key_contact" do patch "/users/#{other_user.id}", headers: headers, params: params other_user.reload - expect(other_user.email).to eq(new_email) + expect(other_user.unconfirmed_email).to eq(new_email) expect(other_user.is_data_protection_officer?).to be true expect(other_user.is_key_contact?).to be true end @@ -1075,7 +1075,7 @@ RSpec.describe UsersController, type: :request do it "allows changing email, dpo, key_contact" do patch "/users/#{other_user.id}", headers: headers, params: params other_user.reload - expect(other_user.email).to eq(new_email) + expect(other_user.unconfirmed_email).to eq(new_email) expect(other_user.is_data_protection_officer?).to be true expect(other_user.is_key_contact?).to be true end diff --git a/spec/services/imports/user_import_service_spec.rb b/spec/services/imports/user_import_service_spec.rb index b3be1cd12..ced9c8247 100644 --- a/spec/services/imports/user_import_service_spec.rb +++ b/spec/services/imports/user_import_service_spec.rb @@ -7,6 +7,14 @@ RSpec.describe Imports::UserImportService do let(:user_file) { File.open("#{fixture_directory}/#{old_user_id}.xml") } let(:storage_service) { instance_double(StorageService) } let(:logger) { instance_double(ActiveSupport::Logger) } + let(:notify_client) { instance_double(Notifications::Client) } + 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) + end context "when importing users" do subject(:import_service) { described_class.new(storage_service, logger) }