diff --git a/backend/app/auth.py b/backend/app/auth.py index 5874b65..a63afa6 100644 --- a/backend/app/auth.py +++ b/backend/app/auth.py @@ -7,13 +7,14 @@ import httpx 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.orm import Session from app import email as email_mod from app import sysconfig from app.config import settings -from app.db import get_db +from app.db import SessionLocal, get_db from app.models import AuthToken, DemoWhitelist, Invite, OAuthToken, User from app.ratelimit import RateLimiter from app.security import decrypt, encrypt, hash_password, hash_token, verify_password @@ -596,11 +597,24 @@ def register( if not _register_limiter.allow(_client_ip(request)): return {"status": "ok"} # silently throttle; uniform response - existing = db.execute(select(User).where(User.email == email)).scalar_one_or_none() - if existing is None: - # Without working SMTP we can't deliver a verification link, so email ownership can't - # gate sign-in. Admin approval stays the real gate (is_active=False); mark the account - # verified so the flow still completes on a no-SMTP self-host. See email.email_enabled(). + # Do the existence check + account creation OFF the response path (a background task with its own + # session). The create path hashes the password + writes several rows + schedules emails; doing it + # inline would make a NEW email respond measurably slower than an already-registered one (which + # skips all that) — a timing oracle for enumeration. Off-path, any valid email responds the same. + background.add_task(_register_account, email, password) + return {"status": "ok"} + + +def _register_account(email: str, password: str) -> None: + """Background worker for /register: create the pending account (+ verification email + admin + notice) for a genuinely new email; no-op for an already-registered one. Runs after the response + with its own DB session, so the request's timing never reveals whether the email already exists.""" + with SessionLocal() as db: + if db.execute(select(User).where(User.email == email)).scalar_one_or_none() is not None: + return # already registered — nothing to do (the uniform "ok" was already returned) + # Without working SMTP we can't deliver a verification link, so email ownership can't gate + # sign-in. Admin approval stays the real gate (is_active=False); mark the account verified so + # the flow still completes on a no-SMTP self-host. See email.email_enabled(). email_ok = email_mod.email_enabled() user = User( email=email, @@ -610,20 +624,13 @@ def register( ) db.add(user) db.flush() - upsert_pending_invite(db, email) # admin-approval gate + upsert_pending_invite(db, email) # admin-approval gate (commits) if email_ok: raw = _issue_token(db, user, "verify", VERIFY_TTL) - # Fragment (#), not a query token: keeps the token out of proxy/access logs + Referer. - # The SPA reads the fragment and POSTs it to /auth/verify (SB3). - background.add_task( - email_mod.send_verify_email, email, f"{_app_base()}/#verify={raw}" - ) + # Fragment (#), not a query token: keeps the token out of proxy/access logs + Referer (SB3). + email_mod.send_verify_email(email, f"{_app_base()}/#verify={raw}") if settings.admin_email_set: - background.add_task( - email_mod.send_admin_new_request, sorted(settings.admin_email_set), email - ) - # Existing email → do nothing visible (no enumeration). Uniform success either way. - return {"status": "ok"} + email_mod.send_admin_new_request(sorted(settings.admin_email_set), email) @router.post("/verify") @@ -692,21 +699,30 @@ def password_reset_request( payload: dict, request: Request, background: BackgroundTasks, - db: Session = Depends(get_db), ) -> dict: - """Request a password-reset link. Uniform response regardless of whether the email has a - password account, so it can't probe for registered emails.""" + """Request a password-reset link. Uniform response + timing regardless of whether the email has a + 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)): return {"status": "ok"} email = (payload.get("email") or "").strip().lower() + # 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 + # password account — no timing-based enumeration. Any valid email schedules the same task. if valid_email(email): + background.add_task(_send_reset_if_eligible, email) + return {"status": "ok"} + + +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).""" + 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: raw = _issue_token(db, user, "reset", RESET_TTL) - # Token in the URL FRAGMENT (#), not the query (?): a fragment is never sent to the - # server, so it can't leak into proxy/access logs or a Referer header (SB3). - background.add_task(email_mod.send_password_reset, email, f"{_app_base()}/#reset={raw}") - return {"status": "ok"} + email_mod.send_password_reset(email, f"{_app_base()}/#reset={raw}") @router.post("/password-reset/confirm") @@ -741,21 +757,22 @@ def password_reset_confirm(payload: dict, db: Session = Depends(get_db)) -> dict ACTIVE_ACCOUNT_HEADER = "x-siftlode-account" -def resolved_user_id(request: Request) -> tuple[int | None, bool]: - """Which account this request acts as, and whether that's the session's default account. +def resolved_user_id(conn: HTTPConnection) -> tuple[int | None, bool]: + """Which account this connection acts as, and whether that's the session's default account. + Takes any HTTPConnection — an HTTP Request OR a WebSocket — so the WS push channel shares this + exact per-tab resolution instead of re-implementing it. The signed cookie holds the *wallet* (`account_ids`, every account signed into this browser) - plus a default `user_id`. A request may override the default with the X-Siftlode-Account - header, but ONLY for an account already in the wallet — so a tab can't impersonate an account - that never authenticated here. Everything else (WebSocket, plain navigations) keeps using the - cookie default. + plus a default `user_id`. A caller may override the default with the X-Siftlode-Account header, + but ONLY for an account already in the wallet — so a tab can't impersonate an account that never + authenticated here. Everything else (WebSocket, plain navigations) keeps using the cookie default. """ - default_id = request.session.get("user_id") - wallet = request.session.get("account_ids") or [] + default_id = conn.session.get("user_id") + wallet = conn.session.get("account_ids") or [] # Header for normal XHR; ?account= for contexts that can't set headers (WebSocket, and a # plain file download). Both are wallet-gated below, so neither can impersonate an # account that never signed into this browser. - hdr = request.headers.get(ACTIVE_ACCOUNT_HEADER) or request.query_params.get("account") + hdr = conn.headers.get(ACTIVE_ACCOUNT_HEADER) or conn.query_params.get("account") if hdr: try: hid = int(hdr) diff --git a/backend/app/routes/messages.py b/backend/app/routes/messages.py index 420bd58..6089550 100644 --- a/backend/app/routes/messages.py +++ b/backend/app/routes/messages.py @@ -20,7 +20,7 @@ from pydantic import BaseModel from sqlalchemy import and_, func, or_, select from sqlalchemy.orm import Session -from app.auth import require_human +from app.auth import require_human, resolved_user_id from app.db import SessionLocal, get_db from app.models import Message, MessageKey, User from app.ratelimit import RateLimiter @@ -363,19 +363,10 @@ async def messages_ws(ws: WebSocket) -> None: """Live push channel: authenticated by the session cookie, registers the connection so sent messages reach this user's open tabs instantly. We don't consume client→server frames (sending goes through POST /api/messages); the receive loop only detects disconnect.""" - # A browser WebSocket can't send custom headers, so the per-tab account (see - # X-Siftlode-Account on HTTP) rides in the ?account= query param instead — honoured only for - # an account already in this browser's wallet. Falls back to the session default. - sess = ws.session if "session" in ws.scope else {} - uid = sess.get("user_id") - q = ws.query_params.get("account") - if q: - try: - qid = int(q) - except ValueError: - qid = None - if qid is not None and qid in (sess.get("account_ids") or []): - uid = qid + # Which signed-in account this socket acts as — the SHARED per-tab resolution (wallet-gated + # ?account= override, session default otherwise). A browser WebSocket can't send the + # X-Siftlode-Account header, so resolved_user_id falls through to the ?account= query param. + uid, _ = resolved_user_id(ws) if not uid: await ws.close(code=1008) return @@ -385,7 +376,7 @@ async def messages_ws(ws: WebSocket) -> None: # SA4: honour server-side session revocation on the live channel too — reject a cookie whose # recorded epoch is behind the account's current one (mirrors current_user). Without this a # copied cookie could keep receiving pushes after a password reset / "log out everywhere". - epochs = sess.get("epochs") or {} + epochs = ws.session.get("epochs") or {} epoch_ok = user is not None and int(epochs.get(str(uid), 0)) == (user.session_epoch or 0) ok = epoch_ok and is_messageable_user(user) finally: