Skip to content

Commit 43e8622

Browse files
jirhikerclaude
andcommitted
feat(access): write down the access that already exists
The ADR5 tables start empty and empty means default deny, so switching the layer on would take everyone's access away at once. This records what access already is, as grants and consent rows, so the day it starts being consulted nobody's day changes. Data migration `20260829_0001_seed_legacy_access_grants` does both halves: the day-one role baseline from `services/access_seed.py`, and one `publication_consent` row per (`release_status='public'` thing, data type) against each baseline destination. It is one-shot per environment and run by hand, like every data migration here. ## The baseline is five groups, spelled the way the UI spells them `AMP.Admin` > `AMP.Editor` > `AMP.Viewer`, plus `Lexicon.Editor` and `OGC.Internal`. There used to be three orthogonal families gating on names the UI never checked -- a general `Admin`/`Editor`/`Viewer` set, an `AMP*` set, a `Lexicon*` set -- disjoint from `src/utils/accessControl.ts`, which reads the dotted ones. A token satisfying one side could satisfy nothing on the other. `core/dependencies.py` moved with the grants, because gating on one vocabulary while granting in another means no grant ever matches. The `admin_*` and `amp_admin_*` dependencies are now aliases for the same groups, kept so route signatures did not all have to change at once. The two lexicon tiers are one group, which widens what a lexicon editor reaches -- that is the consolidation, not an oversight. This does not rename the groups in Authentik. Until that happens, every role-gated route denies. ## Screens are granted with `view`, not `read` A grant naming a `ui_surface` carries `view`; the data verbs stay with `data_type`. The pairing is enforced in `domain/access.py` rather than left as a convention, because `read` over a screen would be a second spelling of the same permission, and two spellings are two things that can disagree in a listing. ## Consent is grandfathered per destination, and the destinations differ `public-web` (kind `public web`) receives all four data types, because that is what `release_status='public'` already meant. `ngwmn` (kind `harvester`) receives everything except water chemistry: the harvester was never offered it, and being able to say so is the point of the per-data-type model. The kind is load-bearing -- it picks the audience in `core/field-allowlists.yml`, where public web rounds coordinates to a kilometre and a harvester to ten metres. So the migration refuses to write against a slug already registered under a different kind rather than publishing under one nobody chose. This takes the grandfathering branch of PUB-D13 for the legacy data. Every narrowing from here is a revocation somebody makes deliberately, per data type, which is what the model exists to make expressible. ## Also `GET /access/grant` is paginated, ordered by id so pages cannot overlap or skip. The admin-wide view is dozens of rows before anybody grants anything by hand. This changes the response from a bare array to the standard page envelope. Verified against a 2026-07-30 production clone: 9,848 public things, 68,936 consent rows, 61 grants, one audit row per write, and a second run that creates nothing. The lexicon guard caught the real prerequisite there -- alembic never seeds lexicon terms, and every access vocabulary is a foreign key to `lexicon_term.term`, so `oco initialize-lexicon` runs first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent f986b90 commit 43e8622

20 files changed

Lines changed: 1103 additions & 131 deletions

.github/skills/code-review/SKILL.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ router-level `dependencies=[...]`. Two failure modes to flag:
2626
value (`user=viewer_dependency`). The latter silently disables the dependency, and FastAPI
2727
reinterprets it as a query parameter. Flag this every time.
2828

29-
Role families are orthogonal: general `Admin` confers nothing in the `AMP*` or `Lexicon*`
30-
families. Only tiers within one family nest. A diff that treats `Admin` as a superset of
31-
`AMPEditor` is wrong.
29+
The role groups are `AMP.Admin` > `AMP.Editor` > `AMP.Viewer`, one ladder, plus
30+
`Lexicon.Editor`, `OGC.Internal` and `AMP.Staging` outside it. A diff that treats
31+
`AMP.Admin` as a superset of `Lexicon.Editor` or `AMP.Staging` is wrong.
3232

3333
`@in_public_schema` controls anonymous OpenAPI visibility only. It grants no access and
3434
removes no dependency; flag any use that appears to be standing in for authorization.

ADR5.md

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ for merging those tables into the access-control model.
7575
### Principals are already more than users
7676

