Skip to content

Commit f12a6c3

Browse files
Improve the Confirmable race condition test
Co-authored-by: Rune Philosof <57357936+runephilosof-abtion@users.noreply.github.com>
1 parent 6fc84bd commit f12a6c3

1 file changed

Lines changed: 9 additions & 5 deletions

File tree

test/integration/confirmable_test.rb

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -355,22 +355,26 @@ def visit_admin_confirmation_with_token(confirmation_token)
355355
assert admin.reload.pending_reconfirmation?
356356
end
357357

358-
test 'concurrent "update email" requests should not allow confirmation_token and unconfirmed_email to get out of sync' do
358+
test 'concurrent "update email" requests should not allow confirming a victim email address' do
359359
attacker_email = "attacker@example.com"
360360
victim_email = "victim@example.com"
361361

362362
attacker = create_admin
363363
# update the email address of the attacker, but do not confirm it yet
364-
attacker.update(email: attacker_email)
364+
attacker.update!(email: attacker_email)
365+
366+
# A new request starts, to update the unconfirmed email again.
367+
attacker = Admin.find_by(id: attacker.id)
365368

366-
# a concurrent request also updates the email address to the victim, while this request's model is in memory
369+
# A concurrent request also updates the email address to the victim, while the `attacker` request's model is in memory
367370
Admin.where(id: attacker.id).update_all(
368371
unconfirmed_email: victim_email,
369372
confirmation_token: "different token"
370373
)
371374

372-
# now we update to the same prior unconfirmed email address, and confirm
373-
attacker.update(email: attacker_email)
375+
# Now the attacker updates to the same prior unconfirmed email address, and confirm.
376+
# This should update the `unconfirmed_email` in the database, even though it is unchanged from the models point of view.
377+
attacker.update!(email: attacker_email)
374378
attacker_token = attacker.raw_confirmation_token
375379
visit_admin_confirmation_with_token(attacker_token)
376380

0 commit comments

Comments
 (0)