You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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`).
10
+
11
+
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.
12
+
13
+
Separately, an unauthenticated GraphQL request produced a 500 rather than a 401.
14
+
15
+
## Considered Options
16
+
17
+
- One policy in `auth/`, enforced only at the transport edges.
18
+
- Keep the check inline in the domain services.
19
+
- A single global authorisation middleware.
20
+
21
+
## Decision Outcome
22
+
23
+
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.
24
+
25
+
`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.
26
+
27
+
Authentication failures now raise `UnauthenticatedError`, which the error handler renders as a 401.
28
+
29
+
### Consequences
30
+
31
+
- Good: domain services are reusable and context-agnostic; the `auth <-> metadata` cycle is broken.
32
+
- Good: authorisation has one auditable choke point per transport.
33
+
- Bad: two edge mechanisms - the GraphQL permission classes and the REST guard - must stay in step.
34
+
- Bad: the permission adapters put Strawberry types in `shared/`, which is otherwise framework-light.
35
+
36
+
## Details
37
+
38
+
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.
39
+
40
+
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.
41
+
42
+
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.
|`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`|
22
22
|`proposals/`| Proposal metadata and lookup | Proposal models, MyMdC-backed metadata services, path locator (see [ADR-004](adr/004-proposal-path-locator.md)) | Planned |`metadata/`|
23
-
|`auth/`| Authentication and authorisation | OAuth flow, sessions, token store, `User`, permission classes, the membership policy | Partial | Policy still in `metadata/services.py`|
23
+
|`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`|
|`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 |
26
26
|`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:
56
56
2.**Composition root is the top:** it may import everything, but nothing is allowed to import it.
57
57
- If importing a slice from the composition root forces a function-body import to avoid cycles, the type probably belongs in `core/`.
58
58
3.**Composition root reads settings:** everything else receives configuration as parameters (see [ADR-003](adr/003-injected-settings.md)).
59
-
4.**Authorisation applied at the edge:** routes and resolvers use dependencies and permission classes.
59
+
4.**Authorisation applied at the edge:** routes and resolvers use dependencies and permission classes (see [ADR-011](adr/011-authorisation-at-the-edge.md)).
60
60
- This means that services should not apply authorisation rules themselves.
61
61
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)).
62
62
63
63
Note that these are currently only enforced by convention/review. Import linter/archetecture check tool is planned to be added.
64
64
65
65
!!! warning "Current issues"
66
66
67
-
- `auth` <--> `metadata` import cycle
68
67
- `shared/gql.py`'s import-everything role
69
68
- Function-body imports working around circular imports
0 commit comments