diff --git a/README.md b/README.md index 7d31a90..b8f4489 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,11 @@ Then open `http://:5000` and register — **the first account becomes the - The app waits for the database and runs migrations (`alembic upgrade head`) automatically on start. - **Image tags:** `:latest` (stable, built from `main`) · `:dev` (latest `dev` build) · `:` (immutable, for pinning / rollback). +- **Putting it on the public internet:** there are four things to do first — close + registration, terminate TLS and forward `X-Forwarded-Proto`, stop publishing the app + port, and back up the attachment volume as well as the database. See + [docs/public-hosting.md](docs/public-hosting.md), which also lists what the app + hardens on its own and what it deliberately doesn't. - **Install as an app (PWA):** ThoughtSync is installable ("Add to Home Screen" / the browser's install button) for an app-like window. Browsers only offer install over a **secure context**, so put the app behind a reverse proxy terminating **HTTPS** (or reach diff --git a/docs/public-hosting.md b/docs/public-hosting.md new file mode 100644 index 0000000..dcf6ea8 --- /dev/null +++ b/docs/public-hosting.md @@ -0,0 +1,106 @@ +# Putting ThoughtSync on the public internet + +ThoughtSync is built to run on a LAN and works fine there with no ceremony. Exposing +it changes the threat model: anyone can now reach the login form, and any account is +one guessed password away from someone's whole note history. + +This is what the app does about that on its own, and the four things it cannot do for +you. + +## Do these four things first + +**1. Close registration.** `allow_registration` defaults to **on**, because the first +run of a fresh instance has to be able to create the admin account. It stays on +afterwards. Once your own account exists, turn it off in **Settings → Access → Allow +new registrations**, or the first stranger to find the hostname can open an account on +your server. + +The first account created is always the admin, regardless of this setting — so a +brand-new instance is never locked out of itself. + +**2. Terminate TLS in front of it, and forward the scheme.** The app marks the +session cookie `Secure` and sends HSTS only when it can tell the request arrived over +HTTPS. It looks at `X-Forwarded-Proto`, so the proxy has to set it: + +``` +# Traefik does this automatically. For nginx: +proxy_set_header X-Forwarded-Proto $scheme; +proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; +``` + +Without that header the app assumes plain HTTP and leaves the cookie unmarked — the +conservative choice, since forcing `Secure` on an HTTP install stops the browser from +ever sending the cookie back and silently breaks login. + +Once a browser has seen HSTS from your hostname it will refuse plain HTTP there for a +year, even if the header stops. That is the point of it, but it is worth knowing +before you put a hostname behind TLS temporarily. + +**3. Stop publishing the app port.** The default compose binds `0.0.0.0:5000` so LAN +clients can reach it directly. Behind a proxy that is a second, unprotected front +door. In `.env`: + +``` +THOUGHTSYNC_BIND=127.0.0.1 +``` + +**4. Have a backup that includes the files.** Attachments are files on the +`thoughtsync-data` volume, not rows — a `pg_dump` restores notes whose images are all +gone. Back up both: + +``` +docker compose exec -T db pg_dump -U thoughtsync thoughtsync > notes.sql +docker run --rm -v thoughtsync-data:/d -v "$PWD":/out alpine tar czf /out/media.tgz -C /d . +``` + +## What the app already does + +- **The credential endpoints are throttled.** `/api/auth/login`, `/api/auth/register` + and `/api/auth/device-login` count attempts against both the account and the calling + address, and answer `429` with a `Retry-After` once either is over budget — ten + failed sign-ins per account per fifteen minutes, five registrations per address per + hour. The account-keyed limit is the one that holds when the address is forged. + Checked *before* the password is verified, so a throttled attempt costs no bcrypt: + hashing is deliberately slow, and an unauthenticated caller who can trigger it + without limit has a CPU-exhaustion primitive as well as a guessing one. +- **A failed sign-in takes the same time whether or not the account exists.** No + timing oracle for which emails are registered here. +- **Every response carries a CSP** with `script-src 'self'`, `object-src 'none'` and + `frame-ancestors 'none'`, plus `nosniff`, a referrer policy and a permissions + policy. The app has no inline or third-party scripts, so this costs nothing. +- **Link unfurling is SSRF-hardened.** Every hop is resolved and every resolved + address must be publicly routable before a socket is opened, and the connection is + made to the vetted IP so a rebind between check and connect cannot slip through. A + note containing `http://192.168.1.1/` cannot make your server probe your network. +- **Attachments never render inline unless they are a known raster image.** Anything + else — an SVG, an HTML file — is served `Content-Disposition: attachment`, so a file + on a note shared with you can't run script in your session. +- **Session cookies are `HttpOnly` and `SameSite=Lax`**, which is also what stands in + for CSRF protection: a `Lax` cookie is not sent on a cross-site POST. + +## What it does not do + +Know these before you decide who gets an account. + +- **No email verification and no password reset.** `email_verified` exists on the user + row and nothing sets it. A forgotten password needs a hand on the database. +- **No second factor.** A password is the whole of it. +- **No per-user storage quota.** Any account can upload attachments until the volume + is full. `max_attachment_mb` caps a single file, not a total. +- **No audit log.** Device tokens record `last_used_at`; sign-ins are not recorded. + +None of these are hard blockers for an instance whose accounts are you and people you +know. They are the reason not to hand out open registration to strangers. + +## The Android client + +The app allows plain HTTP so a self-hosted server on a LAN is usable at all — Android +blocks cleartext by default from API 28, and `http://192.168.1.10:8000` is exactly the +case ThoughtSync is built for. Over the public internet, link the phone to the +**HTTPS** hostname. The sync screen shows a warning before any credential field +whenever the address it probed was `http://`; on a public network that warning means +what it says. + +The APK the server hands out is signed with the project release key, and the in-app +updater installs over the existing app only because the signature matches. A build +from anywhere else will not install over it. diff --git a/src/thoughtsync/app.py b/src/thoughtsync/app.py index 0919a90..00bd13f 100644 --- a/src/thoughtsync/app.py +++ b/src/thoughtsync/app.py @@ -31,6 +31,19 @@ STATIC_DIR = os.path.join(os.path.dirname(__file__), "static") mimetypes.add_type("application/manifest+json", ".webmanifest") +def _is_https() -> bool: + """Whether this request reached us over TLS — directly, or through a proxy that + terminated it and said so in X-Forwarded-Proto. + + Shared by the session cookie's Secure flag and by HSTS, because they are the same + question and answering it twice is how the two drift apart. + """ + if not has_request_context(): + return False + forwarded = request.headers.get("X-Forwarded-Proto", "").split(",")[0].strip().lower() + return forwarded == "https" or request.is_secure + + class _AutoSecureSessionInterface(SecureCookieSessionInterface): """Mark the session cookie `Secure` whenever the request arrived over HTTPS — directly, or via a TLS-terminating reverse proxy that sets X-Forwarded-Proto. @@ -42,10 +55,7 @@ class _AutoSecureSessionInterface(SecureCookieSessionInterface): """ def get_cookie_secure(self, app: Quart) -> bool: - if not has_request_context(): - return False - forwarded = request.headers.get("X-Forwarded-Proto", "").split(",")[0].strip().lower() - return forwarded == "https" or request.is_secure + return _is_https() def create_app() -> Quart: @@ -101,6 +111,69 @@ def create_app() -> Quart: with suppress(asyncio.CancelledError): await task + @app.after_request + async def _security_headers(response): + """Headers a publicly-reachable instance should be sending. + + None of these change how the app behaves for a legitimate caller; they narrow + what a browser will do if something else goes wrong. + + The CSP is the substantive one. `script-src 'self'` means that even if some + future path did manage to reflect user text into the page, the browser would + refuse to run it — the app has no inline scripts and no third-party scripts, + so nothing legitimate is given up. The exceptions are honest ones: + - `style-src 'unsafe-inline'` — Vue writes inline styles itself (`v-show` + toggling display, TransitionGroup's FLIP setting transforms). Inline + STYLE is not an execution primitive the way inline script is. + - `img-src https: http:` — link previews render the remote og:image of + whatever was linked, which is an arbitrary host by definition. Both + schemes, because a LAN install is served over http and would otherwise + lose every preview image; on an https instance the browser blocks the + http ones as mixed content anyway, so naming it concedes nothing. + - `blob:`/`data:` — attachment previews and the desktop's blob URI scheme. + + `frame-ancestors 'none'` replaces the older X-Frame-Options and is what stops + the app being framed for clickjacking; `form-action 'self'` stops a form from + being pointed at another origin. + """ + response.headers.setdefault( + "Content-Security-Policy", + "default-src 'self'; " + "base-uri 'self'; " + "object-src 'none'; " + "frame-ancestors 'none'; " + "form-action 'self'; " + "script-src 'self'; " + "style-src 'self' 'unsafe-inline'; " + "img-src 'self' data: blob: https: http:; " + "font-src 'self' data:; " + "media-src 'self' blob:; " + "connect-src 'self'", + ) + # Content-type sniffing turns a file we said was text into whatever the bytes + # look like. The attachment route already sets this; every other response + # deserves it too. + response.headers.setdefault("X-Content-Type-Options", "nosniff") + # Note titles and label names end up in the URL of a search or a label lens, + # and a full Referer would hand them to any site a link preview points at. + response.headers.setdefault("Referrer-Policy", "strict-origin-when-cross-origin") + # Nothing here uses a camera, a microphone or a location, so nothing embedded + # in a page should be able to ask on its behalf. + response.headers.setdefault( + "Permissions-Policy", "camera=(), microphone=(), geolocation=(), interest-cohort=()" + ) + # HSTS only where the request already arrived over TLS — the same detection + # the session cookie uses. Sending it on a plain-HTTP LAN install would tell + # the browser to refuse the only scheme that install serves. + # + # Scoped to this host: no `includeSubDomains` and no `preload`, both of which + # commit domains this app does not own. A browser still remembers the policy + # for up to a year after the header stops being sent, which is the point of + # it — worth knowing before putting a hostname behind TLS temporarily. + if _is_https(): + response.headers.setdefault("Strict-Transport-Security", "max-age=31536000") + return response + @app.get("/api/health") async def health(): return jsonify({"status": "ok", "version": app.config["APP_VERSION"]}) diff --git a/src/thoughtsync/auth.py b/src/thoughtsync/auth.py index 5e9d8f6..7960d40 100644 --- a/src/thoughtsync/auth.py +++ b/src/thoughtsync/auth.py @@ -11,7 +11,13 @@ from .common import iso from .db import session_scope from .models.device_token import DeviceToken from .models.user import User -from .security import generate_token, hash_password, hash_token, verify_password +from .ratelimit import ( + client_address, + register_by_address, + sign_in_by_account, + sign_in_by_address, +) +from .security import dummy_verify, generate_token, hash_password, hash_token, verify_password from .settings import get_setting bp = Blueprint("auth", __name__, url_prefix="/api/auth") @@ -106,6 +112,52 @@ def require_admin(fn): return wrapper +def _throttled(retry_after: int): + """The 429 every throttled credential route returns. + + Deliberately says nothing about WHICH limit was hit or how many attempts are + left — that would tell someone probing whether the email they guessed exists. + `Retry-After` is standard and is the one thing a legitimate client (or person) + genuinely needs. + """ + return ( + jsonify({"error": "too many attempts — try again shortly"}), + 429, + {"Retry-After": str(retry_after)}, + ) + + +def _sign_in_block(email: str) -> int | None: + """Seconds to wait before this sign-in may be attempted, or None to proceed. + + Checked BEFORE the password is verified, so a throttled attempt costs no bcrypt + — which is the other half of what this protects: hashing is deliberately slow, + and an unauthenticated caller who can trigger it without limit has a CPU + exhaustion primitive, not just a guessing one. + """ + address = client_address() + waits = [ + sign_in_by_address.retry_after(address), + sign_in_by_account.retry_after(email) if email else None, + ] + live = [w for w in waits if w is not None] + return max(live) if live else None + + +def _sign_in_failed(email: str) -> None: + address = client_address() + sign_in_by_address.record(address) + if email: + sign_in_by_account.record(email) + + +def _sign_in_succeeded(email: str) -> None: + """Clear the account's history on success. The address keeps its count: one + correct password does not vouch for the other attempts from there.""" + if email: + sign_in_by_account.forget(email) + + @bp.post("/register") async def register(): data = await request.get_json(silent=True) or {} @@ -120,6 +172,14 @@ async def register(): if not display_name: display_name = email.split("@", 1)[0] + # Counted by attempt rather than by failure: a rejected registration still cost a + # round trip and a uniqueness check, and on an instance with signups open the + # thing worth bounding is how fast accounts can appear at all. + wait = register_by_address.retry_after(client_address()) + if wait is not None: + return _throttled(wait) + register_by_address.record(client_address()) + async with session_scope() as db: user_count = await db.scalar(select(func.count()).select_from(User)) or 0 is_first = user_count == 0 @@ -150,10 +210,23 @@ async def login(): email = (data.get("email") or "").strip().lower() password = data.get("password") or "" + wait = _sign_in_block(email) + if wait is not None: + return _throttled(wait) + async with session_scope() as db: user = await db.scalar(select(User).where(User.email == email)) - if user is None or not user.password_hash or not verify_password(password, user.password_hash): + if user is None or not user.password_hash: + # Hash anyway. Without this, "no such account" returns in microseconds + # while a wrong password takes bcrypt's deliberate ~100ms, and the + # difference is a reliable oracle for which emails have accounts here. + dummy_verify(password) + _sign_in_failed(email) return jsonify({"error": "invalid email or password"}), 401 + if not verify_password(password, user.password_hash): + _sign_in_failed(email) + return jsonify({"error": "invalid email or password"}), 401 + _sign_in_succeeded(email) session[SESSION_KEY] = str(user.id) session.permanent = True return jsonify(_serialize_user(user)) @@ -210,10 +283,24 @@ async def device_login(): password = data.get("password") or "" if not email or not password: return jsonify({"error": "email and password are required"}), 400 + + # Same budget as the web sign-in, and the SAME counters — this route hands out a + # long-lived bearer token, so leaving it unthrottled would just move the guessing + # here from /login. + wait = _sign_in_block(email) + if wait is not None: + return _throttled(wait) + async with session_scope() as db: user = await db.scalar(select(User).where(User.email == email)) - if user is None or not user.password_hash or not verify_password(password, user.password_hash): + if user is None or not user.password_hash: + dummy_verify(password) + _sign_in_failed(email) return jsonify({"error": "invalid email or password"}), 401 + if not verify_password(password, user.password_hash): + _sign_in_failed(email) + return jsonify({"error": "invalid email or password"}), 401 + _sign_in_succeeded(email) row, token = await _issue_device_token(db, user.id, data.get("name") or "") await db.commit() return jsonify({"token": token, "device": _serialize_device(row), "user": _serialize_user(user)}), 201 diff --git a/src/thoughtsync/ratelimit.py b/src/thoughtsync/ratelimit.py new file mode 100644 index 0000000..30f59d0 --- /dev/null +++ b/src/thoughtsync/ratelimit.py @@ -0,0 +1,140 @@ +"""Throttling for the endpoints that a public deployment leaves exposed. + +Only the credential endpoints are rate-limited: login, register, and the native +device-link exchange. Everything else already needs a session or a device token to +reach, so an attacker has to get through one of these three first. + +## Why in-process is enough here, and where that stops being true + +State lives in module-level dicts, so it is per-process. That is correct for how +this image actually serves — one hypercorn worker (see the Dockerfile, and the +same assumption the trash sweeper documents in app.py). If that ever gains +``--workers N``, each worker would keep its own counters and the effective limit +would multiply by N; the fix then is a shared store (the Postgres connection is +already there), not a bigger number here. + +## Two keys, on purpose + +Every attempt is counted against BOTH the account being tried and the address it +came from, and either one can refuse it: + +- **The account** is the key that matters, and the key that cannot be forged. It + is what stops credential stuffing against one known email, no matter how many + addresses the attempts arrive from. +- **The address** bounds the damage from one source spraying many accounts. It is + best-effort by nature — behind a reverse proxy the client address is read from + ``X-Forwarded-For``, which a caller can set to anything if the app is exposed + directly. That is precisely why it is not the only key. + +Counting is by failure for the sign-in routes and by attempt for registration: a +correct password should never move someone closer to being locked out, but every +registration is a row in the users table whether it succeeds or not. +""" +from __future__ import annotations + +import time +from collections import deque + +from quart import request + +# Failed sign-ins tolerated per account before it stops answering, and for how long. +# Ten is comfortably above a person mistyping a password and far below anything that +# makes a dictionary worth running. +ACCOUNT_LIMIT = 10 +ACCOUNT_WINDOW_S = 15 * 60 + +# Wider, because one address is legitimately many people: a household, an office +# behind NAT, a phone on carrier-grade NAT. +ADDRESS_LIMIT = 50 +ADDRESS_WINDOW_S = 15 * 60 + +# Registration is scarcer than a sign-in — it creates a row, and on a private +# instance the honest number of accounts anyone needs to make is one. +REGISTER_LIMIT = 5 +REGISTER_WINDOW_S = 60 * 60 + +# Never let the bookkeeping become the denial of service: an attacker rotating a +# forged X-Forwarded-For could otherwise mint an unbounded number of buckets. Well +# above any real deployment's distinct-caller count, so a legitimate instance never +# reaches it; when it is reached the oldest buckets are dropped, which at worst +# forgives some attempts. +MAX_BUCKETS = 10_000 + + +class SlidingWindow: + """Counts events per key over a trailing window. + + Sliding rather than a fixed window because a fixed one lets twice the limit + through across a boundary — 10 at 14:59 and 10 at 15:00 — which for a login + limiter is the difference between the number meaning something and not. + """ + + def __init__(self, limit: int, window_s: float) -> None: + self.limit = limit + self.window_s = window_s + self._hits: dict[str, deque[float]] = {} + + def _prune(self, key: str, now: float) -> deque[float]: + hits = self._hits.get(key) + if hits is None: + hits = deque() + # Insertion-ordered, so the first key is the least recently created. + if len(self._hits) >= MAX_BUCKETS: + self._hits.pop(next(iter(self._hits)), None) + self._hits[key] = hits + cutoff = now - self.window_s + while hits and hits[0] <= cutoff: + hits.popleft() + return hits + + def retry_after(self, key: str, now: float | None = None) -> int | None: + """Seconds until `key` may try again, or None while it is still under the + limit. Read-only — it does not count as an attempt.""" + now = time.monotonic() if now is None else now + hits = self._prune(key, now) + if len(hits) < self.limit: + return None + # The window frees up when its OLDEST hit falls out of it. + return max(1, int(hits[0] + self.window_s - now) + 1) + + def record(self, key: str, now: float | None = None) -> None: + now = time.monotonic() if now is None else now + self._prune(key, now).append(now) + + def forget(self, key: str) -> None: + """Drop a key's history. Used after a successful sign-in, so someone who + fumbled a password twice and then got it right starts clean rather than + carrying those two for the next quarter of an hour.""" + self._hits.pop(key, None) + + def clear(self) -> None: + self._hits.clear() + + +sign_in_by_account = SlidingWindow(ACCOUNT_LIMIT, ACCOUNT_WINDOW_S) +sign_in_by_address = SlidingWindow(ADDRESS_LIMIT, ADDRESS_WINDOW_S) +register_by_address = SlidingWindow(REGISTER_LIMIT, REGISTER_WINDOW_S) + + +def client_address() -> str: + """The caller's address, as well as it can be known. + + ``X-Forwarded-For`` is a list appended to by each hop, so the leftmost entry is + the original client — and also the only entry a client can choose for itself. + It is trusted here anyway, because the alternative behind a reverse proxy is to + see the proxy's address for every request on earth and rate-limit the entire + internet as one caller. The account-keyed limit is the one that holds when this + one is lied to. + """ + forwarded = request.headers.get("X-Forwarded-For", "") + if forwarded: + first = forwarded.split(",")[0].strip() + if first: + return first[:64] # bounded: this becomes a dict key + return (request.remote_addr or "unknown")[:64] + + +def reset_all() -> None: + """Drop every counter. For tests — nothing in the app calls this.""" + for window in (sign_in_by_account, sign_in_by_address, register_by_address): + window.clear() diff --git a/src/thoughtsync/security.py b/src/thoughtsync/security.py index c57ca70..fcec317 100644 --- a/src/thoughtsync/security.py +++ b/src/thoughtsync/security.py @@ -14,6 +14,23 @@ def hash_password(password: str) -> str: return bcrypt.hashpw(password.encode("utf-8")[:_MAX_BCRYPT_BYTES], bcrypt.gensalt()).decode("utf-8") +# A real bcrypt hash of a value nobody can present, used only to spend the time a +# verification would have. Computed once at import — generating it per call would +# cost a gensalt+hash on top of the checkpw and make the "no such user" path SLOWER +# than the real one, which is the same oracle pointing the other way. +_DUMMY_HASH = bcrypt.hashpw(secrets.token_bytes(32), bcrypt.gensalt()) + + +def dummy_verify(password: str) -> None: + """Burn one password verification against a throwaway hash. + + For the sign-in path when the email has no account: it makes "no such user" cost + what "wrong password" costs, so response time stops answering the question of + which emails are registered here. + """ + bcrypt.checkpw(password.encode("utf-8")[:_MAX_BCRYPT_BYTES], _DUMMY_HASH) + + def verify_password(password: str, password_hash: str) -> bool: try: return bcrypt.checkpw(password.encode("utf-8")[:_MAX_BCRYPT_BYTES], password_hash.encode("utf-8")) diff --git a/tests/test_ratelimit.py b/tests/test_ratelimit.py new file mode 100644 index 0000000..874a749 --- /dev/null +++ b/tests/test_ratelimit.py @@ -0,0 +1,139 @@ +"""The credential throttle. DB-free, like the rest of this suite. + +The window itself is exercised directly with an injected clock, so nothing here +sleeps: a 15-minute window tested in real time is a test nobody runs twice. + +The routes are exercised only as far as they get WITHOUT a database — a throttled +request returns 429 before any session is opened, which is the whole point of +checking the limit before the password. The happy path can't be reached here and is +not pretended at. +""" +import pytest + +from thoughtsync import ratelimit +from thoughtsync.app import create_app +from thoughtsync.ratelimit import SlidingWindow + + +@pytest.fixture(autouse=True) +def _clean_counters(): + ratelimit.reset_all() + yield + ratelimit.reset_all() + + +@pytest.fixture +def app(): + return create_app() + + +def test_under_the_limit_is_not_blocked(): + w = SlidingWindow(limit=3, window_s=60) + for i in range(3): + assert w.retry_after("k", now=i) is None + w.record("k", now=i) + assert w.retry_after("k", now=3) is not None + + +def test_window_slides_rather_than_resetting(): + w = SlidingWindow(limit=2, window_s=60) + w.record("k", now=0) + w.record("k", now=30) + assert w.retry_after("k", now=31) is not None + # The 0s hit falls out at t=60, which frees exactly one slot — the 30s hit is + # still inside the window, so this is a slide and not a reset. + assert w.retry_after("k", now=61) is None + w.record("k", now=61) + assert w.retry_after("k", now=62) is not None + + +def test_retry_after_points_past_the_oldest_hit(): + w = SlidingWindow(limit=1, window_s=100) + w.record("k", now=10) + wait = w.retry_after("k", now=40) + # The hit at t=10 leaves the window at t=110, i.e. 70s away. Rounded up, never + # under-reported — a client that waits exactly this long must not be refused + # again. + assert wait is not None + assert 70 <= wait <= 72 + assert w.retry_after("k", now=40 + wait) is None + + +def test_keys_are_counted_separately(): + w = SlidingWindow(limit=1, window_s=60) + w.record("a", now=0) + assert w.retry_after("a", now=1) is not None + assert w.retry_after("b", now=1) is None + + +def test_forget_clears_one_key(): + w = SlidingWindow(limit=1, window_s=60) + w.record("a", now=0) + w.record("b", now=0) + w.forget("a") + assert w.retry_after("a", now=1) is None + assert w.retry_after("b", now=1) is not None + + +def test_bucket_count_is_bounded(monkeypatch): + # An attacker rotating a forged X-Forwarded-For must not be able to grow this + # dict without limit — the limiter cannot become the exhaustion it prevents. + monkeypatch.setattr(ratelimit, "MAX_BUCKETS", 8) + w = SlidingWindow(limit=5, window_s=60) + for i in range(50): + w.record(f"addr-{i}", now=i) + assert len(w._hits) <= 8 + + +async def test_login_starts_refusing(app): + client = app.test_client() + body = {"email": "someone@example.com", "password": "wrong-password"} + # Pre-load the account's counter to its limit rather than posting that many + # times: every real attempt would need a database to reach the password check. + for i in range(ratelimit.ACCOUNT_LIMIT): + ratelimit.sign_in_by_account.record("someone@example.com", now=float(i)) + resp = await client.post("/api/auth/login", json=body) + assert resp.status_code == 429 + assert resp.headers.get("Retry-After") + # The refusal says nothing about whether that account exists. + assert "someone@example.com" not in (await resp.get_data(as_text=True)) + + +async def test_device_login_shares_the_account_counter(app): + client = app.test_client() + for i in range(ratelimit.ACCOUNT_LIMIT): + ratelimit.sign_in_by_account.record("someone@example.com", now=float(i)) + resp = await client.post( + "/api/auth/device-login", + json={"email": "someone@example.com", "password": "wrong-password"}, + ) + # Same budget as /login — otherwise guessing just moves to the route that hands + # out a long-lived bearer token. + assert resp.status_code == 429 + + +async def test_register_is_throttled_by_address(app): + client = app.test_client() + for i in range(ratelimit.REGISTER_LIMIT): + ratelimit.register_by_address.record("203.0.113.9", now=float(i)) + resp = await client.post( + "/api/auth/register", + json={"email": "new@example.com", "password": "a-long-enough-password"}, + headers={"X-Forwarded-For": "203.0.113.9"}, + ) + assert resp.status_code == 429 + + +async def test_client_address_prefers_the_forwarded_client(app): + # Behind a reverse proxy, remote_addr is the PROXY for every request on earth — + # keying on it would rate-limit the entire internet as one caller. The leftmost + # X-Forwarded-For entry is the original client. + async with app.test_request_context("/", headers={"X-Forwarded-For": "198.51.100.4, 10.0.0.1"}): + assert ratelimit.client_address() == "198.51.100.4" + + +async def test_client_address_falls_back_to_the_peer(app): + async with app.test_request_context("/"): + # No proxy header: whatever the peer address is, it must be a usable key + # rather than an empty string sharing one bucket with everyone. + assert ratelimit.client_address() diff --git a/tests/test_security_headers.py b/tests/test_security_headers.py new file mode 100644 index 0000000..9009673 --- /dev/null +++ b/tests/test_security_headers.py @@ -0,0 +1,57 @@ +"""Headers every response carries once this is reachable from the internet. + +Asserted on /api/health because it is the one route that needs no database and no +session — the headers are set in an after_request hook, so any response proves the +hook, and this suite has no Postgres. +""" +import pytest + +from thoughtsync.app import create_app + + +@pytest.fixture +def app(): + return create_app() + + +async def test_content_security_policy_locks_scripts_to_self(app): + resp = await app.test_client().get("/api/health") + csp = resp.headers["Content-Security-Policy"] + assert "script-src 'self'" in csp + # The two that matter most if anything ever reflects user text into the page. + assert "object-src 'none'" in csp + assert "frame-ancestors 'none'" in csp + # No blanket unsafe-inline for SCRIPT — style is the only place it's conceded. + assert "script-src 'self' 'unsafe-inline'" not in csp + + +async def test_link_preview_images_are_still_allowed(app): + resp = await app.test_client().get("/api/health") + csp = resp.headers["Content-Security-Policy"] + # A preview renders the og:image of an arbitrary host; both schemes, because a + # LAN install is served over http. + assert "img-src" in csp + assert "https:" in csp + assert "http:" in csp + + +async def test_sniffing_and_referrer_are_pinned(app): + resp = await app.test_client().get("/api/health") + assert resp.headers["X-Content-Type-Options"] == "nosniff" + assert resp.headers["Referrer-Policy"] == "strict-origin-when-cross-origin" + assert "camera=()" in resp.headers["Permissions-Policy"] + + +async def test_no_hsts_on_plain_http(app): + # A plain-HTTP LAN install must not be told to refuse the only scheme it serves. + resp = await app.test_client().get("/api/health") + assert "Strict-Transport-Security" not in resp.headers + + +async def test_hsts_when_a_proxy_terminated_tls(app): + resp = await app.test_client().get("/api/health", headers={"X-Forwarded-Proto": "https"}) + hsts = resp.headers["Strict-Transport-Security"] + assert "max-age=" in hsts + # Scoped to this host: neither of these commits domains the app doesn't own. + assert "includeSubDomains" not in hsts + assert "preload" not in hsts