fix(auth): stop the address-bar mirror eating emailed auth tokens, and unlock revoked accounts

Two independent bugs made an email+password registration unrecoverable in prod.

The verification link did nothing. urlWithParams/clearParams rebuilt the URL as
pathname+query and dropped the fragment — but the one-time auth tokens ride the
fragment (#verify=/#reset=, SB3), and the providers that mirror nav params
(useUrlMirror in FeedFilters/Playlists/Plex/Channels) sit ABOVE the lazily-loaded
<App> in main.tsx. Their mount effect replaceState'd the token away before the auth
card's chunk had loaded, so the card read an empty hash: the link opened a bare
sign-in page, no request, no banner. Both helpers now carry the fragment over.

The account was then locked out for good. SEC-10 nulls password_hash when an
unverified account is attested (here: admin approval landing before the user could
verify — which the bug above made impossible). That is indistinguishable from a
Google-only account, and reset eligibility keyed off password_hash alone, so no
reset link was ever issued: no password to sign in with, and no way to get one.
users.password_revoked_at records the revocation, reset eligibility honours it, and
setting a password again (reset-confirm or set-password) clears it. An account that
never had a password stays ineligible on purpose — its owner adds one from Settings.

/auth/verify now also answers password_reset_required so the card can say why the
registered password stopped working, instead of leaving the user at the uniform
"Invalid email or password". Telling the holder of a valid verify token leaks
nothing: they have already proved they own the mailbox.

The 0078 migration backfills the stamp for rows carrying the exact lockout signature
(local, non-demo, no password, unverified), so existing victims recover unaided.
This commit is contained in:
2026-09-02 01:28:45 +02:00
parent 94c62ca699
commit 3ac3d382e7
11 changed files with 282 additions and 14 deletions
@@ -0,0 +1,50 @@
"""users.password_revoked_at — remember that SEC-10 dropped a preset password
Revision ID: 0078_password_revoked_at
Revises: 0077_plex_playback_class
Create Date: 2026-09-02
SEC-10 (`auth.revoke_preset_password_if_unverified`) nulls `password_hash` when an account whose email
was never verified is attested (Google adoption, or admin approval). That left NO trace, and
password-reset eligibility keyed off `password_hash IS NOT NULL` — so such an account could neither
sign in (no password) nor request a reset link (not "eligible"): a permanent lockout with a silent,
uniform response at every step.
This column is that trace. A row with `password_revoked_at` set is a local account that HAD a password
and had it revoked, so the reset flow treats it as eligible again. Setting a password (reset-confirm or
/auth/set-password) clears the stamp. A Google-only account that never had a password keeps it NULL and
stays reset-ineligible by design — its owner adds a password from Settings while signed in.
Backfill: the pre-existing victims of the lockout can't be identified with certainty, but the signature
is exact and narrow — a local (google_sub IS NULL), non-demo account with no password whose email is
still unverified could ONLY have got there through the revoke, since registration always sets a hash.
Stamp those with the current time so they become recoverable without operator intervention.
"""
from typing import Sequence, Union
import sqlalchemy as sa
from alembic import op
revision: str = "0078_password_revoked_at"
down_revision: Union[str, None] = "0077_plex_playback_class"
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None
def upgrade() -> None:
op.add_column(
"users",
sa.Column("password_revoked_at", sa.DateTime(timezone=True), nullable=True),
)
op.execute(
"UPDATE users SET password_revoked_at = now() "
"WHERE password_hash IS NULL "
"AND google_sub IS NULL "
"AND is_demo = false "
"AND email_verified = false"
)
def downgrade() -> None:
op.drop_column("users", "password_revoked_at")
+6
View File
@@ -47,6 +47,12 @@ class User(Base, TimestampMixin):
email: Mapped[str] = mapped_column(String(320), unique=True, index=True)
# argon2id hash for email+password login; NULL for Google-only / demo accounts.
password_hash: Mapped[str | None] = mapped_column(String(255))
# When SEC-10 dropped a preset password (see auth.revoke_preset_password_if_unverified). Without
# this trace a revoked account was indistinguishable from a Google-only one, so it failed the
# password-reset eligibility check and could never be recovered — it could neither sign in nor
# reset. Set = "had a password, lost it to the guard" -> reset-eligible. Cleared whenever a
# password is set again (reset-confirm / set-password).
password_revoked_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True))
# Email ownership proven (Google sign-in is implicitly verified; password registration
# verifies via a link). Login requires this AND is_active.
email_verified: Mapped[bool] = mapped_column(Boolean, default=False, server_default="false")
+31 -6
View File
@@ -662,15 +662,22 @@ def _register_account(email: str, password: str) -> None:
@router.post("/verify")
def verify_email_confirm(body: TokenIn, db: Session = Depends(get_db)) -> dict:
"""Confirm an email-verification token the SPA read from the URL fragment (SB3). Uniform 400
on any invalid/expired/used token."""
on any invalid/expired/used token.
`password_reset_required` tells the SPA to send the user straight to the reset flow: an admin who
approved this account BEFORE it was verified tripped SEC-10, so the password chosen at registration
is gone and sign-in would answer with the deliberately uniform "Invalid email or password" — true,
but unreadable to the person who just proved they own the mailbox. Saying so leaks nothing: holding
a valid single-use verify token for this account already proves that much."""
user = _consume_token(db, body.token, "verify")
if user is None:
raise HTTPException(
status_code=400, detail="This verification link is invalid or has expired."
)
user.email_verified = True
needs_password = user.password_hash is None and user.password_revoked_at is not None
db.commit()
return {"ok": True}
return {"ok": True, "password_reset_required": needs_password}
@router.post("/verify/resend")
@@ -714,8 +721,10 @@ def verify_email(token: str, db: Session = Depends(get_db)):
if user is None:
return RedirectResponse(url="/?verify=invalid")
user.email_verified = True
needs_password = user.password_hash is None and user.password_revoked_at is not None
db.commit()
return RedirectResponse(url="/?verify=ok")
# Same two outcomes as the POST flow above, so an old link in flight lands on the same guidance.
return RedirectResponse(url="/?verify=" + ("ok_reset" if needs_password else "ok"))
@router.post("/password-login")
@@ -780,10 +789,18 @@ def _send_reset_if_eligible(email: str) -> None:
"""Background worker for password_reset_request: issue a reset token + email the link, but ONLY
for a real (non-demo) password account. Runs after the response with its own DB session, so the
request's timing never reveals whether the account exists. Token rides the URL FRAGMENT (#) so it
can't leak into proxy/access logs or a Referer (SB3)."""
can't leak into proxy/access logs or a Referer (SB3).
"Password account" is `password_hash OR password_revoked_at` — an account SEC-10 stripped had a
password and must be able to get a new one, or it is locked out permanently (no password to sign in
with, and formerly no reset link either). A Google-only account that never had a password stays
ineligible on purpose: its owner adds one from Settings (/auth/set-password) while signed in, which
proves identity through the session rather than by mail."""
with SessionLocal() as db:
user = db.execute(select(User).where(User.email == email)).scalar_one_or_none()
if user is not None and user.password_hash and not user.is_demo:
if user is None or user.is_demo:
return
if user.password_hash or user.password_revoked_at:
raw = _issue_token(db, user, "reset", RESET_TTL)
email_mod.send_password_reset(email, f"{_app_base()}/#reset={raw}")
@@ -802,6 +819,7 @@ def password_reset_confirm(body: TokenPasswordIn, db: Session = Depends(get_db))
if user is None:
raise HTTPException(status_code=400, detail="This reset link is invalid or has expired.")
user.password_hash = hash_password(new_password)
user.password_revoked_at = None # the account has a password again — SEC-10's trace is spent
# A valid reset link proves control of the mailbox, so completing a reset also verifies the
# email — this rescues an account that registered but never clicked the verification link (it
# can then sign in). The separate admin-approval gate (is_active) is deliberately NOT touched
@@ -844,9 +862,15 @@ def revoke_preset_password_if_unverified(user: User) -> None:
ownership is finally attested (Google adoption, or admin approval), drop that password and bump the
epoch to revoke any session it minted; the owner re-sets one via password-reset. No-op for an
already-verified account, so a legit no-SMTP registration is unaffected. The single source for this
rule — both the Google-callback and admin-activation paths call it so they can't drift."""
rule — both the Google-callback and admin-activation paths call it so they can't drift.
`password_revoked_at` is what makes "the owner re-sets one via password-reset" actually true. The
revoke leaves `password_hash IS NULL`, which is also what a Google-only account looks like — and
_send_reset_if_eligible keyed off exactly that, so a revoked account silently got no reset link and
was locked out for good (it could not sign in either). The stamp separates the two cases."""
if not user.email_verified:
user.password_hash = None
user.password_revoked_at = datetime.now(timezone.utc)
bump_session_epoch(user)
@@ -916,6 +940,7 @@ def set_password(
if not verify_password(body.current_password or "", user.password_hash):
raise HTTPException(status_code=403, detail="Current password is incorrect.")
user.password_hash = hash_password(new_password)
user.password_revoked_at = None # same as the reset path: a password exists again
# SA4: a password change logs out every OTHER session; re-stamp this cookie at the new epoch so
# the person making the change stays signed in here. Commit BEFORE re-stamping so a failed commit
# can't leave the cookie at a newer epoch than the DB (which would wrongly 401 THIS session).
+131
View File
@@ -341,3 +341,134 @@ def test_register_route_is_refused_when_registration_is_closed(client, db, monke
db.execute(_select(User).where(User.email == "closed@example.com")).scalar_one_or_none()
is None
)
# --- SEC-10 lockout: a revoked preset password must stay RECOVERABLE ---------------------------
#
# Regression pack for the prod incident of 2026-09-02. An account registered with a password, then
# approved by an admin BEFORE its email was verified, ended up with password_hash = NULL. That is
# also what a Google-only account looks like, and reset eligibility keyed off `password_hash` alone —
# so the account could neither sign in (no password) nor obtain a reset link, and every response on
# the way was a deliberately uniform "ok"/401. `password_revoked_at` is the trace that tells the two
# apart; these pin each step of the way back in.
def _capture_reset_links(monkeypatch) -> list[str]:
"""Patch the reset mailer and collect the recipients it is asked to write to."""
from app.routes import auth as auth_mod
sent: list[str] = []
monkeypatch.setattr(auth_mod.email_mod, "send_password_reset", lambda to, link: sent.append(to))
return sent
def test_the_approval_revoke_stamps_password_revoked_at(db, make_user):
from app.routes.admin import _activate_user_by_email
victim = make_user(
email="stamped@example.com",
password="attacker-chosen",
email_verified=False,
is_active=False,
)
assert victim.password_revoked_at is None
_activate_user_by_email(db, "stamped@example.com")
assert victim.password_hash is None
assert victim.password_revoked_at is not None # …so the reset flow can tell it HAD a password
def test_an_account_whose_password_was_revoked_can_still_get_a_reset_link(
db, make_user, monkeypatch
):
# THE lockout bug: without the stamp this sent nothing, and the account was unrecoverable.
from app.routes.admin import _activate_user_by_email
from app.routes.auth import _send_reset_if_eligible
make_user(
email="lockedout@example.com",
password="chosen at registration",
email_verified=False,
is_active=False,
)
_activate_user_by_email(db, "lockedout@example.com")
sent = _capture_reset_links(monkeypatch)
_send_reset_if_eligible("lockedout@example.com")
assert sent == ["lockedout@example.com"]
def test_a_google_only_account_is_still_not_reset_eligible(db, make_user, monkeypatch):
# The other side of the rule: an account that NEVER had a password gets no reset link — its owner
# adds one from Settings while signed in (/auth/set-password), which proves identity by session.
from app.routes.auth import _send_reset_if_eligible
make_user(email="googler@example.com", password=None, google_sub="sub-123")
sent = _capture_reset_links(monkeypatch)
_send_reset_if_eligible("googler@example.com")
assert sent == []
def test_completing_a_reset_clears_the_revocation_stamp(client, db, make_user):
from app.routes.admin import _activate_user_by_email
from app.routes.auth import _issue_token
user = make_user(
email="recovered@example.com",
password="chosen at registration",
email_verified=False,
is_active=False,
)
_activate_user_by_email(db, "recovered@example.com")
raw = _issue_token(db, user, "reset", timedelta(hours=1))
db.commit()
ok = client.post(
"/auth/password-reset/confirm", json={"token": raw, "password": "a brand new one"}
)
assert ok.status_code == 200
db.expire_all()
assert user.password_hash is not None
assert user.password_revoked_at is None # spent — not a permanently reset-eligible account
assert _login(client, "recovered@example.com", "a brand new one").status_code == 200
def test_verify_tells_a_revoked_account_that_it_needs_a_new_password(client, db, make_user):
# The banner's input. Saying so leaks nothing: the caller just proved mailbox control by holding a
# valid single-use verify token. Without it the user meets only the uniform "Invalid email or
# password" and has no way to learn why the password they chose stopped working.
from app.routes.admin import _activate_user_by_email
from app.routes.auth import _issue_token
user = make_user(
email="needspw@example.com",
password="chosen at registration",
email_verified=False,
is_active=False,
)
_activate_user_by_email(db, "needspw@example.com")
raw = _issue_token(db, user, "verify", timedelta(hours=1))
db.commit()
r = client.post("/auth/verify", json={"token": raw})
assert r.status_code == 200
assert r.json() == {"ok": True, "password_reset_required": True}
def test_verify_reports_no_reset_needed_for_an_intact_account(client, db, make_user):
from app.routes.auth import _issue_token
user = make_user(email="intact@example.com", email_verified=False, is_active=False)
raw = _issue_token(db, user, "verify", timedelta(hours=1))
db.commit()
r = client.post("/auth/verify", json={"token": raw})
assert r.status_code == 200
assert r.json() == {"ok": True, "password_reset_required": False}