13 - API (feat): Move Authn/z to Edge Guards

api/docs/adr/000-vertical-slice-architecture.md

@@ -83,7 +83,7 @@ damnit_api/
 │   ├── serialization.py, preview.py
 │   └── gql.py       #   Strawberry types + Query/Subscription contributions
 │
-├── auth/            # OIDC flow, sessions, tokens, users, authz policy
+├── auth/            # OIDC flow, sessions, tokens, users, authz policy; see ADR-011
 ├── contextfile/     # context-file (REST) endpoints
 ├── mymdc/           # MyMdC port
 ├── appdb/           # application-DB engine/session/models; see ADR-010

api/docs/adr/011-authorisation-at-the-edge.md

@@ -0,0 +1,42 @@
+---
+date: 2026-07-08
+---
+
+# ADR-011 - Authorisation at the edge
+
+## Context and Problem Statement
+
+Proposal-membership authorisation was scattered and pointed the wrong way. The single predicate lived in `metadata/services.py` (`_check_user_allowed`), and the metadata services called it inline before each operation. The GraphQL permission classes reached into that same private function - an `auth`-to-`metadata` import which, together with `metadata` importing `auth` for the `User` type, formed a dependency cycle that [ADR-000](000-vertical-slice-architecture.md) forbids (`metadata -> auth`).
+
+Domain services performing authorisation also couples them to the request. A service can then only be called where a `User` is in scope, and the same check runs redundantly at the resolver and in the service.
+
+Separately, an unauthenticated GraphQL request produced a 500 rather than a 401.
+
+## Considered Options
+
+- One policy in `auth/`, enforced only at the transport edges.
+- Keep the check inline in the domain services.
+- A single global authorisation middleware.
+
+## Decision Outcome
+
+Chosen option: "one policy in `auth/`, enforced at the edges", because it gives the membership decision a single home in the slice that owns identity and leaves the domain services authorisation-free.
+
+`auth/policy.py` owns `require_proposal_member(user, proposal_number)`. Enforcement happens only at the edges: Strawberry permission classes for GraphQL fields, and a Litestar guard (`proposal_member_guard`) wired onto the proposal-scoped REST routers in the composition root. Local mode composes the guard out ([ADR-008](008-local-mode-composition.md)). The permission classes live in `shared/permissions.py` as transport adapters over the policy, so any slice's GraphQL contribution can attach them without importing `auth`. Domain services take plain parameters.
+
+Authentication failures now raise `UnauthenticatedError`, which the error handler renders as a 401.
+
+### Consequences
+
+- Good: domain services are reusable and context-agnostic; the `auth <-> metadata` cycle is broken.
+- Good: authorisation has one auditable choke point per transport.
+- Bad: two edge mechanisms - the GraphQL permission classes and the REST guard - must stay in step.
+- Bad: the permission adapters put Strawberry types in `shared/`, which is otherwise framework-light.
+
+## Details
+
+Authentication (OIDC on server-side sessions) is unchanged; it is covered by the framework and session decisions in [ADR-006](006-litestar.md). This decision concerns authorisation and the authentication *edge*, the 401.
+
+The adapters live in `shared/permissions.py` rather than the `graphql/` transport package so that a slice depends only on `shared`, which is always an allowed direction, and never on the composition/transport package. The policy predicate stays in `auth/` because membership is identity: `auth -> proposals`/`metadata` is the one allowed cross-slice edge ([ADR-000](000-vertical-slice-architecture.md)), and `auth/policy.py` itself needs no `metadata` import.
+
+The 401 is a clean JSON body, not a redirect. A GraphQL request is an XHR call, so the single-page app performs the login redirect on a 401 rather than following a server redirect to the login page.

api/docs/architecture.md

@@ -20,7 +20,7 @@ For more information, see [ADR-000](adr/000-vertical-slice-architecture.md).
 | --------------------------------- | --------------------------------------- | ------------------------------------------------------------------------------------------------------------------------- | ------- | ------------------------------------------------------------------------- |
 | `runs/`                           | Run/variable data - the core domain     | Domain models, repository interface + implementations (see [ADR-005](adr/005-repository-pattern.md)), serialisation, preview extraction, its GraphQL types and resolvers | Partial | `runs/` (repository, models, sqlite + csv backends); resolvers still in `graphql/queries.py`/`subscriptions.py` |
 | `proposals/`                      | Proposal metadata and lookup            | Proposal models, MyMdC-backed metadata services, path locator (see [ADR-004](adr/004-proposal-path-locator.md))            | Planned | `metadata/`                                                               |
