From d7b15b20255043896ddadac804f4ad7a60d2006c Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Thu, 18 May 2023 16:23:19 +0100 Subject: [PATCH] Switch to original devise gem (#1610) # Context - We're currently using a custom fork of devise # Changes - Switch back to original devise gem - This gives us the benefit of being able to more easily upgrade when the time comes - Minor fixes to validation copy during password reset process which were not correct --- Gemfile | 3 +-- Gemfile.lock | 20 +++++++------------- app/mailers/devise_notify_mailer.rb | 2 +- config/locales/en.yml | 7 ++++++- spec/features/auth/sign_in_spec.rb | 15 +++++++++++++++ spec/features/auth/user_lockout_spec.rb | 2 +- spec/models/user_spec.rb | 6 +++--- 7 files changed, 34 insertions(+), 21 deletions(-) create mode 100644 spec/features/auth/sign_in_spec.rb diff --git a/Gemfile b/Gemfile index 5322a25b2..229ae16e6 100644 --- a/Gemfile +++ b/Gemfile @@ -32,8 +32,7 @@ gem "roo" # Json Schema gem "json-schema" # Authentication -# Point at branch until devise is compatible with Turbo, see https://github.com/heartcombo/devise/pull/5340 -gem "devise", github: "baarkerlounger/devise", branch: "dluhc-fixes" +gem "devise" # Two-factor Authentication for devise models. gem "devise_two_factor_authentication" # UK postcode parsing and validation diff --git a/Gemfile.lock b/Gemfile.lock index 3ebae16b2..66a699356 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,15 +1,3 @@ -GIT - remote: https://github.com/baarkerlounger/devise.git - revision: 9b93eff1be452683b9fed61ec8c350fbc8387e7f - branch: dluhc-fixes - specs: - devise (4.8.1) - bcrypt (~> 3.0) - orm_adapter (~> 0.1) - railties (>= 4.1.0) - responders - warden (~> 1.2.3) - GEM remote: https://rubygems.org/ specs: @@ -140,6 +128,12 @@ GEM rexml crass (1.0.6) date (3.3.3) + devise (4.8.1) + bcrypt (~> 3.0) + orm_adapter (~> 0.1) + railties (>= 4.1.0) + responders + warden (~> 1.2.3) devise_two_factor_authentication (3.0.0) devise encryptor @@ -457,7 +451,7 @@ DEPENDENCIES capybara capybara-lockstep capybara-screenshot - devise! + devise devise_two_factor_authentication dotenv-rails erb_lint diff --git a/app/mailers/devise_notify_mailer.rb b/app/mailers/devise_notify_mailer.rb index 827e81b56..5560e7925 100644 --- a/app/mailers/devise_notify_mailer.rb +++ b/app/mailers/devise_notify_mailer.rb @@ -51,7 +51,7 @@ class DeviseNotifyMailer < Devise::Mailer end def email_allowlist - Rails.application.credentials[:email_allowlist] + Rails.application.credentials[:email_allowlist] || [] end private diff --git a/config/locales/en.yml b/config/locales/en.yml index ce969b9f9..e57e01da4 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -94,6 +94,9 @@ en: activerecord: + attributes: + user: + email: email errors: models: scheme: @@ -142,6 +145,9 @@ en: role: invalid: "Role must be data accessor, data provider or data coordinator" blank: "Select role" + password: + blank: Enter a password + too_short: Password is too short (minimum is %{count} characters) merge_request: attributes: absorbing_organisation_id: @@ -157,7 +163,6 @@ en: new_organisation_telephone_number: blank: "Enter a valid telephone number" - validations: organisation: name_missing: "Enter the name of the organisation" diff --git a/spec/features/auth/sign_in_spec.rb b/spec/features/auth/sign_in_spec.rb new file mode 100644 index 000000000..7b6103734 --- /dev/null +++ b/spec/features/auth/sign_in_spec.rb @@ -0,0 +1,15 @@ +require "rails_helper" + +RSpec.describe "User sign in" do + let(:user) { FactoryBot.create(:user) } + + context "when wrong credentials" do + it "shows correct error message" do + visit("/account/sign-in") + fill_in("user[email]", with: user.email) + fill_in("user[password]", with: "wrong_password") + click_button("Sign in") + expect(page).to have_content("Incorrect email or password") + end + end +end diff --git a/spec/features/auth/user_lockout_spec.rb b/spec/features/auth/user_lockout_spec.rb index 68cf78b2a..dce964c30 100644 --- a/spec/features/auth/user_lockout_spec.rb +++ b/spec/features/auth/user_lockout_spec.rb @@ -21,7 +21,7 @@ RSpec.describe "User Lockout" do fill_in("user[email]", with: user.email) fill_in("user[password]", with: user.password) click_button("Sign in") - expect(page).to have_http_status(:unprocessable_entity) + expect(page).to have_http_status(:ok) expect(page).to have_content(I18n.t("devise.failure.locked")) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 8d9407dc2..1cdbe7cd4 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -268,7 +268,7 @@ RSpec.describe User, type: :model do context "when a too short password is entered" do let(:password) { "123" } - let(:error_message) { "Validation failed: Password #{I18n.t('errors.messages.too_short', count: 8)}" } + let(:error_message) { "Validation failed: Password #{I18n.t('activerecord.errors.models.user.attributes.password.too_short', count: 8)}" } it "validates password length" do expect { FactoryBot.create(:user, password:) } @@ -278,7 +278,7 @@ RSpec.describe User, type: :model do context "when an invalid email is entered" do let(:invalid_email) { "not_an_email" } - let(:error_message) { "Validation failed: Email #{I18n.t('activerecord.errors.models.user.attributes.email.invalid')}" } + let(:error_message) { "Validation failed: email #{I18n.t('activerecord.errors.models.user.attributes.email.invalid')}" } it "validates email format" do expect { FactoryBot.create(:user, email: invalid_email) } @@ -288,7 +288,7 @@ RSpec.describe User, type: :model do context "when the email entered has already been used" do let(:user) { FactoryBot.create(:user) } - let(:error_message) { "Validation failed: Email #{I18n.t('activerecord.errors.models.user.attributes.email.taken')}" } + let(:error_message) { "Validation failed: email #{I18n.t('activerecord.errors.models.user.attributes.email.taken')}" } it "validates email uniqueness" do expect { FactoryBot.create(:user, email: user.email) }