diff --git a/app/controllers/auth/passwords_controller.rb b/app/controllers/auth/passwords_controller.rb index 6c05debb5..35a1d7ca7 100644 --- a/app/controllers/auth/passwords_controller.rb +++ b/app/controllers/auth/passwords_controller.rb @@ -24,12 +24,24 @@ class Auth::PasswordsController < Devise::PasswordsController def edit super - render "users/reset_password" + render "devise/passwords/reset_password" end protected + def resource_class_name + resource_class.name.underscore + end + def after_sending_reset_password_instructions_path_for(_resource) - confirmations_reset_path(email: params.dig("user", "email")) + confirmations_reset_path(email: params.dig(resource_class_name, "email")) + end + + def after_resetting_password_path_for(resource) + if Devise.sign_in_after_reset_password + resource_class == AdminUser ? admin_user_two_factor_authentication_path : after_sign_in_path_for(resource) + else + new_session_path(resource_name) + end end end diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 270b89b2e..95654e520 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -3,12 +3,12 @@ class Auth::SessionsController < Devise::SessionsController def create self.resource = resource_class.new - if params.dig("user", "email").empty? + if params.dig(resource_class_name, "email").empty? resource.errors.add :email, "Enter an email address" - elsif !email_valid?(params.dig("user", "email")) + elsif !email_valid?(params.dig(resource_class_name, "email")) resource.errors.add :email, "Enter an email address in the correct format, like name@example.com" end - if params.dig("user", "password").empty? + if params.dig(resource_class_name, "password").empty? resource.errors.add :password, "Enter a password" end if resource.errors.present? @@ -20,7 +20,19 @@ class Auth::SessionsController < Devise::SessionsController private + def resource_class + request.path.include?("admin") ? AdminUser : User + end + + def resource_class_name + resource_class.name.underscore + end + def after_sign_in_path_for(resource) - params.dig("user", "start").present? ? case_logs_path : super + if resource_class == AdminUser + admin_user_two_factor_authentication_path + else + params.dig(resource_class_name, "start").present? ? case_logs_path : super + end end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index f1eb661e1..63863c17f 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -12,7 +12,8 @@ class UsersController < ApplicationController redirect_to user_path(@user) elsif user_params.key?("password") format_error_messages - render "devise/passwords/edit", status: :unprocessable_entity + @minimum_password_length = User.password_length.min + render "devise/passwords/edit", locals: { resource: @user, resource_name: "user" }, status: :unprocessable_entity else format_error_messages render :edit, status: :unprocessable_entity @@ -41,7 +42,7 @@ class UsersController < ApplicationController def edit_password @minimum_password_length = User.password_length.min - render "devise/passwords/edit" + render "devise/passwords/edit", locals: { resource: @user, resource_name: "user" } end private diff --git a/app/views/devise/passwords/edit.html.erb b/app/views/devise/passwords/edit.html.erb index 6d405ba0f..68ff77514 100644 --- a/app/views/devise/passwords/edit.html.erb +++ b/app/views/devise/passwords/edit.html.erb @@ -7,7 +7,7 @@ ) %> <% end %> -<%= form_for(@user, as: :user, html: { method: :patch }) do |f| %> +<%= form_for(resource, as: resource_name, html: { method: :patch }) do |f| %>
<%= f.govuk_error_summary(presenter: ErrorSummaryFullMessagesPresenter) %> diff --git a/app/views/users/reset_password.html.erb b/app/views/devise/passwords/reset_password.html.erb similarity index 88% rename from app/views/users/reset_password.html.erb rename to app/views/devise/passwords/reset_password.html.erb index e12a0beed..bd96a9c5d 100644 --- a/app/views/users/reset_password.html.erb +++ b/app/views/devise/passwords/reset_password.html.erb @@ -7,7 +7,7 @@ ) %> <% end %> -<%= form_for(@user, as: :user, url: password_path(User), html: { method: :put }) do |f| %> +<%= form_for(resource, as: resource_name, url: password_path(resource_name), html: { method: :put }) do |f| %> <%= f.hidden_field :reset_password_token %>
diff --git a/app/views/devise/sessions/new.html.erb b/app/views/devise/sessions/new.html.erb index fab47b4a1..1fd1d7be8 100644 --- a/app/views/devise/sessions/new.html.erb +++ b/app/views/devise/sessions/new.html.erb @@ -1,4 +1,10 @@ -<% content_for :title, "Sign in to your account to submit CORE data" %> +<% if resource_name == :admin_user %> + <% title = "CORE Admin Sign in" %> +<% else %> + <% title = "Sign in to your account to submit CORE data" %> +<% end %> + +<% content_for :title, title %> <%= form_for(resource, as: resource_name, url: session_path(resource_name)) do |f| %>
diff --git a/config/routes.rb b/config/routes.rb index 02b3a1e40..c9738f1e3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -2,14 +2,14 @@ Rails.application.routes.draw do devise_for :admin_users, { path: :admin, controllers: { - sessions: "active_admin/devise/sessions", - passwords: "active_admin/devise/passwords", + sessions: "auth/sessions", + passwords: "auth/passwords", unlocks: "active_admin/devise/unlocks", registrations: "active_admin/devise/registrations", confirmations: "active_admin/devise/confirmations", two_factor_authentication: "auth/two_factor_authentication", }, - path_names: { sign_in: "login", sign_out: "logout", two_factor_authentication: "two-factor-authentication" }, + path_names: { sign_in: "sign-in", sign_out: "sign-out", two_factor_authentication: "two-factor-authentication" }, sign_out_via: %i[delete get], } diff --git a/spec/features/admin_panel_spec.rb b/spec/features/admin_panel_spec.rb index 93a0e0a1f..02202e9c1 100644 --- a/spec/features/admin_panel_spec.rb +++ b/spec/features/admin_panel_spec.rb @@ -11,6 +11,12 @@ RSpec.describe "Admin Panel" do allow(notify_client).to receive(:send_sms).and_return(true) end + it "shows the admin sign in page" do + visit("/admin") + expect(page).to have_current_path("/admin/sign-in") + expect(page).to have_content("CORE Admin Sign in") + end + context "with a valid 2FA code" do before do allow(SecureRandom).to receive(:random_number).and_return(otp) @@ -23,7 +29,7 @@ RSpec.describe "Admin Panel" do expect(notify_client).to receive(:send_sms).with( hash_including(phone_number: admin.phone, template_id: mfa_template_id), ) - click_button("Login") + click_button("Sign in") fill_in("code", with: otp) click_button("Submit") expect(page).to have_content("Dashboard") @@ -32,7 +38,7 @@ RSpec.describe "Admin Panel" do context "but it is more than 15 minutes old" do it "does not authenticate successfully" do - click_button("Login") + click_button("Sign in") admin.update!(direct_otp_sent_at: 16.minutes.ago) fill_in("code", with: otp) click_button("Submit") @@ -49,7 +55,7 @@ RSpec.describe "Admin Panel" do visit("/admin") fill_in("admin_user[email]", with: admin.email) fill_in("admin_user[password]", with: admin.password) - click_button("Login") + click_button("Sign in") fill_in("code", with: otp) click_button("Submit") expect(page).to have_content("Check your phone") @@ -64,7 +70,7 @@ RSpec.describe "Admin Panel" do visit("/admin") fill_in("admin_user[email]", with: admin.email) fill_in("admin_user[password]", with: admin.password) - click_button("Login") + click_button("Sign in") end it "displays the resend view" do @@ -88,14 +94,68 @@ RSpec.describe "Admin Panel" do visit("/admin") fill_in("admin_user[email]", with: admin.email) fill_in("admin_user[password]", with: admin.password) - click_button("Login") + click_button("Sign in") fill_in("code", with: otp) click_button("Submit") click_link("Logout") + visit("/admin") fill_in("admin_user[email]", with: admin.email) fill_in("admin_user[password]", with: admin.password) - click_button("Login") + click_button("Sign in") expect(page).to have_content("Check your phone") 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 + + it " is redirected to the reset password page when they click the reset password link" do + visit("/admin") + click_link("reset your password") + expect(page).to have_current_path("/admin/password/new") + end + + it " is shown an error message if they submit without entering an email address" do + visit("/admin/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 admin login page after reset email is sent" do + visit("/admin/password/new") + fill_in("admin_user[email]", with: admin_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: admin_user.email, + template_id: admin_user.reset_password_notify_template, + personalisation: { + name: admin_user.email, + email: admin_user.email, + organisation: "", + link: "http://localhost:3000/admin/password/edit?reset_password_token=#{reset_password_token}", + }, + }, + ) + visit("/admin/password/new") + fill_in("admin_user[email]", with: admin_user.email) + click_button("Send email") + end + end end diff --git a/spec/features/admin_user_spec.rb b/spec/features/admin_user_spec.rb deleted file mode 100644 index 4ca656643..000000000 --- a/spec/features/admin_user_spec.rb +++ /dev/null @@ -1,55 +0,0 @@ -require "rails_helper" - -RSpec.describe "Admin Features" 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 - - context "when the admin has forgotten their password" do - it " is redirected to the reset password page when they click the reset password link" do - visit("/admin") - click_link("Forgot your password?") - expect(page).to have_current_path("/admin/password/new") - end - - it " is shown an error message if they submit without entering an email address" do - visit("/admin/password/new") - click_button("Reset My Password") - expect(page).to have_selector("#error_explanation") - expect(page).to have_content("can't be blank") - end - - it " is redirected to admin login page after reset email is sent" do - visit("/admin/password/new") - fill_in("admin_user[email]", with: admin_user.email) - click_button("Reset My Password") - expect(page).to have_current_path("/admin/login") - end - - it " is sent a reset password email via Notify" do - expect(notify_client).to receive(:send_email).with( - { - email_address: admin_user.email, - template_id: admin_user.reset_password_notify_template, - personalisation: { - name: admin_user.email, - email: admin_user.email, - organisation: "", - link: "http://localhost:3000/admin/password/edit?reset_password_token=#{reset_password_token}", - }, - }, - ) - visit("/admin/password/new") - fill_in("admin_user[email]", with: admin_user.email) - click_button("Reset My Password") - end - end -end diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index 70eeeb5fa..bb08854db 100644 --- a/spec/features/user_spec.rb +++ b/spec/features/user_spec.rb @@ -18,6 +18,7 @@ RSpec.describe "User Features" do it " is required to log in" do visit("/logs") expect(page).to have_current_path("/users/sign-in") + expect(page).to have_content("Sign in to your account to submit CORE data") end it "does not see the default devise error message" do diff --git a/spec/requests/active_admin/devise/passwords_controller_spec.rb b/spec/requests/active_admin/devise/passwords_controller_spec.rb deleted file mode 100644 index 948b15309..000000000 --- a/spec/requests/active_admin/devise/passwords_controller_spec.rb +++ /dev/null @@ -1,65 +0,0 @@ -require "rails_helper" - -RSpec.describe ActiveAdmin::Devise::PasswordsController, type: :request do - let(:admin_user) { FactoryBot.create(:admin_user) } - let(:headers) { { "Accept" => "text/html" } } - let(:page) { Capybara::Node::Simple.new(response.body) } - let(:new_value) { "new-password" } - 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 - - describe "reset password" 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("h2", text: "DLUHC CORE Change your password") - end - - context "when passwords entered don't match" do - let(:raw) { admin_user.send_reset_password_instructions } - let(:params) do - { - id: admin_user.id, - admin_user: { - password: new_value, - password_confirmation: "something_else", - reset_password_token: raw, - }, - } - end - - it "shows an error" do - put "/admin/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) { admin_user.send_reset_password_instructions } - let(:params) do - { - id: admin_user.id, - admin_user: { - password: new_value, - password_confirmation: new_value, - reset_password_token: raw, - }, - } - end - - it "updates the password" do - expect { - put "/admin/password", headers: headers, params: params - admin_user.reload - }.to change(admin_user, :encrypted_password) - end - end - end -end diff --git a/spec/requests/auth/passwords_controller_spec.rb b/spec/requests/auth/passwords_controller_spec.rb index b39b3a1f8..5b188a498 100644 --- a/spec/requests/auth/passwords_controller_spec.rb +++ b/spec/requests/auth/passwords_controller_spec.rb @@ -2,7 +2,6 @@ require "rails_helper" require_relative "../../support/devise" RSpec.describe Auth::PasswordsController, type: :request do - let(:params) { { user: { email: } } } let(:page) { Capybara::Node::Simple.new(response.body) } let(:notify_client) { instance_double(Notifications::Client) } let(:devise_notify_mailer) { DeviseNotifyMailer.new } @@ -13,60 +12,124 @@ RSpec.describe Auth::PasswordsController, type: :request do allow(notify_client).to receive(:send_email).and_return(true) end - context "when a password reset is requested for a valid email" do - let(:user) { FactoryBot.create(:user) } - let(:email) { user.email } + context "when a regular user" do + let(:params) { { user: { email: } } } - it "redirects to the email sent page" do - post "/users/password", params: params - expect(response).to have_http_status(:redirect) - follow_redirect! - expect(response.body).to match(/Check your email/) + context "when a password reset is requested for a valid email" do + let(:user) { FactoryBot.create(:user) } + let(:email) { user.email } + + it "redirects to the email sent page" do + post "/users/password", params: params + expect(response).to have_http_status(:redirect) + follow_redirect! + expect(response.body).to match(/Check your email/) + end end - end - context "when a password reset is requested with an email that doesn't exist in the system" do - before do - allow(Devise.navigational_formats).to receive(:include?).and_return(false) + context "when a password reset is requested with an email that doesn't exist in the system" do + before do + allow(Devise.navigational_formats).to receive(:include?).and_return(false) + end + + let(:email) { "madeup_email@test.com" } + + it "redirects to the email sent page anyway" do + post "/users/password", params: params + expect(response).to have_http_status(:redirect) + follow_redirect! + expect(response.body).to match(/Check your email/) + end end - let(:email) { "madeup_email@test.com" } + describe "#Update - reset password" do + let(:user) { FactoryBot.create(:user) } + let(:token) { user.send(:set_reset_password_token) } + let(:updated_password) { "updated_password_280" } + let(:update_password_params) do + { + user: + { + reset_password_token: token, + password: updated_password, + password_confirmation: updated_password, + }, + } + end + let(:message) { "Your password has been changed successfully. You are now signed in" } - it "redirects to the email sent page anyway" do - post "/users/password", params: params - expect(response).to have_http_status(:redirect) - follow_redirect! - expect(response.body).to match(/Check your email/) + it "changes the password" do + expect { put "/users/password", params: update_password_params } + .to(change { user.reload.encrypted_password }) + end + + it "after password change, the user is signed in" do + put "/users/password", params: update_password_params + # Devise redirects once after re-sign in with new password and then root redirects as well. + follow_redirect! + follow_redirect! + expect(page).to have_css("div", class: "govuk-notification-banner__heading", text: message) + end end end - describe "#Update - reset password" do - let(:user) { FactoryBot.create(:user) } - let(:token) { user.send(:set_reset_password_token) } - let(:updated_password) { "updated_password_280" } - let(:update_password_params) do - { - user: + context "when an admin user" do + let(:admin_user) { FactoryBot.create(:admin_user) } + + describe "reset password" do + let(:new_value) { "new-password" } + + 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") + end + + context "when passwords entered don't match" do + let(:raw) { admin_user.send_reset_password_instructions } + let(:params) do { - reset_password_token: token, - password: updated_password, - password_confirmation: updated_password, - }, - } - end - let(:message) { "Your password has been changed successfully. You are now signed in" } + id: admin_user.id, + admin_user: { + password: new_value, + password_confirmation: "something_else", + reset_password_token: raw, + }, + } + end - it "changes the password" do - expect { put "/users/password", params: update_password_params } - .to(change { user.reload.encrypted_password }) - end + it "shows an error" do + put "/admin/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) { admin_user.send_reset_password_instructions } + let(:params) do + { + id: admin_user.id, + admin_user: { + password: new_value, + password_confirmation: new_value, + reset_password_token: raw, + }, + } + end + + it "updates the password" do + expect { + put "/admin/password", headers: headers, params: params + admin_user.reload + }.to change(admin_user, :encrypted_password) + end - it "after password change, the user is signed in" do - put "/users/password", params: update_password_params - # Devise redirects once after re-sign in with new password and then root redirects as well. - follow_redirect! - follow_redirect! - expect(page).to have_css("div", class: "govuk-notification-banner__heading", text: message) + it "sends you to the 2FA page" do + put "/admin/password", headers: headers, params: params + expect(response).to redirect_to("/admin/two-factor-authentication") + end + end end end end