Skip to content

control-plane-api: create_data_plane gates ops admin via Snapshot - #3435

Open
bbartman wants to merge 4 commits into
bmb/2781-publications-spec-authzfrom
bmb/strangle-1-create-data-plane
Open

control-plane-api: create_data_plane gates ops admin via Snapshot#3435
bbartman wants to merge 4 commits into
bmb/2781-publications-spec-authzfrom
bmb/strangle-1-create-data-plane

Conversation

@bbartman

@bbartman bbartman commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Part 1 of a 3-PR stack strangling the last reachable Rust call sites of internal.user_roles(), continuing the Snapshot-authorization work of #2781. Stacked on the publications-spec-authz branch.

Plan

After the publications migration, exactly three reachable Rust paths still authorize via internal.user_roles() SQL:

  1. /admin/create-data-plane — this PR.
  2. /admin/update-l2-reporting — next PR, an identical swap.
  3. The storage-mappings directive (user_has_admin_capability) — final PR, which must thread the agent's snapshot_watch into DirectiveHandler.

Everything else that references user_roles/auth_roles is SQL-land reachable only via PostgREST/RLS (live_specs_ext, combined_grants_ext, gateway_auth_token, …) and is out of scope for this stack. In particular, the #2848 workaround ensure_private_data_plane_grants stays: its removal is gated on the RLS-side migration, not on these three.

This change

  • Swaps create_data_plane's SQL gate for evaluate_names_authorization(snapshot, claims, Admin, ["ops/"]), evaluated through Envelope::authorization_outcome. Denials now follow the standard authorization-retry protocol (early Snapshot refresh + 307 retry when the Snapshot may be stale) instead of failing terminally, matching the authorize_* endpoints. This is an HTTP-envelope mechanism; GraphQL resolvers and the (upcoming) directive handler instead take one-shot pinned-Snapshot checks.
  • The denial message becomes the crate-standard "…is not authorized to access prefix or name 'ops/' with required capability admin". (An earlier revision introduced a dedicated evaluate_ops_admin helper with a bespoke message; review flagged it as a Middle Man over evaluate_names_authorization and it was folded away.)
  • Adds shared test fixtures make_grants / snapshot_of_grants to test_server, deduplicating the grant-table construction previously repeated in the authorized_prefixes tests.

Deliberate semantic change

internal.user_roles() walks the grant graph only downward from roles the user holds. The Snapshot walk (tables::UserGrant::is_authorized) also reaches grants held by ancestor subject roles — the same gap tracked by #2848. An admin chain into ops/ through an ancestor subject (e.g. admin of estuary/support/ with a (estuary/ → ops/, admin) grant) was wrongly denied by the SQL gate and is authorized here. The named test test_evaluate_ops_admin_ancestor_subject_chain pins this.

Tests

  • Unit tests in server::tests pin the ops-gate semantics: direct ops/ admin, unrelated-tenant admin denied (with the exact denial message), read ≠ admin, the ancestor-subject chain allowed, and the same chain denied without admin capability.
  • Endpoint tests cover the denial paths only (no bearer → 401, non-ops-admin → terminal 403 with the expected message). An authorized request proceeds into data-plane provisioning, which authorization tests have no business exercising — so allowed cases live in the unit tests.

Review

An xhigh two-axis review (standards + spec) ran against this PR; its findings — a named-test granularity nit, the Middle Man helper, and the duplicated test fixture — are addressed in the two follow-up commits.

The /admin/create-data-plane and /admin/update-l2-reporting endpoints
gate on the user being an admin of 'ops/', each via an identical SQL
query over internal.user_roles(). As part of strangling that function
out of reachable Rust paths, introduce a Snapshot-based policy helper
shaped for Envelope::authorization_outcome, alongside the existing
evaluate_names_authorization.

The helper deliberately changes semantics: user_roles() walks the grant
graph only downward from roles the user holds, while the Snapshot walk
also reaches grants held by ancestor subject roles — the same gap
tracked by #2848. An admin chain into 'ops/' through an ancestor
subject was wrongly denied by the SQL gate and is authorized here. The
unit test pins this case by name.

Endpoints are switched to the helper in follow-up commits.
Swap the endpoint's internal.user_roles() SQL gate for
evaluate_ops_admin over the request's pinned Snapshot, evaluated
through Envelope::authorization_outcome. Beyond strangling the SQL
function, this upgrades a denial from a terminal 403 to the standard
retry protocol: a freshly-granted ops admin triggers an early Snapshot
refresh and a 307 retry rather than a hard failure, matching the
authorize_* endpoints.

The identical SQL gate in update_l2_reporting is migrated separately,
in the next change of this stack.

Endpoint tests cover the denial paths only: an authorized request
proceeds into data-plane provisioning, which authorization tests have
no business exercising. Allowed cases are pinned by evaluate_ops_admin
unit tests.
Review of the stack's first change asked that the deliberate semantic
divergence from internal.user_roles() — authorizing admin chains
reached through ancestor subject roles — be pinned by a named test,
rather than a commented section inside the combined case list.
Review flagged evaluate_ops_admin as a possible Middle Man: it was
evaluate_names_authorization with a fixed Admin capability, a fixed
'ops/' prefix, and a bespoke error message. The bespoke message isn't
worth a second walk implementation — and the generic message is the
denial text every other endpoint already emits — so the endpoint now
calls evaluate_names_authorization directly. The named tests keep
pinning the ops-gate semantics, including the ancestor-subject chain
divergence from internal.user_roles().

Review also flagged the test-side grant-building duplication: the new
snapshot_of_grants helper repeated make_grants from the
authorized_prefixes tests. Both now share one fixture in test_server.
@bbartman
bbartman force-pushed the bmb/strangle-1-create-data-plane branch from fc54bc0 to e167eca Compare September 2, 2026 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant