authz: scope a request's authority to one branch of the grant graph - #3407
Draft
GregorShear wants to merge 1 commit into
Draft
authz: scope a request's authority to one branch of the grant graph#3407GregorShear wants to merge 1 commit into
GregorShear wants to merge 1 commit into
Conversation
Adds an `X-Estuary-Scope-Prefix` header which narrows a request to the part of the user's access reachable from a named catalog prefix. Authorization computes the prefixes reachable from the user's grants, computes the prefixes reachable from the scope prefix, and intersects them. A user who admins `acmeCo/` and `betaCo/`, where `acmeCo/` holds a role grant to `charlieCo/`, sees `acmeCo/` and `charlieCo/` under a scope of `acmeCo/` and does not see `betaCo/`. The scope side of that intersection is `RoleGrant::scope_nodes`, which walks role grants in both prefix directions the way `next_neighbors` already does. So a scope of `acmeCo/team/` still reaches `charlieCo/`: the role grant hangs off `acmeCo/`, and a grant on `acmeCo/` reaches everything under it. Because the result is an intersection with the user's own reachable prefixes, a scope can only remove authority. A scope naming a prefix the user cannot reach yields nothing rather than access to it, and no scope value can produce a prefix outside the unscoped set. That property is what makes the header safe to accept from the client, so the dashboard can use it as a tenant selector and switch freely without re-authenticating. Authorization now takes a `tables::Principal`, pairing the user id with the optional scope, instead of a bare `Uuid`. Handlers build one from `Envelope::principal`. Making it the only accepted input means a call site cannot honor the token while overlooking the scope; every existing site was updated because the signature change made it a compile error. Discovers keep unscoped authority. They authorize against the user recorded on the job rather than the request that enqueued it, so no scope reaches them.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds an
X-Estuary-Scope-Prefixheader that narrows a request to one branch of the caller's authorization graph:Authorization computes the prefixes reachable from the user's grants, computes the prefixes reachable from the scope prefix, and intersects them. A user who admins
acmeCo/andbetaCo/, whereacmeCo/holds a role grant tocharlieCo/, seesacmeCo/andcharlieCo/under this header and does not seebetaCo/.Why the header can be trusted from the client
The result is an intersection with the user's own reachable prefixes, so a scope can only remove authority:
test_scope_cannot_widenasserts this over a set of scopes including ones outside the user's grants.That is what makes it usable as a dashboard tenant selector. The user picks which tenant they are working in and switches freely, with no token refresh and no server-side state, because switching is re-narrowing from an unscoped token rather than widening a scoped one.
A credential that must not be able to widen its own reach is a separate problem, and would carry its scope in the access token rather than the request. This PR does not add that.
Prefix direction
The scope side of the intersection is
RoleGrant::scope_nodes, which walks role grants in both prefix directions the waynext_neighborsalready does. So a scope ofacmeCo/team/still reachescharlieCo/— the role grant hangs offacmeCo/, and a grant onacmeCo/reaches everything under it.Making the scope impossible to skip
Authorization now takes a
tables::Principal(user id plus optional scope) instead of a bareUuid. Handlers build one fromEnvelope::principal. Since it is the only accepted input, a call site cannot honor the token while overlooking the scope. Every existing call site was updated because the signature change made each one a compile error.Decisions worth a reviewer's attention
Capability is intersected, not maxed. If a user admins
charlieCo/ops/directly and reachescharlieCo/throughacmeCo/'s read grant, then under a scope ofacmeCo/they hold Read oncharlieCo/ops/, not Admin. The reading is that scoping toacmeCo/means "whatacmeCo/can reach, asacmeCo/reaches it," so a grant from outside the scope is not in play. Intersecting is the conservative choice; taking the max would be the risky one. Covered bytest_scope_caps_capability_at_what_it_delegates. Say so if you want the other behavior.Discovers stay unscoped. They authorize against the user recorded on the job, not the request that enqueued it, so no scope reaches them. Threading one through would mean persisting it on the job row. Marked with a comment at both call sites.
An empty header value is rejected rather than treated as unscoped. It would otherwise match every prefix and silently do nothing; a client meaning "no scope" omits the header.
Testing
Verified:
cargo test -p tables— 41 pass, including 4 new scope tests (branch selection, ancestor delegations under a narrower scope, capability intersection, and the cannot-widen property).envelope.rs.cargo check -p tables -p control-plane-api -p agentclean, no warnings.cargo fmt --checkclean.Not run locally: the
control-plane-apitest suite. Its test-onlysqlx::query!macros need a live database, andmise run local:supabaserequires systemd, which is unavailable on this macOS host. The same 5 errors reproduce on a cleanorigin/masterin this worktree, so they are environmental and not from this change. These tests need to pass in CI or inside the Lima VM before merge.No GraphQL schema change, so no
flow-clientregeneration. No SQL change, so nocargo sqlx prepare.Follow-ups not in this PR
refresh_tokensrow following thepg_rolepattern and be non-widenable.