diff --git a/app/controllers/users/emails_controller.rb b/app/controllers/users/emails_controller.rb new file mode 100644 index 000000000..12f701ebb --- /dev/null +++ b/app/controllers/users/emails_controller.rb @@ -0,0 +1,52 @@ +class Users::EmailsController < ApplicationController + layout "admin_layout" + + before_action :authenticate_user! + before_action :load_user + before_action :authorize_user + before_action :redirect_to_account_page_if_acting_on_own_user, only: %i[edit] + + def edit; end + + def update + updater = UserUpdate.new(@user, user_params, current_user, user_ip_address) + if updater.call + redirect_to edit_user_path(@user), notice: "Updated user #{@user.email} successfully" + else + render :edit + end + end + + def resend_email_change + @user.resend_confirmation_instructions + if @user.errors.empty? + redirect_to edit_user_path(@user), notice: "Successfully resent email change email to #{@user.unconfirmed_email}" + else + redirect_to edit_user_email_path(@user), alert: "Failed to send email change email" + end + end + + def cancel_email_change + @user.cancel_email_change! + + redirect_to edit_user_path(@user) + end + +private + + def load_user + @user = User.find(params[:user_id]) + end + + def authorize_user + authorize @user + end + + def user_params + params.require(:user).permit(*current_user.permitted_params.intersection([:email])) + end + + def redirect_to_account_page_if_acting_on_own_user + redirect_to edit_account_email_path if current_user == @user + end +end diff --git a/app/controllers/users/names_controller.rb b/app/controllers/users/names_controller.rb index aebc60795..dc78aad97 100644 --- a/app/controllers/users/names_controller.rb +++ b/app/controllers/users/names_controller.rb @@ -4,6 +4,7 @@ class Users::NamesController < ApplicationController before_action :authenticate_user! before_action :load_user before_action :authorize_user + before_action :redirect_to_account_page_if_acting_on_own_user, only: %i[edit] def edit; end @@ -29,4 +30,8 @@ def authorize_user def user_params params.require(:user).permit(*current_user.permitted_params.intersection([:name])) end + + def redirect_to_account_page_if_acting_on_own_user + redirect_to account_path if current_user == @user + end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 4d75be8fc..a5d7a3cd1 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -54,21 +54,6 @@ def unlock redirect_to users_path end - def resend_email_change - @user.resend_confirmation_instructions - if @user.errors.empty? - redirect_to users_path, notice: "Successfully resent email change email to #{@user.unconfirmed_email}" - else - redirect_to edit_user_path(@user), alert: "Failed to send email change email" - end - end - - def cancel_email_change - @user.cancel_email_change! - - redirect_to users_path - end - def event_logs authorize @user @logs = @user.event_logs.page(params[:page]).per(100) if @user diff --git a/app/views/users/_form_fields.html.erb b/app/views/users/_form_fields.html.erb index 4784d2139..105de457b 100644 --- a/app/views/users/_form_fields.html.erb +++ b/app/views/users/_form_fields.html.erb @@ -2,26 +2,10 @@ Name: <%= @user.name %> <%= link_to 'Change'.html_safe, edit_user_name_path(@user) %>

-

- <%= f.label :email %> - <%= f.text_field :email, autocomplete: "off", class: 'form-control input-md-6 add-label-margin' %> - <% if f.object.invited_but_not_yet_accepted? %> - Changes will trigger a new signup email. - <% end %> +

+ Email: <%= @user.email %> <%= link_to 'Change'.html_safe, edit_user_email_path(@user) %>

-<% if f.object.unconfirmed_email.present? %> -

- <%= f.label :unconfirmed_email, "Pending email" %> - <%= f.text_field :unconfirmed_email, readonly: "readonly", disabled: "disabled", class: 'form-control' %> - -

- <%= link_to "Resend confirmation email", resend_email_change_user_path(f.object), :method => :put, :class => "btn btn-primary add-right-margin" %> - <%= link_to "Cancel change", cancel_email_change_user_path(f.object), :method => :delete, :class => "btn btn-danger" %> -
-

-<% end %> - <% if policy(User).assign_role? %> <% if @user.reason_for_2sv_exemption.blank? %>

