diff --git a/app/controllers/user/confirmations_controller.rb b/app/controllers/user/confirmations_controller.rb new file mode 100644 index 000000000..82a1002ea --- /dev/null +++ b/app/controllers/user/confirmations_controller.rb @@ -0,0 +1,7 @@ +class User::ConfirmationsController < Devise::ConfirmationsController +protected + + def after_confirmation_path_for(_resource_name, resource) + new_user_confirmation_path(resource) + end +end diff --git a/app/controllers/auth/passwords_controller.rb b/app/controllers/user/passwords_controller.rb similarity index 83% rename from app/controllers/auth/passwords_controller.rb rename to app/controllers/user/passwords_controller.rb index 6c05debb5..f0aa3688f 100644 --- a/app/controllers/auth/passwords_controller.rb +++ b/app/controllers/user/passwords_controller.rb @@ -1,4 +1,4 @@ -class Auth::PasswordsController < Devise::PasswordsController +class User::PasswordsController < Devise::PasswordsController include Helpers::Email def reset_confirmation @@ -11,7 +11,7 @@ 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 - render "devise/confirmations/reset" + render "devise/passwords/reset_confirmation" end end @@ -30,6 +30,6 @@ class Auth::PasswordsController < Devise::PasswordsController protected def after_sending_reset_password_instructions_path_for(_resource) - confirmations_reset_path(email: params.dig("user", "email")) + password_reset_path(email: params.dig("user", "email")) end end diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/user/sessions_controller.rb similarity index 92% rename from app/controllers/auth/sessions_controller.rb rename to app/controllers/user/sessions_controller.rb index 270b89b2e..a542805cd 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/user/sessions_controller.rb @@ -1,4 +1,4 @@ -class Auth::SessionsController < Devise::SessionsController +class User::SessionsController < Devise::SessionsController include Helpers::Email def create diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index e60f9439f..3482979cc 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -34,7 +34,6 @@ class UsersController < ApplicationController render :new, status: :unprocessable_entity else @user = User.create!(user_params.merge(org_params).merge(password_params)) - @user.send_reset_password_instructions redirect_to users_organisation_path(current_user.organisation) end end diff --git a/app/models/user.rb b/app/models/user.rb index 8e18a55db..d8f1458ff 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -2,9 +2,9 @@ class User < ApplicationRecord include Constants::User # Include default devise modules. Others available are: - # :confirmable, :lockable, :timeoutable and :omniauthable + # :lockable, :timeoutable and :omniauthable devise :database_authenticatable, :recoverable, :rememberable, :validatable, - :trackable + :trackable, :confirmable belongs_to :organisation has_many :owned_case_logs, through: :organisation diff --git a/app/views/devise/confirmations/new.html.erb b/app/views/devise/confirmations/new.html.erb index ac0537c68..e84c78adf 100644 --- a/app/views/devise/confirmations/new.html.erb +++ b/app/views/devise/confirmations/new.html.erb @@ -1,16 +1,32 @@ -

Resend confirmation instructions

+<% content_for :title, "Set your password" %> -<%= form_for(resource, as: resource_name, url: confirmation_path(resource_name), html: { method: :post }) do |f| %> - <%= render "devise/shared/error_messages", resource: resource %> +<% content_for :before_content do %> + <%= govuk_back_link( + text: 'Back', + href: :back, + ) %> +<% end %> - <%= f.govuk_email_field :email, - label: { text: "Email address" }, - autocomplete: "email", - spellcheck: "false", - value: (resource.pending_reconfirmation? ? resource.unconfirmed_email : resource.email) - %> +<%= form_for(@user, as: :user, html: { method: :patch }) do |f| %> +
+
+ <%= f.govuk_error_summary(presenter: ErrorSummaryFullMessagesPresenter) %> - <%= f.govuk_submit "Resend confirmation instructions" %> -<% end %> +

+ <%= 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" + %> -<%= render "devise/shared/links" %> + <%= f.govuk_password_field :password_confirmation, + label: { text: "Confirm new password" } + %> + + <%= f.govuk_submit "Update" %> +
+
+<% end %> diff --git a/app/views/devise/confirmations/reset.html.erb b/app/views/devise/passwords/reset_confirmation.html.erb similarity index 100% rename from app/views/devise/confirmations/reset.html.erb rename to app/views/devise/passwords/reset_confirmation.html.erb diff --git a/app/views/devise/shared/_links.html.erb b/app/views/devise/shared/_links.html.erb index f66521541..a6e4ee369 100644 --- a/app/views/devise/shared/_links.html.erb +++ b/app/views/devise/shared/_links.html.erb @@ -10,10 +10,6 @@

You can <%= govuk_link_to "reset your password", new_password_path(resource_name) %> if you’ve forgotten it.


