From 5842f7aed79ebdf182a9c6a010cb3517c515b9e7 Mon Sep 17 00:00:00 2001 From: baarkerlounger <5101747+baarkerlounger@users.noreply.github.com> Date: Thu, 9 Dec 2021 09:41:58 +0000 Subject: [PATCH 1/4] Fix password reset flow (#157) * Render gov uk reset view * Pass password reset token * Test password update and sign in * The cop is working --- app/controllers/auth/passwords_controller.rb | 5 +++ app/views/users/reset_password.html.erb | 31 +++++++++++++++++++ .../auth/passwords_controller_spec.rb | 31 ++++++++++++++++++- spec/requests/user_controller_spec.rb | 8 +++++ 4 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 app/views/users/reset_password.html.erb diff --git a/app/controllers/auth/passwords_controller.rb b/app/controllers/auth/passwords_controller.rb index 0f6e1c9b0..317c4aa29 100644 --- a/app/controllers/auth/passwords_controller.rb +++ b/app/controllers/auth/passwords_controller.rb @@ -23,6 +23,11 @@ class Auth::PasswordsController < Devise::PasswordsController respond_with({}, location: after_sending_reset_password_instructions_path_for(resource_name)) end + def edit + super + render "users/reset_password" + end + protected def after_sending_reset_password_instructions_path_for(_resource) diff --git a/app/views/users/reset_password.html.erb b/app/views/users/reset_password.html.erb new file mode 100644 index 000000000..656eaa264 --- /dev/null +++ b/app/views/users/reset_password.html.erb @@ -0,0 +1,31 @@ +<% content_for :title, "Reset your password" %> + +<% content_for :before_content do %> + <%= govuk_back_link( + text: 'Back', + href: :back, + ) %> +<% end %> + +<%= form_for(@user, as: :user, url: password_path(User), html: { method: :put }) do |f| %> + <%= f.hidden_field :reset_password_token %> +
+
+

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

+ + <%= f.govuk_password_field :password, + label: { text: "New password" }, + hint: @minimum_password_length ? { text: "Your password must be at least #{@minimum_password_length} characters and hard to guess." } : nil, + autocomplete: "new-password" + %> + + <%= f.govuk_password_field :password_confirmation, + label: { text: "Confirm new password" } + %> + + <%= f.govuk_submit "Update" %> +
+
+<% end %> diff --git a/spec/requests/auth/passwords_controller_spec.rb b/spec/requests/auth/passwords_controller_spec.rb index 7bb617f78..b6fbb8ac1 100644 --- a/spec/requests/auth/passwords_controller_spec.rb +++ b/spec/requests/auth/passwords_controller_spec.rb @@ -3,12 +3,13 @@ require_relative "../../support/devise" RSpec.describe Auth::PasswordsController, type: :request do let(:params) { { user: { email: email } } } + let(:page) { Capybara::Node::Simple.new(response.body) } 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 anyway" do + it "redirects to the email sent page" do post "/users/password", params: params expect(response).to have_http_status(:redirect) follow_redirect! @@ -43,4 +44,32 @@ RSpec.describe Auth::PasswordsController, type: :request do expect(email_content).to match(email) end end + + context "#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 "changes the password" do + expect { put "/users/password", params: update_password_params } + .to(change { user.reload.encrypted_password }) + end + + it "signs in" do + put "/users/password", params: update_password_params + follow_redirect! + expect(page).to have_css("div", class: "govuk-notification-banner__heading", text: message) + end + end end diff --git a/spec/requests/user_controller_spec.rb b/spec/requests/user_controller_spec.rb index 26f7bd959..c247ced48 100644 --- a/spec/requests/user_controller_spec.rb +++ b/spec/requests/user_controller_spec.rb @@ -37,6 +37,14 @@ RSpec.describe UsersController, type: :request do expect(response).to redirect_to("/users/sign-in") end end + + describe "reset password" do + it "renders the user edit password view" do + _raw, enc = Devise.token_generator.generate(User, :reset_password_token) + get "/users/password/edit?reset_password_token=#{enc}" + expect(page).to have_css("h1", class: "govuk-heading-l", text: "Reset your password") + end + end end describe "#show" do From b188b0cec2bb4002e18813fc3dc0c41ac009a478 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Thu, 9 Dec 2021 09:59:42 +0000 Subject: [PATCH 2/4] Don't add port to url on gov paas --- config/environments/production.rb | 2 +- config/environments/sandbox.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config/environments/production.rb b/config/environments/production.rb index 513645019..175c0d9a3 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -58,7 +58,7 @@ Rails.application.configure do config.action_mailer.perform_caching = false - config.action_mailer.default_url_options = { host: ENV["APP_HOST"], port: 3000 } + config.action_mailer.default_url_options = { host: ENV["APP_HOST"] } config.action_mailer.delivery_method = :smtp config.action_mailer.smtp_settings = { address: "smtp.gmail.com", diff --git a/config/environments/sandbox.rb b/config/environments/sandbox.rb index 513645019..175c0d9a3 100644 --- a/config/environments/sandbox.rb +++ b/config/environments/sandbox.rb @@ -58,7 +58,7 @@ Rails.application.configure do config.action_mailer.perform_caching = false - config.action_mailer.default_url_options = { host: ENV["APP_HOST"], port: 3000 } + config.action_mailer.default_url_options = { host: ENV["APP_HOST"] } config.action_mailer.delivery_method = :smtp config.action_mailer.smtp_settings = { address: "smtp.gmail.com", From 2f76c06670b034611598d14296ec2c148c8d6f87 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Thu, 9 Dec 2021 10:27:34 +0000 Subject: [PATCH 3/4] Remove flash alert from reset password page --- app/controllers/auth/passwords_controller.rb | 1 - spec/features/user_spec.rb | 7 ------- 2 files changed, 8 deletions(-) diff --git a/app/controllers/auth/passwords_controller.rb b/app/controllers/auth/passwords_controller.rb index 317c4aa29..6c05debb5 100644 --- a/app/controllers/auth/passwords_controller.rb +++ b/app/controllers/auth/passwords_controller.rb @@ -11,7 +11,6 @@ class Auth::PasswordsController < Devise::PasswordsController resource.errors.add :email, "Enter an email address in the correct format, like name@example.com" render "devise/passwords/new", status: :unprocessable_entity else - flash[:notice] = "Reset password instructions have been sent to #{@email}" render "devise/confirmations/reset" end end diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index 7b8351013..f14cd43fd 100644 --- a/spec/features/user_spec.rb +++ b/spec/features/user_spec.rb @@ -72,13 +72,6 @@ RSpec.describe "User Features" do fill_in("user[email]", with: user.email) expect { click_button("Send email") }.to change { ActionMailer::Base.deliveries.count }.by(1) end - - it " is shown the password reset confirmation page and successful flash message shows" do - visit("/users/password/new") - fill_in("user[email]", with: user.email) - click_button("Send email") - expect(page).to have_css ".govuk-notification-banner.govuk-notification-banner--success" - end end context "If user not logged in" do From b9493f2cac1a4abbe4477a19d43cdea16e1113dc Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Thu, 9 Dec 2021 10:42:38 +0000 Subject: [PATCH 4/4] Fix admin password update --- app/admin/admin_users.rb | 2 +- app/admin/users.rb | 2 +- app/views/users/show.html.erb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/admin/admin_users.rb b/app/admin/admin_users.rb index c09dc1804..fd15aa33d 100644 --- a/app/admin/admin_users.rb +++ b/app/admin/admin_users.rb @@ -3,7 +3,7 @@ ActiveAdmin.register AdminUser do controller do def update_resource(object, attributes) - update_method = attributes.first[:password].present? ? :update_attributes : :update_without_password + update_method = attributes.first[:password].present? ? :update : :update_without_password object.send(update_method, *attributes) end end diff --git a/app/admin/users.rb b/app/admin/users.rb index 781ef046c..59a235b99 100644 --- a/app/admin/users.rb +++ b/app/admin/users.rb @@ -3,7 +3,7 @@ ActiveAdmin.register User do controller do def update_resource(object, attributes) - update_method = attributes.first[:password].present? ? :update_attributes : :update_without_password + update_method = attributes.first[:password].present? ? :update : :update_without_password object.send(update_method, *attributes) end end diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 27ecb39e9..7472d46f8 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -31,7 +31,7 @@ <%= summary_list.row do |row| row.key { 'Organisation' } - row.value { current_user.organisation } + row.value { current_user.organisation.name } row.action() end %>