From c08b2ccbe86b4eec8c6488384634c556c20d3610 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 21 May 2026 22:49:09 +0200 Subject: [PATCH 01/16] docs(spec): add auth principal-resolver chain design (#163) Design for an extension-point that lets downstream modules (e.g. smpy_gis) plug in a bearer-token / PAT resolver without upstreaming PAT storage. --- ...26-05-21-auth-principal-resolver-design.md | 193 ++++++++++++++++++ 1 file changed, 193 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-21-auth-principal-resolver-design.md diff --git a/docs/superpowers/specs/2026-05-21-auth-principal-resolver-design.md b/docs/superpowers/specs/2026-05-21-auth-principal-resolver-design.md new file mode 100644 index 00000000..0ecf827c --- /dev/null +++ b/docs/superpowers/specs/2026-05-21-auth-principal-resolver-design.md @@ -0,0 +1,193 @@ +# Auth principal-resolver chain + +**Status:** Approved, ready for implementation plan. +**Issue:** [#163 — simple_module_auth: expose a principal resolver that handles session cookie OR PAT bearer token](https://github.com/antosubash/simple_module_python/issues/163) +**Driver:** Unblock `smpy_gis` T3 (Personal Access Token workstream) by **end of M3 (2026-08-31)**. + +## Problem + +`smpy_gis` needs scripted, non-browser callers (CI jobs, GIS automation) to hit `/api/gis/*` using `Authorization: Bearer pat_xxx`. Today the only way to authenticate against `simple_module_python` is the session cookie set by `users.AuthMiddleware`; there is no extension point to plug in a second credential source. + +Goal: a single `UserContext` arrives on `request.state.user` regardless of whether the caller authenticated with a session cookie or a bearer token. Every existing `get_current_user` / `require_permission` consumer keeps working without change. + +Non-goal: shipping a PAT model, endpoints, or admin UI in upstream. Token storage is GIS-scoped (`gis_personal_access_tokens` table, per smpy_gis spec) and stays in the downstream module. + +## Decision + +Add a **principal-resolver chain** as an extension point owned by the `auth` module. Downstream modules register async resolvers in their `on_startup` hook; `users.AuthMiddleware` consults them after the session-cookie path fails. + +Resolution order on every request: +1. **Session cookie** (existing fast path with `session["user_ctx"]` cache → DB fall-back). +2. **Registered resolvers**, in registration order, first non-`None` wins. +3. **Unauthenticated** → 401 JSON for `/api/*` paths, 302 to `/users/login` for view paths (with the original URL stashed in `session["next"]`). + +Resolvers are request-scoped: they MUST NOT write to the session. A PAT call does not silently elevate to a long-lived session cookie. + +## Architecture + +### New types in `auth.contracts.resolver` + +```python +from collections.abc import Awaitable, Callable +from starlette.requests import Request +from auth.contracts.schemas import UserContext + +PrincipalResolver = Callable[[Request], Awaitable[UserContext | None]] +``` + +Documented invariants on every `PrincipalResolver`: +- Async; safe to call on every request (cheap bail when the credential type is absent — e.g., no `Authorization` header). +- Performs its own active/disabled checks before returning a `UserContext`. +- Returns `None` (never raises) for "credentials absent" or "credentials invalid"; the chain continues. +- Does not write to the session. + +### New `auth.state.AuthState` + +```python +from dataclasses import dataclass, field +from auth.contracts.resolver import PrincipalResolver + +@dataclass +class AuthState: + principal_resolvers: list[PrincipalResolver] = field(default_factory=list) +``` + +### `AuthModule.register_settings` + +```python +def register_settings(self, app: FastAPI) -> None: + from auth.state import AuthState + app.state.auth = AuthState() +``` + +Module ordering: `UsersModule` already declares `depends_on=["Auth"]`. The framework runs `AuthModule.register_settings` before `UsersModule.register_middleware`, so `app.state.auth.principal_resolvers` is guaranteed to exist by the time `AuthMiddleware` reads it. Third-party modules that register resolvers also declare `depends_on=["Auth"]` and register in `on_startup`, which runs after every module's `register_*` hooks. + +### `auth/__init__.py` re-exports + +```python +from auth.contracts.resolver import PrincipalResolver +from auth.contracts.schemas import UserContext + +__all__ = ["PrincipalResolver", "UserContext"] +``` + +So downstream authors do `from auth import PrincipalResolver, UserContext` without reaching into `contracts/`. + +### `users.AuthMiddleware` changes + +Two narrowly-scoped changes in `modules/users/users/middleware.py`: + +**(1) Resolver fall-through.** After the existing session-cookie block, if `user_ctx is None`, iterate `app.state.auth.principal_resolvers`: + +```python +if user_ctx is None: + resolvers = getattr(scope["app"].state.auth, "principal_resolvers", ()) + if resolvers: + request = Request(scope) + for resolver in resolvers: + try: + user_ctx = await resolver(request) + except Exception: + logger.exception( + "Principal resolver %r raised; treating as no-match", resolver + ) + continue + if user_ctx is not None: + break +``` + +**(2) 401 JSON for `/api/*` instead of redirect.** Replace the existing single-branch redirect with: + +```python +if user_ctx is None and not is_public: + if path.startswith("/api/"): + response = JSONResponse({"detail": "Not authenticated"}, status_code=401) + else: + request = Request(scope) + session[_SESSION_NEXT_KEY] = str(request.url) + response = RedirectResponse(_LOGIN_REDIRECT, status_code=302) + await response(scope, receive, send) + return +``` + +Everything downstream of `user_ctx` resolution stays identical — the `current_user_id` ContextVar set/reset, `request.state.user = user_ctx` assignment, and the call to `self.app(...)` are unchanged. Resolvers populate the same `UserContext` shape the session path produces, so `TenantMiddleware`, `InertiaLayoutDataMiddleware`, audit listeners, and every `require_permission`-protected endpoint keep working with no edits. + +## Behavior changes for existing deployments + +There is one observable behavior change for session-only deployments: an unauthenticated XHR to a private `/api/*` route today returns `302 → /users/login` (HTML), and after this change returns `401 {"detail": "Not authenticated"}` (JSON). This is strictly better for scripted callers and for frontend XHR error handling; no existing test relies on the 302-for-API behavior (verified during exploration — the relevant tests in `modules/users/tests/test_users_middleware.py` cover view-route redirects). + +No other change is visible to session-cookie users. The resolver chain is empty by default; the fall-through path is a no-op when no module registers a resolver. + +## Public surface (apps build PAT resolvers against this) + +```python +# In a downstream module's on_startup +from auth import PrincipalResolver, UserContext + +async def my_pat_resolver(request: Request) -> UserContext | None: + header = request.headers.get("Authorization") + if not header or not header.startswith("Bearer "): + return None + token = header.removeprefix("Bearer ") + # Look up token in app's own storage, validate, load user + roles + record = await my_token_store.find_active(token) + if record is None: + return None + user = await my_user_loader.load(record.user_id) + if user is None or not user.is_active or user.disabled_at is not None: + return None + return UserContext.from_user(user) + +# in on_startup: +app.state.auth.principal_resolvers.append(my_pat_resolver) +``` + +## Testing + +**`modules/auth/tests/test_resolver_registry.py`** (new): +- `AuthState()` initializes with empty `principal_resolvers`. +- `AuthModule().register_settings(app)` populates `app.state.auth` with an `AuthState` instance. +- Type-only smoke: a callable matching `PrincipalResolver` typechecks. + +**`modules/users/tests/test_users_middleware.py`** (extend): +- Single resolver returns a `UserContext` → `request.state.user` is set, session keys unchanged. +- Two resolvers, first returns `None`, second returns context → second's context is used. +- All resolvers return `None` → unauthenticated path (redirect for view, 401 JSON for `/api/*`). +- Resolver raises → exception logged, chain continues to next resolver. +- `/api/private` unauthenticated → 401 JSON, no redirect. +- `/some-view` unauthenticated → 302 to `/users/login`, `session["next"]` set. +- Valid session cookie + registered resolver → session wins, resolver not called. + +**`tests/test_principal_resolver_integration.py`** (new, repo root): +- Build an app with the standard fixtures plus a fake bearer-token resolver pointing at a test user. +- `Authorization: Bearer good` on a permission-protected endpoint → 200, principal honored. +- `Authorization: Bearer bad` → 401 JSON. +- No auth header → 401 JSON. +- Valid session cookie + `Bearer bad` → 200 via session; resolver chain not consulted. + +No e2e changes — this is plumbing; the GIS app adds e2e for its own PAT flow. + +## Documentation + +- **`docs/framework/principal-resolvers.md`** (new, authoritative reference): the resolver contract, invariants, ordering, worked Bearer-token example, "when NOT to use a resolver". +- **`docs/framework-conventions.md`** (edit): one-paragraph pointer to the new doc under the existing auth section. +- `CLAUDE.md`: no change. The framework conventions doc + the new framework doc cover the surface. + +## Out of scope + +- PAT model, endpoints, or admin UI in upstream (GIS owns its `gis_personal_access_tokens` table and admin pages). +- LRU/in-memory caching of resolver results — resolvers are free to cache internally; the framework doesn't impose a strategy. +- Priority/ordering controls beyond registration order. +- Resolver-driven session writes (explicitly forbidden by the contract). +- Changes to OAuth or fastapi-users routers. +- Multi-tenant resolver shimming — resolvers populate `UserContext.tenant_id` themselves, and `TenantMiddleware` already consumes it. + +## Acceptance (mirrors issue #163) + +- `get_current_user` (and the underlying middleware) resolves session OR bearer in the documented order. +- Existing session-only consumers unaffected. +- Documented worked example in `docs/framework/principal-resolvers.md` shows an app adding a custom resolver. + +## Timeline + +- Implementation + tests + docs PR: needed in `main` by **end of M3 (2026-08-31)** so smpy_gis T3 can pick it up. From 024e7c68b570f85aff6128e43acf3b5ca0655953 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 21 May 2026 22:54:20 +0200 Subject: [PATCH 02/16] docs(plan): add auth principal-resolver implementation plan (#163) Step-by-step TDD plan covering type alias, AuthState registry, AuthModule hook, middleware fall-through + 401-for-/api/* branch, integration test, and framework docs. --- .../2026-05-21-auth-principal-resolver.md | 1102 +++++++++++++++++ 1 file changed, 1102 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-21-auth-principal-resolver.md diff --git a/docs/superpowers/plans/2026-05-21-auth-principal-resolver.md b/docs/superpowers/plans/2026-05-21-auth-principal-resolver.md new file mode 100644 index 00000000..554aaffe --- /dev/null +++ b/docs/superpowers/plans/2026-05-21-auth-principal-resolver.md @@ -0,0 +1,1102 @@ +# Auth Principal-Resolver Chain Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Expose a principal-resolver chain on `app.state.auth` so downstream modules (e.g. `smpy_gis`) can plug in a PAT / bearer-token resolver alongside the existing session-cookie auth, with no changes to existing session-only consumers. + +**Architecture:** New `PrincipalResolver` type + `AuthState` registry owned by the `auth` module. `users.AuthMiddleware` falls through `session-cookie → registered resolvers → unauthenticated`. Unauthenticated `/api/*` paths return 401 JSON; view paths still 302-redirect to `/users/login`. Resolvers are request-scoped (must not write the session). + +**Tech Stack:** Python 3.12, FastAPI, Starlette, SQLModel, pytest-anyio, httpx (test client). + +**Spec:** `docs/superpowers/specs/2026-05-21-auth-principal-resolver-design.md` + +**Issue:** [#163](https://github.com/antosubash/simple_module_python/issues/163) + +--- + +## File map + +| Path | Action | Purpose | +|---|---|---| +| `modules/auth/auth/contracts/resolver.py` | Create | `PrincipalResolver` type alias + invariant docstring | +| `modules/auth/auth/state.py` | Create | `AuthState` dataclass (registry holder) | +| `modules/auth/auth/module.py` | Modify | Add `register_settings` that puts `AuthState` on `app.state.auth` | +| `modules/auth/auth/__init__.py` | Modify | Re-export `PrincipalResolver` and `UserContext` | +| `modules/auth/tests/test_resolver_registry.py` | Create | Unit tests for the registry + `register_settings` | +| `modules/users/users/middleware.py` | Modify | Resolver fall-through + 401-JSON-for-`/api/*` branch | +| `modules/users/tests/_middleware_support.py` | Modify | `_build_app` accepts optional resolvers and seeds `app.state.auth` | +| `modules/users/tests/test_users_middleware.py` | Modify | New tests: resolver flow + API-401 branch | +| `tests/test_principal_resolver_integration.py` | Create | End-to-end test against `create_app(settings)` with a fake bearer resolver | +| `docs/framework/principal-resolvers.md` | Create | Authoritative reference + worked Bearer-token example | +| `docs/framework-conventions.md` | Modify | One-paragraph pointer to the new doc | + +--- + +## Task 1: Add the `PrincipalResolver` type alias + +**Files:** +- Create: `modules/auth/auth/contracts/resolver.py` +- Test: `modules/auth/tests/test_resolver_registry.py` (first test) + +- [ ] **Step 1: Write the failing test** + +Create `modules/auth/tests/test_resolver_registry.py`: + +```python +"""Tests for the auth.contracts.resolver type + AuthState registry.""" + +from __future__ import annotations + +from collections.abc import Awaitable, Callable + +from auth.contracts.resolver import PrincipalResolver +from auth.contracts.schemas import UserContext +from starlette.requests import Request + + +def test_principal_resolver_type_accepts_async_callable(): + """A typical resolver signature should satisfy the type alias at runtime. + + The alias is documentation + a checkable shape — we exercise the shape + by constructing one and asserting it's a Callable that returns an + awaitable. + """ + + async def fake_resolver(request: Request) -> UserContext | None: + return None + + # Runtime — alias resolves to Callable[..., Awaitable[...]] + resolver: PrincipalResolver = fake_resolver + assert callable(resolver) + # Sanity: the function actually returns an awaitable when called. + from unittest.mock import MagicMock + + result = resolver(MagicMock(spec=Request)) + assert isinstance(result, Awaitable) + result.close() # don't leave an unawaited coroutine +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `uv run pytest modules/auth/tests/test_resolver_registry.py -v` +Expected: FAIL with `ModuleNotFoundError: No module named 'auth.contracts.resolver'`. + +- [ ] **Step 3: Create the resolver module** + +Create `modules/auth/auth/contracts/resolver.py`: + +```python +"""Principal-resolver extension point — apps register additional auth sources here. + +A ``PrincipalResolver`` is an async callable that inspects an incoming +``Request`` and returns a :class:`~auth.contracts.schemas.UserContext` if it +can authenticate the caller, or ``None`` to fall through to the next resolver +in the chain. + +The chain is consulted by ``users.middleware.AuthMiddleware`` *after* the +session-cookie path has been tried, in registration order, and the first +non-``None`` return wins. + +Invariants every resolver MUST satisfy: + +* **Async.** Resolvers are awaited. +* **Cheap fast-path bail.** Resolvers run on every request; return ``None`` + immediately when the credential type isn't present (e.g., no ``Authorization`` + header, no matching scheme). +* **Self-checks active/disabled state.** The middleware does NOT re-validate + the user after the resolver returns — return ``None`` for disabled, + unverified, or otherwise blocked users. +* **Never raise on bad credentials.** Return ``None`` and let the chain + continue. The middleware wraps each resolver in ``try/except`` for + defense in depth, but resolver authors should not rely on it. +* **Request-scoped — no session writes.** A PAT call must not silently + elevate to a long-lived session cookie. If the resolver needs to mint a + session, that's an entirely separate code path (the standard login flow). +""" + +from __future__ import annotations + +from collections.abc import Awaitable, Callable + +from starlette.requests import Request + +from auth.contracts.schemas import UserContext + +PrincipalResolver = Callable[[Request], Awaitable[UserContext | None]] +"""Async callable: ``(Request) -> UserContext | None``. See module docstring +for the invariants resolver authors must uphold.""" + +__all__ = ["PrincipalResolver"] +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `uv run pytest modules/auth/tests/test_resolver_registry.py -v` +Expected: PASS — single test green. + +- [ ] **Step 5: Commit** + +```bash +git add modules/auth/auth/contracts/resolver.py modules/auth/tests/test_resolver_registry.py +git commit -m "feat(auth): add PrincipalResolver type alias for credential-chain extension" +``` + +--- + +## Task 2: Add the `AuthState` dataclass + +**Files:** +- Create: `modules/auth/auth/state.py` +- Test: `modules/auth/tests/test_resolver_registry.py` (append) + +- [ ] **Step 1: Write the failing test** + +Append to `modules/auth/tests/test_resolver_registry.py`: + +```python +def test_auth_state_initializes_with_empty_resolvers(): + from auth.state import AuthState + + state = AuthState() + assert state.principal_resolvers == [] + + +def test_auth_state_resolvers_is_mutable_list(): + """Modules register resolvers by appending; the list must be a list, not a tuple.""" + from auth.state import AuthState + + state = AuthState() + + async def resolver(request): # pragma: no cover - registration smoke only + return None + + state.principal_resolvers.append(resolver) + assert state.principal_resolvers == [resolver] +``` + +- [ ] **Step 2: Run tests to verify the new ones fail** + +Run: `uv run pytest modules/auth/tests/test_resolver_registry.py -v` +Expected: FAIL with `ModuleNotFoundError: No module named 'auth.state'`. + +- [ ] **Step 3: Create `state.py`** + +Create `modules/auth/auth/state.py`: + +```python +"""Module-owned state attached to ``app.state.auth`` by ``AuthModule.register_settings``. + +Holds the principal-resolver registry (see +``auth.contracts.resolver.PrincipalResolver``). Apps register additional +resolvers from their ``on_startup`` hook:: + + app.state.auth.principal_resolvers.append(my_pat_resolver) +""" + +from __future__ import annotations + +from dataclasses import dataclass, field + +from auth.contracts.resolver import PrincipalResolver + + +@dataclass +class AuthState: + """Per-app auth registry. Initialized empty; modules append resolvers.""" + + principal_resolvers: list[PrincipalResolver] = field(default_factory=list) + + +__all__ = ["AuthState"] +``` + +- [ ] **Step 4: Run the suite to verify all tests pass** + +Run: `uv run pytest modules/auth/tests/test_resolver_registry.py -v` +Expected: PASS — three tests green. + +- [ ] **Step 5: Commit** + +```bash +git add modules/auth/auth/state.py modules/auth/tests/test_resolver_registry.py +git commit -m "feat(auth): add AuthState registry for principal-resolvers" +``` + +--- + +## Task 3: Wire `AuthModule.register_settings` + +**Files:** +- Modify: `modules/auth/auth/module.py` +- Test: `modules/auth/tests/test_resolver_registry.py` (append) + +- [ ] **Step 1: Write the failing test** + +Append to `modules/auth/tests/test_resolver_registry.py`: + +```python +def test_auth_module_register_settings_populates_app_state(): + """``AuthModule.register_settings(app)`` must put an AuthState on ``app.state.auth``.""" + from fastapi import FastAPI + + from auth.module import AuthModule + from auth.state import AuthState + + app = FastAPI() + AuthModule().register_settings(app) + + assert isinstance(app.state.auth, AuthState) + assert app.state.auth.principal_resolvers == [] +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `uv run pytest modules/auth/tests/test_resolver_registry.py::test_auth_module_register_settings_populates_app_state -v` +Expected: FAIL with `AttributeError: 'State' object has no attribute 'auth'` (because `register_settings` is the no-op default from `ModuleBase`). + +- [ ] **Step 3: Add `register_settings` to `AuthModule`** + +Modify `modules/auth/auth/module.py` — add the new hook just before `locale_dirs`: + +```python +"""Auth module — shared contracts (UserContext, deps). + +Intentionally minimal: this module owns the PUBLIC interface (UserContext, +PrincipalResolver, get_current_user, CurrentUser, require_permission) that +every other module imports. Keeping it stable prevents churn when auth +internals change. + +All authentication logic (middleware, login, signup, OAuth) lives in the +users module. The ``principal_resolvers`` registry on ``app.state.auth`` is +the extension point downstream modules use to plug in additional credential +sources (PAT bearer tokens, API keys, etc.) — see +``docs/framework/principal-resolvers.md`` for the worked example. +""" + +from __future__ import annotations + +import importlib.resources +from pathlib import Path +from typing import TYPE_CHECKING + +from simple_module_core.module import ModuleBase, ModuleMeta + +if TYPE_CHECKING: + from fastapi import FastAPI + + +class AuthModule(ModuleBase): + meta = ModuleMeta( + name="Auth", + route_prefix="/auth", + ) + + def register_settings(self, app: FastAPI) -> None: + from auth.state import AuthState + + app.state.auth = AuthState() + + def locale_dirs(self) -> dict[str, Path]: + return {"auth": Path(str(importlib.resources.files(__package__) / "locales"))} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `uv run pytest modules/auth/tests/test_resolver_registry.py modules/auth/tests/test_module.py -v` +Expected: PASS — all four resolver tests + the three existing module tests stay green. + +- [ ] **Step 5: Commit** + +```bash +git add modules/auth/auth/module.py modules/auth/tests/test_resolver_registry.py +git commit -m "feat(auth): seed app.state.auth with AuthState in register_settings" +``` + +--- + +## Task 4: Re-export the public surface from `auth/__init__.py` + +**Files:** +- Modify: `modules/auth/auth/__init__.py` +- Test: `modules/auth/tests/test_resolver_registry.py` (append) + +- [ ] **Step 1: Write the failing test** + +Append to `modules/auth/tests/test_resolver_registry.py`: + +```python +def test_auth_package_reexports_public_surface(): + """Downstream authors should be able to ``from auth import PrincipalResolver, UserContext``.""" + import auth + + assert hasattr(auth, "PrincipalResolver") + assert hasattr(auth, "UserContext") + assert "PrincipalResolver" in auth.__all__ + assert "UserContext" in auth.__all__ + + # Identity check — re-exports point at the canonical definitions. + from auth.contracts.resolver import PrincipalResolver + from auth.contracts.schemas import UserContext + + assert auth.PrincipalResolver is PrincipalResolver + assert auth.UserContext is UserContext +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `uv run pytest modules/auth/tests/test_resolver_registry.py::test_auth_package_reexports_public_surface -v` +Expected: FAIL with `AssertionError` (no `PrincipalResolver` attribute on the `auth` package — its `__init__.py` is currently just a docstring). + +- [ ] **Step 3: Add the re-exports** + +Overwrite `modules/auth/auth/__init__.py`: + +```python +"""Auth module — shared contracts (UserContext, PrincipalResolver, deps).""" + +from auth.contracts.resolver import PrincipalResolver +from auth.contracts.schemas import UserContext + +__all__ = ["PrincipalResolver", "UserContext"] +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `uv run pytest modules/auth/tests/test_resolver_registry.py -v` +Expected: PASS — all five tests green. + +- [ ] **Step 5: Commit** + +```bash +git add modules/auth/auth/__init__.py modules/auth/tests/test_resolver_registry.py +git commit -m "feat(auth): re-export PrincipalResolver + UserContext from package root" +``` + +--- + +## Task 5: Update `_middleware_support` to seed `app.state.auth` + +**Files:** +- Modify: `modules/users/tests/_middleware_support.py` + +The middleware change in Task 6 will read `app.state.auth.principal_resolvers`. We update the test helper first so every existing middleware test keeps passing once the middleware lands, and so new tests can register resolvers via a keyword arg. + +- [ ] **Step 1: Modify `_build_app` to accept resolvers and seed `app.state.auth`** + +Edit `modules/users/tests/_middleware_support.py` — replace the `_build_app` function with: + +```python +async def _build_app(db_state, inner_handler=None, *, principal_resolvers=None): + """Build a minimal ASGI app with AuthMiddleware + SessionMiddleware. + + ``principal_resolvers`` (optional) is a list of resolvers seeded onto + ``app.state.auth.principal_resolvers`` before the middleware runs. + Defaults to an empty registry — matches a production app where no + downstream module has registered anything. + """ + from auth.state import AuthState + + async def _default_handler(request: Request): + user = getattr(request.state, "user", None) + return JSONResponse( + { + "path": request.url.path, + "user": ( + { + "id": user.id, + "email": user.email, + "name": user.name, + "roles": user.roles, + "tenant_id": user.tenant_id, + } + if user is not None + else None + ), + } + ) + + handler = inner_handler or _default_handler + + app = FastAPI() + app.state.sm = SimpleNamespace(db=db_state) + app.state.auth = AuthState( + principal_resolvers=list(principal_resolvers or []), + ) + + @app.get("/{path:path}") + async def _catch_all(request: Request, path: str = ""): + return await handler(request) + + # Middleware is applied in reverse order: SessionMiddleware outermost. + app.add_middleware(AuthMiddleware) + app.add_middleware(SessionMiddleware, secret_key=SECRET_KEY) + return app +``` + +- [ ] **Step 2: Confirm the existing middleware suite still passes** + +Run: `uv run pytest modules/users/tests/test_users_middleware.py modules/users/tests/test_users_middleware_public_paths.py -v` +Expected: PASS — no behavior change yet (resolver registry is empty; AuthMiddleware hasn't been touched). + +- [ ] **Step 3: Commit** + +```bash +git add modules/users/tests/_middleware_support.py +git commit -m "test(users): seed app.state.auth in middleware test helper" +``` + +--- + +## Task 6: Add resolver fall-through + 401-for-`/api/*` to `AuthMiddleware` + +**Files:** +- Modify: `modules/users/users/middleware.py` +- Test: `modules/users/tests/test_users_middleware.py` (append) + +We write the full suite of middleware tests for the new behavior first, then implement. + +- [ ] **Step 1: Write the failing tests** + +Append the following section to `modules/users/tests/test_users_middleware.py` (just before any final pytest module-level fixtures, or at the bottom — order doesn't matter): + +```python +# --------------------------------------------------------------------------- +# Principal-resolver chain +# --------------------------------------------------------------------------- + + +def _ctx(uid: str = "11111111-1111-1111-1111-111111111111", **overrides): + """Build a UserContext for resolver tests.""" + from auth.contracts.schemas import UserContext + + fields = dict( + id=uid, + email="pat@example.com", + name="PAT User", + roles=["user"], + tenant_id=None, + ) + fields.update(overrides) + return UserContext(**fields) + + +@pytest.mark.anyio +async def test_resolver_returning_context_authenticates_request(db_state): + """A registered resolver that returns a UserContext authenticates the request.""" + + async def stub_resolver(request): + return _ctx() + + app = await _build_app(db_state, principal_resolvers=[stub_resolver]) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard") + + assert resp.status_code == 200 + data = resp.json() + assert data["user"]["email"] == "pat@example.com" + + +@pytest.mark.anyio +async def test_resolver_first_non_none_wins(db_state): + """The first resolver returning a context wins; later resolvers are not consulted.""" + second_called = False + + async def first_none(request): + return None + + async def second_returns(request): + nonlocal second_called + second_called = True + return _ctx(email="second@example.com") + + async def third_should_not_run(request): + raise AssertionError("third resolver should not run after a match") + + app = await _build_app( + db_state, + principal_resolvers=[first_none, second_returns, third_should_not_run], + ) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard") + + assert resp.status_code == 200 + assert resp.json()["user"]["email"] == "second@example.com" + assert second_called + + +@pytest.mark.anyio +async def test_all_resolvers_return_none_falls_through_to_redirect(db_state): + """When every resolver returns None for a view route → 302 to /users/login.""" + + async def none_resolver(request): + return None + + app = await _build_app(db_state, principal_resolvers=[none_resolver]) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard", follow_redirects=False) + + assert resp.status_code == 302 + assert resp.headers["location"] == "/users/login" + + +@pytest.mark.anyio +async def test_resolver_raising_does_not_crash_middleware(db_state, caplog): + """A resolver that raises is logged and the chain continues to the next.""" + import logging + + async def boom(request): + raise RuntimeError("resolver kaboom") + + async def fallback(request): + return _ctx(email="fallback@example.com") + + app = await _build_app(db_state, principal_resolvers=[boom, fallback]) + transport = httpx.ASGITransport(app=app) + with caplog.at_level(logging.ERROR, logger="users.middleware"): + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard") + + assert resp.status_code == 200 + assert resp.json()["user"]["email"] == "fallback@example.com" + assert any("resolver" in rec.message.lower() for rec in caplog.records) + + +@pytest.mark.anyio +async def test_api_path_unauthenticated_returns_401_json(db_state): + """Unauthenticated /api/private should return 401 JSON, not a 302 redirect.""" + app = await _build_app(db_state) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/api/private-thing", follow_redirects=False) + + assert resp.status_code == 401 + assert resp.headers["content-type"].startswith("application/json") + assert resp.json() == {"detail": "Not authenticated"} + + +@pytest.mark.anyio +async def test_view_path_unauthenticated_still_redirects(db_state): + """View routes (non-/api/*) keep the existing 302-to-login behavior.""" + app = await _build_app(db_state) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard", follow_redirects=False) + + assert resp.status_code == 302 + assert resp.headers["location"] == "/users/login" + + +@pytest.mark.anyio +async def test_session_wins_over_resolver(db_state, mw_active_user): + """A valid session cookie short-circuits — the resolver chain is not consulted.""" + resolver_called = False + + async def should_not_run(request): + nonlocal resolver_called + resolver_called = True + return _ctx(email="should-not-win@example.com") + + app = await _build_app(db_state, principal_resolvers=[should_not_run]) + transport = httpx.ASGITransport(app=app) + cookies = _session_cookie({"user_id": str(mw_active_user.id)}) + async with httpx.AsyncClient( + transport=transport, base_url="http://testserver", cookies=cookies + ) as client: + resp = await client.get("/dashboard") + + assert resp.status_code == 200 + assert resp.json()["user"]["email"] == "middleware-test@example.com" + assert resolver_called is False + + +@pytest.mark.anyio +async def test_resolver_does_not_write_session(db_state): + """Resolver-authenticated requests must not persist anything to the session.""" + captured = {} + + async def capture(request: Request): + captured["session"] = dict(request.session) + user = getattr(request.state, "user", None) + return JSONResponse({"authenticated": user is not None}) + + async def stub_resolver(request): + return _ctx() + + app = await _build_app(db_state, capture, principal_resolvers=[stub_resolver]) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard") + + assert resp.status_code == 200 + assert resp.json() == {"authenticated": True} + # Session must not contain a user_id, user_ctx, or anything resolver-added. + assert "user_id" not in captured["session"] + assert "user_ctx" not in captured["session"] +``` + +- [ ] **Step 2: Run the new tests to verify they fail** + +Run: `uv run pytest modules/users/tests/test_users_middleware.py -v -k "resolver or api_path or view_path or session_wins"` +Expected: most/all of the eight new tests FAIL — `test_resolver_returning_context_authenticates_request` will redirect (302) instead of 200, `test_api_path_unauthenticated_returns_401_json` will get 302 not 401, etc. + +- [ ] **Step 3: Update `AuthMiddleware`** + +Modify `modules/users/users/middleware.py`. Two changes inside `AuthMiddleware.__call__`: + +**(a)** Replace the import block at the top of the file. Add `JSONResponse`: + +```python +from starlette.responses import JSONResponse, RedirectResponse +``` + +**(b)** Replace the body of `__call__` from the `session = scope["session"]` line through the end of the `if user_ctx is None and not is_public:` block with the version below. The DB-load fast path, `request.state.user` assignment, and `current_user_id` ContextVar lifecycle stay exactly as they were. + +```python + session = scope["session"] + raw_user_id = session.get(_SESSION_USER_ID_KEY) + + user_ctx: UserContext | None = None + if raw_user_id: + user_id_str = str(raw_user_id) + # Fast path — rebuild from the signed session cookie. + user_ctx = UserContext.from_session_dict(session.get(SESSION_USER_CTX_KEY)) + if user_ctx is None or user_ctx.id != user_id_str: + try: + user_uuid = uuid.UUID(user_id_str) + except (ValueError, TypeError): + logger.warning("Invalid user_id in session: %r", raw_user_id) + session.pop(_SESSION_USER_ID_KEY, None) + session.pop(SESSION_USER_CTX_KEY, None) + user_ctx = None + else: + user_ctx = await self._load_user(scope, user_uuid) + if user_ctx is None: + # User was deleted / disabled since session creation. + session.pop(_SESSION_USER_ID_KEY, None) + session.pop(SESSION_USER_CTX_KEY, None) + else: + session[SESSION_USER_CTX_KEY] = user_ctx.to_session_dict() + + # Fall-through: registered principal resolvers (PAT, API key, ...). + # The session-cookie path above is authoritative; resolvers only run + # when no session-authenticated user was resolved. + if user_ctx is None: + auth_state = getattr(scope["app"].state, "auth", None) + resolvers = getattr(auth_state, "principal_resolvers", ()) if auth_state else () + if resolvers: + request = Request(scope) + for resolver in resolvers: + try: + user_ctx = await resolver(request) + except Exception: + logger.exception( + "Principal resolver %r raised; treating as no-match", + resolver, + ) + continue + if user_ctx is not None: + break + + if user_ctx is None and not is_public: + if path.startswith("/api/"): + response = JSONResponse( + {"detail": "Not authenticated"}, status_code=401 + ) + else: + request = Request(scope) + session[_SESSION_NEXT_KEY] = str(request.url) + response = RedirectResponse(_LOGIN_REDIRECT, status_code=302) + await response(scope, receive, send) + return +``` + +(Everything after this block — the `if user_ctx is not None:` setting `request.state.user` and managing `current_user_id` — stays exactly as today.) + +- [ ] **Step 4: Run the new tests to verify they pass** + +Run: `uv run pytest modules/users/tests/test_users_middleware.py -v` +Expected: PASS — both the eight new resolver/API tests AND every pre-existing middleware test stay green. + +- [ ] **Step 5: Run the entire users test directory to catch any collateral damage** + +Run: `uv run pytest modules/users/tests/ -q` +Expected: PASS — no regressions in the OAuth, bootstrap, admin, public-paths, or other suites. + +- [ ] **Step 6: Commit** + +```bash +git add modules/users/users/middleware.py modules/users/tests/test_users_middleware.py +git commit -m "feat(users): consult app.state.auth.principal_resolvers + 401-JSON for /api/*" +``` + +--- + +## Task 7: End-to-end integration test + +**Files:** +- Create: `tests/test_principal_resolver_integration.py` + +This exercises the full `create_app(settings)` boot path — `AuthModule.register_settings` runs, `UsersModule.register_middleware` installs `AuthMiddleware`, and a resolver registered before the first request is consulted. + +- [ ] **Step 1: Write the failing test** + +Create `tests/test_principal_resolver_integration.py`: + +```python +"""End-to-end test: a fake bearer-token resolver authenticates against the full app stack.""" + +from __future__ import annotations + +from collections.abc import AsyncGenerator + +import httpx +import pytest +from auth.contracts.schemas import UserContext + + +@pytest.fixture +async def app_with_pat_resolver(app): + """Reuses the standard ``app`` fixture and appends a fake bearer-token resolver. + + The resolver recognizes a single hardcoded token ``"good"`` mapped to a + deterministic UserContext. ``"bad"`` (or absent header) returns None. + """ + + async def fake_pat_resolver(request) -> UserContext | None: + header = request.headers.get("Authorization", "") + if not header.startswith("Bearer "): + return None + token = header.removeprefix("Bearer ") + if token != "good": + return None + return UserContext( + id="22222222-2222-2222-2222-222222222222", + email="pat-user@example.com", + name="PAT User", + roles=["admin"], + tenant_id=None, + ) + + app.state.auth.principal_resolvers.append(fake_pat_resolver) + yield app + app.state.auth.principal_resolvers.remove(fake_pat_resolver) + + +@pytest.fixture +async def pat_client(app_with_pat_resolver) -> AsyncGenerator[httpx.AsyncClient, None]: + transport = httpx.ASGITransport(app=app_with_pat_resolver) + async with httpx.AsyncClient( + transport=transport, base_url="http://testserver" + ) as c: + yield c + + +@pytest.mark.anyio +async def test_bearer_token_authenticates_against_protected_view(pat_client): + """Valid bearer token → 200 on a protected view path (users admin).""" + resp = await pat_client.get( + "/users/admin", + headers={"Authorization": "Bearer good"}, + follow_redirects=False, + ) + # /users/admin is a view route; with a valid resolver the request gets + # through AuthMiddleware (200) instead of redirecting to /users/login. + assert resp.status_code == 200 + + +@pytest.mark.anyio +async def test_invalid_bearer_token_returns_401_on_api_path(pat_client): + """Bad bearer on an /api/* path → 401 JSON, not a redirect.""" + resp = await pat_client.get( + "/api/users/admin/users", + headers={"Authorization": "Bearer bad"}, + follow_redirects=False, + ) + assert resp.status_code == 401 + assert resp.json() == {"detail": "Not authenticated"} + + +@pytest.mark.anyio +async def test_no_auth_header_on_api_returns_401(pat_client): + """No Authorization header on a private /api/* path → 401 JSON.""" + resp = await pat_client.get( + "/api/users/admin/users", follow_redirects=False + ) + assert resp.status_code == 401 + assert resp.json() == {"detail": "Not authenticated"} + + +@pytest.mark.anyio +async def test_session_wins_over_bad_bearer(authenticated_client): + """A valid session cookie + Bearer bad → 200 via session; resolver not consulted. + + The ``authenticated_client`` fixture already carries an admin session cookie; + here we additionally send a bad bearer to prove the session path wins.""" + resp = await authenticated_client.get( + "/api/users/admin/users", + headers={"Authorization": "Bearer bad"}, + ) + assert resp.status_code == 200 +``` + +- [ ] **Step 2: Run the new tests** + +Run: `uv run pytest tests/test_principal_resolver_integration.py -v` +Expected: PASS — four green tests. + +If a test fails with "Authorization header was rejected by an inner FastAPI dependency": adjust the target URL to a permission-protected endpoint that doesn't require additional dependency-injected arguments. `/api/users/admin/users` is a known admin route (see `modules/users/users/admin/api.py`); if its actual prefix differs, run `git grep 'admin_router' modules/users/` to locate the right URL and update the test. + +- [ ] **Step 3: Commit** + +```bash +git add tests/test_principal_resolver_integration.py +git commit -m "test: end-to-end integration test for principal-resolver chain" +``` + +--- + +## Task 8: Authoritative documentation + +**Files:** +- Create: `docs/framework/principal-resolvers.md` + +- [ ] **Step 1: Write the new doc** + +Create `docs/framework/principal-resolvers.md`: + +````markdown +# Principal-resolver chain + +The `auth` module exposes an extension point — a list of async resolvers on +`app.state.auth.principal_resolvers` — that lets downstream modules plug in +additional credential sources alongside the built-in session cookie. This is +the supported way to add Personal Access Tokens, API keys, JWT bearers, or +any other request-scoped authentication scheme without forking +`users.AuthMiddleware`. + +## The contract + +```python +from collections.abc import Awaitable, Callable +from starlette.requests import Request +from auth import PrincipalResolver, UserContext + +PrincipalResolver = Callable[[Request], Awaitable[UserContext | None]] +``` + +A resolver MUST: + +- Be **async** (it is awaited by the middleware). +- **Bail fast** when its credential type isn't present (e.g., return `None` + immediately if there is no `Authorization` header) — resolvers run on + every request, including completely unauthenticated ones. +- **Self-check active / disabled state** before returning a `UserContext`. + The middleware does not re-validate. +- **Never raise** on bad credentials — return `None` so the chain continues + to the next resolver. The middleware swallows exceptions defensively but + resolver authors should not rely on it. +- **Not write to the session.** Resolver-authenticated requests are + per-request only; they never silently elevate into a long-lived session + cookie. (To mint a session, use the standard login flow.) + +## Resolution order + +`users.AuthMiddleware` consults credential sources in this order: + +1. **Session cookie** — the existing fast/cached path. If the session + carries a valid `user_id` (and matching cached `user_ctx`), the user is + authenticated and the resolver chain is **not** consulted. +2. **Registered resolvers**, in registration order. The first non-`None` + return wins. +3. **Unauthenticated** — for `/api/*` paths the middleware returns + `401 {"detail": "Not authenticated"}`; for view paths it 302-redirects to + `/users/login` and stashes the original URL in `session["next"]`. + +## Worked example — bearer-token resolver + +A module that ships its own Personal-Access-Token table registers a +resolver from its `on_startup` hook: + +```python +# modules/example/example/module.py +from __future__ import annotations + +from typing import TYPE_CHECKING + +from auth import PrincipalResolver, UserContext +from simple_module_core.module import ModuleBase, ModuleMeta +from starlette.requests import Request + +if TYPE_CHECKING: + from fastapi import FastAPI + + +class ExampleModule(ModuleBase): + meta = ModuleMeta(name="Example", depends_on=["Auth", "Users"]) + + async def on_startup(self, app: FastAPI) -> None: + app.state.auth.principal_resolvers.append(self._build_pat_resolver(app)) + + @staticmethod + def _build_pat_resolver(app: FastAPI) -> PrincipalResolver: + async def resolve_pat(request: Request) -> UserContext | None: + header = request.headers.get("Authorization", "") + if not header.startswith("Bearer "): + return None + token = header.removeprefix("Bearer ") + + # Look up the token in the module's own storage and load the user. + async with app.state.sm.db.session_factory() as session: + record = await find_active_token(session, token) + if record is None: + return None + user = await load_user_with_roles(session, record.user_id) + if user is None or not user.is_active or user.disabled_at is not None: + return None + return UserContext.from_user(user) + + return resolve_pat +``` + +`depends_on=["Auth", "Users"]` ensures `AuthModule.register_settings` has +run (so `app.state.auth` exists) and `UsersModule.register_middleware` has +installed `AuthMiddleware` (which calls the resolvers). + +## When NOT to write a resolver + +- **You want to mint a long-lived session.** Use the standard login flow + (`/users/login` or OAuth). Resolvers are explicitly forbidden from + writing the session. +- **You only need a per-endpoint API-key check.** A FastAPI dependency + (`require_api_key`) on the route signature is simpler and keeps the + authenticated-user shape clean. +- **You want to override the `users` module's behavior** (e.g., reject + active users, change role semantics). Resolvers add credential sources; + they don't change the rules of authentication. For that, swap + `UsersModule`/`AuthMiddleware` outright. + +## Testing your resolver + +Write resolver tests against a minimal app (see +`modules/users/tests/_middleware_support.py::_build_app` for the pattern +used by the framework's own resolver suite — it takes a +`principal_resolvers=` keyword and seeds `app.state.auth` for you). + +End-to-end tests should drive the full `create_app(settings)` stack and +append your resolver to `app.state.auth.principal_resolvers` in a fixture — +see `tests/test_principal_resolver_integration.py` for a worked example. +```` + +- [ ] **Step 2: Commit** + +```bash +git add docs/framework/principal-resolvers.md +git commit -m "docs(framework): add principal-resolver chain reference" +``` + +--- + +## Task 9: Pointer from `framework-conventions.md` + +**Files:** +- Modify: `docs/framework-conventions.md` + +- [ ] **Step 1: Locate the auth section** + +Run: `grep -n -i "auth\|principal\|session" docs/framework-conventions.md | head -20` + +Identify a sensible location near the existing auth/session discussion. If there is no dedicated auth section, append a new "Authentication extension points" section at the end of the document. + +- [ ] **Step 2: Add a pointer paragraph** + +Add this paragraph (adjust the surrounding heading level to match the file's style): + +```markdown +### Authentication extension points + +The `auth` module exposes a principal-resolver chain on +`app.state.auth.principal_resolvers` — a list of async callables that +`users.AuthMiddleware` consults after the session-cookie path. Use it to add +non-cookie credential sources (Personal Access Tokens, API keys, JWTs) +without forking the middleware. See +[`docs/framework/principal-resolvers.md`](framework/principal-resolvers.md) +for the contract, ordering rules, and a worked Bearer-token example. +``` + +- [ ] **Step 3: Commit** + +```bash +git add docs/framework-conventions.md +git commit -m "docs: link framework-conventions to the principal-resolver reference" +``` + +--- + +## Task 10: Final verification — lint, doctor, full test suite + +**Files:** none + +- [ ] **Step 1: Run lint** + +Run: `make lint` +Expected: PASS — Ruff format-check, Ruff, `ty`, Biome, `tsc`, and the 300-line file-size check all green. If `ty` flags the new resolver type, double-check the alias is importable from a stable path; if Ruff flags an unused import, prune it. + +- [ ] **Step 2: Run module-doctor** + +Run: `make doctor` +Expected: no new `SM*` errors. Specifically: +- `SM007` (module overrides no hooks) should NOT fire for `Auth` — `register_settings` is now overridden. +- `SM012` (`register_settings` overridden but nothing on `app.state.`) should NOT fire — we set `app.state.auth`. +- All other codes unaffected by this change. + +- [ ] **Step 3: Run the full Python test suite** + +Run: `make test-py` +Expected: PASS — no regressions across `framework/`, `modules/`, or root `tests/`. + +- [ ] **Step 4: Confirm the commit log** + +Run: `git log --oneline -15` +Expected: roughly nine commits since the spec — one per task (1-3 from Task 1, 2, 3; one each from Tasks 4-9). If any task left uncommitted changes, commit them now with an appropriate message before opening the PR. + +- [ ] **Step 5: Open the PR** + +The branch is `worktree-issue-163-principal-resolver-spec`. Push and open the PR: + +```bash +git push -u origin worktree-issue-163-principal-resolver-spec +gh pr create --title "feat(auth): principal-resolver chain for session-or-bearer auth (#163)" --body "$(cat <<'EOF' +## Summary +- Adds `app.state.auth.principal_resolvers`, an extension point for plugging in non-cookie auth (PAT bearer tokens, API keys, JWT, etc.) — closes #163. +- `users.AuthMiddleware` now falls through `session cookie → registered resolvers → unauthenticated`; unauthenticated `/api/*` returns `401 {"detail": "Not authenticated"}`, view paths still 302 to `/users/login`. +- New authoritative doc at `docs/framework/principal-resolvers.md` with a worked Bearer-token example. + +## Test plan +- [x] `uv run pytest modules/auth/tests/test_resolver_registry.py -v` +- [x] `uv run pytest modules/users/tests/test_users_middleware.py -v` +- [x] `uv run pytest tests/test_principal_resolver_integration.py -v` +- [x] `make lint` +- [x] `make doctor` +- [x] `make test-py` +EOF +)" +``` + +--- + +## Self-review (engineer should re-skim before starting Task 1) + +- **Spec coverage** — every section in `docs/superpowers/specs/2026-05-21-auth-principal-resolver-design.md` maps to a task here: + - Architecture → Tasks 1-4 (types, state, module hook, re-exports). + - Middleware changes (resolver fall-through + 401 for `/api/*`) → Task 6. + - Test list → Tasks 5 (helper setup) + 6 (middleware tests) + 7 (integration). + - Docs section → Tasks 8 + 9. + - "Behavior changes for existing deployments" — covered by the `test_api_path_unauthenticated_returns_401_json` + `test_view_path_unauthenticated_still_redirects` pair in Task 6, plus the full middleware suite re-run in Task 6 Step 5. + - "Out of scope" — by construction, nothing in this plan adds PAT models, caching, priority controls, or session-writing resolvers. + +- **Placeholder scan** — every code step ships the actual code. No TBDs, no "similar to above", no "add error handling". + +- **Type consistency** — `PrincipalResolver`, `AuthState`, `UserContext` are named identically across every task; `principal_resolvers` (snake_case, plural) is consistent everywhere; the resolver signature `(Request) -> Awaitable[UserContext | None]` matches between the type alias, the middleware loop, the worked-example doc, and the integration test. From 2a0562c5e289504bd73e024a3285eeef3fe29945 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 21 May 2026 22:58:20 +0200 Subject: [PATCH 03/16] feat(auth): add PrincipalResolver type alias for credential-chain extension --- modules/auth/auth/contracts/resolver.py | 41 ++++++++++++++++++++ modules/auth/tests/test_resolver_registry.py | 31 +++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 modules/auth/auth/contracts/resolver.py create mode 100644 modules/auth/tests/test_resolver_registry.py diff --git a/modules/auth/auth/contracts/resolver.py b/modules/auth/auth/contracts/resolver.py new file mode 100644 index 00000000..d0621cda --- /dev/null +++ b/modules/auth/auth/contracts/resolver.py @@ -0,0 +1,41 @@ +"""Principal-resolver extension point — apps register additional auth sources here. + +A ``PrincipalResolver`` is an async callable that inspects an incoming +``Request`` and returns a :class:`~auth.contracts.schemas.UserContext` if it +can authenticate the caller, or ``None`` to fall through to the next resolver +in the chain. + +The chain is consulted by ``users.middleware.AuthMiddleware`` *after* the +session-cookie path has been tried, in registration order, and the first +non-``None`` return wins. + +Invariants every resolver MUST satisfy: + +* **Async.** Resolvers are awaited. +* **Cheap fast-path bail.** Resolvers run on every request; return ``None`` + immediately when the credential type isn't present (e.g., no ``Authorization`` + header, no matching scheme). +* **Self-checks active/disabled state.** The middleware does NOT re-validate + the user after the resolver returns — return ``None`` for disabled, + unverified, or otherwise blocked users. +* **Never raise on bad credentials.** Return ``None`` and let the chain + continue. The middleware wraps each resolver in ``try/except`` for + defense in depth, but resolver authors should not rely on it. +* **Request-scoped — no session writes.** A PAT call must not silently + elevate to a long-lived session cookie. If the resolver needs to mint a + session, that's an entirely separate code path (the standard login flow). +""" + +from __future__ import annotations + +from collections.abc import Awaitable, Callable + +from starlette.requests import Request + +from auth.contracts.schemas import UserContext + +PrincipalResolver = Callable[[Request], Awaitable[UserContext | None]] +"""Async callable: ``(Request) -> UserContext | None``. See module docstring +for the invariants resolver authors must uphold.""" + +__all__ = ["PrincipalResolver"] diff --git a/modules/auth/tests/test_resolver_registry.py b/modules/auth/tests/test_resolver_registry.py new file mode 100644 index 00000000..56662f49 --- /dev/null +++ b/modules/auth/tests/test_resolver_registry.py @@ -0,0 +1,31 @@ +"""Tests for the auth.contracts.resolver type + AuthState registry.""" + +from __future__ import annotations + +from collections.abc import Awaitable, Callable + +from auth.contracts.resolver import PrincipalResolver +from auth.contracts.schemas import UserContext +from starlette.requests import Request + + +def test_principal_resolver_type_accepts_async_callable(): + """A typical resolver signature should satisfy the type alias at runtime. + + The alias is documentation + a checkable shape — we exercise the shape + by constructing one and asserting it's a Callable that returns an + awaitable. + """ + + async def fake_resolver(request: Request) -> UserContext | None: + return None + + # Runtime — alias resolves to Callable[..., Awaitable[...]] + resolver: PrincipalResolver = fake_resolver + assert callable(resolver) + # Sanity: the function actually returns an awaitable when called. + from unittest.mock import MagicMock + + result = resolver(MagicMock(spec=Request)) + assert isinstance(result, Awaitable) + result.close() # don't leave an unawaited coroutine From cc76d7fa942c4b81dba4f61a01fa71078c83a3fd Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 21 May 2026 22:58:50 +0200 Subject: [PATCH 04/16] feat(auth): add AuthState registry for principal-resolvers --- modules/auth/auth/state.py | 24 ++++++++++++++++++++ modules/auth/tests/test_resolver_registry.py | 20 ++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 modules/auth/auth/state.py diff --git a/modules/auth/auth/state.py b/modules/auth/auth/state.py new file mode 100644 index 00000000..cc457da7 --- /dev/null +++ b/modules/auth/auth/state.py @@ -0,0 +1,24 @@ +"""Module-owned state attached to ``app.state.auth`` by ``AuthModule.register_settings``. + +Holds the principal-resolver registry (see +``auth.contracts.resolver.PrincipalResolver``). Apps register additional +resolvers from their ``on_startup`` hook:: + + app.state.auth.principal_resolvers.append(my_pat_resolver) +""" + +from __future__ import annotations + +from dataclasses import dataclass, field + +from auth.contracts.resolver import PrincipalResolver + + +@dataclass +class AuthState: + """Per-app auth registry. Initialized empty; modules append resolvers.""" + + principal_resolvers: list[PrincipalResolver] = field(default_factory=list) + + +__all__ = ["AuthState"] diff --git a/modules/auth/tests/test_resolver_registry.py b/modules/auth/tests/test_resolver_registry.py index 56662f49..70e26cad 100644 --- a/modules/auth/tests/test_resolver_registry.py +++ b/modules/auth/tests/test_resolver_registry.py @@ -29,3 +29,23 @@ async def fake_resolver(request: Request) -> UserContext | None: result = resolver(MagicMock(spec=Request)) assert isinstance(result, Awaitable) result.close() # don't leave an unawaited coroutine + + +def test_auth_state_initializes_with_empty_resolvers(): + from auth.state import AuthState + + state = AuthState() + assert state.principal_resolvers == [] + + +def test_auth_state_resolvers_is_mutable_list(): + """Modules register resolvers by appending; the list must be a list, not a tuple.""" + from auth.state import AuthState + + state = AuthState() + + async def resolver(request): # pragma: no cover - registration smoke only + return None + + state.principal_resolvers.append(resolver) + assert state.principal_resolvers == [resolver] From cdf400604c146b58a5779ec8b7ea0e6b78bd3c24 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 21 May 2026 22:59:22 +0200 Subject: [PATCH 05/16] feat(auth): seed app.state.auth with AuthState in register_settings --- modules/auth/auth/module.py | 19 ++++++++++++++++--- modules/auth/tests/test_resolver_registry.py | 14 ++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/modules/auth/auth/module.py b/modules/auth/auth/module.py index 880f35e0..6a75bd4b 100644 --- a/modules/auth/auth/module.py +++ b/modules/auth/auth/module.py @@ -1,20 +1,28 @@ """Auth module — shared contracts (UserContext, deps). Intentionally minimal: this module owns the PUBLIC interface (UserContext, -get_current_user, CurrentUser, require_permission) that every other module -imports. Keeping it stable prevents churn when auth internals change. +PrincipalResolver, get_current_user, CurrentUser, require_permission) that +every other module imports. Keeping it stable prevents churn when auth +internals change. All authentication logic (middleware, login, signup, OAuth) lives in the -users module. +users module. The ``principal_resolvers`` registry on ``app.state.auth`` is +the extension point downstream modules use to plug in additional credential +sources (PAT bearer tokens, API keys, etc.) — see +``docs/framework/principal-resolvers.md`` for the worked example. """ from __future__ import annotations import importlib.resources from pathlib import Path +from typing import TYPE_CHECKING from simple_module_core.module import ModuleBase, ModuleMeta +if TYPE_CHECKING: + from fastapi import FastAPI + class AuthModule(ModuleBase): meta = ModuleMeta( @@ -22,5 +30,10 @@ class AuthModule(ModuleBase): route_prefix="/auth", ) + def register_settings(self, app: FastAPI) -> None: + from auth.state import AuthState + + app.state.auth = AuthState() + def locale_dirs(self) -> dict[str, Path]: return {"auth": Path(str(importlib.resources.files(__package__) / "locales"))} diff --git a/modules/auth/tests/test_resolver_registry.py b/modules/auth/tests/test_resolver_registry.py index 70e26cad..63a52742 100644 --- a/modules/auth/tests/test_resolver_registry.py +++ b/modules/auth/tests/test_resolver_registry.py @@ -49,3 +49,17 @@ async def resolver(request): # pragma: no cover - registration smoke only state.principal_resolvers.append(resolver) assert state.principal_resolvers == [resolver] + + +def test_auth_module_register_settings_populates_app_state(): + """``AuthModule.register_settings(app)`` must put an AuthState on ``app.state.auth``.""" + from fastapi import FastAPI + + from auth.module import AuthModule + from auth.state import AuthState + + app = FastAPI() + AuthModule().register_settings(app) + + assert isinstance(app.state.auth, AuthState) + assert app.state.auth.principal_resolvers == [] From adb06c334bfabe257a7ffc51796398c2a5443b51 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 21 May 2026 22:59:51 +0200 Subject: [PATCH 06/16] feat(auth): re-export PrincipalResolver + UserContext from package root --- modules/auth/auth/__init__.py | 7 ++++++- modules/auth/tests/test_resolver_registry.py | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/modules/auth/auth/__init__.py b/modules/auth/auth/__init__.py index 5ed05575..1188109b 100644 --- a/modules/auth/auth/__init__.py +++ b/modules/auth/auth/__init__.py @@ -1 +1,6 @@ -"""Auth module — shared contracts (UserContext, deps).""" +"""Auth module — shared contracts (UserContext, PrincipalResolver, deps).""" + +from auth.contracts.resolver import PrincipalResolver +from auth.contracts.schemas import UserContext + +__all__ = ["PrincipalResolver", "UserContext"] diff --git a/modules/auth/tests/test_resolver_registry.py b/modules/auth/tests/test_resolver_registry.py index 63a52742..e669b9df 100644 --- a/modules/auth/tests/test_resolver_registry.py +++ b/modules/auth/tests/test_resolver_registry.py @@ -63,3 +63,20 @@ def test_auth_module_register_settings_populates_app_state(): assert isinstance(app.state.auth, AuthState) assert app.state.auth.principal_resolvers == [] + + +def test_auth_package_reexports_public_surface(): + """Downstream authors should be able to ``from auth import PrincipalResolver, UserContext``.""" + import auth + + assert hasattr(auth, "PrincipalResolver") + assert hasattr(auth, "UserContext") + assert "PrincipalResolver" in auth.__all__ + assert "UserContext" in auth.__all__ + + # Identity check — re-exports point at the canonical definitions. + from auth.contracts.resolver import PrincipalResolver + from auth.contracts.schemas import UserContext + + assert auth.PrincipalResolver is PrincipalResolver + assert auth.UserContext is UserContext From e2c03b4b7fffa589c6adc795b5022b8917a3cba1 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 21 May 2026 23:03:38 +0200 Subject: [PATCH 07/16] chore(auth): drop unused Callable import in resolver registry tests --- modules/auth/tests/test_resolver_registry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/auth/tests/test_resolver_registry.py b/modules/auth/tests/test_resolver_registry.py index e669b9df..4b615fc8 100644 --- a/modules/auth/tests/test_resolver_registry.py +++ b/modules/auth/tests/test_resolver_registry.py @@ -2,7 +2,7 @@ from __future__ import annotations -from collections.abc import Awaitable, Callable +from collections.abc import Awaitable from auth.contracts.resolver import PrincipalResolver from auth.contracts.schemas import UserContext From dfeca2e614d649c0cc83bde8e8b202d9f102f5bf Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 21 May 2026 23:04:39 +0200 Subject: [PATCH 08/16] test(users): seed app.state.auth in middleware test helper --- modules/users/tests/_middleware_support.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/modules/users/tests/_middleware_support.py b/modules/users/tests/_middleware_support.py index 03615624..fcd53ced 100644 --- a/modules/users/tests/_middleware_support.py +++ b/modules/users/tests/_middleware_support.py @@ -36,8 +36,15 @@ def _session_cookie(data: dict[str, Any]) -> dict[str, str]: return {"session": _sign_session(data)} -async def _build_app(db_state, inner_handler=None): - """Build a minimal ASGI app with AuthMiddleware + SessionMiddleware.""" +async def _build_app(db_state, inner_handler=None, *, principal_resolvers=None): + """Build a minimal ASGI app with AuthMiddleware + SessionMiddleware. + + ``principal_resolvers`` (optional) is a list of resolvers seeded onto + ``app.state.auth.principal_resolvers`` before the middleware runs. + Defaults to an empty registry — matches a production app where no + downstream module has registered anything. + """ + from auth.state import AuthState async def _default_handler(request: Request): user = getattr(request.state, "user", None) @@ -62,6 +69,9 @@ async def _default_handler(request: Request): app = FastAPI() app.state.sm = SimpleNamespace(db=db_state) + app.state.auth = AuthState( + principal_resolvers=list(principal_resolvers or []), + ) @app.get("/{path:path}") async def _catch_all(request: Request, path: str = ""): From ba7c1df0113d7eb62f3ba7619d5b26480a81e4c4 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 21 May 2026 23:11:33 +0200 Subject: [PATCH 09/16] feat(users): consult app.state.auth.principal_resolvers + 401-JSON for /api/* --- modules/users/tests/test_api_admin.py | 17 +- modules/users/tests/test_users_middleware.py | 177 +++++++++++++++++++ modules/users/users/middleware.py | 33 +++- 3 files changed, 214 insertions(+), 13 deletions(-) diff --git a/modules/users/tests/test_api_admin.py b/modules/users/tests/test_api_admin.py index 8322db3a..37642bc0 100644 --- a/modules/users/tests/test_api_admin.py +++ b/modules/users/tests/test_api_admin.py @@ -46,11 +46,10 @@ class TestAdminList: @pytest.mark.anyio async def test_list_without_auth_is_rejected(self, anon_client): resp = await anon_client.get("/api/users/admin", follow_redirects=False) - # AuthMiddleware redirects unauthenticated non-public API paths to - # /users/login. Preserving the 302 here so a regression to 401 or - # pass-through is caught. - assert resp.status_code == 302 - assert resp.headers["location"].endswith("/users/login") + # AuthMiddleware returns 401 JSON for unauthenticated /api/* paths + # (view routes still get a 302 redirect to /users/login). + assert resp.status_code == 401 + assert resp.json() == {"detail": "Not authenticated"} @pytest.mark.anyio async def test_list_as_admin_returns_200(self, admin_client, users_db): @@ -128,8 +127,8 @@ async def test_invite_without_auth_is_rejected(self, anon_client): json={"email": "hacker@example.com"}, follow_redirects=False, ) - assert resp.status_code == 302 - assert resp.headers["location"].endswith("/users/login") + assert resp.status_code == 401 + assert resp.json() == {"detail": "Not authenticated"} # --------------------------------------------------------------------------- @@ -205,8 +204,8 @@ async def test_set_roles_without_auth_is_rejected(self, anon_client): json={"role_names": ["admin"]}, follow_redirects=False, ) - assert resp.status_code == 302 - assert resp.headers["location"].endswith("/users/login") + assert resp.status_code == 401 + assert resp.json() == {"detail": "Not authenticated"} @pytest.mark.anyio async def test_set_roles_nonexistent_returns_404(self, admin_client): diff --git a/modules/users/tests/test_users_middleware.py b/modules/users/tests/test_users_middleware.py index 43b77c6d..ec6ebd0a 100644 --- a/modules/users/tests/test_users_middleware.py +++ b/modules/users/tests/test_users_middleware.py @@ -217,3 +217,180 @@ async def _capture_contextvar(request: Request): # After the request completes, the ContextVar should be reset to its # default (no value set in this outer scope). assert current_user_id.get(None) is None + + +# --------------------------------------------------------------------------- +# Principal-resolver chain +# --------------------------------------------------------------------------- + + +def _ctx(uid: str = "11111111-1111-1111-1111-111111111111", **overrides): + """Build a UserContext for resolver tests.""" + from auth.contracts.schemas import UserContext + + fields = dict( + id=uid, + email="pat@example.com", + name="PAT User", + roles=["user"], + tenant_id=None, + ) + fields.update(overrides) + return UserContext(**fields) + + +@pytest.mark.anyio +async def test_resolver_returning_context_authenticates_request(db_state): + """A registered resolver that returns a UserContext authenticates the request.""" + + async def stub_resolver(request): + return _ctx() + + app = await _build_app(db_state, principal_resolvers=[stub_resolver]) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard") + + assert resp.status_code == 200 + data = resp.json() + assert data["user"]["email"] == "pat@example.com" + + +@pytest.mark.anyio +async def test_resolver_first_non_none_wins(db_state): + """The first resolver returning a context wins; later resolvers are not consulted.""" + second_called = False + + async def first_none(request): + return None + + async def second_returns(request): + nonlocal second_called + second_called = True + return _ctx(email="second@example.com") + + async def third_should_not_run(request): + raise AssertionError("third resolver should not run after a match") + + app = await _build_app( + db_state, + principal_resolvers=[first_none, second_returns, third_should_not_run], + ) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard") + + assert resp.status_code == 200 + assert resp.json()["user"]["email"] == "second@example.com" + assert second_called + + +@pytest.mark.anyio +async def test_all_resolvers_return_none_falls_through_to_redirect(db_state): + """When every resolver returns None for a view route → 302 to /users/login.""" + + async def none_resolver(request): + return None + + app = await _build_app(db_state, principal_resolvers=[none_resolver]) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard", follow_redirects=False) + + assert resp.status_code == 302 + assert resp.headers["location"] == "/users/login" + + +@pytest.mark.anyio +async def test_resolver_raising_does_not_crash_middleware(db_state, caplog): + """A resolver that raises is logged and the chain continues to the next.""" + import logging + + async def boom(request): + raise RuntimeError("resolver kaboom") + + async def fallback(request): + return _ctx(email="fallback@example.com") + + app = await _build_app(db_state, principal_resolvers=[boom, fallback]) + transport = httpx.ASGITransport(app=app) + with caplog.at_level(logging.ERROR, logger="users.middleware"): + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard") + + assert resp.status_code == 200 + assert resp.json()["user"]["email"] == "fallback@example.com" + assert any("resolver" in rec.message.lower() for rec in caplog.records) + + +@pytest.mark.anyio +async def test_api_path_unauthenticated_returns_401_json(db_state): + """Unauthenticated /api/private should return 401 JSON, not a 302 redirect.""" + app = await _build_app(db_state) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/api/private-thing", follow_redirects=False) + + assert resp.status_code == 401 + assert resp.headers["content-type"].startswith("application/json") + assert resp.json() == {"detail": "Not authenticated"} + + +@pytest.mark.anyio +async def test_view_path_unauthenticated_still_redirects(db_state): + """View routes (non-/api/*) keep the existing 302-to-login behavior.""" + app = await _build_app(db_state) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard", follow_redirects=False) + + assert resp.status_code == 302 + assert resp.headers["location"] == "/users/login" + + +@pytest.mark.anyio +async def test_session_wins_over_resolver(db_state, mw_active_user): + """A valid session cookie short-circuits — the resolver chain is not consulted.""" + resolver_called = False + + async def should_not_run(request): + nonlocal resolver_called + resolver_called = True + return _ctx(email="should-not-win@example.com") + + app = await _build_app(db_state, principal_resolvers=[should_not_run]) + transport = httpx.ASGITransport(app=app) + cookies = _session_cookie({"user_id": str(mw_active_user.id)}) + async with httpx.AsyncClient( + transport=transport, base_url="http://testserver", cookies=cookies + ) as client: + resp = await client.get("/dashboard") + + assert resp.status_code == 200 + assert resp.json()["user"]["email"] == "middleware-test@example.com" + assert resolver_called is False + + +@pytest.mark.anyio +async def test_resolver_does_not_write_session(db_state): + """Resolver-authenticated requests must not persist anything to the session.""" + captured = {} + + async def capture(request: Request): + captured["session"] = dict(request.session) + user = getattr(request.state, "user", None) + return JSONResponse({"authenticated": user is not None}) + + async def stub_resolver(request): + return _ctx() + + app = await _build_app(db_state, capture, principal_resolvers=[stub_resolver]) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard") + + assert resp.status_code == 200 + assert resp.json() == {"authenticated": True} + # Session must not contain a user_id, user_ctx, or anything resolver-added. + assert "user_id" not in captured["session"] + assert "user_ctx" not in captured["session"] diff --git a/modules/users/users/middleware.py b/modules/users/users/middleware.py index 2f0d2ee4..8e05ee93 100644 --- a/modules/users/users/middleware.py +++ b/modules/users/users/middleware.py @@ -24,7 +24,7 @@ from sqlalchemy import select from sqlalchemy.orm import selectinload from starlette.requests import Request -from starlette.responses import RedirectResponse +from starlette.responses import JSONResponse, RedirectResponse from starlette.types import ASGIApp, Receive, Scope, Send from users.constants import SESSION_USER_ID_KEY @@ -102,10 +102,35 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: else: session[SESSION_USER_CTX_KEY] = user_ctx.to_session_dict() + # Fall-through: registered principal resolvers (PAT, API key, ...). + # The session-cookie path above is authoritative; resolvers only run + # when no session-authenticated user was resolved. + if user_ctx is None: + auth_state = getattr(scope["app"].state, "auth", None) + resolvers = getattr(auth_state, "principal_resolvers", ()) if auth_state else () + if resolvers: + request = Request(scope) + for resolver in resolvers: + try: + user_ctx = await resolver(request) + except Exception: + logger.exception( + "Principal resolver %r raised; treating as no-match", + resolver, + ) + continue + if user_ctx is not None: + break + if user_ctx is None and not is_public: - request = Request(scope) - session[_SESSION_NEXT_KEY] = str(request.url) - response = RedirectResponse(_LOGIN_REDIRECT, status_code=302) + if path.startswith("/api/"): + response = JSONResponse( + {"detail": "Not authenticated"}, status_code=401 + ) + else: + request = Request(scope) + session[_SESSION_NEXT_KEY] = str(request.url) + response = RedirectResponse(_LOGIN_REDIRECT, status_code=302) await response(scope, receive, send) return From 225369e859cd44ac8d2f2dfef2a411ac1e3a210d Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 21 May 2026 23:19:04 +0200 Subject: [PATCH 10/16] test(users): split resolver-chain tests into sibling file to honour 300-line cap --- modules/users/tests/test_users_middleware.py | 177 ---------------- .../tests/test_users_middleware_resolvers.py | 190 ++++++++++++++++++ 2 files changed, 190 insertions(+), 177 deletions(-) create mode 100644 modules/users/tests/test_users_middleware_resolvers.py diff --git a/modules/users/tests/test_users_middleware.py b/modules/users/tests/test_users_middleware.py index ec6ebd0a..43b77c6d 100644 --- a/modules/users/tests/test_users_middleware.py +++ b/modules/users/tests/test_users_middleware.py @@ -217,180 +217,3 @@ async def _capture_contextvar(request: Request): # After the request completes, the ContextVar should be reset to its # default (no value set in this outer scope). assert current_user_id.get(None) is None - - -# --------------------------------------------------------------------------- -# Principal-resolver chain -# --------------------------------------------------------------------------- - - -def _ctx(uid: str = "11111111-1111-1111-1111-111111111111", **overrides): - """Build a UserContext for resolver tests.""" - from auth.contracts.schemas import UserContext - - fields = dict( - id=uid, - email="pat@example.com", - name="PAT User", - roles=["user"], - tenant_id=None, - ) - fields.update(overrides) - return UserContext(**fields) - - -@pytest.mark.anyio -async def test_resolver_returning_context_authenticates_request(db_state): - """A registered resolver that returns a UserContext authenticates the request.""" - - async def stub_resolver(request): - return _ctx() - - app = await _build_app(db_state, principal_resolvers=[stub_resolver]) - transport = httpx.ASGITransport(app=app) - async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: - resp = await client.get("/dashboard") - - assert resp.status_code == 200 - data = resp.json() - assert data["user"]["email"] == "pat@example.com" - - -@pytest.mark.anyio -async def test_resolver_first_non_none_wins(db_state): - """The first resolver returning a context wins; later resolvers are not consulted.""" - second_called = False - - async def first_none(request): - return None - - async def second_returns(request): - nonlocal second_called - second_called = True - return _ctx(email="second@example.com") - - async def third_should_not_run(request): - raise AssertionError("third resolver should not run after a match") - - app = await _build_app( - db_state, - principal_resolvers=[first_none, second_returns, third_should_not_run], - ) - transport = httpx.ASGITransport(app=app) - async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: - resp = await client.get("/dashboard") - - assert resp.status_code == 200 - assert resp.json()["user"]["email"] == "second@example.com" - assert second_called - - -@pytest.mark.anyio -async def test_all_resolvers_return_none_falls_through_to_redirect(db_state): - """When every resolver returns None for a view route → 302 to /users/login.""" - - async def none_resolver(request): - return None - - app = await _build_app(db_state, principal_resolvers=[none_resolver]) - transport = httpx.ASGITransport(app=app) - async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: - resp = await client.get("/dashboard", follow_redirects=False) - - assert resp.status_code == 302 - assert resp.headers["location"] == "/users/login" - - -@pytest.mark.anyio -async def test_resolver_raising_does_not_crash_middleware(db_state, caplog): - """A resolver that raises is logged and the chain continues to the next.""" - import logging - - async def boom(request): - raise RuntimeError("resolver kaboom") - - async def fallback(request): - return _ctx(email="fallback@example.com") - - app = await _build_app(db_state, principal_resolvers=[boom, fallback]) - transport = httpx.ASGITransport(app=app) - with caplog.at_level(logging.ERROR, logger="users.middleware"): - async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: - resp = await client.get("/dashboard") - - assert resp.status_code == 200 - assert resp.json()["user"]["email"] == "fallback@example.com" - assert any("resolver" in rec.message.lower() for rec in caplog.records) - - -@pytest.mark.anyio -async def test_api_path_unauthenticated_returns_401_json(db_state): - """Unauthenticated /api/private should return 401 JSON, not a 302 redirect.""" - app = await _build_app(db_state) - transport = httpx.ASGITransport(app=app) - async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: - resp = await client.get("/api/private-thing", follow_redirects=False) - - assert resp.status_code == 401 - assert resp.headers["content-type"].startswith("application/json") - assert resp.json() == {"detail": "Not authenticated"} - - -@pytest.mark.anyio -async def test_view_path_unauthenticated_still_redirects(db_state): - """View routes (non-/api/*) keep the existing 302-to-login behavior.""" - app = await _build_app(db_state) - transport = httpx.ASGITransport(app=app) - async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: - resp = await client.get("/dashboard", follow_redirects=False) - - assert resp.status_code == 302 - assert resp.headers["location"] == "/users/login" - - -@pytest.mark.anyio -async def test_session_wins_over_resolver(db_state, mw_active_user): - """A valid session cookie short-circuits — the resolver chain is not consulted.""" - resolver_called = False - - async def should_not_run(request): - nonlocal resolver_called - resolver_called = True - return _ctx(email="should-not-win@example.com") - - app = await _build_app(db_state, principal_resolvers=[should_not_run]) - transport = httpx.ASGITransport(app=app) - cookies = _session_cookie({"user_id": str(mw_active_user.id)}) - async with httpx.AsyncClient( - transport=transport, base_url="http://testserver", cookies=cookies - ) as client: - resp = await client.get("/dashboard") - - assert resp.status_code == 200 - assert resp.json()["user"]["email"] == "middleware-test@example.com" - assert resolver_called is False - - -@pytest.mark.anyio -async def test_resolver_does_not_write_session(db_state): - """Resolver-authenticated requests must not persist anything to the session.""" - captured = {} - - async def capture(request: Request): - captured["session"] = dict(request.session) - user = getattr(request.state, "user", None) - return JSONResponse({"authenticated": user is not None}) - - async def stub_resolver(request): - return _ctx() - - app = await _build_app(db_state, capture, principal_resolvers=[stub_resolver]) - transport = httpx.ASGITransport(app=app) - async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: - resp = await client.get("/dashboard") - - assert resp.status_code == 200 - assert resp.json() == {"authenticated": True} - # Session must not contain a user_id, user_ctx, or anything resolver-added. - assert "user_id" not in captured["session"] - assert "user_ctx" not in captured["session"] diff --git a/modules/users/tests/test_users_middleware_resolvers.py b/modules/users/tests/test_users_middleware_resolvers.py new file mode 100644 index 00000000..ef3f8d26 --- /dev/null +++ b/modules/users/tests/test_users_middleware_resolvers.py @@ -0,0 +1,190 @@ +"""AuthMiddleware tests for the principal-resolver chain. + +Covers ``app.state.auth.principal_resolvers`` consultation order, error +isolation, the session-short-circuit precedence, and the /api/* vs view-path +unauthenticated split (401 JSON vs 302 redirect). + +Helpers + fixtures live in ``_middleware_support`` alongside the broader +middleware tests in ``test_users_middleware`` and the PUBLIC_PATHS suite in +``test_users_middleware_public_paths``. +""" + +from __future__ import annotations + +import httpx +import pytest +from _middleware_support import _build_app, _session_cookie +from fastapi import Request +from starlette.responses import JSONResponse + + +def _ctx(uid: str = "11111111-1111-1111-1111-111111111111", **overrides): + """Build a UserContext for resolver tests.""" + from auth.contracts.schemas import UserContext + + fields = dict( + id=uid, + email="pat@example.com", + name="PAT User", + roles=["user"], + tenant_id=None, + ) + fields.update(overrides) + return UserContext(**fields) + + +@pytest.mark.anyio +async def test_resolver_returning_context_authenticates_request(db_state): + """A registered resolver that returns a UserContext authenticates the request.""" + + async def stub_resolver(request): + return _ctx() + + app = await _build_app(db_state, principal_resolvers=[stub_resolver]) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard") + + assert resp.status_code == 200 + data = resp.json() + assert data["user"]["email"] == "pat@example.com" + + +@pytest.mark.anyio +async def test_resolver_first_non_none_wins(db_state): + """The first resolver returning a context wins; later resolvers are not consulted.""" + second_called = False + + async def first_none(request): + return None + + async def second_returns(request): + nonlocal second_called + second_called = True + return _ctx(email="second@example.com") + + async def third_should_not_run(request): + raise AssertionError("third resolver should not run after a match") + + app = await _build_app( + db_state, + principal_resolvers=[first_none, second_returns, third_should_not_run], + ) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard") + + assert resp.status_code == 200 + assert resp.json()["user"]["email"] == "second@example.com" + assert second_called + + +@pytest.mark.anyio +async def test_all_resolvers_return_none_falls_through_to_redirect(db_state): + """When every resolver returns None for a view route → 302 to /users/login.""" + + async def none_resolver(request): + return None + + app = await _build_app(db_state, principal_resolvers=[none_resolver]) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard", follow_redirects=False) + + assert resp.status_code == 302 + assert resp.headers["location"] == "/users/login" + + +@pytest.mark.anyio +async def test_resolver_raising_does_not_crash_middleware(db_state, caplog): + """A resolver that raises is logged and the chain continues to the next.""" + import logging + + async def boom(request): + raise RuntimeError("resolver kaboom") + + async def fallback(request): + return _ctx(email="fallback@example.com") + + app = await _build_app(db_state, principal_resolvers=[boom, fallback]) + transport = httpx.ASGITransport(app=app) + with caplog.at_level(logging.ERROR, logger="users.middleware"): + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard") + + assert resp.status_code == 200 + assert resp.json()["user"]["email"] == "fallback@example.com" + assert any("resolver" in rec.message.lower() for rec in caplog.records) + + +@pytest.mark.anyio +async def test_api_path_unauthenticated_returns_401_json(db_state): + """Unauthenticated /api/private should return 401 JSON, not a 302 redirect.""" + app = await _build_app(db_state) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/api/private-thing", follow_redirects=False) + + assert resp.status_code == 401 + assert resp.headers["content-type"].startswith("application/json") + assert resp.json() == {"detail": "Not authenticated"} + + +@pytest.mark.anyio +async def test_view_path_unauthenticated_still_redirects(db_state): + """View routes (non-/api/*) keep the existing 302-to-login behavior.""" + app = await _build_app(db_state) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard", follow_redirects=False) + + assert resp.status_code == 302 + assert resp.headers["location"] == "/users/login" + + +@pytest.mark.anyio +async def test_session_wins_over_resolver(db_state, mw_active_user): + """A valid session cookie short-circuits — the resolver chain is not consulted.""" + resolver_called = False + + async def should_not_run(request): + nonlocal resolver_called + resolver_called = True + return _ctx(email="should-not-win@example.com") + + app = await _build_app(db_state, principal_resolvers=[should_not_run]) + transport = httpx.ASGITransport(app=app) + cookies = _session_cookie({"user_id": str(mw_active_user.id)}) + async with httpx.AsyncClient( + transport=transport, base_url="http://testserver", cookies=cookies + ) as client: + resp = await client.get("/dashboard") + + assert resp.status_code == 200 + assert resp.json()["user"]["email"] == "middleware-test@example.com" + assert resolver_called is False + + +@pytest.mark.anyio +async def test_resolver_does_not_write_session(db_state): + """Resolver-authenticated requests must not persist anything to the session.""" + captured = {} + + async def capture(request: Request): + captured["session"] = dict(request.session) + user = getattr(request.state, "user", None) + return JSONResponse({"authenticated": user is not None}) + + async def stub_resolver(request): + return _ctx() + + app = await _build_app(db_state, capture, principal_resolvers=[stub_resolver]) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + resp = await client.get("/dashboard") + + assert resp.status_code == 200 + assert resp.json() == {"authenticated": True} + # Session must not contain a user_id, user_ctx, or anything resolver-added. + assert "user_id" not in captured["session"] + assert "user_ctx" not in captured["session"] From 74aee1a099e79a29bdfcb6e85e37bad1c866b4ec Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 21 May 2026 23:25:40 +0200 Subject: [PATCH 11/16] test: end-to-end integration test for principal-resolver chain --- tests/test_principal_resolver_integration.py | 98 ++++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 tests/test_principal_resolver_integration.py diff --git a/tests/test_principal_resolver_integration.py b/tests/test_principal_resolver_integration.py new file mode 100644 index 00000000..34110922 --- /dev/null +++ b/tests/test_principal_resolver_integration.py @@ -0,0 +1,98 @@ +"""End-to-end test: a fake bearer-token resolver authenticates against the full app stack.""" + +from __future__ import annotations + +from collections.abc import AsyncGenerator + +import httpx +import pytest +from auth.contracts.schemas import UserContext + + +@pytest.fixture +async def app_with_pat_resolver(app): + """Reuses the standard ``app`` fixture and appends a fake bearer-token resolver. + + The resolver recognizes a single hardcoded token ``"good"`` mapped to a + deterministic UserContext. ``"bad"`` (or absent header) returns None. + """ + + async def fake_pat_resolver(request) -> UserContext | None: + header = request.headers.get("Authorization", "") + if not header.startswith("Bearer "): + return None + token = header.removeprefix("Bearer ") + if token != "good": + return None + return UserContext( + id="22222222-2222-2222-2222-222222222222", + email="pat-user@example.com", + name="PAT User", + roles=["admin"], + tenant_id=None, + ) + + app.state.auth.principal_resolvers.append(fake_pat_resolver) + yield app + app.state.auth.principal_resolvers.remove(fake_pat_resolver) + + +@pytest.fixture +async def pat_client(app_with_pat_resolver) -> AsyncGenerator[httpx.AsyncClient, None]: + transport = httpx.ASGITransport(app=app_with_pat_resolver) + async with httpx.AsyncClient( + transport=transport, base_url="http://testserver" + ) as c: + yield c + + +@pytest.mark.anyio +async def test_bearer_token_authenticates_against_protected_view(pat_client): + """Valid bearer token -> 200 on a protected view path (users admin).""" + resp = await pat_client.get( + "/users/admin", + headers={"Authorization": "Bearer good"}, + follow_redirects=False, + ) + # /users/admin is a view route; with a valid resolver the request gets + # through AuthMiddleware (200) instead of redirecting to /users/login. + assert resp.status_code == 200 + + +@pytest.mark.anyio +async def test_invalid_bearer_token_returns_401_on_api_path(pat_client): + """Bad bearer on an /api/* path -> 401 JSON, not a redirect.""" + resp = await pat_client.get( + "/api/users/admin", + headers={"Authorization": "Bearer bad"}, + follow_redirects=False, + ) + assert resp.status_code == 401 + assert resp.json() == {"detail": "Not authenticated"} + + +@pytest.mark.anyio +async def test_no_auth_header_on_api_returns_401(pat_client): + """No Authorization header on a private /api/* path -> 401 JSON.""" + resp = await pat_client.get( + "/api/users/admin", follow_redirects=False + ) + assert resp.status_code == 401 + assert resp.json() == {"detail": "Not authenticated"} + + +@pytest.mark.anyio +async def test_session_wins_over_bad_bearer(authenticated_client): + """A valid session cookie + Bearer bad -> 200 via session; resolver not consulted. + + The ``authenticated_client`` fixture already carries an admin session cookie; + here we additionally send a bad bearer to prove the session path wins. + + Targets ``/api/permissions/`` (admin-readable, doesn't enumerate users) to + avoid the unrelated email-validation issue in ``/api/users/admin`` triggered + by the fixture's ``admin@test`` seeded email.""" + resp = await authenticated_client.get( + "/api/permissions/", + headers={"Authorization": "Bearer bad"}, + ) + assert resp.status_code == 200 From afc1f05f20594730ace9181d9ab31aed9f644f92 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 21 May 2026 23:29:43 +0200 Subject: [PATCH 12/16] docs(framework): add principal-resolver chain reference --- docs/framework/principal-resolvers.md | 120 ++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 docs/framework/principal-resolvers.md diff --git a/docs/framework/principal-resolvers.md b/docs/framework/principal-resolvers.md new file mode 100644 index 00000000..f45e8980 --- /dev/null +++ b/docs/framework/principal-resolvers.md @@ -0,0 +1,120 @@ +# Principal-resolver chain + +The `auth` module exposes an extension point — a list of async resolvers on +`app.state.auth.principal_resolvers` — that lets downstream modules plug in +additional credential sources alongside the built-in session cookie. This is +the supported way to add Personal Access Tokens, API keys, JWT bearers, or +any other request-scoped authentication scheme without forking +`users.AuthMiddleware`. + +## The contract + +```python +from collections.abc import Awaitable, Callable +from starlette.requests import Request +from auth import PrincipalResolver, UserContext + +PrincipalResolver = Callable[[Request], Awaitable[UserContext | None]] +``` + +A resolver MUST: + +- Be **async** (it is awaited by the middleware). +- **Bail fast** when its credential type isn't present (e.g., return `None` + immediately if there is no `Authorization` header) — resolvers run on + every request, including completely unauthenticated ones. +- **Self-check active / disabled state** before returning a `UserContext`. + The middleware does not re-validate. +- **Never raise** on bad credentials — return `None` so the chain continues + to the next resolver. The middleware swallows exceptions defensively but + resolver authors should not rely on it. +- **Not write to the session.** Resolver-authenticated requests are + per-request only; they never silently elevate into a long-lived session + cookie. (To mint a session, use the standard login flow.) + +## Resolution order + +`users.AuthMiddleware` consults credential sources in this order: + +1. **Session cookie** — the existing fast/cached path. If the session + carries a valid `user_id` (and matching cached `user_ctx`), the user is + authenticated and the resolver chain is **not** consulted. +2. **Registered resolvers**, in registration order. The first non-`None` + return wins. +3. **Unauthenticated** — for `/api/*` paths the middleware returns + `401 {"detail": "Not authenticated"}`; for view paths it 302-redirects to + `/users/login` and stashes the original URL in `session["next"]`. + +## Worked example — bearer-token resolver + +A module that ships its own Personal-Access-Token table registers a +resolver from its `on_startup` hook: + +```python +# modules/example/example/module.py +from __future__ import annotations + +from typing import TYPE_CHECKING + +from auth import PrincipalResolver, UserContext +from simple_module_core.module import ModuleBase, ModuleMeta +from starlette.requests import Request + +if TYPE_CHECKING: + from fastapi import FastAPI + + +class ExampleModule(ModuleBase): + meta = ModuleMeta(name="Example", depends_on=["Auth", "Users"]) + + async def on_startup(self, app: FastAPI) -> None: + app.state.auth.principal_resolvers.append(self._build_pat_resolver(app)) + + @staticmethod + def _build_pat_resolver(app: FastAPI) -> PrincipalResolver: + async def resolve_pat(request: Request) -> UserContext | None: + header = request.headers.get("Authorization", "") + if not header.startswith("Bearer "): + return None + token = header.removeprefix("Bearer ") + + # Look up the token in the module's own storage and load the user. + async with app.state.sm.db.session_factory() as session: + record = await find_active_token(session, token) + if record is None: + return None + user = await load_user_with_roles(session, record.user_id) + if user is None or not user.is_active or user.disabled_at is not None: + return None + return UserContext.from_user(user) + + return resolve_pat +``` + +`depends_on=["Auth", "Users"]` ensures `AuthModule.register_settings` has +run (so `app.state.auth` exists) and `UsersModule.register_middleware` has +installed `AuthMiddleware` (which calls the resolvers). + +## When NOT to write a resolver + +- **You want to mint a long-lived session.** Use the standard login flow + (`/users/login` or OAuth). Resolvers are explicitly forbidden from + writing the session. +- **You only need a per-endpoint API-key check.** A FastAPI dependency + (`require_api_key`) on the route signature is simpler and keeps the + authenticated-user shape clean. +- **You want to override the `users` module's behavior** (e.g., reject + active users, change role semantics). Resolvers add credential sources; + they don't change the rules of authentication. For that, swap + `UsersModule`/`AuthMiddleware` outright. + +## Testing your resolver + +Write resolver tests against a minimal app (see +`modules/users/tests/_middleware_support.py::_build_app` for the pattern +used by the framework's own resolver suite — it takes a +`principal_resolvers=` keyword and seeds `app.state.auth` for you). + +End-to-end tests should drive the full `create_app(settings)` stack and +append your resolver to `app.state.auth.principal_resolvers` in a fixture — +see `tests/test_principal_resolver_integration.py` for a worked example. From 9bb1dae24ce4acb257c269c9e615b6f61e67cacd Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 21 May 2026 23:30:00 +0200 Subject: [PATCH 13/16] docs: link framework-conventions to the principal-resolver reference --- docs/framework-conventions.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/framework-conventions.md b/docs/framework-conventions.md index fbdbe3f7..99f24cb8 100644 --- a/docs/framework-conventions.md +++ b/docs/framework-conventions.md @@ -237,6 +237,16 @@ async def create_order(...): ... `DEFAULT_ROLE_PERMISSIONS` in `simple_module_hosting.permissions` ships only `admin: ["*"]`. Host apps configure their own role → permission map; the framework does not know about plugin permission strings. +## Authentication extension points + +The `auth` module exposes a principal-resolver chain on +`app.state.auth.principal_resolvers` — a list of async callables that +`users.AuthMiddleware` consults after the session-cookie path. Use it to add +non-cookie credential sources (Personal Access Tokens, API keys, JWTs) +without forking the middleware. See +[`docs/framework/principal-resolvers.md`](framework/principal-resolvers.md) +for the contract, ordering rules, and a worked Bearer-token example. + ## Events Base class: `Event` from `simple_module_core.events`. Subclass per domain event: From 73b7fb295104c1130c91dec54aa4568a8805ab59 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 21 May 2026 23:33:42 +0200 Subject: [PATCH 14/16] style: apply ruff format to middleware + resolver integration test --- modules/users/users/middleware.py | 4 +--- tests/test_principal_resolver_integration.py | 8 ++------ 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/modules/users/users/middleware.py b/modules/users/users/middleware.py index 8e05ee93..540ed9e9 100644 --- a/modules/users/users/middleware.py +++ b/modules/users/users/middleware.py @@ -124,9 +124,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: if user_ctx is None and not is_public: if path.startswith("/api/"): - response = JSONResponse( - {"detail": "Not authenticated"}, status_code=401 - ) + response = JSONResponse({"detail": "Not authenticated"}, status_code=401) else: request = Request(scope) session[_SESSION_NEXT_KEY] = str(request.url) diff --git a/tests/test_principal_resolver_integration.py b/tests/test_principal_resolver_integration.py index 34110922..69ca3b20 100644 --- a/tests/test_principal_resolver_integration.py +++ b/tests/test_principal_resolver_integration.py @@ -40,9 +40,7 @@ async def fake_pat_resolver(request) -> UserContext | None: @pytest.fixture async def pat_client(app_with_pat_resolver) -> AsyncGenerator[httpx.AsyncClient, None]: transport = httpx.ASGITransport(app=app_with_pat_resolver) - async with httpx.AsyncClient( - transport=transport, base_url="http://testserver" - ) as c: + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as c: yield c @@ -74,9 +72,7 @@ async def test_invalid_bearer_token_returns_401_on_api_path(pat_client): @pytest.mark.anyio async def test_no_auth_header_on_api_returns_401(pat_client): """No Authorization header on a private /api/* path -> 401 JSON.""" - resp = await pat_client.get( - "/api/users/admin", follow_redirects=False - ) + resp = await pat_client.get("/api/users/admin", follow_redirects=False) assert resp.status_code == 401 assert resp.json() == {"detail": "Not authenticated"} From d21e59ee3a73b63fb3657b94da103ac8f53d0efa Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 21 May 2026 23:34:14 +0200 Subject: [PATCH 15/16] chore(tests): use dict-literal + sort imports to satisfy ruff (C408, I001) --- modules/auth/tests/test_resolver_registry.py | 3 +-- .../users/tests/test_users_middleware_resolvers.py | 14 +++++++------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/modules/auth/tests/test_resolver_registry.py b/modules/auth/tests/test_resolver_registry.py index 4b615fc8..86e71af9 100644 --- a/modules/auth/tests/test_resolver_registry.py +++ b/modules/auth/tests/test_resolver_registry.py @@ -53,10 +53,9 @@ async def resolver(request): # pragma: no cover - registration smoke only def test_auth_module_register_settings_populates_app_state(): """``AuthModule.register_settings(app)`` must put an AuthState on ``app.state.auth``.""" - from fastapi import FastAPI - from auth.module import AuthModule from auth.state import AuthState + from fastapi import FastAPI app = FastAPI() AuthModule().register_settings(app) diff --git a/modules/users/tests/test_users_middleware_resolvers.py b/modules/users/tests/test_users_middleware_resolvers.py index ef3f8d26..d45d8f25 100644 --- a/modules/users/tests/test_users_middleware_resolvers.py +++ b/modules/users/tests/test_users_middleware_resolvers.py @@ -22,13 +22,13 @@ def _ctx(uid: str = "11111111-1111-1111-1111-111111111111", **overrides): """Build a UserContext for resolver tests.""" from auth.contracts.schemas import UserContext - fields = dict( - id=uid, - email="pat@example.com", - name="PAT User", - roles=["user"], - tenant_id=None, - ) + fields = { + "id": uid, + "email": "pat@example.com", + "name": "PAT User", + "roles": ["user"], + "tenant_id": None, + } fields.update(overrides) return UserContext(**fields) From a00cd183b4541d4c959fa16b110d8d2f3974fa0a Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Fri, 22 May 2026 00:19:30 +0200 Subject: [PATCH 16/16] refactor(auth): apply xhigh code-review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - _middleware_support: drop hand-rolled _sign_session, use framework's simple_module_test.forge_session_cookie (eliminates duplication). - integration test: import UserContext from `auth` (the documented public re-export) so the symbol's public-surface contract is exercised by an actual test. - middleware: extract _API_PATH_PREFIX and _UNAUTH_DETAIL constants alongside _LOGIN_REDIRECT — single-source the wire format. - principal-resolvers doc: document that resolvers run on public paths too (telemetry use case), plus add per-request DB-cost caveat with LRU caching guidance for high-traffic deployments. - test docstring: drop drive-by reference to an unrelated fixture bug. --- docs/framework/principal-resolvers.md | 21 ++++++++++++++++++++ modules/users/tests/_middleware_support.py | 12 ++--------- modules/users/users/middleware.py | 6 ++++-- tests/test_principal_resolver_integration.py | 7 ++----- 4 files changed, 29 insertions(+), 17 deletions(-) diff --git a/docs/framework/principal-resolvers.md b/docs/framework/principal-resolvers.md index f45e8980..ba380554 100644 --- a/docs/framework/principal-resolvers.md +++ b/docs/framework/principal-resolvers.md @@ -45,6 +45,13 @@ A resolver MUST: `401 {"detail": "Not authenticated"}`; for view paths it 302-redirects to `/users/login` and stashes the original URL in `session["next"]`. +The chain runs on **every** request, including public paths (`/health`, +`/openapi.json`, the login page itself). That lets a resolver attach +`request.state.user` for telemetry even on unauthenticated routes. The +unauthenticated response is suppressed for public paths regardless of +resolver outcome — the chain populates the principal, public-path +allow-listing controls the response. + ## Worked example — bearer-token resolver A module that ships its own Personal-Access-Token table registers a @@ -95,6 +102,20 @@ class ExampleModule(ModuleBase): run (so `app.state.auth` exists) and `UsersModule.register_middleware` has installed `AuthMiddleware` (which calls the resolvers). +### Performance — caching the token lookup + +The middleware does not cache resolver results (there's nothing safe to key +on — credentials must be re-validated every request to honor revocation). +For each authenticated request, the resolver opens a fresh DB session +and queries the token store. That's one round-trip per request. + +For high-traffic deployments, cache the token-to-user mapping *inside* +the resolver — typically an LRU keyed by the token (or its hash) with a +short TTL. The resolver still runs every request, but the DB lookup is +skipped on cache hits. Pick a TTL short enough that revocation latency +stays acceptable. The framework deliberately stays out of this — caching +policy is a per-module concern. + ## When NOT to write a resolver - **You want to mint a long-lived session.** Use the standard login flow diff --git a/modules/users/tests/_middleware_support.py b/modules/users/tests/_middleware_support.py index fcd53ced..e4454f41 100644 --- a/modules/users/tests/_middleware_support.py +++ b/modules/users/tests/_middleware_support.py @@ -9,15 +9,13 @@ from __future__ import annotations -import json import uuid -from base64 import b64encode from types import SimpleNamespace from typing import Any import pytest from fastapi import FastAPI, Request -from itsdangerous import TimestampSigner +from simple_module_test import forge_session_cookie from starlette.middleware.sessions import SessionMiddleware from starlette.responses import JSONResponse from users.constants import ADMIN_ROLE_ID, USER_ROLE_ID @@ -26,14 +24,8 @@ SECRET_KEY = "test-secret-key-for-session-middleware" -def _sign_session(data: dict[str, Any], secret: str = SECRET_KEY) -> str: - """Encode and sign a session dict exactly as Starlette's SessionMiddleware does.""" - raw = b64encode(json.dumps(data).encode()).decode() - return TimestampSigner(secret).sign(raw).decode("utf-8") - - def _session_cookie(data: dict[str, Any]) -> dict[str, str]: - return {"session": _sign_session(data)} + return {"session": forge_session_cookie(SECRET_KEY, data)} async def _build_app(db_state, inner_handler=None, *, principal_resolvers=None): diff --git a/modules/users/users/middleware.py b/modules/users/users/middleware.py index 540ed9e9..f78d01e7 100644 --- a/modules/users/users/middleware.py +++ b/modules/users/users/middleware.py @@ -37,6 +37,8 @@ _SESSION_NEXT_KEY = "next" _SCOPE_HTTP = "http" _LOGIN_REDIRECT = "/users/login" +_API_PATH_PREFIX = "/api/" +_UNAUTH_DETAIL = "Not authenticated" # Paths that don't require authentication. PUBLIC_PATHS = ( @@ -123,8 +125,8 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: break if user_ctx is None and not is_public: - if path.startswith("/api/"): - response = JSONResponse({"detail": "Not authenticated"}, status_code=401) + if path.startswith(_API_PATH_PREFIX): + response = JSONResponse({"detail": _UNAUTH_DETAIL}, status_code=401) else: request = Request(scope) session[_SESSION_NEXT_KEY] = str(request.url) diff --git a/tests/test_principal_resolver_integration.py b/tests/test_principal_resolver_integration.py index 69ca3b20..38ab337d 100644 --- a/tests/test_principal_resolver_integration.py +++ b/tests/test_principal_resolver_integration.py @@ -6,7 +6,7 @@ import httpx import pytest -from auth.contracts.schemas import UserContext +from auth import UserContext # exercises the documented public re-export @pytest.fixture @@ -83,10 +83,7 @@ async def test_session_wins_over_bad_bearer(authenticated_client): The ``authenticated_client`` fixture already carries an admin session cookie; here we additionally send a bad bearer to prove the session path wins. - - Targets ``/api/permissions/`` (admin-readable, doesn't enumerate users) to - avoid the unrelated email-validation issue in ``/api/users/admin`` triggered - by the fixture's ``admin@test`` seeded email.""" + Endpoint is any admin-readable, non-user-enumerating route.""" resp = await authenticated_client.get( "/api/permissions/", headers={"Authorization": "Bearer bad"},