<% 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' %> <%= govuk_link_to "Didn’t receive unlock instructions?", new_unlock_path(resource_name) %>
<% end %> diff --git a/config/routes.rb b/config/routes.rb index 8f7a1db47..4406c03ae 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,12 +1,13 @@ Rails.application.routes.draw do devise_for :admin_users, ActiveAdmin::Devise.config devise_for :users, controllers: { - passwords: "auth/passwords", - sessions: "auth/sessions", + passwords: "user/passwords", + sessions: "user/sessions", + confirmations: "user/confirmations", }, path_names: { sign_in: "sign-in", sign_out: "sign-out" } devise_scope :user do - get "confirmations/reset", to: "auth/passwords#reset_confirmation" + get "password/reset", to: "user/passwords#reset_confirmation" end # For details on the DSL available within this file, see https://guides.rubyonrails.org/routing.html diff --git a/db/migrate/20220121153444_users_confirmable.rb b/db/migrate/20220121153444_users_confirmable.rb new file mode 100644 index 000000000..22cfe7991 --- /dev/null +++ b/db/migrate/20220121153444_users_confirmable.rb @@ -0,0 +1,11 @@ +class UsersConfirmable < ActiveRecord::Migration[7.0] + def change + change_table :users, bulk: true do |t| + ## Confirmable + t.string :confirmation_token + t.datetime :confirmed_at + t.datetime :confirmation_sent_at + t.string :unconfirmed_email # Only if using reconfirmable + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 766b1cf55..405f3ea5a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_01_14_105351) do +ActiveRecord::Schema.define(version: 2022_01_21_153444) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -236,6 +236,10 @@ ActiveRecord::Schema.define(version: 2022_01_14_105351) do t.string "current_sign_in_ip" t.string "last_sign_in_ip" t.integer "role" + t.string "confirmation_token" + t.datetime "confirmed_at" + t.datetime "confirmation_sent_at" + t.string "unconfirmed_email" t.index ["email"], name: "index_users_on_email", unique: true t.index ["organisation_id"], name: "index_users_on_organisation_id" t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true diff --git a/db/seeds.rb b/db/seeds.rb index a59808694..11fb151d2 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -21,6 +21,7 @@ User.create!( password: "password", organisation: org, role: "data_provider", + confirmed_at: Time.zone.now, ) User.create!( @@ -28,6 +29,7 @@ User.create!( password: "password", organisation: org, role: "data_coordinator", + confirmed_at: Time.zone.now, ) AdminUser.create!(email: "admin@example.com", password: "password") diff --git a/spec/factories/user.rb b/spec/factories/user.rb index aa08b1d99..5ca2c03ef 100644 --- a/spec/factories/user.rb +++ b/spec/factories/user.rb @@ -10,5 +10,9 @@ FactoryBot.define do end created_at { Time.zone.now } updated_at { Time.zone.now } + confirmed_at { Time.zone.now } + trait :unconfirmed do + confirmed_at { nil } + end end end diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index 71375f4d2..042fe4dfe 100644 --- a/spec/features/user_spec.rb +++ b/spec/features/user_spec.rb @@ -63,7 +63,7 @@ RSpec.describe "User Features" do visit("/users/password/new") fill_in("user[email]", with: "idontexist@example.com") click_button("Send email") - expect(page).to have_current_path("/confirmations/reset?email=idontexist%40example.com") + expect(page).to have_current_path("/password/reset?email=idontexist%40example.com") end it " is sent a reset password email" do diff --git a/spec/requests/auth/passwords_controller_spec.rb b/spec/requests/user/passwords_controller_spec.rb similarity index 95% rename from spec/requests/auth/passwords_controller_spec.rb rename to spec/requests/user/passwords_controller_spec.rb index b6fbb8ac1..625eaa5c0 100644 --- a/spec/requests/auth/passwords_controller_spec.rb +++ b/spec/requests/user/passwords_controller_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" require_relative "../../support/devise" -RSpec.describe Auth::PasswordsController, type: :request do +RSpec.describe User::PasswordsController, type: :request do let(:params) { { user: { email: email } } } let(:page) { Capybara::Node::Simple.new(response.body) } @@ -19,7 +19,7 @@ RSpec.describe Auth::PasswordsController, type: :request do context "when a password reset is requested with an email that doesn't exist in the system" do before do - allow_any_instance_of(Auth::PasswordsController).to receive(:is_navigational_format?).and_return(false) + allow_any_instance_of(User::PasswordsController).to receive(:is_navigational_format?).and_return(false) end let(:email) { "madeup_email@test.com" } diff --git a/spec/requests/user_controller_spec.rb b/spec/requests/user_controller_spec.rb index 0553aba95..b2d7c7b46 100644 --- a/spec/requests/user_controller_spec.rb +++ b/spec/requests/user_controller_spec.rb @@ -37,6 +37,18 @@ RSpec.describe "password_reset", type: :request do end end + describe "confirm" do + context "a user that has not confirmed their email and set a password" do + let(:user) { FactoryBot.create(:user, :unconfirmed) } + let(:raw) { user.send_confirmation_instructions } + + it "renders the user set password view" do + get "/users/confirmation?confirmation_token=#{raw}" + expect(page).to have_css("h1", class: "govuk-heading-l", text: "Set your password") + end + end + end + describe "reset password" do it "renders the user edit password view" do _raw, enc = Devise.token_generator.generate(User, :reset_password_token) @@ -45,16 +57,21 @@ RSpec.describe "password_reset", type: :request do end context "update password" do - context "valid reset token" do + context "valid confirmation token" do + let(:raw) { user.send_reset_password_instructions } let(:params) do { - id: user.id, user: { password: new_value, password_confirmation: "something_else" } + id: user.id, + user: { + password: new_value, + password_confirmation: "something_else", + reset_password_token: raw, + }, } end before do - sign_in user - put "/users/#{user.id}", headers: headers, params: params + put "/users/password", headers: headers, params: params end it "shows an error if passwords don't match" do