From d527f91f477606beebd61eacb393bebfaf1325d8 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 9 Sep 2025 12:50:08 +0200 Subject: [PATCH] chore: update rego policy to respect user and organisation scopes (#19741) Prior to this change, user and org scopes were always rejected --- coderd/rbac/authz_internal_test.go | 51 ++++++++++++++++++++++++++++++ coderd/rbac/policy.rego | 11 +++++-- 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index 838c7bce1c..9e7ec07b6e 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -1110,6 +1110,57 @@ func TestAuthorizeScope(t *testing.T) { {resource: ResourceOrganization.WithID(defOrg)}, }), ) + + // Test setting a scope on the org and the user level + // This is a bit of a contrived example that would not exist in practice. + // It combines a specific organization scope with a user scope to verify + // that both are applied. + // The test uses the `Owner` role, so by default the user can do everything. + user = Subject{ + ID: "me", + Roles: Roles{ + must(RoleByName(RoleOwner())), + // TODO: There is a __bug__ in the policy.rego. If the user is not a + // member of the organization, the org_scope fails. This happens because + // the org_allow_set uses "org_members". + // This is odd behavior, as without this membership role, the test for + // the workspace fails. Maybe scopes should just assume the user + // is a member. + must(RoleByName(ScopedRoleOrgMember(defOrg))), + }, + Scope: Scope{ + Role: Role{ + Identifier: RoleIdentifier{ + Name: "org-and-user-scope", + OrganizationID: defOrg, + }, + DisplayName: "OrgAndUserScope", + Site: nil, + Org: map[string][]Permission{ + defOrg.String(): Permissions(map[string][]policy.Action{ + ResourceWorkspace.Type: {policy.ActionRead}, + }), + }, + User: Permissions(map[string][]policy.Action{ + ResourceUser.Type: {policy.ActionRead}, + }), + }, + AllowIDList: []string{policy.WildcardSymbol}, + }, + } + + testAuthorize(t, "OrgAndUserScope", user, + // Allowed by scope: + []authTestCase{ + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), allow: true, actions: []policy.Action{policy.ActionRead}}, + {resource: ResourceUser.WithOwner(user.ID), allow: true, actions: []policy.Action{policy.ActionRead}}, + }, + // Not allowed by scope: + []authTestCase{ + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), allow: false, actions: []policy.Action{policy.ActionCreate}}, + {resource: ResourceUser.WithOwner(user.ID), allow: false, actions: []policy.Action{policy.ActionUpdate}}, + }, + ) } // cases applies a given function to all test cases. This makes generalities easier to create. diff --git a/coderd/rbac/policy.rego b/coderd/rbac/policy.rego index 2ee47c35c8..c0b747fef0 100644 --- a/coderd/rbac/policy.rego +++ b/coderd/rbac/policy.rego @@ -106,6 +106,13 @@ site_allow(roles) := num if { # ------------------- # org_members is the list of organizations the actor is apart of. +# TODO: Should there be an org_members for the scope too? Without it, +# the membership is determined by the user's roles, not their scope permissions. +# So if an owner (who is not an org member) has an org scope, that org scope +# will fail to return '1'. Since we assume all non members return '-1' for org +# level permissions. +# Adding a second org_members set might affect the partial evaluation. +# This is being left until org scopes are used. org_members := {orgID | input.subject.roles[_].org[orgID] } @@ -116,7 +123,7 @@ default org := 0 org := org_allow(input.subject.roles) default scope_org := 0 -scope_org := org_allow([input.scope]) +scope_org := org_allow([input.subject.scope]) # org_allow_set is a helper function that iterates over all orgs that the actor # is a member of. For each organization it sets the numerical allow value @@ -221,7 +228,7 @@ default user := 0 user := user_allow(input.subject.roles) default scope_user := 0 -scope_user := user_allow([input.scope]) +scope_user := user_allow([input.subject.scope]) user_allow(roles) := num if { input.object.owner != ""