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

@@ -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

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

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