diff --git a/backend/app/auth.py b/backend/app/auth.py index cdd8490..cb5b83e 100644 --- a/backend/app/auth.py +++ b/backend/app/auth.py @@ -8,7 +8,7 @@ from authlib.integrations.starlette_client import OAuth, OAuthError from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Request from fastapi.responses import JSONResponse, RedirectResponse from starlette.requests import HTTPConnection -from sqlalchemy import delete, func, select +from sqlalchemy import delete, func, select, update from sqlalchemy.orm import Session from app import email as email_mod @@ -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) +_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. _suspend_email_limiter = RateLimiter(max_events=1, window_seconds=3600) @@ -512,16 +513,26 @@ async def logout(request: Request): @router.post("/request-access") async def request_access( payload: dict, + request: Request, background: BackgroundTasks, db: Session = Depends(get_db), ) -> dict: """Public: ask for access. Idempotent — repeat requests for the same email don't - duplicate or re-spam admins (upsert returns None for an already-pending row).""" + duplicate or re-spam admins (upsert returns None for an already-pending row). + + Answers a uniform ``"pending"`` for EVERY outcome — already-allowed, newly-pending, + already-pending, and rate-limited alike. The old ``"approved"`` reply for an allow-listed + address made this a public enumeration oracle for the allow-list (which the rest of the module + 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 + endpoint can't be probed for throttling either.""" email = (payload.get("email") or "").strip().lower() if not valid_email(email): raise HTTPException(status_code=400, detail="Enter a valid email address.") + if not _request_access_limiter.allow(_client_ip(request)): + return {"status": "pending"} if is_allowed(db, email): - return {"status": "approved"} # already allowed — just sign in + return {"status": "pending"} # already allowed — don't reveal it inv = upsert_pending_invite(db, email) admins = admin_notify_emails(db) if inv is not None and admins: @@ -583,18 +594,27 @@ def _issue_token(db: Session, user: User, kind: str, ttl: timedelta) -> str: def _consume_token(db: Session, raw: str | None, kind: str) -> User | None: - """Validate + burn a token. Returns its user, or None if missing/expired/used/wrong kind.""" + """Validate + burn a token atomically. Returns its user, or None if missing/expired/used/wrong + kind. The burn is a single conditional UPDATE (…WHERE used_at IS NULL…RETURNING) rather than a + read-check-write, so two concurrent requests can't both consume the same verify/reset token — + only the one whose UPDATE flips used_at gets a row back.""" if not raw: return None + now = datetime.now(timezone.utc) row = db.execute( - select(AuthToken).where( - AuthToken.token_hash == hash_token(raw), AuthToken.kind == kind + update(AuthToken) + .where( + AuthToken.token_hash == hash_token(raw), + AuthToken.kind == kind, + AuthToken.used_at.is_(None), + AuthToken.expires_at >= now, ) - ).scalar_one_or_none() - if row is None or row.used_at is not None or row.expires_at < datetime.now(timezone.utc): - return None - row.used_at = datetime.now(timezone.utc) + .values(used_at=now) + .returning(AuthToken.user_id) + ).first() db.commit() + if row is None: + return None return db.get(User, row.user_id)