test(auth): pin what rounds 1 and 2 left unpinned (E9.5 review round 3)
Three low findings, all in what the review rounds themselves wrote. The comment on `test_approving_an_unverified_account_keeps_its_password` still asserted the argument rounds 1 and 2 refuted — that dropping the approval-time revoke was safe because `password_login` reads `email_verified` first — while the docstrings on the two functions it describes now say the opposite. Two contradictory "why" records for one rule is how the round-1 takeover primitive got reasoned into existence in the first place, so the test comment now carries the corrected account and names SEC-16. `test_a_later_registration_leaves_a_link_already_in_flight_alive` asserted a subset relation that is vacuously true on an empty set, and the guard that made it non-empty had gone out with the revert. It now pins both halves: the old link survives, and exactly one new one appears. Round 2's own fix — activation moved to after a successful send — was unpinned; putting it back above the send left the suite green. The failed-invitation test now also asserts the account was not activated by a request the UI reported as a failure.
This commit is contained in:
@@ -99,10 +99,16 @@ def test_reset_confirm_is_single_use_and_kills_existing_sessions(client, make_us
|
||||
|
||||
def test_approving_an_unverified_account_keeps_its_password(db, make_user):
|
||||
# E9.5 S1, inverting an earlier rule. Approval used to drop a password set while the email was
|
||||
# unverified (SEC-10, account pre-emption). That was redundant: password_login refuses an
|
||||
# unverified account BEFORE it reads is_active, so approval hands a pre-emptor's password nothing
|
||||
# — see the pair of tests below. All it did was destroy the password a legitimate registrant had
|
||||
# just chosen, in what is the NORMAL ordering (the admin decides before the user reads their mail).
|
||||
# unverified (SEC-10, account pre-emption). It was dropped because it destroyed the password a
|
||||
# legitimate registrant had just chosen, in what is the NORMAL ordering — the admin decides
|
||||
# before the user reads their mail — making "your password was cleared for security" the default
|
||||
# experience.
|
||||
#
|
||||
# S1 ALSO argued the revoke was redundant, because password_login refuses an unverified account
|
||||
# before it reads is_active. The epic's closing review refuted that: the gate holds only while
|
||||
# the address is unverified, and S6's approval mail exists to get the real owner to end that
|
||||
# state. Do not restore that reasoning from this comment — the residual is SEC-16, and round 2
|
||||
# of the review tried and reverted a guard for it (see test_review_round.py).
|
||||
from app.routes.admin import _activate_user_by_email
|
||||
|
||||
user = make_user(
|
||||
@@ -122,8 +128,10 @@ def test_approving_an_unverified_account_keeps_its_password(db, make_user):
|
||||
|
||||
|
||||
def test_an_approved_but_unverified_account_still_cannot_sign_in(client, make_user):
|
||||
# Why dropping the approval-path revoke is safe: this is the gate that actually stops a
|
||||
# pre-emptor, and it runs whatever the admin decided.
|
||||
# The gate that stops a pre-emptor BEFORE the address is confirmed, whatever the admin decided.
|
||||
# It is not the whole story — confirming the address opens it, which is SEC-16 — but it is the
|
||||
# invariant this test owns, and a change that let an unverified account sign in would be a
|
||||
# straightforwardly worse bug than the residual.
|
||||
make_user(
|
||||
email="notyet@example.com",
|
||||
password="attacker-chosen",
|
||||
|
||||
Reference in New Issue
Block a user