From fb63724af9869853859b06783acf389dcc4ad726 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 15 Oct 2026 10:21:07 +0200 Subject: [PATCH 1/9] =?UTF-8?q?feat(auth):=20token=20principals=20?= =?UTF-8?q?=E2=80=94=20auth=5Fkind/scopes,=20scope=20cap,=20provider=20car?= =?UTF-8?q?ve-out=20(#455)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UserContext gains auth_kind ("session" by default) and scopes (None = uncapped), wired through from_user / to_session_dict / from_session_dict. auth.contracts.tokens holds the credential kinds and the smpat_/smsk_ prefixes. - resolve_principal_permissions and the bare-router fallback cap the resolved set at the principal's scopes (apply_scope_cap); an admin's wildcard expands to the scope list only. - require_permission skips its admin shortcut for a scoped principal; the /admin index, the Doctor page and the maintenance bypass use the new is_admin_principal, which is False for any token. - users and keycloak providers return None at once for an api_tokens bearer value instead of looking it up. - the tenants resolver binds a service principal to its pinned tenant and ignores the tenant header and subdomain for it. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../core/simple_module_core/permissions.py | 15 +++++++ .../simple_module_hosting/maintenance.py | 4 +- .../simple_module_hosting/permissions.py | 31 ++++++++++++-- host/routes.py | 4 +- modules/auth/auth/contracts/schemas.py | 41 ++++++++++++++++++- modules/auth/auth/contracts/tokens.py | 41 +++++++++++++++++++ modules/auth/auth/deps.py | 6 ++- .../dashboard/dashboard/endpoints/views.py | 4 +- modules/keycloak/keycloak/provider.py | 8 +++- modules/tenants/tenants/resolver.py | 21 ++++++++++ modules/users/users/provider.py | 10 ++++- 11 files changed, 171 insertions(+), 14 deletions(-) create mode 100644 modules/auth/auth/contracts/tokens.py diff --git a/framework/core/simple_module_core/permissions.py b/framework/core/simple_module_core/permissions.py index 9e6b5681c..baaea6f21 100644 --- a/framework/core/simple_module_core/permissions.py +++ b/framework/core/simple_module_core/permissions.py @@ -34,6 +34,21 @@ def is_admin(roles: list[str] | None) -> bool: return bool(roles) and ADMIN_ROLE in roles +def is_admin_principal(user: Any) -> bool: + """Whether *user* may use the admin role's shortcuts — admin and not a token. + + A principal carrying ``scopes`` (an ``api_tokens`` credential) is capped + at those scopes even when its owner is an admin, so every check that + admits admins on the role alone must use this rather than + :func:`is_admin`; otherwise an admin's ``feature_flags.view`` token would + still open the Doctor page. Duck-typed: ``UserContext`` lives in the auth + module, which framework code must not import. + """ + if user is None or getattr(user, "scopes", None) is not None: + return False + return is_admin(getattr(user, "roles", None)) + + def grants(held: Collection[str], required: str) -> bool: """Whether a principal holding *held* satisfies *required*. diff --git a/framework/hosting/simple_module_hosting/maintenance.py b/framework/hosting/simple_module_hosting/maintenance.py index c4e8925c8..94db274b8 100644 --- a/framework/hosting/simple_module_hosting/maintenance.py +++ b/framework/hosting/simple_module_hosting/maintenance.py @@ -14,7 +14,7 @@ import logging -from simple_module_core.permissions import is_admin +from simple_module_core.permissions import is_admin_principal from starlette.requests import Request from starlette.responses import JSONResponse from starlette.types import ASGIApp, Receive, Scope, Send @@ -83,7 +83,7 @@ def _may_bypass(request: Request, scope: Scope) -> bool: """Admins, anyone heading for the auth provider's own routes, and any route a module deliberately opened to anonymous visitors.""" user = getattr(request.state, "user", None) - if user is not None and is_admin(getattr(user, "roles", None)): + if user is not None and is_admin_principal(user): return True path: str = scope["path"] diff --git a/framework/hosting/simple_module_hosting/permissions.py b/framework/hosting/simple_module_hosting/permissions.py index 438e3f19c..c5a9b895e 100644 --- a/framework/hosting/simple_module_hosting/permissions.py +++ b/framework/hosting/simple_module_hosting/permissions.py @@ -18,6 +18,7 @@ "PERMISSION_DENIED_PREFIX", "WILDCARD", "RequiresPermission", + "apply_scope_cap", "ensure_resolved_permissions", "expand_permissions", "resolve_permissions", @@ -58,6 +59,29 @@ def _role_and_claimed(user: Any, role_map: dict[str, list[str]] | None) -> set[s return permissions +def apply_scope_cap(permissions: set[str], user: Any) -> set[str]: + """Cap *permissions* at the principal's token scopes, if it carries any. + + ``scopes`` is ``None`` for an ordinary session: nothing to cap. A list — + an API token's — is a ceiling: the result is what the principal holds + *and* the token names. Holding the wildcard (an admin) expands it to the + scope list, so an admin's token is worth exactly its scopes rather than + everything. Applied on every request on top of the owner's *live* grants, + so demoting the owner shrinks every token they hold at once. + + A ``*`` inside the scope list is ignored: a token can never carry the + wildcard, whatever row it was built from. Read duck-typed — the + ``UserContext`` field lives in the auth module (SM009). + """ + scopes = getattr(user, "scopes", None) + if scopes is None: + return permissions + allowed = {s for s in scopes if s and s != WILDCARD} + if WILDCARD in permissions: + return allowed + return permissions & allowed + + async def resolve_principal_permissions( request: Request, user: Any, @@ -73,11 +97,12 @@ async def resolve_principal_permissions( rather than an unauthorised one — and is logged rather than turned into a 500 on every request. A principal already holding the wildcard skips the sources: nothing they return could widen it, and a source may cost a read. + A token principal's set is then capped at its scopes (:func:`apply_scope_cap`). """ role_map = registry.role_map if registry is not None else None permissions = _role_and_claimed(user, role_map) if registry is None or WILDCARD in permissions: - return permissions + return apply_scope_cap(permissions, user) for source in registry.grant_sources: try: # Grants are additive keys only: the wildcard (admin) comes from @@ -85,7 +110,7 @@ async def resolve_principal_permissions( permissions.update(k for k in await source(request, user) if k != WILDCARD) except Exception: logger.exception("Grant source %r raised; contributing nothing", source) - return permissions + return apply_scope_cap(permissions, user) def _registry_for(request: Request) -> PermissionRegistry | None: @@ -150,7 +175,7 @@ def resolved_permissions_for(request: Request) -> set[str]: perm_registry = _registry_for(request) role_map = perm_registry.role_map if perm_registry is not None else None - permissions = _role_and_claimed(user, role_map) + permissions = apply_scope_cap(_role_and_claimed(user, role_map), user) request.state.resolved_permissions = permissions return permissions diff --git a/host/routes.py b/host/routes.py index 6e20f5a6d..95a30cc34 100644 --- a/host/routes.py +++ b/host/routes.py @@ -14,7 +14,7 @@ from __future__ import annotations from fastapi import APIRouter, HTTPException, Request -from simple_module_core.permissions import is_admin +from simple_module_core.permissions import is_admin_principal from simple_module_hosting.inertia_deps import InertiaDep from simple_module_inertia import InertiaResponse @@ -59,7 +59,7 @@ async def admin_overview(request: Request, inertia: InertiaDep) -> InertiaRespon user = getattr(request.state, "user", None) if user is None: raise HTTPException(status_code=403, detail="Administrator access required") - if not (is_admin(getattr(user, "roles", None)) or _has_admin_entries(request)): + if not (is_admin_principal(user) or _has_admin_entries(request)): raise HTTPException(status_code=403, detail="Administrator access required") return await inertia.render("Admin", {}) diff --git a/modules/auth/auth/contracts/schemas.py b/modules/auth/auth/contracts/schemas.py index 78cb34303..af5769fbc 100644 --- a/modules/auth/auth/contracts/schemas.py +++ b/modules/auth/auth/contracts/schemas.py @@ -5,6 +5,8 @@ from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any +from auth.contracts.tokens import AUTH_KIND_SESSION + if TYPE_CHECKING: # Runtime import would be circular: auth -> users -> auth. # Only imported for type-hints, never at runtime. @@ -24,9 +26,31 @@ class UserContext: # provider maps in. Frozen in the session cookie like ``roles``; anything # revocable belongs in a grant source. The resolver drops ``*`` from here. permissions: list[str] = field(default_factory=list) + # Which credential authenticated the request: ``"session"`` for a browser + # sign-in or an IdP bearer token, ``"pat"`` / ``"service"`` for an + # ``api_tokens`` credential (see ``auth.contracts.tokens``). Code that must + # only ever act for a person at a keyboard — minting more credentials — + # refuses anything but ``"session"``. + auth_kind: str = AUTH_KIND_SESSION + # The ceiling a token carries. ``None`` means "no cap" (an ordinary + # session); a list — even an empty one — is intersected with whatever the + # principal resolves to, and the admin-role shortcuts are skipped, so an + # admin's token is worth exactly the scopes it was given. + scopes: list[str] | None = None + + @property + def is_scoped(self) -> bool: + """Whether a scope cap applies (any token-authenticated principal).""" + return self.scopes is not None @classmethod - def from_user(cls, user: User | Any) -> UserContext: + def from_user( + cls, + user: User | Any, + *, + auth_kind: str = AUTH_KIND_SESSION, + scopes: list[str] | None = None, + ) -> UserContext: """Build a UserContext from a users.models.User with eagerly-loaded roles. Duck-typed to avoid importing users.models at runtime — any object @@ -41,6 +65,8 @@ def from_user(cls, user: User | Any) -> UserContext: email=user.email, name=user.full_name or user.email, roles=[r.name for r in user.roles], + auth_kind=auth_kind, + scopes=list(scopes) if scopes is not None else None, ) def to_session_dict(self) -> dict[str, Any]: @@ -56,6 +82,8 @@ def to_session_dict(self) -> dict[str, Any]: "roles": list(self.roles), "tenant_id": self.tenant_id, "permissions": list(self.permissions), + "auth_kind": self.auth_kind, + "scopes": list(self.scopes) if self.scopes is not None else None, } @classmethod @@ -73,6 +101,8 @@ def from_session_dict(cls, payload: Any) -> UserContext | None: tenant_id=payload.get("tenant_id"), # Absent on cookies written before the field existed. permissions=list(payload.get("permissions") or []), + auth_kind=str(payload.get("auth_kind") or AUTH_KIND_SESSION), + scopes=_scopes_from(payload.get("scopes")), ) except (KeyError, TypeError, ValueError): return None @@ -89,3 +119,12 @@ def has_permission(self, permission: str) -> bool: def has_any_role(self, roles: list[str]) -> bool: return bool(set(self.roles) & set(roles)) + + +def _scopes_from(raw: Any) -> list[str] | None: + """``None`` stays ``None`` (uncapped); anything else must be a list.""" + if raw is None: + return None + if not isinstance(raw, list): + raise TypeError("scopes must be a list") + return [str(s) for s in raw] diff --git a/modules/auth/auth/contracts/tokens.py b/modules/auth/auth/contracts/tokens.py new file mode 100644 index 000000000..97d7a0edf --- /dev/null +++ b/modules/auth/auth/contracts/tokens.py @@ -0,0 +1,41 @@ +"""Credential kinds and API-token prefixes shared by every auth source. + +Lives in ``auth`` rather than in the ``api_tokens`` module because the auth +*providers* need it: ``users`` and ``keycloak`` both claim every +``Authorization: Bearer`` header, and must step aside for a value that is +recognisably an ``api_tokens`` credential — otherwise each token request costs +a wasted lookup against the provider's own token store (or a JWT parse), and +the provider's ``None`` there is final, so the principal resolver that +actually owns the token never gets to see it. +""" + +from __future__ import annotations + +AUTH_KIND_SESSION = "session" +"""A browser sign-in or an identity provider's own bearer token.""" + +AUTH_KIND_PAT = "pat" +"""A personal access token acting for its owner, capped by its scopes.""" + +AUTH_KIND_SERVICE = "service" +"""A service API key: no person behind it, roles and tenant fixed at creation.""" + +PAT_PREFIX = "smpat_" +SERVICE_KEY_PREFIX = "smsk_" +API_TOKEN_PREFIXES: tuple[str, ...] = (PAT_PREFIX, SERVICE_KEY_PREFIX) + + +def is_api_token(credential: str) -> bool: + """Whether a bearer value is an ``api_tokens`` credential (by prefix only).""" + return credential.startswith(API_TOKEN_PREFIXES) + + +__all__ = [ + "API_TOKEN_PREFIXES", + "AUTH_KIND_PAT", + "AUTH_KIND_SERVICE", + "AUTH_KIND_SESSION", + "PAT_PREFIX", + "SERVICE_KEY_PREFIX", + "is_api_token", +] diff --git a/modules/auth/auth/deps.py b/modules/auth/auth/deps.py index 50ae670ef..89f212251 100644 --- a/modules/auth/auth/deps.py +++ b/modules/auth/auth/deps.py @@ -42,8 +42,10 @@ async def check( t: TranslatorDep, user: UserContext = Depends(get_current_user), ): - # Admin role bypasses permission checks - if _ADMIN_ROLE in user.roles: + # Admin role bypasses permission checks — but not for a token: an + # admin's PAT is worth only the scopes it carries, and the resolved + # set below already applies that cap. + if _ADMIN_ROLE in user.roles and user.scopes is None: return # The same resolution RequiresPermission reads: the registry's role map diff --git a/modules/dashboard/dashboard/endpoints/views.py b/modules/dashboard/dashboard/endpoints/views.py index 31792a838..82143ffcf 100644 --- a/modules/dashboard/dashboard/endpoints/views.py +++ b/modules/dashboard/dashboard/endpoints/views.py @@ -9,7 +9,7 @@ import asyncio from fastapi import APIRouter, Depends, HTTPException, Request -from simple_module_core.permissions import grants, is_admin +from simple_module_core.permissions import grants, is_admin_principal from simple_module_db.deps import get_db from simple_module_hosting.i18n_deps import TranslatorDep from simple_module_hosting.inertia_deps import InertiaDep @@ -29,7 +29,7 @@ async def _require_admin(request: Request) -> None: only. The ``/admin`` prefix is a URL convention, not a permission, so this is the guard that actually keeps a non-admin, signed-in user out.""" user = getattr(request.state, "user", None) - if user is None or not is_admin(getattr(user, "roles", None)): + if user is None or not is_admin_principal(user): raise HTTPException(status_code=403, detail="Administrator access required") diff --git a/modules/keycloak/keycloak/provider.py b/modules/keycloak/keycloak/provider.py index eef2ca4b0..905c2f1f5 100644 --- a/modules/keycloak/keycloak/provider.py +++ b/modules/keycloak/keycloak/provider.py @@ -7,6 +7,7 @@ from typing import TYPE_CHECKING, Any from auth.contracts.schemas import UserContext +from auth.contracts.tokens import is_api_token from simple_module_core.tenancy import is_tenant_role from simple_module_db import is_valid_tenant_id from starlette.requests import Request @@ -46,7 +47,12 @@ def _settings(self) -> KeycloakSettings | None: async def resolve_user(self, request: Request) -> UserContext | None: auth_header = request.headers.get("authorization", "") if auth_header.startswith("Bearer "): - return await self._resolve_bearer(request, auth_header[7:]) + credential = auth_header[7:] + # An ``api_tokens`` credential is not a JWT: skip the parse and + # leave it to the principal resolver that owns it. + if is_api_token(credential): + return None + return await self._resolve_bearer(request, credential) session = request.scope.get("session", {}) ctx = UserContext.from_session_dict(session.get(_SESSION_USER_CTX_KEY)) diff --git a/modules/tenants/tenants/resolver.py b/modules/tenants/tenants/resolver.py index 86341c326..35d210660 100644 --- a/modules/tenants/tenants/resolver.py +++ b/modules/tenants/tenants/resolver.py @@ -18,6 +18,7 @@ import logging from typing import Any +from auth.contracts.tokens import AUTH_KIND_SERVICE from cachetools import TTLCache from fastapi import FastAPI from simple_module_core.invalidation import Invalidation, InvalidationBus @@ -146,6 +147,12 @@ async def _resolve(request: Request) -> tuple[str | None, str | None, tuple[str, # every branch), so a shared cache must key on it too; an anonymous one did # not, and anonymous public pages stay cacheable (#418). credential: tuple[str, ...] = ("Cookie",) if user is not None else () + if getattr(user, "auth_kind", None) == AUTH_KIND_SERVICE: + # A service key is pinned to the tenant it was created in. It has no + # memberships to validate against and no session preference; neither a + # header nor a subdomain may move it to another tenant. + tenant_id = await _service_tenant(request, getattr(user, "tenant_id", None)) + return tenant_id, "token", ("Authorization",) slug = subdomain_slug(request) if slug is not None: tenant_id = await _resolve_subdomain(request, user, slug) @@ -214,6 +221,20 @@ async def _resolve_subdomain(request: Request, user: Any, slug: str) -> str | No return tenant_id +async def _service_tenant(request: Request, tenant_id: Any) -> str | None: + """The service key's pinned tenant, while that tenant exists and is active.""" + if not is_valid_tenant_id(tenant_id): + return None + from tenants.models import Tenant + + async with request.app.state.sm.db.session_factory() as db: + tenant = await db.get(Tenant, tenant_id) + if tenant is None or tenant.status != TenantStatus.ACTIVE: + request.state.tenant_suspended = tenant is not None + return None + return tenant_id + + def _header_name(request: Request) -> str: settings = getattr(getattr(request.app.state, "sm", None), "settings", None) return getattr(settings, "tenant_header", "") or "" diff --git a/modules/users/users/provider.py b/modules/users/users/provider.py index 5207a1260..61df970d4 100644 --- a/modules/users/users/provider.py +++ b/modules/users/users/provider.py @@ -13,6 +13,7 @@ from typing import Any from auth.contracts.schemas import UserContext +from auth.contracts.tokens import is_api_token from simple_module_hosting.session import ( SESSION_COOKIE_MAX_AGE, SESSION_EXPIRES_AT_KEY, @@ -90,7 +91,14 @@ async def resolve_user(self, request: Request) -> UserContext | None: # uncollected. auth_header = request.headers.get("authorization", "") if auth_header.startswith("Bearer "): - return await self._resolve_bearer(request.scope, auth_header[7:]) + credential = auth_header[7:] + # An ``api_tokens`` credential is not ours: answering None at once + # saves a pointless ``users_access_token`` lookup and leaves the + # token to the principal resolver that owns it. Still no fallback + # to the session cookie — the header decided the request. + if is_api_token(credential): + return None + return await self._resolve_bearer(request.scope, credential) session = request.scope.get("session", {}) raw_user_id = session.get(SESSION_USER_ID_KEY) From 8d018a0e0e46ef9789f0b2168461d730021cf3c3 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 15 Oct 2026 10:21:07 +0200 Subject: [PATCH 2/9] feat(api_tokens): personal access tokens and service API keys (#455) New module simple_module_api_tokens. Tokens are smpat_/smsk_ + 32 random bytes in base62; only sha256(token) and an 8-char display prefix are stored, and the secret is returned once. A principal resolver does one indexed lookup by hash per request (no cache), refuses revoked, expired, unknown and inactive/disabled-owner tokens, and throttles last_used_at to one conditional UPDATE per 5 minutes in its own transaction. Scopes must be held by the creator at creation and cap the principal on every request on top of the owner's live grants. Service keys act as svc: with roles chosen at creation (never admin or tenant:*) and are pinned to their tenant. Tokens can only be created, listed and revoked from a session (403 for any token), with RequiresCsrf on every mutation. Self-service page at /api-tokens/ (user menu), admin screen at /admin/api-tokens/ (Access group). Create/revoke are audited; the hash is excluded. Migration 51b2058d8afd (branch label api_tokens), verified with the Postgres round trip. Added to the release matrix; not to `smpy new`. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .github/workflows/release.yml | 1 + .../51b2058d8afd_api_tokens_initial.py | 72 +++++++ host/pyproject.toml | 2 + modules/api_tokens/LICENSE | 19 ++ modules/api_tokens/README.md | 38 ++++ modules/api_tokens/api_tokens/__init__.py | 1 + modules/api_tokens/api_tokens/audit.py | 52 +++++ modules/api_tokens/api_tokens/constants.py | 76 +++++++ .../api_tokens/contracts/__init__.py | 10 + .../api_tokens/contracts/schemas.py | 64 ++++++ modules/api_tokens/api_tokens/deps.py | 61 ++++++ .../api_tokens/endpoints/__init__.py | 0 .../api_tokens/endpoints/admin_api.py | 98 +++++++++ .../api_tokens/api_tokens/endpoints/api.py | 88 +++++++++ .../api_tokens/api_tokens/endpoints/views.py | 96 +++++++++ modules/api_tokens/api_tokens/locales/en.json | 94 +++++++++ modules/api_tokens/api_tokens/models.py | 58 ++++++ modules/api_tokens/api_tokens/module.py | 109 ++++++++++ modules/api_tokens/api_tokens/pages/Admin.tsx | 92 +++++++++ modules/api_tokens/api_tokens/pages/Index.tsx | 70 +++++++ modules/api_tokens/api_tokens/pages/api.ts | 42 ++++ .../pages/components/CreateTokenDialog.tsx | 186 ++++++++++++++++++ .../pages/components/CreatedTokenDialog.tsx | 64 ++++++ .../pages/components/RevokeTokenDialog.tsx | 29 +++ .../pages/components/TokenTable.tsx | 117 +++++++++++ .../pages/components/useTokenActions.ts | 62 ++++++ modules/api_tokens/api_tokens/pages/types.ts | 40 ++++ modules/api_tokens/api_tokens/py.typed | 0 modules/api_tokens/api_tokens/resolver.py | 145 ++++++++++++++ modules/api_tokens/api_tokens/scopes.py | 58 ++++++ modules/api_tokens/api_tokens/service.py | 151 ++++++++++++++ modules/api_tokens/api_tokens/services.py | 14 ++ modules/api_tokens/api_tokens/settings.py | 17 ++ modules/api_tokens/api_tokens/tokens.py | 83 ++++++++ modules/api_tokens/package.json | 19 ++ modules/api_tokens/pyproject.toml | 66 +++++++ .../api_tokens/tests/_api_tokens_support.py | 87 ++++++++ modules/api_tokens/tests/conftest.py | 49 +++++ .../tests/test_api_tokens_endpoints.py | 144 ++++++++++++++ .../tests/test_api_tokens_format.py | 112 +++++++++++ .../tests/test_api_tokens_resolver.py | 143 ++++++++++++++ .../tests/test_api_tokens_scopes.py | 127 ++++++++++++ .../tests/test_api_tokens_service_keys.py | 85 ++++++++ modules/api_tokens/tsconfig.json | 10 + package-lock.json | 21 ++ packages/i18n/src/generated-resources.ts | 68 +++++++ packages/i18n/src/keys.generated.ts | 96 +++++++++ pyproject.toml | 3 +- 48 files changed, 3138 insertions(+), 1 deletion(-) create mode 100644 host/migrations/versions/51b2058d8afd_api_tokens_initial.py create mode 100644 modules/api_tokens/LICENSE create mode 100644 modules/api_tokens/README.md create mode 100644 modules/api_tokens/api_tokens/__init__.py create mode 100644 modules/api_tokens/api_tokens/audit.py create mode 100644 modules/api_tokens/api_tokens/constants.py create mode 100644 modules/api_tokens/api_tokens/contracts/__init__.py create mode 100644 modules/api_tokens/api_tokens/contracts/schemas.py create mode 100644 modules/api_tokens/api_tokens/deps.py create mode 100644 modules/api_tokens/api_tokens/endpoints/__init__.py create mode 100644 modules/api_tokens/api_tokens/endpoints/admin_api.py create mode 100644 modules/api_tokens/api_tokens/endpoints/api.py create mode 100644 modules/api_tokens/api_tokens/endpoints/views.py create mode 100644 modules/api_tokens/api_tokens/locales/en.json create mode 100644 modules/api_tokens/api_tokens/models.py create mode 100644 modules/api_tokens/api_tokens/module.py create mode 100644 modules/api_tokens/api_tokens/pages/Admin.tsx create mode 100644 modules/api_tokens/api_tokens/pages/Index.tsx create mode 100644 modules/api_tokens/api_tokens/pages/api.ts create mode 100644 modules/api_tokens/api_tokens/pages/components/CreateTokenDialog.tsx create mode 100644 modules/api_tokens/api_tokens/pages/components/CreatedTokenDialog.tsx create mode 100644 modules/api_tokens/api_tokens/pages/components/RevokeTokenDialog.tsx create mode 100644 modules/api_tokens/api_tokens/pages/components/TokenTable.tsx create mode 100644 modules/api_tokens/api_tokens/pages/components/useTokenActions.ts create mode 100644 modules/api_tokens/api_tokens/pages/types.ts create mode 100644 modules/api_tokens/api_tokens/py.typed create mode 100644 modules/api_tokens/api_tokens/resolver.py create mode 100644 modules/api_tokens/api_tokens/scopes.py create mode 100644 modules/api_tokens/api_tokens/service.py create mode 100644 modules/api_tokens/api_tokens/services.py create mode 100644 modules/api_tokens/api_tokens/settings.py create mode 100644 modules/api_tokens/api_tokens/tokens.py create mode 100644 modules/api_tokens/package.json create mode 100644 modules/api_tokens/pyproject.toml create mode 100644 modules/api_tokens/tests/_api_tokens_support.py create mode 100644 modules/api_tokens/tests/conftest.py create mode 100644 modules/api_tokens/tests/test_api_tokens_endpoints.py create mode 100644 modules/api_tokens/tests/test_api_tokens_format.py create mode 100644 modules/api_tokens/tests/test_api_tokens_resolver.py create mode 100644 modules/api_tokens/tests/test_api_tokens_scopes.py create mode 100644 modules/api_tokens/tests/test_api_tokens_service_keys.py create mode 100644 modules/api_tokens/tsconfig.json diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 759e5b6df..c6da54b06 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -116,6 +116,7 @@ jobs: - simple_module_hosting - simple_module_inertia - simple_module_test + - simple_module_api_tokens - simple_module_audit_log - simple_module_auth - simple_module_background_tasks diff --git a/host/migrations/versions/51b2058d8afd_api_tokens_initial.py b/host/migrations/versions/51b2058d8afd_api_tokens_initial.py new file mode 100644 index 000000000..f60531731 --- /dev/null +++ b/host/migrations/versions/51b2058d8afd_api_tokens_initial.py @@ -0,0 +1,72 @@ +"""api_tokens: initial schema (personal access tokens and service keys) + +One table for both credential kinds. Only the SHA-256 of a token is stored; +the unique key is ``(token_hash, tenant_id)`` — hash first, so a lookup by +hash alone is an index seek, and tenant in the key per SM024. + +Revision ID: 51b2058d8afd +Revises: e5a8c1f27b94 +Create Date: 2026-10-15 09:49:09.602365 +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op +from fastapi_users_db_sqlalchemy.generics import GUID + +# revision identifiers, used by Alembic. +revision: str = "51b2058d8afd" +down_revision: str | None = "e5a8c1f27b94" +branch_labels: str | Sequence[str] | None = ("api_tokens",) +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + op.create_table( + "api_tokens_token", + sa.Column("tenant_id", sa.String(length=50), nullable=False), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("(CURRENT_TIMESTAMP)"), + nullable=False, + ), + sa.Column("updated_at", sa.DateTime(timezone=True), nullable=True), + sa.Column("created_by", sa.String(length=255), nullable=True), + sa.Column("updated_by", sa.String(length=255), nullable=True), + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("kind", sa.String(length=16), nullable=False), + sa.Column("name", sa.String(length=100), nullable=False), + sa.Column("prefix", sa.String(length=16), nullable=False), + sa.Column("token_hash", sa.String(length=64), nullable=False), + sa.Column("owner_user_id", GUID(), nullable=True), + sa.Column("scopes", sa.JSON(), nullable=False), + sa.Column("roles", sa.JSON(), nullable=False), + sa.Column("expires_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("last_used_at", sa.DateTime(timezone=True), nullable=True), + sa.Column("revoked_at", sa.DateTime(timezone=True), nullable=True), + sa.PrimaryKeyConstraint("id", name=op.f("pk_api_tokens_token")), + ) + op.create_index( + "ix_api_tokens_token_hash_tenant", + "api_tokens_token", + ["token_hash", "tenant_id"], + unique=True, + ) + op.create_index( + op.f("ix_api_tokens_token_owner_user_id"), + "api_tokens_token", + ["owner_user_id"], + unique=False, + ) + op.create_index( + op.f("ix_api_tokens_token_tenant_id"), "api_tokens_token", ["tenant_id"], unique=False + ) + + +def downgrade() -> None: + op.drop_index(op.f("ix_api_tokens_token_tenant_id"), table_name="api_tokens_token") + op.drop_index(op.f("ix_api_tokens_token_owner_user_id"), table_name="api_tokens_token") + op.drop_index("ix_api_tokens_token_hash_tenant", table_name="api_tokens_token") + op.drop_table("api_tokens_token") diff --git a/host/pyproject.toml b/host/pyproject.toml index bc3fec34e..5c1efbd16 100644 --- a/host/pyproject.toml +++ b/host/pyproject.toml @@ -16,6 +16,7 @@ dependencies = [ "simple_module_branding", "simple_module_site_lock", "simple_module_tenants", + "simple_module_api_tokens", "python-multipart>=0.0.6", ] @@ -32,3 +33,4 @@ simple_module_audit_log = { workspace = true } simple_module_branding = { workspace = true } simple_module_site_lock = { workspace = true } simple_module_tenants = { workspace = true } +simple_module_api_tokens = { workspace = true } diff --git a/modules/api_tokens/LICENSE b/modules/api_tokens/LICENSE new file mode 100644 index 000000000..9cf106272 --- /dev/null +++ b/modules/api_tokens/LICENSE @@ -0,0 +1,19 @@ +MIT License + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/modules/api_tokens/README.md b/modules/api_tokens/README.md new file mode 100644 index 000000000..8545e53e5 --- /dev/null +++ b/modules/api_tokens/README.md @@ -0,0 +1,38 @@ +# simple_module_api_tokens + +Personal access tokens and service API keys for +[simple_module](https://github.com/antosubash/simple_module_python) apps. + +- `smpat_…` tokens act for their owner, `smsk_…` service keys act for no one, + with roles chosen at creation (never `admin`) and a pinned tenant. +- Every token carries **scopes**, which cap what it can do on every request, + even when its owner is an admin. Demoting the owner shrinks the token at once. +- Only `sha256(token)` is stored, and the secret is shown once. Revocation + applies on the next request. +- Tokens can only be created, listed and revoked from a signed-in browser + session, with a CSRF token. + +## Install + +```bash +pip install simple_module_api_tokens +``` + +Add `simple_module_api_tokens` to your host's dependencies and generate a +migration. It needs the `auth`, `users` and `settings` modules. + +## Usage + +```bash +curl -H "Authorization: Bearer smpat_…" https://app.example.com/api/feature_flags/ +``` + +| Route | Page | Purpose | +|---|---|---| +| `GET /api-tokens/` | `ApiTokens/Index` | Your own tokens (`api_tokens.use`) | +| `GET /admin/api-tokens/` | `ApiTokens/Admin` | All tokens and service keys (`api_tokens.manage`) | + +Settings: `max_lifetime_days` (default 365), DB-backed. + +See [docs/modules/api_tokens.md](https://github.com/antosubash/simple_module_python/blob/main/docs/modules/api_tokens.md) +for the resolution order, the scope cap and the tenancy rules. diff --git a/modules/api_tokens/api_tokens/__init__.py b/modules/api_tokens/api_tokens/__init__.py new file mode 100644 index 000000000..cb40b8786 --- /dev/null +++ b/modules/api_tokens/api_tokens/__init__.py @@ -0,0 +1 @@ +"""ApiTokens module.""" diff --git a/modules/api_tokens/api_tokens/audit.py b/modules/api_tokens/api_tokens/audit.py new file mode 100644 index 000000000..999cdf178 --- /dev/null +++ b/modules/api_tokens/api_tokens/audit.py @@ -0,0 +1,52 @@ +"""How an api_tokens row is named, and reached, from the audit log. + +Create and revoke are recorded by ``AuditMixin`` (the hash and +``last_used_at`` are excluded on the model). A token is named by its own +name and display prefix — never by anything secret. +""" + +from __future__ import annotations + +import uuid + +from simple_module_core.audit_links import AuditLink +from simple_module_db import ALL_TENANTS_OPTION +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from api_tokens.constants import ADMIN_URL, AUDIT_LINK_LABEL, AUDIT_LINK_LABEL_KEY, QP_TOKEN +from api_tokens.models import ApiToken + + +async def resolve_token_labels(db: AsyncSession, ids: list[str]) -> dict[str, str]: + """Map token id -> ``name (prefix…)`` for every id that still resolves. + + Read across tenants like the rest of the audit log: the audit screen is a + platform view, and the label carries nothing the row itself does not. + """ + parsed: dict[uuid.UUID, str] = {} + for raw in ids: + try: + parsed[uuid.UUID(raw)] = raw + except (ValueError, AttributeError, TypeError): + continue + if not parsed: + return {} + stmt = ( + select(ApiToken.id, ApiToken.name, ApiToken.prefix) + .where(ApiToken.id.in_(list(parsed))) + .execution_options(**{ALL_TENANTS_OPTION: True}) + ) + rows = (await db.execute(stmt)).all() + return {parsed.get(tid, str(tid)): f"{name} ({prefix}…)" for tid, name, prefix in rows} + + +def build_token_audit_link() -> AuditLink: + return AuditLink( + entity_type=ApiToken.__name__, + url_template=f"{ADMIN_URL}?{QP_TOKEN}={{id}}", + label=AUDIT_LINK_LABEL, + label_key=AUDIT_LINK_LABEL_KEY, + table_name=ApiToken.__tablename__, + label_resolver=resolve_token_labels, + ) diff --git a/modules/api_tokens/api_tokens/constants.py b/modules/api_tokens/api_tokens/constants.py new file mode 100644 index 000000000..fcc76e99e --- /dev/null +++ b/modules/api_tokens/api_tokens/constants.py @@ -0,0 +1,76 @@ +"""Stable identifiers for the api_tokens module.""" + +from __future__ import annotations + +from enum import StrEnum +from typing import Final + +from auth.contracts.tokens import PAT_PREFIX, SERVICE_KEY_PREFIX + +MODULE_NAME: Final = "ApiTokens" +MODULE_PACKAGE: Final = "api_tokens" +LOCALE_NAMESPACE: Final = "api_tokens" + +# Modules this one needs registered first: Auth owns the resolver chain, +# Users owns the accounts a personal token acts for, Settings stores +# ``max_lifetime_days``. +MODULE_AUTH: Final = "Auth" +MODULE_USERS: Final = "Users" +MODULE_SETTINGS: Final = "Settings" + + +class TokenKind(StrEnum): + PERSONAL = "personal" + SERVICE = "service" + + +KIND_PREFIX: Final = {TokenKind.PERSONAL: PAT_PREFIX, TokenKind.SERVICE: SERVICE_KEY_PREFIX} + +# Secret: 32 random bytes in base62, left-padded to a fixed width so a +# well-formed token can be recognised without touching the database. +SECRET_BYTES: Final = 32 +SECRET_LENGTH: Final = 43 # ceil(256 / log2(62)) +DISPLAY_PREFIX_LENGTH: Final = 8 + +# Prefix is load-bearing on SQLite, which has no per-module schemas. +TABLE_TOKEN: Final = "api_tokens_token" +IX_TOKEN_HASH: Final = "ix_api_tokens_token_hash_tenant" +NAME_MAX_LENGTH: Final = 100 +MAX_SCOPES: Final = 200 + +# How stale ``last_used_at`` may get before a request rewrites it. Bounds +# the write load to one conditional UPDATE per token per window. +LAST_USED_WRITE_INTERVAL_SECONDS: Final = 300 + +PERM_GROUP: Final = "API tokens" +PERM_USE: Final = "api_tokens.use" +PERM_MANAGE: Final = "api_tokens.manage" +USER_ROLE: Final = "user" + +# Roles a service key may never hold: ``admin`` would be the wildcard on a +# credential with no person behind it. +FORBIDDEN_SERVICE_ROLES: Final = frozenset({"admin"}) + +API_PREFIX: Final = "/api/api_tokens" +VIEW_PREFIX: Final = "/api-tokens" +ADMIN_VIEW_PREFIX: Final = "/admin/api-tokens" +# Trailing slash: both index routes are registered at "/" under their prefix. +USER_URL: Final = f"{VIEW_PREFIX}/" +ADMIN_URL: Final = f"{ADMIN_VIEW_PREFIX}/" + +PAGE_INDEX: Final = "ApiTokens/Index" +PAGE_ADMIN: Final = "ApiTokens/Admin" + +MENU_LABEL: Final = "API tokens" +MENU_LABEL_KEY: Final = "api_tokens.nav.api_tokens" +MENU_USER_ORDER: Final = 980 +MENU_ADMIN_ORDER: Final = 130 +MENU_ICON: Final = "key" + +AUDIT_LINK_LABEL: Final = "API token" +AUDIT_LINK_LABEL_KEY: Final = "api_tokens.audit.token" +QP_TOKEN: Final = "token" + +# Expiry presets offered by the pages, in days; the server accepts any whole +# number of days up to ``max_lifetime_days``. +EXPIRY_PRESETS_DAYS: Final = (7, 30, 90, 365) diff --git a/modules/api_tokens/api_tokens/contracts/__init__.py b/modules/api_tokens/api_tokens/contracts/__init__.py new file mode 100644 index 000000000..b2ff6476b --- /dev/null +++ b/modules/api_tokens/api_tokens/contracts/__init__.py @@ -0,0 +1,10 @@ +"""api_tokens contracts — public interface for other modules.""" + +from api_tokens.contracts.schemas import ( + CreatedToken, + PersonalTokenCreate, + ServiceKeyCreate, + TokenView, +) + +__all__ = ["CreatedToken", "PersonalTokenCreate", "ServiceKeyCreate", "TokenView"] diff --git a/modules/api_tokens/api_tokens/contracts/schemas.py b/modules/api_tokens/api_tokens/contracts/schemas.py new file mode 100644 index 000000000..ad1fc5f78 --- /dev/null +++ b/modules/api_tokens/api_tokens/contracts/schemas.py @@ -0,0 +1,64 @@ +"""SQLModel DTOs for the api_tokens module — its public surface.""" + +from __future__ import annotations + +import uuid +from datetime import datetime + +from pydantic import ConfigDict, field_validator +from sqlmodel import Field, SQLModel + +from api_tokens.constants import MAX_SCOPES, NAME_MAX_LENGTH + + +class TokenView(SQLModel): + """A token as lists show it. Never carries the secret or its hash.""" + + model_config = ConfigDict(from_attributes=True) + + id: uuid.UUID + kind: str + name: str + prefix: str + scopes: list[str] + roles: list[str] = [] + owner_user_id: uuid.UUID | None = None + owner_email: str | None = None + tenant_id: str | None = None + created_at: datetime | None = None + created_by: str | None = None + expires_at: datetime + last_used_at: datetime | None = None + revoked_at: datetime | None = None + + +class CreatedToken(SQLModel): + """The one response that carries the plaintext token.""" + + token: str + item: TokenView + + +class _CreateBase(SQLModel): + name: str = Field(min_length=1, max_length=NAME_MAX_LENGTH) + scopes: list[str] = Field(min_length=1, max_length=MAX_SCOPES) + # Bounded above by the ``max_lifetime_days`` setting, checked in the service. + expires_in_days: int = Field(ge=1) + + @field_validator("name") + @classmethod + def _strip_name(cls, value: str) -> str: + stripped = value.strip() + if not stripped: + raise ValueError("name must not be blank") + return stripped + + +class PersonalTokenCreate(_CreateBase): + """Create a personal access token for the signed-in user.""" + + +class ServiceKeyCreate(_CreateBase): + """Create a service key. ``roles`` may not include ``admin``.""" + + roles: list[str] = Field(default_factory=list, max_length=50) diff --git a/modules/api_tokens/api_tokens/deps.py b/modules/api_tokens/api_tokens/deps.py new file mode 100644 index 000000000..c202947ce --- /dev/null +++ b/modules/api_tokens/api_tokens/deps.py @@ -0,0 +1,61 @@ +"""FastAPI dependencies for the api_tokens module.""" + +from __future__ import annotations + +import uuid +from typing import Annotated + +from auth.contracts.schemas import UserContext +from auth.contracts.tokens import AUTH_KIND_SESSION +from fastapi import Depends, HTTPException, Request +from simple_module_db.deps import get_db +from simple_module_hosting.i18n_deps import TranslatorDep +from sqlalchemy.ext.asyncio import AsyncSession + +from api_tokens.service import ApiTokenService, TokenError +from api_tokens.settings import ApiTokensSettings + + +async def get_api_token_service(db: AsyncSession = Depends(get_db)) -> ApiTokenService: + return ApiTokenService(db) + + +def get_settings(request: Request) -> ApiTokensSettings: + return request.app.state.api_tokens.settings + + +async def require_session_principal(request: Request, t: TranslatorDep) -> UserContext: + """The signed-in person — refusing any token-authenticated request. + + Tokens may not mint, list or revoke tokens: a leaked token must not be + able to make itself permanent by creating a sibling, or hide by revoking + the one an administrator would look for. + """ + user = getattr(request.state, "user", None) + if user is None: + raise HTTPException(status_code=401, detail=t.t("auth.errors.not_authenticated")) + if getattr(user, "auth_kind", AUTH_KIND_SESSION) != AUTH_KIND_SESSION or ( + getattr(user, "scopes", None) is not None + ): + raise HTTPException(status_code=403, detail=t.t("api_tokens.errors.session_required")) + return user + + +def user_uuid(user: UserContext, t) -> uuid.UUID: + """The signed-in user's id as a UUID; 403 for a principal that is not an account.""" + try: + return uuid.UUID(str(user.id)) + except (TypeError, ValueError): + raise HTTPException( + status_code=403, detail=t.t("api_tokens.errors.session_required") + ) from None + + +def http_error(t, exc: TokenError) -> HTTPException: + """Translate a service refusal into a 422 the pages can show as-is.""" + return HTTPException(status_code=422, detail=t.t(exc.key, **exc.params)) + + +ApiTokenServiceDep = Annotated[ApiTokenService, Depends(get_api_token_service)] +SettingsDep = Annotated[ApiTokensSettings, Depends(get_settings)] +SessionPrincipal = Annotated[UserContext, Depends(require_session_principal)] diff --git a/modules/api_tokens/api_tokens/endpoints/__init__.py b/modules/api_tokens/api_tokens/endpoints/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/modules/api_tokens/api_tokens/endpoints/admin_api.py b/modules/api_tokens/api_tokens/endpoints/admin_api.py new file mode 100644 index 000000000..014cebb82 --- /dev/null +++ b/modules/api_tokens/api_tokens/endpoints/admin_api.py @@ -0,0 +1,98 @@ +"""JSON API for administrators: every token in the tenant, and service keys. + +Session-only and CSRF-checked like the personal API, and gated on +``api_tokens.manage``. Reads and revokes are tenant-scoped (fail-closed): an +administrator acts on the active tenant, never one named in the URL. +""" + +from __future__ import annotations + +import uuid + +from fastapi import APIRouter, Depends, HTTPException, Request +from simple_module_db import TenantIsolationError +from simple_module_hosting.csrf import RequiresCsrf +from simple_module_hosting.i18n_deps import TranslatorDep +from simple_module_hosting.permissions import RequiresPermission + +from api_tokens.constants import FORBIDDEN_SERVICE_ROLES, PERM_MANAGE, TokenKind +from api_tokens.contracts.schemas import CreatedToken, ServiceKeyCreate, TokenView +from api_tokens.deps import ApiTokenServiceDep, SettingsDep, http_error, require_session_principal +from api_tokens.scopes import assignable_roles, assignable_scopes, check_subset +from api_tokens.service import ApiTokenService, TokenError + +router = APIRouter( + prefix="/admin", + dependencies=[ + Depends(require_session_principal), + Depends(RequiresCsrf()), + Depends(RequiresPermission(PERM_MANAGE)), + ], +) + + +async def tenant_token_views(service: ApiTokenService) -> list[TokenView]: + tokens = await service.list_tenant() + emails = await service.owner_emails(tokens) + return [ + service.to_view(t, emails.get(t.owner_user_id) if t.owner_user_id else None) for t in tokens + ] + + +@router.get("/tokens", response_model=list[TokenView]) +async def list_all(service: ApiTokenServiceDep, t: TranslatorDep) -> list[TokenView]: + try: + return await tenant_token_views(service) + except TenantIsolationError: + raise HTTPException( + status_code=409, detail=t.t("api_tokens.errors.tenant_required") + ) from None + + +@router.post("/service-keys", response_model=CreatedToken, status_code=201) +async def create_service_key( + body: ServiceKeyCreate, + request: Request, + service: ApiTokenServiceDep, + settings: SettingsDep, + t: TranslatorDep, +) -> CreatedToken: + try: + forbidden = sorted(set(body.roles) & FORBIDDEN_SERVICE_ROLES) + if forbidden: + raise TokenError("api_tokens.errors.role_forbidden", role=forbidden[0]) + roles = check_subset( + body.roles, assignable_roles(request), "api_tokens.errors.role_not_assignable" + ) + scopes = check_subset( + body.scopes, await assignable_scopes(request), "api_tokens.errors.scope_not_held" + ) + token, plaintext = await service.create( + kind=TokenKind.SERVICE, + name=body.name, + scopes=scopes, + roles=roles, + expires_in_days=body.expires_in_days, + max_lifetime_days=settings.max_lifetime_days, + ) + except TokenError as exc: + raise http_error(t, exc) from None + except TenantIsolationError: + raise HTTPException( + status_code=409, detail=t.t("api_tokens.errors.tenant_required") + ) from None + return CreatedToken(token=plaintext, item=service.to_view(token)) + + +@router.post("/tokens/{token_id}/revoke", response_model=TokenView) +async def revoke_any( + token_id: uuid.UUID, service: ApiTokenServiceDep, t: TranslatorDep +) -> TokenView: + try: + token = await service.get_in_tenant(token_id) + except TenantIsolationError: + token = None + if token is None: + raise HTTPException(status_code=404, detail=t.t("api_tokens.errors.not_found")) + await service.revoke(token) + return service.to_view(token) diff --git a/modules/api_tokens/api_tokens/endpoints/api.py b/modules/api_tokens/api_tokens/endpoints/api.py new file mode 100644 index 000000000..9e0f475bf --- /dev/null +++ b/modules/api_tokens/api_tokens/endpoints/api.py @@ -0,0 +1,88 @@ +"""JSON API for a user's own personal access tokens. + +Session-only (``require_session_principal``) and CSRF-checked on every +mutation: the pages call these with the session cookie, and a token may never +manage tokens. +""" + +from __future__ import annotations + +import uuid + +from fastapi import APIRouter, Depends, HTTPException, Request +from simple_module_db import TenantIsolationError +from simple_module_hosting.csrf import RequiresCsrf +from simple_module_hosting.i18n_deps import TranslatorDep +from simple_module_hosting.permissions import RequiresPermission + +from api_tokens.constants import PERM_USE, TokenKind +from api_tokens.contracts.schemas import CreatedToken, PersonalTokenCreate, TokenView +from api_tokens.deps import ( + ApiTokenServiceDep, + SessionPrincipal, + SettingsDep, + http_error, + require_session_principal, + user_uuid, +) +from api_tokens.scopes import assignable_scopes, check_subset +from api_tokens.service import TokenError + +router = APIRouter( + prefix="/tokens", + dependencies=[ + Depends(require_session_principal), + Depends(RequiresCsrf()), + Depends(RequiresPermission(PERM_USE)), + ], +) + + +@router.get("/", response_model=list[TokenView]) +async def list_tokens( + user: SessionPrincipal, service: ApiTokenServiceDep, t: TranslatorDep +) -> list[TokenView]: + tokens = await service.list_for_owner(user_uuid(user, t)) + return [service.to_view(token) for token in tokens] + + +@router.post("/", response_model=CreatedToken, status_code=201) +async def create_token( + body: PersonalTokenCreate, + request: Request, + user: SessionPrincipal, + service: ApiTokenServiceDep, + settings: SettingsDep, + t: TranslatorDep, +) -> CreatedToken: + owner = user_uuid(user, t) + try: + scopes = check_subset( + body.scopes, await assignable_scopes(request), "api_tokens.errors.scope_not_held" + ) + token, plaintext = await service.create( + kind=TokenKind.PERSONAL, + name=body.name, + scopes=scopes, + expires_in_days=body.expires_in_days, + max_lifetime_days=settings.max_lifetime_days, + owner_user_id=owner, + ) + except TokenError as exc: + raise http_error(t, exc) from None + except TenantIsolationError: + raise HTTPException( + status_code=409, detail=t.t("api_tokens.errors.tenant_required") + ) from None + return CreatedToken(token=plaintext, item=service.to_view(token)) + + +@router.post("/{token_id}/revoke", response_model=TokenView) +async def revoke_token( + token_id: uuid.UUID, user: SessionPrincipal, service: ApiTokenServiceDep, t: TranslatorDep +) -> TokenView: + token = await service.get_owned(token_id, user_uuid(user, t)) + if token is None: + raise HTTPException(status_code=404, detail=t.t("api_tokens.errors.not_found")) + await service.revoke(token, cross_tenant=True) + return service.to_view(token) diff --git a/modules/api_tokens/api_tokens/endpoints/views.py b/modules/api_tokens/api_tokens/endpoints/views.py new file mode 100644 index 000000000..5b0100f02 --- /dev/null +++ b/modules/api_tokens/api_tokens/endpoints/views.py @@ -0,0 +1,96 @@ +"""Inertia pages: ``/api-tokens/`` (your own tokens), ``/admin/api-tokens/`` (all). + +Both pages only render; every change goes through the JSON API with the CSRF +token handed out here, because the one response carrying a new token's +secret has to reach the page as data, not as a redirect. +""" + +from __future__ import annotations + +from fastapi import APIRouter, Depends, Request +from simple_module_db import TenantIsolationError +from simple_module_hosting.csrf import get_csrf_token +from simple_module_hosting.i18n_deps import TranslatorDep +from simple_module_hosting.inertia_deps import InertiaDep +from simple_module_hosting.permissions import RequiresPermission +from simple_module_inertia import InertiaResponse + +from api_tokens.constants import ( + API_PREFIX, + EXPIRY_PRESETS_DAYS, + PAGE_ADMIN, + PAGE_INDEX, + PERM_MANAGE, + PERM_USE, +) +from api_tokens.deps import ( + ApiTokenServiceDep, + SessionPrincipal, + SettingsDep, + require_session_principal, + user_uuid, +) +from api_tokens.endpoints.admin_api import tenant_token_views +from api_tokens.scopes import assignable_roles, assignable_scopes + +router = APIRouter( + dependencies=[Depends(require_session_principal), Depends(RequiresPermission(PERM_USE))] +) +admin_router = APIRouter( + dependencies=[Depends(require_session_principal), Depends(RequiresPermission(PERM_MANAGE))] +) + + +def _form_props(request: Request, max_days: int) -> dict: + return { + "max_lifetime_days": max_days, + "expiry_presets": [d for d in EXPIRY_PRESETS_DAYS if d <= max_days] or [max_days], + "csrf_token": get_csrf_token(request), + "api_url": API_PREFIX, + } + + +@router.get("/", response_model=None) +async def index( + request: Request, + inertia: InertiaDep, + user: SessionPrincipal, + service: ApiTokenServiceDep, + settings: SettingsDep, + t: TranslatorDep, +) -> InertiaResponse: + tokens = await service.list_for_owner(user_uuid(user, t)) + return await inertia.render( + PAGE_INDEX, + { + "tokens": [service.to_view(tok).model_dump(mode="json") for tok in tokens], + "scopes": await assignable_scopes(request), + **_form_props(request, settings.max_lifetime_days), + }, + ) + + +@admin_router.get("/", response_model=None) +async def admin_index( + request: Request, + inertia: InertiaDep, + service: ApiTokenServiceDep, + settings: SettingsDep, +) -> InertiaResponse: + try: + views = await tenant_token_views(service) + tenant_required = False + except TenantIsolationError: + # Strict tenancy with no active tenant: nothing to list, and nowhere + # to create a service key. The page says so instead of a 500. + views, tenant_required = [], True + return await inertia.render( + PAGE_ADMIN, + { + "tokens": [v.model_dump(mode="json") for v in views], + "scopes": await assignable_scopes(request), + "roles": assignable_roles(request), + "tenant_required": tenant_required, + **_form_props(request, settings.max_lifetime_days), + }, + ) diff --git a/modules/api_tokens/api_tokens/locales/en.json b/modules/api_tokens/api_tokens/locales/en.json new file mode 100644 index 000000000..8e6d4b83b --- /dev/null +++ b/modules/api_tokens/api_tokens/locales/en.json @@ -0,0 +1,94 @@ +{ + "nav": { + "api_tokens": "API tokens" + }, + "audit": { + "token": "API token" + }, + "errors": { + "session_required": "API tokens can only be managed from a signed-in browser session, not with a token.", + "scopes_required": "Choose at least one scope.", + "scope_not_held": "You cannot grant the scope \"{value}\": you do not hold it.", + "role_forbidden": "A service key cannot have the \"{role}\" role.", + "role_not_assignable": "The role \"{value}\" cannot be given to a service key.", + "lifetime_too_long": "A token can be valid for at most {days} days.", + "invalid_kind": "This kind of token cannot be created here.", + "tenant_required": "Choose an organisation before managing its tokens.", + "not_found": "Token not found." + }, + "index": { + "title": "API tokens", + "description": "Personal access tokens let scripts and tools call the API as you, limited to the scopes you choose.", + "usage": "Send a token in the Authorization header: Authorization: Bearer ." + }, + "admin": { + "title": "API tokens", + "description": "Every personal access token and service key in this organisation.", + "tenant_required": "Choose an organisation to see and create its API tokens.", + "new_service_key": "New service key" + }, + "list": { + "name": "Name", + "token": "Token", + "kind": "Kind", + "owner": "Owner", + "scopes": "Scopes", + "roles": "Roles", + "expires": "Expires", + "last_used": "Last used", + "status": "Status", + "actions": "Actions", + "never": "Never", + "revoke": "Revoke", + "empty_title": "No API tokens yet", + "empty_description": "Create a token to call the API from a script or another service.", + "scope_count_one": "{count} scope", + "scope_count_other": "{count} scopes" + }, + "kind": { + "personal": "Personal", + "service": "Service key" + }, + "status": { + "active": "Active", + "revoked": "Revoked", + "expired": "Expired" + }, + "form": { + "new_token": "New token", + "create_title": "Create a personal access token", + "create_service_title": "Create a service key", + "create_description": "The token can do only what its scopes allow, and never more than you can.", + "create_service_description": "A service key acts for no person. It gets the roles you pick, limited to the scopes you choose.", + "name": "Name", + "name_placeholder": "e.g. CI deploy", + "scopes": "Scopes", + "scopes_hint": "Only permissions you hold are listed.", + "roles": "Roles", + "roles_hint": "The administrator role cannot be given to a service key.", + "no_roles": "No assignable roles.", + "expiry": "Expires after", + "days_one": "{count} day", + "days_other": "{count} days", + "submit": "Create token", + "cancel": "Cancel" + }, + "created": { + "title": "Copy your new token", + "warning": "This is the only time the token is shown. Store it somewhere safe now — it cannot be recovered.", + "copy": "Copy", + "copied": "Copied", + "done": "Done" + }, + "revoke": { + "title": "Revoke \"{name}\"?", + "description": "Anything using this token stops working on its next request. This cannot be undone.", + "confirm": "Revoke", + "cancel": "Cancel" + }, + "toasts": { + "created": "Token created", + "revoked": "Token revoked", + "failed": "Something went wrong. Please try again." + } +} diff --git a/modules/api_tokens/api_tokens/models.py b/modules/api_tokens/api_tokens/models.py new file mode 100644 index 000000000..5e26ffbc3 --- /dev/null +++ b/modules/api_tokens/api_tokens/models.py @@ -0,0 +1,58 @@ +"""SQLModel tables for the api_tokens module. + +One table holds both credential kinds. Only ``sha256(token)`` is stored; the +secret itself is shown once, in the response that created it, and nowhere +else. ``prefix`` is a short, non-secret slice so a person can tell their +tokens apart in a list. +""" + +from __future__ import annotations + +import uuid +from datetime import datetime +from typing import ClassVar + +import sqlalchemy as sa +from fastapi_users_db_sqlalchemy.generics import GUID +from simple_module_db.base import create_module_base +from simple_module_db.mixins import AuditMixin, MultiTenantMixin +from simple_module_db.types import AwareDateTime +from sqlalchemy import Index +from sqlmodel import Field + +from api_tokens.constants import IX_TOKEN_HASH, NAME_MAX_LENGTH, TABLE_TOKEN, TokenKind + +Base = create_module_base("api_tokens") + + +class ApiToken(Base, AuditMixin, MultiTenantMixin, table=True): # ty: ignore[unsupported-base] + """A personal access token or a service API key.""" + + __tablename__ = TABLE_TOKEN + # The hash is a credential-equivalent lookup key and ``last_used_at`` is + # bumped by a Core UPDATE that never reaches the audit listener anyway; + # neither belongs in the audit trail. Create and revoke still do. + __audit_exclude_fields__: ClassVar[set[str]] = {"token_hash", "last_used_at"} + # ``token_hash`` leads, so the per-request lookup by hash alone is an + # index seek; ``tenant_id`` is in the key because every unique key on a + # tenant-scoped table must be (SM024). A SHA-256 of 32 random bytes does + # not collide, so the wider key loses nothing. + __table_args__ = (Index(IX_TOKEN_HASH, "token_hash", "tenant_id", unique=True),) + + id: uuid.UUID = Field(default_factory=uuid.uuid4, primary_key=True) + kind: str = Field(max_length=16, default=TokenKind.PERSONAL) + name: str = Field(max_length=NAME_MAX_LENGTH) + prefix: str = Field(max_length=16) + # Looked up before any tenant is known — see ``__table_args__``. + token_hash: str = Field(max_length=64) + # ``GUID``, the type ``users_user.id`` has, so the values compare equal on + # SQLite too (``sa.Uuid`` stores bare hex there — GH #339). No foreign key: + # every module owns its own ``MetaData``, so one cannot name another + # module's table. A deleted owner is handled where it matters — the + # resolver finds no account and refuses the token. + owner_user_id: uuid.UUID | None = Field(default=None, sa_type=GUID, index=True) + scopes: list[str] = Field(default_factory=list, sa_type=sa.JSON()) + roles: list[str] = Field(default_factory=list, sa_type=sa.JSON()) + expires_at: datetime = Field(sa_type=AwareDateTime()) + last_used_at: datetime | None = Field(default=None, sa_type=AwareDateTime()) + revoked_at: datetime | None = Field(default=None, sa_type=AwareDateTime()) diff --git a/modules/api_tokens/api_tokens/module.py b/modules/api_tokens/api_tokens/module.py new file mode 100644 index 000000000..8bdb76c88 --- /dev/null +++ b/modules/api_tokens/api_tokens/module.py @@ -0,0 +1,109 @@ +"""ApiTokens module: personal access tokens and service API keys.""" + +from __future__ import annotations + +import importlib +import importlib.resources +from pathlib import Path +from typing import TYPE_CHECKING + +from simple_module_core.menu import MenuItem, MenuRegistry, MenuSection +from simple_module_core.module import ModuleBase, ModuleMeta +from simple_module_core.tenancy import TenantRole, tenant_role + +from api_tokens import constants as c + +if TYPE_CHECKING: + from fastapi import APIRouter, FastAPI + from simple_module_core.audit_links import AuditLinkRegistry + from simple_module_core.permissions import PermissionRegistry + + +class ApiTokensModule(ModuleBase): + meta = ModuleMeta( + name=c.MODULE_NAME, + route_prefix=c.API_PREFIX, + view_prefix=c.VIEW_PREFIX, + admin_view_prefix=c.ADMIN_VIEW_PREFIX, + # Auth owns the resolver chain this module appends to; Users owns the + # accounts a personal token acts for; Settings stores the lifetime cap. + depends_on=[c.MODULE_AUTH, c.MODULE_USERS, c.MODULE_SETTINGS], + ) + + def register_settings(self, app: FastAPI) -> None: + from api_tokens.services import ApiTokensServices + from api_tokens.settings import ApiTokensSettings + + # SM009 is AST-based: resolve via importlib like the other + # settings-backed modules. + register_module_settings = importlib.import_module( + "settings.registration" + ).register_module_settings + register_module_settings( + app, + c.MODULE_PACKAGE, + ApiTokensSettings, + lambda s: ApiTokensServices(settings=s), + ) + + def register_routes(self, api_router: APIRouter, view_router: APIRouter) -> None: + from api_tokens.endpoints.admin_api import router as admin_api + from api_tokens.endpoints.api import router as api + from api_tokens.endpoints.views import router as views + + api_router.include_router(api) + api_router.include_router(admin_api) + view_router.include_router(views) + + def register_admin_routes(self, admin_router: APIRouter) -> None: + from api_tokens.endpoints.views import admin_router as admin_views + + admin_router.include_router(admin_views) + + def register_menu_items(self, registry: MenuRegistry) -> None: + registry.add( + MenuItem( + label=c.MENU_LABEL, + label_key=c.MENU_LABEL_KEY, + url=c.USER_URL, + icon=c.MENU_ICON, + order=c.MENU_USER_ORDER, + section=MenuSection.USER_DROPDOWN, + permissions=[c.PERM_USE], + ) + ) + registry.add( + MenuItem( + label=c.MENU_LABEL, + label_key=c.MENU_LABEL_KEY, + url=c.ADMIN_URL, + icon=c.MENU_ICON, + order=c.MENU_ADMIN_ORDER, + section=MenuSection.ADMIN_SIDEBAR, + group="Access", + group_key="ui.nav_groups.access", + permissions=[c.PERM_MANAGE], + ) + ) + + def register_permissions(self, registry: PermissionRegistry) -> None: + registry.add_group(c.PERM_GROUP, [c.PERM_USE, c.PERM_MANAGE]) + registry.map_role(c.USER_ROLE, [c.PERM_USE]) + for role in TenantRole: + registry.map_role(tenant_role(role), [c.PERM_USE]) + + def register_audit_links(self, registry: AuditLinkRegistry) -> None: + from api_tokens.audit import build_token_audit_link + + registry.register(build_token_audit_link()) + + def locale_dirs(self) -> dict[str, Path]: + base = Path(str(importlib.resources.files(__package__) / "locales")) + return {c.LOCALE_NAMESPACE: base} + + async def on_startup(self, app: FastAPI) -> None: + from api_tokens.resolver import resolve_api_token + + resolvers = app.state.auth.principal_resolvers + if resolve_api_token not in resolvers: + resolvers.append(resolve_api_token) diff --git a/modules/api_tokens/api_tokens/pages/Admin.tsx b/modules/api_tokens/api_tokens/pages/Admin.tsx new file mode 100644 index 000000000..fc5bb91fe --- /dev/null +++ b/modules/api_tokens/api_tokens/pages/Admin.tsx @@ -0,0 +1,92 @@ +import { Head, usePage } from '@inertiajs/react'; +import { keys, useT } from '@simple-module-py/i18n'; +import { InlineBanner } from '@simple-module-py/ui/components/InlineBanner'; +import { PageShell } from '@simple-module-py/ui/components/PageShell'; +import { Button } from '@simple-module-py/ui/components/ui/button'; +import { AdminLayout } from '@simple-module-py/ui/layouts/AdminLayout'; +import { Building2, Plus } from 'lucide-react'; +import { useState } from 'react'; +import { CreatedTokenDialog } from './components/CreatedTokenDialog'; +import { CreateTokenDialog } from './components/CreateTokenDialog'; +import { RevokeTokenDialog } from './components/RevokeTokenDialog'; +import { TokenTable } from './components/TokenTable'; +import { useTokenActions } from './components/useTokenActions'; +import type { CreatePayload, TokenFormProps } from './types'; + +interface Props extends TokenFormProps { + roles: string[]; + tenant_required: boolean; +} + +/** Every token in the active tenant; service keys are created here. */ +function Admin() { + const props = usePage<{ props: Props }>().props as unknown as Props; + const { t } = useT(); + const [creating, setCreating] = useState(false); + // An audit-log link lands here as ?token=; mark that row. + const highlightId = + typeof window === 'undefined' ? null : new URLSearchParams(window.location.search).get('token'); + const actions = useTokenActions({ + csrfToken: props.csrf_token, + createUrl: `${props.api_url}/admin/service-keys`, + revokeUrl: (id) => `${props.api_url}/admin/tokens/${id}/revoke`, + }); + + async function submit(payload: CreatePayload) { + const ok = await actions.create(payload); + if (ok) setCreating(false); + return ok; + } + + return ( + <> + + setCreating(true)} disabled={props.tenant_required}> + + {t(keys.api_tokens.admin.new_service_key)} + + } + > + {props.tenant_required ? ( + + ) : ( + + )} + + + + + actions.setPendingRevoke(null)} + onConfirm={actions.confirmRevoke} + /> + + ); +} + +Admin.layout = [AdminLayout]; +export default Admin; diff --git a/modules/api_tokens/api_tokens/pages/Index.tsx b/modules/api_tokens/api_tokens/pages/Index.tsx new file mode 100644 index 000000000..2a252f7eb --- /dev/null +++ b/modules/api_tokens/api_tokens/pages/Index.tsx @@ -0,0 +1,70 @@ +import { Head, usePage } from '@inertiajs/react'; +import { keys, useT } from '@simple-module-py/i18n'; +import { PageShell } from '@simple-module-py/ui/components/PageShell'; +import { Button } from '@simple-module-py/ui/components/ui/button'; +import { AuthenticatedLayout } from '@simple-module-py/ui/layouts/AuthenticatedLayout'; +import { Plus } from 'lucide-react'; +import { useState } from 'react'; +import { CreatedTokenDialog } from './components/CreatedTokenDialog'; +import { CreateTokenDialog } from './components/CreateTokenDialog'; +import { RevokeTokenDialog } from './components/RevokeTokenDialog'; +import { TokenTable } from './components/TokenTable'; +import { useTokenActions } from './components/useTokenActions'; +import type { CreatePayload, TokenFormProps } from './types'; + +/** Your own personal access tokens: list, create (secret shown once), revoke. */ +function Index() { + const props = usePage<{ props: TokenFormProps }>().props as unknown as TokenFormProps; + const { t } = useT(); + const [creating, setCreating] = useState(false); + const actions = useTokenActions({ + csrfToken: props.csrf_token, + createUrl: `${props.api_url}/tokens/`, + revokeUrl: (id) => `${props.api_url}/tokens/${id}/revoke`, + }); + + async function submit(payload: CreatePayload) { + const ok = await actions.create(payload); + if (ok) setCreating(false); + return ok; + } + + return ( + <> + + setCreating(true)} disabled={props.scopes.length === 0}> + + {t(keys.api_tokens.form.new_token)} + + } + > +

{t(keys.api_tokens.index.usage)}

+ + + + + + actions.setPendingRevoke(null)} + onConfirm={actions.confirmRevoke} + /> + + ); +} + +Index.layout = [AuthenticatedLayout]; +export default Index; diff --git a/modules/api_tokens/api_tokens/pages/api.ts b/modules/api_tokens/api_tokens/pages/api.ts new file mode 100644 index 000000000..849cdf64c --- /dev/null +++ b/modules/api_tokens/api_tokens/pages/api.ts @@ -0,0 +1,42 @@ +/** + * JSON calls to the api_tokens endpoints. + * + * Plain `fetch`, not Inertia's router: creating a token must hand the page + * its secret as data, once, which a redirect cannot carry. Every mutation + * echoes the session's CSRF token (`RequiresCsrf` on the server). + */ + +export interface ApiResult { + ok: boolean; + data: T | null; + /** The server's message when it sent a plain string; null otherwise. */ + detail: string | null; +} + +export async function postJson( + url: string, + csrfToken: string, + body?: unknown, +): Promise> { + let response: Response; + try { + response = await fetch(url, { + method: 'POST', + credentials: 'same-origin', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json', + 'X-CSRF-Token': csrfToken, + }, + body: body === undefined ? undefined : JSON.stringify(body), + }); + } catch { + return { ok: false, data: null, detail: null }; + } + const json = await response.json().catch(() => null); + if (!response.ok) { + const detail = json && typeof json.detail === 'string' ? json.detail : null; + return { ok: false, data: null, detail }; + } + return { ok: true, data: json as T, detail: null }; +} diff --git a/modules/api_tokens/api_tokens/pages/components/CreateTokenDialog.tsx b/modules/api_tokens/api_tokens/pages/components/CreateTokenDialog.tsx new file mode 100644 index 000000000..bf74cb43a --- /dev/null +++ b/modules/api_tokens/api_tokens/pages/components/CreateTokenDialog.tsx @@ -0,0 +1,186 @@ +import { keys, useT } from '@simple-module-py/i18n'; +import { Button } from '@simple-module-py/ui/components/ui/button'; +import { Checkbox } from '@simple-module-py/ui/components/ui/checkbox'; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@simple-module-py/ui/components/ui/dialog'; +import { Input } from '@simple-module-py/ui/components/ui/input'; +import { Label } from '@simple-module-py/ui/components/ui/label'; +import { NativeSelect, NativeSelectOption } from '@simple-module-py/ui/components/ui/native-select'; +import { type FormEvent, useId, useState } from 'react'; +import type { CreatePayload } from '../types'; + +interface Props { + open: boolean; + onOpenChange: (open: boolean) => void; + /** `service` adds the role picker and changes the copy. */ + variant: 'personal' | 'service'; + scopes: string[]; + roles?: string[]; + expiryPresets: number[]; + busy: boolean; + /** Resolves true when the token was created, so the form can reset. */ + onSubmit: (payload: CreatePayload) => Promise; +} + +function toggle(list: string[], value: string, on: boolean): string[] { + return on ? [...list, value] : list.filter((v) => v !== value); +} + +function CheckList({ + idPrefix, + options, + selected, + onChange, +}: { + idPrefix: string; + options: string[]; + selected: string[]; + onChange: (next: string[]) => void; +}) { + return ( +
+ {options.map((option) => { + const id = `${idPrefix}-${option}`; + return ( +
+ onChange(toggle(selected, option, on === true))} + /> + +
+ ); + })} +
+ ); +} + +export function CreateTokenDialog({ + open, + onOpenChange, + variant, + scopes, + roles = [], + expiryPresets, + busy, + onSubmit, +}: Props) { + const { t } = useT(); + const id = useId(); + const [name, setName] = useState(''); + const [chosenScopes, setChosenScopes] = useState([]); + const [chosenRoles, setChosenRoles] = useState([]); + const [days, setDays] = useState(expiryPresets[Math.min(1, expiryPresets.length - 1)]); + const isService = variant === 'service'; + + async function handleSubmit(event: FormEvent) { + event.preventDefault(); + const payload: CreatePayload = { + name: name.trim(), + scopes: chosenScopes, + expires_in_days: days, + ...(isService ? { roles: chosenRoles } : {}), + }; + if (await onSubmit(payload)) { + setName(''); + setChosenScopes([]); + setChosenRoles([]); + } + } + + const canSubmit = name.trim().length > 0 && chosenScopes.length > 0 && !busy; + + return ( + + +
+ + + {isService + ? t(keys.api_tokens.form.create_service_title) + : t(keys.api_tokens.form.create_title)} + + + {isService + ? t(keys.api_tokens.form.create_service_description) + : t(keys.api_tokens.form.create_description)} + + + +
+ + setName(e.target.value)} + required + /> +
+ +
+ +

{t(keys.api_tokens.form.scopes_hint)}

+ +
+ + {isService && ( +
+ +

{t(keys.api_tokens.form.roles_hint)}

+ {roles.length > 0 ? ( + + ) : ( +

{t(keys.api_tokens.form.no_roles)}

+ )} +
+ )} + +
+ + setDays(Number(e.target.value))} + > + {expiryPresets.map((preset) => ( + + {t(keys.api_tokens.form.days, { count: preset })} + + ))} + +
+ + + + + +
+ + + ); +} diff --git a/modules/api_tokens/api_tokens/pages/components/CreatedTokenDialog.tsx b/modules/api_tokens/api_tokens/pages/components/CreatedTokenDialog.tsx new file mode 100644 index 000000000..23b471edb --- /dev/null +++ b/modules/api_tokens/api_tokens/pages/components/CreatedTokenDialog.tsx @@ -0,0 +1,64 @@ +import { keys, useT } from '@simple-module-py/i18n'; +import { Button } from '@simple-module-py/ui/components/ui/button'; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@simple-module-py/ui/components/ui/dialog'; +import { Check, Copy } from 'lucide-react'; +import { useState } from 'react'; + +interface Props { + /** The plaintext token; the dialog is open while it is set. */ + token: string | null; + onClose: () => void; +} + +/** Shows a new token's secret — the only time anything ever will. */ +export function CreatedTokenDialog({ token, onClose }: Props) { + const { t } = useT(); + const [copied, setCopied] = useState(false); + + async function copy() { + if (!token) return; + try { + await navigator.clipboard.writeText(token); + setCopied(true); + } catch { + setCopied(false); + } + } + + function close() { + setCopied(false); + onClose(); + } + + return ( + !open && close()}> + + + {t(keys.api_tokens.created.title)} + {t(keys.api_tokens.created.warning)} + +
+ + {token} + + +
+ + + + + + ); +} diff --git a/modules/api_tokens/api_tokens/pages/components/RevokeTokenDialog.tsx b/modules/api_tokens/api_tokens/pages/components/RevokeTokenDialog.tsx new file mode 100644 index 000000000..644e5cbaf --- /dev/null +++ b/modules/api_tokens/api_tokens/pages/components/RevokeTokenDialog.tsx @@ -0,0 +1,29 @@ +import { keys, useT } from '@simple-module-py/i18n'; +import { ConfirmActionDialog } from '@simple-module-py/ui/components/ConfirmActionDialog'; +import { KeyRound } from 'lucide-react'; +import type { ApiToken } from '../types'; + +interface Props { + token: ApiToken | null; + busy: boolean; + onCancel: () => void; + onConfirm: () => void; +} + +export function RevokeTokenDialog({ token, busy, onCancel, onConfirm }: Props) { + const { t } = useT(); + return ( + !open && onCancel()} + tone="destructive" + icon={KeyRound} + title={t(keys.api_tokens.revoke.title, { name: token?.name ?? '' })} + description={t(keys.api_tokens.revoke.description)} + confirmLabel={t(keys.api_tokens.revoke.confirm)} + cancelLabel={t(keys.api_tokens.revoke.cancel)} + busy={busy} + onConfirm={onConfirm} + /> + ); +} diff --git a/modules/api_tokens/api_tokens/pages/components/TokenTable.tsx b/modules/api_tokens/api_tokens/pages/components/TokenTable.tsx new file mode 100644 index 000000000..da8a30786 --- /dev/null +++ b/modules/api_tokens/api_tokens/pages/components/TokenTable.tsx @@ -0,0 +1,117 @@ +import { keys, useT } from '@simple-module-py/i18n'; +import { EmptyState } from '@simple-module-py/ui/components/EmptyState'; +import { Badge } from '@simple-module-py/ui/components/ui/badge'; +import { Button } from '@simple-module-py/ui/components/ui/button'; +import { + Table, + TableBody, + TableCell, + TableHead, + TableHeader, + TableRow, +} from '@simple-module-py/ui/components/ui/table'; +import { formatDayMonthYear } from '@simple-module-py/ui/lib/date-format'; +import { KeyRound } from 'lucide-react'; +import { type ApiToken, type TokenStatus, tokenStatus } from '../types'; + +interface Props { + tokens: ApiToken[]; + /** Admin view: show kind and owner columns. */ + showOwner?: boolean; + highlightId?: string | null; + onRevoke: (token: ApiToken) => void; +} + +const STATUS_VARIANT: Record = { + active: 'secondary', + revoked: 'destructive', + expired: 'outline', +}; + +export function TokenTable({ tokens, showOwner = false, highlightId, onRevoke }: Props) { + const { t } = useT(); + const statusLabel: Record = { + active: t(keys.api_tokens.status.active), + revoked: t(keys.api_tokens.status.revoked), + expired: t(keys.api_tokens.status.expired), + }; + const never = t(keys.api_tokens.list.never); + + if (tokens.length === 0) { + return ( + + ); + } + + return ( +
+ + + + {t(keys.api_tokens.list.name)} + {showOwner && {t(keys.api_tokens.list.kind)}} + {showOwner && {t(keys.api_tokens.list.owner)}} + {t(keys.api_tokens.list.scopes)} + {t(keys.api_tokens.list.expires)} + {t(keys.api_tokens.list.last_used)} + {t(keys.api_tokens.list.status)} + {t(keys.api_tokens.list.actions)} + + + + {tokens.map((token) => { + const status = tokenStatus(token); + return ( + + +
{token.name}
+ {token.prefix}… + + {showOwner && ( + + {token.kind === 'service' + ? t(keys.api_tokens.kind.service) + : t(keys.api_tokens.kind.personal)} + + )} + {showOwner && ( + + {token.owner_email ?? token.roles.join(', ')} + + )} + + + {t(keys.api_tokens.list.scope_count, { count: token.scopes.length })} + + + + {formatDayMonthYear(token.expires_at, never)} + + + {formatDayMonthYear(token.last_used_at, never)} + + + {statusLabel[status]} + + + {status === 'active' && ( + + )} + + + ); + })} + +
+
+ ); +} diff --git a/modules/api_tokens/api_tokens/pages/components/useTokenActions.ts b/modules/api_tokens/api_tokens/pages/components/useTokenActions.ts new file mode 100644 index 000000000..5b34752cb --- /dev/null +++ b/modules/api_tokens/api_tokens/pages/components/useTokenActions.ts @@ -0,0 +1,62 @@ +import { router } from '@inertiajs/react'; +import { keys, useT } from '@simple-module-py/i18n'; +import { useState } from 'react'; +import { toast } from 'sonner'; +import { postJson } from '../api'; +import type { ApiToken, CreatePayload } from '../types'; + +interface Options { + csrfToken: string; + createUrl: string; + revokeUrl: (id: string) => string; +} + +interface Created { + token: string; + item: ApiToken; +} + +/** Create / revoke through the JSON API, then reload the page's `tokens` prop. */ +export function useTokenActions({ csrfToken, createUrl, revokeUrl }: Options) { + const { t } = useT(); + const [created, setCreated] = useState(null); + const [pendingRevoke, setPendingRevoke] = useState(null); + const [busy, setBusy] = useState(false); + + async function create(payload: CreatePayload): Promise { + setBusy(true); + const result = await postJson(createUrl, csrfToken, payload); + setBusy(false); + if (!result.ok || !result.data) { + toast.error(result.detail ?? t(keys.api_tokens.toasts.failed)); + return false; + } + setCreated(result.data.token); + router.reload({ only: ['tokens'] }); + return true; + } + + async function confirmRevoke() { + if (!pendingRevoke) return; + setBusy(true); + const result = await postJson(revokeUrl(pendingRevoke.id), csrfToken); + setBusy(false); + setPendingRevoke(null); + if (!result.ok) { + toast.error(result.detail ?? t(keys.api_tokens.toasts.failed)); + return; + } + toast.success(t(keys.api_tokens.toasts.revoked)); + router.reload({ only: ['tokens'] }); + } + + return { + busy, + created, + clearCreated: () => setCreated(null), + create, + pendingRevoke, + setPendingRevoke, + confirmRevoke, + }; +} diff --git a/modules/api_tokens/api_tokens/pages/types.ts b/modules/api_tokens/api_tokens/pages/types.ts new file mode 100644 index 000000000..9697e24bd --- /dev/null +++ b/modules/api_tokens/api_tokens/pages/types.ts @@ -0,0 +1,40 @@ +export interface ApiToken { + id: string; + kind: 'personal' | 'service'; + name: string; + prefix: string; + scopes: string[]; + roles: string[]; + owner_user_id: string | null; + owner_email: string | null; + tenant_id: string | null; + created_at: string | null; + expires_at: string; + last_used_at: string | null; + revoked_at: string | null; +} + +/** Props both pages receive for their create form. */ +export interface TokenFormProps { + tokens: ApiToken[]; + scopes: string[]; + max_lifetime_days: number; + expiry_presets: number[]; + csrf_token: string; + api_url: string; +} + +export interface CreatePayload { + name: string; + scopes: string[]; + roles?: string[]; + expires_in_days: number; +} + +export type TokenStatus = 'active' | 'revoked' | 'expired'; + +export function tokenStatus(token: ApiToken, now: number = Date.now()): TokenStatus { + if (token.revoked_at) return 'revoked'; + if (new Date(token.expires_at).getTime() <= now) return 'expired'; + return 'active'; +} diff --git a/modules/api_tokens/api_tokens/py.typed b/modules/api_tokens/api_tokens/py.typed new file mode 100644 index 000000000..e69de29bb diff --git a/modules/api_tokens/api_tokens/resolver.py b/modules/api_tokens/api_tokens/resolver.py new file mode 100644 index 000000000..ad204c35e --- /dev/null +++ b/modules/api_tokens/api_tokens/resolver.py @@ -0,0 +1,145 @@ +"""The principal resolver: ``Authorization: Bearer smpat_…`` / ``smsk_…`` → principal. + +Appended to ``app.state.auth.principal_resolvers`` at startup. The ``users`` +and ``keycloak`` providers return ``None`` for these prefixes, which is what +hands the request to this resolver. + +Per request: a regex check, one indexed lookup by hash (no cache, so a revoke +is effective on the very next request), and for a personal token one primary +key read of the owner with their roles. Everything that is not a usable token +— unknown, revoked, expired, an inactive or disabled owner — is ``None``, +which the middleware turns into a 401. +""" + +from __future__ import annotations + +import logging +from datetime import UTC, datetime, timedelta +from typing import Any + +from auth.contracts.schemas import UserContext +from auth.contracts.tokens import AUTH_KIND_PAT, AUTH_KIND_SERVICE, is_api_token +from simple_module_db import ALL_TENANTS_OPTION +from sqlalchemy import or_, select, update +from sqlalchemy.orm import noload, selectinload +from starlette.requests import Request + +from api_tokens.constants import ( + FORBIDDEN_SERVICE_ROLES, + LAST_USED_WRITE_INTERVAL_SECONDS, + TokenKind, +) +from api_tokens.models import ApiToken +from api_tokens.tokens import hash_token, is_well_formed + +logger = logging.getLogger(__name__) + +SERVICE_PRINCIPAL_PREFIX = "svc:" +"""Service principals are ``svc:``: not a user UUID, so they hold no +direct grants and can never be mistaken for an account.""" + + +def _bearer(request: Request) -> str | None: + header = request.headers.get("authorization", "") + if not header.startswith("Bearer "): + return None + credential = header[7:] + if not is_api_token(credential) or not is_well_formed(credential): + return None + return credential + + +def _usable(token: ApiToken, now: datetime) -> bool: + return token.revoked_at is None and token.expires_at > now + + +async def _personal(db: Any, token: ApiToken) -> UserContext | None: + from users.models import User + + if token.owner_user_id is None: + return None + stmt = ( + select(User) + .where(User.id == token.owner_user_id) + .options(selectinload(User.roles), noload(User.oauth_accounts)) + ) + user = (await db.execute(stmt)).scalar_one_or_none() + if user is None or not user.is_active or user.disabled_at is not None: + return None + return UserContext.from_user(user, auth_kind=AUTH_KIND_PAT, scopes=list(token.scopes)) + + +def _service(token: ApiToken) -> UserContext: + return UserContext( + id=f"{SERVICE_PRINCIPAL_PREFIX}{token.id}", + email="", + name=token.name, + # Refused at creation; dropped here too so a hand-edited row cannot + # turn a service key into an admin. + roles=[r for r in token.roles if r not in FORBIDDEN_SERVICE_ROLES], + tenant_id=token.tenant_id, + auth_kind=AUTH_KIND_SERVICE, + scopes=list(token.scopes), + ) + + +async def touch_last_used(app: Any, token: ApiToken, now: datetime) -> None: + """Record use at most once per interval, in its own short transaction. + + The ``WHERE`` repeats the staleness test, so concurrent requests on one + token race to a single write rather than one each. ``updated_at`` is set + to itself: it would otherwise be bumped by its ``onupdate`` and stop + meaning "last edited". A failure here never fails the request. + """ + cutoff = now - timedelta(seconds=LAST_USED_WRITE_INTERVAL_SECONDS) + if token.last_used_at is not None and token.last_used_at > cutoff: + return + stmt = ( + update(ApiToken) + .where( + ApiToken.id == token.id, + or_(ApiToken.last_used_at.is_(None), ApiToken.last_used_at <= cutoff), + ) + .values(last_used_at=now, updated_at=ApiToken.updated_at) + .execution_options(**{ALL_TENANTS_OPTION: True}) + ) + try: + async with app.state.sm.db.session_factory() as db: + await db.execute(stmt) + await db.commit() + except Exception: + logger.warning("api_tokens: could not record last use of %s", token.id, exc_info=True) + + +async def resolve_api_token(request: Request) -> UserContext | None: + """``PrincipalResolver`` for ``api_tokens`` credentials. Never raises.""" + credential = _bearer(request) + if credential is None: + return None + now = datetime.now(UTC) + app = request.app + try: + async with app.state.sm.db.session_factory() as db: + stmt = ( + select(ApiToken) + .where(ApiToken.token_hash == hash_token(credential)) + .execution_options(**{ALL_TENANTS_OPTION: True}) + ) + token = (await db.execute(stmt)).scalar_one_or_none() + if token is None or not _usable(token, now): + return None + if token.kind == TokenKind.PERSONAL: + principal = await _personal(db, token) + elif token.kind == TokenKind.SERVICE: + principal = _service(token) + else: + principal = None + except Exception: + logger.exception("api_tokens: token lookup failed; treating as unauthenticated") + return None + if principal is not None: + await touch_last_used(app, token, now) + return principal + + +__all__ = ["SERVICE_PRINCIPAL_PREFIX", "resolve_api_token", "touch_last_used"] diff --git a/modules/api_tokens/api_tokens/scopes.py b/modules/api_tokens/api_tokens/scopes.py new file mode 100644 index 000000000..d295b8ddc --- /dev/null +++ b/modules/api_tokens/api_tokens/scopes.py @@ -0,0 +1,58 @@ +"""What a creator may put on a token: scopes they hold, roles that are safe. + +A scope must be a registered permission the creator holds *right now*, so a +token can never be created wider than the person creating it. That is the +check at creation; the cap the token carries is then applied on every +request on top of the owner's live grants (``apply_scope_cap``), so a later +demotion shrinks the token without anyone touching it. +""" + +from __future__ import annotations + +from collections.abc import Iterable + +from fastapi import Request +from simple_module_core.permissions import WILDCARD +from simple_module_core.tenancy import is_tenant_role +from simple_module_hosting.permissions import ensure_resolved_permissions + +from api_tokens.constants import FORBIDDEN_SERVICE_ROLES +from api_tokens.service import TokenError + + +def _registry(request: Request): + return request.app.state.sm.permissions + + +async def assignable_scopes(request: Request) -> list[str]: + """Every registered permission the current principal holds, wildcard expanded.""" + registered = _registry(request).all_permissions + held = await ensure_resolved_permissions(request) + if WILDCARD in held: + return sorted(p for p in registered if p != WILDCARD) + return sorted(p for p in registered if p in held and p != WILDCARD) + + +def assignable_roles(request: Request) -> list[str]: + """Platform roles a service key may carry: never ``admin``, never ``tenant:*``. + + Tenant roles come from a membership, which a service key does not have. + """ + return sorted( + role + for role in _registry(request).role_map + if role not in FORBIDDEN_SERVICE_ROLES and not is_tenant_role(role) + ) + + +def check_subset(requested: Iterable[str], allowed: Iterable[str], error_key: str) -> list[str]: + """``requested`` deduplicated and sorted, or :class:`TokenError` naming the first stray.""" + allowed_set = set(allowed) + wanted = sorted(set(requested)) + for item in wanted: + if item not in allowed_set: + raise TokenError(error_key, value=item) + return wanted + + +__all__ = ["assignable_roles", "assignable_scopes", "check_subset"] diff --git a/modules/api_tokens/api_tokens/service.py b/modules/api_tokens/api_tokens/service.py new file mode 100644 index 000000000..97641de78 --- /dev/null +++ b/modules/api_tokens/api_tokens/service.py @@ -0,0 +1,151 @@ +"""Creating, listing and revoking API tokens. + +Authorisation decisions — who may create what, which scopes a creator may +hand out — are made by the caller (``api_tokens.scopes`` and the endpoints); +this layer enforces only the invariants every token must satisfy wherever it +came from: a non-empty scope list, no ``admin`` role on a service key, an +expiry within ``max_lifetime_days``. +""" + +from __future__ import annotations + +import uuid +from collections.abc import Sequence +from datetime import UTC, datetime, timedelta + +from simple_module_db import ALL_TENANTS_OPTION, all_tenants +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from api_tokens.constants import FORBIDDEN_SERVICE_ROLES, TokenKind +from api_tokens.contracts.schemas import TokenView +from api_tokens.models import ApiToken +from api_tokens.tokens import generate + + +class TokenError(Exception): + """A request the service refuses. ``key`` is an ``api_tokens.errors.*`` key.""" + + def __init__(self, key: str, **params: object) -> None: + super().__init__(key) + self.key = key + self.params = params + + +def _utcnow() -> datetime: + return datetime.now(UTC) + + +class ApiTokenService: + def __init__(self, db: AsyncSession) -> None: + self.db = db + + async def create( + self, + *, + kind: TokenKind, + name: str, + scopes: Sequence[str], + expires_in_days: int, + max_lifetime_days: int, + owner_user_id: uuid.UUID | None = None, + roles: Sequence[str] = (), + ) -> tuple[ApiToken, str]: + """Persist a new token; return it with its plaintext (shown once).""" + if not scopes: + raise TokenError("api_tokens.errors.scopes_required") + if not 1 <= expires_in_days <= max_lifetime_days: + raise TokenError("api_tokens.errors.lifetime_too_long", days=max_lifetime_days) + if kind is TokenKind.PERSONAL: + if owner_user_id is None or roles: + raise TokenError("api_tokens.errors.invalid_kind") + elif owner_user_id is not None: + raise TokenError("api_tokens.errors.invalid_kind") + forbidden = sorted(set(roles) & FORBIDDEN_SERVICE_ROLES) + if forbidden: + raise TokenError("api_tokens.errors.role_forbidden", role=forbidden[0]) + + new = generate(kind) + token = ApiToken( + kind=kind, + name=name, + prefix=new.prefix, + token_hash=new.token_hash, + owner_user_id=owner_user_id, + scopes=sorted(set(scopes)), + roles=sorted(set(roles)), + expires_at=_utcnow() + timedelta(days=expires_in_days), + ) + self.db.add(token) + await self.db.flush() + return token, new.plaintext + + async def list_for_owner(self, owner_user_id: uuid.UUID) -> list[ApiToken]: + """Every personal token *owner_user_id* holds, in any tenant. + + Across tenants on purpose: a token is its owner's, and one created + while another organisation was active must still be listable — and + revocable — from wherever the owner is now. + """ + stmt = ( + select(ApiToken) + .where(ApiToken.owner_user_id == owner_user_id, ApiToken.kind == TokenKind.PERSONAL) + .order_by(ApiToken.created_at.desc()) + .execution_options(**{ALL_TENANTS_OPTION: True}) + ) + return list((await self.db.execute(stmt)).scalars()) + + async def list_tenant(self) -> list[ApiToken]: + """Every token in the active tenant (tenant-filtered, fail-closed).""" + stmt = select(ApiToken).order_by(ApiToken.created_at.desc()) + return list((await self.db.execute(stmt)).scalars()) + + async def get_owned(self, token_id: uuid.UUID, owner_user_id: uuid.UUID) -> ApiToken | None: + stmt = ( + select(ApiToken) + .where( + ApiToken.id == token_id, + ApiToken.owner_user_id == owner_user_id, + ApiToken.kind == TokenKind.PERSONAL, + ) + .execution_options(**{ALL_TENANTS_OPTION: True}) + ) + return (await self.db.execute(stmt)).scalar_one_or_none() + + async def get_in_tenant(self, token_id: uuid.UUID) -> ApiToken | None: + stmt = select(ApiToken).where(ApiToken.id == token_id) + return (await self.db.execute(stmt)).scalar_one_or_none() + + async def revoke(self, token: ApiToken, *, cross_tenant: bool = False) -> ApiToken: + """Mark *token* revoked. Idempotent: a second revoke keeps the first time. + + ``cross_tenant`` is for an owner revoking their own token from another + tenant; the flush runs inside ``all_tenants()`` so the tenant guard + lets the row be written. Flushed here, not at commit, for that reason. + """ + if token.revoked_at is None: + token.revoked_at = _utcnow() + if cross_tenant: + with all_tenants(): + await self.db.flush() + else: + await self.db.flush() + return token + + async def owner_emails(self, tokens: Sequence[ApiToken]) -> dict[uuid.UUID, str]: + from users.models import User + + ids = {t.owner_user_id for t in tokens if t.owner_user_id is not None} + if not ids: + return {} + rows = (await self.db.execute(select(User.id, User.email).where(User.id.in_(ids)))).all() + return {row[0]: row[1] for row in rows} + + @staticmethod + def to_view(token: ApiToken, owner_email: str | None = None) -> TokenView: + view = TokenView.model_validate(token) + view.owner_email = owner_email + return view + + +__all__ = ["ApiTokenService", "TokenError"] diff --git a/modules/api_tokens/api_tokens/services.py b/modules/api_tokens/api_tokens/services.py new file mode 100644 index 000000000..7cf67f492 --- /dev/null +++ b/modules/api_tokens/api_tokens/services.py @@ -0,0 +1,14 @@ +"""Module-scoped state container, stored as ``app.state.api_tokens``.""" + +from __future__ import annotations + +from dataclasses import dataclass + +from api_tokens.settings import ApiTokensSettings + + +@dataclass +class ApiTokensServices: + """api_tokens singletons. ``settings`` is swapped on a settings save.""" + + settings: ApiTokensSettings diff --git a/modules/api_tokens/api_tokens/settings.py b/modules/api_tokens/api_tokens/settings.py new file mode 100644 index 000000000..be9b58182 --- /dev/null +++ b/modules/api_tokens/api_tokens/settings.py @@ -0,0 +1,17 @@ +"""api_tokens settings — DB-backed via ``register_module_settings``.""" + +from __future__ import annotations + +from pydantic import Field +from pydantic_settings import SettingsConfigDict +from simple_module_core.settings_base import DbBackedSettings + + +class ApiTokensSettings(DbBackedSettings): + """Configuration for personal access tokens and service keys.""" + + # ``DbBackedSettings`` so the DB is the only source (GH #283). + model_config = SettingsConfigDict(extra="ignore") + + max_lifetime_days: int = Field(default=365, ge=1, le=3650) + """The longest expiry a new token may be given. Existing tokens keep theirs.""" diff --git a/modules/api_tokens/api_tokens/tokens.py b/modules/api_tokens/api_tokens/tokens.py new file mode 100644 index 000000000..4585f4e79 --- /dev/null +++ b/modules/api_tokens/api_tokens/tokens.py @@ -0,0 +1,83 @@ +"""Token format, generation and hashing — no I/O. + +A token is ````: ``smpat_`` for a personal access token, +``smsk_`` for a service key, then 32 bytes from :func:`secrets.token_bytes` +in base62, left-padded to a fixed width. Only ``sha256(token)`` is stored. + +SHA-256 rather than a slow password hash is deliberate: the input carries 256 +bits of entropy, so there is nothing to brute-force, and the hash is computed +on every authenticated request. +""" + +from __future__ import annotations + +import hashlib +import re +import secrets +from typing import NamedTuple + +from auth.contracts.tokens import API_TOKEN_PREFIXES + +from api_tokens.constants import ( + DISPLAY_PREFIX_LENGTH, + KIND_PREFIX, + SECRET_BYTES, + SECRET_LENGTH, + TokenKind, +) + +_ALPHABET = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" +_FORMAT = re.compile( + "^(?:" + + "|".join(re.escape(p) for p in API_TOKEN_PREFIXES) + + f")[0-9A-Za-z]{{{SECRET_LENGTH}}}$" +) + + +class NewToken(NamedTuple): + """A freshly minted token. ``plaintext`` must reach the caller exactly once.""" + + plaintext: str + token_hash: str + prefix: str + + +def base62(data: bytes) -> str: + """Encode *data* in base62, left-padded with ``0`` to :data:`SECRET_LENGTH`.""" + number = int.from_bytes(data, "big") + out: list[str] = [] + while number: + number, rem = divmod(number, 62) + out.append(_ALPHABET[rem]) + return "".join(reversed(out)).rjust(SECRET_LENGTH, _ALPHABET[0]) + + +def hash_token(plaintext: str) -> str: + """The stored form of a token: lowercase hex SHA-256 of the whole string.""" + return hashlib.sha256(plaintext.encode("utf-8")).hexdigest() + + +def is_well_formed(candidate: str) -> bool: + """Whether *candidate* has the exact shape of a token we could have issued. + + Checked before any database read, so junk in an ``Authorization`` header + costs a regex match and nothing else. + """ + return _FORMAT.fullmatch(candidate) is not None + + +def display_prefix(plaintext: str) -> str: + """The non-secret slice shown in lists: the kind prefix plus 8 characters.""" + for prefix in API_TOKEN_PREFIXES: + if plaintext.startswith(prefix): + return plaintext[: len(prefix) + DISPLAY_PREFIX_LENGTH] + raise ValueError("not an API token") + + +def generate(kind: TokenKind) -> NewToken: + """Mint a token of *kind*.""" + plaintext = KIND_PREFIX[kind] + base62(secrets.token_bytes(SECRET_BYTES)) + return NewToken(plaintext, hash_token(plaintext), display_prefix(plaintext)) + + +__all__ = ["NewToken", "base62", "display_prefix", "generate", "hash_token", "is_well_formed"] diff --git a/modules/api_tokens/package.json b/modules/api_tokens/package.json new file mode 100644 index 000000000..babfb9658 --- /dev/null +++ b/modules/api_tokens/package.json @@ -0,0 +1,19 @@ +{ + "name": "@simple-module-py/api-tokens", + "version": "0.1.0", + "private": true, + "description": "Frontend assets for the ApiTokens module", + "peerDependencies": { + "react": "^19.0.0", + "react-dom": "^19.0.0", + "@inertiajs/react": "^3.7.0", + "@simple-module-py/ui": "*" + }, + "devDependencies": { + "@simple-module-py/tsconfig": "*" + }, + "dependencies": { + "lucide-react": "^1.43.0", + "sonner": "^2.0.8" + } +} diff --git a/modules/api_tokens/pyproject.toml b/modules/api_tokens/pyproject.toml new file mode 100644 index 000000000..6e7b65f40 --- /dev/null +++ b/modules/api_tokens/pyproject.toml @@ -0,0 +1,66 @@ +[project] +name = "simple_module_api_tokens" +version = "0.0.36" +description = "Personal access tokens and service API keys for simple_module" +readme = "README.md" +license = "MIT" +license-files = ["LICENSE"] +requires-python = ">=3.12" +authors = [{ name = "Anto Subash", email = "antosubash@live.com" }] +keywords = ["simple-module", "api-tokens", "personal-access-tokens", "api-keys", "fastapi"] +classifiers = [ + "Development Status :: 3 - Alpha", + "Framework :: FastAPI", + "Intended Audience :: Developers", + "License :: OSI Approved :: MIT License", + "Operating System :: OS Independent", + "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3.12", + "Topic :: Internet :: WWW/HTTP", + "Topic :: Software Development :: Libraries :: Application Frameworks", + "Typing :: Typed", +] +dependencies = [ + "simple_module_core==0.0.36", + "simple_module_db==0.0.36", + "simple_module_hosting==0.0.36", + "simple_module_inertia==0.0.36", + "simple_module_auth==0.0.36", + "simple_module_settings==0.0.36", + # A personal token acts for a ``users`` account: the resolver loads the + # owner row and the table's FK points at ``users_user``. + "simple_module_users==0.0.36", +] + +[project.entry-points.simple_module] +api_tokens = "api_tokens.module:ApiTokensModule" + +[project.urls] +Homepage = "https://github.com/antosubash/simple_module_python" +Repository = "https://github.com/antosubash/simple_module_python" +Issues = "https://github.com/antosubash/simple_module_python/issues" +Changelog = "https://github.com/antosubash/simple_module_python/blob/main/CHANGELOG.md" + +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +# The distribution name (simple_module_api_tokens) doesn't match the package +# directory (api_tokens), so hatchling can't infer what to ship — without +# this the wheel builds empty and the entry point fails to import. +[tool.hatch.build.targets.wheel] +packages = ["api_tokens"] + +# Ship the module-root package.json inside the wheel so the host can +# discover JS deps via importlib.resources after a pip install. +[tool.hatch.build.targets.wheel.force-include] +"package.json" = "api_tokens/package.json" + +[tool.uv.sources] +simple_module_core = { workspace = true } +simple_module_db = { workspace = true } +simple_module_hosting = { workspace = true } +simple_module_inertia = { workspace = true } +simple_module_auth = { workspace = true } +simple_module_settings = { workspace = true } +simple_module_users = { workspace = true } diff --git a/modules/api_tokens/tests/_api_tokens_support.py b/modules/api_tokens/tests/_api_tokens_support.py new file mode 100644 index 000000000..48e2a746f --- /dev/null +++ b/modules/api_tokens/tests/_api_tokens_support.py @@ -0,0 +1,87 @@ +"""Shared helpers for the api_tokens tests (fixtures live in ``conftest.py``).""" + +from __future__ import annotations + +import uuid +from dataclasses import dataclass +from typing import Any + +import httpx +from simple_module_core.tenancy import TenantRole + +CSRF = "csrf-test-token" +CSRF_HEADERS = {"X-CSRF-Token": CSRF} +INERTIA_HEADERS = {"X-Inertia": "true", "Accept": "application/json"} +PAT_URL = "/api/api_tokens/tokens/" +SERVICE_URL = "/api/api_tokens/admin/service-keys" +FLAGS_URL = "/api/feature_flags/" + + +@dataclass +class Actor: + client: httpx.AsyncClient + user_id: str + tenant_id: str + + +async def join_tenant(app: Any, user_id: str, email: str, tenant_id: str | None = None) -> str: + """Make *user_id* an owner of a new tenant (or of *tenant_id*); return its id.""" + from tenants.models import Membership, Tenant + from tenants.resolver import forget + + async with app.state.sm.db.session_factory() as session: + if tenant_id is None: + tenant = Tenant(slug=f"t-{uuid.uuid4().hex[:12]}", name="Test Org") + session.add(tenant) + await session.flush() + tenant_id = tenant.id + session.add( + Membership(tenant_id=tenant_id, user_id=user_id, role=TenantRole.OWNER, email=email) + ) + await session.commit() + forget(user_id) + return tenant_id + + +async def make_admin(app: Any, email: str) -> str: + from users.bootstrap import create_admin + + async with app.state.sm.db.session_factory() as session: + result = await create_admin(session, email=email, password="pw-123456", full_name="Admin") + return str(result.user.id) + + +async def create_pat(client: httpx.AsyncClient, scopes: list[str], *, days: int = 30) -> dict: + """POST a personal token and return the response body (``token`` + ``item``).""" + resp = await client.post( + PAT_URL, + json={"name": "ci", "scopes": scopes, "expires_in_days": days}, + headers=CSRF_HEADERS, + ) + assert resp.status_code == 201, resp.text + return resp.json() + + +async def update_token(app: Any, token_id: str, **values: Any) -> None: + """Write columns of a token row directly (expiry, last use) for a test.""" + from api_tokens.models import ApiToken + from simple_module_db import ALL_TENANTS_OPTION + from sqlalchemy import update + + async with app.state.sm.db.session_factory() as session: + await session.execute( + update(ApiToken) + .where(ApiToken.id == uuid.UUID(token_id)) + .values(**values) + .execution_options(**{ALL_TENANTS_OPTION: True}) + ) + await session.commit() + + +async def load_token(app: Any, token_id: str): + from api_tokens.models import ApiToken + from simple_module_db import all_tenants + + async with app.state.sm.db.session_factory() as session: + with all_tenants(): + return await session.get(ApiToken, uuid.UUID(token_id)) diff --git a/modules/api_tokens/tests/conftest.py b/modules/api_tokens/tests/conftest.py new file mode 100644 index 000000000..57b04634e --- /dev/null +++ b/modules/api_tokens/tests/conftest.py @@ -0,0 +1,49 @@ +"""Fixtures for the api_tokens tests: signed-in actors and bearer calls.""" + +from __future__ import annotations + +import uuid +from collections.abc import AsyncGenerator, AsyncIterator, Callable +from contextlib import AbstractAsyncContextManager, asynccontextmanager +from typing import Any + +import httpx +import pytest +from _api_tokens_support import CSRF, Actor, join_tenant, make_admin +from simple_module_test.tenant_client import create_user, session_client + + +@pytest.fixture +def actor(app) -> Callable[..., AbstractAsyncContextManager[Actor]]: + """``async with actor(admin=True) as a:`` — a signed-in user with a tenant. + + The session carries a known CSRF token (``CSRF``) so mutations can echo it. + """ + from tenants.constants import SESSION_ACTIVE_TENANT + + @asynccontextmanager + async def factory( + *, admin: bool = False, tenant_id: str | None = None + ) -> AsyncGenerator[Actor]: + email = f"{'admin' if admin else 'user'}-{uuid.uuid4().hex[:8]}@example.com" + user_id = await (make_admin(app, email) if admin else create_user(app, email)) + tenant_id = await join_tenant(app, user_id, email, tenant_id) + data = {"user_id": user_id, SESSION_ACTIVE_TENANT: tenant_id, "sm_csrf_token": CSRF} + async with session_client(app, data) as client: + yield Actor(client, user_id, tenant_id) + + return factory + + +@pytest.fixture +async def bearer(app) -> AsyncIterator[Callable[..., Any]]: + """``await bearer("GET", url, token)`` — a cookie-less request with a bearer token.""" + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=app), base_url="http://testserver" + ) as client: + + async def call(method: str, url: str, token: str, **kwargs: Any) -> httpx.Response: + headers = {"Authorization": f"Bearer {token}", **kwargs.pop("headers", {})} + return await client.request(method, url, headers=headers, **kwargs) + + yield call diff --git a/modules/api_tokens/tests/test_api_tokens_endpoints.py b/modules/api_tokens/tests/test_api_tokens_endpoints.py new file mode 100644 index 000000000..62db038f8 --- /dev/null +++ b/modules/api_tokens/tests/test_api_tokens_endpoints.py @@ -0,0 +1,144 @@ +"""Who may manage tokens, and how: session only, CSRF on every mutation.""" + +from __future__ import annotations + +from _api_tokens_support import ( + CSRF_HEADERS, + INERTIA_HEADERS, + PAT_URL, + SERVICE_URL, + create_pat, +) + +BODY = {"name": "x", "scopes": ["api_tokens.use"], "expires_in_days": 7} + + +async def test_a_token_cannot_create_list_or_revoke_tokens(actor, bearer): + async with actor() as user: + created = await create_pat(user.client, ["api_tokens.use"]) + token, token_id = created["token"], created["item"]["id"] + + assert (await bearer("GET", PAT_URL, token)).status_code == 403 + assert ( + await bearer("POST", PAT_URL, token, json=BODY, headers=CSRF_HEADERS) + ).status_code == 403 + revoke = await bearer("POST", f"{PAT_URL}{token_id}/revoke", token, headers=CSRF_HEADERS) + assert revoke.status_code == 403 + assert (await bearer("GET", "/api-tokens/", token)).status_code == 403 + + +async def test_an_admin_token_cannot_reach_the_admin_endpoints(actor, bearer): + async with actor(admin=True) as a: + token = (await create_pat(a.client, ["api_tokens.manage", "api_tokens.use"]))["token"] + assert (await bearer("GET", "/api/api_tokens/admin/tokens", token)).status_code == 403 + body = {**BODY, "roles": []} + assert ( + await bearer("POST", SERVICE_URL, token, json=body, headers=CSRF_HEADERS) + ).status_code == 403 + + +async def test_cookie_mutations_require_the_csrf_token(actor): + async with actor() as user: + assert (await user.client.post(PAT_URL, json=BODY)).status_code == 403 + wrong = await user.client.post(PAT_URL, json=BODY, headers={"X-CSRF-Token": "nope"}) + assert wrong.status_code == 403 + made = await user.client.post(PAT_URL, json=BODY, headers=CSRF_HEADERS) + assert made.status_code == 201 + token_id = made.json()["item"]["id"] + assert (await user.client.post(f"{PAT_URL}{token_id}/revoke")).status_code == 403 + + +async def test_secret_is_returned_once_and_never_listed(actor): + async with actor() as user: + created = await create_pat(user.client, ["api_tokens.use"]) + listed = (await user.client.get(PAT_URL)).json() + assert len(listed) == 1 + item = listed[0] + assert "token" not in item and "token_hash" not in item + assert created["token"].startswith(item["prefix"]) + assert created["token"] not in str(listed) + + +async def test_users_see_and_revoke_only_their_own_tokens(actor): + async with actor() as alice, actor() as bob: + mine = await create_pat(alice.client, ["api_tokens.use"]) + assert (await bob.client.get(PAT_URL)).json() == [] + stolen = await bob.client.post( + f"{PAT_URL}{mine['item']['id']}/revoke", headers=CSRF_HEADERS + ) + assert stolen.status_code == 404 + + +async def test_admin_lists_and_revokes_tokens_in_the_tenant(actor): + async with actor(admin=True) as a: + async with actor(tenant_id=a.tenant_id) as member: + theirs = await create_pat(member.client, ["api_tokens.use"]) + listed = (await a.client.get("/api/api_tokens/admin/tokens")).json() + row = next(r for r in listed if r["id"] == theirs["item"]["id"]) + assert row["owner_email"].startswith("user-") + + revoked = await a.client.post( + f"/api/api_tokens/admin/tokens/{row['id']}/revoke", headers=CSRF_HEADERS + ) + assert revoked.status_code == 200 + assert revoked.json()["revoked_at"] is not None + + +async def test_admin_cannot_see_another_tenants_tokens(actor): + async with actor(admin=True) as a, actor() as outsider: + theirs = await create_pat(outsider.client, ["api_tokens.use"]) + listed = (await a.client.get("/api/api_tokens/admin/tokens")).json() + assert theirs["item"]["id"] not in {r["id"] for r in listed} + foreign = await a.client.post( + f"/api/api_tokens/admin/tokens/{theirs['item']['id']}/revoke", headers=CSRF_HEADERS + ) + assert foreign.status_code == 404 + + +async def test_pages_render_with_their_props(actor): + async with actor(admin=True) as a: + page = await a.client.get("/api-tokens/", headers=INERTIA_HEADERS) + assert page.status_code == 200, page.text + props = page.json()["props"] + assert "feature_flags.view" in props["scopes"] + assert "*" not in props["scopes"] + assert props["csrf_token"] + + admin = await a.client.get("/admin/api-tokens/", headers=INERTIA_HEADERS) + assert admin.status_code == 200, admin.text + assert "admin" not in admin.json()["props"]["roles"] + + async with actor() as user: + assert (await user.client.get("/admin/api-tokens/")).status_code == 403 + own = await user.client.get("/api-tokens/", headers=INERTIA_HEADERS) + assert own.status_code == 200 + assert "feature_flags.view" not in own.json()["props"]["scopes"] + + +async def test_create_and_revoke_are_audited_without_the_hash(app, actor): + from audit_log.models import AuditEntry + from simple_module_db import ALL_TENANTS_OPTION + from sqlalchemy import select + + async with actor() as user: + created = await create_pat(user.client, ["api_tokens.use"]) + token_id = created["item"]["id"] + await user.client.post(f"{PAT_URL}{token_id}/revoke", headers=CSRF_HEADERS) + + async with app.state.sm.db.session_factory() as session: + rows = ( + ( + await session.execute( + select(AuditEntry) + .where(AuditEntry.entity_type == "ApiToken", AuditEntry.entity_id == token_id) + .execution_options(**{ALL_TENANTS_OPTION: True}) + ) + ) + .scalars() + .all() + ) + actions = sorted(r.action for r in rows) + assert actions == ["created", "updated"] + blob = str([r.changes for r in rows]) + assert "token_hash" not in blob + assert "revoked_at" in blob diff --git a/modules/api_tokens/tests/test_api_tokens_format.py b/modules/api_tokens/tests/test_api_tokens_format.py new file mode 100644 index 000000000..9aeeed204 --- /dev/null +++ b/modules/api_tokens/tests/test_api_tokens_format.py @@ -0,0 +1,112 @@ +"""Token format, hashing, and the pure permission helpers the cap is built from.""" + +from __future__ import annotations + +import hashlib +from types import SimpleNamespace + +from api_tokens.constants import SECRET_LENGTH, TokenKind +from api_tokens.tokens import base62, display_prefix, generate, hash_token, is_well_formed +from auth.contracts.schemas import UserContext +from auth.contracts.tokens import is_api_token +from simple_module_core.permissions import is_admin_principal +from simple_module_hosting.permissions import apply_scope_cap + + +class TestTokenFormat: + def test_personal_token_shape(self): + new = generate(TokenKind.PERSONAL) + assert new.plaintext.startswith("smpat_") + assert len(new.plaintext) == len("smpat_") + SECRET_LENGTH + assert is_well_formed(new.plaintext) + assert is_api_token(new.plaintext) + + def test_service_key_shape(self): + new = generate(TokenKind.SERVICE) + assert new.plaintext.startswith("smsk_") + assert is_well_formed(new.plaintext) + + def test_only_the_sha256_is_kept(self): + new = generate(TokenKind.PERSONAL) + assert new.token_hash == hashlib.sha256(new.plaintext.encode()).hexdigest() + assert new.token_hash == hash_token(new.plaintext) + assert new.plaintext not in new.token_hash + + def test_display_prefix_is_kind_plus_eight(self): + new = generate(TokenKind.PERSONAL) + assert new.prefix == new.plaintext[: len("smpat_") + 8] + assert display_prefix("smsk_" + "a" * SECRET_LENGTH) == "smsk_aaaaaaaa" + + def test_tokens_are_unique(self): + assert len({generate(TokenKind.PERSONAL).plaintext for _ in range(200)}) == 200 + + def test_base62_pads_to_fixed_width(self): + assert base62(b"\x00" * 32) == "0" * SECRET_LENGTH + assert len(base62(b"\xff" * 32)) == SECRET_LENGTH + + def test_malformed_values_are_rejected_before_any_lookup(self): + good = generate(TokenKind.PERSONAL).plaintext + for bad in ( + "", + "smpat_", + good[:-1], + good + "x", + good.replace("smpat_", "smxx_"), + "smpat_" + "-" * SECRET_LENGTH, + "eyJhbGciOiJIUzI1NiJ9.e30.x", + ): + assert not is_well_formed(bad), bad + + +class TestUserContextFields: + def test_defaults_are_an_uncapped_session(self): + ctx = UserContext(id="1", email="a@b", name="A") + assert ctx.auth_kind == "session" + assert ctx.scopes is None + assert not ctx.is_scoped + + def test_session_dict_round_trip_keeps_kind_and_scopes(self): + ctx = UserContext(id="1", email="a@b", name="A", auth_kind="pat", scopes=["x.read"]) + back = UserContext.from_session_dict(ctx.to_session_dict()) + assert back is not None + assert back.auth_kind == "pat" + assert back.scopes == ["x.read"] + + def test_old_cookie_without_the_fields_is_a_session(self): + back = UserContext.from_session_dict({"id": "1", "email": "a@b", "name": "A"}) + assert back is not None + assert back.auth_kind == "session" + assert back.scopes is None + + def test_from_user_threads_kind_and_scopes(self): + user = SimpleNamespace(id="u", email="a@b", full_name=None, roles=[]) + ctx = UserContext.from_user(user, auth_kind="pat", scopes=["x.read"]) + assert (ctx.auth_kind, ctx.scopes) == ("pat", ["x.read"]) + + +class TestScopeCap: + def test_no_scopes_means_no_cap(self): + user = SimpleNamespace(scopes=None) + assert apply_scope_cap({"a", "b"}, user) == {"a", "b"} + + def test_intersection_with_held(self): + user = SimpleNamespace(scopes=["a", "c"]) + assert apply_scope_cap({"a", "b"}, user) == {"a"} + + def test_wildcard_expands_to_the_scope_list_only(self): + user = SimpleNamespace(scopes=["a", "c"]) + assert apply_scope_cap({"*"}, user) == {"a", "c"} + + def test_a_wildcard_scope_is_never_honoured(self): + user = SimpleNamespace(scopes=["*", "a"]) + assert apply_scope_cap({"*"}, user) == {"a"} + assert apply_scope_cap({"b"}, SimpleNamespace(scopes=["*"])) == set() + + def test_empty_scope_list_caps_to_nothing(self): + assert apply_scope_cap({"*"}, SimpleNamespace(scopes=[])) == set() + + def test_admin_principal_excludes_tokens(self): + assert is_admin_principal(SimpleNamespace(roles=["admin"], scopes=None)) + assert not is_admin_principal(SimpleNamespace(roles=["admin"], scopes=["a"])) + assert not is_admin_principal(SimpleNamespace(roles=["user"], scopes=None)) + assert not is_admin_principal(None) diff --git a/modules/api_tokens/tests/test_api_tokens_resolver.py b/modules/api_tokens/tests/test_api_tokens_resolver.py new file mode 100644 index 000000000..0fbb2de62 --- /dev/null +++ b/modules/api_tokens/tests/test_api_tokens_resolver.py @@ -0,0 +1,143 @@ +"""Resolving a bearer token: what is accepted, what is refused, and what it costs.""" + +from __future__ import annotations + +import uuid +from datetime import UTC, datetime, timedelta + +from _api_tokens_support import FLAGS_URL, create_pat, load_token, update_token +from api_tokens.constants import TokenKind +from api_tokens.tokens import generate +from starlette.requests import Request + + +async def test_valid_pat_reads_but_cannot_write(actor, bearer): + """The acceptance case: scope ``feature_flags.view`` can GET, cannot PUT.""" + async with actor(admin=True) as a: + token = (await create_pat(a.client, ["feature_flags.view"]))["token"] + + assert (await bearer("GET", FLAGS_URL, token)).status_code == 200 + put = await bearer("PUT", f"{FLAGS_URL}anything", token, json={"enabled": True}) + assert put.status_code == 403 + + +async def test_revoke_takes_effect_on_the_next_request(actor, bearer): + async with actor(admin=True) as a: + created = await create_pat(a.client, ["feature_flags.view"]) + token, token_id = created["token"], created["item"]["id"] + assert (await bearer("GET", FLAGS_URL, token)).status_code == 200 + + resp = await a.client.post( + f"/api/api_tokens/tokens/{token_id}/revoke", headers={"X-CSRF-Token": "csrf-test-token"} + ) + assert resp.status_code == 200 + assert resp.json()["revoked_at"] is not None + + assert (await bearer("GET", FLAGS_URL, token)).status_code == 401 + + +async def test_expired_token_is_refused(app, actor, bearer): + async with actor(admin=True) as a: + created = await create_pat(a.client, ["feature_flags.view"]) + await update_token( + app, created["item"]["id"], expires_at=datetime.now(UTC) - timedelta(seconds=1) + ) + assert (await bearer("GET", FLAGS_URL, created["token"])).status_code == 401 + + +async def test_unknown_and_malformed_tokens_are_refused(bearer): + unknown = generate(TokenKind.PERSONAL).plaintext + assert (await bearer("GET", FLAGS_URL, unknown)).status_code == 401 + assert (await bearer("GET", FLAGS_URL, "smpat_short")).status_code == 401 + assert ( + await bearer("GET", FLAGS_URL, generate(TokenKind.SERVICE).plaintext) + ).status_code == 401 + + +async def _set_owner(app, user_id: str, **values) -> None: + from sqlalchemy import update + from users.models import User + + async with app.state.sm.db.session_factory() as session: + await session.execute(update(User).where(User.id == uuid.UUID(user_id)).values(**values)) + await session.commit() + + +async def test_inactive_owner_is_refused(app, actor, bearer): + async with actor(admin=True) as a: + token = (await create_pat(a.client, ["feature_flags.view"]))["token"] + await _set_owner(app, a.user_id, is_active=False) + assert (await bearer("GET", FLAGS_URL, token)).status_code == 401 + + +async def test_disabled_owner_is_refused(app, actor, bearer): + async with actor(admin=True) as a: + token = (await create_pat(a.client, ["feature_flags.view"]))["token"] + await _set_owner(app, a.user_id, disabled_at=datetime.now(UTC)) + assert (await bearer("GET", FLAGS_URL, token)).status_code == 401 + + +async def test_token_survives_a_fresh_resolver_state(app, actor): + """Nothing is cached in-process: a restart only needs the database row.""" + from api_tokens.resolver import resolve_api_token + + async with actor(admin=True) as a: + token = (await create_pat(a.client, ["feature_flags.view"]))["token"] + scope = { + "type": "http", + "method": "GET", + "path": "/", + "headers": [(b"authorization", f"Bearer {token}".encode())], + "app": app, + } + ctx = await resolve_api_token(Request(scope)) + assert ctx is not None + assert (ctx.auth_kind, ctx.scopes, ctx.id) == ("pat", ["feature_flags.view"], a.user_id) + + +async def test_last_used_is_written_at_most_once_per_window(app, actor, bearer): + async with actor(admin=True) as a: + created = await create_pat(a.client, ["feature_flags.view"]) + token, token_id = created["token"], created["item"]["id"] + assert (await load_token(app, token_id)).last_used_at is None + + await bearer("GET", FLAGS_URL, token) + first = (await load_token(app, token_id)).last_used_at + assert first is not None + + await bearer("GET", FLAGS_URL, token) + assert (await load_token(app, token_id)).last_used_at == first + + stale = datetime.now(UTC) - timedelta(minutes=10) + await update_token(app, token_id, last_used_at=stale) + await bearer("GET", FLAGS_URL, token) + assert (await load_token(app, token_id)).last_used_at > stale + + +async def test_users_provider_does_not_look_up_api_tokens(monkeypatch): + """The provider steps aside for ``smpat_``/``smsk_`` without touching its store.""" + import users.token_strategy + from users.provider import UsersAuthProvider + + calls: list[str] = [] + + async def spy(scope, token): + calls.append(token) + return + + monkeypatch.setattr(users.token_strategy, "resolve_bearer", spy) + provider = UsersAuthProvider() + for credential in ( + generate(TokenKind.PERSONAL).plaintext, + generate(TokenKind.SERVICE).plaintext, + ): + scope = { + "type": "http", + "headers": [(b"authorization", f"Bearer {credential}".encode())], + } + assert await provider.resolve_user(Request(scope)) is None + assert calls == [] + + other = {"type": "http", "headers": [(b"authorization", b"Bearer opaque-session-token")]} + await provider.resolve_user(Request(other)) + assert calls == ["opaque-session-token"] diff --git a/modules/api_tokens/tests/test_api_tokens_scopes.py b/modules/api_tokens/tests/test_api_tokens_scopes.py new file mode 100644 index 000000000..a574f7f4a --- /dev/null +++ b/modules/api_tokens/tests/test_api_tokens_scopes.py @@ -0,0 +1,127 @@ +"""The scope cap: admins are capped, demotion shrinks, creation is bounded by the creator.""" + +from __future__ import annotations + +import uuid +from types import SimpleNamespace + +import pytest +from _api_tokens_support import CSRF_HEADERS, FLAGS_URL, PAT_URL, create_pat +from fastapi import HTTPException +from starlette.requests import Request + + +async def test_admin_token_is_capped_by_its_scopes(actor, bearer): + """An admin's token skips every admin shortcut: it is worth its scopes only.""" + async with actor(admin=True) as a: + token = (await create_pat(a.client, ["feature_flags.view"]))["token"] + + assert (await bearer("GET", FLAGS_URL, token)).status_code == 200 + assert (await bearer("PUT", f"{FLAGS_URL}x", token, json={"enabled": True})).status_code == 403 + assert (await bearer("GET", "/api/users/admin", token)).status_code == 403 + # Role-only admin gates: the Doctor page and the /admin index. + assert (await bearer("GET", "/admin/doctor/", token)).status_code == 403 + + +async def _set_roles(app, user_id: str, names: list[str]) -> None: + from sqlalchemy import delete, select + from users.bootstrap import ensure_user_role + from users.models import Role, UserRole + + async with app.state.sm.db.session_factory() as session: + uid = uuid.UUID(user_id) + await session.execute(delete(UserRole).where(UserRole.user_id == uid)) + for name in names: + role = ( + await session.execute(select(Role).where(Role.name == name)) + ).scalar_one_or_none() + if role is None: + role = await ensure_user_role(session) if name == "user" else Role(name=name) + session.add(role) + await session.flush() + session.add(UserRole(user_id=uid, role_id=role.id)) + await session.commit() + + +async def test_demoting_the_owner_shrinks_the_token(app, actor, bearer): + async with actor(admin=True) as a: + token = (await create_pat(a.client, ["feature_flags.view"]))["token"] + assert (await bearer("GET", FLAGS_URL, token)).status_code == 200 + + await _set_roles(app, a.user_id, ["user"]) + + assert (await bearer("GET", FLAGS_URL, token)).status_code == 403 + + +async def test_scopes_must_be_held_by_the_creator(actor): + async with actor() as user: + refused = await user.client.post( + PAT_URL, + json={"name": "x", "scopes": ["feature_flags.view"], "expires_in_days": 7}, + headers=CSRF_HEADERS, + ) + assert refused.status_code == 422 + assert "feature_flags.view" in refused.json()["detail"] + + made = await user.client.post( + PAT_URL, + json={"name": "x", "scopes": ["users.self.profile"], "expires_in_days": 7}, + headers=CSRF_HEADERS, + ) + assert made.status_code == 201, made.text + assert made.json()["item"]["scopes"] == ["users.self.profile"] + + +async def test_scopes_cannot_be_empty_or_unregistered(actor): + async with actor(admin=True) as a: + empty = await a.client.post( + PAT_URL, json={"name": "x", "scopes": [], "expires_in_days": 7}, headers=CSRF_HEADERS + ) + assert empty.status_code == 422 + bogus = await a.client.post( + PAT_URL, + json={"name": "x", "scopes": ["*"], "expires_in_days": 7}, + headers=CSRF_HEADERS, + ) + assert bogus.status_code == 422 + + +async def test_lifetime_is_capped_by_the_setting(app, actor): + app.state.api_tokens.settings.max_lifetime_days = 30 + async with actor(admin=True) as a: + too_long = await a.client.post( + PAT_URL, + json={"name": "x", "scopes": ["feature_flags.view"], "expires_in_days": 31}, + headers=CSRF_HEADERS, + ) + assert too_long.status_code == 422 + ok = await a.client.post( + PAT_URL, + json={"name": "x", "scopes": ["feature_flags.view"], "expires_in_days": 30}, + headers=CSRF_HEADERS, + ) + assert ok.status_code == 201 + + +async def test_require_permission_admin_shortcut_skips_tokens(): + """``auth.deps.require_permission`` admits an admin session but not an admin token.""" + from auth.contracts.schemas import UserContext + from auth.deps import require_permission + + check = require_permission("x.write").dependency + translator = SimpleNamespace(t=lambda key, **_: key) + + def request() -> Request: + req = Request({"type": "http", "app": SimpleNamespace(state=SimpleNamespace())}) + req.state.resolved_permissions = set() # what the capped resolution produced + return req + + session_admin = UserContext(id="1", email="a@b", name="A", roles=["admin"]) + assert await check(request(), translator, session_admin) is None + + token_admin = UserContext( + id="1", email="a@b", name="A", roles=["admin"], auth_kind="pat", scopes=["y.read"] + ) + with pytest.raises(HTTPException) as exc: + await check(request(), translator, token_admin) + assert exc.value.status_code == 403 diff --git a/modules/api_tokens/tests/test_api_tokens_service_keys.py b/modules/api_tokens/tests/test_api_tokens_service_keys.py new file mode 100644 index 000000000..05eba3465 --- /dev/null +++ b/modules/api_tokens/tests/test_api_tokens_service_keys.py @@ -0,0 +1,85 @@ +"""Service keys: roles chosen at creation, never admin, pinned to their tenant.""" + +from __future__ import annotations + +from _api_tokens_support import CSRF_HEADERS, FLAGS_URL, SERVICE_URL +from starlette.requests import Request + + +async def _create_key(client, *, roles, scopes, days=30): + return await client.post( + SERVICE_URL, + json={"name": "deployer", "roles": roles, "scopes": scopes, "expires_in_days": days}, + headers=CSRF_HEADERS, + ) + + +async def test_service_key_gets_its_roles_capped_by_its_scopes(app, actor, bearer): + # Give the ``user`` role the read permission so the key's role grants it. + app.state.sm.permissions.map_role("user", ["feature_flags.view"]) + async with actor(admin=True) as a: + resp = await _create_key(a.client, roles=["user"], scopes=["feature_flags.view"]) + assert resp.status_code == 201, resp.text + body = resp.json() + assert body["token"].startswith("smsk_") + assert body["item"]["owner_user_id"] is None + assert body["item"]["roles"] == ["user"] + + token = body["token"] + assert (await bearer("GET", FLAGS_URL, token)).status_code == 200 + assert (await bearer("PUT", f"{FLAGS_URL}x", token, json={"enabled": True})).status_code == 403 + + +async def test_scope_without_a_role_granting_it_grants_nothing(actor, bearer): + async with actor(admin=True) as a: + resp = await _create_key(a.client, roles=[], scopes=["feature_flags.view"]) + assert resp.status_code == 201 + assert (await bearer("GET", FLAGS_URL, resp.json()["token"])).status_code == 403 + + +async def test_service_key_cannot_be_admin(actor): + async with actor(admin=True) as a: + resp = await _create_key(a.client, roles=["admin"], scopes=["feature_flags.view"]) + assert resp.status_code == 422 + assert "admin" in resp.json()["detail"] + + tenant_role = await _create_key( + a.client, roles=["tenant:owner"], scopes=["feature_flags.view"] + ) + assert tenant_role.status_code == 422 + + +async def test_only_managers_create_service_keys(actor): + async with actor() as user: + resp = await _create_key(user.client, roles=[], scopes=["users.self.profile"]) + assert resp.status_code == 403 + + +async def test_service_principal_is_pinned_to_its_tenant(app, actor): + from api_tokens.resolver import resolve_api_token + from tenants.resolver import resolve_tenant + + async with actor(admin=True) as a: + resp = await _create_key(a.client, roles=["user"], scopes=["feature_flags.view"]) + token, home = resp.json()["token"], a.tenant_id + async with actor(admin=True) as other: + elsewhere = other.tenant_id + + def request(*extra: tuple[bytes, bytes]) -> Request: + headers = [(b"authorization", f"Bearer {token}".encode()), *extra] + return Request( + {"type": "http", "method": "GET", "path": "/", "headers": headers, "app": app} + ) + + ctx = await resolve_api_token(request()) + assert ctx is not None + assert ctx.id.startswith("svc:") + assert ctx.roles == ["user"] + assert ctx.tenant_id == home + assert (ctx.auth_kind, ctx.scopes) == ("service", ["feature_flags.view"]) + + # Asking for another tenant by header does not move it. + req = request((b"x-tenant-id", elsewhere.encode())) + req.state.user = ctx + resolution = await resolve_tenant(req) + assert resolution.tenant_id == home diff --git a/modules/api_tokens/tsconfig.json b/modules/api_tokens/tsconfig.json new file mode 100644 index 000000000..8fef44a1a --- /dev/null +++ b/modules/api_tokens/tsconfig.json @@ -0,0 +1,10 @@ +{ + "extends": "@simple-module-py/tsconfig/base.json", + "compilerOptions": { + "paths": { + "@/*": ["./api_tokens/*"], + "@simple-module-py/ui/*": ["../../packages/ui/src/*"] + } + }, + "include": ["api_tokens/**/*.ts", "api_tokens/**/*.tsx"] +} diff --git a/package-lock.json b/package-lock.json index a64df2a8b..1bae95734 100644 --- a/package-lock.json +++ b/package-lock.json @@ -78,6 +78,23 @@ "@types/react": "^19.2.0" } }, + "modules/api_tokens": { + "name": "@simple-module-py/api-tokens", + "version": "0.1.0", + "dependencies": { + "lucide-react": "^1.43.0", + "sonner": "^2.0.8" + }, + "devDependencies": { + "@simple-module-py/tsconfig": "*" + }, + "peerDependencies": { + "@inertiajs/react": "^3.7.0", + "@simple-module-py/ui": "*", + "react": "^19.0.0", + "react-dom": "^19.0.0" + } + }, "modules/audit_log": { "name": "@simple-module-py/audit-log", "version": "0.1.0", @@ -2817,6 +2834,10 @@ "node": "^20.19.0 || >=22.12.0" } }, + "node_modules/@simple-module-py/api-tokens": { + "resolved": "modules/api_tokens", + "link": true + }, "node_modules/@simple-module-py/audit-log": { "resolved": "modules/audit_log", "link": true diff --git a/packages/i18n/src/generated-resources.ts b/packages/i18n/src/generated-resources.ts index 23cb37020..e09b4e247 100644 --- a/packages/i18n/src/generated-resources.ts +++ b/packages/i18n/src/generated-resources.ts @@ -2,6 +2,74 @@ // Regenerate by booting the host in development mode or `make gen-i18n`. export default { translation: { + 'api_tokens.admin.description': '', + 'api_tokens.admin.new_service_key': '', + 'api_tokens.admin.tenant_required': '', + 'api_tokens.admin.title': '', + 'api_tokens.audit.token': '', + 'api_tokens.created.copied': '', + 'api_tokens.created.copy': '', + 'api_tokens.created.done': '', + 'api_tokens.created.title': '', + 'api_tokens.created.warning': '', + 'api_tokens.errors.invalid_kind': '', + 'api_tokens.errors.lifetime_too_long': '', + 'api_tokens.errors.not_found': '', + 'api_tokens.errors.role_forbidden': '', + 'api_tokens.errors.role_not_assignable': '', + 'api_tokens.errors.scope_not_held': '', + 'api_tokens.errors.scopes_required': '', + 'api_tokens.errors.session_required': '', + 'api_tokens.errors.tenant_required': '', + 'api_tokens.form.cancel': '', + 'api_tokens.form.create_description': '', + 'api_tokens.form.create_service_description': '', + 'api_tokens.form.create_service_title': '', + 'api_tokens.form.create_title': '', + 'api_tokens.form.days_one': '', + 'api_tokens.form.days_other': '', + 'api_tokens.form.expiry': '', + 'api_tokens.form.name': '', + 'api_tokens.form.name_placeholder': '', + 'api_tokens.form.new_token': '', + 'api_tokens.form.no_roles': '', + 'api_tokens.form.roles': '', + 'api_tokens.form.roles_hint': '', + 'api_tokens.form.scopes': '', + 'api_tokens.form.scopes_hint': '', + 'api_tokens.form.submit': '', + 'api_tokens.index.description': '', + 'api_tokens.index.title': '', + 'api_tokens.index.usage': '', + 'api_tokens.kind.personal': '', + 'api_tokens.kind.service': '', + 'api_tokens.list.actions': '', + 'api_tokens.list.empty_description': '', + 'api_tokens.list.empty_title': '', + 'api_tokens.list.expires': '', + 'api_tokens.list.kind': '', + 'api_tokens.list.last_used': '', + 'api_tokens.list.name': '', + 'api_tokens.list.never': '', + 'api_tokens.list.owner': '', + 'api_tokens.list.revoke': '', + 'api_tokens.list.roles': '', + 'api_tokens.list.scope_count_one': '', + 'api_tokens.list.scope_count_other': '', + 'api_tokens.list.scopes': '', + 'api_tokens.list.status': '', + 'api_tokens.list.token': '', + 'api_tokens.nav.api_tokens': '', + 'api_tokens.revoke.cancel': '', + 'api_tokens.revoke.confirm': '', + 'api_tokens.revoke.description': '', + 'api_tokens.revoke.title': '', + 'api_tokens.status.active': '', + 'api_tokens.status.expired': '', + 'api_tokens.status.revoked': '', + 'api_tokens.toasts.created': '', + 'api_tokens.toasts.failed': '', + 'api_tokens.toasts.revoked': '', 'audit_log.actions.created': '', 'audit_log.actions.deleted': '', 'audit_log.actions.soft_deleted': '', diff --git a/packages/i18n/src/keys.generated.ts b/packages/i18n/src/keys.generated.ts index 837fccf17..2b2e80cb2 100644 --- a/packages/i18n/src/keys.generated.ts +++ b/packages/i18n/src/keys.generated.ts @@ -2,6 +2,102 @@ // Regenerate by booting the host in development mode or `make gen-i18n`. export const keys = { + api_tokens: { + admin: { + description: 'api_tokens.admin.description', + new_service_key: 'api_tokens.admin.new_service_key', + tenant_required: 'api_tokens.admin.tenant_required', + title: 'api_tokens.admin.title', + }, + audit: { + token: 'api_tokens.audit.token', + }, + created: { + copied: 'api_tokens.created.copied', + copy: 'api_tokens.created.copy', + done: 'api_tokens.created.done', + title: 'api_tokens.created.title', + warning: 'api_tokens.created.warning', + }, + errors: { + invalid_kind: 'api_tokens.errors.invalid_kind', + lifetime_too_long: 'api_tokens.errors.lifetime_too_long', + not_found: 'api_tokens.errors.not_found', + role_forbidden: 'api_tokens.errors.role_forbidden', + role_not_assignable: 'api_tokens.errors.role_not_assignable', + scope_not_held: 'api_tokens.errors.scope_not_held', + scopes_required: 'api_tokens.errors.scopes_required', + session_required: 'api_tokens.errors.session_required', + tenant_required: 'api_tokens.errors.tenant_required', + }, + form: { + cancel: 'api_tokens.form.cancel', + create_description: 'api_tokens.form.create_description', + create_service_description: 'api_tokens.form.create_service_description', + create_service_title: 'api_tokens.form.create_service_title', + create_title: 'api_tokens.form.create_title', + days: 'api_tokens.form.days', + days_one: 'api_tokens.form.days_one', + days_other: 'api_tokens.form.days_other', + expiry: 'api_tokens.form.expiry', + name: 'api_tokens.form.name', + name_placeholder: 'api_tokens.form.name_placeholder', + new_token: 'api_tokens.form.new_token', + no_roles: 'api_tokens.form.no_roles', + roles: 'api_tokens.form.roles', + roles_hint: 'api_tokens.form.roles_hint', + scopes: 'api_tokens.form.scopes', + scopes_hint: 'api_tokens.form.scopes_hint', + submit: 'api_tokens.form.submit', + }, + index: { + description: 'api_tokens.index.description', + title: 'api_tokens.index.title', + usage: 'api_tokens.index.usage', + }, + kind: { + personal: 'api_tokens.kind.personal', + service: 'api_tokens.kind.service', + }, + list: { + actions: 'api_tokens.list.actions', + empty_description: 'api_tokens.list.empty_description', + empty_title: 'api_tokens.list.empty_title', + expires: 'api_tokens.list.expires', + kind: 'api_tokens.list.kind', + last_used: 'api_tokens.list.last_used', + name: 'api_tokens.list.name', + never: 'api_tokens.list.never', + owner: 'api_tokens.list.owner', + revoke: 'api_tokens.list.revoke', + roles: 'api_tokens.list.roles', + scope_count: 'api_tokens.list.scope_count', + scope_count_one: 'api_tokens.list.scope_count_one', + scope_count_other: 'api_tokens.list.scope_count_other', + scopes: 'api_tokens.list.scopes', + status: 'api_tokens.list.status', + token: 'api_tokens.list.token', + }, + nav: { + api_tokens: 'api_tokens.nav.api_tokens', + }, + revoke: { + cancel: 'api_tokens.revoke.cancel', + confirm: 'api_tokens.revoke.confirm', + description: 'api_tokens.revoke.description', + title: 'api_tokens.revoke.title', + }, + status: { + active: 'api_tokens.status.active', + expired: 'api_tokens.status.expired', + revoked: 'api_tokens.status.revoked', + }, + toasts: { + created: 'api_tokens.toasts.created', + failed: 'api_tokens.toasts.failed', + revoked: 'api_tokens.toasts.revoked', + }, + }, audit_log: { actions: { created: 'audit_log.actions.created', diff --git a/pyproject.toml b/pyproject.toml index 6b0157aba..57129751d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -83,6 +83,7 @@ extra-paths = [ "modules/site_lock", "modules/branding", "modules/tenants", + "modules/api_tokens", "host", "scripts", ] @@ -118,7 +119,7 @@ invalid-assignment = "ignore" [tool.pytest.ini_options] asyncio_mode = "auto" -testpaths = ["framework/cli/tests", "framework/core/tests", "framework/db/tests", "framework/hosting/tests", "framework/inertia/tests", "framework/testing/tests", "host/tests", "modules/auth/tests", "modules/dashboard/tests", "modules/users/tests", "modules/permissions/tests", "modules/background_tasks/tests", "modules/file_storage/tests", "modules/settings/tests", "modules/feature_flags/tests", "modules/keycloak/tests", "modules/audit_log/tests", "modules/branding/tests", "modules/site_lock/tests", "modules/tenants/tests", "scripts/tests", "tests"] +testpaths = ["framework/cli/tests", "framework/core/tests", "framework/db/tests", "framework/hosting/tests", "framework/inertia/tests", "framework/testing/tests", "host/tests", "modules/auth/tests", "modules/dashboard/tests", "modules/users/tests", "modules/permissions/tests", "modules/background_tasks/tests", "modules/file_storage/tests", "modules/settings/tests", "modules/feature_flags/tests", "modules/keycloak/tests", "modules/audit_log/tests", "modules/branding/tests", "modules/site_lock/tests", "modules/tenants/tests", "modules/api_tokens/tests", "scripts/tests", "tests"] markers = [ "e2e: end-to-end tests requiring a live browser", "perf: performance benchmarks (opt-in; run via `make bench`)", From 8bdf27fe59ffea50a3402ff1d16211b8c987bdd7 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 15 Oct 2026 10:21:07 +0200 Subject: [PATCH 3/9] fix(scaffold): new-module tsconfig, Inertia import and peer version The scaffolded tsconfig set baseUrl, which TypeScript 6 rejects (TS5102) and fails `make lint`; views.py imported InertiaResponse from a package that does not exist; package.json peered @inertiajs/react ^2. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- scripts/_templates_endpoints.py | 2 +- scripts/_templates_js.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/scripts/_templates_endpoints.py b/scripts/_templates_endpoints.py index 0ee691050..7764e1181 100644 --- a/scripts/_templates_endpoints.py +++ b/scripts/_templates_endpoints.py @@ -80,7 +80,7 @@ def views_py(ctx: ScaffoldContext) -> str: from __future__ import annotations from fastapi import APIRouter, Depends - from inertia import InertiaResponse + from simple_module_inertia import InertiaResponse from simple_module_hosting.inertia_deps import InertiaDep from {ctx.pkg}.deps import get_{ctx.singular}_service diff --git a/scripts/_templates_js.py b/scripts/_templates_js.py index 9cfc896e0..1278604d0 100644 --- a/scripts/_templates_js.py +++ b/scripts/_templates_js.py @@ -15,7 +15,7 @@ def package_json(ctx: ScaffoldContext) -> str: "peerDependencies": {{ "react": "^19.0.0", "react-dom": "^19.0.0", - "@inertiajs/react": "^2.0.0", + "@inertiajs/react": "^3.7.0", "@simple-module-py/ui": "*" }}, "devDependencies": {{ @@ -31,7 +31,6 @@ def tsconfig_json(ctx: ScaffoldContext) -> str: {{ "extends": "@simple-module-py/tsconfig/base.json", "compilerOptions": {{ - "baseUrl": ".", "paths": {{ "@/*": ["./{ctx.pkg}/*"], "@simple-module-py/ui/*": ["../../packages/ui/src/*"] From b45ca950b2b610c84320292fcc5505bbadd8100a Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 15 Oct 2026 10:21:07 +0200 Subject: [PATCH 4/9] docs(api_tokens): module page, resolver carve-out, changelog (#455) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- CHANGELOG.md | 30 ++++++ docs/.vitepress/config.ts | 1 + docs/framework/principal-resolvers.md | 12 +++ docs/modules/api_tokens.md | 135 ++++++++++++++++++++++++++ docs/modules/index.md | 3 +- 5 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 docs/modules/api_tokens.md diff --git a/CHANGELOG.md b/CHANGELOG.md index d3a060976..96028818f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,24 @@ All notable changes to this project are documented in this file. The format is b ## [Unreleased] ### Added +- **New `api_tokens` module** (#455): personal access tokens (`smpat_…`) and + service API keys (`smsk_…`). Only `sha256(token)` is stored and the secret + is shown once; a token is resolved by one indexed lookup per request (no + cache), so a revoke applies on the next request. Every token carries scopes + that cap the principal's permissions on top of the owner's live grants (an + admin's token is worth its scopes only). Service keys get roles chosen at + creation (never `admin`) and are pinned to their tenant. Self-service page at + `/api-tokens/` (user menu), admin screen at `/admin/api-tokens/`; tokens can + only be managed from a session, with `RequiresCsrf`. New PyPI package + `simple_module_api_tokens`; not added to the `smpy new` scaffold. Needs a + migration (`branch_labels = ("api_tokens",)`). +- `UserContext` gains `auth_kind` (`"session"` by default; `"pat"` / + `"service"` for tokens) and `scopes` (`None` = uncapped), carried through + `from_user`, `to_session_dict` and `from_session_dict`. `auth.contracts.tokens` + holds the kinds and the `smpat_` / `smsk_` prefixes. +- `simple_module_hosting.permissions.apply_scope_cap` and + `simple_module_core.permissions.is_admin_principal`. A role-only admin gate + should use `is_admin_principal(user)`, which is `False` for any token. - `from background_tasks import heartbeat` (#451): a long-running task calls `heartbeat()` to refresh `heartbeat_at` (throttled to one write per 30 s; a no-op outside a task). `@shared_task(..., stuck_after=)` sets a @@ -109,6 +127,18 @@ All notable changes to this project are documented in this file. The format is b production-mode containers pass `UsersSettings` boot validation. ### Changed +- The `users` and `keycloak` auth providers return `None` at once for a bearer + value with an `api_tokens` prefix instead of looking it up (#455), leaving it + to the principal resolver chain. +- `require_permission` (auth) skips its admin-role shortcut, and + `resolve_principal_permissions` / `resolved_permissions_for` cap the set, + when the principal carries `scopes`. The `/admin` index, the Doctor page and + the maintenance bypass no longer admit an admin's token on the role alone. +- The `tenants` resolver binds a service principal to its pinned tenant and + ignores the tenant header and subdomain for it. +- `make new-module`: the scaffolded `tsconfig.json` no longer sets `baseUrl` + (removed in TypeScript 6), `views.py` imports `InertiaResponse` from + `simple_module_inertia`, and `package.json` peers `@inertiajs/react` `^3.7.0`. - **Dashboard stats are admin-only on single-tenant installs too** (#464). `dashboard.view` was mapped to the ordinary `user` role when `multi_tenant` was off, so any self-registered account could read user diff --git a/docs/.vitepress/config.ts b/docs/.vitepress/config.ts index b8fade0e9..7a78a7ed9 100644 --- a/docs/.vitepress/config.ts +++ b/docs/.vitepress/config.ts @@ -178,6 +178,7 @@ export default defineConfig({ { text: "auth", link: "/modules/auth" }, { text: "users", link: "/modules/users" }, { text: "keycloak", link: "/modules/keycloak" }, + { text: "api_tokens", link: "/modules/api_tokens" }, { text: "permissions", link: "/modules/permissions" }, { text: "settings", link: "/modules/settings" }, { text: "feature_flags", link: "/modules/feature_flags" }, diff --git a/docs/framework/principal-resolvers.md b/docs/framework/principal-resolvers.md index f97e519e9..ef246974e 100644 --- a/docs/framework/principal-resolvers.md +++ b/docs/framework/principal-resolvers.md @@ -55,6 +55,18 @@ unauthenticated response is suppressed for public paths regardless of resolver outcome — the chain populates the principal, public-path allow-listing controls the response. +A bearer-token resolver only ever sees the request if the provider steps +aside. The `users` and `keycloak` providers treat any `Authorization: Bearer` +header as theirs and return a final `None` for a value they do not know — +except values with an `api_tokens` prefix (`auth.contracts.tokens.is_api_token`: +`smpat_`, `smsk_`), which they skip without a lookup. A resolver for a new +bearer format needs the same carve-out. + +The shipped [`api_tokens`](/modules/api_tokens) module is the reference +implementation: it sets `UserContext.auth_kind` and `scopes`, and the framework +caps the principal's permissions at those scopes (`apply_scope_cap`) and skips +admin-role shortcuts for it (`is_admin_principal`). + ## Worked example — bearer-token resolver A module that ships its own Personal-Access-Token table registers a diff --git a/docs/modules/api_tokens.md b/docs/modules/api_tokens.md new file mode 100644 index 000000000..f1ecc4a55 --- /dev/null +++ b/docs/modules/api_tokens.md @@ -0,0 +1,135 @@ +# api_tokens + +Personal access tokens (PATs) and service API keys. A script, CI job or another +service sends `Authorization: Bearer ` and is treated as a principal +whose permissions are **capped by the token's scopes**. Tokens are hashed at +rest, shown once, and revocable with effect on the next request. + +## ModuleMeta + +| Field | Value | +|---|---| +| `name` | `ApiTokens` | +| `route_prefix` | `/api/api_tokens` | +| `view_prefix` | `/api-tokens` | +| `admin_view_prefix` | `/admin/api-tokens` | +| `depends_on` | `Auth`, `Users`, `Settings` | + +Installed in this repository's host. It is **not** part of the `smpy new` +scaffold; add `simple_module_api_tokens` to a host's dependencies to enable it, +then generate a migration (its first revision carries +`branch_labels = ("api_tokens",)`). + +## Token kinds + +| Kind | Format | Acts as | Permissions | +|---|---|---|---| +| Personal access token | `smpat_<43 base62 chars>` | its owner (a `users` account) | owner's **live** roles and grants ∩ scopes | +| Service key | `smsk_<43 base62 chars>` | `svc:` — no person | roles chosen at creation ∩ scopes | + +The secret is 32 bytes from `secrets.token_bytes`, base62-encoded and padded to +a fixed width. Only `sha256(token)` is stored, with an 8-character display +prefix (`smpat_AbCd1234…`) so people can tell tokens apart. The plaintext is in +the create response and nowhere else. + +## How a request is resolved + +1. The auth provider (`users`, and `keycloak`) sees a bearer value starting + with `smpat_` / `smsk_` and returns `None` immediately — no wasted lookup + in its own token store. +2. `AuthMiddleware` falls through to the principal-resolver chain, where this + module's resolver is registered at startup. +3. The resolver checks the format with a regex, then does **one indexed lookup + by hash** (no cache). Revoked, expired or unknown → `None` → 401. +4. A PAT also loads its owner with roles; an inactive or disabled owner → 401. +5. The principal is a `UserContext` with `auth_kind` (`"pat"` / `"service"`) + and `scopes` set. `last_used_at` is written at most once every 5 minutes, + by a conditional `UPDATE` in its own short transaction. + +Because nothing is cached, a token keeps working across restarts and stops on +the very next request after it is revoked. + +## The scope cap + +`UserContext.scopes` is `None` for an ordinary session. For a token it is a +list, and: + +- `resolve_principal_permissions` (used by `RequiresPermission`, the menu + filter and the frontend's `auth.permissions`) intersects the resolved set + with the scopes. An admin's wildcard expands to the scope list only. +- `auth.deps.require_permission` skips its admin-role shortcut. +- Role-only admin gates (`/admin`, the Doctor page, the maintenance bypass) use + `simple_module_core.permissions.is_admin_principal`, which is `False` for any + token. + +So an admin's token with scope `feature_flags.view` can `GET /api/feature_flags/` +and nothing else. The cap sits on top of the owner's live grants, so demoting +the owner shrinks every token they hold at once. + +If you write a check that admits admins by role, use `is_admin_principal(user)` +rather than `is_admin(user.roles)`. + +The cap works on **permissions**. A route that asks only for an authenticated +principal (`CurrentUser`, `request.state.user`) and checks no permission is +reachable by any valid token of that user, whatever its scopes. Gate anything +a token should not do with a permission, or refuse +`user.auth_kind != "session"` the way this module's own endpoints do. The +`users` self-service routes (`/api/users/me`, password change, sign out +everywhere) authenticate through the session cookie only and never see a token. + +## Creating tokens + +| Rule | Where | +|---|---| +| Scopes must be non-empty registered permissions the creator holds **now** (`*` is never assignable) | create endpoints | +| Expiry is required and at most `max_lifetime_days` (setting, default 365) | service | +| Service key roles: platform roles only — never `admin`, never `tenant:*` | admin endpoint + service + resolver | +| Only a **session** may create, list or revoke tokens (`auth_kind == "session"`); a token gets 403 | every endpoint and page | +| Cookie-authenticated mutations require `X-CSRF-Token` (`RequiresCsrf`) | every mutation | + +## Tenancy + +The table uses `MultiTenantMixin`; rows are stamped with the active tenant. + +- **Personal tokens** follow their owner like a session would: the tenant + resolver picks from the owner's memberships (or the tenant header). A user's + own list spans every tenant, so a token made in one organisation can be + revoked from another. +- **Service keys** are pinned to the tenant they were created in. The + `tenants` resolver binds that tenant (while it exists and is active) and + ignores headers and subdomains. +- The admin list and admin revoke act on the **active** tenant only. With + strict tenancy and no active tenant the page says so; the API answers 409. + +## Permissions + +| Permission | Default holders | Grants | +|---|---|---| +| `api_tokens.use` | `user`, `tenant:member`/`admin`/`owner` | own personal tokens at `/api-tokens/` | +| `api_tokens.manage` | `admin` | every token in the tenant, service keys, at `/admin/api-tokens/` | + +## Routes + +| Method | Path | Purpose | +|---|---|---| +| `GET` | `/api-tokens/` | Page `ApiTokens/Index` (user dropdown, "API tokens") | +| `GET` | `/admin/api-tokens/` | Page `ApiTokens/Admin` (admin sidebar, Access group) | +| `GET` / `POST` | `/api/api_tokens/tokens/` | List / create own personal tokens | +| `POST` | `/api/api_tokens/tokens/{id}/revoke` | Revoke own token | +| `GET` | `/api/api_tokens/admin/tokens` | Every token in the active tenant | +| `POST` | `/api/api_tokens/admin/service-keys` | Create a service key | +| `POST` | `/api/api_tokens/admin/tokens/{id}/revoke` | Revoke any token in the tenant | + +## Audit + +Create and revoke are recorded by `AuditMixin`. `token_hash` and +`last_used_at` are excluded from the recorded changes. Audit rows link back to +`/admin/api-tokens/?token=` and are labelled `name (prefix…)`. + +## Settings + +| Setting | Default | Meaning | +|---|---|---| +| `max_lifetime_days` | `365` | Longest expiry a new token may get (1–3650). Existing tokens keep theirs. | + +DB-backed (edited in the settings admin), like other module settings. diff --git a/docs/modules/index.md b/docs/modules/index.md index 1fdfafed4..e97fc32a3 100644 --- a/docs/modules/index.md +++ b/docs/modules/index.md @@ -1,12 +1,13 @@ # Bundled modules -simple_module_python ships with thirteen first-party modules. Each is a regular Python package — same shape as a module you'd write yourself — registered through the `simple_module` entry point and discovered at boot. They are independent: install only what you need. +simple_module_python ships with fourteen first-party modules. Each is a regular Python package — same shape as a module you'd write yourself — registered through the `simple_module` entry point and discovered at boot. They are independent: install only what you need. | Module | Depends on | What it provides | |---|---|---| | [`auth`](/modules/auth) | — | Tiny public surface (`UserContext`, `AuthProvider`, `get_current_user`, `require_permission`) other modules import, plus the provider-agnostic `AuthMiddleware` and principal-resolver chain. The actual login/session logic lives in an auth-provider module (`users` or `keycloak`). | | [`users`](/modules/users) | `auth` | Email + password auth, OAuth/OIDC (Google, GitHub, Microsoft/Entra ID, generic OIDC), sessions, roles, invites, password reset, email verification, admin UI, mailer backends, `smpy users create-admin`. | | [`keycloak`](/modules/keycloak) | `auth`, `settings` | Alternative auth provider: Keycloak OIDC single sign-on. | +| [`api_tokens`](/modules/api_tokens) | `auth`, `users`, `settings` | Personal access tokens and service API keys: hashed at rest, scope-capped on every request, revocable on the next request; self-service page and admin screen. | | [`permissions`](/modules/permissions) | `auth`, `users` | Role / direct-grant assignment store + admin UI; `RequiresPermission` dependency that honours both forms. | | [`settings`](/modules/settings) | — | DB-backed key/value store with system / tenant / user precedence; per-module pydantic settings registration; hot reload; `smpy settings` CLI. | | [`feature_flags`](/modules/feature_flags) | — | Runtime feature toggles with system + tenant overrides. | From 5264f908397bf7cb0d87ce34bddf2f1872e45ab9 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 15 Oct 2026 11:26:29 +0200 Subject: [PATCH 5/9] fix: address code review findings (round 1, pass 1) Hide role-gated menu entries from token principals, vary PAT tenant resolution on Authorization, fix scaffold views import order, and add tests for scaffold templates, UserContext token fields and is_admin_principal. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../tests/test_permissions_admin_principal.py | 23 ++++++++++++++++++ .../simple_module_hosting/middleware.py | 5 ++++ .../tests/test_menu_providers_wiring.py | 13 +++++++++- .../tests/test_api_tokens_resolver.py | 23 ++++++++++++++++++ modules/auth/tests/test_user_context.py | 24 +++++++++++++++++++ modules/tenants/tenants/resolver.py | 5 +++- scripts/_templates_endpoints.py | 2 +- scripts/tests/test_scaffold.py | 13 ++++++++++ 8 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 framework/core/tests/test_permissions_admin_principal.py diff --git a/framework/core/tests/test_permissions_admin_principal.py b/framework/core/tests/test_permissions_admin_principal.py new file mode 100644 index 000000000..3f163c6ce --- /dev/null +++ b/framework/core/tests/test_permissions_admin_principal.py @@ -0,0 +1,23 @@ +"""``is_admin_principal``: the role-only admin gate that tokens never pass.""" + +from __future__ import annotations + +from types import SimpleNamespace + +from simple_module_core.permissions import is_admin_principal + + +def test_admin_session_passes() -> None: + assert is_admin_principal(SimpleNamespace(roles=["admin"], scopes=None)) + assert is_admin_principal(SimpleNamespace(roles=["admin"])) # no scopes attribute + + +def test_admin_token_never_passes_even_with_empty_scopes() -> None: + assert not is_admin_principal(SimpleNamespace(roles=["admin"], scopes=["x.read"])) + assert not is_admin_principal(SimpleNamespace(roles=["admin"], scopes=[])) + + +def test_non_admin_and_anonymous_fail() -> None: + assert not is_admin_principal(SimpleNamespace(roles=["user"], scopes=None)) + assert not is_admin_principal(SimpleNamespace(roles=None, scopes=None)) + assert not is_admin_principal(None) diff --git a/framework/hosting/simple_module_hosting/middleware.py b/framework/hosting/simple_module_hosting/middleware.py index 7de536b6f..ff8d5fc26 100644 --- a/framework/hosting/simple_module_hosting/middleware.py +++ b/framework/hosting/simple_module_hosting/middleware.py @@ -217,6 +217,11 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: user = getattr(request.state, "user", None) is_authenticated = user is not None roles = getattr(user, "roles", []) if user else [] + if getattr(user, "scopes", None) is not None: + # A token's menu is filtered on its (capped) permissions alone: a + # ``roles=["admin"]`` entry would otherwise list admin screens the + # token is refused at (see ``is_admin_principal``). + roles = [] # Resolve permissions once — roles plus module grant sources (GH #337) — # and cache on request.state for RequiresPermission and the menu filter. diff --git a/framework/hosting/tests/test_menu_providers_wiring.py b/framework/hosting/tests/test_menu_providers_wiring.py index b9adf52e9..97341a3cd 100644 --- a/framework/hosting/tests/test_menu_providers_wiring.py +++ b/framework/hosting/tests/test_menu_providers_wiring.py @@ -30,7 +30,8 @@ def shared(request: Request) -> JSONResponse: async def fake_auth(request: Request, call_next): role = request.headers.get("x-role") if role: - request.state.user = SimpleNamespace(roles=[role]) + scopes = [] if request.headers.get("x-scoped") else None + request.state.user = SimpleNamespace(roles=[role], scopes=scopes) request.state.tenant_id = request.headers.get("x-tenant", "") return await call_next(request) @@ -71,6 +72,16 @@ def test_sync_provider_and_role_filtering() -> None: assert _labels(client.get("/shared").json()) == [] # anonymous: requires_auth +def test_role_gated_item_is_hidden_from_a_scoped_admin_token() -> None: + menu = MenuRegistry() + menu.add_provider( + lambda _r: [MenuItem(label="Admins", url="/a", roles=["admin"]), MenuItem("All", "/x")] + ) + client = _build(menu) + body = client.get("/shared", headers={"x-role": "admin", "x-scoped": "1"}).json() + assert _labels(body) == ["All"] + + def test_failing_provider_is_isolated(caplog) -> None: menu = MenuRegistry() menu.add(MenuItem(label="Static", url="/s")) diff --git a/modules/api_tokens/tests/test_api_tokens_resolver.py b/modules/api_tokens/tests/test_api_tokens_resolver.py index 0fbb2de62..a54835299 100644 --- a/modules/api_tokens/tests/test_api_tokens_resolver.py +++ b/modules/api_tokens/tests/test_api_tokens_resolver.py @@ -141,3 +141,26 @@ async def spy(scope, token): other = {"type": "http", "headers": [(b"authorization", b"Bearer opaque-session-token")]} await provider.resolve_user(Request(other)) assert calls == ["opaque-session-token"] + + +async def test_personal_token_tenant_answer_varies_on_authorization(app, actor): + """The tenant chosen for a PAT depended on its Authorization header, not a cookie.""" + from api_tokens.resolver import resolve_api_token + from tenants.resolver import resolve_tenant + + async with actor(admin=True) as a: + token = (await create_pat(a.client, ["feature_flags.view"]))["token"] + req = Request( + { + "type": "http", + "method": "GET", + "path": "/", + "headers": [(b"authorization", f"Bearer {token}".encode())], + "app": app, + } + ) + req.state.user = await resolve_api_token(req) + assert req.state.user is not None + resolution = await resolve_tenant(req) + assert "Authorization" in resolution.vary + assert "Cookie" not in resolution.vary diff --git a/modules/auth/tests/test_user_context.py b/modules/auth/tests/test_user_context.py index 40625fd9c..88935a0d8 100644 --- a/modules/auth/tests/test_user_context.py +++ b/modules/auth/tests/test_user_context.py @@ -135,3 +135,27 @@ async def test_has_permission_exact_match(self): assert ctx.has_permission("x.read") is True assert ctx.has_permission("x.write") is False assert ctx.has_permission("x") is False + + +class TestUserContextTokenFields: + async def test_defaults_are_an_uncapped_session(self): + ctx = UserContext(id="1", email="a@b.com", name="A") + assert (ctx.auth_kind, ctx.scopes, ctx.is_scoped) == ("session", None, False) + + async def test_legacy_cookie_decodes_as_uncapped_session(self): + """A cookie from before token principals must not become scoped.""" + payload = {"id": "1", "email": "a@b.com", "name": "A", "roles": ["admin"]} + ctx = UserContext.from_session_dict(payload) + assert ctx is not None + assert (ctx.auth_kind, ctx.scopes) == ("session", None) + + async def test_token_fields_round_trip_and_empty_scopes_stay_capped(self): + for scopes in (["x.read"], []): + ctx = UserContext(id="1", email="", name="A", auth_kind="pat", scopes=scopes) + back = UserContext.from_session_dict(ctx.to_session_dict()) + assert back == ctx + assert back is not None and back.is_scoped + + async def test_malformed_scopes_reject_the_payload(self): + payload = {"id": "1", "email": "a@b.com", "name": "A", "scopes": "x.read"} + assert UserContext.from_session_dict(payload) is None diff --git a/modules/tenants/tenants/resolver.py b/modules/tenants/tenants/resolver.py index 35d210660..9b15ce4a1 100644 --- a/modules/tenants/tenants/resolver.py +++ b/modules/tenants/tenants/resolver.py @@ -18,7 +18,7 @@ import logging from typing import Any -from auth.contracts.tokens import AUTH_KIND_SERVICE +from auth.contracts.tokens import AUTH_KIND_PAT, AUTH_KIND_SERVICE, AUTH_KIND_SESSION from cachetools import TTLCache from fastapi import FastAPI from simple_module_core.invalidation import Invalidation, InvalidationBus @@ -147,6 +147,9 @@ async def _resolve(request: Request) -> tuple[str | None, str | None, tuple[str, # every branch), so a shared cache must key on it too; an anonymous one did # not, and anonymous public pages stay cacheable (#418). credential: tuple[str, ...] = ("Cookie",) if user is not None else () + if getattr(user, "auth_kind", AUTH_KIND_SESSION) == AUTH_KIND_PAT: + # A personal token is carried by the Authorization header, not a cookie. + credential = ("Authorization",) if getattr(user, "auth_kind", None) == AUTH_KIND_SERVICE: # A service key is pinned to the tenant it was created in. It has no # memberships to validate against and no session preference; neither a diff --git a/scripts/_templates_endpoints.py b/scripts/_templates_endpoints.py index 7764e1181..8a8c55123 100644 --- a/scripts/_templates_endpoints.py +++ b/scripts/_templates_endpoints.py @@ -80,8 +80,8 @@ def views_py(ctx: ScaffoldContext) -> str: from __future__ import annotations from fastapi import APIRouter, Depends - from simple_module_inertia import InertiaResponse from simple_module_hosting.inertia_deps import InertiaDep + from simple_module_inertia import InertiaResponse from {ctx.pkg}.deps import get_{ctx.singular}_service from {ctx.pkg}.service import {ctx.singular_class}Service diff --git a/scripts/tests/test_scaffold.py b/scripts/tests/test_scaffold.py index 1a95513e7..313c4fe44 100644 --- a/scripts/tests/test_scaffold.py +++ b/scripts/tests/test_scaffold.py @@ -72,6 +72,19 @@ def test_scaffold_model_is_singular(self, scaffolded_orders: Path): content = (scaffolded_orders / "models.py").read_text() assert "class Order(Base, AuditMixin, table=True):" in content + def test_scaffold_views_import_the_real_inertia_package(self, scaffolded_orders: Path): + """``inertia`` is not a package here; the adapter is ``simple_module_inertia``.""" + content = (scaffolded_orders / "endpoints" / "views.py").read_text() + assert "from simple_module_inertia import InertiaResponse" in content + assert "from inertia import" not in content + + def test_scaffold_js_config_matches_the_workspace(self, scaffolded_orders: Path): + """Inertia v3 peer range, and no ``baseUrl`` (deprecated; paths resolve + relative to the tsconfig) so the scaffold passes ``make lint``.""" + mod_dir = scaffolded_orders.parent + assert '"@inertiajs/react": "^3.7.0"' in (mod_dir / "package.json").read_text() + assert "baseUrl" not in (mod_dir / "tsconfig.json").read_text() + def test_scaffold_rejects_duplicate(self, module_root: Path): (module_root / "modules" / "orders").mkdir() From 8d04a9615077e49716b43b3cca6ffb9bb7c3ee3b Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 15 Oct 2026 12:25:06 +0200 Subject: [PATCH 6/9] fix(api_tokens): address security review findings (round 1, pass 2) - M1: simple_module_hosting.token_guard, an app-wide dependency. API tokens reach only permission-gated, non-page routes (or TokenAllowed). Other routes get a 403; public routes serve the token as anonymous. - M2: PATs are pinned to their creation tenant like service keys and need an active membership there, else 401. Token answers vary on Authorization. - M3: provider bearer credentials are auth_kind "bearer" (unscoped, full rights). Token management requires a cookie session with no Authorization header. - L2: /users/me fails closed on a non-UUID principal. - L5: drop tenant:* and admin from stored service-key roles. - Refuse a PAT for an account the resolver cannot load (Keycloak), with a 400. - L1: "Revoke all" endpoint and button. L1/L3/L4 are documented decisions. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- CHANGELOG.md | 15 +- docs/framework/permissions.md | 23 ++ docs/framework/principal-resolvers.md | 8 +- docs/modules/api_tokens.md | 78 ++++++- .../simple_module_hosting/_module_routes.py | 12 +- .../simple_module_hosting/app_builder.py | 6 +- .../simple_module_hosting/inertia_deps.py | 6 + .../simple_module_hosting/locales/en.json | 3 + .../simple_module_hosting/locales/es.json | 3 + .../simple_module_hosting/permissions.py | 6 + .../simple_module_hosting/token_guard.py | 175 +++++++++++++++ .../hosting/tests/test_token_route_guard.py | 129 +++++++++++ .../api_tokens/contracts/__init__.py | 3 +- .../api_tokens/contracts/schemas.py | 6 + modules/api_tokens/api_tokens/deps.py | 11 +- .../api_tokens/api_tokens/endpoints/api.py | 23 +- modules/api_tokens/api_tokens/locales/en.json | 14 +- modules/api_tokens/api_tokens/pages/Index.tsx | 37 +++- .../pages/components/useTokenActions.ts | 22 +- modules/api_tokens/api_tokens/resolver.py | 15 +- modules/api_tokens/api_tokens/service.py | 24 ++ .../tests/test_api_tokens_hardening.py | 207 ++++++++++++++++++ .../tests/test_api_tokens_route_guard.py | 122 +++++++++++ modules/auth/auth/contracts/schemas.py | 3 +- modules/auth/auth/contracts/tokens.py | 9 +- modules/auth/auth/deps.py | 5 +- modules/auth/tests/test_deps.py | 15 ++ modules/keycloak/keycloak/provider.py | 9 +- .../tests/test_keycloak_bearer_kind.py | 42 ++++ modules/tenants/tenants/resolver.py | 30 +-- modules/tenants/tenants/token_tenant.py | 70 ++++++ modules/users/users/auth_local/views.py | 8 +- modules/users/users/token_strategy.py | 5 +- packages/i18n/src/generated-resources.ts | 9 + packages/i18n/src/keys.generated.ts | 14 ++ 35 files changed, 1102 insertions(+), 65 deletions(-) create mode 100644 framework/hosting/simple_module_hosting/token_guard.py create mode 100644 framework/hosting/tests/test_token_route_guard.py create mode 100644 modules/api_tokens/tests/test_api_tokens_hardening.py create mode 100644 modules/api_tokens/tests/test_api_tokens_route_guard.py create mode 100644 modules/keycloak/tests/test_keycloak_bearer_kind.py create mode 100644 modules/tenants/tenants/token_tenant.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 96028818f..5dad80aa5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,9 +18,10 @@ All notable changes to this project are documented in this file. The format is b cache), so a revoke applies on the next request. Every token carries scopes that cap the principal's permissions on top of the owner's live grants (an admin's token is worth its scopes only). Service keys get roles chosen at - creation (never `admin`) and are pinned to their tenant. Self-service page at + creation (never `admin`); every token is pinned to the tenant it was created + in (a PAT only while its owner is still a member). Self-service page at `/api-tokens/` (user menu), admin screen at `/admin/api-tokens/`; tokens can - only be managed from a session, with `RequiresCsrf`. New PyPI package + only be managed from a cookie session, with `RequiresCsrf`. New PyPI package `simple_module_api_tokens`; not added to the `smpy new` scaffold. Needs a migration (`branch_labels = ("api_tokens",)`). - `UserContext` gains `auth_kind` (`"session"` by default; `"pat"` / @@ -30,6 +31,16 @@ All notable changes to this project are documented in this file. The format is b - `simple_module_hosting.permissions.apply_scope_cap` and `simple_module_core.permissions.is_admin_principal`. A role-only admin gate should use `is_admin_principal(user)`, which is `False` for any token. +- `simple_module_hosting.token_guard` (#455): an app-wide dependency that + refuses an API token (403) on every route without a permission gate + (`RequiresPermission` / `require_permission`) and on every page; a public + route serves it as anonymous. `TokenAllowed()` opts a route in explicitly. + Sessions and provider bearer tokens are unaffected. **Behaviour change** for + any module that relied on a token reaching an ungated route: gate it with a + permission, or mark it `TokenAllowed`. +- `auth.contracts.tokens.AUTH_KIND_BEARER`: a `users` access token or Keycloak + JWT now reports `auth_kind == "bearer"` (was `"session"`). Same rights; it + only stops such credentials from managing API tokens. - `from background_tasks import heartbeat` (#451): a long-running task calls `heartbeat()` to refresh `heartbeat_at` (throttled to one write per 30 s; a no-op outside a task). `@shared_task(..., stuck_after=)` sets a diff --git a/docs/framework/permissions.md b/docs/framework/permissions.md index a75372f93..736f62606 100644 --- a/docs/framework/permissions.md +++ b/docs/framework/permissions.md @@ -101,6 +101,29 @@ class UserContext: The active auth provider (the `users` or `keycloak` module) owns extraction from the session cookie / bearer token. `InertiaLayoutDataMiddleware` then expands roles into a permission set (cached on `request.state.resolved_permissions`) and serializes `auth.user`, `auth.isAuthenticated`, `auth.permissions` into the Inertia shared props. Permissions are derived mostly from roles via the `PermissionRegistry.role_map`. `UserContext.permissions` adds keys the principal carries itself — a token scope or IdP claim a provider or principal resolver maps in. `resolve_principal_permissions` (and so `RequiresPermission`, `require_permission` and `auth.permissions`) merges them into the role-derived set, **dropping `*`**: like a grant source, a claim can never confer the wildcard; admin comes from roles only. They are written into the session cookie with `roles` and frozen there until the next sign-in, so anything an admin must be able to revoke belongs in a grant source, not here. `UserContext.has_permission(key)` is an exact-match check against that list alone. +## Scoped principals (API tokens) and ungated routes + +A principal with `UserContext.scopes` set — an [`api_tokens`](/modules/api_tokens) +personal access token or service key — has its permission set capped at those +scopes. Because the cap can only constrain a route that asks for a permission, +`simple_module_hosting.token_guard` (installed by `create_app` as an app-wide +dependency) applies a fail-closed rule to every route: + +- a route whose dependency tree contains `RequiresPermission(...)` or + `auth.deps.require_permission(...)` admits tokens — the gate decides; +- a page (a module view router, or any route depending on `InertiaDep`) + refuses tokens with 403, gated or not; +- `Depends(TokenAllowed())` from `simple_module_hosting.token_guard` admits + tokens explicitly, without a permission; +- every other route refuses tokens with 403 — except a public route, where the + token's principal is dropped and the request runs anonymously. + +So **gate a route with a permission dependency if API tokens should reach it**; +a permission checked inline in the handler does not count. Sessions and a +provider's own bearer tokens (`auth_kind == "bearer"`) are unscoped and +unaffected. A custom gate dependency can mark itself with +`mark_token_gate(fn, GATE_PERMISSION)`. + ## Role shorthand for menus `MenuItem.roles` hides an item from users who hold none of the listed roles (an empty list = visible to all authenticated users); `requires_auth` (default `True`) hides it from anonymous visitors. Filtering happens in `MenuRegistry.get_for_user`, called by `InertiaLayoutDataMiddleware` *before* the menu reaches the client — so the client never sees menu items it can't use. diff --git a/docs/framework/principal-resolvers.md b/docs/framework/principal-resolvers.md index ef246974e..6bffafde9 100644 --- a/docs/framework/principal-resolvers.md +++ b/docs/framework/principal-resolvers.md @@ -64,8 +64,12 @@ bearer format needs the same carve-out. The shipped [`api_tokens`](/modules/api_tokens) module is the reference implementation: it sets `UserContext.auth_kind` and `scopes`, and the framework -caps the principal's permissions at those scopes (`apply_scope_cap`) and skips -admin-role shortcuts for it (`is_admin_principal`). +caps the principal's permissions at those scopes (`apply_scope_cap`), skips +admin-role shortcuts for it (`is_admin_principal`), and refuses it on every +route without a permission gate and on every page (`token_guard`, see +[Permissions](/framework/permissions#scoped-principals-api-tokens-and-ungated-routes)). +A provider's own bearer credential (a `users` access token, a Keycloak JWT) +reports `auth_kind = "bearer"`: unscoped, full rights, but not a cookie session. ## Worked example — bearer-token resolver diff --git a/docs/modules/api_tokens.md b/docs/modules/api_tokens.md index f1ecc4a55..d312d9192 100644 --- a/docs/modules/api_tokens.md +++ b/docs/modules/api_tokens.md @@ -69,12 +69,30 @@ the owner shrinks every token they hold at once. If you write a check that admits admins by role, use `is_admin_principal(user)` rather than `is_admin(user.roles)`. -The cap works on **permissions**. A route that asks only for an authenticated -principal (`CurrentUser`, `request.state.user`) and checks no permission is -reachable by any valid token of that user, whatever its scopes. Gate anything -a token should not do with a permission, or refuse -`user.auth_kind != "session"` the way this module's own endpoints do. The -`users` self-service routes (`/api/users/me`, password change, sign out +### Which routes a token may reach + +The cap works on **permissions**, so on its own it could not constrain a route +that asks only for a signed-in principal. The framework closes that gap with a +fail-closed rule, enforced by `simple_module_hosting.token_guard` as an +app-wide dependency on every `APIRoute` (module, host and framework routes): + +| Route | A token (`scopes` set) gets | +|---|---| +| Has a permission gate in its dependency tree — `RequiresPermission(...)` or `auth.deps.require_permission(...)` | admitted; the gate decides, capped at the scopes | +| A page — on a module view router, or depends on `InertiaDep` | **403**, even when gated: page props are a second, unscoped read API | +| Marked `Depends(TokenAllowed())` | admitted, with no permission check — use only for routes harmless for any token | +| Anything else | **403** (JSON on `/api/*`) | +| A public route (exempted from auth) without a gate | served, but **as anonymous**: the token's principal is dropped | + +So a token cannot create or leave an organisation (`/api/tenants/`), switch +tenant, open `/users/me`, the dashboard or any other page, whatever its scopes. +The decision is computed once per route from its dependency tree and cached. +Browser sessions and a provider's own bearer tokens (`auth_kind == "bearer"`) +are unscoped and never affected. A route that checks a permission inline in its +handler rather than through a dependency is refused for tokens; move the check +into `RequiresPermission` if tokens should reach it. + +The `users` self-service routes (`/api/users/me`, password change, sign out everywhere) authenticate through the session cookie only and never see a token. ## Creating tokens @@ -84,20 +102,26 @@ everywhere) authenticate through the session cookie only and never see a token. | Scopes must be non-empty registered permissions the creator holds **now** (`*` is never assignable) | create endpoints | | Expiry is required and at most `max_lifetime_days` (setting, default 365) | service | | Service key roles: platform roles only — never `admin`, never `tenant:*` | admin endpoint + service + resolver | -| Only a **session** may create, list or revoke tokens (`auth_kind == "session"`); a token gets 403 | every endpoint and page | +| Only a **cookie session** may create, list or revoke tokens: `auth_kind == "session"` and no `Authorization` header. A token, a `users` access token or a Keycloak JWT gets 403 | every endpoint and page | +| The creator must have a local `users_user` row the resolver can load (under Keycloak, a cached user); otherwise 400 `api_tokens.errors.account_required` | personal create endpoint | | Cookie-authenticated mutations require `X-CSRF-Token` (`RequiresCsrf`) | every mutation | ## Tenancy The table uses `MultiTenantMixin`; rows are stamped with the active tenant. -- **Personal tokens** follow their owner like a session would: the tenant - resolver picks from the owner's memberships (or the tenant header). A user's - own list spans every tenant, so a token made in one organisation can be - revoked from another. +- **Personal tokens** are pinned to the tenant they were created in, exactly + like service keys: the `tenants` resolver binds that tenant and ignores the + tenant header, subdomains and the session preference. The owner must still be + an **active member** of it: once they are removed, or the tenant is suspended, + the token is de-authenticated (401) — it does not fall back to another of the + owner's organisations. A user's own list spans every tenant, so a token made + in one organisation can be revoked from another; the tenant's admins see it + in `/admin/api-tokens/` of that tenant. - **Service keys** are pinned to the tenant they were created in. The `tenants` resolver binds that tenant (while it exists and is active) and - ignores headers and subdomains. + ignores headers and subdomains. A token's tenant answer varies on + `Authorization` only. - The admin list and admin revoke act on the **active** tenant only. With strict tenancy and no active tenant the page says so; the API answers 409. @@ -116,10 +140,40 @@ The table uses `MultiTenantMixin`; rows are stamped with the active tenant. | `GET` | `/admin/api-tokens/` | Page `ApiTokens/Admin` (admin sidebar, Access group) | | `GET` / `POST` | `/api/api_tokens/tokens/` | List / create own personal tokens | | `POST` | `/api/api_tokens/tokens/{id}/revoke` | Revoke own token | +| `POST` | `/api/api_tokens/tokens/revoke-all` | Revoke every own personal token ("Revoke all" on the page) | | `GET` | `/api/api_tokens/admin/tokens` | Every token in the active tenant | | `POST` | `/api/api_tokens/admin/service-keys` | Create a service key | | `POST` | `/api/api_tokens/admin/tokens/{id}/revoke` | Revoke any token in the tenant | +## Security model + +Decisions made after the security review of #455, and why: + +- **Tokens survive a password change and "sign out everywhere".** That matches + GitHub and most platforms: a token is a deliberate, separately managed + credential, and a password rotation should not silently break every CI job. + End them explicitly — individually, or with **Revoke all** on `/api-tokens/` + (`POST /api/api_tokens/tokens/revoke-all`), which is the step to take after + a suspected compromise. +- **Re-enabling a disabled user revives their unexpired tokens.** Disabling is + a reversible suspension; the resolver refuses every token while the account + is disabled. If the account was disabled because of a compromise, revoke its + tokens (the admin list) before re-enabling it. +- **No token-specific rate limit.** Token requests are signed-in requests and + go through the existing middleware only. A token is 256 bits, so guessing is + not a practical attack; each well-formed bearer costs one indexed lookup. +- **Token management needs a person at a keyboard** — a cookie session with no + `Authorization` header. A `users` access token (`/api/users/auth/token`) or a + Keycloak JWT is a `"bearer"` principal: it keeps its user's full rights + (no scope cap) but cannot mint, list or revoke tokens, so a short-lived + credential cannot be turned into a year-long one. +- **Tokens never reach pages or ungated routes** (see *Which routes a token may + reach*). A service key's `svc:` principal therefore never reaches code that + expects an account id. +- **Stored roles are re-filtered**: the resolver drops `admin` and `tenant:*` + from a service key's row, so a hand-edited row cannot make a key an admin or + a tenant owner. + ## Audit Create and revoke are recorded by `AuditMixin`. `token_hash` and diff --git a/framework/hosting/simple_module_hosting/_module_routes.py b/framework/hosting/simple_module_hosting/_module_routes.py index 037d221cc..13731dd53 100644 --- a/framework/hosting/simple_module_hosting/_module_routes.py +++ b/framework/hosting/simple_module_hosting/_module_routes.py @@ -6,9 +6,11 @@ from __future__ import annotations -from fastapi import APIRouter, FastAPI +from fastapi import APIRouter, Depends, FastAPI from fastapi.routing import APIRoute +from simple_module_hosting.token_guard import page_route + __all__ = ["wire_module_routes"] @@ -24,7 +26,12 @@ def wire_module_routes(app: FastAPI, module) -> bool: a new router type. """ api_router = APIRouter(prefix=module.meta.route_prefix, tags=[module.meta.name]) - view_router = APIRouter(prefix=module.meta.view_prefix, tags=[f"{module.meta.name} Views"]) + # View routers carry the page marker: an API token is refused on every + # page, gated or not (``token_guard``). + pages = [Depends(page_route)] + view_router = APIRouter( + prefix=module.meta.view_prefix, tags=[f"{module.meta.name} Views"], dependencies=pages + ) module.register_routes(api_router, view_router) _clone_bare_prefix_route(view_router, module.meta.view_prefix) has_views = bool(view_router.routes) @@ -39,6 +46,7 @@ def wire_module_routes(app: FastAPI, module) -> bool: admin_router = APIRouter( prefix=admin_prefix, tags=[f"{module.meta.name} Admin Views"], + dependencies=pages, ) module.register_admin_routes(admin_router) _clone_bare_prefix_route(admin_router, admin_prefix) diff --git a/framework/hosting/simple_module_hosting/app_builder.py b/framework/hosting/simple_module_hosting/app_builder.py index 55f58c904..85323436b 100644 --- a/framework/hosting/simple_module_hosting/app_builder.py +++ b/framework/hosting/simple_module_hosting/app_builder.py @@ -4,7 +4,7 @@ import logging -from fastapi import FastAPI +from fastapi import Depends, FastAPI from simple_module_core import BodyLimitRegistry, CspSourceRegistry from simple_module_core.audit_links import AuditLinkRegistry from simple_module_core.design_packs import DesignPackRegistry @@ -50,6 +50,7 @@ from simple_module_hosting.setup_wizard import mount_setup_wizard from simple_module_hosting.static_files import PrecompressedStaticFiles from simple_module_hosting.tenancy import mode_for +from simple_module_hosting.token_guard import token_route_guard logger = logging.getLogger(__name__) @@ -150,6 +151,9 @@ def create_app(settings: Settings | None = None) -> FastAPI: docs_url=_DOCS_URL if settings.is_development else None, redoc_url=_REDOC_URL if settings.is_development else None, lifespan=lifespan, + # API tokens reach only permission-gated, non-page routes (or ones + # marked ``TokenAllowed``); see ``token_guard``. + dependencies=[Depends(token_route_guard)], ) # ── Phase 4: Module settings ─────────────────────────── diff --git a/framework/hosting/simple_module_hosting/inertia_deps.py b/framework/hosting/simple_module_hosting/inertia_deps.py index ee83190b6..64a3d980d 100644 --- a/framework/hosting/simple_module_hosting/inertia_deps.py +++ b/framework/hosting/simple_module_hosting/inertia_deps.py @@ -7,6 +7,8 @@ from fastapi import Depends, Request from simple_module_inertia import Inertia +from simple_module_hosting.token_guard import GATE_PAGE, mark_token_gate + async def get_inertia(request: Request) -> Inertia: """Resolve the Inertia instance from the app's configured dependency. @@ -24,4 +26,8 @@ async def get_inertia(request: Request) -> Inertia: return inertia +# A route rendering a page refuses API tokens even behind a permission gate +# (``token_guard``): its props are a second, unscoped read API. +mark_token_gate(get_inertia, GATE_PAGE) + InertiaDep = Annotated[Inertia, Depends(get_inertia)] diff --git a/framework/hosting/simple_module_hosting/locales/en.json b/framework/hosting/simple_module_hosting/locales/en.json index 8ee076b9d..a1acbd149 100644 --- a/framework/hosting/simple_module_hosting/locales/en.json +++ b/framework/hosting/simple_module_hosting/locales/en.json @@ -20,5 +20,8 @@ }, "working": "Working…", "done": "Done. Reloading…" + }, + "errors": { + "token_not_allowed": "API tokens cannot be used on this route. Use a signed-in session, or a route gated by a permission the token is scoped to." } } diff --git a/framework/hosting/simple_module_hosting/locales/es.json b/framework/hosting/simple_module_hosting/locales/es.json index 59868a7fc..73b095b57 100644 --- a/framework/hosting/simple_module_hosting/locales/es.json +++ b/framework/hosting/simple_module_hosting/locales/es.json @@ -20,5 +20,8 @@ }, "working": "Procesando…", "done": "Hecho. Recargando…" + }, + "errors": { + "token_not_allowed": "Los tokens de API no se pueden usar en esta ruta. Usa una sesión iniciada o una ruta protegida por un permiso incluido en el token." } } diff --git a/framework/hosting/simple_module_hosting/permissions.py b/framework/hosting/simple_module_hosting/permissions.py index c5a9b895e..1384c5c89 100644 --- a/framework/hosting/simple_module_hosting/permissions.py +++ b/framework/hosting/simple_module_hosting/permissions.py @@ -8,6 +8,8 @@ from fastapi import HTTPException, Request from simple_module_core.permissions import DEFAULT_ROLE_PERMISSIONS, WILDCARD, grants +from simple_module_hosting.token_guard import GATE_PERMISSION + if TYPE_CHECKING: from simple_module_core.permissions import PermissionRegistry @@ -193,6 +195,10 @@ async def create_product(...): ... """ + # Tells ``token_guard`` this route is permission-gated, so an API token + # may reach it — capped at its scopes by the check below. + __sm_token_gate__ = GATE_PERMISSION + def __init__(self, permission: str) -> None: self.permission = permission diff --git a/framework/hosting/simple_module_hosting/token_guard.py b/framework/hosting/simple_module_hosting/token_guard.py new file mode 100644 index 000000000..ee9654196 --- /dev/null +++ b/framework/hosting/simple_module_hosting/token_guard.py @@ -0,0 +1,175 @@ +"""Which routes an API-token principal may reach. + +A token (``api_tokens``' personal access tokens and service keys) is a +*scoped* principal: ``scopes`` is a list, and every permission check caps what +it holds at that list. The cap can only constrain a route that asks for a +permission, though — a route that needs nothing but a signed-in principal +(create an organisation, leave one, read your profile) would otherwise accept +any token, whatever it was scoped to. + +So the rule is fail-closed and decided per route, from its dependency tree: + +* A route whose tree holds a **permission gate** (``RequiresPermission``, the + auth module's ``require_permission``) admits tokens — the gate, capped at + the token's scopes, is what decides. +* A **page** route (anything rendering through Inertia, and every route on a + module's view router) refuses tokens even when gated: tokens are API + credentials, and a page's props are a second, unscoped read API. +* :class:`TokenAllowed` on a route admits tokens explicitly, for a route that + is safe for any token without naming a permission. +* Everything else refuses a token with ``403``. On a route the public-route + registry exempts, the token is not refused but **ignored**: the request runs + as the anonymous visitor it could have been anyway. + +Installed once as an app-wide dependency by ``create_app``, so it covers every +``APIRoute`` — module routers, host routes and framework routes alike. Gates +mark themselves with :data:`TOKEN_GATE_ATTR` (duck-typed, so the auth +module's ``require_permission`` can mark itself without framework code +importing a module — SM009). An unscoped principal (a browser session, an +identity provider's bearer token) is never affected. +""" + +from __future__ import annotations + +import contextlib +from collections.abc import Callable +from typing import Any + +from fastapi import HTTPException, Request +from simple_module_db.listeners import current_user_id + +TOKEN_GATE_ATTR = "__sm_token_gate__" +"""Attribute a dependency callable carries to tell the guard what it is.""" + +GATE_PERMISSION = "permission" +"""A permission check: admits tokens, which it caps at their scopes.""" + +GATE_ALLOW = "allow" +"""An explicit opt-in: tokens may reach this route without a permission.""" + +GATE_PAGE = "page" +"""A page (Inertia) route: tokens are refused even behind a permission.""" + +_POLICY_ATTR = "_sm_token_policy" + + +def mark_token_gate[F: Callable[..., Any]](fn: F, kind: str) -> F: + """Tag *fn* (a dependency callable) as a gate of *kind*; returns *fn*.""" + setattr(fn, TOKEN_GATE_ATTR, kind) + return fn + + +class TokenAllowed: + """Dependency marker: API tokens may call this route without a permission. + + Use it only for a route whose effect is harmless for *any* token of the + principal's, whatever it was scoped to — the route's own code is then the + only check. Prefer a ``RequiresPermission`` gate, which a scope can name. + + Usage:: + + @router.get("/ping", dependencies=[Depends(TokenAllowed())]) + """ + + __sm_token_gate__ = GATE_ALLOW + + async def __call__(self) -> None: + return None + + +async def page_route() -> None: + """Dependency marker for a page route (added to every module view router).""" + + +mark_token_gate(page_route, GATE_PAGE) + + +def _gate_kinds(dependant: Any, out: set[str], seen: set[int]) -> None: + if id(dependant) in seen: + return + seen.add(id(dependant)) + kind = getattr(getattr(dependant, "call", None), TOKEN_GATE_ATTR, None) + if isinstance(kind, str): + out.add(kind) + for sub in getattr(dependant, "dependencies", None) or (): + _gate_kinds(sub, out, seen) + + +def route_admits_tokens(route: Any) -> bool: + """Whether *route* (the matched ``APIRoute``) may serve a token principal. + + Computed from the route's dependency tree and cached on the route object; + anything unexpected answers ``False`` (refuse). + """ + cached = getattr(route, _POLICY_ATTR, None) + if isinstance(cached, bool): + return cached + dependant = getattr(route, "dependant", None) + if dependant is None: + return False + kinds: set[str] = set() + _gate_kinds(dependant, kinds, set()) + if GATE_ALLOW in kinds: + admits = True + elif GATE_PAGE in kinds: + admits = False + else: + admits = GATE_PERMISSION in kinds + with contextlib.suppress(AttributeError, TypeError): # a slotted/frozen route + setattr(route, _POLICY_ATTR, admits) + return admits + + +def is_token_principal(user: Any) -> bool: + """A scoped principal — an API token — as opposed to a session or IdP bearer.""" + return user is not None and getattr(user, "scopes", None) is not None + + +def _translate(request: Request, key: str, fallback: str) -> str: + sm = getattr(request.app.state, "sm", None) + registry = getattr(sm, "i18n_registry", None) + if registry is None: + return fallback + from simple_module_core.i18n import Translator + + default = sm.settings.i18n_default_locale + locale = getattr(request.state, "locale", default) + return Translator(registry, locale=locale, default_locale=default).t(key) + + +async def token_route_guard(request: Request) -> None: + """App-wide dependency enforcing the rule in this module's docstring.""" + user = getattr(request.state, "user", None) + if not is_token_principal(user): + return + if route_admits_tokens(request.scope.get("route")): + return + if (request.scope.get("state") or {}).get("auth_public"): + # A public route: serve the token holder as the anonymous visitor + # anyone may be, never as the account behind the token. + request.state.user = None + request.state.resolved_permissions = set() + current_user_id.set(None) + return + raise HTTPException( + status_code=403, + detail=_translate( + request, + "hosting.errors.token_not_allowed", + "API tokens cannot be used on this route.", + ), + ) + + +__all__ = [ + "GATE_ALLOW", + "GATE_PAGE", + "GATE_PERMISSION", + "TOKEN_GATE_ATTR", + "TokenAllowed", + "is_token_principal", + "mark_token_gate", + "page_route", + "route_admits_tokens", + "token_route_guard", +] diff --git a/framework/hosting/tests/test_token_route_guard.py b/framework/hosting/tests/test_token_route_guard.py new file mode 100644 index 000000000..984140a22 --- /dev/null +++ b/framework/hosting/tests/test_token_route_guard.py @@ -0,0 +1,129 @@ +"""``token_guard``: an API token reaches only permission-gated, non-page routes. + +Security review M1: the scope cap only constrains a route that asks for a +permission, so a route needing nothing but a signed-in principal accepted any +token. The guard decides per route from its dependency tree, fail-closed. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field + +import httpx +from fastapi import APIRouter, Depends, FastAPI, Request +from simple_module_hosting.inertia_deps import InertiaDep +from simple_module_hosting.permissions import RequiresPermission +from simple_module_hosting.token_guard import ( + TokenAllowed, + page_route, + route_admits_tokens, + token_route_guard, +) + + +@dataclass +class _Principal: + id: str = "u1" + roles: list[str] = field(default_factory=lambda: ["user"]) + permissions: list[str] = field(default_factory=list) + scopes: list[str] | None = None + + +def _app(principal: _Principal | None, *, public: bool = False) -> FastAPI: + app = FastAPI(dependencies=[Depends(token_route_guard)]) + app.state.sm = None + api = APIRouter(prefix="/api") + + @api.get("/open") + async def open_route(request: Request) -> dict: + return {"user": getattr(request.state.user, "id", None)} + + @api.get("/gated", dependencies=[Depends(RequiresPermission("x.view"))]) + async def gated() -> dict: + return {"ok": True} + + @api.get("/allowed", dependencies=[Depends(TokenAllowed())]) + async def allowed() -> dict: + return {"ok": True} + + pages = APIRouter(prefix="/pages", dependencies=[Depends(page_route)]) + + @pages.get("/gated", dependencies=[Depends(RequiresPermission("x.view"))]) + async def gated_page() -> dict: + return {"ok": True} + + app.include_router(api) + app.include_router(pages) + + @app.middleware("http") + async def fake_auth(request: Request, call_next): + request.state.user = principal + request.scope.setdefault("state", {})["auth_public"] = public + return await call_next(request) + + return app + + +async def _get(app: FastAPI, path: str) -> httpx.Response: + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=app), base_url="http://t" + ) as client: + return await client.get(path) + + +TOKEN = _Principal(roles=["admin"], scopes=["x.view"]) + + +async def test_token_refused_on_a_route_without_a_permission_gate(): + resp = await _get(_app(TOKEN), "/api/open") + assert resp.status_code == 403 + + +async def test_token_admitted_on_a_permission_gated_route(): + app = _app(_Principal(scopes=["x.view"], permissions=["x.view"])) + assert (await _get(app, "/api/gated")).status_code == 200 + + +async def test_token_admitted_on_an_explicitly_allowed_route(): + assert (await _get(_app(TOKEN), "/api/allowed")).status_code == 200 + + +async def test_token_refused_on_a_page_even_when_gated(): + app = _app(_Principal(scopes=["x.view"], permissions=["x.view"])) + assert (await _get(app, "/pages/gated")).status_code == 403 + + +async def test_session_principal_is_never_affected(): + app = _app(_Principal(scopes=None)) + assert (await _get(app, "/api/open")).status_code == 200 + + +async def test_token_on_a_public_route_runs_as_anonymous(): + resp = await _get(_app(TOKEN, public=True), "/api/open") + assert resp.status_code == 200 + assert resp.json() == {"user": None} + + +def _route(app: FastAPI, path: str): + return next(r for r in app.router.routes if getattr(r, "path", "") == path) + + +def test_inertia_dependency_marks_a_route_as_a_page(): + app = FastAPI() + + @app.get("/p", dependencies=[Depends(RequiresPermission("x.view"))]) + async def page(inertia: InertiaDep) -> None: + return None + + assert route_admits_tokens(_route(app, "/p")) is False + + +def test_a_route_with_no_gate_refuses_tokens(): + app = FastAPI() + + @app.get("/n") + async def nothing() -> None: + return None + + assert route_admits_tokens(_route(app, "/n")) is False + assert route_admits_tokens(None) is False diff --git a/modules/api_tokens/api_tokens/contracts/__init__.py b/modules/api_tokens/api_tokens/contracts/__init__.py index b2ff6476b..62c6400a6 100644 --- a/modules/api_tokens/api_tokens/contracts/__init__.py +++ b/modules/api_tokens/api_tokens/contracts/__init__.py @@ -3,8 +3,9 @@ from api_tokens.contracts.schemas import ( CreatedToken, PersonalTokenCreate, + RevokedCount, ServiceKeyCreate, TokenView, ) -__all__ = ["CreatedToken", "PersonalTokenCreate", "ServiceKeyCreate", "TokenView"] +__all__ = ["CreatedToken", "PersonalTokenCreate", "RevokedCount", "ServiceKeyCreate", "TokenView"] diff --git a/modules/api_tokens/api_tokens/contracts/schemas.py b/modules/api_tokens/api_tokens/contracts/schemas.py index ad1fc5f78..2a8f762d2 100644 --- a/modules/api_tokens/api_tokens/contracts/schemas.py +++ b/modules/api_tokens/api_tokens/contracts/schemas.py @@ -62,3 +62,9 @@ class ServiceKeyCreate(_CreateBase): """Create a service key. ``roles`` may not include ``admin``.""" roles: list[str] = Field(default_factory=list, max_length=50) + + +class RevokedCount(SQLModel): + """How many tokens a bulk revoke ended.""" + + revoked: int diff --git a/modules/api_tokens/api_tokens/deps.py b/modules/api_tokens/api_tokens/deps.py index c202947ce..e2669d1e5 100644 --- a/modules/api_tokens/api_tokens/deps.py +++ b/modules/api_tokens/api_tokens/deps.py @@ -29,13 +29,18 @@ async def require_session_principal(request: Request, t: TranslatorDep) -> UserC Tokens may not mint, list or revoke tokens: a leaked token must not be able to make itself permanent by creating a sibling, or hide by revoking - the one an administrator would look for. + the one an administrator would look for. That covers a provider's own + bearer credential too (``auth_kind == "bearer"``): a fifteen-minute access + token must not become a year-long PAT. Only a cookie session qualifies, so + any request carrying an ``Authorization`` header is refused outright. """ user = getattr(request.state, "user", None) if user is None: raise HTTPException(status_code=401, detail=t.t("auth.errors.not_authenticated")) - if getattr(user, "auth_kind", AUTH_KIND_SESSION) != AUTH_KIND_SESSION or ( - getattr(user, "scopes", None) is not None + if ( + getattr(user, "auth_kind", None) != AUTH_KIND_SESSION + or getattr(user, "scopes", None) is not None + or "authorization" in request.headers ): raise HTTPException(status_code=403, detail=t.t("api_tokens.errors.session_required")) return user diff --git a/modules/api_tokens/api_tokens/endpoints/api.py b/modules/api_tokens/api_tokens/endpoints/api.py index 9e0f475bf..61044b1ae 100644 --- a/modules/api_tokens/api_tokens/endpoints/api.py +++ b/modules/api_tokens/api_tokens/endpoints/api.py @@ -16,7 +16,12 @@ from simple_module_hosting.permissions import RequiresPermission from api_tokens.constants import PERM_USE, TokenKind -from api_tokens.contracts.schemas import CreatedToken, PersonalTokenCreate, TokenView +from api_tokens.contracts.schemas import ( + CreatedToken, + PersonalTokenCreate, + RevokedCount, + TokenView, +) from api_tokens.deps import ( ApiTokenServiceDep, SessionPrincipal, @@ -56,6 +61,10 @@ async def create_token( t: TranslatorDep, ) -> CreatedToken: owner = user_uuid(user, t) + if not await service.has_usable_account(owner): + # No local account the resolver could load (e.g. Keycloak): refuse + # instead of minting a token that can never authenticate. + raise HTTPException(status_code=400, detail=t.t("api_tokens.errors.account_required")) try: scopes = check_subset( body.scopes, await assignable_scopes(request), "api_tokens.errors.scope_not_held" @@ -77,6 +86,18 @@ async def create_token( return CreatedToken(token=plaintext, item=service.to_view(token)) +@router.post("/revoke-all", response_model=RevokedCount) +async def revoke_all_tokens( + user: SessionPrincipal, service: ApiTokenServiceDep, t: TranslatorDep +) -> RevokedCount: + """Revoke every personal token the signed-in user holds — the panic button. + + Tokens deliberately survive a password change and "sign out everywhere" + (see docs/modules/api_tokens.md, Security model); this is how to end them. + """ + return RevokedCount(revoked=await service.revoke_all_for_owner(user_uuid(user, t))) + + @router.post("/{token_id}/revoke", response_model=TokenView) async def revoke_token( token_id: uuid.UUID, user: SessionPrincipal, service: ApiTokenServiceDep, t: TranslatorDep diff --git a/modules/api_tokens/api_tokens/locales/en.json b/modules/api_tokens/api_tokens/locales/en.json index 8e6d4b83b..72073363e 100644 --- a/modules/api_tokens/api_tokens/locales/en.json +++ b/modules/api_tokens/api_tokens/locales/en.json @@ -14,7 +14,8 @@ "lifetime_too_long": "A token can be valid for at most {days} days.", "invalid_kind": "This kind of token cannot be created here.", "tenant_required": "Choose an organisation before managing its tokens.", - "not_found": "Token not found." + "not_found": "Token not found.", + "account_required": "Your account has no local user record, so a personal access token could never sign in as you. Ask an administrator for a service key instead." }, "index": { "title": "API tokens", @@ -89,6 +90,15 @@ "toasts": { "created": "Token created", "revoked": "Token revoked", - "failed": "Something went wrong. Please try again." + "failed": "Something went wrong. Please try again.", + "revoked_all_one": "{count} token revoked", + "revoked_all_other": "{count} tokens revoked" + }, + "revoke_all": { + "button": "Revoke all", + "title": "Revoke all your tokens?", + "description": "Every personal access token you hold stops working on its next request, in every organisation. Use this if a token may have leaked: changing your password or signing out everywhere does not end tokens.", + "confirm": "Revoke all", + "cancel": "Cancel" } } diff --git a/modules/api_tokens/api_tokens/pages/Index.tsx b/modules/api_tokens/api_tokens/pages/Index.tsx index 2a252f7eb..54da541ab 100644 --- a/modules/api_tokens/api_tokens/pages/Index.tsx +++ b/modules/api_tokens/api_tokens/pages/Index.tsx @@ -1,16 +1,17 @@ import { Head, usePage } from '@inertiajs/react'; import { keys, useT } from '@simple-module-py/i18n'; +import { ConfirmActionDialog } from '@simple-module-py/ui/components/ConfirmActionDialog'; import { PageShell } from '@simple-module-py/ui/components/PageShell'; import { Button } from '@simple-module-py/ui/components/ui/button'; import { AuthenticatedLayout } from '@simple-module-py/ui/layouts/AuthenticatedLayout'; -import { Plus } from 'lucide-react'; +import { Ban, Plus } from 'lucide-react'; import { useState } from 'react'; import { CreatedTokenDialog } from './components/CreatedTokenDialog'; import { CreateTokenDialog } from './components/CreateTokenDialog'; import { RevokeTokenDialog } from './components/RevokeTokenDialog'; import { TokenTable } from './components/TokenTable'; import { useTokenActions } from './components/useTokenActions'; -import type { CreatePayload, TokenFormProps } from './types'; +import { type CreatePayload, type TokenFormProps, tokenStatus } from './types'; /** Your own personal access tokens: list, create (secret shown once), revoke. */ function Index() { @@ -21,7 +22,9 @@ function Index() { csrfToken: props.csrf_token, createUrl: `${props.api_url}/tokens/`, revokeUrl: (id) => `${props.api_url}/tokens/${id}/revoke`, + revokeAllUrl: `${props.api_url}/tokens/revoke-all`, }); + const hasLive = props.tokens.some((tok) => tokenStatus(tok) === 'active'); async function submit(payload: CreatePayload) { const ok = await actions.create(payload); @@ -36,10 +39,20 @@ function Index() { title={t(keys.api_tokens.index.title)} description={t(keys.api_tokens.index.description)} actions={ - +
+ + +
} >

