Files
coder/coderd/rbac/scopes_test.go
T
Bobby HoandClaude Opus 5 dea3a2944a feat(coderd/rbac): compare scopes by permission coverage (#28167)
Split out of #28045
([PLAT-479](https://linear.app/codercom/issue/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 fail-closed rules are the part worth reviewing:

- Coverage models site-level grants only. A scope carrying org or user
permissions, a negative permission, or a narrowed allow list is refused
on either side rather than compared on the part that is modeled.
Answering "covered" from the fraction that was read would report
authority that was never examined.
- An unknown name on either side is an error, not a false, since a
caller cannot tell "not covered" apart from "could not decide".
- A wildcard request is covered only by a wildcard grant. Enumerating
every workspace action that exists today does not cover workspace:*,
because the wildcard also authorizes the actions added tomorrow.

**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. Both sides of ScopesCover must already be
canonical, which its parameter names restate at every call site.

Covered by 17 coverage cases, guard cases that drive each unmodeled
shape through both sides of the comparison, and an exhaustive sweep over
the external catalog: coder:all covers all of it, and every catalog name
covers itself.

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-20 09:16:04 -07:00

255 lines
8.4 KiB
Go

package rbac_test
import (
"testing"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/coderd/rbac/policy"
)
func TestExpandScope(t *testing.T) {
t.Parallel()
t.Run("low_level_pairs", func(t *testing.T) {
t.Parallel()
cases := []struct {
name string
resource string
action policy.Action
}{
{name: "workspace:start", resource: rbac.ResourceWorkspace.Type, action: policy.ActionWorkspaceStart},
{name: "workspace:ssh", resource: rbac.ResourceWorkspace.Type, action: policy.ActionSSH},
{name: "template:use", resource: rbac.ResourceTemplate.Type, action: policy.ActionUse},
{name: "api_key:read", resource: rbac.ResourceApiKey.Type, action: policy.ActionRead},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
s, err := rbac.ScopeName(tc.name).Expand()
require.NoError(t, err)
// site-only single permission
require.Len(t, s.Site, 1)
require.Equal(t, tc.resource, s.Site[0].ResourceType)
require.Equal(t, tc.action, s.Site[0].Action)
require.Empty(t, s.ByOrgID)
require.Empty(t, s.User)
require.Equal(t, []rbac.AllowListElement{rbac.AllowListAll()}, s.AllowIDList)
})
}
})
t.Run("invalid_low_level", func(t *testing.T) {
t.Parallel()
invalid := []string{
"", // empty
"workspace:", // missing action
":read", // missing resource
"unknown:read", // unknown resource
"workspace:bogus", // unknown action
"a:b:c", // too many parts
}
for _, name := range invalid {
t.Run(name, func(t *testing.T) {
t.Parallel()
_, err := rbac.ScopeName(name).Expand()
require.Error(t, err)
})
}
})
}
func TestScopesCover(t *testing.T) {
t.Parallel()
tests := []struct {
name string
allowed []rbac.ScopeName
requested rbac.ScopeName
want bool
// wantErrContains names the side that could not be expanded. Asserting
// the side keeps rows that fail through different paths from passing
// on each other's errors.
wantErrContains string
}{
{
name: "IdenticalName",
allowed: []rbac.ScopeName{"workspace:read"},
requested: "workspace:read",
want: true,
},
{
// Name matching alone cannot decide this: coder:workspaces.access
// never mentions workspace:ssh by name, but its expansion covers it.
name: "CompositeCoversItsMember",
allowed: []rbac.ScopeName{"coder:workspaces.access"},
requested: "workspace:ssh",
want: true,
},
{
// The composite grants no permission on this resource at all.
name: "CompositeDoesNotCoverUnrelatedResource",
allowed: []rbac.ScopeName{"coder:workspaces.access"},
requested: "user_secret:delete",
want: false,
},
{
// Same resource, action the composite does not grant. Coverage
// compares the pair, not the resource alone.
name: "CompositeDoesNotCoverUngrantedActionOnSameResource",
allowed: []rbac.ScopeName{"coder:workspaces.access"},
requested: "workspace:delete",
want: false,
},
{
name: "AllCoversEverything",
allowed: []rbac.ScopeName{rbac.ScopeAll},
requested: "user_secret:delete",
want: true,
},
{
name: "NarrowScopeDoesNotCoverAll",
allowed: []rbac.ScopeName{"workspace:read"},
requested: rbac.ScopeAll,
want: false,
},
{
name: "ResourceWildcardCoversOneAction",
allowed: []rbac.ScopeName{"workspace:*"},
requested: "workspace:ssh",
want: true,
},
{
name: "OneActionDoesNotCoverResourceWildcard",
allowed: []rbac.ScopeName{"workspace:ssh"},
requested: "workspace:*",
want: false,
},
{
// A composite is covered only when every permission it expands
// to is granted, so a strict subset of them is not enough.
name: "PartialUnionDoesNotCoverComposite",
allowed: []rbac.ScopeName{"template:read", "file:create"},
requested: "coder:templates.build",
want: false,
},
{
// The allowed side is a union rather than a set of independent
// candidates, so one composite's permissions may be drawn from
// several allowed entries at once.
name: "UnionOfAllowedScopesCoversComposite",
allowed: []rbac.ScopeName{"template:read", "file:*", "provisioner_jobs:read"},
requested: "coder:templates.build",
want: true,
},
{
name: "EmptyAllowedCoversNothing",
allowed: nil,
requested: "workspace:read",
want: false,
},
{
// Not a false: a caller cannot distinguish "known and not
// covered" from "we could not tell", so an undecidable
// comparison is surfaced rather than answered.
name: "UnknownRequestedScopeErrors",
allowed: []rbac.ScopeName{rbac.ScopeAll},
requested: "not_a_real_scope",
wantErrContains: "expand requested scope",
},
{
name: "UnknownAllowedScopeErrors",
allowed: []rbac.ScopeName{"not_a_real_scope"},
requested: "workspace:read",
wantErrContains: "expand allowed scope",
},
{
// The aliases IsExternalScope accepts are not expandable names,
// so callers must canonicalize before asking about coverage.
name: "NonCanonicalAliasErrorsRequestedAll",
allowed: []rbac.ScopeName{rbac.ScopeAll},
requested: "all",
wantErrContains: "expand requested scope",
},
{
name: "NonCanonicalAliasErrorsRequestedApplicationConnect",
allowed: []rbac.ScopeName{rbac.ScopeAll},
requested: "application_connect",
wantErrContains: "expand requested scope",
},
{
// The only row that would catch someone canonicalizing inside the
// allowed loop, which would silently widen what an allow list
// accepts without any caller asking for it.
name: "NonCanonicalAliasErrorsAllowedApplicationConnect",
allowed: []rbac.ScopeName{"application_connect"},
requested: rbac.ScopeApplicationConnect,
wantErrContains: "expand allowed scope",
},
{
name: "NonCanonicalAliasErrorsAllowedAll",
allowed: []rbac.ScopeName{"all"},
requested: rbac.ScopeAll,
wantErrContains: "expand allowed scope",
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()
got, err := rbac.ScopesCover(test.allowed, test.requested)
if test.wantErrContains != "" {
require.ErrorContains(t, err, test.wantErrContains)
require.False(t, got, "an undecided comparison must not report coverage")
return
}
require.NoError(t, err)
require.Equal(t, test.want, got)
})
}
}
// TestCanonicalScopeName pins the alias mapping. Without it the two case arms
// could be swapped, so that requesting `all` persisted application_connect and
// the reverse, and the suite would stay green.
func TestCanonicalScopeName(t *testing.T) {
t.Parallel()
require.Equal(t, rbac.ScopeAll, rbac.CanonicalScopeName("all"))
require.Equal(t, rbac.ScopeApplicationConnect, rbac.CanonicalScopeName("application_connect"))
// Canonical names and low-level names are returned unchanged, so callers
// can canonicalize unconditionally.
require.Equal(t, rbac.ScopeAll, rbac.CanonicalScopeName(rbac.ScopeAll))
require.Equal(t, rbac.ScopeApplicationConnect, rbac.CanonicalScopeName(rbac.ScopeApplicationConnect))
require.Equal(t, rbac.ScopeName("workspace:read"), rbac.CanonicalScopeName("workspace:read"))
require.Equal(t, rbac.ScopeName("not_a_real_scope"), rbac.CanonicalScopeName("not_a_real_scope"))
}
// TestScopesCoverEveryExternalScope asserts the property the OAuth2 allowlist
// check depends on: coder:all is a ceiling over the whole external catalog, and
// every catalog name covers itself. A name that cannot be compared at all would
// otherwise reject every request naming it, which is a rejection no app owner
// could act on.
func TestScopesCoverEveryExternalScope(t *testing.T) {
t.Parallel()
// The aliases inherit this without being named here: TestScopeAliases pins
// that each one canonicalizes to a name this list offers.
for _, name := range rbac.ExternalScopeNames() {
scope := rbac.ScopeName(name)
covered, err := rbac.ScopesCover([]rbac.ScopeName{rbac.ScopeAll}, scope)
require.NoErrorf(t, err, "coder:all vs %q", scope)
require.Truef(t, covered, "coder:all must cover %q", scope)
covered, err = rbac.ScopesCover([]rbac.ScopeName{scope}, scope)
require.NoErrorf(t, err, "%q vs itself", scope)
require.Truef(t, covered, "%q must cover itself", scope)
}
}