diff --git a/.github/skills/code-review/SKILL.md b/.github/skills/code-review/SKILL.md index 723969783..b81fb51b1 100644 --- a/.github/skills/code-review/SKILL.md +++ b/.github/skills/code-review/SKILL.md @@ -26,9 +26,9 @@ router-level `dependencies=[...]`. Two failure modes to flag: value (`user=viewer_dependency`). The latter silently disables the dependency, and FastAPI reinterprets it as a query parameter. Flag this every time. -Role families are orthogonal: general `Admin` confers nothing in the `AMP*` or `Lexicon*` -families. Only tiers within one family nest. A diff that treats `Admin` as a superset of -`AMPEditor` is wrong. +The role groups are `AMP.Admin` > `AMP.Editor` > `AMP.Viewer`, one ladder, plus +`Lexicon.Editor`, `OGC.Internal` and `AMP.Staging` outside it. A diff that treats +`AMP.Admin` as a superset of `Lexicon.Editor` or `AMP.Staging` is wrong. `@in_public_schema` controls anonymous OpenAPI visibility only. It grants no access and removes no dependency; flag any use that appears to be standing in for authorization. diff --git a/ADR5.md b/ADR5.md index 536399163..8c0d21de6 100644 --- a/ADR5.md +++ b/ADR5.md @@ -75,7 +75,7 @@ for merging those tables into the access-control model. ### Principals are already more than users `/ogcapi-internal` is gated at the ASGI layer by `core/internal_ogc_auth.py`, -which accepts an Authentik JWT carrying `OGCInternal` **or** a static API key, +which accepts an Authentik JWT carrying `OGC.Internal` **or** a static API key, because ArcGIS Pro cannot present a bearer token. That key is a principal with no user behind it, and today its scope is "everything the internal mount serves." Any model that assumes principal == person is already wrong here. @@ -101,8 +101,9 @@ second axis, because some destinations want approved data only. ### 1. Two tables **`permission_grant`** — internal authorization. Principal (user subject, role, -API key), capability (read, enter, correct, administer), scope (project, thing, -data type, field group), time bounds, `granted_by`, `granted_at`, `reason`. +API key), capability (read, enter, correct, delete, administer), scope +(project, thing, data type, field group), time bounds, `granted_by`, +`granted_at`, `reason`. Governed by data services staff. Answers "is this person trusted with this." **`publication_consent`** — landowner-facing publication. One row per @@ -222,11 +223,28 @@ told. `entered_by`. 5. **Per environment, by hand:** `oco seed-access-grants` writes the day-one role baseline (ADR5, 5.2) -- one global grant per Authentik role, - capability and data type, so nobody's access changes when the layer starts - being consulted. Until it runs, that environment denies everyone. - Idempotent, and it does not resurrect a revoked seeded grant. + capability and data type, plus one per role and UI surface, so nobody's + access changes when the layer starts being consulted. Until it runs, that + environment denies everyone. Idempotent, and it does not resurrect a + revoked seeded grant. + + Data migration `20260829_0001_seed_legacy_access_grants` runs that seeder + and grandfathers the consent half in the same pass: every thing carrying + `release_status='public'` gets a consent row per access data type against + each baseline destination — `public-web` and `ngwmn`. Both, because + `release_status` never distinguished them: a public well was in the OGC + collections and in what NGWMN harvests, and there was no way to say yes to + one and no to the other. There is now, and saying no is a revocation. **That takes the grandfathering branch of + PUB-D13** (below) for the legacy data, on the reading that + `release_status='public'` already publishes everything about a record, so + the widest consent is the one that describes what is true today. Narrowing + it is a revocation somebody makes deliberately, per data type, in the + console. Data migrations have no CD path, so this too is run by hand per + environment. 6. Console administration. -7. Healy migration, after the data owner decides grandfathering. +7. Healy migration, after the data owner decides grandfathering. The legacy + rows are grandfathered by step 5; what remains is whether Healy's wells are + re-consented per data type rather than inheriting that. ## References diff --git a/CLAUDE.md b/CLAUDE.md index 7a65b1fde..da4c56600 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -157,20 +157,27 @@ place. Read **`ADR4.md`** before extending the layer. The system uses **Authentik** for OAuth2 authentication with role-based access control: **Permission Levels** (defined in `core/dependencies.py`): -- **Viewer**: Read-only access to all public entities -- **Editor**: Can modify existing records (includes Viewer permissions) -- **Admin**: Can create new records (includes Editor + Viewer permissions) +- **`AMP.Viewer`**: Read-only access to all public entities +- **`AMP.Editor`**: Can modify existing records (includes Viewer permissions) +- **`AMP.Admin`**: Can create new records (includes Editor + Viewer permissions) The hierarchy is enforced in code, via `authenticated(any_of=[...])` group lists — -`Admin` satisfies an editor- or viewer-gated route without needing all three +`AMP.Admin` satisfies an editor- or viewer-gated route without needing all three Authentik groups granted. -**AMP-Specific Roles**: `AMPAdmin`, `AMPEditor`, `AMPViewer` for legacy AMPAPI integration +**One ladder, dotted names.** There used to be three families gating on names +the UI never checked (`Admin`/`Editor`/`Viewer`, `AMP*`, `Lexicon*`). They are +consolidated into the dotted groups the UI already reads: `AMP.Admin`, +`AMP.Editor`, `AMP.Viewer`, plus `Lexicon.Editor` and `OGC.Internal`. The +`admin_*` and `amp_admin_*` dependencies are now aliases for the same groups — +either spelling works on a route, and neither is a separate tier. +`Lexicon.Editor` is the only lexicon group, so what required lexicon *admin* is +now reachable by a lexicon editor. -**Role families are orthogonal**: general `Admin` confers nothing in the AMP or -Lexicon families. Only tiers *within* a family nest. +**Vocabulary and the GIS mount stay outside the ladder**: `AMP.Admin` confers +nothing in `Lexicon.Editor`, `OGC.Internal`, or `AMP.Staging`. -**`AMP.Staging`** is a standalone group, not a fourth AMP tier — `AMPAdmin` +**`AMP.Staging`** is a standalone group, not a fourth AMP tier — `AMP.Admin` does not satisfy it. It gates the hydrograph corrector's publish and range-delete routes while the workbench is being validated against real logger files, so they ship dark. Read **`docs/hydrograph-correction-publish.md`** before changing @@ -194,7 +201,7 @@ that genuinely have none. **`/ogcapi-internal` is gated outside `Depends()`.** It is a raw Starlette Mount, so `core/internal_ogc_auth.py` gates it at the ASGI layer instead. It -accepts a bearer Authentik JWT carrying `OGCInternal`, **or** a static API key +accepts a bearer Authentik JWT carrying `OGC.Internal`, **or** a static API key presented as a bearer token, as the Basic password, or as `?token=`. Only the key digests are stored, as `label:sha256hex` entries in `INTERNAL_OGC_API_KEYS` — sourced in deployed environments from the Secret Manager secret @@ -246,10 +253,21 @@ The storage and the evaluator exist; the field projection does not. **`docs/access-field-projection.md`** before touching the allowlists. - **The role baseline is seeded by hand, per environment.** `oco seed-access-grants` writes one global grant per (Authentik role, - capability, data type) so today's roles keep today's access; it previews by - default and needs `--apply` to write. Idempotent, and it will not resurrect - a seeded grant somebody revoked, because narrowing the baseline is the point. - Until it is run in an environment, `/access/decision` denies everyone there. + capability, data type) and one per (role, UI surface) for the five groups + that exist — `AMP.Admin`, `AMP.Editor`, `AMP.Viewer`, `OGC.Internal`, + `Lexicon.Editor` — so today's roles keep today's access; it previews by default and needs `--apply` to write. + Idempotent, and it will not resurrect a seeded grant somebody revoked, + because narrowing the baseline is the point. Until it is run in an + environment, `/access/decision` denies everyone there. +- **Legacy publication is grandfathered, not re-consented.** Data migration + `20260829_0001_seed_legacy_access_grants` runs that seeder and writes one + `publication_consent` row per (`release_status='public'` thing, access data + type) against each baseline destination — `public-web` (kind `public web`) + and `ngwmn` (kind `harvester`) — because that is what the column already + meant for both. The kind is not decoration: it picks the audience in + `core/field-allowlists.yml`, so the migration refuses to write against a + destination already registered under a different kind. One-shot per + environment and run by hand like every data migration here. - **Default deny, no wildcards, expiry at use.** A grant with no matching row is a no; a grant names its `data_type` (there is no term meaning "all"); and nothing sweeps expired rows, so every check compares against the date asked diff --git a/api/access.py b/api/access.py index 7bdf54dd9..691cb637e 100644 --- a/api/access.py +++ b/api/access.py @@ -38,9 +38,11 @@ from datetime import date from fastapi import APIRouter, Query +from fastapi_pagination.ext.sqlalchemy import paginate from sqlalchemy import select from starlette.status import HTTP_201_CREATED +from api.pagination import CustomPage from core.dependencies import ( admin_dependency, session_dependency, @@ -70,8 +72,10 @@ ) from domain.access import ( AmbiguousGrantSubject, + GRANT_SUBJECTS, MissingDataType, ScopedSurfaceGrant, + SUBJECT_UI_SURFACE, ) from services.exceptions_helper import PydanticStyleException from services.visibility import ( @@ -193,13 +197,27 @@ def get_permission_grants( capability: str = Query(default=None), data_type: str = Query(default=None), ui_surface: str = Query(default=None), + subject: str = Query( + default=None, + description="Which kind of grant: 'data_type' or 'ui_surface'", + ), scope_type: str = Query(default=None), include_revoked: bool = Query(default=False), -) -> list[PermissionGrantResponse]: +) -> CustomPage[PermissionGrantResponse]: """All grants, or a narrower slice of them. Every filter is optional, so the bare route is the admin-wide audit view; passing ``principal_id`` narrows it to one principal, as before. + + ``data_type`` and ``ui_surface`` name one subject exactly. ``subject`` asks + the coarser question -- which kind of grant, not which one -- because a + console offering "screens only" cannot ask it by naming every screen, and + filtering a page client-side would filter a page rather than the set. + + Paginated, because the admin-wide view is not small: the day-one baseline + alone is dozens of rows before anybody grants anything by hand, and a + console that loads every grant to show twenty-five is a page that gets + slower every time somebody uses the system correctly. """ statement = select(PermissionGrant) if principal_id is not None: @@ -210,15 +228,32 @@ def get_permission_grants( statement = statement.where(PermissionGrant.data_type == data_type) if ui_surface is not None: statement = statement.where(PermissionGrant.ui_surface == ui_surface) + if subject is not None: + if subject not in GRANT_SUBJECTS: + raise _invalid( + "subject", + f"A grant covers a data type or a UI surface. Use one of " + f"{', '.join(sorted(GRANT_SUBJECTS))}.", + subject, + ) + # Exactly one of the two columns is set on any row, so "names a screen" + # and "names a data type" are the same question asked either way round. + statement = statement.where( + PermissionGrant.ui_surface.is_not(None) + if subject == SUBJECT_UI_SURFACE + else PermissionGrant.ui_surface.is_(None) + ) if scope_type is not None: statement = statement.where(PermissionGrant.scope_type == scope_type) if not include_revoked: statement = statement.where(PermissionGrant.revoked_at.is_(None)) - return [ - PermissionGrantResponse.model_validate(row) - for row in session.execute(statement).scalars() - ] + # Ordered so paging is stable. Without it Postgres may return rows in a + # different order per page and the console would show one grant twice and + # another never. + statement = statement.order_by(PermissionGrant.id) + + return paginate(query=statement, conn=session) @router.get("/decision", summary="Ask the visibility layer about yourself") diff --git a/api/gis_artifacts.py b/api/gis_artifacts.py index d815f0566..ffcd85f44 100644 --- a/api/gis_artifacts.py +++ b/api/gis_artifacts.py @@ -23,7 +23,7 @@ The internal connection file is gated, not because the file is secret, but because the internal mount's existence is not something to advertise to -anonymous callers. Holding it still gets you nothing without an `OGCInternal` +anonymous callers. Holding it still gets you nothing without an `OGC.Internal` API key. Read docs/ogc-desktop-gis-artifacts.md before changing what is emitted. @@ -159,7 +159,7 @@ def qgis_connections_internal(user: viewer_dependency) -> Response: """QGIS connections file covering the public and internal mounts. Carries no credential. The internal entry only resolves for a client that - attaches its own `OGCInternal` API key -- see + attaches its own `OGC.Internal` API key -- see docs/internal-ogc-desktop-gis.md for how one is issued and attached. """ body = qgis_connections_xml( diff --git a/core/dependencies.py b/core/dependencies.py index 95d11f3c8..2a00cfe39 100644 --- a/core/dependencies.py +++ b/core/dependencies.py @@ -35,35 +35,55 @@ + create new objects That hierarchy is enforced here, by `any_of=` group lists rather than by -Authentik group membership overlap: an Admin-only account satisfies an -editor- or viewer-gated route because "Admin" appears in those lists. Before -this was explicit, `authenticated(permissions=["Viewer"])` required the -literal Viewer group, so the hierarchy held only as long as whoever -provisioned the Authentik groups granted all three tiers to every admin. - -The three families below are deliberately orthogonal -- general `Admin` does -not confer `AMPAdmin` or `LexiconAdmin`. Only tiers *within* a family nest. +Authentik group membership overlap: an admin account satisfies an editor- or +viewer-gated route because `AMP.Admin` appears in those lists. Before this was +explicit, `authenticated(permissions=["Viewer"])` required the literal Viewer +group, so the hierarchy held only as long as whoever provisioned the Authentik +groups granted all three tiers to every admin. + +## One family, dotted names + +There used to be three orthogonal families -- a general `Admin`/`Editor`/ +`Viewer` set, an `AMP*` set, and a `Lexicon*` set -- gating on names the UI +never checked (`src/utils/accessControl.ts` reads `AMP.Viewer` and friends). +The two vocabularies were disjoint, which meant a token satisfying one side +could satisfy nothing on the other. + +The groups are now the dotted ones, and the general family is the AMP family: +`AMP.Admin`, `AMP.Editor`, `AMP.Viewer`, plus `Lexicon.Editor` for vocabulary +and `OGC.Internal` for the desktop-GIS mount. The `admin_*` and `amp_admin_*` +dependencies therefore resolve to the same groups. Both spellings are kept so +route signatures did not all have to change in one commit; they are aliases, +not tiers, and a route may use either. + +`Lexicon.Editor` is the only lexicon group: what were the `LexiconAdmin` and +`LexiconEditor` tiers are one group now, so a lexicon editor reaches the +routes that used to require lexicon admin. That is a widening, and it is the +consolidation, not an accident. + +`AMP.Staging` keeps its own spelling and stays outside the ladder. """ # General Purpose Authentication/Permissions ----------------------------------- +# Aliases for the AMP groups below; see the module docstring. -admin_function = authenticated(any_of=["Admin"]) -editor_function = authenticated(any_of=["Admin", "Editor"]) -viewer_function = authenticated(any_of=["Admin", "Editor", "Viewer"]) +admin_function = authenticated(any_of=["AMP.Admin"]) +editor_function = authenticated(any_of=["AMP.Admin", "AMP.Editor"]) +viewer_function = authenticated(any_of=["AMP.Admin", "AMP.Editor", "AMP.Viewer"]) # AMP-Specific Authentication/Permissions -------------------------------------- -amp_admin_function = authenticated(any_of=["AMPAdmin"]) -amp_editor_function = authenticated(any_of=["AMPAdmin", "AMPEditor"]) -amp_viewer_function = authenticated(any_of=["AMPAdmin", "AMPEditor", "AMPViewer"]) +amp_admin_function = authenticated(any_of=["AMP.Admin"]) +amp_editor_function = authenticated(any_of=["AMP.Admin", "AMP.Editor"]) +amp_viewer_function = authenticated(any_of=["AMP.Admin", "AMP.Editor", "AMP.Viewer"]) # Hydrograph-Corrector Staging Permissions ------------------------------------- # The hydrograph corrector's publish and range-delete routes write and destroy # transducer records, and the workbench driving them is still being validated # against real logger files. `AMP.Staging` is its own group with no tier below -# it and no AMP tier above it -- an AMPAdmin does not satisfy it. Nobody holds +# it and no AMP tier above it -- an `AMP.Admin` does not satisfy it. Nobody holds # it until it is granted in Authentik, so the routes ship dark and reachable # only by whoever is testing them. # @@ -76,12 +96,12 @@ # Lexicon-Specific Authentication/Permissions ---------------------------------- -lexicon_admin_function = authenticated(any_of=["LexiconAdmin"]) -lexicon_editor_function = authenticated(any_of=["LexiconAdmin", "LexiconEditor"]) +lexicon_admin_function = authenticated(any_of=["Lexicon.Editor"]) +lexicon_editor_function = authenticated(any_of=["Lexicon.Editor"]) # OGC-Internal Authentication/Permissions -------------------------------------- -# INTERNAL_OGC_GROUP ("OGCInternal") lives in core/permissions.py, not here -- +# INTERNAL_OGC_GROUP ("OGC.Internal") lives in core/permissions.py, not here -- # it gates core/internal_ogc_auth.py's ASGI middleware in front of the # /ogcapi-internal mount, which runs outside FastAPI's Depends() machinery. diff --git a/core/lexicon.json b/core/lexicon.json index 761e783b0..667ffcd27 100644 --- a/core/lexicon.json +++ b/core/lexicon.json @@ -8583,6 +8583,20 @@ "term": "administer", "definition": "May grant and revoke access." }, + { + "categories": [ + "capability" + ], + "term": "view", + "definition": "May open a screen. The capability a ui_surface grant carries; data is reached with read." + }, + { + "categories": [ + "capability" + ], + "term": "delete", + "definition": "May destroy existing records. Distinct from correct, which revises them." + }, { "categories": [ "grant_scope_type" diff --git a/core/permissions.py b/core/permissions.py index ad3406f41..2b0088338 100644 --- a/core/permissions.py +++ b/core/permissions.py @@ -275,7 +275,7 @@ class TokenInvalid(Exception): # Required Authentik group for the authenticated internal OGC mount # (/ogcapi-internal). Not Depends()-shaped like the roles above -- see the # cross-reference note in core/dependencies.py for why it still lives here. -INTERNAL_OGC_GROUP = "OGCInternal" +INTERNAL_OGC_GROUP = "OGC.Internal" def decode_token_payload(token: str) -> dict: diff --git a/data_migrations/migrations/20260829_0001_seed_legacy_access_grants.py b/data_migrations/migrations/20260829_0001_seed_legacy_access_grants.py new file mode 100644 index 000000000..b4e86cc5b --- /dev/null +++ b/data_migrations/migrations/20260829_0001_seed_legacy_access_grants.py @@ -0,0 +1,402 @@ +# =============================================================================== +# Copyright 2026 ross +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# =============================================================================== +""" +The access-control tables start empty, and empty means default deny (ADR5). +This writes down the access that already exists, so switching the layer on +changes nobody's day: + +1. **Role grants.** ``services/access_seed.py``, the day-one baseline from + ADR5 5.2: one global grant per Authentik role, capability and access data + type, plus one per role and UI surface. +2. **Landowner consent for what is already public.** Every thing carrying + ``release_status='public'`` gets a consent row per access data type against + each baseline destination -- the anonymous public web and NGWMN -- which is + the grandfathering half of PUB-D13. Both get the same rows because + ``release_status`` never distinguished them: a public well was in the OGC + collections and in what NGWMN harvests, and there was no way to say yes to + one and no to the other. There is now, and it is a revocation. + +The seeder is called rather than copied. Its mapping is the one the CLI shows +in ``oco seed-access-grants``, and two copies of a security baseline is how the +two drift. + +## Why the consent half is a migration and not a screen + +Publication today is a ``release_status`` column, set in bulk. Consent rows are +per (thing, destination, data type). Nothing converts one into the other, so +until something does, turning consent on would unpublish every well the public +can see today. This is that conversion, and it is deliberately the *widest* +reading of the existing state: a public well is grandfathered for all four data +types, because that is what ``release_status='public'`` already means. + +ADR5 leaves the choice between grandfathering and re-consenting to the data +owner, and this takes the grandfathering branch. Re-consenting per data type +happens by revoking these rows through the console, which is the part of the +model that makes a narrower answer expressible at all. + +Nothing here reads the grant tables back, so running it changes no behaviour on +its own -- no endpoint consults the visibility layer yet. + +Idempotent both halves: the seeder skips what it has written before (including +what somebody has since revoked, which stays revoked), and the consent half +skips any (thing, data type) that already has a live row for this destination. +""" + +from dataclasses import dataclass +from datetime import date, timezone, datetime + +from sqlalchemy import func, insert, select +from sqlalchemy.orm import Session + +from data_migrations.base import DataMigration +from db.authorization_audit import AuthorizationAudit, CONSENT_RECORDED +from db.destination import Destination +from db.lexicon import LexiconTerm +from db.publication_consent import PublicationConsent +from db.thing import Thing +from domain.access import CAPABILITY_VIEW, PRINCIPAL_ROLE, SCOPE_GLOBAL +from services.access_admin import register_destination +from services.access_seed import ( + ROLE_BASELINE, + SURFACE_BASELINE, + data_types, + seed_role_grants, +) +from transfers.logger import logger + + +@dataclass(frozen=True) +class BaselineDestination: + """A destination this migration expects to exist, and registers if not. + + ``excluded_data_types`` names what this destination is *not* consented to + receive. It is a subtraction rather than a list of what it does get, + because the grandfathering claim is "whatever public already meant, minus + what this destination was never offered" -- and writing it as a subtraction + means a data type added to the lexicon next year is covered by the same + reasoning instead of being silently dropped from one destination and not + the other. + """ + + slug: str + name: str + kind: str + description: str + excluded_data_types: tuple = () + + +# The destinations the legacy data is already offered to. Both receive the +# same grandfathered consent, because `release_status='public'` did not +# distinguish between them: a public well is in the OGC collections and in +# what NGWMN harvests. +# +# The kinds differ, and that is what makes them different destinations rather +# than one: `core/field-allowlists.yml` keys audiences by kind, so the public +# web gets kilometre-rounded coordinates and a harvester gets ten-metre ones. +PUBLIC_DESTINATION = BaselineDestination( + slug="public-web", + name="Public web", + kind="public web", + description=( + "Anonymous access to the public OGC collections and the public API. " + "The destination every unauthenticated caller is." + ), +) +NGWMN_DESTINATION = BaselineDestination( + slug="ngwmn", + name="National Ground Water Monitoring Network", + kind="harvester", + description=( + "The USGS-run federal network that harvests well records and water " + "levels on a schedule. Harvested copies live in someone else's " + "system, so withdrawing consent stops the offering and does not " + "recall what was already taken (ADR5, 3.6)." + ), + # Water chemistry is not part of what NGWMN is offered. Grandfathering it + # would hand a federal harvester a data type nobody agreed to send it, and + # the whole point of the per-data-type model is that this is expressible. + excluded_data_types=("water chemistry",), +) +BASELINE_DESTINATIONS = (PUBLIC_DESTINATION, NGWMN_DESTINATION) + +# Kept as a module constant because the lexicon guard and the tests name it. +PUBLIC_DESTINATION_SLUG = PUBLIC_DESTINATION.slug +PUBLIC_DESTINATION_KIND = PUBLIC_DESTINATION.kind + +# Recorded as having captured the consent. Not a person on purpose: nobody +# made a phone call for these. They exist because the record was already +# public, and the log should say so rather than name someone who did not +# decide it. +GRANDFATHER_ACTOR = "system:legacy-grandfather" +GRANDFATHER_NOTES = ( + "Grandfathered from release_status='public' (ADR5, PUB-D13). Not a " + "consent anyone gave in these terms: it records that this well's data was " + "already published before consent was tracked per data type. Narrowing it " + "is a revocation somebody makes deliberately." +) + +# The level that means the record is offered to anonymous callers today. The +# other levels in the release_status lexicon are not published, and the review +# states in there (`provisional`, `final`) are the historical rows the split +# to `data_maturity` has not moved yet -- neither of which is consent to +# publish. +PUBLIC_RELEASE_STATUS = "public" + + +def _require_lexicon_terms(session: Session, terms: tuple) -> None: + """Fail before writing rather than on a foreign key deep into a batch. + + Both grant subjects and the data type on a consent row are lexicon terms. + An environment whose lexicon has not been seeded cannot hold these rows, + and the useful error names the command that fixes it. + """ + present = set( + session.execute( + select(LexiconTerm.term).where(LexiconTerm.term.in_(terms)) + ).scalars() + ) + missing = sorted(set(terms) - present) + if missing: + raise RuntimeError( + f"lexicon terms missing: {', '.join(missing)}. " + "Run `oco initialize-lexicon` before this migration." + ) + + +class DestinationKindConflict(RuntimeError): + """A registered destination has a different kind than the baseline expects.""" + + +def _destination(session: Session, spec: BaselineDestination) -> Destination: + """The registry row for this destination, registered if it is missing. + + A row that already exists under this slug is used as it stands -- never + edited. If its kind disagrees this refuses rather than continuing, because + the kind is what picks the field allowlist: writing consent against a + destination whose kind says "public web" when the baseline meant + "harvester" would publish a different set of fields than the operator + intended, in whichever direction. Somebody decides that, not this script. + """ + destination = session.execute( + select(Destination).where(Destination.slug == spec.slug) + ).scalar_one_or_none() + if destination is not None: + if destination.destination_kind != spec.kind: + raise DestinationKindConflict( + f"destination '{spec.slug}' is registered as " + f"'{destination.destination_kind}', and this baseline expects " + f"'{spec.kind}'. The kind selects the field allowlist in " + "core/field-allowlists.yml, so fix the registry row (or the " + "baseline) rather than publishing under the wrong one." + ) + return destination + + return register_destination( + session, + GRANDFATHER_ACTOR, + slug=spec.slug, + name=spec.name, + destination_kind=spec.kind, + description=spec.description, + ) + + +def _existing_consent(session: Session, destination_id: int) -> set: + """(thing, data type) pairs this destination already has a live row for. + + Revoked rows are not here: the unique index only covers live ones, and a + consent somebody withdrew is not re-recorded by running this again. + """ + rows = session.execute( + select(PublicationConsent.thing_id, PublicationConsent.data_type).where( + PublicationConsent.destination_id == destination_id, + PublicationConsent.revoked_at.is_(None), + ) + ).all() + return {(thing_id, data_type) for thing_id, data_type in rows} + + +def consented_data_types(spec: BaselineDestination) -> tuple: + """The data types this destination is grandfathered for.""" + return tuple( + data_type + for data_type in data_types() + if data_type not in spec.excluded_data_types + ) + + +def _public_thing_ids(session: Session) -> list: + return list( + session.execute( + select(Thing.id).where(Thing.release_status == PUBLIC_RELEASE_STATUS) + ).scalars() + ) + + +def _grandfather_public_things( + session: Session, + spec: BaselineDestination = PUBLIC_DESTINATION, + public_thing_ids: list = None, +) -> int: + """One consent row per (public thing, data type). Returns rows written.""" + destination = _destination(session, spec) + already = _existing_consent(session, destination.id) + + if public_thing_ids is None: + public_thing_ids = _public_thing_ids(session) + logger.info( + f"{len(public_thing_ids)} thing(s) are release_status=" + f"'{PUBLIC_RELEASE_STATUS}'." + ) + + starts_at = date.today() + rows = [ + { + "thing_id": thing_id, + "destination_id": destination.id, + "data_type": data_type, + # NULL: the decision was institutional, not a landowner's. The + # model allows this precisely so grandfathered rows do not have to + # invent a consenting contact. + "contact_id": None, + "recorded_by": GRANDFATHER_ACTOR, + "notes": GRANDFATHER_NOTES, + "starts_at": starts_at, + "ends_at": None, + } + for thing_id in public_thing_ids + for data_type in consented_data_types(spec) + if (thing_id, data_type) not in already + ] + if not rows: + return 0 + + # Core insert rather than record_consent(): that commits per row and this + # is one transaction over tens of thousands of them. The audit rows are + # written here instead, from what the insert returns, so the promise that + # no consent lands without a trace still holds. + written = session.execute( + insert(PublicationConsent).returning( + PublicationConsent.id, + PublicationConsent.thing_id, + PublicationConsent.data_type, + ), + rows, + ).all() + + recorded_at = datetime.now(tz=timezone.utc) + session.execute( + insert(AuthorizationAudit), + [ + { + "event_type": CONSENT_RECORDED, + "actor": GRANDFATHER_ACTOR, + "subject_table": PublicationConsent.__tablename__, + "subject_id": consent_id, + "created_at": recorded_at, + "detail": { + "thing_id": thing_id, + "destination_id": destination.id, + "data_type": data_type, + "contact_id": None, + "starts_at": starts_at.isoformat(), + "ends_at": None, + "grandfathered_from": PUBLIC_RELEASE_STATUS, + }, + } + for consent_id, thing_id, data_type in written + ], + ) + session.commit() + return len(written) + + +def _required_terms() -> tuple: + """Every controlled term this migration writes into a lexicon-backed column. + + Capability, scope type, principal type, data type, UI surface and + destination kind are all foreign keys to ``lexicon_term.term``. Checking + them together means an unseeded environment fails on the first statement + with a list, rather than partway through a batch with a constraint name. + """ + capabilities = { + capability + for capabilities in ROLE_BASELINE.values() + for capability in capabilities + } | {CAPABILITY_VIEW} + surfaces = { + surface for surfaces in SURFACE_BASELINE.values() for surface in surfaces + } + return tuple( + sorted( + set(data_types()) + | capabilities + | surfaces + | {spec.kind for spec in BASELINE_DESTINATIONS} + | {PRINCIPAL_ROLE, SCOPE_GLOBAL} + ) + ) + + +def run(session: Session) -> None: + _require_lexicon_terms(session, _required_terms()) + + plan = seed_role_grants(session, apply=True) + logger.info( + f"Baseline grants: created {len(plan.created)}, " + f"left {len(plan.skipped)} alone." + ) + + public_thing_ids = _public_thing_ids(session) + logger.info( + f"{len(public_thing_ids)} thing(s) are release_status=" + f"'{PUBLIC_RELEASE_STATUS}'." + ) + for spec in BASELINE_DESTINATIONS: + written = _grandfather_public_things(session, spec, public_thing_ids) + logger.info( + f"Grandfathered {written} consent row(s) for {spec.slug} " + f"({', '.join(consented_data_types(spec))})." + ) + if spec.excluded_data_types: + logger.info( + f" {spec.slug} is not consented for: " + f"{', '.join(spec.excluded_data_types)}." + ) + + total = session.execute( + select(func.count()).select_from(PublicationConsent) + ).scalar_one() + logger.info(f"{total} publication consent row(s) now exist.") + + +MIGRATION = DataMigration( + id="20260829_0001_seed_legacy_access_grants", + alembic_revision="a396d7d9928d", + name="Seed the legacy access baseline", + description=( + "Writes down the access that already exists so the ADR5 layer can be " + "switched on without changing anyone's day: the day-one role baseline " + "(data types and UI surfaces) from services/access_seed.py, and one " + "publication consent row per (release_status='public' thing, access " + "data type) against the public-web destination, grandfathering " + "PUB-D13." + ), + run=run, + is_repeatable=False, +) + +# ============= EOF ============================================= diff --git a/docs/hydrograph-correction-publish.md b/docs/hydrograph-correction-publish.md index 3d8a59fbb..8a9da5698 100644 --- a/docs/hydrograph-correction-publish.md +++ b/docs/hydrograph-correction-publish.md @@ -10,7 +10,7 @@ proposal it was built from is ## Authorization Both write routes are gated on **`AMP.Staging`**, a standalone Authentik group. -It is not a fourth rung on the AMP ladder: `AMPAdmin` does not satisfy it, and +It is not a fourth rung on the AMP ladder: `AMP.Admin` does not satisfy it, and it satisfies nothing else. Nobody holds it until it is granted, so the routes ship dark and are reachable only by whoever is validating the workbench against real logger files. diff --git a/docs/internal-ogc-desktop-gis.md b/docs/internal-ogc-desktop-gis.md index 7efa66403..229655786 100644 --- a/docs/internal-ogc-desktop-gis.md +++ b/docs/internal-ogc-desktop-gis.md @@ -30,7 +30,7 @@ per user solves both problems. | `Authorization: Basic ` | API key (or JWT) as the password | ArcGIS Pro, QGIS Basic | | `?token=` | API key (or JWT) | ArcGIS Pro custom request parameters | -A JWT must additionally carry the `OGCInternal` group (`INTERNAL_OGC_GROUP` in +A JWT must additionally carry the `OGC.Internal` group (`INTERNAL_OGC_GROUP` in `core/permissions.py`); a valid JWT without it gets 403. An API key is a pre-authorized stand-in for that group and carries no per-user claims. @@ -118,7 +118,7 @@ issues, including paging. Staff who prefer real Authentik identity can instead configure QGIS's **OAuth2** authentication method against the Authentik provider; the mount accepts those -tokens unchanged, provided the account is in `OGCInternal`. +tokens unchanged, provided the account is in `OGC.Internal`. ## Advertised URLs @@ -134,6 +134,6 @@ different host than the public `/ogcapi` mount. | Symptom | Cause | | --- | --- | | 401 with `WWW-Authenticate: Basic` | No credential reached the server. In QGIS, confirm the auth config is selected on the *connection*, not just created. | -| 403 | Valid Authentik token, but the account is not in the `OGCInternal` group. | +| 403 | Valid Authentik token, but the account is not in the `OGC.Internal` group. | | 424 | `AUTHENTIK_DISABLE_AUTHENTICATION=1` with `MODE` other than `development`. Misconfigured deploy. | | First page loads, paging fails against `localhost` | `PYGEOAPI_SERVER_URL` unset or wrong for the environment. | diff --git a/domain/access.py b/domain/access.py index 7f0b57477..c44a66d2c 100644 --- a/domain/access.py +++ b/domain/access.py @@ -49,14 +49,37 @@ SCOPE_THING = "thing" SCOPE_TYPES = frozenset({SCOPE_GLOBAL, SCOPE_GROUP, SCOPE_THING}) -# Capabilities. +# Capabilities. `view` is the screen verb and the others are data verbs; the +# pairing is enforced in `validate_grant` rather than left as a convention, +# because two ways to spell "may see this screen" is two things that can +# disagree. CAPABILITY_READ = "read" CAPABILITY_ENTER = "enter" CAPABILITY_CORRECT = "correct" CAPABILITY_ADMINISTER = "administer" -CAPABILITIES = frozenset( - {CAPABILITY_READ, CAPABILITY_ENTER, CAPABILITY_CORRECT, CAPABILITY_ADMINISTER} +# Destroying a record is not revising one. `correct` covers a value that was +# wrong; `delete` covers a row that should not exist, which is the one action +# whose mistakes cannot be read back out of the data. +CAPABILITY_DELETE = "delete" +CAPABILITY_VIEW = "view" +DATA_CAPABILITIES = frozenset( + { + CAPABILITY_READ, + CAPABILITY_ENTER, + CAPABILITY_CORRECT, + CAPABILITY_DELETE, + CAPABILITY_ADMINISTER, + } ) +SURFACE_CAPABILITIES = frozenset({CAPABILITY_VIEW}) +CAPABILITIES = DATA_CAPABILITIES | SURFACE_CAPABILITIES + +# The two kinds of subject a grant can name, as the columns are spelled. Asking +# which kind is a coarser question than asking which one, and a caller that +# wants every screen grant cannot ask it by naming every screen. +SUBJECT_DATA_TYPE = "data_type" +SUBJECT_UI_SURFACE = "ui_surface" +GRANT_SUBJECTS = frozenset({SUBJECT_DATA_TYPE, SUBJECT_UI_SURFACE}) # Principal types. A destination is not here: publishing to one is recorded as # consent, not as a grant, which is the two-table half of ADR5. @@ -99,6 +122,10 @@ class ScopedSurfaceGrant(AccessRuleError): """A UI-surface grant was scoped to a group or a thing.""" +class CapabilitySubjectMismatch(AccessRuleError): + """A screen was granted with a data verb, or data with the screen verb.""" + + class ScopeIdMismatch(AccessRuleError): pass @@ -204,6 +231,19 @@ def validate_grant( "no wildcard, so a new data type or screen is never covered by an " "existing grant." ) + if ui_surface and capability not in SURFACE_CAPABILITIES: + # `read` over a screen would be a second spelling of `view`, and a + # listing could then show two grants that look like the same + # permission but only one of which the UI actually asks about. + raise CapabilitySubjectMismatch( + f"A UI-surface grant carries '{CAPABILITY_VIEW}', not " + f"'{capability}'. The data verbs belong to a data_type grant." + ) + if data_type and capability in SURFACE_CAPABILITIES: + raise CapabilitySubjectMismatch( + f"'{CAPABILITY_VIEW}' opens a screen, not a data type. Use one of " + f"{', '.join(sorted(DATA_CAPABILITIES))}." + ) if ui_surface and scope_type != SCOPE_GLOBAL: # Navigation is app-wide: the UI asks "may this caller see this # screen", never "for this well". A scoped surface grant could not diff --git a/services/access_seed.py b/services/access_seed.py index fdf4e3b09..6e93516be 100644 --- a/services/access_seed.py +++ b/services/access_seed.py @@ -38,6 +38,17 @@ starting point, not a statement about what each role should have: narrowing it is the point of the whole exercise, and every narrowing is a revocation somebody makes deliberately. + +Two axes are seeded, because a grant names one or the other: + +* **Data.** ``ROLE_BASELINE`` -- role x capability x access data type. +* **Screens.** ``SURFACE_BASELINE`` -- role x UI surface, always ``view`` + and always global, because that is the only shape a surface grant has. + +The screen half is widen-only: ``services/visibility.may_see_surface`` answers +whether a *grant* opens a screen, and the UI falls back to its own role policy +when the answer is no. Seeding it therefore takes nothing away; it writes +today's screen access down as grants so the fallback can eventually go. """ from dataclasses import dataclass, field @@ -50,8 +61,10 @@ from domain.access import ( CAPABILITY_ADMINISTER, CAPABILITY_CORRECT, + CAPABILITY_DELETE, CAPABILITY_ENTER, CAPABILITY_READ, + CAPABILITY_VIEW, PRINCIPAL_ROLE, SCOPE_GLOBAL, ) @@ -68,24 +81,64 @@ READ_ONLY = (CAPABILITY_READ,) EDIT = (CAPABILITY_READ, CAPABILITY_ENTER, CAPABILITY_CORRECT) -FULL = EDIT + (CAPABILITY_ADMINISTER,) +# `delete` sits with `administer` at the top rather than with the editing +# verbs. That mirrors where destruction is gated today: assets are an +# editor's to remove, but the routes that delete the records these data types +# name -- things, groups, observations -- are all admin-gated. +FULL = EDIT + (CAPABILITY_DELETE, CAPABILITY_ADMINISTER) # Authentik group -> capabilities, mirroring core/dependencies.py. The tiers -# nest within a family, so an Admin's row set is a superset of an Editor's. +# nest, so `AMP.Admin`'s row set is a superset of `AMP.Editor`'s. +# +# One ladder, not three: the general Admin/Editor/Viewer family and the AMP +# family were consolidated into the dotted groups the UI already reads. # -# Lexicon* is absent: it gates vocabulary, not data. AMP.Staging is absent +# `Lexicon.Editor` is absent here: it gates vocabulary, not data, so it holds +# a screen grant below and no data grants at all. `AMP.Staging` is absent # because it gates a workbench that ships dark, and seeding it would be the # one thing nobody intended -- granting access to something still being # validated. ROLE_BASELINE = { - "Viewer": READ_ONLY, - "Editor": EDIT, - "Admin": FULL, - "AMPViewer": READ_ONLY, - "AMPEditor": EDIT, - "AMPAdmin": FULL, + "AMP.Viewer": READ_ONLY, + "AMP.Editor": EDIT, + "AMP.Admin": FULL, # The desktop-GIS mount reads; it has never written. - "OGCInternal": READ_ONLY, + "OGC.Internal": READ_ONLY, +} + +# Screens every role that reaches data at all can already open. +BROWSE_SURFACES = ( + "ocotillo.map", + "ocotillo.thing-well", + "ocotillo.thing-well-projects", + "ocotillo.contact", + "ocotillo.location", + "ocotillo.collections", + "ocotillo.asset-unassociated", +) +# Field sheets are printed by the people who go to the well. +EDIT_SURFACES = BROWSE_SURFACES + ("ocotillo.thing-well-batch-export",) + +# Authentik group -> UI surfaces. Capability is `view` throughout: a surface +# grant means "may open this screen", and what may be done once inside is the +# data half above, which speaks in `read`/`enter`/`correct`/`administer`. +# +# Two surfaces are deliberately unseeded: +# +# * `ocotillo.hydrograph-correction` gates a workbench that ships dark behind +# AMP.Staging while it is validated against real logger files. Seeding it +# would grant access to the one thing nobody intended to hand out yet. +# * `ocotillo.access-grants` goes to `AMP.Admin` only. It is the console that +# changes who may see what, and that is the top of the one ladder. +# +# `Lexicon.Editor` appears here although it is absent from ROLE_BASELINE: it +# gates vocabulary rather than data, so it gets the vocabulary screen and no +# data grants, and `AMP.Admin` does not inherit the screen from it. +SURFACE_BASELINE = { + "AMP.Viewer": BROWSE_SURFACES, + "AMP.Editor": EDIT_SURFACES, + "AMP.Admin": EDIT_SURFACES + ("ocotillo.access-grants",), + "Lexicon.Editor": ("ocotillo.lexicon",), } @@ -97,8 +150,9 @@ class SeedPlan: skipped: list = field(default_factory=list) def describe(self, entry) -> str: - role, capability, data_type = entry - return f"role:{role} may {capability} {data_type} (global)" + role, capability, data_type, ui_surface = entry + subject = data_type if data_type else f"the {ui_surface} screen" + return f"role:{role} may {capability} {subject} (global)" def data_types() -> tuple: @@ -111,16 +165,27 @@ def data_types() -> tuple: def planned_entries() -> list: - return [ - (role, capability, data_type) + """Every baseline row, as (role, capability, data type, UI surface). + + One of the last two is always None, because a grant names exactly one + subject. + """ + data_entries = [ + (role, capability, data_type, None) for role, capabilities in ROLE_BASELINE.items() for capability in capabilities for data_type in data_types() ] + surface_entries = [ + (role, CAPABILITY_VIEW, None, ui_surface) + for role, surfaces in SURFACE_BASELINE.items() + for ui_surface in surfaces + ] + return data_entries + surface_entries def _already_seeded(session) -> set: - """Every (role, capability, data type) this seeder has ever written. + """Every entry this seeder has ever written. Revoked rows count. A grant somebody took away is not re-created by running the seeder again. @@ -130,12 +195,13 @@ def _already_seeded(session) -> set: PermissionGrant.principal_id, PermissionGrant.capability, PermissionGrant.data_type, + PermissionGrant.ui_surface, ).where( PermissionGrant.principal_type == PRINCIPAL_ROLE, PermissionGrant.granted_by == SEED_ACTOR, ) ).all() - return {(role, capability, data_type) for role, capability, data_type in rows} + return {tuple(row) for row in rows} def seed_role_grants(session, starts_at: date = None, apply: bool = True) -> SeedPlan: @@ -157,7 +223,7 @@ def seed_role_grants(session, starts_at: date = None, apply: bool = True) -> See if not apply: continue - role, capability, data_type = entry + role, capability, data_type, ui_surface = entry create_grant( session, SEED_ACTOR, @@ -167,6 +233,7 @@ def seed_role_grants(session, starts_at: date = None, apply: bool = True) -> See scope_type=SCOPE_GLOBAL, scope_id=None, data_type=data_type, + ui_surface=ui_surface, starts_at=starts_at, ends_at=None, reason=SEED_REASON, diff --git a/services/visibility.py b/services/visibility.py index 9429786c3..73be4c164 100644 --- a/services/visibility.py +++ b/services/visibility.py @@ -54,7 +54,7 @@ ) from domain.access import ( AccessRequest, - CAPABILITY_READ, + CAPABILITY_VIEW, Consent, Grant, PRINCIPAL_ROLE, @@ -171,7 +171,8 @@ def may_see_surface( """May these principals see this screen? A thin reading of ``may``: surface grants are always global and always - ``read``, so the caller does not restate either. Widen-only by + carry ``view`` -- the screen verb, distinct from the ``read`` that reaches + data -- so the caller does not restate either. Widen-only by construction -- this answers whether a *grant* opens the screen, and the UI falls back to its role policy when the answer is no, so a missing grant can never take away what a role already allows. @@ -179,7 +180,7 @@ def may_see_surface( return may( session, principals, - capability=CAPABILITY_READ, + capability=CAPABILITY_VIEW, ui_surface=ui_surface, on_date=on_date, ) diff --git a/tests/test_access.py b/tests/test_access.py index 9de015d90..cbc696c4c 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -33,7 +33,7 @@ from db.publication_consent import PublicationConsent from tests import client, override_authentication -ADMIN_PAYLOAD = {"sub": "test-admin", "groups": ["Admin"]} +ADMIN_PAYLOAD = {"sub": "test-admin", "groups": ["AMP.Admin"]} SLUG = "test-harvester" TODAY = date.today() @@ -322,6 +322,10 @@ def make_grant(grants, **overrides): "starts_at": TODAY.isoformat(), "reason": "test", } + # A screen is opened with `view`; the data verbs belong to a data_type + # grant, and the route refuses the crossing. + if overrides.get("ui_surface") and "capability" not in overrides: + payload["capability"] = "view" payload.update(overrides) response = client.post("/access/grant", json=payload) if response.status_code == 201: @@ -329,6 +333,27 @@ def make_grant(grants, **overrides): return response +def grant_rows(**params): + """Whole rows on one page of the grant listing, for assertions about what + a filter returned rather than which ids it happened to include.""" + params.setdefault("size", 1000) + response = client.get("/access/grant", params=params) + assert response.status_code == 200, response.text + return response.json()["items"] + + +def listed_grants(**params): + """Ids on one page of the grant listing. + + `size` is generous because the seeded baseline shares this database and a + default page would push a freshly written grant off the end. + """ + params.setdefault("size", 200) + response = client.get("/access/grant", params=params) + assert response.status_code == 200, response.text + return [row["id"] for row in response.json()["items"]] + + def decision(**params): response = client.get("/access/decision", params=params) assert response.status_code == 200, response.text @@ -388,32 +413,57 @@ def test_a_grant_naming_no_data_type_cannot_be_written(grants): def test_listing_grants_with_no_filter_returns_everything(grants): grant_id = make_grant(grants).json()["id"] - everyone = client.get("/access/grant") - assert grant_id in [row["id"] for row in everyone.json()] + assert grant_id in listed_grants() def test_listing_grants_filters_by_data_type(grants): grant_id = make_grant(grants).json()["id"] - match = client.get("/access/grant", params={"data_type": "water level"}) - assert grant_id in [row["id"] for row in match.json()] + assert grant_id in listed_grants(data_type="water level") + assert grant_id not in listed_grants(data_type="water chemistry") + + +def test_listing_grants_filters_by_subject(grants): + """Which kind of grant, not which one. A console offering "screens only" + cannot ask by naming every screen.""" + data_grant = make_grant(grants).json()["id"] + surface_grant = make_grant( + grants, + capability="view", + data_type=None, + ui_surface="ocotillo.lexicon", + ).json()["id"] + + # Asserted as a property of every row rather than by membership: the + # seeded baseline shares this database, and a page-bounded membership + # check turns "filtered wrongly" and "fell off the page" into one failure. + surfaces = grant_rows(subject="ui_surface") + assert surface_grant in [row["id"] for row in surfaces] + assert all(row["ui_surface"] for row in surfaces) + assert all(row["data_type"] is None for row in surfaces) + + data = grant_rows(subject="data_type") + assert data_grant in [row["id"] for row in data] + assert all(row["data_type"] for row in data) + assert all(row["ui_surface"] is None for row in data) + + +def test_listing_grants_rejects_a_subject_that_is_neither(grants): + """Default deny would be a lie here: an unrecognized subject is a caller + bug, and answering it with an empty page reads as "none exist".""" + response = client.get("/access/grant", params={"subject": "screens"}) - no_match = client.get("/access/grant", params={"data_type": "water chemistry"}) - assert grant_id not in [row["id"] for row in no_match.json()] + assert response.status_code == 422 def test_listing_grants_hides_revoked_ones_by_default(grants): grant_id = make_grant(grants).json()["id"] client.post(f"/access/grant/{grant_id}/revocation") - live = client.get("/access/grant", params={"principal_id": ADMIN_PAYLOAD["sub"]}) - assert live.json() == [] - - history = client.get( - "/access/grant", - params={"principal_id": ADMIN_PAYLOAD["sub"], "include_revoked": True}, - ) - assert [row["id"] for row in history.json()] == [grant_id] + assert listed_grants(principal_id=ADMIN_PAYLOAD["sub"]) == [] + assert listed_grants(principal_id=ADMIN_PAYLOAD["sub"], include_revoked=True) == [ + grant_id + ] # ------ audit ---------- @@ -479,14 +529,14 @@ def test_a_surface_grant_opens_that_screen(grants): == 201 ) - assert decision(capability="read", ui_surface="ocotillo.lexicon")["allowed"] is True + assert decision(capability="view", ui_surface="ocotillo.lexicon")["allowed"] is True def test_a_surface_grant_opens_only_that_screen(grants): make_grant(grants, data_type=None, ui_surface="ocotillo.lexicon") assert ( - decision(capability="read", ui_surface="ocotillo.location")["allowed"] is False + decision(capability="view", ui_surface="ocotillo.location")["allowed"] is False ) @@ -494,7 +544,7 @@ def test_a_data_grant_does_not_open_a_screen(grants): make_grant(grants) assert ( - decision(capability="read", ui_surface="ocotillo.lexicon")["allowed"] is False + decision(capability="view", ui_surface="ocotillo.lexicon")["allowed"] is False ) @@ -529,12 +579,12 @@ def test_revoking_a_surface_grant_closes_the_screen(grants): grant_id = make_grant(grants, data_type=None, ui_surface="ocotillo.lexicon").json()[ "id" ] - assert decision(capability="read", ui_surface="ocotillo.lexicon")["allowed"] is True + assert decision(capability="view", ui_surface="ocotillo.lexicon")["allowed"] is True assert client.post(f"/access/grant/{grant_id}/revocation").status_code == 201 assert ( - decision(capability="read", ui_surface="ocotillo.lexicon")["allowed"] is False + decision(capability="view", ui_surface="ocotillo.lexicon")["allowed"] is False ) @@ -563,8 +613,7 @@ def test_listing_grants_filters_by_ui_surface(grants): ).json()["id"] data_id = make_grant(grants).json()["id"] - match = client.get("/access/grant", params={"ui_surface": "ocotillo.lexicon"}) - ids = [row["id"] for row in match.json()] + ids = listed_grants(ui_surface="ocotillo.lexicon") assert surface_id in ids assert data_id not in ids @@ -588,3 +637,42 @@ def test_a_surface_grant_is_logged_like_any_other(grants): # ============= EOF ============================================= + + +def test_the_grant_listing_is_paged(grants): + """The admin-wide view is not small: the day-one baseline alone is dozens + of rows before anybody grants anything by hand.""" + for _ in range(3): + make_grant(grants) + + page = client.get("/access/grant", params={"size": 2, "page": 1}) + body = page.json() + + assert page.status_code == 200 + assert len(body["items"]) == 2 + assert body["total"] >= 3 + assert body["size"] == 2 + + second = client.get("/access/grant", params={"size": 2, "page": 2}).json() + assert not set(row["id"] for row in body["items"]) & set( + row["id"] for row in second["items"] + ) + + +def test_paging_is_stable_across_pages(grants): + """Ordered by id, so a row cannot appear on two pages or on none.""" + for _ in range(5): + make_grant(grants) + + everything = listed_grants(size=500) + walked = [] + for page_number in (1, 2, 3): + walked.extend( + row["id"] + for row in client.get( + "/access/grant", params={"size": 2, "page": page_number} + ).json()["items"] + ) + + assert walked == everything[: len(walked)] + assert len(walked) == len(set(walked)) diff --git a/tests/test_access_seed.py b/tests/test_access_seed.py index f88037a82..ccb7e6a20 100644 --- a/tests/test_access_seed.py +++ b/tests/test_access_seed.py @@ -28,17 +28,21 @@ from core.dependencies import admin_function, viewer_function from db.authorization_audit import AuthorizationAudit from db.engine import session_ctx +from core.enums import UISurface from db.permission_grant import PermissionGrant from main import app from services.access_seed import ( ROLE_BASELINE, SEED_ACTOR, + SURFACE_BASELINE, planned_entries, seed_role_grants, ) +from domain.access import PRINCIPAL_ROLE +from services.visibility import may_see_surface from tests import client, override_authentication -VIEWER_PAYLOAD = {"sub": "test-viewer", "groups": ["Viewer"]} +VIEWER_PAYLOAD = {"sub": "test-viewer", "groups": ["AMP.Viewer"]} @pytest.fixture(autouse=True) @@ -107,7 +111,7 @@ def test_a_revoked_baseline_grant_is_not_resurrected(): session.execute( select(PermissionGrant).where( PermissionGrant.granted_by == SEED_ACTOR, - PermissionGrant.principal_id == "Viewer", + PermissionGrant.principal_id == "AMP.Viewer", PermissionGrant.data_type == "water chemistry", ) ) @@ -150,9 +154,9 @@ def test_seeding_is_audited(): def test_a_viewer_gets_read_and_nothing_else(): """The tiers nest within a family; the baseline has to say so.""" - assert ROLE_BASELINE["Viewer"] == ("read",) - assert set(ROLE_BASELINE["Admin"]) > set(ROLE_BASELINE["Editor"]) - assert set(ROLE_BASELINE["Editor"]) > set(ROLE_BASELINE["Viewer"]) + assert ROLE_BASELINE["AMP.Viewer"] == ("read",) + assert set(ROLE_BASELINE["AMP.Admin"]) > set(ROLE_BASELINE["AMP.Editor"]) + assert set(ROLE_BASELINE["AMP.Editor"]) > set(ROLE_BASELINE["AMP.Viewer"]) def test_the_dark_workbench_group_is_not_seeded(): @@ -180,7 +184,7 @@ def test_decision_says_yes_to_a_role_holder_after_seeding(): "/access/decision", params={"capability": "read", "data_type": "water level"} ).json() assert allowed["allowed"] is True - assert "role:Viewer" in allowed["principals"] + assert "role:AMP.Viewer" in allowed["principals"] def test_a_viewer_still_cannot_correct_after_seeding(): @@ -215,3 +219,83 @@ def test_seeded_grants_start_today_not_retroactively(): rows = seeded_rows(session) assert all(row.starts_at == date.today() for row in rows) + + +# ------ the screen half ---------- + + +def test_a_seeded_surface_grant_is_global_view_over_no_data_type(): + """The only shape a surface grant has; anything else could never match.""" + with session_ctx() as session: + seed_role_grants(session) + rows = [row for row in seeded_rows(session) if row.ui_surface] + + assert rows + assert all(row.capability == "view" for row in rows) + assert all(row.data_type is None for row in rows) + assert all(row.scope_type == "global" and row.scope_id is None for row in rows) + + +def test_every_seeded_surface_is_a_real_term(): + """A typo here would be a foreign key error mid-seed, in production.""" + known = {member.value for member in UISurface} + seeded = {surface for surfaces in SURFACE_BASELINE.values() for surface in surfaces} + assert seeded <= known + + +def test_the_dark_workbench_screen_is_not_seeded(): + """Same reason AMP.Staging holds no data grants: it ships dark.""" + seeded = {surface for surfaces in SURFACE_BASELINE.values() for surface in surfaces} + assert "ocotillo.hydrograph-correction" not in seeded + + +def test_only_admin_gets_the_access_console(): + holders = { + role + for role, surfaces in SURFACE_BASELINE.items() + if "ocotillo.access-grants" in surfaces + } + assert holders == {"AMP.Admin"} + + +def test_the_lexicon_screen_follows_the_lexicon_family(): + """Vocabulary is not data: the lexicon group gets the screen and no + data grants, and the top of the data ladder does not inherit it.""" + assert SURFACE_BASELINE["Lexicon.Editor"] == ("ocotillo.lexicon",) + assert "ocotillo.lexicon" not in SURFACE_BASELINE["AMP.Admin"] + assert "Lexicon.Editor" not in ROLE_BASELINE + + +def test_a_seeded_surface_grant_opens_the_screen(): + with session_ctx() as session: + seed_role_grants(session) + allowed = may_see_surface( + session, + principals=((PRINCIPAL_ROLE, "AMP.Viewer"),), + ui_surface="ocotillo.map", + ) + denied = may_see_surface( + session, + principals=((PRINCIPAL_ROLE, "AMP.Viewer"),), + ui_surface="ocotillo.access-grants", + ) + + assert allowed is True + assert denied is False + + +def test_only_the_admin_tier_may_delete(): + """Destruction is the one action whose mistakes cannot be read back out of + the data, and today's delete routes for these data types are admin-gated.""" + holders = { + role for role, capabilities in ROLE_BASELINE.items() if "delete" in capabilities + } + + assert holders == {"AMP.Admin"} + + +def test_delete_is_not_correct(): + """A row that should not exist is a different question from a value that + was wrong, so an editor revising records cannot remove them.""" + assert "correct" in ROLE_BASELINE["AMP.Editor"] + assert "delete" not in ROLE_BASELINE["AMP.Editor"] diff --git a/tests/test_authorization.py b/tests/test_authorization.py index 5593d8e9e..9a50ab0eb 100644 --- a/tests/test_authorization.py +++ b/tests/test_authorization.py @@ -164,18 +164,20 @@ def test_public_schema_advertises_only_anonymous_routes(): @pytest.mark.parametrize( "groups, expected", [ - (["Admin"], True), - (["Editor"], True), - (["Viewer"], True), - (["AMPAdmin"], False), + (["AMP.Admin"], True), + (["AMP.Editor"], True), + (["AMP.Viewer"], True), + (["Lexicon.Editor"], False), ([], False), ], ) def test_admin_satisfies_viewer_tier(groups, expected): - """Admin > Editor > Viewer is enforced in code, not by Authentik overlap.""" + """AMP.Admin > AMP.Editor > AMP.Viewer is enforced in code, not by + Authentik overlap.""" assert ( permissions.authorize_groups( - {"groups": groups}, require_any=["Admin", "Editor", "Viewer"] + {"groups": groups}, + require_any=["AMP.Admin", "AMP.Editor", "AMP.Viewer"], ) is expected ) @@ -184,42 +186,44 @@ def test_admin_satisfies_viewer_tier(groups, expected): @pytest.mark.parametrize( "groups, expected", [ - (["Admin"], True), - (["Editor"], False), - (["Viewer"], False), + (["AMP.Admin"], True), + (["AMP.Editor"], False), + (["AMP.Viewer"], False), ], ) def test_admin_tier_does_not_accept_lower_roles(groups, expected): assert ( - permissions.authorize_groups({"groups": groups}, require_any=["Admin"]) + permissions.authorize_groups({"groups": groups}, require_any=["AMP.Admin"]) is expected ) -def test_role_families_stay_orthogonal(): - """General Admin confers nothing in the AMP or Lexicon families.""" - payload = {"groups": ["Admin"]} - assert not permissions.authorize_groups( - payload, require_any=["AMPAdmin", "AMPEditor", "AMPViewer"] - ) - assert not permissions.authorize_groups( - payload, require_any=["LexiconAdmin", "LexiconEditor"] - ) +def test_data_roles_confer_nothing_over_vocabulary(): + """The ladder was consolidated into one AMP family, but vocabulary and the + desktop-GIS mount stay outside it: the top of the data ladder is not a + lexicon editor.""" + payload = {"groups": ["AMP.Admin"]} + assert not permissions.authorize_groups(payload, require_any=["Lexicon.Editor"]) + assert not permissions.authorize_groups(payload, require_any=["OGC.Internal"]) + assert not permissions.authorize_groups(payload, require_any=["AMP.Staging"]) def test_require_all_demands_every_group(): assert permissions.authorize_groups( - {"groups": ["Admin", "AMPAdmin"]}, require_all=["Admin", "AMPAdmin"] + {"groups": ["AMP.Admin", "Lexicon.Editor"]}, + require_all=["AMP.Admin", "Lexicon.Editor"], ) assert not permissions.authorize_groups( - {"groups": ["Admin"]}, require_all=["Admin", "AMPAdmin"] + {"groups": ["AMP.Admin"]}, require_all=["AMP.Admin", "Lexicon.Editor"] ) def test_missing_groups_claim_denies(): """A token with no `groups` claim must not satisfy a role requirement.""" - assert not permissions.authorize_groups({}, require_any=["Viewer"]) - assert not permissions.authorize_groups({"groups": None}, require_any=["Viewer"]) + assert not permissions.authorize_groups({}, require_any=["AMP.Viewer"]) + assert not permissions.authorize_groups( + {"groups": None}, require_any=["AMP.Viewer"] + ) # Bypass configuration guard --------------------------------------------------- diff --git a/tests/test_data_migrations.py b/tests/test_data_migrations.py index bf349711e..d8bf71f18 100644 --- a/tests/test_data_migrations.py +++ b/tests/test_data_migrations.py @@ -14,6 +14,8 @@ # limitations under the License. # =============================================================================== import importlib + +import pytest from datetime import datetime, timedelta, timezone from sqlalchemy import delete, select @@ -27,11 +29,20 @@ backfill_acoustic_maturity = importlib.import_module( "data_migrations.migrations.20260820_0001_backfill_acoustic_data_maturity" ) +seed_legacy_access = importlib.import_module( + "data_migrations.migrations.20260829_0001_seed_legacy_access_grants" +) +from db.authorization_audit import AuthorizationAudit +from db.destination import Destination from db.location import Location +from db.permission_grant import PermissionGrant +from db.publication_consent import PublicationConsent +from db.thing import Thing from db.notes import Notes from db.group import Group from db.engine import session_ctx from db.transducer import TransducerObservation +from services.access_seed import SEED_ACTOR, data_types from tests import get_parameter_id @@ -233,3 +244,257 @@ def test_backfill_acoustic_data_maturity_is_idempotent( ) ) session.commit() + + +def _consents_for(session, thing_id): + return ( + session.execute( + select(PublicationConsent).where(PublicationConsent.thing_id == thing_id) + ) + .scalars() + .all() + ) + + +def test_grandfathering_covers_public_things_and_leaves_the_rest_alone(): + """release_status='public' already publishes everything about a record, so + the grandfathered consent is the widest one: every data type.""" + with session_ctx() as session: + public = Thing(name="Grandfathered Well", thing_type="water well") + public.release_status = "public" + private = Thing(name="Unpublished Well", thing_type="water well") + private.release_status = "draft" + session.add_all([public, private]) + session.commit() + public_id, private_id = public.id, private.id + + try: + seed_legacy_access.run(session) + + granted = _consents_for(session, public_id) + destinations = { + row.destination.slug: { + consent.data_type + for consent in granted + if consent.destination_id == row.destination_id + } + for row in granted + } + assert destinations == { + spec.slug: set(seed_legacy_access.consented_data_types(spec)) + for spec in seed_legacy_access.BASELINE_DESTINATIONS + } + assert all( + row.recorded_by == seed_legacy_access.GRANDFATHER_ACTOR + for row in granted + ) + # Institutional, not a landowner's: no contact is invented. + assert all(row.contact_id is None for row in granted) + assert _consents_for(session, private_id) == [] + finally: + session.execute( + delete(PublicationConsent).where( + PublicationConsent.recorded_by + == seed_legacy_access.GRANDFATHER_ACTOR + ) + ) + session.execute( + delete(AuthorizationAudit).where( + AuthorizationAudit.actor == seed_legacy_access.GRANDFATHER_ACTOR + ) + ) + session.execute( + delete(PermissionGrant).where(PermissionGrant.granted_by == SEED_ACTOR) + ) + session.execute( + delete(AuthorizationAudit).where(AuthorizationAudit.actor == SEED_ACTOR) + ) + session.execute( + delete(Destination).where( + Destination.slug.in_( + [spec.slug for spec in seed_legacy_access.BASELINE_DESTINATIONS] + ) + ) + ) + session.execute(delete(Thing).where(Thing.id.in_([public_id, private_id]))) + session.commit() + + +def test_grandfathering_twice_writes_nothing_the_second_time(): + with session_ctx() as session: + thing = Thing(name="Twice Grandfathered Well", thing_type="water well") + thing.release_status = "public" + session.add(thing) + session.commit() + thing_id = thing.id + + try: + first = seed_legacy_access._grandfather_public_things(session) + second = seed_legacy_access._grandfather_public_things(session) + ngwmn = seed_legacy_access._grandfather_public_things( + session, seed_legacy_access.NGWMN_DESTINATION + ) + + ngwmn_types = seed_legacy_access.consented_data_types( + seed_legacy_access.NGWMN_DESTINATION + ) + assert first >= len(data_types()) + assert second == 0 + assert ngwmn >= len(ngwmn_types) + assert len(_consents_for(session, thing_id)) == len(data_types()) + len( + ngwmn_types + ) + finally: + session.execute( + delete(PublicationConsent).where( + PublicationConsent.recorded_by + == seed_legacy_access.GRANDFATHER_ACTOR + ) + ) + session.execute( + delete(AuthorizationAudit).where( + AuthorizationAudit.actor == seed_legacy_access.GRANDFATHER_ACTOR + ) + ) + session.execute( + delete(Destination).where( + Destination.slug.in_( + [spec.slug for spec in seed_legacy_access.BASELINE_DESTINATIONS] + ) + ) + ) + session.execute(delete(Thing).where(Thing.id == thing_id)) + session.commit() + + +def test_every_grandfathered_consent_is_audited(): + """No row that changes what is published lands without a trace.""" + with session_ctx() as session: + thing = Thing(name="Audited Grandfathered Well", thing_type="water well") + thing.release_status = "public" + session.add(thing) + session.commit() + thing_id = thing.id + + try: + seed_legacy_access._grandfather_public_things(session) + consent_ids = {row.id for row in _consents_for(session, thing_id)} + events = ( + session.execute( + select(AuthorizationAudit).where( + AuthorizationAudit.subject_id.in_(consent_ids), + AuthorizationAudit.subject_table == "publication_consent", + ) + ) + .scalars() + .all() + ) + + assert {event.subject_id for event in events} == consent_ids + assert {event.event_type for event in events} == {"consent.recorded"} + assert all( + event.detail["grandfathered_from"] == "public" for event in events + ) + finally: + session.execute( + delete(PublicationConsent).where( + PublicationConsent.recorded_by + == seed_legacy_access.GRANDFATHER_ACTOR + ) + ) + session.execute( + delete(AuthorizationAudit).where( + AuthorizationAudit.actor == seed_legacy_access.GRANDFATHER_ACTOR + ) + ) + session.execute( + delete(Destination).where( + Destination.slug.in_( + [spec.slug for spec in seed_legacy_access.BASELINE_DESTINATIONS] + ) + ) + ) + session.execute(delete(Thing).where(Thing.id == thing_id)) + session.commit() + + +def test_a_destination_registered_under_another_kind_is_refused(): + """The kind picks the field allowlist, so publishing under the wrong one + would send a different set of fields than anybody chose.""" + spec = seed_legacy_access.NGWMN_DESTINATION + with session_ctx() as session: + wrong = Destination( + slug=spec.slug, + name=spec.name, + destination_kind="public web", + active=True, + ) + session.add(wrong) + session.commit() + + try: + with pytest.raises(seed_legacy_access.DestinationKindConflict): + seed_legacy_access._destination(session, spec) + finally: + session.execute(delete(Destination).where(Destination.slug == spec.slug)) + session.commit() + + +def test_ngwmn_is_not_grandfathered_for_water_chemistry(): + """The harvester was never offered chemistry. Grandfathering it would hand + a federal network a data type nobody agreed to send.""" + spec = seed_legacy_access.NGWMN_DESTINATION + consented = seed_legacy_access.consented_data_types(spec) + + assert "water chemistry" not in consented + assert set(consented) == set(data_types()) - {"water chemistry"} + # The public web keeps everything: that is what the column already meant. + assert set( + seed_legacy_access.consented_data_types(seed_legacy_access.PUBLIC_DESTINATION) + ) == set(data_types()) + + +def test_the_excluded_type_is_absent_from_the_rows_written(): + with session_ctx() as session: + thing = Thing(name="Chemistry Excluded Well", thing_type="water well") + thing.release_status = "public" + session.add(thing) + session.commit() + thing_id = thing.id + + try: + seed_legacy_access._grandfather_public_things( + session, seed_legacy_access.NGWMN_DESTINATION + ) + written = { + row.data_type + for row in _consents_for(session, thing_id) + if row.destination.slug == "ngwmn" + } + + assert "water chemistry" not in written + assert written == set( + seed_legacy_access.consented_data_types( + seed_legacy_access.NGWMN_DESTINATION + ) + ) + finally: + session.execute( + delete(PublicationConsent).where( + PublicationConsent.thing_id == thing_id + ) + ) + session.execute( + delete(AuthorizationAudit).where( + AuthorizationAudit.actor == seed_legacy_access.GRANDFATHER_ACTOR + ) + ) + session.execute( + delete(Destination).where( + Destination.slug.in_( + [spec.slug for spec in seed_legacy_access.BASELINE_DESTINATIONS] + ) + ) + ) + session.execute(delete(Thing).where(Thing.id == thing_id)) + session.commit() diff --git a/tests/test_domain_access.py b/tests/test_domain_access.py index 15fd9d82b..36a1b70e5 100644 --- a/tests/test_domain_access.py +++ b/tests/test_domain_access.py @@ -22,6 +22,10 @@ from domain.access import ( AccessRequest, CAPABILITIES, + CAPABILITY_DELETE, + CapabilitySubjectMismatch, + DATA_CAPABILITIES, + SURFACE_CAPABILITIES, PRINCIPAL_TYPES, SCOPE_TYPES, AmbiguousGrantSubject, @@ -45,7 +49,7 @@ TODAY = date(2026, 8, 24) STUDENT = ("user", "authentik-sub-1") -EDITOR_ROLE = ("role", "Editor") +EDITOR_ROLE = ("role", "AMP.Editor") def a_grant(**overrides): @@ -114,7 +118,7 @@ def test_principal_type_is_part_of_identity(): def test_a_role_grant_covers_a_caller_holding_that_role(): - grant = a_grant(principal_type="role", principal_id="Editor") + grant = a_grant(principal_type="role", principal_id="AMP.Editor") request = a_request(principals=(STUDENT, EDITOR_ROLE)) assert grant_covers(grant, request, TODAY) is True @@ -191,7 +195,7 @@ def test_a_thing_grant_needs_a_scope_id(): def test_an_unknown_capability_is_rejected(): with pytest.raises(UnknownCapability): - validate_grant("user", "delete", "global", None, "water level", TODAY, None) + validate_grant("user", "adjudicate", "global", None, "water level", TODAY, None) def test_an_unknown_principal_type_is_rejected(): @@ -382,7 +386,7 @@ def test_a_grant_naming_neither_subject_is_rejected(): def test_a_surface_grant_is_accepted_without_a_data_type(): validate_grant( "user", - "read", + "view", "global", None, None, @@ -398,7 +402,7 @@ def test_a_scoped_surface_grant_is_rejected(scope_type, scope_id): with pytest.raises(ScopedSurfaceGrant): validate_grant( "user", - "read", + "view", scope_type, scope_id, None, @@ -408,4 +412,50 @@ def test_a_scoped_surface_grant_is_rejected(scope_type, scope_id): ) +def test_a_screen_cannot_be_granted_with_a_data_verb(): + """`read` over a screen would be a second spelling of `view`, and the UI + only ever asks about one of them.""" + with pytest.raises(CapabilitySubjectMismatch): + validate_grant( + "user", + "read", + "global", + None, + None, + TODAY, + None, + ui_surface="ocotillo.lexicon", + ) + + +def test_data_cannot_be_granted_with_the_screen_verb(): + with pytest.raises(CapabilitySubjectMismatch): + validate_grant("user", "view", "global", None, "water level", TODAY, None) + + +def test_the_two_capability_sets_do_not_overlap(): + assert DATA_CAPABILITIES.isdisjoint(SURFACE_CAPABILITIES) + assert CAPABILITIES == DATA_CAPABILITIES | SURFACE_CAPABILITIES + + # ============= EOF ============================================= + + +def test_delete_is_a_data_verb_not_a_screen_one(): + assert CAPABILITY_DELETE in DATA_CAPABILITIES + assert CAPABILITY_DELETE not in SURFACE_CAPABILITIES + validate_grant("user", "delete", "global", None, "water level", TODAY, None) + + +def test_a_screen_cannot_be_deleted(): + with pytest.raises(CapabilitySubjectMismatch): + validate_grant( + "user", + "delete", + "global", + None, + None, + TODAY, + None, + ui_surface="ocotillo.lexicon", + ) diff --git a/tests/test_internal_ogc_auth.py b/tests/test_internal_ogc_auth.py index 68abb3126..a6c739455 100644 --- a/tests/test_internal_ogc_auth.py +++ b/tests/test_internal_ogc_auth.py @@ -146,7 +146,7 @@ def test_bearer_api_key_is_accepted(gate, api_key): def test_bearer_jwt_still_requires_the_internal_group(gate, monkeypatch): monkeypatch.setattr( - permissions, "decode_token_payload", lambda token: {"groups": ["Viewer"]} + permissions, "decode_token_payload", lambda token: {"groups": ["AMP.Viewer"]} ) response = gate.get(