mirror of
https://github.com/basecamp/once-campfire.git
synced 2026-10-29 00:44:05 +09:00
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.
From #371.
(cherry picked from commit 160ba295b5)
This commit is contained in:
1 parent
00abff4088
commit
58a25c6281
3 files changed
+60
-2
No files matched your search
@@ -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
|
||||
@@ -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
|
||||
Reference in new issue
Block a user