-| `auth/`                           | Authentication and authorisation        | OAuth flow, sessions, token store, `User`, permission classes, the membership policy                                      | Partial | Policy still in `metadata/services.py`                                    |
+| `auth/`                           | Authentication and authorisation        | OAuth flow, sessions, token store, `User`, the membership policy (see [ADR-011](adr/011-authorisation-at-the-edge.md))     | Partial | Policy in `auth/policy.py`; permission adapters in `shared/permissions.py` |
 | `contextfile/`                    | Context-file viewing                    | File reading, watching, its routes                                                                                        | Done    | As-is                                                                     |
 | `graphql/`                        | GraphQL transport only                  | Schema assembly, context, directives, controller binding - no resolvers, no domain logic (see [ADR-007](adr/007-graphql-transport-only.md)) | Partial | Assembly still in `shared/gql.py`; resolvers still here                   |
 | `appdb/`                          | The app's own database (infrastructure) | Models, engine/session plumbing for `dw_api.sqlite` (see [ADR-010](adr/010-two-databases.md))                            | Partial | `_db/` (Advanced Alchemy `SQLAlchemyPlugin`); `metadata/repository.py`    |
@@ -56,15 +56,14 @@ The key rules are:
 2. **Composition root is the top:** it may import everything, but nothing is allowed to import it.
   - If importing a slice from the composition root forces a function-body import to avoid cycles, the type probably belongs in `core/`.
 3. **Composition root reads settings:** everything else receives configuration as parameters (see [ADR-003](adr/003-injected-settings.md)).
-4. **Authorisation applied at the edge:** routes and resolvers use dependencies and permission classes.
+4. **Authorisation applied at the edge:** routes and resolvers use dependencies and permission classes (see [ADR-011](adr/011-authorisation-at-the-edge.md)).
   - This means that services should not apply authorisation rules themselves.
 5. **No `if settings.is_local:` outside the composition root:** Local mode is selected by composition, not conditionals throughout the codebase (see [ADR-008](adr/008-local-mode-composition.md)).
 
 Note that these are currently only enforced by convention/review. Import linter/archetecture check tool is planned to be added.
 
 !!! warning "Current issues"
 
-    - `auth` <--> `metadata` import cycle
     - `shared/gql.py`'s import-everything role
     - Function-body imports working around circular imports
     - Imports 'across' many modules and their files

api/src/damnit_api/auth/models.py

@@ -8,6 +8,7 @@
 from .. import get_logger
 from .._db.dependencies import DBSession
 from .._mymdc.dependencies import MyMdCClient
+from ..shared.errors import UnauthenticatedError
 from ..shared.models import ProposalNumber
 
 logger = get_logger()
@@ -47,7 +48,7 @@ def from_connection(cls, connection: ASGIConnection) -> Self:
             if settings.is_local:
                 return DEV_USER  # type: ignore[return-value]
             msg = "No user info in session"
-            raise ValueError(msg)
+            raise UnauthenticatedError(msg)
 
         return cls.model_validate(user_dict)
 

api/src/damnit_api/auth/policy.py

@@ -0,0 +1,67 @@
+"""Proposal-membership authorization policy (ADR-011).
+
+Authorization is enforced only at the edges: Strawberry permission classes
+for GraphQL fields and a Litestar guard for REST routes, both delegating to
+`require_proposal_member`. Domain services take plain parameters and perform
+no authorization.
+"""
+
+from typing import TYPE_CHECKING
+
+from .. import get_logger
+from ..shared.errors import ForbiddenError
+from ..shared.models import ProposalNumber
+from . import models
+
+if TYPE_CHECKING:
+    from litestar.connection import ASGIConnection
+    from litestar.handlers.base import BaseRouteHandler
+
+logger = get_logger()
+
+
+async def require_proposal_member(
+    user: "models.User",
+    proposal_number: ProposalNumber,
+) -> None:
+    """Raise `ForbiddenError` unless `user` is a member of the proposal."""
+    from ..shared.settings import settings
+
+    if settings.is_local:
+        return
+
+    if proposal_number not in user.proposals:
+        msg = (
+            f"User not authorised for proposal {proposal_number}, or proposal does not "
+            "exist."
+        )
+        await logger.ainfo("Forbidden", message=msg)
+        raise ForbiddenError(msg)
+
+
+async def proposal_member_guard(
+    connection: "ASGIConnection",
+    _: "BaseRouteHandler",
+) -> None:
+    """Litestar guard enforcing proposal membership on REST routes.
+
+    Attached to the proposal-scoped routers in the composition root (ADR-008),
+    so the slices themselves stay authorization-free. Fails closed if a guarded
+    route carries no `proposal_number`.
+    """
+    raw = connection.path_params.get("proposal_number") or connection.query_params.get(
+        "proposal_number"
+    )
+    if raw is None:
+        # A guarded route without the parameter is a wiring bug; fail closed.
+        msg = "Route requires proposal membership but has no proposal_number"
+        raise ForbiddenError(msg)
+    proposal_number = ProposalNumber(int(raw))
+
+    app_state = connection.app.state.app_state
+    async with app_state.db_sessionmaker() as session:
+        user = await models.User.from_connection(
+            connection, app_state.mymdc_client, session
+        )
+
+    await require_proposal_member(user, proposal_number)