7777
`/ogcapi-internal` is gated at the ASGI layer by `core/internal_ogc_auth.py`,
78-
which accepts an Authentik JWT carrying `OGCInternal` **or** a static API key,
78+
which accepts an Authentik JWT carrying `OGC.Internal` **or** a static API key,
7979
because ArcGIS Pro cannot present a bearer token. That key is a principal with
8080
no user behind it, and today its scope is "everything the internal mount
8181
serves." Any model that assumes principal == person is already wrong here.
@@ -222,11 +222,28 @@ told.
222222
`entered_by`.
223223
5. **Per environment, by hand:** `oco seed-access-grants` writes the day-one
224224
role baseline (ADR5, 5.2) -- one global grant per Authentik role,
225-
capability and data type, so nobody's access changes when the layer starts
226-
being consulted. Until it runs, that environment denies everyone.
227-
Idempotent, and it does not resurrect a revoked seeded grant.
225+
capability and data type, plus one per role and UI surface, so nobody's
226+
access changes when the layer starts being consulted. Until it runs, that
227+
environment denies everyone. Idempotent, and it does not resurrect a
228+
revoked seeded grant.
229+
230+
Data migration `20260829_0001_seed_legacy_access_grants` runs that seeder
231+
and grandfathers the consent half in the same pass: every thing carrying
232+
`release_status='public'` gets a consent row per access data type against
233+
each baseline destination — `public-web` and `ngwmn`. Both, because
234+
`release_status` never distinguished them: a public well was in the OGC
235+
collections and in what NGWMN harvests, and there was no way to say yes to
236+
one and no to the other. There is now, and saying no is a revocation. **That takes the grandfathering branch of
237+
PUB-D13** (below) for the legacy data, on the reading that
238+
`release_status='public'` already publishes everything about a record, so
239+
the widest consent is the one that describes what is true today. Narrowing
240+
it is a revocation somebody makes deliberately, per data type, in the
241+
console. Data migrations have no CD path, so this too is run by hand per
242+
environment.
228243
6. Console administration.
229-
7. Healy migration, after the data owner decides grandfathering.
244+
7. Healy migration, after the data owner decides grandfathering. The legacy
245+
rows are grandfathered by step 5; what remains is whether Healy's wells are
246+
re-consented per data type rather than inheriting that.
230247

231248
## References
232249

CLAUDE.md

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -157,20 +157,27 @@ place. Read **`ADR4.md`** before extending the layer.
157157
The system uses **Authentik** for OAuth2 authentication with role-based access control:
158158

159159
**Permission Levels** (defined in `core/dependencies.py`):
160-
- **Viewer**: Read-only access to all public entities
161-
- **Editor**: Can modify existing records (includes Viewer permissions)
162-
- **Admin**: Can create new records (includes Editor + Viewer permissions)
160+
- **`AMP.Viewer`**: Read-only access to all public entities
161+
- **`AMP.Editor`**: Can modify existing records (includes Viewer permissions)
162+
- **`AMP.Admin`**: Can create new records (includes Editor + Viewer permissions)
163163

164164
The hierarchy is enforced in code, via `authenticated(any_of=[...])` group lists —
165-
`Admin` satisfies an editor- or viewer-gated route without needing all three
165+
`AMP.Admin` satisfies an editor- or viewer-gated route without needing all three
166166
Authentik groups granted.
167167

168-
**AMP-Specific Roles**: `AMPAdmin`, `AMPEditor`, `AMPViewer` for legacy AMPAPI integration
168+
**One ladder, dotted names.** There used to be three families gating on names
169+
the UI never checked (`Admin`/`Editor`/`Viewer`, `AMP*`, `Lexicon*`). They are
170+
consolidated into the dotted groups the UI already reads: `AMP.Admin`,
171+
`AMP.Editor`, `AMP.Viewer`, plus `Lexicon.Editor` and `OGC.Internal`. The
172+
`admin_*` and `amp_admin_*` dependencies are now aliases for the same groups —
173+
either spelling works on a route, and neither is a separate tier.
174+
`Lexicon.Editor` is the only lexicon group, so what required lexicon *admin* is
175+
now reachable by a lexicon editor.
169176

170-
**Role families are orthogonal**: general `Admin` confers nothing in the AMP or
171-
Lexicon families. Only tiers *within* a family nest.
177+
**Vocabulary and the GIS mount stay outside the ladder**: `AMP.Admin` confers
178+
nothing in `Lexicon.Editor`, `OGC.Internal`, or `AMP.Staging`.
172179

