diff --git a/.env.example b/.env.example index 4a0a07b..72a8e46 100644 --- a/.env.example +++ b/.env.example @@ -24,6 +24,10 @@ OIDC_CLIENT_ID= OIDC_CLIENT_SECRET= OIDC_SCOPES=openid profile email groups REQUIRED_GROUP=dropbox +# Admit tokens with no groups claim at all. Off by default — a missing claim +# means the provider's scope mapping has drifted, not that the user has no +# groups. Turn on only while repairing that mapping. +ALLOW_MISSING_GROUPS_CLAIM=false # ── Upload limits / abuse controls ────────────────────────────────────────── MAX_UPLOAD_BYTES=5368709120 diff --git a/CHANGELOG.md b/CHANGELOG.md index 6124075..51628ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,22 +5,16 @@ pre-1.0 scheme; dates are when the change reached `main`. ## Unreleased -### Security -- The client address behind the anonymous rate limit and the upload audit trail - is no longer take-your-word-for-it. Forwarded headers are honoured only from - peers listed in the new `TRUSTED_PROXIES` setting, and `X-Forwarded-For` is - read right to left so hops appended by the client are discarded. Previously - any caller that could open a socket to the app could name itself, which reset - the rate-limit budget and wrote a false address into the audit trail; - `TRUST_FORWARDED_FOR=false` did not prevent it, because the server was - rewriting the peer address before the application saw it. That setting is - replaced by `TRUSTED_PROXIES`. - ### Added - Full reference documentation under `docs/`: architecture, configuration, operations runbook, security model, and API reference, with a docs index and a `CONTRIBUTING` guide. +### Changed +- Template rendering goes through a small `render()`/`error_page()` helper, + which also moves the app onto Starlette's current `TemplateResponse` + signature. + ### Security - Updated dependencies to clear published advisories against the pinned versions: Authlib 1.4.0 → 1.7.2 (signature-verification bypass in JWS JWK @@ -28,6 +22,28 @@ pre-1.0 scheme; dates are when the change reached `main`. path), python-multipart 0.0.20 → 0.0.32 (denial-of-service and parameter smuggling in multipart parsing, reachable from the public upload form), and Jinja2 3.1.5 → 3.1.6 (CVE-2025-27516). +- Moved to Starlette 1.3.1 (FastAPI 0.140.0), clearing seven advisories that + applied to the previously resolved 0.41.3 — most notably CVE-2025-62727, a + quadratic-time denial of service reachable through the `Range` header on any + share download, and CVE-2026-54283, unenforced form-body limits on + URL-encoded posts. Starlette is now pinned explicitly rather than left to + dependency resolution. +- The client address behind the anonymous rate limit and the upload audit trail + is no longer take-your-word-for-it. Forwarded headers are honoured only from + peers listed in the new `TRUSTED_PROXIES` setting, and `X-Forwarded-For` is + read right to left so hops appended by the client are discarded. Previously + any caller that could open a socket to the app could name itself, which reset + the rate-limit budget and wrote a false address into the audit trail; + `TRUST_FORWARDED_FOR=false` did not prevent it, because the server was + rewriting the peer address before the application saw it. That setting is + replaced by `TRUSTED_PROXIES`. +- An OIDC token that carries no `groups` claim is now refused instead of being + admitted on the strength of Authentik's application binding. The provider is + configured to send the claim, so its absence means the configuration has + drifted — and falling back reduced a deliberately two-layer gate to the one + layer the in-app check exists not to depend on. `ALLOW_MISSING_GROUPS_CLAIM` + (default `false`) restores the old behaviour for repairing a broken scope + mapping, and warns at startup for as long as it is set. ## 2026-06-23 diff --git a/app/auth.py b/app/auth.py index e003409..98c018a 100644 --- a/app/auth.py +++ b/app/auth.py @@ -38,13 +38,28 @@ def __init__(self, data: dict): @property def in_required_group(self) -> bool: - # If Authentik returns no groups claim (scope mapping not configured), - # trust Authentik's application-level group binding instead of locking - # everyone out — but log it so the gap is visible. - if not self.groups: - log.warning("OIDC token carried no groups claim for %s; relying on Authentik app binding", self.sub) + if self.groups: + return settings.required_group in self.groups + # No groups claim at all. Authentik is configured to send one, so its + # absence means the provider's scope mapping has drifted, not that this + # user happens to belong to nothing. Authentik's own binding on the + # application should still have kept non-members out, but that is the + # layer we don't control — and this check exists precisely to not depend + # on it. So the answer is no, unless an operator has deliberately said + # otherwise while repairing the mapping. + if settings.allow_missing_groups_claim: + log.warning( + "OIDC token carried no groups claim for %s; admitting anyway because " + "ALLOW_MISSING_GROUPS_CLAIM is set. Fix the provider's groups scope mapping.", + self.sub, + ) return True - return settings.required_group in self.groups + log.warning( + "OIDC token carried no groups claim for %s; refusing. The provider's groups " + "scope mapping is missing or not bound to this application.", + self.sub, + ) + return False def get_current_user(request: Request) -> CurrentUser | None: diff --git a/app/config.py b/app/config.py index e4f3230..3cd2739 100644 --- a/app/config.py +++ b/app/config.py @@ -39,6 +39,11 @@ class Settings(BaseSettings): # Only members of this group may use the authenticated interface. Authentik # also gates the application by this group; this is the in-app backstop. required_group: str = "dropbox" + # A token with no `groups` claim leaves the in-app check with nothing to + # check, so it is refused. Turn this on only as a temporary measure while a + # broken scope mapping is repaired: it collapses a deliberately two-layer + # gate down to Authentik's binding alone. The app says so on every startup. + allow_missing_groups_claim: bool = False # ── Upload limits & abuse controls ─────────────────────────────────── # Authenticated members get a generous cap; anonymous (landing page) is tight. diff --git a/app/main.py b/app/main.py index 2b1b97c..4a94649 100644 --- a/app/main.py +++ b/app/main.py @@ -39,6 +39,9 @@ async def lifespan(app: FastAPI): init_db() log.info("%s started (oidc_configured=%s, email_enabled=%s)", settings.app_name, settings.oidc_configured, settings.email_enabled) + if settings.allow_missing_groups_claim: + log.warning("ALLOW_MISSING_GROUPS_CLAIM is set: tokens with no groups claim will be " + "admitted to the workspace. Clear it once the scope mapping is fixed.") yield diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 054bfd6..06e07bf 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -42,6 +42,7 @@ ignored. Booleans accept `true`/`false`/`1`/`0`. | `OIDC_CLIENT_SECRET` | *(empty)* | OAuth2 client secret. | | `OIDC_SCOPES` | `openid profile email groups` | Requested scopes. The `groups` scope is required for the in-app group check. | | `REQUIRED_GROUP` | `dropbox` | Group required for the authenticated workspace. Enforced at Authentik **and** in-app. | +| `ALLOW_MISSING_GROUPS_CLAIM` | `false` | Admit tokens that carry no `groups` claim at all. Off by default: without the claim the in-app check has nothing to check, and admitting the user silently reduces the gate to Authentik's binding alone. Turn on only while repairing a broken scope mapping; the app logs a warning at startup for as long as it is set. | > Sign-in stays disabled until all three of discovery URL, client ID, and client > secret are set (`settings.oidc_configured`). The public landing page, uploads, diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 09f37b1..cd869a9 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -19,10 +19,17 @@ abuse controls, the data handled, and the threat model. can't complete the flow at the IdP. 2. **In-app backstop** — `/app` and every management route re-check the `groups` claim (`CurrentUser.in_required_group`). -- **No-groups fallback.** If a token arrives with no `groups` claim (scope - mapping missing), the app trusts Authentik's application-level binding rather - than locking everyone out, and logs a warning so the gap is visible. Keep the - `groups` scope configured so the in-app check is authoritative. +- **A missing `groups` claim is refused.** Authentik is configured to send the + claim (a `groups` scope mapping is bound to the provider), so a token without + one means the provider's configuration has drifted — not that the user belongs + to nothing. Admitting them would quietly reduce a deliberately two-layer gate + to Authentik's binding alone, which is the layer this check exists not to + depend on. The request is refused and the reason is logged. +- **Escape hatch.** `ALLOW_MISSING_GROUPS_CLAIM` (default `false`) restores the + old behaviour so a broken scope mapping can be repaired without locking the + members out of their own files. It re-opens the gap it exists to close, so the + app logs a warning at startup for as long as it is set. Clear it once the + mapping is fixed. ## 2. Anonymous-upload abuse controls @@ -125,7 +132,9 @@ Every upload writes an `upload_events` row for review — see below. - Set a strong, unique `SECRET_KEY` in production; rotating it invalidates all sessions. -- Keep the `groups` scope mapping configured so the in-app group check is - authoritative rather than relying on the fallback. +- Keep the `groups` scope mapping bound to the provider. Without it nobody + reaches the workspace, which is the intended failure direction — check + `docker compose logs app` for the "no groups claim" warning if sign-in starts + ending in a 403. - Review the `upload_events` table periodically for anomalous IPs/fingerprints. - Tune `ANON_*` limits to the host's tolerance; they are all environment-driven. diff --git a/tests/backend/conftest.py b/tests/backend/conftest.py index 906f63d..1d5a6ce 100644 --- a/tests/backend/conftest.py +++ b/tests/backend/conftest.py @@ -68,6 +68,12 @@ def nonmember(monkeypatch): return _as_user(monkeypatch, ["other"]) +@pytest.fixture +def groupless(monkeypatch): + """Authenticated user whose token carried no `groups` claim at all.""" + return _as_user(monkeypatch, []) + + @pytest.fixture def db_session(): s = appdb.SessionLocal() diff --git a/tests/backend/test_auth.py b/tests/backend/test_auth.py index 0e96e16..8524426 100644 --- a/tests/backend/test_auth.py +++ b/tests/backend/test_auth.py @@ -27,6 +27,28 @@ def test_member_sees_source_link(client, member): assert settings.source_url in r.text +def test_missing_groups_claim_is_refused(client, groupless): + # A token with no groups claim leaves the in-app check with nothing to + # check. Absence of evidence is not evidence of membership. + r = client.get("/app") + assert r.status_code == 403 + assert "Access required" in r.text + + +def test_missing_groups_claim_also_blocks_the_management_api(client, groupless): + r = client.post("/api/files", files={"file": ("x.txt", b"x", "text/plain")}, data={"expiry_hours": "24"}) + assert r.status_code == 403 + + +def test_missing_groups_claim_can_be_admitted_deliberately(client, groupless, monkeypatch): + # The escape hatch for repairing a broken scope mapping without locking the + # only two members out of their own files. + from app.config import settings + + monkeypatch.setattr(settings, "allow_missing_groups_claim", True) + assert client.get("/app").status_code == 200 + + def test_api_upload_forbidden_for_anonymous(client): r = client.post("/api/files", files={"file": ("x.txt", b"x", "text/plain")}, data={"expiry_hours": "24"}) assert r.status_code == 403