diff --git a/app/views/users/emails/edit.html.erb b/app/views/users/emails/edit.html.erb new file mode 100644 index 000000000..f1d8fbe67 --- /dev/null +++ b/app/views/users/emails/edit.html.erb @@ -0,0 +1,72 @@ +<% content_for :title_caption, "Manage other users" %> +<% content_for :title, "Change email for #{@user.name}" %> + +<% content_for :breadcrumbs, + render("govuk_publishing_components/components/breadcrumbs", { + collapse_on_mobile: true, + breadcrumbs: [ + { + title: "Dashboard", + url: root_path, + }, + { + title: "Users", + url: users_path, + }, + { + title: @user.name, + url: edit_user_path(@user), + }, + { + title: "Change email", + } + ] + }) +%> + +<% if @user.unconfirmed_email.present? %> + <%= render "govuk_publishing_components/components/notice", { + title: "A request has been made to change the email address" + } do %> +

An email has been sent to <%= @user.unconfirmed_email %> with a link to confirm the change. If they haven't received this email, we can send it again:

+ + <%= link_to "Resend confirmation email", resend_email_change_user_email_path(@user), method: :put, class: "govuk-button app-button--no-margin" %> + <%= link_to "Cancel change", cancel_email_change_user_email_path(@user), method: :delete, class: "govuk-link app-link--inline" %> + <% end %> +<% end %> + +<% if @user.errors.count > 0 %> + <% content_for :error_summary do %> + <%= render "govuk_publishing_components/components/error_summary", { + title: "There is a problem", + items: @user.errors.map do |error| + { + text: error.full_message, + href: "#user_#{error.attribute}", + } + end, + } %> + <% end %> +<% end %> + +
+
+ <%= form_for @user, url: user_email_path do %> + <%= render "govuk_publishing_components/components/input", { + label: { + text: "Email" + }, + name: "user[email]", + id: "user_email", + type: "email", + value: @user.email, + hint: @user.invited_but_not_yet_accepted? ? "Changes will trigger a new signup email." : nil, + autocomplete: "off", + error_items: @user.errors.full_messages_for(:email).map { |message| { text: message } } + } %> + <%= render "govuk_publishing_components/components/button", { + text: "Change email" + } %> + <% end %> +
+
diff --git a/config/routes.rb b/config/routes.rb index ff4918eb5..79f846c7d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -39,13 +39,15 @@ resources :users, except: [:show] do member do post :unlock - put :resend_email_change - delete :cancel_email_change get :event_logs patch :reset_two_step_verification get :require_2sv end resource :name, only: %i[edit update], controller: "users/names" + resource :email, only: %i[edit update], controller: "users/emails" do + put :resend_email_change + delete :cancel_email_change + end end get "user", to: "oauth_users#show" diff --git a/test/controllers/account/emails_controller_test.rb b/test/controllers/account/emails_controller_test.rb index b9187f061..a4f9c5722 100644 --- a/test/controllers/account/emails_controller_test.rb +++ b/test/controllers/account/emails_controller_test.rb @@ -100,8 +100,8 @@ class Account::EmailsControllerTest < ActionController::TestCase perform_enqueued_jobs do @user = create( :user_with_pending_email_change, + :with_expired_confirmation_token, confirmation_token: "old token", - confirmation_sent_at: 15.days.ago, ) sign_in @user diff --git a/test/controllers/invitations_controller_test.rb b/test/controllers/invitations_controller_test.rb index 62400143c..e82f79f31 100644 --- a/test/controllers/invitations_controller_test.rb +++ b/test/controllers/invitations_controller_test.rb @@ -167,7 +167,7 @@ class InvitationsControllerTest < ActionController::TestCase should "require inviter to be signed in" do get :new - assert_redirected_to new_user_session_path + assert_not_authenticated end end end @@ -484,7 +484,7 @@ class InvitationsControllerTest < ActionController::TestCase should "require inviter to be signed in" do post :create - assert_redirected_to new_user_session_path + assert_not_authenticated end end end @@ -549,7 +549,7 @@ class InvitationsControllerTest < ActionController::TestCase user_to_resend_for = create(:user) post :resend, params: { id: user_to_resend_for } - assert_redirected_to new_user_session_path + assert_not_authenticated end end end diff --git a/test/controllers/two_step_verification_controller_test.rb b/test/controllers/two_step_verification_controller_test.rb index b2c8df7c6..fe4ee1267 100644 --- a/test/controllers/two_step_verification_controller_test.rb +++ b/test/controllers/two_step_verification_controller_test.rb @@ -15,7 +15,7 @@ class TwoStepVerificationControllerTest < ActionController::TestCase should "redirect to login upon attempted prompt" do get :prompt - assert_redirected_to new_user_session_path + assert_not_authenticated end end diff --git a/test/controllers/user_research_recruitment_controller_test.rb b/test/controllers/user_research_recruitment_controller_test.rb index 91d7e7a3b..329b46bbc 100644 --- a/test/controllers/user_research_recruitment_controller_test.rb +++ b/test/controllers/user_research_recruitment_controller_test.rb @@ -7,7 +7,7 @@ class UserResearchRecruitmentControllerTest < ActionController::TestCase should "require users to be signed in" do put :update - assert_redirected_to new_user_session_path + assert_not_authenticated end context "when user clicks the button to dismiss the banner" do diff --git a/test/controllers/users/emails_controller_test.rb b/test/controllers/users/emails_controller_test.rb new file mode 100644 index 000000000..9377e581f --- /dev/null +++ b/test/controllers/users/emails_controller_test.rb @@ -0,0 +1,420 @@ +require "test_helper" + +class Users::EmailsControllerTest < ActionController::TestCase + include ActiveJob::TestHelper + include ActionMailer::TestHelper + + context "GET edit" do + context "signed in as Admin user" do + setup do + @admin = create(:admin_user) + sign_in(@admin) + end + + should "display form with email field" do + user = create(:user, email: "user@gov.uk") + + get :edit, params: { user_id: user } + + assert_template :edit + assert_select "form[action='#{user_email_path(user)}']" do + assert_select "input[name='user[email]']", value: "user@gov.uk" + end + end + + should "show the pending email & action links if applicable" do + user = create(:user_with_pending_email_change) + + get :edit, params: { user_id: user } + + expected_text = /An email has been sent to #{user.unconfirmed_email} with a link to confirm the change./ + assert_select ".govuk-notification-banner", text: expected_text + assert_select "a", href: resend_email_change_user_email_path(user), text: "Resend confirmation email" + assert_select "a", href: cancel_email_change_user_email_path(user), text: "Cancel change" + end + + should "authorize access if UserPolicy#edit? returns true" do + user = create(:user) + + user_policy = stub_everything("user-policy", edit?: true) + UserPolicy.stubs(:new).returns(user_policy) + + get :edit, params: { user_id: user } + + assert_template :edit + end + + should "not authorize access if UserPolicy#edit? returns false" do + user = create(:user) + + user_policy = stub_everything("user-policy", edit?: false) + UserPolicy.stubs(:new).returns(user_policy) + + get :edit, params: { user_id: user } + + assert_not_authorised + end + + should "redirect to account edit email page if admin is acting on their own user" do + get :edit, params: { user_id: @admin } + + assert_redirected_to edit_account_email_path + end + end + + context "signed in as Normal user" do + setup do + sign_in(create(:user)) + end + + should "not be authorized" do + user = create(:user) + + get :edit, params: { user_id: user } + + assert_not_authorised + end + end + + context "not signed in" do + should "not be allowed access" do + user = create(:user) + + get :edit, params: { user_id: user } + + assert_not_authenticated + end + end + end + + context "PUT update" do + context "signed in as Admin user" do + setup do + @admin = create(:admin_user) + sign_in(@admin) + end + + should "update user email" do + user = create(:user, email: "user@gov.uk") + + put :update, params: { user_id: user, user: { email: "new-user@gov.uk" } } + + assert_equal "new-user@gov.uk", user.reload.email + end + + should "not reconfirm user email" do + user = create(:user, email: "user@gov.uk") + + put :update, params: { user_id: user, user: { email: "new-user@gov.uk" } } + + assert user.reload.unconfirmed_email.blank? + end + + should "send email change notifications to old and new email address" do + perform_enqueued_jobs do + user = create(:user, email: "user@gov.uk") + + put :update, params: { user_id: user, user: { email: "new-user@gov.uk" } } + + expected_subject = "Your GOV.UK Signon development email address has been updated" + emails = ActionMailer::Base.deliveries.select { |e| e.subject = expected_subject } + assert_equal(%w[user@gov.uk new-user@gov.uk], emails.map { |mail| mail.to.first }) + end + end + + should "not send email change notifications if email has not changed" do + user = create(:user, email: "user@gov.uk") + + assert_no_enqueued_emails do + put :update, params: { user_id: user, user: { email: "user@gov.uk" } } + end + end + + should "also send an invitation email if user has not accepted invitation" do + perform_enqueued_jobs do + user = create(:invited_user, email: "user@gov.uk") + + put :update, params: { user_id: user, user: { email: "new-user@gov.uk" } } + + expected_subject = "Please confirm your account" + invitation_email = ActionMailer::Base.deliveries.find { |e| e.subject = expected_subject } + assert_equal "new-user@gov.uk", invitation_email.to.first + end + end + + should "not send an invitation email if email has not changed" do + user = create(:invited_user, email: "user@gov.uk") + + assert_no_enqueued_emails do + put :update, params: { user_id: user, user: { email: "user@gov.uk" } } + end + end + + should "record email change" do + user = create(:user, email: "user@gov.uk") + + EventLog.expects(:record_email_change).with(user, "user@gov.uk", "new-user@gov.uk", @admin) + + put :update, params: { user_id: user, user: { email: "new-user@gov.uk" } } + end + + should "should not record email change if email has not changed" do + user = create(:user, email: "user@gov.uk") + + EventLog.expects(:record_email_change).never + + put :update, params: { user_id: user, user: { email: "user@gov.uk" } } + end + + should "redirect to edit user page and display success notice" do + user = create(:user, email: "user@gov.uk") + + put :update, params: { user_id: user, user: { email: "new-user@gov.uk" } } + + assert_redirected_to edit_user_path(user) + assert "Updated user new-user@gov.uk successfully", flash[:notice] + end + + should "update user email if UserPolicy#update? returns true" do + user = create(:user, email: "user@gov.uk") + + user_policy = stub_everything("user-policy", update?: true) + UserPolicy.stubs(:new).returns(user_policy) + + put :update, params: { user_id: user, user: { email: "new-user@gov.uk" } } + + assert_equal "new-user@gov.uk", user.reload.email + end + + should "not update user email if UserPolicy#update? returns false" do + user = create(:user, email: "user@gov.uk") + + user_policy = stub_everything("user-policy", update?: false) + UserPolicy.stubs(:new).returns(user_policy) + + put :update, params: { user_id: user, user: { email: "new-user@gov.uk" } } + + assert_equal "user@gov.uk", user.reload.email + assert_not_authorised + end + + should "redisplay form if email is not valid" do + user = create(:user, email: "user@gov.uk") + + put :update, params: { user_id: user, user: { email: "" } } + + assert_template :edit + assert_select "form[action='#{user_email_path(user)}']" do + assert_select "input[name='user[email]']", value: "" + end + end + + should "display errors if email is not valid" do + user = create(:user) + + put :update, params: { user_id: user, user: { email: "" } } + + assert_select ".govuk-error-summary" do + assert_select "a", href: "#user_email", text: "Email can't be blank" + end + assert_select ".govuk-form-group" do + assert_select ".govuk-error-message", text: "Error: Email can't be blank" + assert_select "input[name='user[email]'].govuk-input--error" + end + end + end + + context "signed in as Normal user" do + setup do + sign_in(create(:user)) + end + + should "not be authorized" do + user = create(:user) + + put :update, params: { user_id: user, user: { email: "new-user@gov.uk" } } + + assert_not_authorised + end + end + + context "not signed in" do + should "not be allowed access" do + user = create(:user) + + get :edit, params: { user_id: user } + + assert_not_authenticated + end + end + end + + context "PUT resend_email_change" do + context "signed in as Admin user" do + setup do + sign_in(create(:admin_user)) + end + + should "send an email change confirmation email" do + perform_enqueued_jobs do + user = create(:user_with_pending_email_change) + + put :resend_email_change, params: { user_id: user } + + assert_equal "Confirm your email change", ActionMailer::Base.deliveries.last.subject + end + end + + should "use a new token if the old one has expired" do + user = create( + :user_with_pending_email_change, + :with_expired_confirmation_token, + confirmation_token: "old-token", + ) + + put :resend_email_change, params: { user_id: user } + + assert_not_equal "old-token", user.reload.confirmation_token + end + + should "redirect to edit user page and display success notice" do + user = create(:user_with_pending_email_change) + + put :resend_email_change, params: { user_id: user } + + assert_redirected_to edit_user_path(user) + assert_equal "Successfully resent email change email to #{user.unconfirmed_email}", flash[:notice] + end + + should "redirect to edit user email page and display failure alert if resending fails" do + user = create(:user) + + put :resend_email_change, params: { user_id: user } + + assert_redirected_to edit_user_email_path(user) + assert_equal "Failed to send email change email", flash[:alert] + end + + should "send an email change confirmation email if UserPolicy#resend_email_change? returns true" do + user = create(:user_with_pending_email_change) + + user_policy = stub_everything("user-policy", resend_email_change?: true) + UserPolicy.stubs(:new).returns(user_policy) + + assert_enqueued_emails 1 do + put :resend_email_change, params: { user_id: user } + end + assert_redirected_to edit_user_path(user) + end + + should "not send an email change confirmation email if UserPolicy#resend_email_change? returns false" do + user = create(:user_with_pending_email_change) + + user_policy = stub_everything("user-policy", resend_email_change?: false) + UserPolicy.stubs(:new).returns(user_policy) + + assert_no_enqueued_emails do + put :resend_email_change, params: { user_id: user } + end + assert_not_authorised + end + end + + context "signed in as Normal user" do + setup do + sign_in(create(:user)) + end + + should "not be authorized" do + user = create(:user_with_pending_email_change) + + put :resend_email_change, params: { user_id: user } + + assert_not_authorised + end + end + + context "not signed in" do + should "not be allowed access" do + user = create(:user_with_pending_email_change) + + get :resend_email_change, params: { user_id: user } + + assert_not_authenticated + end + end + end + + context "DELETE cancel_email_change" do + context "signed in as Admin user" do + setup do + sign_in(create(:admin_user)) + end + + should "clear unconfirmed_email & confirmation_token" do + user = create(:user_with_pending_email_change) + + delete :cancel_email_change, params: { user_id: user } + + user.reload + assert user.unconfirmed_email.blank? + assert user.confirmation_token.blank? + end + + should "redirect to the edit user page" do + user = create(:user_with_pending_email_change) + + delete :cancel_email_change, params: { user_id: user } + + assert_redirected_to edit_user_path(user) + end + + should "clear email & token if UserPolicy#cancel_email_change? returns true" do + user = create(:user_with_pending_email_change) + + user_policy = stub_everything("user-policy", cancel_email_change?: true) + UserPolicy.stubs(:new).returns(user_policy) + + put :cancel_email_change, params: { user_id: user } + + assert user.reload.unconfirmed_email.blank? + end + + should "not clear email & token if UserPolicy#cancel_email_change? returns false" do + user = create(:user_with_pending_email_change) + + user_policy = stub_everything("user-policy", cancel_email_change?: false) + UserPolicy.stubs(:new).returns(user_policy) + + put :cancel_email_change, params: { user_id: user } + + assert user.reload.unconfirmed_email.present? + assert_not_authorised + end + end + + context "signed in as Normal user" do + setup do + sign_in(create(:user)) + end + + should "not be authorized" do + user = create(:user_with_pending_email_change) + + put :cancel_email_change, params: { user_id: user } + + assert_not_authorised + end + end + + context "not signed in" do + should "not be allowed access" do + user = create(:user_with_pending_email_change) + + get :cancel_email_change, params: { user_id: user } + + assert_not_authenticated + end + end + end +end diff --git a/test/controllers/users/names_controller_test.rb b/test/controllers/users/names_controller_test.rb index 5a0cedbeb..7c10bdea8 100644 --- a/test/controllers/users/names_controller_test.rb +++ b/test/controllers/users/names_controller_test.rb @@ -4,7 +4,8 @@ class Users::NamesControllerTest < ActionController::TestCase context "GET edit" do context "signed in as Admin user" do setup do - sign_in(create(:admin_user)) + @admin = create(:admin_user) + sign_in(@admin) end should "display form with name field" do @@ -39,6 +40,12 @@ class Users::NamesControllerTest < ActionController::TestCase assert_not_authorised end + + should "redirect to account page if admin is acting on their own user" do + get :edit, params: { user_id: @admin } + + assert_redirected_to account_path + end end context "signed in as Normal user" do @@ -61,7 +68,7 @@ class Users::NamesControllerTest < ActionController::TestCase get :edit, params: { user_id: user } - assert_redirected_to new_user_session_path + assert_not_authenticated end end end @@ -183,7 +190,7 @@ class Users::NamesControllerTest < ActionController::TestCase get :edit, params: { user_id: user } - assert_redirected_to new_user_session_path + assert_not_authenticated end end end diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 4b7d85c84..f047db879 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -269,18 +269,11 @@ class UsersControllerTest < ActionController::TestCase assert_select "a", href: edit_user_name_path(not_an_admin), text: "Change name" end - should "show the form with an email field" do - not_an_admin = create(:user) + should "display the user's email and a link to change the email" do + not_an_admin = create(:user, email: "user-name@gov.uk") get :edit, params: { id: not_an_admin.id } - assert_select "form[action='#{user_path(not_an_admin)}']" do - assert_select "input[name='user[email]'][value='#{not_an_admin.email}']" - end - end - - should "show the pending email if applicable" do - another_user = create(:user_with_pending_email_change) - get :edit, params: { id: another_user.id } - assert_select "input[name='user[unconfirmed_email]'][value='#{another_user.unconfirmed_email}']" + assert_select "*", text: /Email: user-name@gov.uk/ + assert_select "a", href: edit_user_email_path(not_an_admin), text: "Change email" end should "show the organisation to which the user belongs" do @@ -592,9 +585,10 @@ class UsersControllerTest < ActionController::TestCase end should "not be able to update superadmins" do + organisation = create(:organisation) superadmin = create(:superadmin_user) - put :edit, params: { id: superadmin.id, user: { email: "normal_user@example.com" } } + put :edit, params: { id: superadmin.id, user: { organisation_id: organisation.id } } assert_not_authorised end @@ -613,50 +607,6 @@ class UsersControllerTest < ActionController::TestCase assert_equal "normal", not_an_admin.reload.role end - context "changing an email" do - should "not re-confirm email" do - normal_user = create(:user, email: "old@email.com") - put :update, params: { id: normal_user.id, user: { email: "new@email.com" } } - - assert_nil normal_user.reload.unconfirmed_email - assert_equal "new@email.com", normal_user.email - end - - should "log an event" do - normal_user = create(:user, email: "old@email.com") - put :update, params: { id: normal_user.id, user: { email: "new@email.com" } } - - assert_equal 1, EventLog.where(event_id: EventLog::EMAIL_CHANGED.id, uid: normal_user.uid, initiator_id: @user.id).count - end - - should "send email change notifications to old and new email address" do - perform_enqueued_jobs do - normal_user = create(:user, email: "old@email.com") - put :update, params: { id: normal_user.id, user: { email: "new@email.com" } } - - email_change_notifications = ActionMailer::Base.deliveries[-2..] - assert_equal email_change_notifications.map(&:subject).uniq.count, 1 - assert_match(/Your .* Signon development email address has been updated/, email_change_notifications.map(&:subject).first) - assert_equal(%w[old@email.com new@email.com], email_change_notifications.map { |mail| mail.to.first }) - end - end - - context "an invited-but-not-yet-accepted user" do - should "change the email, and send an invitation email" do - perform_enqueued_jobs do - another_user = User.invite!(name: "Ali", email: "old@email.com") - put :update, params: { id: another_user.id, user: { email: "new@email.com" } } - - another_user.reload - assert_equal "new@email.com", another_user.reload.email - invitation_email = ActionMailer::Base.deliveries[-3] - assert_equal "Please confirm your account", invitation_email.subject - assert_equal "new@email.com", invitation_email.to.first - end - end - end - end - should "push changes out to apps" do another_user = create(:user, name: "Old Name") PermissionUpdater.expects(:perform_on).with(another_user).once @@ -739,8 +689,9 @@ class UsersControllerTest < ActionController::TestCase should "redisplay the form if save fails" do organisation = @organisation_admin.organisation organisation_admin_for_same_organisation = create(:organisation_admin_user, organisation:) + UserUpdate.stubs(:new).returns(stub("UserUpdate", call: false)) - put :update, params: { id: organisation_admin_for_same_organisation.id, user: { email: "" } } + put :update, params: { id: organisation_admin_for_same_organisation.id, user: {} } assert_select "form#edit_user_#{organisation_admin_for_same_organisation.id}" end @@ -755,64 +706,12 @@ class UsersControllerTest < ActionController::TestCase should "redisplay the form if save fails" do organisation = @super_organisation_admin.organisation super_organisation_admin_for_same_organisation = create(:super_organisation_admin_user, organisation:) + UserUpdate.stubs(:new).returns(stub("UserUpdate", call: false)) - put :update, params: { id: super_organisation_admin_for_same_organisation.id, user: { email: "" } } + put :update, params: { id: super_organisation_admin_for_same_organisation.id, user: {} } assert_select "form#edit_user_#{super_organisation_admin_for_same_organisation.id}" end end end - - context "PUT resend_email_change" do - context "signed in as Admin user" do - setup do - @user = create(:admin_user, email: "admin@gov.uk") - sign_in @user - end - - should "send an email change confirmation email" do - perform_enqueued_jobs do - another_user = create(:user_with_pending_email_change) - put :resend_email_change, params: { id: another_user.id } - - assert_equal "Confirm your email change", ActionMailer::Base.deliveries.last.subject - end - end - - should "use a new token if it's expired" do - another_user = create( - :user_with_pending_email_change, - confirmation_token: "old token", - confirmation_sent_at: 15.days.ago, - ) - put :resend_email_change, params: { id: another_user.id } - - assert_not_equal "old token", another_user.reload.confirmation_token - end - end - end - - context "DELETE cancel_email_change" do - context "signed in as Admin user" do - setup do - @user = create(:admin_user, email: "admin@gov.uk") - sign_in @user - - @another_user = create(:user_with_pending_email_change) - end - - should "clear the unconfirmed_email and the confirmation_token" do - delete :cancel_email_change, params: { id: @another_user.id } - - @another_user.reload - assert_nil @another_user.unconfirmed_email - assert_nil @another_user.confirmation_token - end - - should "redirect to the users page" do - delete :cancel_email_change, params: { id: @another_user.id } - assert_redirected_to users_path - end - end - end end diff --git a/test/factories/users.rb b/test/factories/users.rb index 1c4f23ec7..0cd29c0a1 100644 --- a/test/factories/users.rb +++ b/test/factories/users.rb @@ -65,6 +65,11 @@ confirmation_sent_at { Time.zone.now } end + trait :with_expired_confirmation_token do + confirmation_token { "expired-token" } + confirmation_sent_at { Devise.confirm_within.ago - 1.day } + end + factory :superadmin_user, parent: :user do sequence(:email) { |n| "superadmin#{n}@example.com" } role { Roles::Superadmin.role_name } diff --git a/test/integration/email_change_test.rb b/test/integration/email_change_test.rb index 030de99c4..6e6979316 100644 --- a/test/integration/email_change_test.rb +++ b/test/integration/email_change_test.rb @@ -80,6 +80,7 @@ class EmailChangeTest < ActionDispatch::IntegrationTest visit new_user_session_path signin_with(@admin) visit edit_user_path(user) + click_link "Change email" click_link "Cancel change" signout diff --git a/test/support/user_helpers.rb b/test/support/user_helpers.rb index 76f4f4d9c..275bbe1f4 100644 --- a/test/support/user_helpers.rb +++ b/test/support/user_helpers.rb @@ -27,8 +27,9 @@ def signout def admin_changes_email_address(options) visit edit_user_path(options[:user].id) + click_link "Change email" fill_in "Email", with: options[:new_email] - click_button "Update User" + click_button "Change email" end def enter_2sv_code(secret) diff --git a/test/test_helper.rb b/test/test_helper.rb index cc521aa6d..85f39aa11 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -48,6 +48,10 @@ def sign_out(_user) @controller.unstub(:current_user) end + def assert_not_authenticated + assert_redirected_to new_user_session_path + end + def assert_not_authorised assert_redirected_to root_path assert_equal "You do not have permission to perform this action.", flash[:alert]