fix(auth): close the account pre-emption + wrong-account link holes (E1 S5, SEC-10/11)

SEC-10 (account pre-emption / "pre-hijack"): a password set on an account whose
email was never verified is untrusted — whoever registered the address (to
pre-empt the real owner) could have chosen it. Both the Google-adoption callback
and admin activation (_activate_user_by_email) now drop that pre-set password and
bump the session epoch when the account isn't email_verified, so a later Google
login / admin approval can't hand the pre-set password a working login. No-op for
a verified account (the legit no-SMTP flow). +2 tests.

SEC-11 (link/upgrade bound to the session default): the OAuth link/upgrade GETs
resolved to the session DEFAULT account, so a tab acting as a secondary account
would link Google / the YouTube grant to the wrong account. The frontend now
threads the active account through the navigation URL (withActiveAccount ->
?account=<active>), and _complete_link accepts any wallet account, not only the
default, so the identity lands on the account that started the flow.
This commit is contained in:
2026-08-18 03:02:34 +02:00
parent 8ffa65a9da
commit 18edbecb1e
6 changed files with 68 additions and 6 deletions
+8
View File
@@ -19,6 +19,7 @@ from app.auth import (
)
from app.db import get_db
from app.demo import reset_demo_state
from app.routes.auth import bump_session_epoch
from app.models import (
DemoWhitelist,
Invite,
@@ -62,6 +63,13 @@ def _activate_user_by_email(db: Session, email: str) -> None:
it can sign in. No-op if no such user exists yet (a Google user is created at first login)."""
user = db.execute(select(User).where(User.email == email.lower())).scalar_one_or_none()
if user is not None and not user.is_active:
# SEC-10: same guard as the Google-adoption path — approving an account whose email was never
# verified must not activate a password set on it by whoever registered the address (a
# pre-emption). Drop the untrusted password + revoke any session it minted; the owner re-sets
# one via password-reset. (No-op when email_verified is True, e.g. legit no-SMTP registrations.)
if not user.email_verified:
user.password_hash = None
bump_session_epoch(user)
user.is_active = True
db.commit()
+14 -2
View File
@@ -286,6 +286,14 @@ async def callback(
return RedirectResponse(url="/?access=denied")
user = existing
user.google_sub = sub
# SEC-10: a password set on this account BEFORE its email was ever verified is untrusted —
# whoever registered the address (to pre-empt the real owner, who is only now arriving via
# Google) could have chosen it. Drop it now that Google attests ownership, and bump the epoch
# to revoke any session that pre-set password may have minted. The owner can set a fresh
# password via password-reset later if they also want email+password sign-in.
if not user.email_verified:
user.password_hash = None
bump_session_epoch(user)
# Google verified the email and is_allowed granted access, so finish activating a
# previously pending password registration (otherwise current_user would bounce it).
user.email_verified = True
@@ -392,8 +400,12 @@ def _complete_link(
Google identity owned by another account, or to silently swap a different linked identity.
`explicit` (the popped oauth_link_explicit marker) distinguishes a "Connect Google" click from
a YouTube upgrade — the caller pops it up front so an aborted flow leaves nothing behind."""
# The marker must match the live session user (guards a stale/forged marker).
if link_uid != request.session.get("user_id"):
# SEC-11: the marker must name an account signed into THIS browser — the session default OR any
# account in the wallet — not only the default. A tab acting as a secondary account links Google to
# ITSELF (the marker was set from current_user, which resolves the ?account= it navigated with), so
# accepting wallet membership stops the identity/token landing on the default account A instead of B.
accounts = request.session.get("account_ids") or []
if link_uid != request.session.get("user_id") and link_uid not in accounts:
return RedirectResponse(url="/?link=error")
user = db.get(User, link_uid)
if user is None or user.is_demo:
+29
View File
@@ -90,3 +90,32 @@ def test_reset_confirm_is_single_use_and_kills_existing_sessions(client, make_us
# The token is single-use: a replay is a clean 400, not a second reset.
replay = client.post("/auth/password-reset/confirm", json={"token": raw, "password": "another pw!!"})
assert replay.status_code == 400
def test_activating_an_unverified_account_drops_its_preset_password(db, make_user):
# SEC-10 (account pre-emption): an attacker registers victim@x with a password before the approved
# owner arrives. The address was never verified, so admin approval must NOT hand that pre-set
# password a working login — it is dropped and any session it minted is revoked.
from app.routes.admin import _activate_user_by_email
victim = make_user(
email="victim@example.com", password="attacker-chosen", email_verified=False, is_active=False
)
epoch0 = victim.session_epoch or 0
_activate_user_by_email(db, "victim@example.com")
assert victim.is_active is True
assert victim.password_hash is None
assert (victim.session_epoch or 0) == epoch0 + 1
def test_activating_a_verified_account_keeps_its_password(db, make_user):
# A legit account whose email IS verified (e.g. a no-SMTP registration) keeps its password on
# approval — the guard must not break the normal flow.
from app.routes.admin import _activate_user_by_email
user = make_user(
email="legit@example.com", password="my own password", email_verified=True, is_active=False
)
_activate_user_by_email(db, "legit@example.com")
assert user.is_active is True
assert user.password_hash is not None