diff --git a/.env.example b/.env.example index 965022a9..c1472369 100644 --- a/.env.example +++ b/.env.example @@ -21,6 +21,11 @@ SM_SECRET_KEY=change-me-in-production # Dev-only: Vite asset URL (ignored in production builds) SM_VITE_DEV_URL=http://localhost:5050 +# Host-level anonymous-access path prefixes (JSON array). Escape hatch for +# exposing a route without a session when no module owns it. Modules should +# prefer the method-aware `register_public_routes` hook instead. +# SM_AUTH_PUBLIC_PATHS=["/api/integrations/webhook", "/status"] + # First-boot admin seed (optional). Only applied when the users table is empty. # Leave unset and use `uv run sm-users create-admin` instead if you prefer. # SM_USERS_BOOTSTRAP_EMAIL=admin@example.com diff --git a/.verify/01-public-landing-anonymous.png b/.verify/01-public-landing-anonymous.png new file mode 100644 index 00000000..e22ec333 Binary files /dev/null and b/.verify/01-public-landing-anonymous.png differ diff --git a/.verify/02-protected-redirects-to-login.png b/.verify/02-protected-redirects-to-login.png new file mode 100644 index 00000000..78e4dbfd Binary files /dev/null and b/.verify/02-protected-redirects-to-login.png differ diff --git a/.verify/03-dashboard-authenticated.png b/.verify/03-dashboard-authenticated.png new file mode 100644 index 00000000..fcabe7c4 Binary files /dev/null and b/.verify/03-dashboard-authenticated.png differ diff --git a/.verify/04-public-route-mechanism.png b/.verify/04-public-route-mechanism.png new file mode 100644 index 00000000..3df4c3ea Binary files /dev/null and b/.verify/04-public-route-mechanism.png differ diff --git a/.verify/qa-report.md b/.verify/qa-report.md new file mode 100644 index 00000000..a2ba2b31 --- /dev/null +++ b/.verify/qa-report.md @@ -0,0 +1,54 @@ +# QA Report: Auth-gating (issue #191 public-routes extension point) + +**Date:** 2026-06-05 +**Tester:** Claude QA (Senior) +**Target:** http://localhost:8000 (API) + Vite :5050 +**Depth:** scoped (backend middleware change — no new UI) +**Iteration:** 1 of 3 + +## Scope rationale + +Issue #191 adds a method-aware public-route extension point to `AuthMiddleware`. +It is a **backend change with no new UI**. The risk surface is therefore not a +page to fuzz, but **which routes the middleware gates vs. lets through +anonymously**. This QA validates exactly that, in a real browser + at the HTTP +layer, including a live end-to-end exercise of the new mechanism. + +## Summary +| Category | Passed | Failed | Skipped | +|----------|--------|--------|---------| +| Public routes (anonymous load) | 3 | 0 | 0 | +| Protected-route gating | 2 | 0 | 0 | +| Login + authenticated access | 2 | 0 | 0 | +| New public-route mechanism (live) | 2 | 0 | 0 | +| **Total** | **9** | **0** | **0** | + +## Critical / Major / Minor Issues +None. + +## Observations (P3 — not bugs) +- **OBS-001** `GET /favicon.ico → 404` appears in the console on every page. + Cosmetic, pre-existing, unrelated to #191 (no favicon shipped by the host). +- **OBS-002** Dev-workspace has both `users` and `keycloak` installed as entry + points, so a bare boot trips `SM020` (multiple auth providers). Pre-existing, + unrelated to #191; worked around by `SM_MODULES_ENABLED` excluding Keycloak + (the same approach the existing test suite uses). + +## Passed Tests +| # | Scenario | Result | Evidence | +|---|----------|--------|----------| +| TEST-001 | Public `/` loads anonymously | PASS | 00-landing-anonymous.png | +| TEST-002 | Public `/users/login` loads anonymously | PASS | 01-protected-redirects-to-login.png | +| TEST-003 | `/health` 200; `/api/users/auth/` 404 (passed gating) | PASS | curl smoke | +| TEST-004 | Protected `/dashboard` → 302 → login (anonymous) | PASS | 01-protected-redirects-to-login.png | +| TEST-005 | Protected API `/api/users/admin` → 401 (no redirect) | PASS | curl smoke | +| TEST-006 | Login as admin establishes session | PASS | 02-dashboard-authenticated.png | +| TEST-007 | Authenticated `/dashboard` renders | PASS | 02-dashboard-authenticated.png | +| TEST-008 | `SM_AUTH_PUBLIC_PATHS` flips gating live: 302 → 404 | PASS | 03-public-route-mechanism-404-not-redirect.png | +| TEST-009 | Control: unlisted path stays gated (302) | PASS | curl smoke | + +## Verdict +**ALL CLEAN.** The #191 change preserves every existing auth-gating behavior +(public routes load anonymously; protected routes redirect/401; login works; +authenticated access works) and the new public-route mechanism works end-to-end +in the live app with no over-matching. No bugs found → no fix loop required. diff --git a/CLAUDE.md b/CLAUDE.md index 56c12bfa..d351c508 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -67,7 +67,7 @@ modules/// ``` **Lifecycle hooks** (in `framework/core/simple_module_core/module.py`) — all no-op by default; subclasses override as needed: -`register_settings` → `register_menu_items` / `register_permissions` / `register_feature_flags` / `register_event_handlers` / `register_health_checks` → `register_exception_handlers` → `register_middleware` → `register_routes(api_router, view_router)` → async `on_startup` / `on_shutdown` (reverse order). +`register_settings` → `register_menu_items` / `register_permissions` / `register_feature_flags` / `register_event_handlers` / `register_health_checks` / `register_public_routes` → `register_exception_handlers` → `register_middleware` → `register_routes(api_router, view_router)` → async `on_startup` / `on_shutdown` (reverse order). `register_public_routes(registry)` lets a module exempt anonymous/read-only routes (STAC/OGC, webhooks) from `AuthMiddleware`; rules are method-aware (`registry.add_regex(r"…/tilejson$", methods={"GET"})`), so a GET read route can be public while sibling POST/PATCH mutations under the same prefix stay gated. See [docs/framework/public-routes.md](docs/framework/public-routes.md). **Middleware pipeline** (Starlette `add_middleware` is LIFO — last added runs first). Execution order on a request: `CorrelationId → RequestLogging → SecurityHeaders → Session → → Tenant (opt-in) → Locale → InertiaLayoutData → app`. When two modules add middleware at the same dependency tier, the module that sorts **later** wraps outermost. Use `depends_on` to express relative order — don't rely on names. diff --git a/docs/framework-conventions.md b/docs/framework-conventions.md index 58c2ec25..287d8ef3 100644 --- a/docs/framework-conventions.md +++ b/docs/framework-conventions.md @@ -263,6 +263,25 @@ a 302 redirect to the provider's login URL. Boot-time diagnostic `SM020` fails if multiple auth providers are installed. `SM021` warns if none is installed. +### Public routes (anonymous access) + +To expose a route without a session — a read-only STAC / OGC API, a TileJSON +endpoint, an inbound webhook — a module overrides `register_public_routes`: + +```python +def register_public_routes(self, registry): + registry.add_prefix("/api/gis/stac") + registry.add_regex(r"/api/gis/datasets/[^/]+/tilejson$", methods={"GET"}) +``` + +Rules are **method-aware**, so a GET read route can be exempted while sibling +`POST`/`PATCH` mutations under the same prefix stay gated. The host aggregates +every module's rules into one `PublicRouteRegistry` (plus host-level +`SM_AUTH_PUBLIC_PATHS` prefixes) and publishes it at `app.state.public_routes`, +which `AuthMiddleware` consults on every request. See +[`docs/framework/public-routes.md`](framework/public-routes.md) for match kinds +and resolution order. + ## Events Base class: `Event` from `simple_module_core.events`. Subclass per domain event: diff --git a/docs/framework/public-routes.md b/docs/framework/public-routes.md new file mode 100644 index 00000000..a5692e4a --- /dev/null +++ b/docs/framework/public-routes.md @@ -0,0 +1,102 @@ +# Public routes (anonymous access) + +`AuthMiddleware` (in `auth/middleware.py`) gates **every** request behind the +active auth provider — unauthenticated browser requests get a 302 to the login +URL, unauthenticated API requests get a 401. Modules that need to expose a +route without a session (a read-only STAC / OGC API, a TileJSON endpoint, an +inbound webhook, a status page) declare those routes through the +`register_public_routes` hook. The host collects every module's contributions +into one `PublicRouteRegistry` at boot, and the middleware consults it on every +request. + +This is the supported alternative to the legacy `AuthProvider.get_public_paths` +contract, which is **prefix-only and method-agnostic** — it cannot express +"expose `GET /api/gis/datasets/{id}/tilejson` but keep `PATCH …/visibility` +authenticated" because read and write routes share a prefix. + +## The hook + +```python +from simple_module_core import ModuleBase, ModuleMeta, PublicRouteRegistry + +class GisModule(ModuleBase): + meta = ModuleMeta(name="Gis", route_prefix="/api/gis") + + def register_public_routes(self, registry: PublicRouteRegistry) -> None: + # Whole read-only subtrees — any verb, any subpath. + registry.add_prefix("/api/gis/stac") + registry.add_prefix("/api/gis/ogc/") + # A single anonymous search endpoint. + registry.add_exact("/api/gis/catalog/search") + # A GET read route nested under a mutation-bearing prefix: expose the + # read, keep POST/PATCH siblings gated. + registry.add_regex(r"/api/gis/datasets/[^/]+/tilejson$", methods={"GET"}) +``` + +The hook runs once at boot, in dependency order, alongside the other +`register_*` registration hooks. + +## Match kinds + +A `PublicRoute` is **method-aware** and supports four match kinds. Every helper +takes an optional `methods=` set (case-insensitive); omitting it means *any* +verb matches. + +| Helper | Matches when | Use for | +|---|---|---| +| `registry.add_prefix(p)` | `path.startswith(p)` | a whole read-only subtree | +| `registry.add_exact(p)` | `path == p` | a single endpoint | +| `registry.add_suffix(p)` | `path.endswith(p)` | a tail shared across resources | +| `registry.add_regex(p)` | `re.match(p, path)` (anchored at start) | a read route nested under a mutation prefix | + +`registry.add(route_or_pattern, *, methods=, kind=)` is the general form; +pass a prebuilt `PublicRoute` or a string. + +## Why method-awareness matters + +`/api/gis/datasets/{id}/` carries both reads and mutations: + +- `GET /api/gis/datasets/{id}/tilejson` — safe to expose anonymously +- `PATCH /api/gis/datasets/{id}/visibility` — must stay authenticated +- `POST /api/gis/datasets/{id}/reprocess` — must stay authenticated + +A prefix rule would open all three. A method-scoped regex +(`methods={"GET"}`) exempts only the read, so the mutations keep returning 401 +to anonymous callers. + +## Resolution order + +For each request, `AuthMiddleware` treats the path as public if **any** of: + +1. **Framework defaults** — `/health`, `/static/`, `/api/docs`, `/openapi.json`, + `/i18n/`, the root `/`. +2. **`app.state.public_routes`** — the `PublicRouteRegistry` (module hooks + + `SM_AUTH_PUBLIC_PATHS`). Method-aware. +3. **`provider.get_public_paths()`** — the auth provider's own login / register + routes (legacy, prefix-only). Kept for back-compat. + +A public path still resolves the user when a valid session is present (so a +public landing page can show "Open Dashboard" to a logged-in visitor) — it just +never *redirects* an anonymous caller away. + +## Host-level escape hatch + +When no module owns a route, an app can expose prefixes from the environment +without writing a module: + +```bash +SM_AUTH_PUBLIC_PATHS='["/api/integrations/webhook", "/status"]' +``` + +These are seeded as prefix rules (method-agnostic). Prefer the +`register_public_routes` hook inside a module when you need method-awareness or +want the exemption to travel with the module that owns the route. + +## Where it lives + +- `PublicRoute` / `PublicRouteRegistry` — `simple_module_core.public_routes` +- The hook — `ModuleBase.register_public_routes` +- Wiring — `simple_module_hosting.app_builder.create_app` populates the + registry and publishes it at `app.state.public_routes` (also + `app.state.sm.public_routes`) +- Enforcement — `auth.middleware.AuthMiddleware` diff --git a/framework/core/simple_module_core/__init__.py b/framework/core/simple_module_core/__init__.py index 4c3edc64..f2085720 100644 --- a/framework/core/simple_module_core/__init__.py +++ b/framework/core/simple_module_core/__init__.py @@ -33,6 +33,7 @@ from simple_module_core.menu import MenuItem, MenuRegistry, MenuSection from simple_module_core.module import ModuleBase, ModuleMeta from simple_module_core.permissions import PermissionRegistry +from simple_module_core.public_routes import PublicRoute, PublicRouteRegistry from simple_module_core.services import Services from simple_module_core.versioning import FRAMEWORK_API_VERSION, check_framework_compatibility @@ -60,6 +61,8 @@ "ModuleMeta", "NotFoundError", "PermissionRegistry", + "PublicRoute", + "PublicRouteRegistry", "Services", "Translator", "ValidationError", diff --git a/framework/core/simple_module_core/diagnostics/_module.py b/framework/core/simple_module_core/diagnostics/_module.py index b58e1a68..64a2e836 100644 --- a/framework/core/simple_module_core/diagnostics/_module.py +++ b/framework/core/simple_module_core/diagnostics/_module.py @@ -95,6 +95,7 @@ def _check_empty_modules(self, modules: list[ModuleBase]) -> list[Diagnostic]: "register_event_handlers", "register_middleware", "register_health_checks", + "register_public_routes", "register_exception_handlers", "register_settings", "template_dirs", diff --git a/framework/core/simple_module_core/module.py b/framework/core/simple_module_core/module.py index 84b91610..c7cec220 100644 --- a/framework/core/simple_module_core/module.py +++ b/framework/core/simple_module_core/module.py @@ -15,6 +15,7 @@ from simple_module_core.health import HealthRegistry from simple_module_core.menu import MenuRegistry from simple_module_core.permissions import PermissionRegistry + from simple_module_core.public_routes import PublicRouteRegistry @dataclass(frozen=True) @@ -118,6 +119,24 @@ def register_event_handlers(self, bus: EventBus, app: FastAPI | None = None) -> def register_health_checks(self, registry: HealthRegistry) -> None: """Contribute health checks for the ``/health/ready`` endpoint.""" + def register_public_routes(self, registry: PublicRouteRegistry) -> None: + """Declare routes that must bypass authentication (anonymous access). + + ``AuthMiddleware`` gates every request behind the active auth provider. + Override this hook to exempt read-only or webhook routes that are meant + to be reached without a session — e.g. a STAC / OGC API surface:: + + def register_public_routes(self, registry): + registry.add_prefix("/api/gis/stac") + registry.add_regex( + r"/api/gis/datasets/[^/]+/tilejson$", methods={"GET"} + ) + + Rules are method-aware, so a GET read route nested under a prefix that + also carries POST/PATCH mutations can be exempted without opening the + mutations. Called once at boot, in dependency order. + """ + def register_middleware(self, app: FastAPI) -> None: """Add middleware to the application. diff --git a/framework/core/simple_module_core/public_routes.py b/framework/core/simple_module_core/public_routes.py new file mode 100644 index 00000000..6b9379e6 --- /dev/null +++ b/framework/core/simple_module_core/public_routes.py @@ -0,0 +1,129 @@ +"""Public-route registry — modules declare routes the auth layer must NOT gate. + +``AuthMiddleware`` gates every request behind the active auth provider. Modules +that expose anonymous read APIs (STAC / OGC API / TileJSON, public webhooks, +status pages) contribute exemptions here via +:meth:`~simple_module_core.module.ModuleBase.register_public_routes`. The host +collects them into one registry at boot and the middleware consults it on every +request. + +Unlike the legacy ``AuthProvider.get_public_paths`` contract — a flat tuple of +prefixes matched with ``str.startswith`` — a :class:`PublicRoute` is +**method-aware** and supports prefix / exact / suffix / regex matching. That +lets a module expose ``GET /api/gis/datasets/{id}/tilejson`` while leaving +``PATCH``/``POST`` siblings under the same prefix authenticated. +""" + +from __future__ import annotations + +import re +from collections.abc import Iterable + +_MatchKind = str # one of: "prefix" | "exact" | "suffix" | "regex" +_VALID_KINDS = ("prefix", "exact", "suffix", "regex") + + +class PublicRoute: + """A single anonymous-access rule. + + Args: + pattern: The path (or path fragment / regex) to match against + ``request.url.path``. + methods: HTTP methods this rule applies to (case-insensitive). ``None`` + (the default) means *any* method — the rule matches every verb. + kind: How ``pattern`` is interpreted — ``"prefix"`` (default, matches + any path that starts with it), ``"exact"``, ``"suffix"``, or + ``"regex"`` (anchored at the start of the path via ``re.match``). + """ + + __slots__ = ("_regex", "kind", "methods", "pattern") + + def __init__( + self, + pattern: str, + *, + methods: Iterable[str] | None = None, + kind: _MatchKind = "prefix", + ) -> None: + if kind not in _VALID_KINDS: + raise ValueError(f"Unknown match kind {kind!r}; expected one of {_VALID_KINDS}") + self.pattern = pattern + self.methods: frozenset[str] | None = ( + None if methods is None else frozenset(m.upper() for m in methods) + ) + self.kind = kind + self._regex = re.compile(pattern) if kind == "regex" else None + + def matches(self, method: str, path: str) -> bool: + """Return ``True`` if *method* + *path* are exempt under this rule.""" + if self.methods is not None and method.upper() not in self.methods: + return False + if self.kind == "prefix": + return path.startswith(self.pattern) + if self.kind == "exact": + return path == self.pattern + if self.kind == "suffix": + return path.endswith(self.pattern) + assert self._regex is not None # kind == "regex" + return self._regex.match(path) is not None + + def __repr__(self) -> str: + methods = "*" if self.methods is None else ",".join(sorted(self.methods)) + return f"PublicRoute({self.pattern!r}, kind={self.kind!r}, methods={methods})" + + +class PublicRouteRegistry: + """Aggregates every module's :class:`PublicRoute` rules. + + Populated once during boot (``register_public_routes`` hook) and read on + every unauthenticated request by ``AuthMiddleware`` — effectively immutable + after the registration phase. + """ + + def __init__(self) -> None: + self._routes: list[PublicRoute] = [] + + def add( + self, + route: PublicRoute | str, + *, + methods: Iterable[str] | None = None, + kind: _MatchKind = "prefix", + ) -> None: + """Register a rule — either a prebuilt :class:`PublicRoute` or a pattern. + + Passing a string builds a :class:`PublicRoute` from ``methods``/``kind``; + passing a :class:`PublicRoute` ignores those keyword arguments. + """ + if isinstance(route, PublicRoute): + self._routes.append(route) + else: + self._routes.append(PublicRoute(route, methods=methods, kind=kind)) + + def add_prefix(self, prefix: str, *, methods: Iterable[str] | None = None) -> None: + """Exempt any path starting with *prefix*.""" + self._routes.append(PublicRoute(prefix, methods=methods, kind="prefix")) + + def add_exact(self, path: str, *, methods: Iterable[str] | None = None) -> None: + """Exempt exactly *path*.""" + self._routes.append(PublicRoute(path, methods=methods, kind="exact")) + + def add_suffix(self, suffix: str, *, methods: Iterable[str] | None = None) -> None: + """Exempt any path ending with *suffix*.""" + self._routes.append(PublicRoute(suffix, methods=methods, kind="suffix")) + + def add_regex(self, pattern: str, *, methods: Iterable[str] | None = None) -> None: + """Exempt any path whose start matches *pattern* (``re.match`` semantics).""" + self._routes.append(PublicRoute(pattern, methods=methods, kind="regex")) + + def matches(self, method: str, path: str) -> bool: + """Return ``True`` if any registered rule exempts *method* + *path*.""" + return any(route.matches(method, path) for route in self._routes) + + @property + def routes(self) -> list[PublicRoute]: + """All registered rules (a copy — mutating it doesn't affect the registry).""" + return list(self._routes) + + +__all__ = ["PublicRoute", "PublicRouteRegistry"] diff --git a/framework/core/simple_module_core/services.py b/framework/core/simple_module_core/services.py index 9090176d..d8126221 100644 --- a/framework/core/simple_module_core/services.py +++ b/framework/core/simple_module_core/services.py @@ -27,6 +27,7 @@ from simple_module_core.menu import MenuRegistry from simple_module_core.module import ModuleBase from simple_module_core.permissions import PermissionRegistry + from simple_module_core.public_routes import PublicRouteRegistry @dataclass(frozen=True, slots=True) @@ -40,6 +41,7 @@ class Services: permissions: PermissionRegistry feature_flags: FeatureFlagRegistry health_registry: HealthRegistry + public_routes: PublicRouteRegistry i18n_registry: I18nRegistry inertia_config: InertiaConfig modules: tuple[ModuleBase, ...] diff --git a/framework/core/tests/test_module_base.py b/framework/core/tests/test_module_base.py index 5e76cde0..a5f0b672 100644 --- a/framework/core/tests/test_module_base.py +++ b/framework/core/tests/test_module_base.py @@ -101,6 +101,30 @@ async def test_register_settings_default_noop(self): mod = DummyModule() mod.register_settings(None) + async def test_register_public_routes_default_noop(self): + from simple_module_core.public_routes import PublicRouteRegistry + + mod = DummyModule() + reg = PublicRouteRegistry() + mod.register_public_routes(reg) + assert reg.routes == [] + + async def test_register_public_routes_override(self): + from simple_module_core.public_routes import PublicRouteRegistry + + class ModWithPublic(ModuleBase): + meta = ModuleMeta(name="WithPublic") + + def register_public_routes(self, registry): + registry.add_prefix("/api/with-public/stac") + registry.add_regex(r"/api/with-public/datasets/[^/]+/tilejson$", methods={"GET"}) + + reg = PublicRouteRegistry() + ModWithPublic().register_public_routes(reg) + assert reg.matches("GET", "/api/with-public/stac/collections") + assert reg.matches("GET", "/api/with-public/datasets/9/tilejson") + assert not reg.matches("PATCH", "/api/with-public/datasets/9/tilejson") + class TestModuleAssetHooks: async def test_template_dirs_default_empty(self): diff --git a/framework/core/tests/test_module_diagnostics.py b/framework/core/tests/test_module_diagnostics.py index 1dbe6662..6b103d77 100644 --- a/framework/core/tests/test_module_diagnostics.py +++ b/framework/core/tests/test_module_diagnostics.py @@ -193,6 +193,36 @@ async def test_silent_when_no_pages_dir(self, tmp_path: Path): assert results == [] +class TestSM007EmptyModules: + """SM007 fires only when a module overrides no registration hooks at all.""" + + def _diags(self, modules): + from simple_module_core.diagnostics._module import ModuleDiagnostics + + return list(ModuleDiagnostics()._check_empty_modules(modules)) + + async def test_fires_when_no_hooks_overridden(self): + from simple_module_core.module import ModuleBase, ModuleMeta + + class Bare(ModuleBase): + meta = ModuleMeta(name="Bare") + + results = self._diags([Bare()]) + assert len(results) == 1 + assert results[0].code == "SM007" + + async def test_silent_when_only_public_routes_registered(self): + from simple_module_core.module import ModuleBase, ModuleMeta + + class PublicOnly(ModuleBase): + meta = ModuleMeta(name="PublicOnly") + + def register_public_routes(self, registry): + registry.add_prefix("/api/public_only/stac") + + assert self._diags([PublicOnly()]) == [] + + class TestSM019ViewsWithoutMenu: """SM019 fires when a module ships view routes but never registers a menu item.""" diff --git a/framework/core/tests/test_public_routes.py b/framework/core/tests/test_public_routes.py new file mode 100644 index 00000000..ca773283 --- /dev/null +++ b/framework/core/tests/test_public_routes.py @@ -0,0 +1,94 @@ +"""Tests for PublicRouteRegistry — method-aware anonymous-access rules. + +The registry is the extension point modules use (via +``ModuleBase.register_public_routes``) to declare routes the auth layer must +let through unauthenticated. Unlike the legacy provider ``get_public_paths`` +contract, a rule can be scoped to specific HTTP methods so a read route nested +under a mutation-bearing prefix can be exempted without opening the mutations. +""" + +from __future__ import annotations + +from simple_module_core.public_routes import PublicRoute, PublicRouteRegistry + + +class TestPublicRouteMatching: + def test_prefix_matches_any_subpath(self): + route = PublicRoute("/api/gis/stac") + assert route.matches("GET", "/api/gis/stac") + assert route.matches("GET", "/api/gis/stac/collections") + assert not route.matches("GET", "/api/gis/datasets") + + def test_exact_matches_only_full_path(self): + route = PublicRoute("/api/gis/catalog/search", kind="exact") + assert route.matches("GET", "/api/gis/catalog/search") + assert not route.matches("GET", "/api/gis/catalog/search/extra") + + def test_suffix_matches_path_tail(self): + route = PublicRoute("/tilejson", kind="suffix") + assert route.matches("GET", "/api/gis/datasets/42/tilejson") + assert not route.matches("GET", "/api/gis/datasets/42/visibility") + + def test_regex_is_anchored_at_start(self): + route = PublicRoute(r"/api/gis/datasets/[^/]+/tilejson$", kind="regex") + assert route.matches("GET", "/api/gis/datasets/42/tilejson") + assert not route.matches("GET", "/api/gis/datasets/42/tilejson/extra") + assert not route.matches("GET", "/prefix/api/gis/datasets/42/tilejson") + + def test_methods_none_matches_every_verb(self): + route = PublicRoute("/api/gis/stac") + for method in ("GET", "POST", "PATCH", "DELETE"): + assert route.matches(method, "/api/gis/stac") + + def test_methods_restrict_to_listed_verbs(self): + route = PublicRoute("/api/gis/datasets/", methods={"GET"}) + assert route.matches("GET", "/api/gis/datasets/42/tilejson") + assert not route.matches("PATCH", "/api/gis/datasets/42/visibility") + assert not route.matches("POST", "/api/gis/datasets/42/reprocess") + + def test_method_matching_is_case_insensitive(self): + route = PublicRoute("/api/gis/stac", methods={"get"}) + assert route.matches("GET", "/api/gis/stac") + + +class TestPublicRouteRegistry: + def test_empty_registry_matches_nothing(self): + registry = PublicRouteRegistry() + assert not registry.matches("GET", "/api/gis/stac") + + def test_add_prefix(self): + registry = PublicRouteRegistry() + registry.add_prefix("/api/gis/ogc/") + assert registry.matches("GET", "/api/gis/ogc/collections") + assert not registry.matches("GET", "/api/gis/datasets") + + def test_add_exact(self): + registry = PublicRouteRegistry() + registry.add_exact("/api/gis/catalog/search") + assert registry.matches("POST", "/api/gis/catalog/search") + assert not registry.matches("POST", "/api/gis/catalog/search/x") + + def test_add_regex_with_method(self): + registry = PublicRouteRegistry() + registry.add_regex(r"/api/gis/datasets/[^/]+/tilejson$", methods={"GET"}) + assert registry.matches("GET", "/api/gis/datasets/7/tilejson") + assert not registry.matches("PATCH", "/api/gis/datasets/7/tilejson") + + def test_matches_is_true_if_any_route_matches(self): + registry = PublicRouteRegistry() + registry.add_prefix("/api/gis/ogc/") + registry.add_exact("/api/gis/catalog/search") + assert registry.matches("GET", "/api/gis/ogc/tiles") + assert registry.matches("GET", "/api/gis/catalog/search") + + def test_routes_exposes_registered_rules(self): + registry = PublicRouteRegistry() + registry.add_prefix("/a") + registry.add_exact("/b") + assert len(registry.routes) == 2 + assert all(isinstance(r, PublicRoute) for r in registry.routes) + + def test_add_accepts_a_prebuilt_route(self): + registry = PublicRouteRegistry() + registry.add(PublicRoute("/api/gis/stac")) + assert registry.matches("GET", "/api/gis/stac") diff --git a/framework/core/tests/test_services.py b/framework/core/tests/test_services.py index ae2b63d8..86cd6228 100644 --- a/framework/core/tests/test_services.py +++ b/framework/core/tests/test_services.py @@ -29,6 +29,7 @@ async def test_services_round_trip_field_access(self) -> None: assert s.permissions is _SENTINEL_PERMS assert s.feature_flags is _SENTINEL_FLAGS assert s.health_registry is _SENTINEL_HEALTH + assert s.public_routes is _SENTINEL_PUBLIC_ROUTES assert s.i18n_registry is _SENTINEL_I18N assert s.inertia_config is _SENTINEL_INERTIA assert s.modules == () @@ -41,6 +42,7 @@ async def test_services_round_trip_field_access(self) -> None: _SENTINEL_PERMS = object() _SENTINEL_FLAGS = object() _SENTINEL_HEALTH = object() +_SENTINEL_PUBLIC_ROUTES = object() _SENTINEL_I18N = object() _SENTINEL_INERTIA = object() @@ -55,6 +57,7 @@ def _make_services() -> Services: permissions=_SENTINEL_PERMS, # type: ignore[arg-type] feature_flags=_SENTINEL_FLAGS, # type: ignore[arg-type] health_registry=_SENTINEL_HEALTH, # type: ignore[arg-type] + public_routes=_SENTINEL_PUBLIC_ROUTES, # type: ignore[arg-type] i18n_registry=_SENTINEL_I18N, # type: ignore[arg-type] inertia_config=_SENTINEL_INERTIA, # type: ignore[arg-type] modules=(), diff --git a/framework/hosting/simple_module_hosting/_phase_helpers.py b/framework/hosting/simple_module_hosting/_phase_helpers.py index 888337a5..fd75ea88 100644 --- a/framework/hosting/simple_module_hosting/_phase_helpers.py +++ b/framework/hosting/simple_module_hosting/_phase_helpers.py @@ -105,6 +105,20 @@ def install_middleware( app.add_middleware(CorrelationIdMiddleware) +def attach_public_routes(app: FastAPI, settings: Settings, registry) -> None: + """Seed host-level public paths and publish the registry for AuthMiddleware. + + Modules contribute method-aware rules through their ``register_public_routes`` + hook (already applied to *registry* by the caller). This adds the host escape + hatch — ``SM_AUTH_PUBLIC_PATHS`` prefixes — then exposes the registry at + ``app.state.public_routes``, where ``auth.middleware.AuthMiddleware`` reads it + on every request. + """ + for prefix in settings.auth_public_paths: + registry.add_prefix(prefix) + app.state.public_routes = registry + + def mount_module_static_dirs(app: FastAPI, modules: list) -> None: """Mount each module's declared static directories. diff --git a/framework/hosting/simple_module_hosting/app_builder.py b/framework/hosting/simple_module_hosting/app_builder.py index 86b67f7a..2a96040d 100644 --- a/framework/hosting/simple_module_hosting/app_builder.py +++ b/framework/hosting/simple_module_hosting/app_builder.py @@ -18,6 +18,7 @@ from simple_module_core.health import HealthRegistry from simple_module_core.menu import MenuRegistry from simple_module_core.permissions import PermissionRegistry +from simple_module_core.public_routes import PublicRouteRegistry from simple_module_core.services import Services from simple_module_db.listeners import register_listeners from simple_module_db.session import init_db @@ -25,6 +26,7 @@ from simple_module_hosting._host_services import _HostServices from simple_module_hosting._inertia_setup import setup_inertia from simple_module_hosting._phase_helpers import ( + attach_public_routes, check_settings_registration, install_middleware, mount_module_static_dirs, @@ -162,6 +164,7 @@ def create_app(settings: Settings | None = None) -> FastAPI: ff_registry = FeatureFlagRegistry() event_bus = EventBus() health_registry = HealthRegistry() + public_route_registry = PublicRouteRegistry() @asynccontextmanager async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: @@ -232,13 +235,18 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: mod.register_feature_flags(ff_registry) _register_event_handlers(mod, event_bus, app) mod.register_health_checks(health_registry) + mod.register_public_routes(public_route_registry) + + attach_public_routes(app, settings, public_route_registry) logger.info( - "Registered %d menu items, %d permissions, %d feature flags, %d health checks", + "Registered %d menu items, %d permissions, %d feature flags, " + "%d health checks, %d public routes", len(menu_registry.all_items), len(perm_registry.all_permissions), len(ff_registry.all_flags), len(health_registry.all_checks), + len(public_route_registry.routes), ) # ── Phase 6: Initialize database ─────────────────────── @@ -281,6 +289,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: permissions=perm_registry, feature_flags=ff_registry, health_registry=health_registry, + public_routes=public_route_registry, i18n_registry=i18n_registry, inertia_config=inertia_config, modules=tuple(modules), diff --git a/framework/hosting/simple_module_hosting/bootstrap_settings.py b/framework/hosting/simple_module_hosting/bootstrap_settings.py index ef7048cf..a7222ebb 100644 --- a/framework/hosting/simple_module_hosting/bootstrap_settings.py +++ b/framework/hosting/simple_module_hosting/bootstrap_settings.py @@ -37,6 +37,15 @@ class BootstrapSettings(BaseSettings): modules_enabled: list[str] | None = None + auth_public_paths: list[str] = [] + """Host-level anonymous-access path prefixes (``SM_AUTH_PUBLIC_PATHS``). + + An escape hatch for exposing a route without a session when no module owns + it. Each entry is treated as a prefix rule and seeded into the + ``PublicRouteRegistry`` at boot. Modules should prefer the + ``register_public_routes`` hook, which is method-aware. + """ + @property def is_development(self) -> bool: return self.environment == "development" diff --git a/framework/hosting/tests/test_app.py b/framework/hosting/tests/test_app.py index e434f510..e938a2d6 100644 --- a/framework/hosting/tests/test_app.py +++ b/framework/hosting/tests/test_app.py @@ -65,6 +65,95 @@ def fake_discover(enabled=None, *, strict=False): paths = {getattr(r, "path", None) for r in app.routes} assert "/modules/fakestatic/static" in paths + async def test_module_register_public_routes_is_wired( + self, + settings: Settings, + monkeypatch, + ): + """register_public_routes() contributions land on app.state.public_routes.""" + from simple_module_core import ModuleBase, ModuleMeta + from simple_module_hosting import app_builder + + class FakePublicMod(ModuleBase): + meta = ModuleMeta(name="FakePublic") + + def register_public_routes(self, registry): + registry.add_prefix("/api/fakepublic/stac") + registry.add_regex(r"/api/fakepublic/datasets/[^/]+/tilejson$", methods={"GET"}) + + real_discover = app_builder.discover_modules + + def fake_discover(enabled=None, *, strict=False): + return [*real_discover(enabled=enabled, strict=strict), FakePublicMod()] + + monkeypatch.setattr(app_builder, "discover_modules", fake_discover) + + app = create_app(settings) + registry = app.state.public_routes + assert registry is app.state.sm.public_routes + assert registry.matches("GET", "/api/fakepublic/stac/collections") + assert registry.matches("GET", "/api/fakepublic/datasets/7/tilejson") + assert not registry.matches("PATCH", "/api/fakepublic/datasets/7/tilejson") + + async def test_host_public_paths_setting_is_seeded(self, settings: Settings): + """SM_AUTH_PUBLIC_PATHS prefixes land on the registry as prefix rules.""" + with_paths = settings.model_copy( + update={"auth_public_paths": ["/api/hostpublic", "/status"]} + ) + app = create_app(with_paths) + registry = app.state.public_routes + assert registry.matches("GET", "/api/hostpublic/anything") + assert registry.matches("POST", "/status") + assert not registry.matches("GET", "/api/private") + + async def test_module_public_route_reachable_anonymously( + self, + settings: Settings, + monkeypatch, + ): + """End-to-end: an unauthenticated GET to a module-declared public route + returns 200, while a sibling gated route under the same prefix 401s. + + This is the repro from issue #191 — a read-only anonymous API + (STAC / OGC) consumed without a session cookie. + """ + from simple_module_core import ModuleBase, ModuleMeta + from simple_module_hosting import app_builder + + class FakeGisMod(ModuleBase): + meta = ModuleMeta(name="FakeGis", route_prefix="/api/fakegis") + + def register_routes(self, api_router, view_router): + @api_router.get("/stac") + async def stac(): + return {"type": "Catalog"} + + @api_router.get("/secret") + async def secret(): + return {"private": True} + + def register_public_routes(self, registry): + registry.add_prefix("/api/fakegis/stac") + + real_discover = app_builder.discover_modules + + def fake_discover(enabled=None, *, strict=False): + return [*real_discover(enabled=enabled, strict=strict), FakeGisMod()] + + monkeypatch.setattr(app_builder, "discover_modules", fake_discover) + + app = create_app(settings) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient( + transport=transport, base_url="http://testserver", follow_redirects=False + ) as client: + public = await client.get("/api/fakegis/stac") + gated = await client.get("/api/fakegis/secret") + + assert public.status_code == 200 + assert public.json() == {"type": "Catalog"} + assert gated.status_code == 401 + async def test_app_state_has_sm_services( self, monkeypatch: pytest.MonkeyPatch, tmp_path ) -> None: diff --git a/modules/auth/auth/middleware.py b/modules/auth/auth/middleware.py index 995a1799..91e8383a 100644 --- a/modules/auth/auth/middleware.py +++ b/modules/auth/auth/middleware.py @@ -47,7 +47,9 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: return path: str = scope["path"] - auth_state = scope["app"].state.auth + method: str = scope.get("method", "GET") + app_state = scope["app"].state + auth_state = app_state.auth provider = auth_state.auth_provider if provider is None: @@ -58,6 +60,14 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: any(path.startswith(p) for p in _FRAMEWORK_PUBLIC_PREFIXES) or path in _FRAMEWORK_PUBLIC_EXACT ) + # Module-contributed public routes (register_public_routes hook). Method + # -aware, so a GET read route can be exempted without opening sibling + # POST/PATCH mutations under the same prefix. + if not is_public: + public_routes = getattr(app_state, "public_routes", None) + is_public = public_routes is not None and public_routes.matches(method, path) + # Legacy provider-declared paths (prefix-only, method-agnostic). Kept for + # back-compat with AuthProvider implementations. if not is_public: prefix_paths, exact_paths = provider.get_public_paths() is_public = any(path.startswith(p) for p in prefix_paths) or path in exact_paths diff --git a/modules/auth/tests/test_auth_middleware.py b/modules/auth/tests/test_auth_middleware.py index e990b3a8..2254bc49 100644 --- a/modules/auth/tests/test_auth_middleware.py +++ b/modules/auth/tests/test_auth_middleware.py @@ -44,15 +44,16 @@ def is_bearer_request(self, request): return auth.startswith("Bearer ") -def _build_app(provider, *, principal_resolvers=None): +def _build_app(provider, *, principal_resolvers=None, public_routes=None): app = FastAPI() app.state.auth = AuthState( auth_provider=provider, principal_resolvers=list(principal_resolvers or []), ) + if public_routes is not None: + app.state.public_routes = public_routes - @app.get("/{path:path}") - async def catch_all(request: Request, path: str = ""): + async def _handler(request: Request, path: str = ""): user = getattr(request.state, "user", None) return JSONResponse( { @@ -60,6 +61,8 @@ async def catch_all(request: Request, path: str = ""): } ) + app.add_api_route("/{path:path}", _handler, methods=["GET", "POST", "PATCH"]) + app.add_middleware(AuthMiddleware) app.add_middleware(SessionMiddleware, secret_key=SECRET) return app @@ -129,6 +132,47 @@ async def test_root_is_public(unauthenticated_app): assert resp.status_code == 200 +async def test_registry_public_route_skips_auth(): + """A module-contributed public route lets an unauthenticated GET through.""" + from simple_module_core.public_routes import PublicRouteRegistry + + registry = PublicRouteRegistry() + registry.add_prefix("/api/gis/stac") + app = _build_app(_StubProvider(user=None), public_routes=registry) + + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as c: + resp = await c.get("/api/gis/stac/collections") + assert resp.status_code == 200 + assert resp.json()["user"] is None + + +async def test_registry_method_scoping_gates_other_verbs(): + """A GET-scoped public rule exempts GET but still gates PATCH on the same path.""" + from simple_module_core.public_routes import PublicRouteRegistry + + registry = PublicRouteRegistry() + registry.add_regex(r"/api/gis/datasets/[^/]+/tilejson$", methods={"GET"}) + app = _build_app(_StubProvider(user=None), public_routes=registry) + + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as c: + ok = await c.get("/api/gis/datasets/42/tilejson") + gated = await c.patch("/api/gis/datasets/42/tilejson") + assert ok.status_code == 200 + assert gated.status_code == 401 + + +async def test_no_registry_falls_back_to_provider_paths(unauthenticated_app): + """Apps built without a public-routes registry still honor provider paths.""" + transport = httpx.ASGITransport(app=unauthenticated_app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as c: + public = await c.get("/stub/public/data") + gated = await c.get("/api/protected") + assert public.status_code == 200 + assert gated.status_code == 401 + + async def test_resolver_chain_fallback(): """When provider returns None, fall through to principal resolvers.""" diff --git a/modules/settings/tests/test_module_settings.py b/modules/settings/tests/test_module_settings.py index ac3ae61c..fb58d27a 100644 --- a/modules/settings/tests/test_module_settings.py +++ b/modules/settings/tests/test_module_settings.py @@ -47,6 +47,7 @@ def test_collect_exposes_type_requires_restart_group(): permissions=None, # type: ignore[arg-type] feature_flags=None, # type: ignore[arg-type] health_registry=None, # type: ignore[arg-type] + public_routes=None, # type: ignore[arg-type] i18n_registry=None, # type: ignore[arg-type] inertia_config=None, # type: ignore[arg-type] modules=(_DemoModule(),), # type: ignore[arg-type]