refactor(auth): typed Pydantic request bodies for the 9 payload:dict handlers (E3 S3, B-core-6)
request-access / demo / register / verify / verify-resend / password-login / password-reset(request|confirm) / set-password each took an untyped `payload: dict` and hand-picked fields with `.get(...)`. Replace with five small BaseModels (EmailIn, EmailPasswordIn, TokenIn, TokenPasswordIn, SetPasswordIn) so FastAPI validates the body shape and publishes the schema. Fields are `str | None` to preserve exact behaviour — a missing/null field still coerces via the existing `(... or "")`, no stricter 422s. Green under the auth-session suite (7), ruff clean.
This commit is contained in:
+53
-22
@@ -10,6 +10,7 @@ from datetime import datetime, timedelta, timezone
|
|||||||
from authlib.integrations.starlette_client import OAuth, OAuthError
|
from authlib.integrations.starlette_client import OAuth, OAuthError
|
||||||
from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Request
|
from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Request
|
||||||
from fastapi.responses import JSONResponse, RedirectResponse
|
from fastapi.responses import JSONResponse, RedirectResponse
|
||||||
|
from pydantic import BaseModel
|
||||||
from sqlalchemy import select, update
|
from sqlalchemy import select, update
|
||||||
from sqlalchemy.exc import IntegrityError
|
from sqlalchemy.exc import IntegrityError
|
||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
@@ -187,6 +188,36 @@ def _oauth_redirect_uri(request: Request) -> str:
|
|||||||
return settings.oauth_redirect_url
|
return settings.oauth_redirect_url
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
# --- Request bodies (typed contracts; B-core-6) ---------------------------------------------
|
||||||
|
# Fields are `str | None` (not required) so the handlers keep their exact prior behaviour — a
|
||||||
|
# missing/null field reads as None and the existing `(... or "")` coercions still apply — while
|
||||||
|
# FastAPI now validates the body is a JSON object and publishes the schema.
|
||||||
|
|
||||||
|
|
||||||
|
class EmailIn(BaseModel):
|
||||||
|
email: str | None = None
|
||||||
|
|
||||||
|
|
||||||
|
class EmailPasswordIn(BaseModel):
|
||||||
|
email: str | None = None
|
||||||
|
password: str | None = None
|
||||||
|
|
||||||
|
|
||||||
|
class TokenIn(BaseModel):
|
||||||
|
token: str | None = None
|
||||||
|
|
||||||
|
|
||||||
|
class TokenPasswordIn(BaseModel):
|
||||||
|
token: str | None = None
|
||||||
|
password: str | None = None
|
||||||
|
|
||||||
|
|
||||||
|
class SetPasswordIn(BaseModel):
|
||||||
|
password: str | None = None
|
||||||
|
current_password: str | None = None
|
||||||
|
|
||||||
|
|
||||||
@router.get("/login")
|
@router.get("/login")
|
||||||
async def login(request: Request, db: Session = Depends(get_db)):
|
async def login(request: Request, db: Session = Depends(get_db)):
|
||||||
# access_type=offline ensures a refresh_token on first authorization. We avoid
|
# access_type=offline ensures a refresh_token on first authorization. We avoid
|
||||||
@@ -448,7 +479,7 @@ async def logout(request: Request):
|
|||||||
|
|
||||||
@router.post("/request-access")
|
@router.post("/request-access")
|
||||||
async def request_access(
|
async def request_access(
|
||||||
payload: dict,
|
body: EmailIn,
|
||||||
request: Request,
|
request: Request,
|
||||||
background: BackgroundTasks,
|
background: BackgroundTasks,
|
||||||
db: Session = Depends(get_db),
|
db: Session = Depends(get_db),
|
||||||
@@ -462,7 +493,7 @@ async def request_access(
|
|||||||
is careful to avoid, cf. demo_login); an already-allowed user can still just sign in with Google.
|
is careful to avoid, cf. demo_login); an already-allowed user can still just sign in with Google.
|
||||||
Rate-limited per client IP like register/login/reset, and a block returns "pending" too so the
|
Rate-limited per client IP like register/login/reset, and a block returns "pending" too so the
|
||||||
endpoint can't be probed for throttling either."""
|
endpoint can't be probed for throttling either."""
|
||||||
email = (payload.get("email") or "").strip().lower()
|
email = (body.email or "").strip().lower()
|
||||||
if not valid_email(email):
|
if not valid_email(email):
|
||||||
raise HTTPException(status_code=400, detail="Enter a valid email address.")
|
raise HTTPException(status_code=400, detail="Enter a valid email address.")
|
||||||
if not _request_access_limiter.allow(_client_ip(request)):
|
if not _request_access_limiter.allow(_client_ip(request)):
|
||||||
@@ -477,7 +508,7 @@ async def request_access(
|
|||||||
|
|
||||||
|
|
||||||
@router.post("/demo")
|
@router.post("/demo")
|
||||||
def demo_login(payload: dict, request: Request, db: Session = Depends(get_db)) -> dict:
|
def demo_login(body: EmailIn, request: Request, db: Session = Depends(get_db)) -> dict:
|
||||||
"""Hidden demo entry: a whitelisted email logs straight into the shared demo account,
|
"""Hidden demo entry: a whitelisted email logs straight into the shared demo account,
|
||||||
no Google OAuth. The login page fires this quietly (debounced) as the user types/pastes,
|
no Google OAuth. The login page fires this quietly (debounced) as the user types/pastes,
|
||||||
so there is no visible 'demo login' button. Rate-limited per IP; when blocked it answers
|
so there is no visible 'demo login' button. Rate-limited per IP; when blocked it answers
|
||||||
@@ -485,7 +516,7 @@ def demo_login(payload: dict, request: Request, db: Session = Depends(get_db)) -
|
|||||||
enumeration oracle. On a match it sets the session and the client reloads into the app."""
|
enumeration oracle. On a match it sets the session and the client reloads into the app."""
|
||||||
if not _demo_limiter.allow(_client_ip(request)):
|
if not _demo_limiter.allow(_client_ip(request)):
|
||||||
return {"authenticated": False}
|
return {"authenticated": False}
|
||||||
email = (payload.get("email") or "").strip().lower()
|
email = (body.email or "").strip().lower()
|
||||||
if valid_email(email) and is_demo_allowed(db, email):
|
if valid_email(email) and is_demo_allowed(db, email):
|
||||||
demo = get_or_create_demo_user(db)
|
demo = get_or_create_demo_user(db)
|
||||||
# Isolate the demo: drop any real-account wallet on this tab first, so the demo session
|
# Isolate the demo: drop any real-account wallet on this tab first, so the demo session
|
||||||
@@ -556,7 +587,7 @@ def _consume_token(db: Session, raw: str | None, kind: str) -> User | None:
|
|||||||
|
|
||||||
@router.post("/register")
|
@router.post("/register")
|
||||||
def register(
|
def register(
|
||||||
payload: dict,
|
body: EmailPasswordIn,
|
||||||
request: Request,
|
request: Request,
|
||||||
background: BackgroundTasks,
|
background: BackgroundTasks,
|
||||||
db: Session = Depends(get_db),
|
db: Session = Depends(get_db),
|
||||||
@@ -565,8 +596,8 @@ def register(
|
|||||||
verification link, and records an access request for admin approval. Two gates before
|
verification link, and records an access request for admin approval. Two gates before
|
||||||
sign-in: verified email AND admin approval. Responses are uniform once input is valid, so
|
sign-in: verified email AND admin approval. Responses are uniform once input is valid, so
|
||||||
the endpoint can't be used to discover which emails are registered."""
|
the endpoint can't be used to discover which emails are registered."""
|
||||||
email = (payload.get("email") or "").strip().lower()
|
email = (body.email or "").strip().lower()
|
||||||
password = payload.get("password") or ""
|
password = body.password or ""
|
||||||
# Input validation is safe to reveal (independent of whether the email exists).
|
# Input validation is safe to reveal (independent of whether the email exists).
|
||||||
if not valid_email(email):
|
if not valid_email(email):
|
||||||
raise HTTPException(status_code=400, detail="Enter a valid email address.")
|
raise HTTPException(status_code=400, detail="Enter a valid email address.")
|
||||||
@@ -628,10 +659,10 @@ def _register_account(email: str, password: str) -> None:
|
|||||||
|
|
||||||
|
|
||||||
@router.post("/verify")
|
@router.post("/verify")
|
||||||
def verify_email_confirm(payload: dict, db: Session = Depends(get_db)) -> dict:
|
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
|
"""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."""
|
||||||
user = _consume_token(db, payload.get("token"), "verify")
|
user = _consume_token(db, body.token, "verify")
|
||||||
if user is None:
|
if user is None:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=400, detail="This verification link is invalid or has expired."
|
status_code=400, detail="This verification link is invalid or has expired."
|
||||||
@@ -643,7 +674,7 @@ def verify_email_confirm(payload: dict, db: Session = Depends(get_db)) -> dict:
|
|||||||
|
|
||||||
@router.post("/verify/resend")
|
@router.post("/verify/resend")
|
||||||
def verify_email_resend(
|
def verify_email_resend(
|
||||||
payload: dict,
|
body: EmailIn,
|
||||||
request: Request,
|
request: Request,
|
||||||
background: BackgroundTasks,
|
background: BackgroundTasks,
|
||||||
) -> dict:
|
) -> dict:
|
||||||
@@ -653,7 +684,7 @@ def verify_email_resend(
|
|||||||
account (the lookup runs off the response path), so it can't probe for registered emails."""
|
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)):
|
if not _verify_resend_limiter.allow(_client_ip(request)):
|
||||||
return {"status": "ok"} # silently throttle; uniform response
|
return {"status": "ok"} # silently throttle; uniform response
|
||||||
email = (payload.get("email") or "").strip().lower()
|
email = (body.email or "").strip().lower()
|
||||||
if valid_email(email):
|
if valid_email(email):
|
||||||
background.add_task(_resend_verify_if_eligible, email)
|
background.add_task(_resend_verify_if_eligible, email)
|
||||||
return {"status": "ok"}
|
return {"status": "ok"}
|
||||||
@@ -688,7 +719,7 @@ def verify_email(token: str, db: Session = Depends(get_db)):
|
|||||||
|
|
||||||
@router.post("/password-login")
|
@router.post("/password-login")
|
||||||
def password_login(
|
def password_login(
|
||||||
payload: dict,
|
body: EmailPasswordIn,
|
||||||
request: Request,
|
request: Request,
|
||||||
background: BackgroundTasks,
|
background: BackgroundTasks,
|
||||||
db: Session = Depends(get_db),
|
db: Session = Depends(get_db),
|
||||||
@@ -700,8 +731,8 @@ def password_login(
|
|||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=429, detail="Too many attempts. Try again shortly."
|
status_code=429, detail="Too many attempts. Try again shortly."
|
||||||
)
|
)
|
||||||
email = (payload.get("email") or "").strip().lower()
|
email = (body.email or "").strip().lower()
|
||||||
password = payload.get("password") or ""
|
password = body.password or ""
|
||||||
user = db.execute(select(User).where(User.email == email)).scalar_one_or_none()
|
user = db.execute(select(User).where(User.email == email)).scalar_one_or_none()
|
||||||
# Always run the KDF (against a decoy hash for an unknown / passwordless / demo account) so the
|
# Always run the KDF (against a decoy hash for an unknown / passwordless / demo account) so the
|
||||||
# response time is the same whether or not the email exists — no timing-based enumeration.
|
# response time is the same whether or not the email exists — no timing-based enumeration.
|
||||||
@@ -731,7 +762,7 @@ def password_login(
|
|||||||
|
|
||||||
@router.post("/password-reset/request")
|
@router.post("/password-reset/request")
|
||||||
def password_reset_request(
|
def password_reset_request(
|
||||||
payload: dict,
|
body: EmailIn,
|
||||||
request: Request,
|
request: Request,
|
||||||
background: BackgroundTasks,
|
background: BackgroundTasks,
|
||||||
) -> dict:
|
) -> dict:
|
||||||
@@ -739,7 +770,7 @@ def password_reset_request(
|
|||||||
password account (the lookup runs off the response path), so it can't probe for registered emails."""
|
password account (the lookup runs off the response path), so it can't probe for registered emails."""
|
||||||
if not _reset_limiter.allow(_client_ip(request)):
|
if not _reset_limiter.allow(_client_ip(request)):
|
||||||
return {"status": "ok"}
|
return {"status": "ok"}
|
||||||
email = (payload.get("email") or "").strip().lower()
|
email = (body.email or "").strip().lower()
|
||||||
# Do the account lookup + token issue + email OFF the response path (a background task with its
|
# Do the account lookup + token issue + email OFF the response path (a background task with its
|
||||||
# own session), so the endpoint takes the same time for any valid email whether or not it has a
|
# own session), so the endpoint takes the same time for any valid email whether or not it has a
|
||||||
# password account — no timing-based enumeration. Any valid email schedules the same task.
|
# password account — no timing-based enumeration. Any valid email schedules the same task.
|
||||||
@@ -761,10 +792,10 @@ def _send_reset_if_eligible(email: str) -> None:
|
|||||||
|
|
||||||
|
|
||||||
@router.post("/password-reset/confirm")
|
@router.post("/password-reset/confirm")
|
||||||
def password_reset_confirm(payload: dict, db: Session = Depends(get_db)) -> dict:
|
def password_reset_confirm(body: TokenPasswordIn, db: Session = Depends(get_db)) -> dict:
|
||||||
"""Set a new password from a valid reset token, then invalidate any other reset tokens."""
|
"""Set a new password from a valid reset token, then invalidate any other reset tokens."""
|
||||||
token = payload.get("token")
|
token = body.token
|
||||||
new_password = payload.get("password") or ""
|
new_password = body.password or ""
|
||||||
if len(new_password) < PASSWORD_MIN_LEN:
|
if len(new_password) < PASSWORD_MIN_LEN:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=400,
|
status_code=400,
|
||||||
@@ -872,7 +903,7 @@ async def link_google(
|
|||||||
|
|
||||||
@router.post("/set-password")
|
@router.post("/set-password")
|
||||||
def set_password(
|
def set_password(
|
||||||
payload: dict,
|
body: SetPasswordIn,
|
||||||
request: Request,
|
request: Request,
|
||||||
user: User = Depends(current_user),
|
user: User = Depends(current_user),
|
||||||
db: Session = Depends(get_db),
|
db: Session = Depends(get_db),
|
||||||
@@ -884,7 +915,7 @@ def set_password(
|
|||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=403, detail="Not available in the demo account."
|
status_code=403, detail="Not available in the demo account."
|
||||||
)
|
)
|
||||||
new_password = payload.get("password") or ""
|
new_password = body.password or ""
|
||||||
if len(new_password) < PASSWORD_MIN_LEN:
|
if len(new_password) < PASSWORD_MIN_LEN:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=400,
|
status_code=400,
|
||||||
@@ -892,7 +923,7 @@ def set_password(
|
|||||||
)
|
)
|
||||||
if user.password_hash:
|
if user.password_hash:
|
||||||
if not verify_password(
|
if not verify_password(
|
||||||
payload.get("current_password") or "", user.password_hash
|
body.current_password or "", user.password_hash
|
||||||
):
|
):
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=403, detail="Current password is incorrect."
|
status_code=403, detail="Current password is incorrect."
|
||||||
|
|||||||
Reference in New Issue
Block a user