feat(auth): R4 S1 — fix auth dead ends
- password reset also sets email_verified (the mail proves the mailbox), rescuing an account that never clicked the verify link; is_active (admin approval) is deliberately left untouched - add POST /auth/verify/resend (rate-limited, off-response-path, anti-enum) + a "resend" mode/CTA on the verifyInvalid banner, so an expired/lost verification link is no longer a dead end (re-register was a silent no-op) - OAuth cancel/deny now redirects to /?login=oauth_cancelled with a friendly banner instead of a raw 400 JSON page; pop the oauth_link markers BEFORE the token exchange so an aborted link flow can't poison the next clean sign-in
This commit is contained in:
+62
-10
@@ -31,6 +31,7 @@ _DECOY_PASSWORD_HASH = hash_password(secrets.token_urlsafe(16))
|
||||
_register_limiter = RateLimiter(max_events=5, window_seconds=300)
|
||||
_login_limiter = RateLimiter(max_events=10, window_seconds=300)
|
||||
_reset_limiter = RateLimiter(max_events=5, window_seconds=300)
|
||||
_verify_resend_limiter = RateLimiter(max_events=5, window_seconds=300)
|
||||
_request_access_limiter = RateLimiter(max_events=5, window_seconds=300)
|
||||
# At most one "your account is suspended" email per address per hour, so a suspended user who
|
||||
# keeps retrying (or a script with their valid credentials) can't be used to mailbomb them.
|
||||
@@ -274,10 +275,21 @@ async def callback(
|
||||
client = google_oauth(db)
|
||||
if client is None:
|
||||
return RedirectResponse(url="/")
|
||||
# Linking mode: an already-authenticated account is attaching this Google identity (a
|
||||
# password account enabling YouTube or connecting SSO). The markers are set by /auth/link and
|
||||
# /auth/upgrade; when present we attach to the current user instead of identifying by sub.
|
||||
# Pop them BEFORE the token exchange: if the user aborts Google's consent screen the exchange
|
||||
# throws below, and a marker left behind would silently turn the NEXT clean sign-in into a link
|
||||
# attempt. Popping up front means an aborted link flow leaves no residue in the session.
|
||||
link_uid = request.session.pop("oauth_link_uid", None)
|
||||
link_explicit = request.session.pop("oauth_link_explicit", False)
|
||||
try:
|
||||
token = await client.authorize_access_token(request)
|
||||
except OAuthError as exc:
|
||||
raise HTTPException(status_code=400, detail=f"OAuth error: {exc.error}")
|
||||
# The user cancelled or denied consent at Google — land back on the login page with a
|
||||
# friendly banner instead of a raw 400 JSON error page (applies to sign-in and link flows).
|
||||
log.info("Google OAuth cancelled/failed: %s", exc.error)
|
||||
return RedirectResponse(url="/?login=oauth_cancelled")
|
||||
|
||||
userinfo = token.get("userinfo")
|
||||
if not userinfo or not userinfo.get("sub"):
|
||||
@@ -286,12 +298,8 @@ async def callback(
|
||||
sub = userinfo["sub"]
|
||||
email = (userinfo.get("email") or "").lower()
|
||||
|
||||
# Linking mode: an already-authenticated account is attaching this Google identity (a
|
||||
# password account enabling YouTube or connecting SSO). The marker is set by /auth/link and
|
||||
# /auth/upgrade; when present we attach to the current user instead of identifying by sub.
|
||||
link_uid = request.session.pop("oauth_link_uid", None)
|
||||
if link_uid is not None:
|
||||
return _complete_link(request, db, link_uid, sub, userinfo, token)
|
||||
return _complete_link(request, db, link_uid, link_explicit, sub, userinfo, token)
|
||||
|
||||
if not is_allowed(db, email):
|
||||
log.warning("Login denied (not approved): %s", email or "<no email>")
|
||||
@@ -456,13 +464,19 @@ def purge_user(db: Session, user: User, background: BackgroundTasks) -> None:
|
||||
|
||||
|
||||
def _complete_link(
|
||||
request: Request, db: Session, link_uid: int, sub: str, userinfo: dict, token: dict
|
||||
request: Request,
|
||||
db: Session,
|
||||
link_uid: int,
|
||||
explicit: bool,
|
||||
sub: str,
|
||||
userinfo: dict,
|
||||
token: dict,
|
||||
):
|
||||
"""Attach the just-authorized Google identity to the already-signed-in account that started
|
||||
the link/upgrade. No is_allowed gate — they're already an active user. Refuses to hijack a
|
||||
Google identity owned by another account, or to silently swap a different linked identity."""
|
||||
# Consume the companion marker up front so it never lingers across the early returns below.
|
||||
explicit = request.session.pop("oauth_link_explicit", False)
|
||||
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"):
|
||||
return RedirectResponse(url="/?link=error")
|
||||
@@ -693,6 +707,39 @@ def verify_email_confirm(payload: dict, db: Session = Depends(get_db)) -> dict:
|
||||
return {"ok": True}
|
||||
|
||||
|
||||
@router.post("/verify/resend")
|
||||
def verify_email_resend(
|
||||
payload: dict,
|
||||
request: Request,
|
||||
background: BackgroundTasks,
|
||||
) -> dict:
|
||||
"""Re-issue an email-verification link. Needed because the original link expires (VERIFY_TTL)
|
||||
and re-registering an already-registered email is a silent no-op, so a user who lost the first
|
||||
mail had no way back. Uniform response + timing regardless of whether the email has a pending
|
||||
account (the lookup runs off the response path), so it can't probe for registered emails."""
|
||||
if not _verify_resend_limiter.allow(_client_ip(request)):
|
||||
return {"status": "ok"} # silently throttle; uniform response
|
||||
email = (payload.get("email") or "").strip().lower()
|
||||
if valid_email(email):
|
||||
background.add_task(_resend_verify_if_eligible, email)
|
||||
return {"status": "ok"}
|
||||
|
||||
|
||||
def _resend_verify_if_eligible(email: str) -> None:
|
||||
"""Background worker for verify/resend: issue a fresh verify token + email the link, but ONLY for
|
||||
a real account that still needs verifying. Runs after the response with its own DB session, so the
|
||||
request's timing never reveals whether the account exists. No-op when SMTP is disabled (accounts
|
||||
are auto-verified on a no-SMTP instance, so there is nothing to resend). Token rides the URL
|
||||
FRAGMENT (#) so it can't leak into proxy/access logs or a Referer (SB3)."""
|
||||
if not email_mod.email_enabled():
|
||||
return
|
||||
with SessionLocal() as db:
|
||||
user = db.execute(select(User).where(User.email == email)).scalar_one_or_none()
|
||||
if user is not None and not user.email_verified and not user.is_demo:
|
||||
raw = _issue_token(db, user, "verify", VERIFY_TTL)
|
||||
email_mod.send_verify_email(email, f"{_app_base()}/#verify={raw}")
|
||||
|
||||
|
||||
@router.get("/verify")
|
||||
def verify_email(token: str, db: Session = Depends(get_db)):
|
||||
"""Legacy query-token verification link (pre-SB3 emails still in flight). New emails use the
|
||||
@@ -787,6 +834,11 @@ def password_reset_confirm(payload: dict, db: Session = Depends(get_db)) -> dict
|
||||
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)
|
||||
# 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
|
||||
# here: a reset proves email ownership, not that an admin approved the account.
|
||||
user.email_verified = True
|
||||
bump_session_epoch(user) # SA4: a reset (often a compromise response) kills all existing sessions.
|
||||
# Burn any other outstanding reset tokens for this user.
|
||||
for row in db.execute(
|
||||
|
||||
Reference in New Issue
Block a user