{t(keys.api_tokens.index.usage)}

@@ -62,6 +75,18 @@ function Index() { onCancel={() => actions.setPendingRevoke(null)} onConfirm={actions.confirmRevoke} /> + !open && actions.setPendingRevokeAll(false)} + tone="destructive" + icon={Ban} + title={t(keys.api_tokens.revoke_all.title)} + description={t(keys.api_tokens.revoke_all.description)} + confirmLabel={t(keys.api_tokens.revoke_all.confirm)} + cancelLabel={t(keys.api_tokens.revoke_all.cancel)} + busy={actions.busy} + onConfirm={actions.confirmRevokeAll} + /> ); } diff --git a/modules/api_tokens/api_tokens/pages/components/useTokenActions.ts b/modules/api_tokens/api_tokens/pages/components/useTokenActions.ts index 5b34752cb..91715f656 100644 --- a/modules/api_tokens/api_tokens/pages/components/useTokenActions.ts +++ b/modules/api_tokens/api_tokens/pages/components/useTokenActions.ts @@ -9,6 +9,8 @@ interface Options { csrfToken: string; createUrl: string; revokeUrl: (id: string) => string; + /** Bulk revoke of every token the caller holds; omitted on the admin page. */ + revokeAllUrl?: string; } interface Created { @@ -17,10 +19,11 @@ interface Created { } /** Create / revoke through the JSON API, then reload the page's `tokens` prop. */ -export function useTokenActions({ csrfToken, createUrl, revokeUrl }: Options) { +export function useTokenActions({ csrfToken, createUrl, revokeUrl, revokeAllUrl }: Options) { const { t } = useT(); const [created, setCreated] = useState(null); const [pendingRevoke, setPendingRevoke] = useState(null); + const [pendingRevokeAll, setPendingRevokeAll] = useState(false); const [busy, setBusy] = useState(false); async function create(payload: CreatePayload): Promise { @@ -50,6 +53,20 @@ export function useTokenActions({ csrfToken, createUrl, revokeUrl }: Options) { router.reload({ only: ['tokens'] }); } + async function confirmRevokeAll() { + if (!revokeAllUrl) return; + setBusy(true); + const result = await postJson<{ revoked: number }>(revokeAllUrl, csrfToken); + setBusy(false); + setPendingRevokeAll(false); + if (!result.ok || !result.data) { + toast.error(result.detail ?? t(keys.api_tokens.toasts.failed)); + return; + } + toast.success(t(keys.api_tokens.toasts.revoked_all, { count: result.data.revoked })); + router.reload({ only: ['tokens'] }); + } + return { busy, created, @@ -58,5 +75,8 @@ export function useTokenActions({ csrfToken, createUrl, revokeUrl }: Options) { pendingRevoke, setPendingRevoke, confirmRevoke, + pendingRevokeAll, + setPendingRevokeAll, + confirmRevokeAll, }; } diff --git a/modules/api_tokens/api_tokens/resolver.py b/modules/api_tokens/api_tokens/resolver.py index ad204c35e..8476fb44e 100644 --- a/modules/api_tokens/api_tokens/resolver.py +++ b/modules/api_tokens/api_tokens/resolver.py @@ -19,6 +19,7 @@ from auth.contracts.schemas import UserContext from auth.contracts.tokens import AUTH_KIND_PAT, AUTH_KIND_SERVICE, is_api_token +from simple_module_core.tenancy import is_tenant_role from simple_module_db import ALL_TENANTS_OPTION from sqlalchemy import or_, select, update from sqlalchemy.orm import noload, selectinload @@ -66,7 +67,12 @@ async def _personal(db: Any, token: ApiToken) -> UserContext | None: user = (await db.execute(stmt)).scalar_one_or_none() if user is None or not user.is_active or user.disabled_at is not None: return None - return UserContext.from_user(user, auth_kind=AUTH_KIND_PAT, scopes=list(token.scopes)) + principal = UserContext.from_user(user, auth_kind=AUTH_KIND_PAT, scopes=list(token.scopes)) + # Pinned to the tenant it was created in, like a service key: the tenant + # resolver acts for this tenant only (and only while the owner is still an + # active member of it), never one named by a header, subdomain or session. + principal.tenant_id = token.tenant_id + return principal def _service(token: ApiToken) -> UserContext: @@ -75,8 +81,11 @@ def _service(token: ApiToken) -> UserContext: email="", name=token.name, # Refused at creation; dropped here too so a hand-edited row cannot - # turn a service key into an admin. - roles=[r for r in token.roles if r not in FORBIDDEN_SERVICE_ROLES], + # turn a service key into an admin — or a tenant owner: ``tenant:*`` + # roles come only from a membership, which a service key never has. + roles=[ + r for r in token.roles if r not in FORBIDDEN_SERVICE_ROLES and not is_tenant_role(r) + ], tenant_id=token.tenant_id, auth_kind=AUTH_KIND_SERVICE, scopes=list(token.scopes), diff --git a/modules/api_tokens/api_tokens/service.py b/modules/api_tokens/api_tokens/service.py index 97641de78..7841fa2ba 100644 --- a/modules/api_tokens/api_tokens/service.py +++ b/modules/api_tokens/api_tokens/service.py @@ -80,6 +80,30 @@ async def create( await self.db.flush() return token, new.plaintext + async def has_usable_account(self, user_id: uuid.UUID) -> bool: + """Whether *user_id* is an active ``users_user`` row the resolver can load. + + Under an auth provider that keeps no local account (Keycloak without a + cached user), a token minted for the id would be dead on arrival. + """ + from users.models import User + + stmt = select(User.id).where( + User.id == user_id, User.is_active.is_(True), User.disabled_at.is_(None) + ) + return (await self.db.execute(stmt)).first() is not None + + async def revoke_all_for_owner(self, owner_user_id: uuid.UUID) -> int: + """Revoke every live personal token *owner_user_id* holds, in any tenant.""" + tokens = [t for t in await self.list_for_owner(owner_user_id) if t.revoked_at is None] + now = _utcnow() + for token in tokens: + token.revoked_at = now + if tokens: + with all_tenants(): + await self.db.flush() + return len(tokens) + async def list_for_owner(self, owner_user_id: uuid.UUID) -> list[ApiToken]: """Every personal token *owner_user_id* holds, in any tenant. diff --git a/modules/api_tokens/tests/test_api_tokens_hardening.py b/modules/api_tokens/tests/test_api_tokens_hardening.py new file mode 100644 index 000000000..b31654ec8 --- /dev/null +++ b/modules/api_tokens/tests/test_api_tokens_hardening.py @@ -0,0 +1,207 @@ +"""Security review round 1, pass 2: tenant pinning, bearer laundering, stored roles. + +* M2 — a PAT acts in the tenant it was created in, never one named by a + header, and only while its owner is still an active member there. +* M3 — a provider's bearer access token keeps its full rights but cannot + manage API tokens, so it cannot be laundered into a year-long PAT. +* L5 — a service key's stored ``tenant:*`` / ``admin`` roles are dropped. +* Keycloak — no PAT for an account the resolver could never load. +* L1 — "revoke all my tokens". +""" + +from __future__ import annotations + +import uuid +from typing import Any + +import httpx +from _api_tokens_support import ( + CSRF, + CSRF_HEADERS, + FLAGS_URL, + INERTIA_HEADERS, + PAT_URL, + create_pat, + join_tenant, + make_admin, +) +from simple_module_test.tenant_client import create_user + +MEMBERS_URL = "/api/tenants/current/members" + + +async def _member_ids(resp: httpx.Response) -> set[str]: + assert resp.status_code == 200, resp.text + return {str(m["user_id"]) for m in resp.json()} + + +async def test_pat_is_pinned_to_its_tenant_whatever_the_header_says(app, actor, bearer): + app.state.sm.settings.tenant_header = "X-Tenant-Id" + async with actor() as u: + token = (await create_pat(u.client, ["tenants.members.view"]))["token"] + tenant_a = u.tenant_id + # The same person also belongs to tenant B, alongside a stranger. + tenant_b = await join_tenant(app, u.user_id, f"b-{uuid.uuid4().hex[:6]}@example.com") + stranger = f"s-{uuid.uuid4().hex[:6]}@example.com" + stranger_id = await create_user(app, stranger) + await join_tenant(app, stranger_id, stranger, tenant_b) + + resp = await bearer("GET", MEMBERS_URL, token, headers={"X-Tenant-Id": tenant_b}) + members = await _member_ids(resp) + assert stranger_id not in members # tenant B's member list stays out of reach + assert u.user_id in members # it answered for tenant A + assert tenant_a != tenant_b + vary = resp.headers.get("vary", "") + assert "Authorization" in vary + assert "X-Tenant-Id" not in vary # the header no longer decides + + +async def _remove_membership(app: Any, user_id: str, tenant_id: str) -> None: + from sqlalchemy import delete + from tenants.models import Membership + from tenants.resolver import forget + + async with app.state.sm.db.session_factory() as session: + await session.execute( + delete(Membership).where( + Membership.user_id == user_id, Membership.tenant_id == tenant_id + ) + ) + await session.commit() + forget(user_id) + + +async def test_pat_stops_working_once_the_owner_leaves_its_tenant(app, actor, bearer): + async with actor(admin=True) as a: + token = (await create_pat(a.client, ["feature_flags.view"]))["token"] + assert (await bearer("GET", FLAGS_URL, token)).status_code == 200 + # Still a member elsewhere — but not of the tenant the token belongs to. + await join_tenant(app, a.user_id, f"x-{uuid.uuid4().hex[:6]}@example.com") + await _remove_membership(app, a.user_id, a.tenant_id) + assert (await bearer("GET", FLAGS_URL, token)).status_code == 401 + + +async def test_pat_stops_working_while_its_tenant_is_suspended(app, actor, bearer): + from sqlalchemy import update + from tenants.constants import TenantStatus + from tenants.models import Tenant + from tenants.resolver import forget + + async with actor(admin=True) as a: + token = (await create_pat(a.client, ["feature_flags.view"]))["token"] + async with app.state.sm.db.session_factory() as session: + await session.execute( + update(Tenant).where(Tenant.id == a.tenant_id).values(status=TenantStatus.SUSPENDED) + ) + await session.commit() + forget(None) + assert (await bearer("GET", FLAGS_URL, token)).status_code == 401 + + +async def _access_token(app: Any) -> tuple[str, str]: + email = f"a-{uuid.uuid4().hex[:6]}@example.com" + user_id = await make_admin(app, email) + await join_tenant(app, user_id, email) + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=app), base_url="http://testserver" + ) as client: + resp = await client.post( + "/api/users/auth/token", json={"email": email, "password": "pw-123456"} + ) + assert resp.status_code == 200, resp.text + return resp.json()["access_token"], user_id + + +async def test_bearer_access_token_cannot_reach_token_management(app, bearer): + access, _ = await _access_token(app) + page = await bearer("GET", "/api-tokens/", access, headers=INERTIA_HEADERS) + assert page.status_code == 403 + for method, url in (("GET", PAT_URL), ("POST", PAT_URL), ("POST", f"{PAT_URL}revoke-all")): + resp = await bearer( + method, + url, + access, + headers=CSRF_HEADERS, + json={"name": "x", "scopes": ["api_tokens.use"], "expires_in_days": 365}, + ) + assert resp.status_code == 403, (method, url, resp.text) + admin = await bearer("GET", "/api/api_tokens/admin/tokens", access, headers=CSRF_HEADERS) + assert admin.status_code == 403 + + +async def test_bearer_access_token_keeps_its_full_rights(app, bearer): + """No scope cap and no token guard for a provider's bearer: unchanged behaviour.""" + access, _ = await _access_token(app) + assert (await bearer("GET", FLAGS_URL, access)).status_code == 200 + assert (await bearer("GET", "/api/tenants/", access)).status_code == 200 + + +async def test_session_with_an_authorization_header_cannot_mint(actor): + async with actor() as u: + resp = await u.client.post( + PAT_URL, + json={"name": "x", "scopes": ["api_tokens.use"], "expires_in_days": 30}, + headers={**CSRF_HEADERS, "Authorization": "Basic Zm9vOmJhcg=="}, + ) + assert resp.status_code == 403 + + +def test_service_key_drops_stored_tenant_and_admin_roles(): + from datetime import UTC, datetime, timedelta + + from api_tokens.constants import TokenKind + from api_tokens.models import ApiToken + from api_tokens.resolver import _service + + row = ApiToken( + id=uuid.uuid4(), + kind=TokenKind.SERVICE, + name="k", + prefix="smsk_abc", + token_hash="h", + scopes=["x.view"], + roles=["admin", "tenant:owner", "tenant:admin", "user"], + expires_at=datetime.now(UTC) + timedelta(days=1), + tenant_id="t1", + ) + assert _service(row).roles == ["user"] + + +async def test_no_pat_for_an_account_the_resolver_cannot_load(app, actor, monkeypatch): + from api_tokens.service import ApiTokenService + + async def missing(self, user_id) -> bool: + return False + + async with actor() as u: + monkeypatch.setattr(ApiTokenService, "has_usable_account", missing) + resp = await u.client.post( + PAT_URL, + json={"name": "x", "scopes": ["api_tokens.use"], "expires_in_days": 30}, + headers=CSRF_HEADERS, + ) + assert resp.status_code == 400 + assert "local user record" in resp.json()["detail"] + + +async def test_has_usable_account(app): + from api_tokens.service import ApiTokenService + + user_id = await create_user(app, f"u-{uuid.uuid4().hex[:6]}@example.com") + async with app.state.sm.db.session_factory() as session: + service = ApiTokenService(session) + assert await service.has_usable_account(uuid.UUID(user_id)) is True + assert await service.has_usable_account(uuid.uuid4()) is False + + +async def test_revoke_all_ends_every_token(actor, bearer): + async with actor(admin=True) as a: + first = (await create_pat(a.client, ["feature_flags.view"]))["token"] + second = (await create_pat(a.client, ["feature_flags.view"]))["token"] + # A token cannot pull the panic button for its owner. + assert (await bearer("POST", f"{PAT_URL}revoke-all", first)).status_code == 403 + resp = await a.client.post(f"{PAT_URL}revoke-all", headers={"X-CSRF-Token": CSRF}) + assert resp.status_code == 200 + assert resp.json() == {"revoked": 2} + assert (await bearer("GET", FLAGS_URL, first)).status_code == 401 + assert (await bearer("GET", FLAGS_URL, second)).status_code == 401 diff --git a/modules/api_tokens/tests/test_api_tokens_route_guard.py b/modules/api_tokens/tests/test_api_tokens_route_guard.py new file mode 100644 index 000000000..252aa0380 --- /dev/null +++ b/modules/api_tokens/tests/test_api_tokens_route_guard.py @@ -0,0 +1,122 @@ +"""Tokens reach only permission-gated API routes (security review M1, L2). + +Routes that need nothing but a signed-in principal — create or leave an +organisation, the profile page, the dashboard — used to accept any token, +whatever it was scoped to. ``simple_module_hosting.token_guard`` now refuses a +token on every route without a permission gate, and on every page. +""" + +from __future__ import annotations + +import uuid +from typing import Any + +from _api_tokens_support import ( + CSRF_HEADERS, + FLAGS_URL, + INERTIA_HEADERS, + SERVICE_URL, + create_pat, + join_tenant, +) +from simple_module_test.tenant_client import create_user + + +async def _memberships(app: Any, user_id: str) -> list[str]: + from sqlalchemy import select + from tenants.models import Membership + + async with app.state.sm.db.session_factory() as session: + rows = await session.execute( + select(Membership.tenant_id).where(Membership.user_id == user_id) + ) + return [r[0] for r in rows] + + +async def test_pat_cannot_use_tenant_self_service_routes(app, actor, bearer): + async with actor() as u: + token = (await create_pat(u.client, ["api_tokens.use"]))["token"] + other = f"o-{uuid.uuid4().hex[:6]}@example.com" + await join_tenant(app, await create_user(app, other), other, u.tenant_id) + before = await _memberships(app, u.user_id) + + assert (await bearer("GET", "/api/tenants/", token)).status_code == 403 + created = await bearer("POST", "/api/tenants/", token, json={"name": "Evil Org"}) + assert created.status_code == 403 + assert created.headers["content-type"].startswith("application/json") + left = await bearer("DELETE", "/api/tenants/current/membership", token) + assert left.status_code == 403 + switch = await bearer("POST", f"/api/tenants/{u.tenant_id}/switch", token) + assert switch.status_code == 403 + accept = await bearer("POST", "/api/tenants/invitations/accept", token, json={"token": "x"}) + assert accept.status_code == 403 + # Nothing happened: no new organisation, still a member. + assert await _memberships(app, u.user_id) == before + + +async def test_pat_cannot_open_pages(actor, bearer): + async with actor(admin=True) as a: + token = (await create_pat(a.client, ["api_tokens.use", "feature_flags.view"]))["token"] + + for path in ("/users/me", "/dashboard/", "/api-tokens/", "/tenants/"): + resp = await bearer("GET", path, token, headers=INERTIA_HEADERS) + assert resp.status_code == 403, path + + +async def test_pat_still_reads_a_permission_gated_api_route(actor, bearer): + async with actor(admin=True) as a: + token = (await create_pat(a.client, ["feature_flags.view"]))["token"] + assert (await bearer("GET", FLAGS_URL, token)).status_code == 200 + + +async def test_the_browser_session_is_unaffected(actor): + async with actor() as u: + assert (await u.client.get("/api/tenants/")).status_code == 200 + assert (await u.client.get("/users/me", headers=INERTIA_HEADERS)).status_code == 200 + + +async def test_token_on_a_public_route_is_served_as_anonymous(actor, bearer): + async with actor() as u: + token = (await create_pat(u.client, ["api_tokens.use"]))["token"] + assert (await bearer("GET", "/health/live", token)).status_code == 200 + + +async def _service_key(app, actor, scopes: list[str], roles: list[str]) -> str: + async with actor(admin=True) as a: + resp = await a.client.post( + SERVICE_URL, + json={"name": "k", "roles": roles, "scopes": scopes, "expires_in_days": 30}, + headers=CSRF_HEADERS, + ) + assert resp.status_code == 201, resp.text + return resp.json()["token"] + + +async def test_service_key_cannot_create_an_organisation(app, actor, bearer): + app.state.sm.permissions.map_role("user", ["feature_flags.view"]) + token = await _service_key(app, actor, ["feature_flags.view"], ["user"]) + assert (await bearer("POST", "/api/tenants/", token, json={"name": "Svc"})).status_code == 403 + me = await bearer("GET", "/users/me", token, headers=INERTIA_HEADERS) + assert me.status_code == 403 # was a 500: uuid.UUID("svc:…") + + +def _parameterless_get_paths(app) -> list[str]: + """Every GET under ``/api/`` with no path parameter, from the OpenAPI schema.""" + paths = app.openapi()["paths"] + return sorted( + p for p, ops in paths.items() if "get" in ops and "{" not in p and p.startswith("/api/") + ) + + +async def test_a_broad_service_key_never_causes_a_500(app, actor, bearer): + """L2: a ``svc:`` principal on any token-reachable route fails closed, never 500s.""" + registry = app.state.sm.permissions + every = sorted(p for p in registry.all_permissions if p != "*") + registry.map_role("user", every) + token = await _service_key(app, actor, every, ["user"]) + failures = [] + for path in _parameterless_get_paths(app): + resp = await bearer("GET", path, token) + if resp.status_code >= 500: + failures.append((path, resp.status_code)) + assert failures == [] diff --git a/modules/auth/auth/contracts/schemas.py b/modules/auth/auth/contracts/schemas.py index af5769fbc..3c705127c 100644 --- a/modules/auth/auth/contracts/schemas.py +++ b/modules/auth/auth/contracts/schemas.py @@ -27,7 +27,8 @@ class UserContext: # revocable belongs in a grant source. The resolver drops ``*`` from here. permissions: list[str] = field(default_factory=list) # Which credential authenticated the request: ``"session"`` for a browser - # sign-in or an IdP bearer token, ``"pat"`` / ``"service"`` for an + # sign-in, ``"bearer"`` for a provider's own bearer token (a ``users`` + # access token, an IdP JWT), ``"pat"`` / ``"service"`` for an # ``api_tokens`` credential (see ``auth.contracts.tokens``). Code that must # only ever act for a person at a keyboard — minting more credentials — # refuses anything but ``"session"``. diff --git a/modules/auth/auth/contracts/tokens.py b/modules/auth/auth/contracts/tokens.py index 97d7a0edf..446ae3ce8 100644 --- a/modules/auth/auth/contracts/tokens.py +++ b/modules/auth/auth/contracts/tokens.py @@ -12,7 +12,13 @@ from __future__ import annotations AUTH_KIND_SESSION = "session" -"""A browser sign-in or an identity provider's own bearer token.""" +"""A browser sign-in: the principal came from the session cookie.""" + +AUTH_KIND_BEARER = "bearer" +"""An auth provider's own ``Authorization: Bearer`` credential — a ``users`` +access token or an identity provider's JWT. Unscoped (it carries its user's +full rights, as before), but not a person at a keyboard: it may not manage +API tokens, so a short-lived access token cannot mint a year-long PAT.""" AUTH_KIND_PAT = "pat" """A personal access token acting for its owner, capped by its scopes.""" @@ -32,6 +38,7 @@ def is_api_token(credential: str) -> bool: __all__ = [ "API_TOKEN_PREFIXES", + "AUTH_KIND_BEARER", "AUTH_KIND_PAT", "AUTH_KIND_SERVICE", "AUTH_KIND_SESSION", diff --git a/modules/auth/auth/deps.py b/modules/auth/auth/deps.py index 89f212251..b330c79b6 100644 --- a/modules/auth/auth/deps.py +++ b/modules/auth/auth/deps.py @@ -8,6 +8,7 @@ from simple_module_core.permissions import grants from simple_module_hosting.i18n_deps import TranslatorDep from simple_module_hosting.permissions import ensure_resolved_permissions +from simple_module_hosting.token_guard import GATE_PERMISSION, mark_token_gate from auth.contracts.schemas import UserContext @@ -61,4 +62,6 @@ async def check( ), ) - return Depends(check) + # A permission gate: API tokens may reach the route, capped at their + # scopes (``simple_module_hosting.token_guard``). + return Depends(mark_token_gate(check, GATE_PERMISSION)) diff --git a/modules/auth/tests/test_deps.py b/modules/auth/tests/test_deps.py index 07bbd3ca5..3d5ac9a0e 100644 --- a/modules/auth/tests/test_deps.py +++ b/modules/auth/tests/test_deps.py @@ -109,3 +109,18 @@ async def test_non_admin_role_mapped_in_registry_passes(self, app): dep = require_permission("products.delete") user = UserContext(id="u1", email="u@t.com", name="User", roles=["clerk"]) await dep.dependency(_request(app, user), _translator(), user) + + +def test_require_permission_is_a_token_gate(): + """``token_guard`` admits API tokens on a route gated by ``require_permission``.""" + from fastapi import FastAPI + from simple_module_hosting.token_guard import route_admits_tokens + + app = FastAPI() + + @app.get("/g", dependencies=[require_permission("x.view")]) + async def gated() -> None: + return None + + route = next(r for r in app.router.routes if getattr(r, "path", "") == "/g") + assert route_admits_tokens(route) is True diff --git a/modules/keycloak/keycloak/provider.py b/modules/keycloak/keycloak/provider.py index 905c2f1f5..8e526d0a2 100644 --- a/modules/keycloak/keycloak/provider.py +++ b/modules/keycloak/keycloak/provider.py @@ -7,7 +7,7 @@ from typing import TYPE_CHECKING, Any from auth.contracts.schemas import UserContext -from auth.contracts.tokens import is_api_token +from auth.contracts.tokens import AUTH_KIND_BEARER, is_api_token from simple_module_core.tenancy import is_tenant_role from simple_module_db import is_valid_tenant_id from starlette.requests import Request @@ -98,7 +98,12 @@ async def _resolve_bearer(self, request: Request, token: str) -> UserContext | N return None cache_id = await self._upsert_user_cache(request, claims) - return self._claims_to_user_context(claims, cache_id=cache_id) + ctx = self._claims_to_user_context(claims, cache_id=cache_id) + # Full rights, but a bearer credential: it may not manage API tokens + # (``auth.contracts.tokens.AUTH_KIND_BEARER``). The browser sign-in + # builds the same context from the callback and keeps ``"session"``. + ctx.auth_kind = AUTH_KIND_BEARER + return ctx def _claims_to_user_context( self, diff --git a/modules/keycloak/tests/test_keycloak_bearer_kind.py b/modules/keycloak/tests/test_keycloak_bearer_kind.py new file mode 100644 index 000000000..3e43abb87 --- /dev/null +++ b/modules/keycloak/tests/test_keycloak_bearer_kind.py @@ -0,0 +1,42 @@ +"""A Keycloak JWT on the Authorization header is a ``bearer`` principal (security review M3). + +It keeps its user's full rights — no scope cap — but is not a cookie session, +so ``api_tokens`` refuses to let it mint or manage tokens. +""" + +from __future__ import annotations + +from auth.contracts.tokens import AUTH_KIND_BEARER +from keycloak.provider import KeycloakAuthProvider +from keycloak.settings import KeycloakSettings +from keycloak.state import KeycloakState +from starlette.requests import Request + + +class _Jwks: + async def validate_jwt(self, token: str) -> dict: + return {"sub": "abc", "email": "kc@example.com", "preferred_username": "kc"} + + +async def test_jwt_bearer_principal_is_bearer_kind_and_unscoped(monkeypatch): + state = KeycloakState(settings=KeycloakSettings()) + provider = KeycloakAuthProvider(state.settings, state=state) + provider.jwks_cache = _Jwks() # type: ignore[assignment] + + async def upsert(request, claims) -> str: + return "00000000-0000-0000-0000-0000000000aa" + + monkeypatch.setattr(provider, "_upsert_user_cache", upsert) + request = Request( + { + "type": "http", + "method": "GET", + "path": "/", + "headers": [(b"authorization", b"Bearer some.jwt.value")], + "session": {}, + } + ) + ctx = await provider.resolve_user(request) + assert ctx is not None + assert ctx.auth_kind == AUTH_KIND_BEARER + assert ctx.scopes is None diff --git a/modules/tenants/tenants/resolver.py b/modules/tenants/tenants/resolver.py index 9b15ce4a1..b0bf4a052 100644 --- a/modules/tenants/tenants/resolver.py +++ b/modules/tenants/tenants/resolver.py @@ -42,6 +42,7 @@ subdomains_enabled, ) from tenants.service import TenantService +from tenants.token_tenant import pinned_token_tenant logger = logging.getLogger(__name__) @@ -147,15 +148,14 @@ async def _resolve(request: Request) -> tuple[str | None, str | None, tuple[str, # every branch), so a shared cache must key on it too; an anonymous one did # not, and anonymous public pages stay cacheable (#418). credential: tuple[str, ...] = ("Cookie",) if user is not None else () - if getattr(user, "auth_kind", AUTH_KIND_SESSION) == AUTH_KIND_PAT: - # A personal token is carried by the Authorization header, not a cookie. + kind = getattr(user, "auth_kind", AUTH_KIND_SESSION) + if kind != AUTH_KIND_SESSION: + # A bearer credential is carried by the Authorization header, not a cookie. credential = ("Authorization",) - if getattr(user, "auth_kind", None) == AUTH_KIND_SERVICE: - # A service key is pinned to the tenant it was created in. It has no - # memberships to validate against and no session preference; neither a - # header nor a subdomain may move it to another tenant. - tenant_id = await _service_tenant(request, getattr(user, "tenant_id", None)) - return tenant_id, "token", ("Authorization",) + if kind in (AUTH_KIND_PAT, AUTH_KIND_SERVICE): + # An API token is pinned to the tenant it was created in: neither a + # header, a subdomain nor a session preference may move it elsewhere. + return await pinned_token_tenant(request, user), "token", credential slug = subdomain_slug(request) if slug is not None: tenant_id = await _resolve_subdomain(request, user, slug) @@ -224,20 +224,6 @@ async def _resolve_subdomain(request: Request, user: Any, slug: str) -> str | No return tenant_id -async def _service_tenant(request: Request, tenant_id: Any) -> str | None: - """The service key's pinned tenant, while that tenant exists and is active.""" - if not is_valid_tenant_id(tenant_id): - return None - from tenants.models import Tenant - - async with request.app.state.sm.db.session_factory() as db: - tenant = await db.get(Tenant, tenant_id) - if tenant is None or tenant.status != TenantStatus.ACTIVE: - request.state.tenant_suspended = tenant is not None - return None - return tenant_id - - def _header_name(request: Request) -> str: settings = getattr(getattr(request.app.state, "sm", None), "settings", None) return getattr(settings, "tenant_header", "") or "" diff --git a/modules/tenants/tenants/token_tenant.py b/modules/tenants/tenants/token_tenant.py new file mode 100644 index 000000000..f51c7918c --- /dev/null +++ b/modules/tenants/tenants/token_tenant.py @@ -0,0 +1,70 @@ +"""The tenant an API-token request acts for: the one the token was created in. + +``api_tokens`` stamps each token with the tenant that was active when it was +created and copies it onto the principal's ``tenant_id``. Both kinds are +pinned there — a header, a subdomain or a session preference never moves them: + +* A **service key** has no person and no memberships; its tenant only has to + still exist and be active. +* A **personal access token** acts for its owner, so the owner must also still + be an active member of that tenant. When they are not (removed, or the + tenant suspended), the request is **de-authenticated** rather than left + signed in without a tenant: every route a token may reach is permission + gated, so it answers 401, the same as for a revoked token. +""" + +from __future__ import annotations + +from typing import Any + +from auth.contracts.tokens import AUTH_KIND_SERVICE +from simple_module_db import is_valid_tenant_id +from simple_module_db.listeners import current_user_id +from starlette.requests import Request + +from tenants.constants import TenantStatus + + +async def pinned_token_tenant(request: Request, user: Any) -> str | None: + """The token's own tenant, or ``None`` (and, for a PAT, no principal).""" + tenant_id = getattr(user, "tenant_id", None) + if getattr(user, "auth_kind", None) == AUTH_KIND_SERVICE: + return await _service_tenant(request, tenant_id) + return await _personal_tenant(request, user, tenant_id) + + +async def _service_tenant(request: Request, tenant_id: Any) -> str | None: + """The service key's pinned tenant, while that tenant exists and is active.""" + if not is_valid_tenant_id(tenant_id): + return None + from tenants.models import Tenant + + async with request.app.state.sm.db.session_factory() as db: + tenant = await db.get(Tenant, tenant_id) + if tenant is None or tenant.status != TenantStatus.ACTIVE: + request.state.tenant_suspended = tenant is not None + return None + return tenant_id + + +async def _personal_tenant(request: Request, user: Any, tenant_id: Any) -> str | None: + from tenants.resolver import _enter, memberships_for + + own = None + if is_valid_tenant_id(tenant_id): + memberships = await memberships_for(request.app, str(user.id)) + own = next((m for m in memberships if m.id == tenant_id), None) + if own is None or own.status != TenantStatus.ACTIVE: + request.state.tenant_suspended = own is not None + _forget_principal(request) + return None + return _enter(request, user, own) + + +def _forget_principal(request: Request) -> None: + request.state.user = None + request.state.resolved_permissions = set() + current_user_id.set(None) + + +__all__ = ["pinned_token_tenant"] diff --git a/modules/users/users/auth_local/views.py b/modules/users/users/auth_local/views.py index 4581298a3..4be0dbda6 100644 --- a/modules/users/users/auth_local/views.py +++ b/modules/users/users/auth_local/views.py @@ -236,7 +236,13 @@ async def profile_page( ctx = getattr(request.state, "user", None) if ctx is None: raise HTTPException(status_code=401) - user = await db.get(User, uuid.UUID(ctx.id)) + try: + user_id = uuid.UUID(str(ctx.id)) + except ValueError: + # Not an account (a service key's ``svc:`` principal): refuse rather + # than 500. The token guard already turns tokens away from pages. + raise HTTPException(status_code=403) from None + user = await db.get(User, user_id) if user is None: raise HTTPException(status_code=401) return await inertia.render( diff --git a/modules/users/users/token_strategy.py b/modules/users/users/token_strategy.py index e8420478c..697debde7 100644 --- a/modules/users/users/token_strategy.py +++ b/modules/users/users/token_strategy.py @@ -23,6 +23,7 @@ from typing import Any from auth.contracts.schemas import UserContext +from auth.contracts.tokens import AUTH_KIND_BEARER from fastapi_users import exceptions, models from fastapi_users.authentication.strategy.db import AccessTokenDatabase, DatabaseStrategy from fastapi_users.manager import BaseUserManager @@ -158,7 +159,9 @@ async def resolve_bearer(scope, token: str) -> UserContext | None: # already collected. Free here — the row is already loaded. if not token_is_live(access, user, now): return None - return UserContext.from_user(user) + # Full rights, but a bearer credential: it may not manage API + # tokens (``auth.contracts.tokens.AUTH_KIND_BEARER``). + return UserContext.from_user(user, auth_kind=AUTH_KIND_BEARER) except Exception: logger.exception("Bearer token resolution failed") return None diff --git a/packages/i18n/src/generated-resources.ts b/packages/i18n/src/generated-resources.ts index e09b4e247..3c5aa719d 100644 --- a/packages/i18n/src/generated-resources.ts +++ b/packages/i18n/src/generated-resources.ts @@ -12,6 +12,7 @@ export default { 'api_tokens.created.done': '', 'api_tokens.created.title': '', 'api_tokens.created.warning': '', + 'api_tokens.errors.account_required': '', 'api_tokens.errors.invalid_kind': '', 'api_tokens.errors.lifetime_too_long': '', 'api_tokens.errors.not_found': '', @@ -64,12 +65,19 @@ export default { 'api_tokens.revoke.confirm': '', 'api_tokens.revoke.description': '', 'api_tokens.revoke.title': '', + 'api_tokens.revoke_all.button': '', + 'api_tokens.revoke_all.cancel': '', + 'api_tokens.revoke_all.confirm': '', + 'api_tokens.revoke_all.description': '', + 'api_tokens.revoke_all.title': '', 'api_tokens.status.active': '', 'api_tokens.status.expired': '', 'api_tokens.status.revoked': '', 'api_tokens.toasts.created': '', 'api_tokens.toasts.failed': '', 'api_tokens.toasts.revoked': '', + 'api_tokens.toasts.revoked_all_one': '', + 'api_tokens.toasts.revoked_all_other': '', 'audit_log.actions.created': '', 'audit_log.actions.deleted': '', 'audit_log.actions.soft_deleted': '', @@ -542,6 +550,7 @@ export default { 'host.offline.description': '', 'host.offline.restored': '', 'host.offline.title': '', + 'hosting.errors.token_not_allowed': '', 'hosting.setup.connections.description': '', 'hosting.setup.connections.heading': '', 'hosting.setup.connections.retest': '', diff --git a/packages/i18n/src/keys.generated.ts b/packages/i18n/src/keys.generated.ts index 2b2e80cb2..a9e012b75 100644 --- a/packages/i18n/src/keys.generated.ts +++ b/packages/i18n/src/keys.generated.ts @@ -20,6 +20,7 @@ export const keys = { warning: 'api_tokens.created.warning', }, errors: { + account_required: 'api_tokens.errors.account_required', invalid_kind: 'api_tokens.errors.invalid_kind', lifetime_too_long: 'api_tokens.errors.lifetime_too_long', not_found: 'api_tokens.errors.not_found', @@ -87,6 +88,13 @@ export const keys = { description: 'api_tokens.revoke.description', title: 'api_tokens.revoke.title', }, + revoke_all: { + button: 'api_tokens.revoke_all.button', + cancel: 'api_tokens.revoke_all.cancel', + confirm: 'api_tokens.revoke_all.confirm', + description: 'api_tokens.revoke_all.description', + title: 'api_tokens.revoke_all.title', + }, status: { active: 'api_tokens.status.active', expired: 'api_tokens.status.expired', @@ -96,6 +104,9 @@ export const keys = { created: 'api_tokens.toasts.created', failed: 'api_tokens.toasts.failed', revoked: 'api_tokens.toasts.revoked', + revoked_all: 'api_tokens.toasts.revoked_all', + revoked_all_one: 'api_tokens.toasts.revoked_all_one', + revoked_all_other: 'api_tokens.toasts.revoked_all_other', }, }, audit_log: { @@ -704,6 +715,9 @@ export const keys = { }, }, hosting: { + errors: { + token_not_allowed: 'hosting.errors.token_not_allowed', + }, setup: { connections: { description: 'hosting.setup.connections.description', From ab944fb1b3d04dde622aa625a6db6efa0d9af99d Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 15 Oct 2026 12:44:22 +0200 Subject: [PATCH 7/9] fix: address code review findings (round 1, pass 3) Guard accepted websockets (HTTPConnection), page marker now enforced at request time so nested include_router cannot hide it, page beats TokenAllowed. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- docs/framework/permissions.md | 5 +- .../simple_module_hosting/token_guard.py | 37 ++++++--- .../hosting/tests/test_token_route_guard.py | 78 ++++++++++++++++++- 3 files changed, 107 insertions(+), 13 deletions(-) diff --git a/docs/framework/permissions.md b/docs/framework/permissions.md index 736f62606..42f9ab9f8 100644 --- a/docs/framework/permissions.md +++ b/docs/framework/permissions.md @@ -119,7 +119,10 @@ dependency) applies a fail-closed rule to every route: token's principal is dropped and the request runs anonymously. So **gate a route with a permission dependency if API tokens should reach it**; -a permission checked inline in the handler does not count. Sessions and a +a permission checked inline in the handler does not count, and neither does +one passed as `include_router(..., dependencies=[...])` (FastAPI keeps those out of +the route's own dependency tree, so the route refuses tokens — declare the gate +on the route or on the router it is declared on). Sessions and a provider's own bearer tokens (`auth_kind == "bearer"`) are unscoped and unaffected. A custom gate dependency can mark itself with `mark_token_gate(fn, GATE_PERMISSION)`. diff --git a/framework/hosting/simple_module_hosting/token_guard.py b/framework/hosting/simple_module_hosting/token_guard.py index ee9654196..70114dd8d 100644 --- a/framework/hosting/simple_module_hosting/token_guard.py +++ b/framework/hosting/simple_module_hosting/token_guard.py @@ -15,7 +15,7 @@ * A **page** route (anything rendering through Inertia, and every route on a module's view router) refuses tokens even when gated: tokens are API credentials, and a page's props are a second, unscoped read API. -* :class:`TokenAllowed` on a route admits tokens explicitly, for a route that +* :class:`TokenAllowed` on a (non-page) route admits tokens explicitly, for a route that is safe for any token without naming a permission. * Everything else refuses a token with ``403``. On a route the public-route registry exempts, the token is not refused but **ignored**: the request runs @@ -35,8 +35,9 @@ from collections.abc import Callable from typing import Any -from fastapi import HTTPException, Request +from fastapi import HTTPException from simple_module_db.listeners import current_user_id +from starlette.requests import HTTPConnection TOKEN_GATE_ATTR = "__sm_token_gate__" """Attribute a dependency callable carries to tell the guard what it is.""" @@ -77,8 +78,15 @@ async def __call__(self) -> None: return None -async def page_route() -> None: - """Dependency marker for a page route (added to every module view router).""" +async def page_route(request: HTTPConnection) -> None: + """Dependency for a page route (added to every module view router). + + It carries the page marker for the tree inspection *and* refuses a token + itself: a router's own ``dependencies`` reach a route inside a nested + ``include_router`` only as an include-level dependency, which FastAPI + keeps out of the route's ``dependant`` — the inspection cannot see it. + """ + _refuse_token(request) mark_token_gate(page_route, GATE_PAGE) @@ -109,10 +117,10 @@ def route_admits_tokens(route: Any) -> bool: return False kinds: set[str] = set() _gate_kinds(dependant, kinds, set()) - if GATE_ALLOW in kinds: + if GATE_PAGE in kinds: + admits = False # a page wins over an explicit opt-in + elif GATE_ALLOW in kinds: admits = True - elif GATE_PAGE in kinds: - admits = False else: admits = GATE_PERMISSION in kinds with contextlib.suppress(AttributeError, TypeError): # a slotted/frozen route @@ -125,7 +133,7 @@ def is_token_principal(user: Any) -> bool: return user is not None and getattr(user, "scopes", None) is not None -def _translate(request: Request, key: str, fallback: str) -> str: +def _translate(request: HTTPConnection, key: str, fallback: str) -> str: sm = getattr(request.app.state, "sm", None) registry = getattr(sm, "i18n_registry", None) if registry is None: @@ -137,13 +145,20 @@ def _translate(request: Request, key: str, fallback: str) -> str: return Translator(registry, locale=locale, default_locale=default).t(key) -async def token_route_guard(request: Request) -> None: +async def token_route_guard(request: HTTPConnection) -> None: """App-wide dependency enforcing the rule in this module's docstring.""" + if not is_token_principal(getattr(request.state, "user", None)): + return # the common case: one attribute read, no tree inspection + if route_admits_tokens(request.scope.get("route")): + return + _refuse_token(request) + + +def _refuse_token(request: HTTPConnection) -> None: + """Turn a token principal away (403), or demote it to anonymous if public.""" user = getattr(request.state, "user", None) if not is_token_principal(user): return - if route_admits_tokens(request.scope.get("route")): - return if (request.scope.get("state") or {}).get("auth_public"): # A public route: serve the token holder as the anonymous visitor # anyone may be, never as the account behind the token. diff --git a/framework/hosting/tests/test_token_route_guard.py b/framework/hosting/tests/test_token_route_guard.py index 984140a22..256c321af 100644 --- a/framework/hosting/tests/test_token_route_guard.py +++ b/framework/hosting/tests/test_token_route_guard.py @@ -10,7 +10,7 @@ from dataclasses import dataclass, field import httpx -from fastapi import APIRouter, Depends, FastAPI, Request +from fastapi import APIRouter, Depends, FastAPI, Request, WebSocket from simple_module_hosting.inertia_deps import InertiaDep from simple_module_hosting.permissions import RequiresPermission from simple_module_hosting.token_guard import ( @@ -127,3 +127,79 @@ async def nothing() -> None: assert route_admits_tokens(_route(app, "/n")) is False assert route_admits_tokens(None) is False + + +async def test_token_admitted_through_a_nested_sub_dependency_gate(): + app = FastAPI(dependencies=[Depends(token_route_guard)]) + app.state.sm = None + + async def inner(_: None = Depends(RequiresPermission("x.view"))) -> None: + return None + + async def outer(_: None = Depends(inner)) -> None: + return None + + router = APIRouter() + + @router.get("/nested") + async def nested(_: None = Depends(outer)) -> dict: + return {"ok": True} + + app.include_router(router) + + @app.middleware("http") + async def fake_auth(request: Request, call_next): + request.state.user = _Principal(scopes=["x.view"], permissions=["x.view"]) + return await call_next(request) + + r = await _get(app, "/nested") + assert r.status_code == 200, r.text + assert (await _get(app, "/missing")).status_code == 404 + + +def test_websocket_route_with_the_guard_does_not_break(): + from starlette.testclient import TestClient + + app = FastAPI(dependencies=[Depends(token_route_guard)]) + app.state.sm = None + + @app.websocket("/ws") + async def ws(websocket: WebSocket) -> None: + await websocket.accept() + await websocket.send_text("hi") + await websocket.close() + + with TestClient(app).websocket_connect("/ws") as sock: + assert sock.receive_text() == "hi" + + +async def test_page_marker_on_a_router_survives_nested_include_router(): + """FastAPI keeps a nested include's router dependencies out of the route's tree.""" + app = FastAPI(dependencies=[Depends(token_route_guard)]) + app.state.sm = None + sub = APIRouter() + + @sub.get("/x", dependencies=[Depends(RequiresPermission("x.view"))]) + async def x() -> dict: + return {"ok": True} + + pages = APIRouter(prefix="/pages", dependencies=[Depends(page_route)]) + pages.include_router(sub) + app.include_router(pages) + + @app.middleware("http") + async def fake_auth(request: Request, call_next): + request.state.user = _Principal(scopes=["x.view"], permissions=["x.view"]) + return await call_next(request) + + assert (await _get(app, "/pages/x")).status_code == 403 + + +def test_page_marker_beats_an_explicit_opt_in(): + app = FastAPI() + + @app.get("/p", dependencies=[Depends(page_route), Depends(TokenAllowed())]) + async def p() -> None: + return None + + assert route_admits_tokens(_route(app, "/p")) is False From d48a52a4981ccab731192c71508014fdea8559c8 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 15 Oct 2026 13:58:29 +0200 Subject: [PATCH 8/9] fix(api_tokens): address security verification findings (round 1, pass 4) - N2: layout shared props render a token principal as anonymous (no auth user, permissions, menus or tenant memberships on 403/public pages). - N3: a PAT requires its owner to currently hold api_tokens.use (UserContext.required_permission, checked tenant-aware after tenant resolution); otherwise the request is demoted to anonymous -> 401. - N4: demoting a token to anonymous also re-resolves the tenant as anonymous (TenantMiddleware re-resolution, Vary kept). - N1: token_guard reads FastAPI's effective route context so include_router(dependencies=[...]) gates and page markers are seen; falls back to the original route (fail closed). Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- docs/framework/permissions.md | 13 +- docs/modules/api_tokens.md | 18 ++- .../simple_module_hosting/_anonymous.py | 27 ++++ .../hosting/simple_module_hosting/_tenant.py | 33 ++++- .../simple_module_hosting/middleware.py | 50 ++++---- .../simple_module_hosting/permissions.py | 73 +++++++++-- .../simple_module_hosting/token_guard.py | 47 +++++-- .../tests/test_menu_providers_wiring.py | 11 +- .../tests/test_token_guard_effective_route.py | 119 ++++++++++++++++++ modules/api_tokens/api_tokens/resolver.py | 6 + .../tests/test_api_tokens_identity.py | 91 ++++++++++++++ modules/auth/auth/contracts/schemas.py | 8 ++ 12 files changed, 441 insertions(+), 55 deletions(-) create mode 100644 framework/hosting/simple_module_hosting/_anonymous.py create mode 100644 framework/hosting/tests/test_token_guard_effective_route.py create mode 100644 modules/api_tokens/tests/test_api_tokens_identity.py diff --git a/docs/framework/permissions.md b/docs/framework/permissions.md index 42f9ab9f8..400997da8 100644 --- a/docs/framework/permissions.md +++ b/docs/framework/permissions.md @@ -116,13 +116,16 @@ dependency) applies a fail-closed rule to every route: - `Depends(TokenAllowed())` from `simple_module_hosting.token_guard` admits tokens explicitly, without a permission; - every other route refuses tokens with 403 — except a public route, where the - token's principal is dropped and the request runs anonymously. + token's principal is dropped and the request runs anonymously (no user, no + permissions, and the tenant an anonymous request would resolve to). So **gate a route with a permission dependency if API tokens should reach it**; -a permission checked inline in the handler does not count, and neither does -one passed as `include_router(..., dependencies=[...])` (FastAPI keeps those out of -the route's own dependency tree, so the route refuses tokens — declare the gate -on the route or on the router it is declared on). Sessions and a +a permission checked inline in the handler does not count. A gate passed as +`include_router(..., dependencies=[...])` counts: the guard reads the route as +FastAPI runs it (`scope["fastapi"]["effective_route_context"]`), include-level +dependencies included. Should a future FastAPI stop recording that context, +such a route falls back to failing closed (tokens refused), and a page still +refuses tokens through `page_route` itself. Sessions and a provider's own bearer tokens (`auth_kind == "bearer"`) are unscoped and unaffected. A custom gate dependency can mark itself with `mark_token_gate(fn, GATE_PERMISSION)`. diff --git a/docs/modules/api_tokens.md b/docs/modules/api_tokens.md index d312d9192..83a4eb856 100644 --- a/docs/modules/api_tokens.md +++ b/docs/modules/api_tokens.md @@ -82,11 +82,12 @@ app-wide dependency on every `APIRoute` (module, host and framework routes): | A page — on a module view router, or depends on `InertiaDep` | **403**, even when gated: page props are a second, unscoped read API | | Marked `Depends(TokenAllowed())` | admitted, with no permission check — use only for routes harmless for any token | | Anything else | **403** (JSON on `/api/*`) | -| A public route (exempted from auth) without a gate | served, but **as anonymous**: the token's principal is dropped | +| A public route (exempted from auth) without a gate | served, but **as anonymous**: the token's principal is dropped, and so is the tenant it pinned | So a token cannot create or leave an organisation (`/api/tenants/`), switch tenant, open `/users/me`, the dashboard or any other page, whatever its scopes. -The decision is computed once per route from its dependency tree and cached. +The decision is computed once per route from its dependency tree — include-level +`include_router(..., dependencies=[...])` gates included — and cached. Browser sessions and a provider's own bearer tokens (`auth_kind == "bearer"`) are unscoped and never affected. A route that checks a permission inline in its handler rather than through a dependency is refused for tokens; move the check @@ -173,6 +174,19 @@ Decisions made after the security review of #455, and why: - **Stored roles are re-filtered**: the resolver drops `admin` and `tenant:*` from a service key's row, so a hand-edited row cannot make a key an admin or a tenant owner. +- **A personal token needs its owner to still hold `api_tokens.use`.** The + principal carries `required_permission = "api_tokens.use"`, checked on every + request against the owner's live permissions — roles, the tenant role in the + token's tenant, and grant sources — before the scope cap. Without it the + request runs as anonymous, so a gated route answers **401**, as for a revoked + token. Taking `api_tokens.use` away also takes away listing and revoking + tokens, so it retires them all at once rather than leaving them unmanageable. + Service keys require nothing: they are governed by `api_tokens.manage` + holders, not by an owner. +- **A token's pages show no one.** Tokens are refused on pages, but a 403 page, + or a public page served to a demoted token, still renders a layout: its + shared props (`auth`, `menus`, the `tenant` memberships) describe the + anonymous visitor, never the token's owner. ## Audit diff --git a/framework/hosting/simple_module_hosting/_anonymous.py b/framework/hosting/simple_module_hosting/_anonymous.py new file mode 100644 index 000000000..30e712387 --- /dev/null +++ b/framework/hosting/simple_module_hosting/_anonymous.py @@ -0,0 +1,27 @@ +"""Demote a request's principal to the anonymous visitor, mid-request. + +Used when a credential authenticated but must not act as its account here: an +API token on a public route (``token_guard``), or a token whose owner no +longer holds the permission it requires (``permissions``). "Anonymous" means +all of it — no user, no permissions, no audit identity, and the tenant an +anonymous request would have resolved to rather than the one the credential +pinned. +""" + +from __future__ import annotations + +from simple_module_db.listeners import current_user_id +from starlette.requests import HTTPConnection + +from simple_module_hosting._tenant import reresolve_tenant + + +async def demote_to_anonymous(request: HTTPConnection) -> None: + """Drop the principal and re-run tenant resolution as anonymous.""" + request.state.user = None + request.state.resolved_permissions = set() + current_user_id.set(None) + await reresolve_tenant(request) + + +__all__ = ["demote_to_anonymous"] diff --git a/framework/hosting/simple_module_hosting/_tenant.py b/framework/hosting/simple_module_hosting/_tenant.py index a53afb3ea..e0ad1b06f 100644 --- a/framework/hosting/simple_module_hosting/_tenant.py +++ b/framework/hosting/simple_module_hosting/_tenant.py @@ -17,6 +17,8 @@ _CREDENTIAL_VARY = ("Cookie", "Authorization") +_STATE_MIDDLEWARE = "_sm_tenant_middleware" + @dataclass(frozen=True, slots=True) class TenantResolution: @@ -123,13 +125,16 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: return request = Request(scope) + # Kept so a principal dropped later in the request can be re-resolved + # (``reresolve_tenant``) with this middleware's own configuration. + scope.setdefault("state", {})[_STATE_MIDDLEWARE] = self resolution = await self._resolve(request, scope) tenant_id = resolution.tenant_id request.state.tenant_id = tenant_id request.state.tenant_source = resolution.source request.state.tenant_vary = resolution.vary - send = self._vary_sender(send, resolution.vary) + send = self._vary_sender(scope, send, resolution.vary) if tenant_id is not None: token = current_tenant_id.set(tenant_id) try: @@ -141,19 +146,21 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: await self.app(scope, receive, send) @staticmethod - def _vary_sender(send: Send, vary: tuple[str, ...]) -> Send: + def _vary_sender(scope: Scope, send: Send, vary: tuple[str, ...]) -> Send: if not vary: return send async def wrapped(message: MutableMapping[str, Any]) -> None: if message["type"] == "http.response.start": + # Re-read: a re-resolution (``reresolve_tenant``) may add names. + names = (scope.get("state") or {}).get("tenant_vary") or vary headers = MutableHeaders(scope=message) lines = headers.getlist("vary") # Every line, not just the first (#423): MutableHeaders.get # reads one, and assigning drops the rest — including a # ``Vary: *`` on a later line. if not any("*" in _tokens(line) for line in lines): - merged = merge_vary(", ".join(lines), vary) + merged = merge_vary(", ".join(lines), tuple(names)) del headers["vary"] headers["vary"] = merged await send(message) @@ -188,10 +195,30 @@ async def _resolve(self, request: Request, scope: Scope) -> TenantResolution: return TenantResolution(None) +async def reresolve_tenant(request: Request | Any) -> None: + """Resolve the tenant again for the request's *current* principal and rebind it. + + For a principal dropped after ``TenantMiddleware`` ran: the tenant it + pinned must not outlive it. No-op without the middleware (single-tenant + hosts never bound one). The headers both answers depended on stay in + ``Vary``. + """ + middleware = (request.scope.get("state") or {}).get(_STATE_MIDDLEWARE) + if not isinstance(middleware, TenantMiddleware): + return + resolution = await middleware._resolve(request, request.scope) + previous = tuple(getattr(request.state, "tenant_vary", None) or ()) + request.state.tenant_id = resolution.tenant_id + request.state.tenant_source = resolution.source + request.state.tenant_vary = (*previous, *resolution.vary) + current_tenant_id.set(resolution.tenant_id) + + __all__ = [ "TENANT_HEADER", "TenantMiddleware", "TenantResolution", "TenantResolver", "merge_vary", + "reresolve_tenant", ] diff --git a/framework/hosting/simple_module_hosting/middleware.py b/framework/hosting/simple_module_hosting/middleware.py index ff8d5fc26..9632aafd4 100644 --- a/framework/hosting/simple_module_hosting/middleware.py +++ b/framework/hosting/simple_module_hosting/middleware.py @@ -34,7 +34,8 @@ TenantResolution, TenantResolver, ) -from simple_module_hosting.permissions import expand_permissions, resolve_principal_permissions +from simple_module_hosting.permissions import expand_permissions, resolve_request_permissions +from simple_module_hosting.token_guard import is_token_principal if TYPE_CHECKING: from simple_module_core.csp import CspSourceRegistry @@ -214,23 +215,27 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: return request = Request(scope) + # Resolve permissions once — roles plus module grant sources (GH #337) — + # and cache on request.state for RequiresPermission and the menu filter. + # May demote the principal (a credential its account no longer backs). + resolved = await resolve_request_permissions(request, self.permission_registry) user = getattr(request.state, "user", None) + if is_token_principal(user): + # An API token never legitimately renders a page, so the layout + # describes the anonymous visitor: no account, roles, permissions, + # menus or memberships on a 403 page or a public one (N2). The + # cached ``resolved`` set above still gates the token's API calls. + shared = await self._shared(request, scope, None, set()) + else: + shared = await self._shared(request, scope, user, resolved) + request.state.inertia_shared = shared + + await self.app(scope, receive, send) + + async def _shared(self, request: Request, scope: Scope, user: Any, resolved: set[str]) -> dict: + """The shared props for *user* (``None``: the anonymous visitor).""" is_authenticated = user is not None roles = getattr(user, "roles", []) if user else [] - if getattr(user, "scopes", None) is not None: - # A token's menu is filtered on its (capped) permissions alone: a - # ``roles=["admin"]`` entry would otherwise list admin screens the - # token is refused at (see ``is_admin_principal``). - roles = [] - - # Resolve permissions once — roles plus module grant sources (GH #337) — - # and cache on request.state for RequiresPermission and the menu filter. - resolved = ( - await resolve_principal_permissions(request, user, self.permission_registry) - if is_authenticated - else set() - ) - request.state.resolved_permissions = resolved # Expand wildcard to full list for frontend (no "*" leak) all_perms = self.permission_registry.all_permissions @@ -264,11 +269,14 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: } # Merge module-registered shared-prop providers (e.g. branding) — read off - # app.state without importing the plugin (SM009), defensively. - await merge_shared_prop_providers(scope["app"], request, shared) + # app.state without importing the plugin (SM009), defensively. They read + # ``request.state.user`` themselves, so they see the same principal. + actual = getattr(request.state, "user", None) + request.state.user = user + try: + await merge_shared_prop_providers(scope["app"], request, shared) + finally: + request.state.user = actual # After the providers, so a module's own value (users) wins. shared.setdefault("homeUrl", default_home_url(scope["app"])) - - request.state.inertia_shared = shared - - await self.app(scope, receive, send) + return shared diff --git a/framework/hosting/simple_module_hosting/permissions.py b/framework/hosting/simple_module_hosting/permissions.py index 1384c5c89..2ceb40c40 100644 --- a/framework/hosting/simple_module_hosting/permissions.py +++ b/framework/hosting/simple_module_hosting/permissions.py @@ -8,6 +8,7 @@ from fastapi import HTTPException, Request from simple_module_core.permissions import DEFAULT_ROLE_PERMISSIONS, WILDCARD, grants +from simple_module_hosting._anonymous import demote_to_anonymous from simple_module_hosting.token_guard import GATE_PERMISSION if TYPE_CHECKING: @@ -23,8 +24,10 @@ "apply_scope_cap", "ensure_resolved_permissions", "expand_permissions", + "lacks_required_permission", "resolve_permissions", "resolve_principal_permissions", + "resolve_request_permissions", "resolved_permissions_for", ] @@ -99,12 +102,26 @@ async def resolve_principal_permissions( rather than an unauthorised one — and is logged rather than turned into a 500 on every request. A principal already holding the wildcard skips the sources: nothing they return could widen it, and a source may cost a read. - A token principal's set is then capped at its scopes (:func:`apply_scope_cap`). + A token principal's set is then capped at its scopes (:func:`apply_scope_cap`), + and is empty when its account no longer holds the permission the + credential requires (:func:`lacks_required_permission`). """ + held = await _held_permissions(request, user, registry) + if lacks_required_permission(user, held): + return set() + return apply_scope_cap(held, user) + + +async def _held_permissions( + request: Request, + user: Any, + registry: PermissionRegistry | None, +) -> set[str]: + """What *user*'s account holds right now, before any scope cap.""" role_map = registry.role_map if registry is not None else None permissions = _role_and_claimed(user, role_map) if registry is None or WILDCARD in permissions: - return apply_scope_cap(permissions, user) + return permissions for source in registry.grant_sources: try: # Grants are additive keys only: the wildcard (admin) comes from @@ -112,7 +129,42 @@ async def resolve_principal_permissions( permissions.update(k for k in await source(request, user) if k != WILDCARD) except Exception: logger.exception("Grant source %r raised; contributing nothing", source) - return apply_scope_cap(permissions, user) + return permissions + + +def lacks_required_permission(user: Any, held: set[str]) -> bool: + """Whether *user*'s credential requires a permission its account lacks. + + A credential may name a permission its account must *currently* hold for + it to authenticate at all (``required_permission``, read duck-typed — an + ``api_tokens`` personal token requires ``api_tokens.use``). Checked + against the uncapped, tenant-aware set, so revoking the permission from a + role, a tenant role or a direct grant retires every such credential at once. + """ + required = getattr(user, "required_permission", None) + return bool(required) and not grants(held, required) + + +async def resolve_request_permissions( + request: Request, registry: PermissionRegistry | None +) -> set[str]: + """Resolve and cache the permission set of the request's principal. + + A principal whose credential requires a permission its account no longer + holds is not authenticated: it is demoted to the anonymous visitor, so a + gated route answers ``401`` — the same as for a revoked credential. + """ + user = getattr(request.state, "user", None) + if user is None: + request.state.resolved_permissions = set() + return set() + held = await _held_permissions(request, user, registry) + if lacks_required_permission(user, held): + await demote_to_anonymous(request) + return set() + permissions = apply_scope_cap(held, user) + request.state.resolved_permissions = permissions + return permissions def _registry_for(request: Request) -> PermissionRegistry | None: @@ -130,12 +182,7 @@ async def ensure_resolved_permissions(request: Request) -> set[str]: cached: set[str] | None = getattr(request.state, "resolved_permissions", None) if cached is not None: return cached - user = getattr(request.state, "user", None) - if user is None: - return set() - permissions = await resolve_principal_permissions(request, user, _registry_for(request)) - request.state.resolved_permissions = permissions - return permissions + return await resolve_request_permissions(request, _registry_for(request)) def expand_permissions( @@ -177,7 +224,8 @@ def resolved_permissions_for(request: Request) -> set[str]: perm_registry = _registry_for(request) role_map = perm_registry.role_map if perm_registry is not None else None - permissions = apply_scope_cap(_role_and_claimed(user, role_map), user) + held = _role_and_claimed(user, role_map) + permissions = set() if lacks_required_permission(user, held) else apply_scope_cap(held, user) request.state.resolved_permissions = permissions return permissions @@ -207,7 +255,10 @@ async def __call__(self, request: Request) -> None: if user is None: raise HTTPException(status_code=401, detail="Authentication required") - if not grants(await ensure_resolved_permissions(request), self.permission): + permissions = await ensure_resolved_permissions(request) + if getattr(request.state, "user", None) is None: # demoted while resolving + raise HTTPException(status_code=401, detail="Authentication required") + if not grants(permissions, self.permission): raise HTTPException( status_code=403, detail=f"{PERMISSION_DENIED_PREFIX}{self.permission}", diff --git a/framework/hosting/simple_module_hosting/token_guard.py b/framework/hosting/simple_module_hosting/token_guard.py index 70114dd8d..97b43f554 100644 --- a/framework/hosting/simple_module_hosting/token_guard.py +++ b/framework/hosting/simple_module_hosting/token_guard.py @@ -36,9 +36,10 @@ from typing import Any from fastapi import HTTPException -from simple_module_db.listeners import current_user_id from starlette.requests import HTTPConnection +from simple_module_hosting._anonymous import demote_to_anonymous + TOKEN_GATE_ATTR = "__sm_token_gate__" """Attribute a dependency callable carries to tell the guard what it is.""" @@ -84,9 +85,11 @@ async def page_route(request: HTTPConnection) -> None: It carries the page marker for the tree inspection *and* refuses a token itself: a router's own ``dependencies`` reach a route inside a nested ``include_router`` only as an include-level dependency, which FastAPI - keeps out of the route's ``dependant`` — the inspection cannot see it. + keeps out of the original route's ``dependant``. The guard reads the + effective route (:func:`effective_route`) to see it, but that is FastAPI + internals — so a page also refuses on its own, whatever the inspection saw. """ - _refuse_token(request) + await _refuse_token(request) mark_token_gate(page_route, GATE_PAGE) @@ -145,26 +148,49 @@ def _translate(request: HTTPConnection, key: str, fallback: str) -> str: return Translator(registry, locale=locale, default_locale=default).t(key) +def effective_route(scope: Any) -> Any: + """The route as FastAPI runs it: include-level dependencies included. + + FastAPI (0.143+) puts the *original* ``APIRoute`` in ``scope["route"]`` + but runs the dependant of the per-inclusion context it records under + ``scope["fastapi"]["effective_route_context"]`` — the one holding + ``include_router(..., dependencies=[...])``. Read that when it belongs to + the matched route; otherwise the matched route itself. + """ + route = scope.get("route") + fastapi_scope = scope.get("fastapi") + context = ( + fastapi_scope.get("effective_route_context") if isinstance(fastapi_scope, dict) else None + ) + if ( + context is not None + and route is not None + and getattr(context, "original_route", None) is route + and getattr(context, "dependant", None) is not None + ): + return context + return route + + async def token_route_guard(request: HTTPConnection) -> None: """App-wide dependency enforcing the rule in this module's docstring.""" if not is_token_principal(getattr(request.state, "user", None)): return # the common case: one attribute read, no tree inspection - if route_admits_tokens(request.scope.get("route")): + if route_admits_tokens(effective_route(request.scope)): return - _refuse_token(request) + await _refuse_token(request) -def _refuse_token(request: HTTPConnection) -> None: +async def _refuse_token(request: HTTPConnection) -> None: """Turn a token principal away (403), or demote it to anonymous if public.""" user = getattr(request.state, "user", None) if not is_token_principal(user): return if (request.scope.get("state") or {}).get("auth_public"): # A public route: serve the token holder as the anonymous visitor - # anyone may be, never as the account behind the token. - request.state.user = None - request.state.resolved_permissions = set() - current_user_id.set(None) + # anyone may be, never as the account behind the token — tenant + # context included. + await demote_to_anonymous(request) return raise HTTPException( status_code=403, @@ -182,6 +208,7 @@ def _refuse_token(request: HTTPConnection) -> None: "GATE_PERMISSION", "TOKEN_GATE_ATTR", "TokenAllowed", + "effective_route", "is_token_principal", "mark_token_gate", "page_route", diff --git a/framework/hosting/tests/test_menu_providers_wiring.py b/framework/hosting/tests/test_menu_providers_wiring.py index 97341a3cd..e8c7082ce 100644 --- a/framework/hosting/tests/test_menu_providers_wiring.py +++ b/framework/hosting/tests/test_menu_providers_wiring.py @@ -72,14 +72,19 @@ def test_sync_provider_and_role_filtering() -> None: assert _labels(client.get("/shared").json()) == [] # anonymous: requires_auth -def test_role_gated_item_is_hidden_from_a_scoped_admin_token() -> None: +def test_a_scoped_admin_token_gets_the_anonymous_menu() -> None: + """A token never renders a page, so its layout is the anonymous visitor's.""" menu = MenuRegistry() menu.add_provider( - lambda _r: [MenuItem(label="Admins", url="/a", roles=["admin"]), MenuItem("All", "/x")] + lambda _r: [ + MenuItem(label="Admins", url="/a", roles=["admin"]), + MenuItem("All", "/x"), + MenuItem("Public", "/p", requires_auth=False), + ] ) client = _build(menu) body = client.get("/shared", headers={"x-role": "admin", "x-scoped": "1"}).json() - assert _labels(body) == ["All"] + assert _labels(body) == ["Public"] def test_failing_provider_is_isolated(caplog) -> None: diff --git a/framework/hosting/tests/test_token_guard_effective_route.py b/framework/hosting/tests/test_token_guard_effective_route.py new file mode 100644 index 000000000..d655f4bf8 --- /dev/null +++ b/framework/hosting/tests/test_token_guard_effective_route.py @@ -0,0 +1,119 @@ +"""``token_guard`` pass 4: include-level gates, and "anonymous" means no tenant either. + +N1: FastAPI keeps ``include_router(..., dependencies=[...])`` out of the +original route's ``dependant``; the guard reads the effective route so an +include-level permission gate admits a token (capped by the gate), instead of +failing closed. N4: a token demoted to anonymous on a public route must not +keep the tenant context it pinned. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field + +import httpx +from fastapi import APIRouter, Depends, FastAPI, Request +from simple_module_db import current_tenant_id +from simple_module_hosting._tenant import TenantMiddleware, TenantResolution +from simple_module_hosting.permissions import RequiresPermission +from simple_module_hosting.token_guard import token_route_guard + + +@dataclass +class _Principal: + id: str = "u1" + roles: list[str] = field(default_factory=list) + permissions: list[str] = field(default_factory=list) + scopes: list[str] | None = None + required_permission: str | None = None + + +async def _get(app: FastAPI, path: str) -> httpx.Response: + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=app), base_url="http://t" + ) as client: + return await client.get(path) + + +def _with_auth(app: FastAPI, principal: _Principal | None, *, public: bool = False) -> FastAPI: + @app.middleware("http") + async def fake_auth(request: Request, call_next): + request.state.user = principal + request.scope.setdefault("state", {})["auth_public"] = public + return await call_next(request) + + return app + + +def _include_gated_app(principal: _Principal) -> FastAPI: + app = FastAPI(dependencies=[Depends(token_route_guard)]) + app.state.sm = None + sub = APIRouter() + + @sub.get("/x") + async def x() -> dict: + return {"ok": True} + + outer = APIRouter(prefix="/api") + outer.include_router(sub, dependencies=[Depends(RequiresPermission("x.view"))]) + app.include_router(outer) + return _with_auth(app, principal) + + +async def test_an_include_level_permission_gate_admits_a_scoped_token(): + app = _include_gated_app(_Principal(scopes=["x.view"], permissions=["x.view"])) + assert (await _get(app, "/api/x")).status_code == 200 + + +async def test_an_include_level_gate_still_caps_the_token(): + app = _include_gated_app(_Principal(scopes=["y.view"], permissions=["x.view"])) + assert (await _get(app, "/api/x")).status_code == 403 + + +async def test_a_principal_missing_its_required_permission_is_unauthenticated(): + app = _include_gated_app( + _Principal(scopes=["x.view"], permissions=["x.view"], required_permission="t.use") + ) + assert (await _get(app, "/api/x")).status_code == 401 + held = _Principal( + scopes=["x.view"], permissions=["x.view", "t.use"], required_permission="t.use" + ) + assert (await _get(_include_gated_app(held), "/api/x")).status_code == 200 + + +def _tenant_app(principal: _Principal) -> FastAPI: + app = FastAPI(dependencies=[Depends(token_route_guard)]) + app.state.sm = None + + async def resolver(request: Request) -> TenantResolution: + user = getattr(request.state, "user", None) + if user is not None: + return TenantResolution("t-token", "token", ("Authorization",)) + return TenantResolution(None, None, ("Host",)) + + app.state.tenant_resolver = resolver + + @app.get("/public") + async def public(request: Request) -> dict: + return { + "user": getattr(request.state.user, "id", None), + "ctx": current_tenant_id.get(), + "state": request.state.tenant_id, + } + + app.add_middleware(TenantMiddleware) + return _with_auth(app, principal, public=True) + + +async def test_a_token_demoted_on_a_public_route_loses_its_pinned_tenant(): + resp = await _get(_tenant_app(_Principal(scopes=["x.view"])), "/public") + assert resp.status_code == 200 + assert resp.json() == {"user": None, "ctx": None, "state": None} + # Both answers' inputs stay in Vary. + vary = {v.strip() for v in resp.headers["vary"].split(",")} + assert {"Authorization", "Host"} <= vary + + +async def test_a_session_on_a_public_route_keeps_its_tenant(): + resp = await _get(_tenant_app(_Principal(scopes=None)), "/public") + assert resp.json() == {"user": "u1", "ctx": "t-token", "state": "t-token"} diff --git a/modules/api_tokens/api_tokens/resolver.py b/modules/api_tokens/api_tokens/resolver.py index 8476fb44e..e82c6d93b 100644 --- a/modules/api_tokens/api_tokens/resolver.py +++ b/modules/api_tokens/api_tokens/resolver.py @@ -28,6 +28,7 @@ from api_tokens.constants import ( FORBIDDEN_SERVICE_ROLES, LAST_USED_WRITE_INTERVAL_SECONDS, + PERM_USE, TokenKind, ) from api_tokens.models import ApiToken @@ -72,6 +73,11 @@ async def _personal(db: Any, token: ApiToken) -> UserContext | None: # resolver acts for this tenant only (and only while the owner is still an # active member of it), never one named by a header, subdomain or session. principal.tenant_id = token.tenant_id + # A personal token lives only as long as its owner may hold tokens: losing + # ``api_tokens.use`` (which also takes away listing and revoking them) + # retires every token they own, checked against their live, tenant-aware + # permissions on each request (``lacks_required_permission``). + principal.required_permission = PERM_USE return principal diff --git a/modules/api_tokens/tests/test_api_tokens_identity.py b/modules/api_tokens/tests/test_api_tokens_identity.py new file mode 100644 index 000000000..2190be3cd --- /dev/null +++ b/modules/api_tokens/tests/test_api_tokens_identity.py @@ -0,0 +1,91 @@ +"""A token never shows its owner's identity, and dies with ``api_tokens.use``. + +Security verification, round 1 pass 4: + +* N2 — a page rendered for a token (the 403 page of a refused page route, a + public page) used to carry the owner's ``auth`` user, roles, permissions, + menus and memberships in its shared props. +* N3 — a personal token kept working after its owner lost ``api_tokens.use`` + (and with it the means to list or revoke it). +""" + +from __future__ import annotations + +from typing import Any + +from _api_tokens_support import CSRF_HEADERS, FLAGS_URL, INERTIA_HEADERS, SERVICE_URL, create_pat + +USE = "api_tokens.use" + + +def _assert_anonymous_props(resp: Any, email: str) -> None: + props = resp.json()["props"] + assert props["auth"] == {"user": None, "isAuthenticated": False, "permissions": []} + assert "tenant" not in props + assert email not in resp.text + + +async def test_refused_page_does_not_render_the_owner(actor, bearer): + async with actor(admin=True) as a: + token = (await create_pat(a.client, [USE, "feature_flags.view"]))["token"] + email = (await a.client.get("/users/me", headers=INERTIA_HEADERS)).json()["props"]["auth"][ + "user" + ]["email"] + + resp = await bearer("GET", "/users/me", token, headers=INERTIA_HEADERS) + assert resp.status_code == 403 + _assert_anonymous_props(resp, email) + + +async def test_public_page_renders_a_token_as_anonymous(actor, bearer): + async with actor() as u: + token = (await create_pat(u.client, [USE]))["token"] + email = (await u.client.get("/users/me", headers=INERTIA_HEADERS)).json()["props"]["auth"][ + "user" + ]["email"] + + resp = await bearer("GET", "/users/login", token, headers=INERTIA_HEADERS) + assert resp.status_code == 200 + _assert_anonymous_props(resp, email) + + +def _revoke_use(app: Any, roles: list[str]) -> None: + registry = app.state.sm.permissions + for role in roles: + registry._role_map.get(role, set()).discard(USE) + registry._invalidate() + + +async def test_pat_stops_working_when_its_owner_loses_the_use_permission(app, actor, bearer): + registry = app.state.sm.permissions + registry.map_role("tenant:owner", ["feature_flags.view"]) + async with actor() as u: + token = (await create_pat(u.client, [USE, "feature_flags.view"]))["token"] + assert (await bearer("GET", FLAGS_URL, token)).status_code == 200 + + # Still held through the tenant role: the check is tenant-aware. + _revoke_use(app, ["user"]) + assert (await bearer("GET", FLAGS_URL, token)).status_code == 200 + + _revoke_use(app, [r for r in registry.role_map if r.startswith("tenant:")]) + assert (await bearer("GET", FLAGS_URL, token)).status_code == 401 + + +async def test_service_key_does_not_depend_on_the_use_permission(app, actor, bearer): + registry = app.state.sm.permissions + registry.map_role("user", ["feature_flags.view"]) + async with actor(admin=True) as a: + resp = await a.client.post( + SERVICE_URL, + json={ + "name": "k", + "roles": ["user"], + "scopes": ["feature_flags.view"], + "expires_in_days": 30, + }, + headers=CSRF_HEADERS, + ) + assert resp.status_code == 201, resp.text + token = resp.json()["token"] + _revoke_use(app, list(registry.role_map)) + assert (await bearer("GET", FLAGS_URL, token)).status_code == 200 diff --git a/modules/auth/auth/contracts/schemas.py b/modules/auth/auth/contracts/schemas.py index 3c705127c..12415b44c 100644 --- a/modules/auth/auth/contracts/schemas.py +++ b/modules/auth/auth/contracts/schemas.py @@ -38,6 +38,12 @@ class UserContext: # principal resolves to, and the admin-role shortcuts are skipped, so an # admin's token is worth exactly the scopes it was given. scopes: list[str] | None = None + # A permission the account must *currently* hold for this credential to + # authenticate at all — ``api_tokens.use`` for a personal access token. + # Checked on every request against the account's live, tenant-aware + # permissions; when missing, the request runs as anonymous (401 on gated + # routes). ``None`` (a session, a service key) requires nothing. + required_permission: str | None = None @property def is_scoped(self) -> bool: @@ -85,6 +91,7 @@ def to_session_dict(self) -> dict[str, Any]: "permissions": list(self.permissions), "auth_kind": self.auth_kind, "scopes": list(self.scopes) if self.scopes is not None else None, + "required_permission": self.required_permission, } @classmethod @@ -104,6 +111,7 @@ def from_session_dict(cls, payload: Any) -> UserContext | None: permissions=list(payload.get("permissions") or []), auth_kind=str(payload.get("auth_kind") or AUTH_KIND_SESSION), scopes=_scopes_from(payload.get("scopes")), + required_permission=payload.get("required_permission") or None, ) except (KeyError, TypeError, ValueError): return None From 1f58d8711ec5e56fb66ad28a190da77f3b3838e6 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 15 Oct 2026 14:21:29 +0200 Subject: [PATCH 9/9] fix: address code review findings (round 1, pass 5) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .../hosting/simple_module_hosting/_tenant.py | 17 +++++----- .../tests/test_token_guard_effective_route.py | 31 +++++++++++++++++++ 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/framework/hosting/simple_module_hosting/_tenant.py b/framework/hosting/simple_module_hosting/_tenant.py index e0ad1b06f..2844e64ff 100644 --- a/framework/hosting/simple_module_hosting/_tenant.py +++ b/framework/hosting/simple_module_hosting/_tenant.py @@ -135,15 +135,14 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: request.state.tenant_vary = resolution.vary send = self._vary_sender(scope, send, resolution.vary) - if tenant_id is not None: - token = current_tenant_id.set(tenant_id) - try: - await self.app(scope, receive, send) - finally: - current_tenant_id.reset(token) - return - - await self.app(scope, receive, send) + # Always bound and reset, even for ``None``: a principal demoted later in + # the request re-resolves the tenant (``reresolve_tenant``) and may set + # one this request did not start with, which must not outlive it. + token = current_tenant_id.set(tenant_id) + try: + await self.app(scope, receive, send) + finally: + current_tenant_id.reset(token) @staticmethod def _vary_sender(scope: Scope, send: Send, vary: tuple[str, ...]) -> Send: diff --git a/framework/hosting/tests/test_token_guard_effective_route.py b/framework/hosting/tests/test_token_guard_effective_route.py index d655f4bf8..34eb84b06 100644 --- a/framework/hosting/tests/test_token_guard_effective_route.py +++ b/framework/hosting/tests/test_token_guard_effective_route.py @@ -117,3 +117,34 @@ async def test_a_token_demoted_on_a_public_route_loses_its_pinned_tenant(): async def test_a_session_on_a_public_route_keeps_its_tenant(): resp = await _get(_tenant_app(_Principal(scopes=None)), "/public") assert resp.json() == {"user": "u1", "ctx": "t-token", "state": "t-token"} + + +async def test_fastapi_still_records_the_effective_route_context(): + """Pins the FastAPI internals ``effective_route`` reads; fails loudly if they move. + + ``scope["fastapi"]["effective_route_context"]`` (FastAPI 0.143) carries + ``original_route`` and the ``dependant`` that includes include-level + dependencies. If an upgrade renames it the guard fails closed (403 for + every token on an include-gated route) — this test names the cause. + """ + from simple_module_hosting.token_guard import effective_route + + app = FastAPI() + sub = APIRouter() + seen: dict = {} + + @sub.get("/x") + async def x(request: Request) -> dict: + seen["route"] = request.scope.get("route") + seen["effective"] = effective_route(request.scope) + seen["context"] = (request.scope.get("fastapi") or {}).get("effective_route_context") + return {} + + app.include_router(sub, dependencies=[Depends(RequiresPermission("x.view"))]) + _with_auth(app, _Principal(permissions=["x.view"])) + assert (await _get(app, "/x")).status_code == 200 + context = seen["context"] + assert context is not None, "FastAPI no longer sets scope['fastapi']['effective_route_context']" + assert context.original_route is seen["route"] + assert context.dependant is not None + assert seen["effective"] is context