From 56c41d45eb0efcb51e25f7c4adbeac8fe794f32f Mon Sep 17 00:00:00 2001
From: "Stanko K.R." <stanko@stanko.io>
Date: Fri, 14 Nov 2025 08:33:00 +0100
Subject: [PATCH] Reinstate email changes

---
 .../confirmations_controller.rb               |  2 +-
 app/mailers/identity_mailer.rb                |  7 +++
 app/models/user.rb                            |  2 +-
 app/models/user/email_address_changeable.rb   | 57 +++++++++++++++++++
 .../email_change_confirmation.html.erb        |  2 +-
 .../email_change_confirmation.text.erb        |  2 +-
 .../confirmations/show.html.erb               |  2 +-
 .../sessions/menus_controller_test.rb         |  6 +-
 .../confirmations_controller_test.rb          | 35 ++++++++++++
 .../users/email_addresses_controller_test.rb  | 48 ++++++++++++++++
 test/helpers/application_helper_test.rb       |  8 +--
 .../previews/identity_mailer_preview.rb       | 13 +++++
 test/models/identity_test.rb                  |  5 +-
 .../user/email_address_changeable_test.rb     | 55 ++++++++++++++++++
 14 files changed, 230 insertions(+), 14 deletions(-)
 create mode 100644 app/mailers/identity_mailer.rb
 create mode 100644 app/models/user/email_address_changeable.rb
 create mode 100644 test/controllers/users/email_addresses/confirmations_controller_test.rb
 create mode 100644 test/controllers/users/email_addresses_controller_test.rb
 create mode 100644 test/mailers/previews/identity_mailer_preview.rb
 create mode 100644 test/models/user/email_address_changeable_test.rb

diff --git a/app/controllers/users/email_addresses/confirmations_controller.rb b/app/controllers/users/email_addresses/confirmations_controller.rb
index 5e5d8f0ed..10ad9ffed 100644
--- a/app/controllers/users/email_addresses/confirmations_controller.rb
+++ b/app/controllers/users/email_addresses/confirmations_controller.rb
@@ -12,7 +12,7 @@ class Users::EmailAddresses::ConfirmationsController < ApplicationController
     user = User.change_email_address_using_token(token)
 
     terminate_session if Current.session
-    start_new_session_for user.reload.identity
+    start_new_session_for user.identity
 
     redirect_to edit_user_url(script_name: user.account.slug, id: user)
   end
diff --git a/app/mailers/identity_mailer.rb b/app/mailers/identity_mailer.rb
new file mode 100644
index 000000000..1e988f2b0
--- /dev/null
+++ b/app/mailers/identity_mailer.rb
@@ -0,0 +1,7 @@
+class IdentityMailer < ApplicationMailer
+  def email_change_confirmation(email_address:, token:, user:)
+    @token = token
+    @user = user
+    mail to: email_address, subject: "Confirm your new email address"
+  end
+end
diff --git a/app/models/user.rb b/app/models/user.rb
index 6b985c2df..6d0bba141 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1,5 +1,5 @@
 class User < ApplicationRecord
-  include Accessor, Assignee, Attachable, Configurable,
+  include Accessor, Assignee, Attachable, Configurable, EmailAddressChangeable,
     Mentionable, Named, Notifiable, Role, Searcher, Watcher
   include Timelined # Depends on Accessor
 
