diff --git a/server/app.py b/server/app.py index 1a381fe..c37bfbd 100644 --- a/server/app.py +++ b/server/app.py @@ -9,6 +9,7 @@ Run (prod): gunicorn -k uvicorn.workers.UvicornWorker -b 127.0.0.1:8000 serve Interactive docs: http:///api/docs """ import base64 +import logging import os import re import uuid @@ -26,7 +27,11 @@ from sqlalchemy import select, delete, func from sqlalchemy.orm import Session from .db import Base, engine, get_db -from . import models, auth, notify, assets_db +from . import models, auth, notify, ldap_auth, assets_db + +# Same naming as the other modules' loggers (wpsuite.auth / .ldap / .notify), so a +# deployment can raise the level on one subsystem without raising it on all of them. +log = logging.getLogger("wpsuite.api") # Schema management: # • Local dev (SQLite) auto-creates tables for a zero-config run. @@ -674,38 +679,139 @@ class AutoAddIn(BaseModel): role: str = "" -LOGIN_MAX_ATTEMPTS = int(os.getenv("AUTH_MAX_ATTEMPTS", "5")) +# D13 / T10.2 — THIS NUMBER IS NOT A UX PREFERENCE, IT IS A SAFETY LIMIT. +# +# Failures are now LDAP binds, so every one counts against the DOMAIN account +# lockout policy. This estate's AD threshold is 5. The throttle below is +# per-process and the API runs 2 gunicorn workers, so a local limit of N lets up +# to 2N binds reach a domain controller: 2 x 2 = 4, one under the threshold. +# +# It defaulted to 5 before this task, which would have allowed up to 10 binds and +# locked the account out of WINDOWS — twice over — before the local lockout ever +# engaged. If you raise this, or add a worker, redo the arithmetic first. +LOGIN_MAX_ATTEMPTS = int(os.getenv("AUTH_MAX_ATTEMPTS", "2")) LOGIN_LOCKOUT_MINUTES = int(os.getenv("AUTH_LOCKOUT_MINUTES", "15")) +# Pre-account throttle, keyed by USERNAME (not by client IP — an attacker rotating +# IPs would sail past an IP-keyed limit, and it is the domain account we are +# protecting, not this endpoint's capacity). +# +# The DB counter on `users` is shared across workers and persists, but it can only +# work for an account that already has a local row. Under D13 accounts are created +# on first successful login, so a REAL domain account can be hammered before any +# row exists — this dict covers exactly that window. Per-worker and lost on +# restart, which is why LOGIN_MAX_ATTEMPTS is halved rather than trusted. +_bind_attempts: dict[str, list[float]] = {} +_BIND_WINDOW_SECONDS = LOGIN_LOCKOUT_MINUTES * 60 + + +def _bind_throttled(sam: str) -> bool: + """True if this username has already used its bind budget in the window. + Checked BEFORE any call to the directory.""" + now = monotonic() + key = (sam or "").strip().lower() + hits = [t for t in _bind_attempts.get(key, []) if now - t < _BIND_WINDOW_SECONDS] + _bind_attempts[key] = hits + if len(_bind_attempts) > 5000: # bound the dict on a long-lived worker + for k in [k for k, v in _bind_attempts.items() + if not v or now - max(v) > _BIND_WINDOW_SECONDS]: + _bind_attempts.pop(k, None) + return len(hits) >= LOGIN_MAX_ATTEMPTS + + +def _record_bind_failure(sam: str) -> None: + _bind_attempts.setdefault((sam or "").strip().lower(), []).append(monotonic()) + + +def _clear_bind_failures(sam: str) -> None: + _bind_attempts.pop((sam or "").strip().lower(), None) + @app.post("/api/auth/login") def login(body: LoginIn, request: Request, response: Response, db: Session = Depends(get_db)): - """Verify credentials and, on success, set the HttpOnly session cookie. - Throttles online password guessing: after LOGIN_MAX_ATTEMPTS consecutive - failures an account is locked for LOGIN_LOCKOUT_MINUTES.""" - user = auth.find_user(db, body.username) + """Authenticate against the domain (D13 / T10.2) and set the session cookie. + + The suite stores no passwords: this is an LDAPS simple bind as + `@prime.local`, and a successful bind IS the authentication. + See server/ldap_auth.py for the transport and its two hard guards. + + ORDER MATTERS HERE. Every throttle check runs BEFORE the directory is touched, + because a failed bind counts against the DOMAIN lockout policy — so this + endpoint must not be usable to lock a colleague out of Windows. Nothing below + reaches `ldap_auth.verify` until the local budget has been checked twice: once + against the persistent per-account counter, once against the pre-account + window that covers usernames with no local row yet. + + Three outcomes, deliberately distinguished: + 401 the credential was rejected, or the account is not in the required + group. One generic message for every case — the response must never + reveal whether an account exists (see `_ERR49` in ldap_auth: the useful + detail goes to the log). + 403 the local account exists and is disabled. Independent of the directory. + 503 OUR fault — LDAP unconfigured, unreachable, untrusted, or the required + group does not resolve. D13 left no password fallback, so this must not + masquerade as 401: "your password is wrong" sends people hunting for a + password they no longer have, while the real problem is a broken deploy. + """ now = models.utcnow() - # Always run the hash comparison first — even for missing or locked accounts — - # so response timing doesn't leak which usernames exist. verify_password - # tolerates an empty hash. - valid = auth.verify_password(body.password, user.password_hash if user else "") + sam = ldap_auth.normalize_username(body.username) + if not sam or not (body.password or "").strip(): + # No directory call for empty input. ldap_auth.verify guards this too; the + # duplication is intentional, since an empty password would otherwise be an + # anonymous bind and anonymous binds SUCCEED. + raise HTTPException(status_code=401, detail="Invalid username or password") + + user = auth.find_user(db, sam) + + # ── throttle 1: the persistent per-account lockout ──────────────────────── locked = user.locked_until if user else None if locked is not None and locked.tzinfo is None: - locked = locked.replace(tzinfo=timezone.utc) # SQLite returns naive datetimes; normalize to UTC + locked = locked.replace(tzinfo=timezone.utc) # SQLite returns naive datetimes if locked is not None and locked > now: raise HTTPException(status_code=429, detail="Too many failed attempts. Try again later.") - if not user or not valid: + + # ── throttle 2: the pre-account window ──────────────────────────────────── + if _bind_throttled(sam): + log.warning("refusing to bind for %r — local attempt budget (%d) spent; " + "protecting the domain account from lockout", sam, LOGIN_MAX_ATTEMPTS) + raise HTTPException(status_code=429, detail="Too many failed attempts. Try again later.") + + # ── the directory ───────────────────────────────────────────────────────── + settings = notify.get_settings(db) + result = ldap_auth.verify(sam, body.password, + required_group=settings.get("ldap_required_group")) + + if result.is_config_problem: + # Ours, not theirs. Never counted against the user, never a 401. + log.error("sign-in unavailable — %s: %s", result.reason, result.detail) + raise HTTPException(status_code=503, + detail="Sign-in is temporarily unavailable. Contact IT.") + + if not result.ok: + log.info("sign-in refused for %r (%s: %s)", sam, result.reason, result.detail) + _record_bind_failure(sam) if user: user.failed_attempts = (user.failed_attempts or 0) + 1 if user.failed_attempts >= LOGIN_MAX_ATTEMPTS: user.locked_until = now + timedelta(minutes=LOGIN_LOCKOUT_MINUTES) user.failed_attempts = 0 - log_event(db, user.username, "login_locked", "user", user.id, summary=user.username, - detail={"minutes": LOGIN_LOCKOUT_MINUTES}) + log_event(db, user.username, "login_locked", "user", user.id, + summary=user.username, + detail={"minutes": LOGIN_LOCKOUT_MINUTES, "reason": result.reason}) db.commit() raise HTTPException(status_code=401, detail="Invalid username or password") + + # ── authenticated ───────────────────────────────────────────────────────── + _clear_bind_failures(sam) + if not user: + # T10.4 provisions the account here. Until that lands, an authenticated + # person with no local row is refused rather than silently admitted. + log.warning("%r authenticated against the domain but has no local account " + "(JIT provisioning arrives in T10.4)", result.sam) + raise HTTPException(status_code=403, detail="No account on this system yet.") if not user.is_active: raise HTTPException(status_code=403, detail="Account is disabled") + user.failed_attempts = 0 user.locked_until = None user.last_login_at = now