diff --git a/app/controllers/users/email_addresses/confirmations_controller.rb b/app/controllers/users/email_addresses/confirmations_controller.rb index dad998671..b5fd334ea 100644 --- a/app/controllers/users/email_addresses/confirmations_controller.rb +++ b/app/controllers/users/email_addresses/confirmations_controller.rb @@ -8,12 +8,14 @@ class Users::EmailAddresses::ConfirmationsController < ApplicationController end def create - user = @user.change_email_address_using_token(token) + if @user.change_email_address_using_token(token) + terminate_session if Current.session + start_new_session_for @user.identity - terminate_session if Current.session - start_new_session_for user.identity - - redirect_to edit_user_url(script_name: user.account.slug, id: user) + redirect_to edit_user_url(script_name: @user.account.slug, id: @user) + else + render :invalid_token, status: :unprocessable_entity + end end private diff --git a/app/models/user/email_address_changeable.rb b/app/models/user/email_address_changeable.rb index ce18d46d0..2c8de7217 100644 --- a/app/models/user/email_address_changeable.rb +++ b/app/models/user/email_address_changeable.rb @@ -7,14 +7,12 @@ module User::EmailAddressChangeable def change_email_address_using_token(token) parsed_token = SignedGlobalID.parse(token, for: EMAIL_CHANGE_TOKEN_PURPOSE) - if parsed_token.nil? - raise ArgumentError, "The token is invalid" - elsif parsed_token.find != self - raise ArgumentError, "The token was generated for a different user" - elsif identity.email_address != parsed_token.params.fetch("old_email_address") - raise ArgumentError, "The token was generated for a different email address" + old_email_address = parsed_token&.params&.fetch("old_email_address") + new_email_address = parsed_token&.params&.fetch("new_email_address") + + if parsed_token.nil? || parsed_token.find != self || identity.email_address != old_email_address + false else - new_email_address = parsed_token.params.fetch("new_email_address") change_email_address(new_email_address) end end @@ -37,8 +35,6 @@ module User::EmailAddressChangeable new_identity = Identity.find_or_create_by!(email_address: new_email_address) update!(identity: new_identity) end - - self end private diff --git a/app/views/users/email_addresses/confirmations/invalid_token.html.erb b/app/views/users/email_addresses/confirmations/invalid_token.html.erb new file mode 100644 index 000000000..13e66c97d --- /dev/null +++ b/app/views/users/email_addresses/confirmations/invalid_token.html.erb @@ -0,0 +1,16 @@ +<% @page_title = "Confirm email change" %> + +
+
+

+ <%= @page_title %> +

+

Something went wrong.

+
+ +

+ It looks like the email confirmation link is no longer valid. + Try changing your email address again from <%= link_to "your profile", edit_user_url(script_name: @user.account.slug, id: @user) %>. + If that doesn't help, please <%= mail_to "send us an email", "support@fizzy.do" %>. +

+
diff --git a/test/controllers/users/email_addresses/confirmations_controller_test.rb b/test/controllers/users/email_addresses/confirmations_controller_test.rb index b16b6ea7c..a68cfd175 100644 --- a/test/controllers/users/email_addresses/confirmations_controller_test.rb +++ b/test/controllers/users/email_addresses/confirmations_controller_test.rb @@ -3,6 +3,7 @@ require "test_helper" class Users::EmailAddresses::ConfirmationsControllerTest < ActionDispatch::IntegrationTest setup do @user = users(:david) + @old_email = @user.identity.email_address @new_email = "newemail@example.com" @token = @user.send(:generate_email_address_change_token, to: @new_email) end @@ -13,8 +14,6 @@ class Users::EmailAddresses::ConfirmationsControllerTest < ActionDispatch::Integ end test "create" do - old_email = @user.identity.email_address - post user_email_address_confirmation_path(user_id: @user.id, email_address_token: @token, script_name: @user.account.slug) assert_equal @new_email, @user.reload.identity.email_address @@ -22,8 +21,10 @@ class Users::EmailAddresses::ConfirmationsControllerTest < ActionDispatch::Integ end test "create with invalid token" do - assert_raises(ArgumentError) do - post user_email_address_confirmation_path(user_id: @user.id, email_address_token: "invalid", script_name: @user.account.slug) - end + post user_email_address_confirmation_path(user_id: @user.id, email_address_token: "invalid", script_name: @user.account.slug) + + assert_equal @user.identity.email_address, @old_email + assert_response :unprocessable_entity + assert_match /Something went wrong/, response.body end end diff --git a/test/models/user/email_address_changeable_test.rb b/test/models/user/email_address_changeable_test.rb index 5e1158cf3..951170297 100644 --- a/test/models/user/email_address_changeable_test.rb +++ b/test/models/user/email_address_changeable_test.rb @@ -7,6 +7,7 @@ class User::EmailAddressChangeableTest < ActiveSupport::TestCase @identity = identities(:kevin) @user = @identity.users.find_by!(account: accounts("37s")) @new_email = "newart@example.com" + @old_email = @identity.email_address end test "send_email_address_change_confirmation" do @@ -42,15 +43,15 @@ class User::EmailAddressChangeableTest < ActiveSupport::TestCase end test "change_email_address_using_token with invalid token" do - assert_raises(ArgumentError, match: /invalid/) do - @user.change_email_address_using_token("invalid_token") - end + assert_not @user.change_email_address_using_token("invalid_token") + assert_equal @old_email, @user.reload.identity.email_address token = @user.send(:generate_email_address_change_token, to: @new_email) - @identity.update!(email_address: "different@example.com") + old_email = "#{SecureRandom.hex(16)}@example.com" + @identity.update!(email_address: old_email) + @user.reload - assert_raises(ArgumentError, match: /different email address/) do - @user.change_email_address_using_token(token) - end + assert_not @user.change_email_address_using_token(token) + assert_equal old_email, @user.reload.identity.email_address end end