diff --git a/app/models/user/email_address_changeable.rb b/app/models/user/email_address_changeable.rb
new file mode 100644
index 000000000..80e194231
--- /dev/null
+++ b/app/models/user/email_address_changeable.rb
@@ -0,0 +1,57 @@
+module User::EmailAddressChangeable
+  EMAIL_CHANGE_TOKEN_PURPOSE = "change_email_address"
+  EMAIL_CHANGE_TOKEN_EXPIRATION = 30.minutes
+
+  extend ActiveSupport::Concern
+
+  class_methods do
+    def change_email_address_using_token(token)
+      parsed_token = SignedGlobalID.parse(token, for: EMAIL_CHANGE_TOKEN_PURPOSE)
+      user = parsed_token&.find
+
+      if parsed_token.nil?
+        raise ArgumentError, "The token is invalid"
+      elsif user.nil?
+        raise ArgumentError, "The user no longer exists"
+      elsif user.identity.email_address != parsed_token.params.fetch("old_email_address")
+        raise ArgumentError, "The token was generated for a different email address"
+      else
+        new_email_address = parsed_token.params.fetch("new_email_address")
+        user.change_email_address(new_email_address)
+      end
+    end
+  end
+
+  def send_email_address_change_confirmation(new_email_address)
+    token = generate_email_address_change_token(
+      to: new_email_address,
+      expires_in: EMAIL_CHANGE_TOKEN_EXPIRATION
+    )
+
+    IdentityMailer.email_change_confirmation(
+      email_address: new_email_address,
+      token: token,
+      user: self
+    ).deliver_later
+  end
+
+  def change_email_address(new_email_address)
+    transaction do
+      new_identity = Identity.find_or_create_by!(email_address: new_email_address)
+      update!(identity: new_identity)
+    end
+
+    self
+  end
+
+  private
+    def generate_email_address_change_token(from: identity.email_address, to:, **options)
+      options = options.reverse_merge(
+        for: EMAIL_CHANGE_TOKEN_PURPOSE,
+        old_email_address: from,
+        new_email_address: to,
+      )
+
+      to_sgid(**options).to_s
+    end
+end
diff --git a/app/views/mailers/identity_mailer/email_change_confirmation.html.erb b/app/views/mailers/identity_mailer/email_change_confirmation.html.erb
index f16c866ab..43782efb1 100644
--- a/app/views/mailers/identity_mailer/email_change_confirmation.html.erb
+++ b/app/views/mailers/identity_mailer/email_change_confirmation.html.erb
@@ -1,6 +1,6 @@
 <p class="subtitle">Confirm your email address change</p>
 
-<%= link_to "Yes use use this email address", email_address_confirmation_url(user_id: @user.id, email_address_token: @token), class: "btn" %>
+<%= link_to "Yes use use this email address", user_email_address_confirmation_url(user_id: @user.id, email_address_token: @token), class: "btn" %>
 
 <p class="margin-block-start-double">If you didn’t request this change, you can ignore this email. Your email address WILL NOT be changed unless you hit the button.</p>
 
diff --git a/app/views/mailers/identity_mailer/email_change_confirmation.text.erb b/app/views/mailers/identity_mailer/email_change_confirmation.text.erb
index b6ecd33c0..39988263d 100644
--- a/app/views/mailers/identity_mailer/email_change_confirmation.text.erb
+++ b/app/views/mailers/identity_mailer/email_change_confirmation.text.erb
@@ -3,6 +3,6 @@ Confirm your email address change
 
 Hit the link below to use this email address in Fizzy:
 
-<%= email_address_confirmation_url(user_id: @user.id, email_address_token: @token) %>
+<%= user_email_address_confirmation_url(user_id: @user.id, email_address_token: @token) %>
 
 If you didn’t request this change, you can ignore this email. Your email address WILL NOT be changed unless you hit the button.
diff --git a/app/views/users/email_addresses/confirmations/show.html.erb b/app/views/users/email_addresses/confirmations/show.html.erb
index 3410a9f74..ed6e24f72 100644
--- a/app/views/users/email_addresses/confirmations/show.html.erb
+++ b/app/views/users/email_addresses/confirmations/show.html.erb
@@ -8,7 +8,7 @@
     <p class="margin-none">Just a sec while we confirm your new email address.</p>
   </header>
 
-  <%= form_with url: email_address_confirmation_path(user_id: @user.id), method: :post, data: { controller: "form auto-submit" } do |form| %>
+  <%= form_with url: user_email_address_confirmation_path(user_id: @user.id), method: :post, data: { controller: "form auto-submit" } do |form| %>
     <%= form.hidden_field :email_address_token, value: params[:email_address_token] %>
 
     <button type="submit" class="btn btn--link center" data-form-target="submit">
diff --git a/test/controllers/sessions/menus_controller_test.rb b/test/controllers/sessions/menus_controller_test.rb
index 870aca45c..615de2076 100644
--- a/test/controllers/sessions/menus_controller_test.rb
+++ b/test/controllers/sessions/menus_controller_test.rb
@@ -5,7 +5,7 @@ class Sessions::MenusControllerTest < ActionDispatch::IntegrationTest
     @identity = identities(:kevin)
   end
 
-  test "show with no memberships" do
+  test "show with no account" do
     sign_in_as @identity
     @identity.users.delete_all
 
@@ -16,7 +16,7 @@ class Sessions::MenusControllerTest < ActionDispatch::IntegrationTest
     assert_response :success, "Renders an empty menu"
   end
 
