From 679179f4045cd6a1439eabc2cb5d11d08fa46c8f Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 17 Sep 2025 15:32:14 +0200 Subject: [PATCH] feat: scope allow_list to include `resource_type` (#19748) This feature allows the `allow_list` in the scopes to specify the `type` --- coderd/rbac/README.md | 2 +- coderd/rbac/astvalue.go | 18 +++- coderd/rbac/authz_internal_test.go | 133 ++++++++++++++++++++++++++++- coderd/rbac/input.json | 8 +- coderd/rbac/policy.rego | 31 +++++-- coderd/rbac/scopes.go | 35 +++++--- 6 files changed, 203 insertions(+), 24 deletions(-) diff --git a/coderd/rbac/README.md b/coderd/rbac/README.md index 78781d3660..d7aa89d6de 100644 --- a/coderd/rbac/README.md +++ b/coderd/rbac/README.md @@ -169,7 +169,7 @@ This command answers the question: “Is the user allowed?” ### Partial Evaluation ```bash -opa eval --partial --format=pretty 'data.authz.allow' -d policy.rego --unknowns input.object.owner --unknowns input.object.org_owner --unknowns input.object.acl_user_list --unknowns input.object.acl_group_list -i input.json +opa eval --partial --format=pretty 'data.authz.allow' -d policy.rego --unknowns input.object.id --unknowns input.object.owner --unknowns input.object.org_owner --unknowns input.object.acl_user_list --unknowns input.object.acl_group_list -i input.json ``` This command performs a partial evaluation of the policy, specifying a set of unknown input parameters. diff --git a/coderd/rbac/astvalue.go b/coderd/rbac/astvalue.go index e2fcedbd43..a125b6bf7a 100644 --- a/coderd/rbac/astvalue.go +++ b/coderd/rbac/astvalue.go @@ -182,9 +182,25 @@ func (s Scope) regoValue() ast.Value { if !ok { panic("developer error: role is not an object") } + + terms := make([]*ast.Term, len(s.AllowIDList)) + for i, v := range s.AllowIDList { + terms[i] = ast.NewTerm(ast.NewObject( + [2]*ast.Term{ + ast.StringTerm("type"), + ast.StringTerm(v.Type), + }, + [2]*ast.Term{ + ast.StringTerm("id"), + ast.StringTerm(v.ID), + }, + ), + ) + } + r.Insert( ast.StringTerm("allow_list"), - ast.NewTerm(regoSliceString(s.AllowIDList...)), + ast.NewTerm(ast.NewArray(terms...)), ) return r } diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index 9e7ec07b6e..284045b11f 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -929,7 +929,7 @@ func TestAuthorizeScope(t *testing.T) { Org: map[string][]Permission{}, User: []Permission{}, }, - AllowIDList: []string{workspaceID.String()}, + AllowIDList: []AllowListElement{{Type: ResourceWorkspace.Type, ID: workspaceID.String()}}, }, } @@ -1019,7 +1019,9 @@ func TestAuthorizeScope(t *testing.T) { User: []Permission{}, }, // Empty string allow_list is allowed for actions like 'create' - AllowIDList: []string{""}, + AllowIDList: []AllowListElement{{ + Type: ResourceWorkspace.Type, ID: "", + }}, }, } @@ -1145,7 +1147,7 @@ func TestAuthorizeScope(t *testing.T) { ResourceUser.Type: {policy.ActionRead}, }), }, - AllowIDList: []string{policy.WildcardSymbol}, + AllowIDList: []AllowListElement{AllowListAll()}, }, } @@ -1163,6 +1165,131 @@ func TestAuthorizeScope(t *testing.T) { ) } +func TestScopeAllowList(t *testing.T) { + t.Parallel() + + defOrg := uuid.New() + + // Some IDs to use + wid := uuid.New() + gid := uuid.New() + + user := Subject{ + ID: "me", + Roles: Roles{ + must(RoleByName(RoleOwner())), + }, + Scope: Scope{ + Role: Role{ + Identifier: RoleIdentifier{ + Name: "AllowList", + OrganizationID: defOrg, + }, + DisplayName: "AllowList", + // Allow almost everything + Site: allPermsExcept(ResourceUser), + }, + AllowIDList: []AllowListElement{ + {Type: ResourceWorkspace.Type, ID: wid.String()}, + {Type: ResourceWorkspace.Type, ID: ""}, // Allow to create + {Type: ResourceTemplate.Type, ID: policy.WildcardSymbol}, + {Type: ResourceGroup.Type, ID: gid.String()}, + + // This scope allows all users, but the permissions do not. + {Type: ResourceUser.Type, ID: policy.WildcardSymbol}, + }, + }, + } + + testAuthorize(t, "AllowList", user, + // Allowed: + cases(func(c authTestCase) authTestCase { + c.allow = true + return c + }, + []authTestCase{ + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID).WithID(wid), actions: []policy.Action{policy.ActionRead}}, + // matching on empty id + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), actions: []policy.Action{policy.ActionCreate}}, + + // Template has wildcard ID, so any uuid is allowed, including the empty + {resource: ResourceTemplate.InOrg(defOrg).WithID(uuid.New()), actions: AllActions()}, + {resource: ResourceTemplate.InOrg(defOrg).WithID(uuid.New()), actions: AllActions()}, + {resource: ResourceTemplate.InOrg(defOrg), actions: AllActions()}, + + // Group + {resource: ResourceGroup.InOrg(defOrg).WithID(gid), actions: []policy.Action{policy.ActionRead}}, + }, + ), + + // Not allowed: + cases(func(c authTestCase) authTestCase { + c.allow = false + return c + }, + []authTestCase{ + // Has the scope and allow list, but not the permission + {resource: ResourceUser.WithOwner(user.ID), actions: []policy.Action{policy.ActionRead}}, + + // `wid` matches on the uuid, but not the type + {resource: ResourceGroup.WithID(wid), actions: []policy.Action{policy.ActionRead}}, + + // no empty id for the create action + {resource: ResourceGroup.InOrg(defOrg), actions: []policy.Action{policy.ActionCreate}}, + }, + ), + ) + + // Wildcard type + user = Subject{ + ID: "me", + Roles: Roles{ + must(RoleByName(RoleOwner())), + }, + Scope: Scope{ + Role: Role{ + Identifier: RoleIdentifier{ + Name: "WildcardType", + OrganizationID: defOrg, + }, + DisplayName: "WildcardType", + // Allow almost everything + Site: allPermsExcept(ResourceUser), + }, + AllowIDList: []AllowListElement{ + {Type: policy.WildcardSymbol, ID: wid.String()}, + }, + }, + } + + testAuthorize(t, "WildcardType", user, + // Allowed: + cases(func(c authTestCase) authTestCase { + c.allow = true + return c + }, + []authTestCase{ + // anything with the id is ok + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID).WithID(wid), actions: []policy.Action{policy.ActionRead}}, + {resource: ResourceGroup.InOrg(defOrg).WithID(wid), actions: []policy.Action{policy.ActionRead}}, + {resource: ResourceTemplate.InOrg(defOrg).WithID(wid), actions: []policy.Action{policy.ActionRead}}, + }, + ), + + // Not allowed: + cases(func(c authTestCase) authTestCase { + c.allow = false + return c + }, + []authTestCase{ + // Anything without the id is not allowed + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), actions: []policy.Action{policy.ActionCreate}}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID).WithID(uuid.New()), actions: []policy.Action{policy.ActionRead}}, + }, + ), + ) +} + // cases applies a given function to all test cases. This makes generalities easier to create. func cases(opt func(c authTestCase) authTestCase, cases []authTestCase) []authTestCase { if opt == nil { diff --git a/coderd/rbac/input.json b/coderd/rbac/input.json index b1e8428d71..b71590c789 100644 --- a/coderd/rbac/input.json +++ b/coderd/rbac/input.json @@ -1,5 +1,5 @@ { - "action": "never-match-action", + "action": "read", "object": { "id": "9046b041-58ed-47a3-9c3a-de302577875a", "owner": "00000000-0000-0000-0000-000000000000", @@ -40,7 +40,11 @@ ], "org": {}, "user": [], - "allow_list": ["*"] + "allow_list": [ + { + "type": "workspace", + "id": "*" + }] } } } diff --git a/coderd/rbac/policy.rego b/coderd/rbac/policy.rego index c0b747fef0..eb9187338c 100644 --- a/coderd/rbac/policy.rego +++ b/coderd/rbac/policy.rego @@ -246,18 +246,39 @@ user_allow(roles) := num if { num := number(allow) } -# Scope allow_list is a list of resource IDs explicitly allowed by the scope. -# If the list is '*', then all resources are allowed. +# Scope allow_list is a list of resource (Type, ID) tuples explicitly allowed by the scope. +# If the list contains `(*,*)`, then all resources are allowed. scope_allow_list if { - "*" in input.subject.scope.allow_list + input.subject.scope.allow_list[_] == {"type": "*", "id": "*"} } +# This is a shortcut if the allow_list contains (type, *), then allow all IDs of that type. +scope_allow_list if { + input.subject.scope.allow_list[_] == {"type": input.object.type, "id": "*"} +} + +# A comprehension that iterates over the allow_list and checks if the +# (object.type, object.id) is in the allowed ids. scope_allow_list if { # If the wildcard is listed in the allow_list, we do not care about the # object.id. This line is included to prevent partial compilations from # ever needing to include the object.id. - not "*" in input.subject.scope.allow_list - input.object.id in input.subject.scope.allow_list + not {"type": "*", "id": "*"} in input.subject.scope.allow_list + # This is equivalent to the above line, as `type` is known at partial query time. + not {"type": input.object.type, "id": "*"} in input.subject.scope.allow_list + + # allows_ids is the set of all ids allowed for the given object.type + allowed_ids := {allowed_id | + # Iterate over all allow list elements + ele := input.subject.scope.allow_list[_] + ele.type in [input.object.type, "*"] + allowed_id := ele.id + } + + # Return if the object.id is in the allowed ids + # This rule is evaluated at the end so the partial query can use the object.id + # against this precomputed set of allowed ids. + input.object.id in allowed_ids } # ------------------- diff --git a/coderd/rbac/scopes.go b/coderd/rbac/scopes.go index 4dd930699a..08754e4219 100644 --- a/coderd/rbac/scopes.go +++ b/coderd/rbac/scopes.go @@ -45,14 +45,15 @@ func WorkspaceAgentScope(params WorkspaceAgentScopeParams) Scope { // incase we change the behavior of the allowlist. The allowlist is new // and evolving. Role: scope.Role, - // This prevents the agent from being able to access any other resource. - // Include the list of IDs of anything that is required for the - // agent to function. - AllowIDList: []string{ - params.WorkspaceID.String(), - params.TemplateID.String(), - params.VersionID.String(), - params.OwnerID.String(), + + // Limit the agent to only be able to access the singular workspace and + // the template/version it was created from. Add additional resources here + // as needed, but do not add more workspace or template resource ids. + AllowIDList: []AllowListElement{ + {Type: ResourceWorkspace.Type, ID: params.WorkspaceID.String()}, + {Type: ResourceTemplate.Type, ID: params.TemplateID.String()}, + {Type: ResourceTemplate.Type, ID: params.VersionID.String()}, + {Type: ResourceUser.Type, ID: params.OwnerID.String()}, }, } } @@ -77,7 +78,7 @@ var builtinScopes = map[ScopeName]Scope{ Org: map[string][]Permission{}, User: []Permission{}, }, - AllowIDList: []string{policy.WildcardSymbol}, + AllowIDList: []AllowListElement{AllowListAll()}, }, ScopeApplicationConnect: { @@ -90,7 +91,7 @@ var builtinScopes = map[ScopeName]Scope{ Org: map[string][]Permission{}, User: []Permission{}, }, - AllowIDList: []string{policy.WildcardSymbol}, + AllowIDList: []AllowListElement{AllowListAll()}, }, ScopeNoUserData: { @@ -101,7 +102,7 @@ var builtinScopes = map[ScopeName]Scope{ Org: map[string][]Permission{}, User: []Permission{}, }, - AllowIDList: []string{policy.WildcardSymbol}, + AllowIDList: []AllowListElement{AllowListAll()}, }, } @@ -129,7 +130,17 @@ func (name ScopeName) Name() RoleIdentifier { // AllowIDList. Eg: 'AllowIDList: []string{WildcardSymbol}' type Scope struct { Role - AllowIDList []string `json:"allow_list"` + AllowIDList []AllowListElement `json:"allow_list"` +} + +type AllowListElement struct { + // ID must be a string to allow for the wildcard symbol. + ID string `json:"id"` + Type string `json:"type"` +} + +func AllowListAll() AllowListElement { + return AllowListElement{ID: policy.WildcardSymbol, Type: policy.WildcardSymbol} } func (s Scope) Expand() (Scope, error) {