From 900695b390d9aa0b62b4295219b3933f47725472 Mon Sep 17 00:00:00 2001 From: Mike Patrick Date: Tue, 29 Aug 2023 16:26:59 +0100 Subject: [PATCH 1/4] Load helper in test_helper.rb instead of test file I'm going to use a method from this helper and other helpers seem to be loaded centralling in test_helper.rb --- test/integration/email_change_test.rb | 2 -- test/test_helper.rb | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/email_change_test.rb b/test/integration/email_change_test.rb index 5468341f1..2fcb570ca 100644 --- a/test/integration/email_change_test.rb +++ b/test/integration/email_change_test.rb @@ -1,8 +1,6 @@ require "test_helper" -require "support/user_account_helpers" class EmailChangeTest < ActionDispatch::IntegrationTest - include UserAccountHelpers include ActiveJob::TestHelper context "by an admin" do diff --git a/test/test_helper.rb b/test/test_helper.rb index acd6e79da..b8232d792 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -73,6 +73,7 @@ def sign_out(_user) require "support/managing_two_sv_helpers" require "support/analytics_helpers" require "support/html_table_helpers" +require "support/user_account_helpers" class ActiveRecord::Base mattr_accessor :shared_connection @@ -94,6 +95,7 @@ class ActionDispatch::IntegrationTest include EmailHelpers include ConfirmationTokenHelpers include AnalyticsHelpers + include UserAccountHelpers def assert_response_contains(content) assert page.has_content?(content), "Expected to find '#{content}' in:\n#{page.text}" From fe5cb8de782ff18c079c129e1c2b3689ff0157b4 Mon Sep 17 00:00:00 2001 From: Mike Patrick Date: Tue, 29 Aug 2023 16:31:04 +0100 Subject: [PATCH 2/4] Remove unnecessary `include` While I'm here and doing this sort of thing, this helper's already included in test_helper.rb, so doesn't need to be included here --- test/integration/inviting_users_test.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/inviting_users_test.rb b/test/integration/inviting_users_test.rb index 7514278a9..724542f6d 100644 --- a/test/integration/inviting_users_test.rb +++ b/test/integration/inviting_users_test.rb @@ -1,7 +1,6 @@ require "test_helper" class InvitingUsersTest < ActionDispatch::IntegrationTest - include EmailHelpers include ActiveJob::TestHelper should "send the user an invitation token" do From 64dce32cd85d3fe1a65490a107e97e4f68780b93 Mon Sep 17 00:00:00 2001 From: Mike Patrick Date: Tue, 29 Aug 2023 16:32:29 +0100 Subject: [PATCH 3/4] Make test name more accurate It should send the user the token, but that's not at all what's being tested here --- test/integration/inviting_users_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/inviting_users_test.rb b/test/integration/inviting_users_test.rb index 724542f6d..156dffea8 100644 --- a/test/integration/inviting_users_test.rb +++ b/test/integration/inviting_users_test.rb @@ -3,7 +3,7 @@ class InvitingUsersTest < ActionDispatch::IntegrationTest include ActiveJob::TestHelper - should "send the user an invitation token" do + should "ask the invited user to set a password" do user = User.invite!(name: "Jim", email: "jim@web.com") visit accept_user_invitation_path(invitation_token: user.raw_invitation_token) From db825dc0c57d6f8495c306a0ed10ee27ee09b436 Mon Sep 17 00:00:00 2001 From: Mike Patrick Date: Tue, 29 Aug 2023 16:34:15 +0100 Subject: [PATCH 4/4] Require user to sign in after accepting invitation To mitigate the security risk posed by leaked invitation tokens, we're no longer automatically signing a user in when they accept an invitation. This is a feature of Devise's Invitable module, so not a lot of work on our part. --- config/initializers/devise.rb | 5 +++++ test/integration/inviting_users_test.rb | 19 ++++++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 040e57cf4..0267e57d4 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -116,6 +116,11 @@ # Default: false config.validate_on_invite = true + # Auto-login after the user accepts the invite. If this is false, + # the user will need to manually log in after accepting the invite. + # Default: true + config.allow_insecure_sign_in_after_accept = false + # ==> Configuration for :confirmable # A period that the user is allowed to access the website even without # confirming their account. For instance, if set to 2.days, the user will be diff --git a/test/integration/inviting_users_test.rb b/test/integration/inviting_users_test.rb index 156dffea8..dd1726dc3 100644 --- a/test/integration/inviting_users_test.rb +++ b/test/integration/inviting_users_test.rb @@ -11,7 +11,24 @@ class InvitingUsersTest < ActionDispatch::IntegrationTest fill_in "Confirm new password", with: "this 1s 4 v3333ry s3cur3 p4ssw0rd.!Z" click_button "Save password" - assert_response_contains("You are now signed in") + assert_response_contains("Your password was set successfully.") + end + + should "require the invited user to sign in after setting their password" do + user = User.invite!(name: "Neptuno Keighley", email: "neptuno.keighley@office.gov.uk") + + accept_invitation( + invitation_token: user.raw_invitation_token, + password: "pretext annoying headpiece waviness header slinky", + ) + + assert_response_contains("Sign in to GOV.UK") + + fill_in "Email", with: "neptuno.keighley@office.gov.uk" + fill_in "Password", with: "pretext annoying headpiece waviness header slinky" + click_button "Sign in" + + assert_response_contains("Make your account more secure by setting up 2‑step verification.") end should "not send invitation token to Google Analytics" do