api/src/damnit_api/graphql/queries.py

@@ -6,10 +6,10 @@
 from strawberry.types.nodes import SelectedField
 
 from .. import get_logger
-from ..auth.permissions import PROPOSAL_PERMISSIONS
 from ..metadata.services import _get_proposal_meta, _update_proposal_meta
 from ..runs.types import DamnitRun
 from ..shared.models import ProposalNumber
+from ..shared.permissions import PROPOSAL_PERMISSIONS
 from .utils import DatabaseInput
 
 logger = get_logger()

api/src/damnit_api/graphql/subscriptions.py

@@ -7,9 +7,9 @@
 from strawberry.types import Info
 
 from .. import get_logger
-from ..auth.permissions import PROPOSAL_PERMISSIONS
 from ..runs.types import Timestamp
 from ..shared.errors import DataUnavailableError
+from ..shared.permissions import PROPOSAL_PERMISSIONS
 from .publisher import proposal_channel
 from .utils import DatabaseInput
 

api/src/damnit_api/main.py

@@ -1,12 +1,13 @@
 from contextlib import asynccontextmanager
 
-from litestar import Litestar
+from litestar import Litestar, Router
 from litestar.di import Provide
 from litestar.exceptions import HTTPException
 
 from . import contextfile, metadata
 from ._mymdc.dependencies import get_mymdc_client
-from .auth.dependencies import get_oauth_user_info, get_user
+from .auth.dependencies import get_oauth_user_info
+from .auth.policy import proposal_member_guard
 
 # Known paths are redirected to the login page after a 401.
 KNOWN_PATHS = ["/graphql"]
@@ -172,13 +173,23 @@ def _file_store(name: str) -> FileStore:
         auth_controller = auth.OAuthController
         stores = StoreRegistry(default_factory=_file_store)
 
+    # Proposal-membership authorization is enforced at the REST edge (ADR-011)
+    # by a single guard on the proposal-scoped routers; local mode composes it
+    # out (ADR-008), so the slices themselves stay authorization-free.
+    proposal_guards = [] if settings.is_local else [proposal_member_guard]
+
     # ── GraphQL controller ────────────────────────────────────────────────────
     gql_controller = get_gql_controller()
 
+    proposal_router = Router(
+        path="",
+        route_handlers=[metadata.router, contextfile.router],
+        guards=proposal_guards,
+    )
+
     return Litestar(
         route_handlers=[
-            metadata.router,
-            contextfile.router,
+            proposal_router,
             auth_controller,
             gql_controller,
         ],
@@ -191,7 +202,6 @@ def _file_store(name: str) -> FileStore:
             ),
             # The `session` dependency comes from the Advanced Alchemy plugin.
             "mymdc": Provide(get_mymdc_client, sync_to_thread=False),