173-
**`AMP.Staging`** is a standalone group, not a fourth AMP tier — `AMPAdmin`
180+
**`AMP.Staging`** is a standalone group, not a fourth AMP tier — `AMP.Admin`
174181
does not satisfy it. It gates the hydrograph corrector's publish and range-delete
175182
routes while the workbench is being validated against real logger files, so they
176183
ship dark. Read **`docs/hydrograph-correction-publish.md`** before changing
@@ -194,7 +201,7 @@ that genuinely have none.
194201

195202
**`/ogcapi-internal` is gated outside `Depends()`.** It is a raw Starlette
196203
Mount, so `core/internal_ogc_auth.py` gates it at the ASGI layer instead. It
197-
accepts a bearer Authentik JWT carrying `OGCInternal`, **or** a static API key
204+
accepts a bearer Authentik JWT carrying `OGC.Internal`, **or** a static API key
198205
presented as a bearer token, as the Basic password, or as `?token=`. Only the
199206
key digests are stored, as `label:sha256hex` entries in `INTERNAL_OGC_API_KEYS`
200207
— sourced in deployed environments from the Secret Manager secret
@@ -246,10 +253,21 @@ The storage and the evaluator exist; the field projection does not.
246253
**`docs/access-field-projection.md`** before touching the allowlists.
247254
- **The role baseline is seeded by hand, per environment.**
248255
`oco seed-access-grants` writes one global grant per (Authentik role,
249-
capability, data type) so today's roles keep today's access; it previews by
250-
default and needs `--apply` to write. Idempotent, and it will not resurrect
251-
a seeded grant somebody revoked, because narrowing the baseline is the point.
252-
Until it is run in an environment, `/access/decision` denies everyone there.
256+
capability, data type) and one per (role, UI surface) for the five groups
257+
that exist — `AMP.Admin`, `AMP.Editor`, `AMP.Viewer`, `OGC.Internal`,
258+
`Lexicon.Editor` — so today's roles keep today's access; it previews by default and needs `--apply` to write.
259+
Idempotent, and it will not resurrect a seeded grant somebody revoked,
260+
because narrowing the baseline is the point. Until it is run in an
261+
environment, `/access/decision` denies everyone there.
262+
- **Legacy publication is grandfathered, not re-consented.** Data migration
263+
`20260829_0001_seed_legacy_access_grants` runs that seeder and writes one
264+
`publication_consent` row per (`release_status='public'` thing, access data
265+
type) against each baseline destination — `public-web` (kind `public web`)
266+
and `ngwmn` (kind `harvester`) — because that is what the column already
267+
meant for both. The kind is not decoration: it picks the audience in
268+
`core/field-allowlists.yml`, so the migration refuses to write against a
269+
destination already registered under a different kind. One-shot per
270+
environment and run by hand like every data migration here.
253271
- **Default deny, no wildcards, expiry at use.** A grant with no matching row
254272
is a no; a grant names its `data_type` (there is no term meaning "all"); and
255273
nothing sweeps expired rows, so every check compares against the date asked

api/access.py

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,11 @@
3838
from datetime import date
3939

4040
from fastapi import APIRouter, Query
41+
from fastapi_pagination.ext.sqlalchemy import paginate
4142
from sqlalchemy import select
4243
from starlette.status import HTTP_201_CREATED
4344