-  test "show with exactly one membership" do
+  test "show with exactly one account" do
     sign_in_as @identity
     @identity.users.delete_all
     account = Account.create!(external_account_id: 9999991, name: "Test Account")
@@ -30,7 +30,7 @@ class Sessions::MenusControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to root_url(script_name: "/9999991")
   end
 
-  test "show with multiple memeberships" do
+  test "show with multiple accounts" do
     sign_in_as @identity
     @identity.users.delete_all
     account1 = Account.create!(external_account_id: 9999992, name: "37signals")
diff --git a/test/controllers/users/email_addresses/confirmations_controller_test.rb b/test/controllers/users/email_addresses/confirmations_controller_test.rb
new file mode 100644
index 000000000..1c64ade73
--- /dev/null
+++ b/test/controllers/users/email_addresses/confirmations_controller_test.rb
@@ -0,0 +1,35 @@
+require "test_helper"
+
+class Users::EmailAddresses::ConfirmationsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    @user = users(:david)
+    @new_email = "newemail@example.com"
+    @token = @user.send(:generate_email_address_change_token, to: @new_email)
+  end
+
+  test "show" do
+    untenanted do
+      get user_email_address_confirmation_path(user_id: @user.id, email_address_token: @token)
+      assert_response :success
+    end
+  end
+
+  test "create" do
+    old_email = @user.identity.email_address
+
+    untenanted do
+      post user_email_address_confirmation_path(user_id: @user.id, email_address_token: @token)
+
+      assert_equal @new_email, @user.reload.identity.email_address
+      assert_redirected_to edit_user_url(script_name: @user.account.slug, id: @user)
+    end
+  end
+
+  test "create with invalid token" do
+    untenanted do
+      assert_raises(ArgumentError) do
+        post user_email_address_confirmation_path(user_id: @user.id, email_address_token: "invalid")
+      end
+    end
+  end
+end
diff --git a/test/controllers/users/email_addresses_controller_test.rb b/test/controllers/users/email_addresses_controller_test.rb
new file mode 100644
index 000000000..062b232c0
--- /dev/null
+++ b/test/controllers/users/email_addresses_controller_test.rb
@@ -0,0 +1,48 @@
+require "test_helper"
+
+class Users::EmailAddressesControllerTest < ActionDispatch::IntegrationTest
+  include ActionMailer::TestHelper
+
+  setup do
+    sign_in_as :david
+    @user = users(:david)
+  end
+
+  test "new" do
+    untenanted do
+      get new_user_email_address_path(@user)
+      assert_response :success
+    end
+  end
+
+  test "create" do
+    untenanted do
+      assert_emails 1 do
+        post user_email_addresses_path(@user), params: { email_address: "newemail@example.com" }
+      end
+      assert_response :success
+    end
+  end
+
+  test "create with existing email in same account" do
+    existing_user = users(:kevin)
+    existing_email = existing_user.identity.email_address
+
+    untenanted do
+      post user_email_addresses_path(@user), params: { email_address: existing_email }
+      assert_redirected_to new_user_email_address_path(@user)
+      assert_equal "You already have a user in this account with that email address", flash[:alert]
+    end
+  end
+
+  test "create for other user" do
+    other_user = users(:kevin)
+
+    untenanted do
+      assert_no_emails do
+        post user_email_addresses_path(other_user), params: { email_address: "newemail@example.com" }
+      end
+      assert_response :not_found
+    end
+  end
+end
diff --git a/test/helpers/application_helper_test.rb b/test/helpers/application_helper_test.rb
index 8fbfa2a0c..9518718c5 100644
--- a/test/helpers/application_helper_test.rb
+++ b/test/helpers/application_helper_test.rb
@@ -18,13 +18,13 @@ class ApplicationHelperTest < ActionView::TestCase
     assert_select parse(page_title_tag), "title", text: "Holodeck | Fizzy"
   end
 
-  test "page_title_tag on tenanted page when user has a single membership" do
+  test "page_title_tag on tenanted page when user has a single account" do
     Current.session = sessions(:david)
 
     assert_select parse(page_title_tag), "title", text: "Fizzy"
   end
 
-  test "page_title_tag on tenanted page when user has multiple memberships" do
+  test "page_title_tag on tenanted page when user has multiple accounts" do
     Current.session = sessions(:david)
     other_account = Account.create!(external_account_id: "dangling-tenant", name: "Other Account")
     identities(:david).users.create!(account: other_account, name: "David")
