Skip to content

feat: compare scopes by permission coverage - #28167

Open
BobbyHo wants to merge 1 commit into
mainfrom
plat479-1-rbac-scope-coverage
Open

feat: compare scopes by permission coverage#28167
BobbyHo wants to merge 1 commit into
mainfrom
plat479-1-rbac-scope-coverage

Conversation

@BobbyHo

@BobbyHo BobbyHo commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Split out of #28045 (PLAT-479) so the coverage rules can be reviewed on their own. First of three. No behavior change: both functions are added without production callers, and the OAuth2 authorize endpoint consumes them in the next PR.

ScopesCover(allowed, requested) reports whether every permission the request grants is also granted by the allowlist. It expands both sides and compares permissions rather than names, so coder:workspaces.access covers workspace:read without ever naming it.

The asymmetry is the part worth reviewing:

  • Unmodelled positive permissions on the allowed side are dropped. That can only make the answer stricter.
  • Anything unmodelled on the requested side is an error, not a false. Answering "covered" about authority that was never compared is the failure that matters.
  • Negative permissions fail closed on both sides. Dropping an anti-grant from the ceiling would widen it, not narrow it.
  • An unknown name on either side is an error, since a caller cannot tell "not covered" apart from "could not decide".

CanonicalScopeName maps the aliases IsExternalScope accepts, all and application_connect, onto the api_key_scope enum spellings. IsExternalScope answers whether a name may be requested, not how it is spelled once persisted, so anything storing a validated name has to canonicalize in between.

Covered by 14 coverage cases plus a property test: coder:all covers the whole external catalog, and every catalog name covers itself.

Stack: this PR, then negotiate and persist the scope, then the consent page and invalid_scope redirect.

Add ScopesCover, which reports whether every permission a requested scope
grants is also granted by at least one of a set of allowed scopes. It
expands both sides and compares the resulting permissions, so
coder:workspaces.access covers workspace:read even though it never names
it, and coder:all covers everything.

The comparison is deliberately asymmetric. Positive permissions on the
allowed side that it does not model are dropped, which can only make the
answer stricter. Anything unmodelled on the requested side is an error
instead, because ignoring it would answer "covered" about authority that
was never compared. Negative permissions are the exception and fail closed
on both sides, since dropping an anti-grant from the ceiling would widen
it rather than narrow it.

Add CanonicalScopeName, which maps the backward-compatibility aliases
IsExternalScope accepts onto the names the api_key_scope enum stores.
IsExternalScope answers whether a name may be requested, not how that name
is spelled once persisted, so a caller that stores what it validated has
to canonicalize in between.

Both functions are added without production callers. The OAuth2 authorize
endpoint uses them to negotiate a requested scope against an app's
configured allowlist, which follows in a separate change.
@BobbyHo BobbyHo changed the title feat(coderd/rbac): compare scopes by permission coverage feat: compare scopes by permission coverage Aug 14, 2026
@BobbyHo
BobbyHo marked this pull request as ready for review August 14, 2026 18:06
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