-            "user": Provide(get_user),
             "oauth_user": Provide(get_oauth_user_info, sync_to_thread=False),
             "channels": Provide(get_channels, sync_to_thread=False),
             "run_update_publisher": Provide(

api/src/damnit_api/metadata/gql.py

@@ -8,10 +8,10 @@
 import strawberry.experimental.pydantic as st_pydantic
 
 from .. import get_logger
-from ..auth.permissions import IsAuthenticated
 from ..shared.models import (
     ProposalNumber,  # noqa: TC001  (Strawberry resolves at runtime)
 )
+from ..shared.permissions import IsAuthenticated
 from . import models, services
 
 if TYPE_CHECKING:

api/src/damnit_api/metadata/routers.py

@@ -5,7 +5,6 @@
 from sqlalchemy.ext.asyncio import AsyncSession
 
 from .._mymdc.dependencies import MyMdCClient
-from ..auth.models import User
 from ..shared.models import ProposalNumber
 from . import services
 from .models import ProposalMeta
@@ -14,11 +13,10 @@
 async def get_proposal_meta(
     proposal_number: ProposalNumber,
     mymdc: MyMdCClient,
-    user: User,
     session: AsyncSession,
 ) -> ProposalMeta:
     """Dependency: resolve ProposalMeta from path/query parameter."""
-    return await services.get_proposal_meta(mymdc, proposal_number, user, session)
+    return await services.get_proposal_meta(mymdc, proposal_number, session)
 
 
 @get("/proposal/{proposal_number:int}", sync_to_thread=False)

api/src/damnit_api/metadata/services.py

@@ -9,7 +9,6 @@
 from anyio import Path as APath
 
 from .. import get_logger
-from ..shared.errors import ForbiddenError
 from ..shared.models import ProposalNumber
 from .models import ProposalMeta, ProposalMetaBase
 from .repository import ProposalMetaRepository
@@ -19,7 +18,6 @@
 if TYPE_CHECKING:
     from .._db.dependencies import DBSession
     from .._mymdc.clients import MyMdCClient
-    from ..auth.dependencies import User
     from ..runs.repository import DamnitRepositoryRegistry
 
 
@@ -162,30 +160,6 @@ async def _search_damnit_dir(path: Path) -> tuple[Path | None, list[Path]]:
     return None, searched_paths
 
 
-async def _check_user_allowed(
-    proposal_number: ProposalNumber,
-    user: "User",
-) -> None:
-    """Check if the user is allowed to access the given proposal number.
-
-    Raises `ForbiddenError` if not allowed.
-    """
-    from ..shared.settings import settings
-
-    if settings.is_local:
-        return
-
-    if proposal_number not in user.proposals:
-        msg = (
-            f"User not authorised for proposal {proposal_number}, or proposal does not "
-            "exist."
-        )
-        await logger.ainfo("Forbidden", message=msg)
-        raise ForbiddenError(msg)
-
-    return
-
-
 async def _get_proposal_meta(
     client: "MyMdCClient",
     proposal_number: ProposalNumber,
@@ -255,7 +229,6 @@ async def _get_proposal_meta_many(
 async def get_proposal_meta(
     client: "MyMdCClient",
     proposal_number: ProposalNumber,
-    user: "User",
     session: "DBSession",
 ) -> ProposalMeta:
     """Get proposal metadata by proposal number, using the repository and/or provided
@@ -265,8 +238,6 @@ async def get_proposal_meta(
     if settings.is_local:
         return _local_proposal_meta(proposal_number)
 
-    await _check_user_allowed(proposal_number, user)
-
     return await _get_proposal_meta(client, proposal_number, session)
 
 
@@ -286,29 +257,20 @@ async def _update_proposal_meta(
 async def update_proposal_meta(
     client: "MyMdCClient",
     proposal_number: ProposalNumber,
-    user: "User",
     session: "DBSession",
 ) -> ProposalMeta:
     """Get proposal metadata by proposal number, using the repository and/or provided
     MyMdC Client."""
-
-    await _check_user_allowed(proposal_number, user)
-
     return await _update_proposal_meta(client, proposal_number, session)
 
 
 async def update_proposal_meta_many(
     client: "MyMdCClient",
     proposal_numbers: list[ProposalNumber],
-    user: "User",
     session: "DBSession",
 ) -> list[ProposalMeta]:
     """Get proposal metadata by proposal number, using the repository and/or provided
     MyMdC Client."""
-
-    for proposal_number in proposal_numbers:
-        await _check_user_allowed(proposal_number, user)
-
     results = []
     for chunk in _chunks(proposal_numbers, n=10):
         new_fetched = await asyncio.gather(

api/src/damnit_api/auth/permissions.py → api/src/damnit_api/shared/permissions.py

@@ -1,12 +1,18 @@
-"""Strawberry permissions for GraphQL authorization."""
+"""Strawberry permissions: transport adapters over the auth policy (ADR-011).
+
+Lives in `shared/` so any slice's GraphQL contribution can attach them without
+importing the `auth` slice (which the import-direction rules forbid). The
+membership decision itself lives in `auth/policy.py`; these classes only adapt
+it to the Strawberry field-permission protocol.
+"""
 
 from strawberry.exceptions import StrawberryGraphQLError
 from strawberry.permission import BasePermission
 from strawberry.types import Info
 
 from .. import get_logger
-from ..metadata.services import _check_user_allowed
-from ..shared.errors import ForbiddenError
+from ..auth.policy import require_proposal_member
+from .errors import ForbiddenError
 
 logger = get_logger()
 
@@ -46,7 +52,7 @@ async def has_permission(self, source, info: Info, **kwargs) -> bool:
             raise StrawberryGraphQLError(msg) from exc
 
         try:
-            await _check_user_allowed(proposal, user)
+            await require_proposal_member(user, proposal)
             return True
         except ForbiddenError:
             return False