45+
from api.pagination import CustomPage
4446
from core.dependencies import (
4547
admin_dependency,
4648
session_dependency,
@@ -195,11 +197,16 @@ def get_permission_grants(
195197
ui_surface: str = Query(default=None),
196198
scope_type: str = Query(default=None),
197199
include_revoked: bool = Query(default=False),
198-
) -> list[PermissionGrantResponse]:
200+
) -> CustomPage[PermissionGrantResponse]:
199201
"""All grants, or a narrower slice of them.
200202
201203
Every filter is optional, so the bare route is the admin-wide audit view;
202204
passing ``principal_id`` narrows it to one principal, as before.
205+
206+
Paginated, because the admin-wide view is not small: the day-one baseline
207+
alone is dozens of rows before anybody grants anything by hand, and a
208+
console that loads every grant to show twenty-five is a page that gets
209+
slower every time somebody uses the system correctly.
203210
"""
204211
statement = select(PermissionGrant)
205212
if principal_id is not None:
@@ -215,10 +222,12 @@ def get_permission_grants(
215222
if not include_revoked:
216223
statement = statement.where(PermissionGrant.revoked_at.is_(None))
217224

218-
return [
219-
PermissionGrantResponse.model_validate(row)
220-
for row in session.execute(statement).scalars()
221-
]
225+
# Ordered so paging is stable. Without it Postgres may return rows in a
226+
# different order per page and the console would show one grant twice and
227+
# another never.
228+
statement = statement.order_by(PermissionGrant.id)
229+
230+
return paginate(query=statement, conn=session)
222231

223232

224233
@router.get("/decision", summary="Ask the visibility layer about yourself")

api/gis_artifacts.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
2424
The internal connection file is gated, not because the file is secret, but
2525
because the internal mount's existence is not something to advertise to
26-
anonymous callers. Holding it still gets you nothing without an `OGCInternal`
26+
anonymous callers. Holding it still gets you nothing without an `OGC.Internal`
2727
API key.
2828
2929
Read docs/ogc-desktop-gis-artifacts.md before changing what is emitted.
@@ -159,7 +159,7 @@ def qgis_connections_internal(user: viewer_dependency) -> Response:
159159
"""QGIS connections file covering the public and internal mounts.
160160
161161
Carries no credential. The internal entry only resolves for a client that
162-
attaches its own `OGCInternal` API key -- see
162+
attaches its own `OGC.Internal` API key -- see
163163
docs/internal-ogc-desktop-gis.md for how one is issued and attached.
164164
"""
165165
body = qgis_connections_xml(
@@ -229,7 +229,8 @@ def gis_index(request: Request, f: Annotated[str | None, Query()] = None) -> Res
229229
f'<td><a href="arcgis/layers/{layer.id}.lyrx">.lyrx</a></td></tr>'
230230
for layer in load_curated_layers()
231231
)
232-
return HTMLResponse(f"""<!doctype html><html><head><meta charset="utf-8">
232+
return HTMLResponse(
233+
f"""<!doctype html><html><head><meta charset="utf-8">
233234
<title>Desktop GIS downloads</title></head>
234235
<body style="{_PAGE_STYLE}">
235236
<h1>Using our OGC layers in QGIS and ArcGIS Pro</h1>
@@ -260,7 +261,8 @@ def gis_index(request: Request, f: Annotated[str | None, Query()] = None) -> Res
260261
<code>{base}/collections/water_chemistry</code>. Neither QGIS nor ArcGIS Pro
261262
can read EDR, so the layers above carry the same measurements summarised per
262263
site instead.</p>
263-
</body></html>""")
264+
</body></html>"""
265+
)
264266

265267

266268
# ============= EOF =============================================

core/dependencies.py

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -35,35 +35,55 @@
3535
+ create new objects
3636
3737
That hierarchy is enforced here, by `any_of=` group lists rather than by
38-
Authentik group membership overlap: an Admin-only account satisfies an
39-
editor- or viewer-gated route because "Admin" appears in those lists. Before
40-
this was explicit, `authenticated(permissions=["Viewer"])` required the
41-
literal Viewer group, so the hierarchy held only as long as whoever
42-
provisioned the Authentik groups granted all three tiers to every admin.
43-
44-
The three families below are deliberately orthogonal -- general `Admin` does
45-
not confer `AMPAdmin` or `LexiconAdmin`. Only tiers *within* a family nest.
38+
Authentik group membership overlap: an admin account satisfies an editor- or
39+
viewer-gated route because `AMP.Admin` appears in those lists. Before this was
40+
explicit, `authenticated(permissions=["Viewer"])` required the literal Viewer
41+
group, so the hierarchy held only as long as whoever provisioned the Authentik
42+
groups granted all three tiers to every admin.
43+
44+
## One family, dotted names
45+
46+
There used to be three orthogonal families -- a general `Admin`/`Editor`/
47+
`Viewer` set, an `AMP*` set, and a `Lexicon*` set -- gating on names the UI
48+
never checked (`src/utils/accessControl.ts` reads `AMP.Viewer` and friends).
49+
The two vocabularies were disjoint, which meant a token satisfying one side
50+
could satisfy nothing on the other.
51+
52+
The groups are now the dotted ones, and the general family is the AMP family:
53+
`AMP.Admin`, `AMP.Editor`, `AMP.Viewer`, plus `Lexicon.Editor` for vocabulary
54+
and `OGC.Internal` for the desktop-GIS mount. The `admin_*` and `amp_admin_*`
55+
dependencies therefore resolve to the same groups. Both spellings are kept so
56+
route signatures did not all have to change in one commit; they are aliases,
57+
not tiers, and a route may use either.
58+
59+
`Lexicon.Editor` is the only lexicon group: what were the `LexiconAdmin` and
60+
`LexiconEditor` tiers are one group now, so a lexicon editor reaches the
61+
routes that used to require lexicon admin. That is a widening, and it is the
62+
consolidation, not an accident.
63+
64+
`AMP.Staging` keeps its own spelling and stays outside the ladder.
4665
"""
4766

4867
# General Purpose Authentication/Permissions -----------------------------------
68+
# Aliases for the AMP groups below; see the module docstring.
4969

50-
admin_function = authenticated(any_of=["Admin"])
51-
editor_function = authenticated(any_of=["Admin", "Editor"])
52-
viewer_function = authenticated(any_of=["Admin", "Editor", "Viewer"])
70+
admin_function = authenticated(any_of=["AMP.Admin"])
71+
editor_function = authenticated(any_of=["AMP.Admin", "AMP.Editor"])
72+
viewer_function = authenticated(any_of=["AMP.Admin", "AMP.Editor", "AMP.Viewer"])
5373

5474

5575
# AMP-Specific Authentication/Permissions --------------------------------------
5676

57-
amp_admin_function = authenticated(any_of=["AMPAdmin"])
58-
amp_editor_function = authenticated(any_of=["AMPAdmin", "AMPEditor"])
59-
amp_viewer_function = authenticated(any_of=["AMPAdmin", "AMPEditor", "AMPViewer"])
77+
amp_admin_function = authenticated(any_of=["AMP.Admin"])
78+
amp_editor_function = authenticated(any_of=["AMP.Admin", "AMP.Editor"])
79+
amp_viewer_function = authenticated(any_of=["AMP.Admin", "AMP.Editor", "AMP.Viewer"])
6080

6181

6282
# Hydrograph-Corrector Staging Permissions -------------------------------------
6383
# The hydrograph corrector's publish and range-delete routes write and destroy
6484
# transducer records, and the workbench driving them is still being validated
6585
# against real logger files. `AMP.Staging` is its own group with no tier below
66-
# it and no AMP tier above it -- an AMPAdmin does not satisfy it. Nobody holds
86+
# it and no AMP tier above it -- an `AMP.Admin` does not satisfy it. Nobody holds
6787
# it until it is granted in Authentik, so the routes ship dark and reachable
6888
# only by whoever is testing them.
6989
#
@@ -76,12 +96,12 @@
7696

7797
# Lexicon-Specific Authentication/Permissions ----------------------------------
7898

79-
lexicon_admin_function = authenticated(any_of=["LexiconAdmin"])
80-
lexicon_editor_function = authenticated(any_of=["LexiconAdmin", "LexiconEditor"])
99+
lexicon_admin_function = authenticated(any_of=["Lexicon.Editor"])
100+
lexicon_editor_function = authenticated(any_of=["Lexicon.Editor"])
81101

82102

83103
# OGC-Internal Authentication/Permissions --------------------------------------
84-
# INTERNAL_OGC_GROUP ("OGCInternal") lives in core/permissions.py, not here --
104+
# INTERNAL_OGC_GROUP ("OGC.Internal") lives in core/permissions.py, not here --
85105
# it gates core/internal_ogc_auth.py's ASGI middleware in front of the
86106
# /ogcapi-internal mount, which runs outside FastAPI's Depends() machinery.
87107

core/lexicon.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8583,6 +8583,13 @@
85838583
"term": "administer",
85848584
"definition": "May grant and revoke access."
85858585
},
8586+
{
8587+
"categories": [
8588+
"capability"
8589+
],
8590+
"term": "view",
8591+
"definition": "May open a screen. The capability a ui_surface grant carries; data is reached with read."
8592+
},
85868593
{
85878594
"categories": [
85888595
"grant_scope_type"

core/permissions.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ class TokenInvalid(Exception):
275275
# Required Authentik group for the authenticated internal OGC mount
276276
# (/ogcapi-internal). Not Depends()-shaped like the roles above -- see the
277277
# cross-reference note in core/dependencies.py for why it still lives here.
278-
INTERNAL_OGC_GROUP = "OGCInternal"
278+
INTERNAL_OGC_GROUP = "OGC.Internal"
279279

280280

281281
def decode_token_payload(token: str) -> dict:

0 commit comments

Comments
 (0)