@@ -32,14 +32,14 @@ class ApplicationHelperTest < ActionView::TestCase
     assert_select parse(page_title_tag), "title", text: "37signals | Fizzy"
   end
 
-  test "page_title_tag on tenanted page with a page title when user has a single membership" do
+  test "page_title_tag on tenanted page with a page title when user has a single account" do
     Current.session = sessions(:david)
     @page_title = "Holodeck"
 
     assert_select parse(page_title_tag), "title", text: "Holodeck | Fizzy"
   end
 
-  test "page_title_tag on tenanted page with a page title when user has multiple memberships" do
+  test "page_title_tag on tenanted page with a page title when user has multiple account" do
     Current.session = sessions(:david)
     other_account = Account.create!(external_account_id: "dangling-tenant", name: "Other Account")
     identities(:david).users.create!(account: other_account, name: "David")
diff --git a/test/mailers/previews/identity_mailer_preview.rb b/test/mailers/previews/identity_mailer_preview.rb
new file mode 100644
index 000000000..428c3e4be
--- /dev/null
+++ b/test/mailers/previews/identity_mailer_preview.rb
@@ -0,0 +1,13 @@
+class IdentityMailerPreview < ActionMailer::Preview
+  def email_change_confirmation
+    new_email_address = "new.email@example.com"
+    user = User.all.sample
+    token = user.send(:generate_email_address_change_token, to: new_email_address)
+
+    IdentityMailer.email_change_confirmation(
+      email_address: new_email_address,
+      token: token,
+      user: user
+    )
+  end
+end
diff --git a/test/models/identity_test.rb b/test/models/identity_test.rb
index b1b0a2e6d..90ec751dd 100644
--- a/test/models/identity_test.rb
+++ b/test/models/identity_test.rb
@@ -19,7 +19,7 @@ class IdentityTest < ActiveSupport::TestCase
     assert_not Identity.new(email_address: "test@example.com").staff?
   end
 
-  test "join creates membership and user for account" do
+  test "join" do
     identity = identities(:david)
     account = accounts(:initech)
 
@@ -27,7 +27,8 @@ class IdentityTest < ActiveSupport::TestCase
       identity.join(account)
     end
 
-    user = account.users.find_by(identity: identity)
+    user = account.users.find_by!(identity: identity)
+
     assert_not_nil user
     assert_equal identity, user.identity
     assert_equal identity.email_address, user.name
diff --git a/test/models/user/email_address_changeable_test.rb b/test/models/user/email_address_changeable_test.rb
new file mode 100644
index 000000000..98b7a6fc7
--- /dev/null
+++ b/test/models/user/email_address_changeable_test.rb
@@ -0,0 +1,55 @@
+require "test_helper"
+
+class User::EmailAddressChangeableTest < ActiveSupport::TestCase
+  include ActionMailer::TestHelper
+
+  setup do
+    @identity = identities(:kevin)
+    @user = @identity.users.find_by!(account: accounts("37s"))
+    @new_email = "newart@example.com"
+  end
+
+  test "send_email_address_change_confirmation" do
+    assert_emails 1 do
+      @user.send_email_address_change_confirmation(@new_email)
+    end
+  end
+
+  test "change_email_address" do
+    old_identity = @identity
+
+    assert_difference -> { Identity.count }, +1 do
+      @user.change_email_address(@new_email)
+    end
+
+    assert_equal @new_email, @user.reload.identity.email_address
+    assert_not old_identity.reload.users.exists?(id: @user.id)
+    assert_equal @new_email, @user.reload.identity.email_address
+
+    assert_no_difference -> { Identity.count } do
+      @user.change_email_address(identities(:david).email_address)
+    end
+    assert_equal identities(:david).email_address, @user.reload.identity.email_address
+  end
+
+  test "change_email_address_using_token" do
+    token = @user.send(:generate_email_address_change_token, to: @new_email)
+
+    User.change_email_address_using_token(token)
+
+    assert_equal @new_email, @user.reload.identity.email_address
+  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
+
+    token = @user.send(:generate_email_address_change_token, to: @new_email)
+    @identity.update!(email_address: "different@example.com")
+
+    assert_raises(ArgumentError, match: /different email address/) do
+      User.change_email_address_using_token(token)
+    end
+  end
+end