From f986b909284ab1575ea4069ef031fd2af65a0635 Mon Sep 17 00:00:00 2001 From: jakeross Date: Thu, 27 Aug 2026 18:58:39 -0700 Subject: [PATCH] feat(access): let a grant open a UI surface, not only reach data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A permission grant could name a data type. It can now name a `ui_surface` instead: a screen or navigation item in the admin UI, so "this person may see the Lexicon editor" is a grant rather than a role change in Authentik. Surfaces are named by the resource identifier the UI already checks (`ocotillo.lexicon`, `ocotillo.thing-well`, ...), so a lexicon term and a nav item cannot drift apart. They are lexicon terms, not an enum type, so adding a screen later is a seed, not a migration. ## The no-wildcard rule is kept, not relaxed `data_type` becomes nullable to make room for `ui_surface`, which reads like a loosened invariant and is not one. A grant still names exactly one subject: the XOR between the two columns is enforced in domain/access.py before any row is written, so it holds for every writer rather than only for the route. Neither vocabulary has a term meaning "all", so a data type or a screen added next year is still never covered by an existing grant. A grant naming both is refused. It would be two grants wearing one revocation, and revoking the data half would silently take the screen away too — write two grants so each can be revoked on its own. ## A surface grant is always global Navigation is app-wide: the UI asks "may this caller see this screen", never "for this well". A group- or thing-scoped surface grant could not match any request the UI makes, so it is refused at the door rather than stored as a row that silently never applies. ## Matching `_subject_matches` never matches an unasked axis. Both a data grant and a surface request carry `None` on the axis the other names, and matching on `None == None` would have made a water-level grant answer "may I see the Lexicon editor" with yes. `may()` takes `ui_surface` alongside `data_type`, and `may_see_surface()` is the thin reading of it the UI will use. A call naming neither subject is a no, not an error: a question this layer cannot answer is not a yes. `GET /access/decision` refuses a call naming both — two questions have two answers — and `GET /access/grant` filters by `ui_surface`. ## Downgrade A surface grant has no data_type to fall back to, so it cannot survive the column being NOT NULL again. The downgrade deletes those rows, which is the honest reading: they are grants the old schema cannot express, and giving them an invented data_type would grant data access nobody asked for. Co-Authored-By: Claude Opus 5 --- ...928d_add_ui_surface_to_permission_grant.py | 70 +++++++++ api/access.py | 45 +++++- core/enums.py | 1 + core/lexicon.json | 81 ++++++++++ db/permission_grant.py | 22 ++- domain/access.py | 63 +++++++- schemas/access.py | 13 +- services/access_admin.py | 4 + services/visibility.py | 35 ++++- tests/test_access.py | 127 +++++++++++++++- tests/test_domain_access.py | 139 ++++++++++++++++++ 11 files changed, 575 insertions(+), 25 deletions(-) create mode 100644 alembic/versions/a396d7d9928d_add_ui_surface_to_permission_grant.py diff --git a/alembic/versions/a396d7d9928d_add_ui_surface_to_permission_grant.py b/alembic/versions/a396d7d9928d_add_ui_surface_to_permission_grant.py new file mode 100644 index 00000000..f569799c --- /dev/null +++ b/alembic/versions/a396d7d9928d_add_ui_surface_to_permission_grant.py @@ -0,0 +1,70 @@ +"""add ui_surface to permission_grant + +A grant could reach data. Now it can also open a screen: `ui_surface` names a +navigation item or page in the admin UI, using the same resource identifier the +UI already checks, so a term and a nav item cannot drift apart. + +`data_type` becomes nullable to make room for it. That does **not** relax the +no-wildcard rule -- a grant still names exactly one subject, and the XOR between +`data_type` and `ui_surface` is enforced in domain/access.py before any row is +written, along with the rule that a surface grant is always global. The rules +live there rather than in a check constraint so they hold for every writer and +read as one sentence; see db/permission_grant.py. + +Existing rows all carry a data_type and are untouched: nothing in the table +means "all", before or after this migration. + +`ui_surface` is a lexicon category seeded from core/lexicon.json by +init_lexicon, not an enum type, so adding a screen later is not a migration. + +Revision ID: a396d7d9928d +Revises: 79a3ab24627e +Create Date: 2026-08-27 18:20:00.000000 + +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "a396d7d9928d" +down_revision: Union[str, Sequence[str], None] = "79a3ab24627e" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.add_column( + "permission_grant", + sa.Column("ui_surface", sa.String(length=100), nullable=True), + ) + op.create_foreign_key( + "fk_permission_grant_ui_surface_lexicon_term", + "permission_grant", + "lexicon_term", + ["ui_surface"], + ["term"], + onupdate="CASCADE", + ) + op.alter_column("permission_grant", "data_type", nullable=True) + + +def downgrade() -> None: + # A surface grant has no data_type to fall back to, so it cannot survive a + # column that is NOT NULL again. Dropping those rows is the honest + # downgrade: they are grants this schema has no way to express, and + # leaving them with an invented data_type would grant data access nobody + # asked for. + op.execute("DELETE FROM permission_grant WHERE ui_surface IS NOT NULL") + op.alter_column("permission_grant", "data_type", nullable=False) + op.drop_constraint( + "fk_permission_grant_ui_surface_lexicon_term", + "permission_grant", + type_="foreignkey", + ) + op.drop_column("permission_grant", "ui_surface") + + +# ============= EOF ============================================= diff --git a/api/access.py b/api/access.py index 000d7abe..7bdf54dd 100644 --- a/api/access.py +++ b/api/access.py @@ -68,6 +68,11 @@ revoke_consent, revoke_grant, ) +from domain.access import ( + AmbiguousGrantSubject, + MissingDataType, + ScopedSurfaceGrant, +) from services.exceptions_helper import PydanticStyleException from services.visibility import ( destination_by_slug, @@ -120,7 +125,8 @@ def create_permission_grant( ) -> PermissionGrantResponse: """Grant a principal one capability over one data type within one scope. - The grant names its data type; there is no wildcard, so a data type added + The grant names its data type, or the UI surface it opens -- exactly one. + There is no wildcard in either vocabulary, so a data type or screen added later is never covered by this row. """ try: @@ -132,11 +138,22 @@ def create_permission_grant( capability=payload.capability.value, scope_type=payload.scope_type.value, scope_id=payload.scope_id, - data_type=payload.data_type.value, + data_type=payload.data_type.value if payload.data_type else None, + ui_surface=payload.ui_surface.value if payload.ui_surface else None, starts_at=payload.starts_at, ends_at=payload.ends_at, reason=payload.reason, ) + except ScopedSurfaceGrant as exception: + raise _invalid("scope_type", str(exception), payload.scope_type.value) + except (AmbiguousGrantSubject, MissingDataType) as exception: + # `.value`, not the enum: the detail is serialized to JSON, and a bare + # Enum member is not serializable. + raise _invalid( + "data_type", + str(exception), + payload.data_type.value if payload.data_type else None, + ) except ValueError as exception: raise _invalid("scope_id", str(exception), payload.scope_id) @@ -175,6 +192,7 @@ def get_permission_grants( ), capability: str = Query(default=None), data_type: str = Query(default=None), + ui_surface: str = Query(default=None), scope_type: str = Query(default=None), include_revoked: bool = Query(default=False), ) -> list[PermissionGrantResponse]: @@ -190,6 +208,8 @@ def get_permission_grants( statement = statement.where(PermissionGrant.capability == capability) if data_type is not None: statement = statement.where(PermissionGrant.data_type == data_type) + if ui_surface is not None: + statement = statement.where(PermissionGrant.ui_surface == ui_surface) if scope_type is not None: statement = statement.where(PermissionGrant.scope_type == scope_type) if not include_revoked: @@ -206,16 +226,27 @@ def get_access_decision( session: session_dependency, user: viewer_dependency, capability: str = Query(), - data_type: str = Query(), + data_type: str = Query(default=None), + ui_surface: str = Query(default=None), thing_id: int = Query(default=None), on_date: date = Query(default=None), ) -> AccessDecision: """May the caller do this? Answered by the one visibility layer. - Default deny: an unrecognized capability or a caller the token says - nothing about gets False rather than an error, because a question this - layer cannot answer is not a yes. + Ask about a data type or about a UI surface, not both: they are separate + questions, and a call naming both would have two answers. + + Default deny: an unrecognized capability, a caller the token says nothing + about, or a call naming neither subject gets False rather than an error, + because a question this layer cannot answer is not a yes. """ + if data_type and ui_surface: + raise _invalid( + "ui_surface", + "Ask about a data type or a UI surface, not both.", + ui_surface, + ) + principals = principals_from_payload(user) return AccessDecision( allowed=may( @@ -223,11 +254,13 @@ def get_access_decision( principals, capability=capability, data_type=data_type, + ui_surface=ui_surface, thing_id=thing_id, on_date=on_date, ), capability=capability, data_type=data_type, + ui_surface=ui_surface, thing_id=thing_id, principals=[f"{kind}:{identifier}" for kind, identifier in principals], ) diff --git a/core/enums.py b/core/enums.py index 675d7fe1..2bfa5841 100644 --- a/core/enums.py +++ b/core/enums.py @@ -91,5 +91,6 @@ Capability: type[Enum] = build_enum_from_lexicon_category("capability") GrantScopeType: type[Enum] = build_enum_from_lexicon_category("grant_scope_type") AccessDataType: type[Enum] = build_enum_from_lexicon_category("access_data_type") +UISurface: type[Enum] = build_enum_from_lexicon_category("ui_surface") DestinationKind: type[Enum] = build_enum_from_lexicon_category("destination_kind") # ============= EOF ============================================= diff --git a/core/lexicon.json b/core/lexicon.json index 10da407b..761e783b 100644 --- a/core/lexicon.json +++ b/core/lexicon.json @@ -264,6 +264,10 @@ "name": "access_data_type", "description": "The kind of data a grant or a publication consent names. Deliberately coarse, and deliberately without a term meaning 'all' (ADR5, PUB-U11)." }, + { + "name": "ui_surface", + "description": "A screen or navigation item in the admin UI that a grant can open up. Named by the resource identifier the UI already checks, so a term and a nav item cannot drift apart (ADR5)." + }, { "name": "destination_kind", "description": "What sort of place published data is offered to: the public web, a harvesting agency, a partner with a standing connection (ADR5)." @@ -8621,6 +8625,83 @@ "term": "site metadata", "definition": "Where the thing is and what it is: name, location, type, status." }, + { + "categories": [ + "ui_surface" + ], + "term": "ocotillo.map", + "definition": "The map view." + }, + { + "categories": [ + "ui_surface" + ], + "term": "ocotillo.thing-well", + "definition": "The wells list and well records." + }, + { + "categories": [ + "ui_surface" + ], + "term": "ocotillo.thing-well-projects", + "definition": "The projects list." + }, + { + "categories": [ + "ui_surface" + ], + "term": "ocotillo.thing-well-batch-export", + "definition": "Field sheet batch export." + }, + { + "categories": [ + "ui_surface" + ], + "term": "ocotillo.contact", + "definition": "The contacts list and contact records." + }, + { + "categories": [ + "ui_surface" + ], + "term": "ocotillo.collections", + "definition": "The published OGC datasets catalogue." + }, + { + "categories": [ + "ui_surface" + ], + "term": "ocotillo.asset-unassociated", + "definition": "Assets not yet attached to a thing." + }, + { + "categories": [ + "ui_surface" + ], + "term": "ocotillo.location", + "definition": "The locations list." + }, + { + "categories": [ + "ui_surface" + ], + "term": "ocotillo.lexicon", + "definition": "The controlled-vocabulary editor." + }, + { + "categories": [ + "ui_surface" + ], + "term": "ocotillo.hydrograph-correction", + "definition": "The hydrograph correction workbench." + }, + { + "categories": [ + "ui_surface" + ], + "term": "ocotillo.access-grants", + "definition": "This access-control console." + }, { "categories": [ "destination_kind" diff --git a/db/permission_grant.py b/db/permission_grant.py index daa19c7c..e378b2f9 100644 --- a/db/permission_grant.py +++ b/db/permission_grant.py @@ -24,12 +24,22 @@ ``db/publication_consent.py``, a separate table with separate governance and the same grammar. Both are evaluated by ``services/visibility.py``. +A grant names exactly one subject: either a ``data_type`` (what data it +reaches) or a ``ui_surface`` (what screen it opens). Both axes are lexicon +terms, both are nullable in the table, and the XOR between them is enforced in +``domain/access.py`` rather than as a check constraint, so the rule holds for +every writer and reads as one sentence. + Invariants, enforced in ``domain/access.py`` before a row is written: -* ``data_type`` is never null and there is no term meaning "all", so a data - type added later is not covered by an existing grant. +* Exactly one of ``data_type`` / ``ui_surface`` is set. Neither is a wildcard + and there is no term meaning "all", so a data type or screen added later is + not covered by an existing grant. * A ``global`` grant carries no ``scope_id``; a ``group`` or ``thing`` grant requires one. +* A ``ui_surface`` grant is always ``global``. Navigation is app-wide -- the UI + never asks "may I see this nav item *for this well*" -- so a scoped screen + grant would be a row that could never match. * Expiry is read at use. Nothing sweeps this table, so a missed job cannot leave a grant standing past its end date. """ @@ -58,7 +68,10 @@ class PermissionGrant(Base, AutoBaseMixin): capability: Mapped[str] = lexicon_term(nullable=False) scope_type: Mapped[str] = lexicon_term(nullable=False) scope_id: Mapped[Optional[int]] = mapped_column(Integer, nullable=True) - data_type: Mapped[str] = lexicon_term(nullable=False) + # Exactly one of these two is set; see the module docstring. Nullable in + # the table, XOR in domain/access.py. + data_type: Mapped[Optional[str]] = lexicon_term(nullable=True) + ui_surface: Mapped[Optional[str]] = lexicon_term(nullable=True) # --- When --- starts_at: Mapped[date] = mapped_column(nullable=False) @@ -89,9 +102,10 @@ class PermissionGrant(Base, AutoBaseMixin): ) def __str__(self): + subject = self.data_type or self.ui_surface return ( f"{self.principal_type}:{self.principal_id} may {self.capability} " - f"{self.data_type} ({self.scope_type})" + f"{subject} ({self.scope_type})" ) diff --git a/domain/access.py b/domain/access.py index 9bdd3bde..7f0b5747 100644 --- a/domain/access.py +++ b/domain/access.py @@ -31,8 +31,9 @@ * **Default deny.** No grant means no. An empty sequence answers False, and every helper returns False rather than raising when it cannot say yes. -* **No wildcards.** A grant names its data type. There is no term meaning - "all", so a data type added next year is never covered by an existing row. +* **No wildcards.** A grant names exactly one subject: a data type, or a UI + surface. There is no term meaning "all" in either vocabulary, so a data type + or a screen added next year is never covered by an existing row. * **Expiry is checked at use.** Nothing sweeps expired rows; every check compares against the date it is asked about. """ @@ -90,6 +91,14 @@ class MissingDataType(AccessRuleError): pass +class AmbiguousGrantSubject(AccessRuleError): + """A grant named both a data type and a UI surface, or neither.""" + + +class ScopedSurfaceGrant(AccessRuleError): + """A UI-surface grant was scoped to a group or a thing.""" + + class ScopeIdMismatch(AccessRuleError): pass @@ -107,10 +116,13 @@ class Grant: capability: str scope_type: str scope_id: int | None - data_type: str + data_type: str | None starts_at: date ends_at: date | None = None revoked_at: datetime | None = None + # A grant reaches data or a screen, never both. Defaulted so every existing + # construction site keeps meaning a data grant. + ui_surface: str | None = None @dataclass(frozen=True) @@ -136,10 +148,13 @@ class AccessRequest: """ capability: str - data_type: str + data_type: str | None = None principals: tuple[tuple[str, str], ...] = () thing_id: int | None = None group_ids: tuple[int, ...] = field(default_factory=tuple) + # Asked instead of `data_type` when the question is "may this caller see + # this screen". A request names one or the other, like a grant does. + ui_surface: str | None = None def validate_grant( @@ -150,6 +165,7 @@ def validate_grant( data_type: str | None, starts_at: date, ends_at: date | None, + ui_surface: str | None = None, ) -> None: """Reject a grant that could not be evaluated honestly. @@ -173,12 +189,29 @@ def validate_grant( raise ScopeIdMismatch("A global grant names no scope_id.") if scope_type != SCOPE_GLOBAL and scope_id is None: raise ScopeIdMismatch(f"A {scope_type}-scoped grant needs a scope_id.") - if not data_type: + if data_type and ui_surface: + # A row naming both would be two grants wearing one revocation, and + # revoking the data half would silently take the screen away too. + raise AmbiguousGrantSubject( + "A grant names a data type or a UI surface, not both. Write two " + "grants so each can be revoked on its own." + ) + if not data_type and not ui_surface: # The no-wildcard rule. There is deliberately no term meaning "all": # a blanket grant is what published data nobody had agreed to publish. raise MissingDataType( - "A grant names its data type. There is no wildcard, so a new data " - "type is never covered by an existing grant." + "A grant names its data type, or the UI surface it opens. There is " + "no wildcard, so a new data type or screen is never covered by an " + "existing grant." + ) + 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 + # match any request the UI makes, so it is refused at the door rather + # than stored as something that silently never applies. + raise ScopedSurfaceGrant( + f"A UI-surface grant is always global; '{scope_type}' would never " + "match, because the UI never asks about a screen for one thing." ) require_forward_range(starts_at, ends_at) @@ -236,7 +269,7 @@ def grant_covers(grant: Grant, request: AccessRequest, on_date: date) -> bool: return False if grant.capability != request.capability: return False - if grant.data_type != request.data_type: + if not _subject_matches(grant, request): return False if not is_active(grant.starts_at, grant.ends_at, grant.revoked_at, on_date): return False @@ -245,6 +278,20 @@ def grant_covers(grant: Grant, request: AccessRequest, on_date: date) -> bool: ) +def _subject_matches(grant: Grant, request: AccessRequest) -> bool: + """Whether the grant is about the thing being asked about. + + A request names a data type or a UI surface, and so does a grant. Matching + on `None == None` would make a data grant answer a screen question, so an + unasked axis never matches. + """ + if request.ui_surface is not None: + return grant.ui_surface == request.ui_surface + if request.data_type is not None: + return grant.data_type == request.data_type + return False + + def any_grant_allows(grants, request: AccessRequest, on_date: date) -> bool: """Default deny: an empty sequence is a no.""" return any(grant_covers(grant, request, on_date) for grant in grants) diff --git a/schemas/access.py b/schemas/access.py index 082a0e81..6afc50a1 100644 --- a/schemas/access.py +++ b/schemas/access.py @@ -34,6 +34,7 @@ DestinationKind, GrantScopeType, PrincipalType, + UISurface, ) from schemas import UTCAwareDatetime @@ -71,7 +72,11 @@ class CreatePermissionGrant(BaseModel): # rule is enforced in domain/access.py rather than here, so it holds for # every writer, not only this route. scope_id: int | None = None - data_type: AccessDataType + # Exactly one of these. A grant reaches data, or it opens a screen; the + # XOR and the global-only rule for surfaces live in domain/access.py, for + # the same reason the scope rule does. + data_type: AccessDataType | None = None + ui_surface: UISurface | None = None starts_at: date ends_at: date | None = None reason: str | None = None @@ -86,7 +91,8 @@ class PermissionGrantResponse(BaseModel): capability: Capability scope_type: GrantScopeType scope_id: int | None - data_type: AccessDataType + data_type: AccessDataType | None + ui_surface: UISurface | None starts_at: date ends_at: date | None granted_by: str @@ -100,7 +106,8 @@ class AccessDecision(BaseModel): allowed: bool capability: Capability - data_type: AccessDataType + data_type: AccessDataType | None = None + ui_surface: UISurface | None = None thing_id: int | None principals: list[str] diff --git a/services/access_admin.py b/services/access_admin.py index 2d94946b..e86d9d45 100644 --- a/services/access_admin.py +++ b/services/access_admin.py @@ -116,6 +116,7 @@ def create_grant( starts_at: date, ends_at: date = None, reason: str = None, + ui_surface: str = None, ) -> PermissionGrant: validate_grant( principal_type=principal_type, @@ -125,6 +126,7 @@ def create_grant( data_type=data_type, starts_at=starts_at, ends_at=ends_at, + ui_surface=ui_surface, ) grant = PermissionGrant( @@ -134,6 +136,7 @@ def create_grant( scope_type=scope_type, scope_id=scope_id, data_type=data_type, + ui_surface=ui_surface, starts_at=starts_at, ends_at=ends_at, granted_by=actor, @@ -154,6 +157,7 @@ def create_grant( "scope_type": scope_type, "scope_id": scope_id, "data_type": data_type, + "ui_surface": ui_surface, "starts_at": starts_at.isoformat(), "ends_at": ends_at.isoformat() if ends_at else None, "reason": reason, diff --git a/services/visibility.py b/services/visibility.py index 57becb33..9429786c 100644 --- a/services/visibility.py +++ b/services/visibility.py @@ -54,6 +54,7 @@ ) from domain.access import ( AccessRequest, + CAPABILITY_READ, Consent, Grant, PRINCIPAL_ROLE, @@ -124,6 +125,7 @@ def load_grants(session, principals: tuple[tuple[str, str], ...]) -> list[Grant] scope_type=row.scope_type, scope_id=row.scope_id, data_type=row.data_type, + ui_surface=row.ui_surface, starts_at=row.starts_at, ends_at=row.ends_at, revoked_at=row.revoked_at, @@ -136,13 +138,16 @@ def may( session, principals: tuple[tuple[str, str], ...], capability: str, - data_type: str, + data_type: str = None, thing_id: int = None, on_date: date = None, + ui_surface: str = None, ) -> bool: - """May these principals do this, to this data type, at this thing? + """May these principals do this, to this data type or screen, at this thing? - Default deny. No principals, no grants, or nothing matching is a no. + Default deny. No principals, no grants, or nothing matching is a no -- + including a call that names neither a data type nor a UI surface, which is + a question this layer cannot answer and so is not a yes. """ request = AccessRequest( capability=capability, @@ -150,12 +155,36 @@ def may( principals=tuple(principals), thing_id=thing_id, group_ids=group_ids_for_thing(session, thing_id), + ui_surface=ui_surface, ) return any_grant_allows( load_grants(session, request.principals), request, on_date or date.today() ) +def may_see_surface( + session, + principals: tuple[tuple[str, str], ...], + ui_surface: str, + on_date: date = None, +) -> bool: + """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 + 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. + """ + return may( + session, + principals, + capability=CAPABILITY_READ, + ui_surface=ui_surface, + on_date=on_date, + ) + + def destination_by_slug(session, slug: str) -> Destination | None: return session.execute( select(Destination).where(Destination.slug == slug) diff --git a/tests/test_access.py b/tests/test_access.py index 367b1df7..9de015d9 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -380,7 +380,8 @@ def test_a_global_grant_with_a_scope_id_is_rejected(grants): def test_a_grant_naming_no_data_type_cannot_be_written(grants): - """No wildcards. Pydantic rejects it before the service is reached.""" + """No wildcards. A grant naming neither subject is rejected by the rule in + domain/access.py, which the route surfaces as a 422.""" assert make_grant(grants, data_type=None).status_code == 422 @@ -463,3 +464,127 @@ def test_the_log_records_who_and_what(destination, water_well_thing): assert entry.subject_table == "publication_consent" assert entry.detail["thing_id"] == water_well_thing.id assert entry.detail["data_type"] == "water level" + + +# ------ UI-surface grants ---------- +# +# A grant can open a screen instead of reaching data. These go through the +# route so the XOR, the global-only rule, and the decision endpoint are all +# exercised the way the admin console will use them. + + +def test_a_surface_grant_opens_that_screen(grants): + assert ( + make_grant(grants, data_type=None, ui_surface="ocotillo.lexicon").status_code + == 201 + ) + + assert decision(capability="read", 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 + ) + + +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 + ) + + +def test_a_surface_grant_does_not_reach_data(grants): + make_grant(grants, data_type=None, ui_surface="ocotillo.lexicon") + + assert decision(capability="read", data_type="water level")["allowed"] is False + + +def test_a_grant_naming_both_subjects_is_rejected(grants): + response = make_grant(grants, ui_surface="ocotillo.lexicon") + + assert response.status_code == 422 + assert "not both" in response.text + + +def test_a_scoped_surface_grant_is_rejected(grants, water_well_thing): + """It could never match: the UI never asks about a screen for one thing.""" + response = make_grant( + grants, + data_type=None, + ui_surface="ocotillo.lexicon", + scope_type="thing", + scope_id=water_well_thing.id, + ) + + assert response.status_code == 422 + assert "always global" in response.text + + +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 client.post(f"/access/grant/{grant_id}/revocation").status_code == 201 + + assert ( + decision(capability="read", ui_surface="ocotillo.lexicon")["allowed"] is False + ) + + +def test_asking_about_both_subjects_at_once_is_rejected(grants): + """Two questions, two answers. The route refuses rather than picking one.""" + response = client.get( + "/access/decision", + params={ + "capability": "read", + "data_type": "water level", + "ui_surface": "ocotillo.lexicon", + }, + ) + + assert response.status_code == 422 + + +def test_asking_about_neither_subject_is_a_no(grants): + """A question the layer cannot answer is not a yes.""" + assert decision(capability="read")["allowed"] is False + + +def test_listing_grants_filters_by_ui_surface(grants): + surface_id = make_grant( + grants, data_type=None, ui_surface="ocotillo.lexicon" + ).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()] + + assert surface_id in ids + assert data_id not in ids + + +def test_a_surface_grant_is_logged_like_any_other(grants): + make_grant(grants, data_type=None, ui_surface="ocotillo.lexicon") + + with session_ctx() as session: + logged = ( + session.execute( + select(AuthorizationAudit).where( + AuthorizationAudit.actor == ADMIN_PAYLOAD["sub"] + ) + ) + .scalars() + .all() + ) + + assert any(entry.detail.get("ui_surface") == "ocotillo.lexicon" for entry in logged) + + +# ============= EOF ============================================= diff --git a/tests/test_domain_access.py b/tests/test_domain_access.py index 0362794e..15fd9d82 100644 --- a/tests/test_domain_access.py +++ b/tests/test_domain_access.py @@ -24,10 +24,12 @@ CAPABILITIES, PRINCIPAL_TYPES, SCOPE_TYPES, + AmbiguousGrantSubject, BackwardsDateRange, Consent, Grant, MissingDataType, + ScopedSurfaceGrant, ScopeIdMismatch, UnknownCapability, UnknownPrincipalType, @@ -270,3 +272,140 @@ def test_withdrawn_consent_stops_being_offered_immediately(): def test_nothing_published_without_a_consent_row(): assert any_consent_publishes([], 7, 2, "water level", TODAY) is False + + +# ------ UI-surface grants ---------- +# +# A grant reaches data, or it opens a screen. These pin the XOR and the +# global-only rule, and that a data grant never answers a screen question. + + +def a_surface_grant(**overrides): + fields = { + "principal_type": "user", + "principal_id": "authentik-sub-1", + "capability": "read", + "scope_type": "global", + "scope_id": None, + "data_type": None, + "ui_surface": "ocotillo.lexicon", + "starts_at": date(2026, 1, 1), + "ends_at": None, + "revoked_at": None, + } + fields.update(overrides) + return Grant(**fields) + + +def a_surface_request(**overrides): + fields = { + "capability": "read", + "data_type": None, + "ui_surface": "ocotillo.lexicon", + "principals": (STUDENT,), + "thing_id": None, + "group_ids": (), + } + fields.update(overrides) + return AccessRequest(**fields) + + +def test_a_surface_grant_opens_that_screen(): + assert any_grant_allows([a_surface_grant()], a_surface_request(), TODAY) is True + + +def test_a_surface_grant_does_not_open_another_screen(): + assert ( + grant_covers( + a_surface_grant(), + a_surface_request(ui_surface="ocotillo.location"), + TODAY, + ) + is False + ) + + +def test_a_data_grant_does_not_answer_a_screen_question(): + """Both carry None on the axis not being asked about; None must not match.""" + assert grant_covers(a_grant(), a_surface_request(), TODAY) is False + + +def test_a_surface_grant_does_not_answer_a_data_question(): + assert grant_covers(a_surface_grant(), a_request(), TODAY) is False + + +def test_a_request_naming_neither_subject_is_a_no(): + """A question this layer cannot answer is not a yes.""" + assert ( + any_grant_allows( + [a_grant(), a_surface_grant()], + a_surface_request(ui_surface=None, data_type=None), + TODAY, + ) + is False + ) + + +def test_a_surface_grant_expires_like_any_other(): + grant = a_surface_grant(ends_at=date(2026, 8, 23)) + assert grant_covers(grant, a_surface_request(), TODAY) is False + + +def test_a_surface_grant_is_dead_once_revoked(): + grant = a_surface_grant(revoked_at=datetime(2026, 8, 1, tzinfo=timezone.utc)) + assert grant_covers(grant, a_surface_request(), TODAY) is False + + +# ------ validation of the two subjects ---------- + + +def test_a_grant_naming_both_subjects_is_rejected(): + """Two grants in one row would share a revocation.""" + with pytest.raises(AmbiguousGrantSubject): + validate_grant( + "user", + "read", + "global", + None, + "water level", + TODAY, + None, + ui_surface="ocotillo.lexicon", + ) + + +def test_a_grant_naming_neither_subject_is_rejected(): + with pytest.raises(MissingDataType): + validate_grant("user", "read", "global", None, None, TODAY, None) + + +def test_a_surface_grant_is_accepted_without_a_data_type(): + validate_grant( + "user", + "read", + "global", + None, + None, + TODAY, + None, + ui_surface="ocotillo.lexicon", + ) + + +@pytest.mark.parametrize("scope_type,scope_id", [("thing", 7), ("group", 3)]) +def test_a_scoped_surface_grant_is_rejected(scope_type, scope_id): + """It could never match: the UI never asks about a screen for one thing.""" + with pytest.raises(ScopedSurfaceGrant): + validate_grant( + "user", + "read", + scope_type, + scope_id, + None, + TODAY, + None, + ui_surface="ocotillo.lexicon", + ) + + +# ============= EOF =============================================