From 160ba295b5e24fc77a1b8d9c192b7f8f3f8bc66b Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Mon, 19 Oct 2026 11:36:16 -0700
Subject: [PATCH] End sign-in links made before a password change

A transfer link signed only the user id, so one copied before the
password changed still started a session for up to four hours after.
Bind it to the password salt with generates_token_for, so a new
password ends those links too. Links made before this upgrade stop
working; they last four hours at most.
---
 app/models/user/transferable.rb               | 11 ++++-
 .../sessions/transfers_controller_test.rb     | 11 +++++
 test/models/user/transferable_test.rb         | 40 +++++++++++++++++++
 3 files changed, 60 insertions(+), 2 deletions(-)
 create mode 100644 test/models/user/transferable_test.rb

diff --git a/app/models/user/transferable.rb b/app/models/user/transferable.rb
index 096913a..3a9ace0 100644
--- a/app/models/user/transferable.rb
+++ b/app/models/user/transferable.rb
@@ -3,13 +3,20 @@ module User::Transferable
 
   TRANSFER_LINK_EXPIRY_DURATION = 4.hours
 
+  included do
+    # Bound to the password, so changing it also ends sign-in links made before.
+    generates_token_for :transfer, expires_in: TRANSFER_LINK_EXPIRY_DURATION do
+      password_salt&.last(10)
+    end
+  end
+
   class_methods do
     def find_by_transfer_id(id)
-      find_signed(id, purpose: :transfer)
+      find_by_token_for(:transfer, id)
     end
   end
 
   def transfer_id
-    signed_id(purpose: :transfer, expires_in: TRANSFER_LINK_EXPIRY_DURATION)
+    generate_token_for(:transfer)
   end
 end
diff --git a/test/controllers/sessions/transfers_controller_test.rb b/test/controllers/sessions/transfers_controller_test.rb
index 6efb37f..18322a8 100644
--- a/test/controllers/sessions/transfers_controller_test.rb
+++ b/test/controllers/sessions/transfers_controller_test.rb
@@ -19,4 +19,15 @@ class Sessions::TransfersControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to root_url
     assert parsed_cookies.signed[:session_token]
   end
+
+  test "update rejects a link made before the password changed" do
+    user = users(:david)
+    transfer_id = user.transfer_id
+    user.update!(password: "a new secret123456")
+
+    put session_transfer_url(transfer_id)
+
+    assert_response :bad_request
+    assert_nil parsed_cookies.signed[:session_token]
+  end
 end
diff --git a/test/models/user/transferable_test.rb b/test/models/user/transferable_test.rb
new file mode 100644
index 0000000..00ce337
--- /dev/null
+++ b/test/models/user/transferable_test.rb
@@ -0,0 +1,40 @@
+require "test_helper"
+
+class User::TransferableTest < ActiveSupport::TestCase
+  test "a transfer id finds its user" do
+    assert_equal users(:david), User.find_by_transfer_id(users(:david).transfer_id)
+  end
+
+  test "a transfer id expires" do
+    transfer_id = users(:david).transfer_id
+
+    travel User::TRANSFER_LINK_EXPIRY_DURATION + 1.second
+
+    assert_nil User.find_by_transfer_id(transfer_id)
+  end
+
+  test "a transfer id made before a password change stops working" do
+    transfer_id = users(:david).transfer_id
+
+    users(:david).update!(password: "a new secret123456")
+
+    assert_nil User.find_by_transfer_id(transfer_id)
+    assert_equal users(:david), User.find_by_transfer_id(users(:david).transfer_id)
+  end
+
+  test "other changes keep a transfer id working" do
+    transfer_id = users(:david).transfer_id
+
+    users(:david).update!(name: "Dave", email_address: "dave@37signals.com")
+
+    assert_equal users(:david), User.find_by_transfer_id(transfer_id)
+  end
+
+  test "a transfer id finds only users in scope" do
+    transfer_id = users(:david).transfer_id
+
+    users(:david).deactivate
+
+    assert_nil User.active.find_by_transfer_id(transfer_id)
+  end
+end