diff --git a/coderd/rbac/POLICY.md b/coderd/rbac/POLICY.md index b3ebdfe9d9..9d95832140 100644 --- a/coderd/rbac/POLICY.md +++ b/coderd/rbac/POLICY.md @@ -68,6 +68,30 @@ Each of these checks gets a "vote", which must one of three values: If a level abstains, then the decision gets deferred to the next level. When there is no "next" level to defer to it is equivalent to being denied. +### Known-org asymmetry (org and org_member levels) + +The org and org_member levels are evaluated differently depending on whether +the object's org id is known. + +When the org id is unknown (partial evaluation, e.g. filtering a list), the org +id must be kept out of comprehensions and must not be branched on (see "Unknown +values" below). To satisfy that, the known-org path tests the object's org id +for membership in a set of allowed org ids instead of looking up its vote: + +- The org level (`check_org_permissions`, known-org clause) only ever votes + `1` (allow) or abstains; it never votes `-1` for a known org. The + `not org = -1` / `not scope_org = -1` gates in the allow rules are therefore + no-ops for a known org and only block in the `any_org` case. +- Org-level deny is instead folded into the org_member level as a ground set + difference (`member_allow - org_deny`), so an org-level deny still blocks a + member-level allow. + +The `any_org` path ("can the subject do this in any org?") still uses the full +`-1`/`0`/`1` vote (the `max` over the vote map), because there is no specific +object org id to be unknown. So do not assume `org == -1` signals an org-level +deny for a known org; reconstruct it from `org_ids_with_vote(role_org_votes, -1)` +if you need it. + ### Scope Additionally, each input has a "scope" that can be thought of as a second set of permissions, where each permission belongs to one of the four levels–exactly the same as role permissions. An action is only allowed if it is allowed by both the subject's permissions _and_ their current scope. This is to allow issuing tokens for a subject that have a subset of the full subjects permissions. diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index 8316724508..c461590040 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -1009,6 +1009,87 @@ func TestAuthorizeLevels(t *testing.T) { {resource: ResourceWorkspace.WithOwner("not-me"), allow: false}, })) + + // Org-level deny must block a member-level allow, member-level deny must win + // within an org, and org-level allow must override a member-level deny, all on + // owned in-org objects. These exercise the known-org set-difference gate. + denyOrg := uuid.New() // member allows read; org denies read + memberDenyOrg := uuid.New() // member both allows and denies read (deny wins) + orgAllowOrg := uuid.New() // org allows read; member denies read + user = Subject{ + ID: "me", + Scope: must(ExpandScope(ScopeAll)), + Roles: Roles{ + { + Identifier: RoleIdentifier{Name: "member-allow", OrganizationID: defOrg}, + ByOrgID: map[string]OrgPermissions{ + defOrg.String(): { + Member: []Permission{{ResourceType: ResourceWorkspace.Type, Action: policy.ActionRead}}, + }, + }, + }, + { + Identifier: RoleIdentifier{Name: "member-allow-org-deny", OrganizationID: denyOrg}, + ByOrgID: map[string]OrgPermissions{ + denyOrg.String(): { + // Org denies only read; member allows read and update, so the + // deny is action-scoped (update stays allowed). + Org: []Permission{{Negate: true, ResourceType: ResourceWorkspace.Type, Action: policy.ActionRead}}, + Member: []Permission{ + {ResourceType: ResourceWorkspace.Type, Action: policy.ActionRead}, + {ResourceType: ResourceWorkspace.Type, Action: policy.ActionUpdate}, + }, + }, + }, + }, + { + Identifier: RoleIdentifier{Name: "member-deny-wins", OrganizationID: memberDenyOrg}, + ByOrgID: map[string]OrgPermissions{ + memberDenyOrg.String(): { + Member: []Permission{ + {ResourceType: ResourceWorkspace.Type, Action: policy.ActionRead}, + {Negate: true, ResourceType: ResourceWorkspace.Type, Action: policy.ActionRead}, + }, + }, + }, + }, + { + Identifier: RoleIdentifier{Name: "org-allow-member-deny", OrganizationID: orgAllowOrg}, + ByOrgID: map[string]OrgPermissions{ + orgAllowOrg.String(): { + Org: []Permission{{ResourceType: ResourceWorkspace.Type, Action: policy.ActionRead}}, + Member: []Permission{{Negate: true, ResourceType: ResourceWorkspace.Type, Action: policy.ActionRead}}, + }, + }, + }, + }, + } + + testAuthorize(t, "OrgDenyBlocksMember", user, + cases(func(c authTestCase) authTestCase { + c.actions = []policy.Action{policy.ActionRead} + return c + }, []authTestCase{ + // Member level allows the owned, in-org object. + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), allow: true}, + // Org-level deny blocks the owned object even though member allows it. + {resource: ResourceWorkspace.InOrg(denyOrg).WithOwner(user.ID), allow: false}, + // Member-level deny wins over a member-level allow in the same org. + {resource: ResourceWorkspace.InOrg(memberDenyOrg).WithOwner(user.ID), allow: false}, + // Org-level allow overrides a member-level deny, regardless of owner. + {resource: ResourceWorkspace.InOrg(orgAllowOrg).WithOwner(user.ID), allow: true}, + {resource: ResourceWorkspace.InOrg(orgAllowOrg).WithOwner("not-me"), allow: true}, + // The member grant does not extend to objects the subject does not own. + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me"), allow: false}, + // Not a member of this org at all. + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.ID), allow: false}, + }), + // The org-level deny is scoped to the action it names: update stays allowed + // in denyOrg because only read is denied. + []authTestCase{ + {resource: ResourceWorkspace.InOrg(denyOrg).WithOwner(user.ID), actions: []policy.Action{policy.ActionUpdate}, allow: true}, + }, + ) } func TestAuthorizeScope(t *testing.T) { @@ -1341,6 +1422,61 @@ func TestAuthorizeScope(t *testing.T) { {resource: ResourceUser.WithOwner(user.ID), allow: false, actions: []policy.Action{policy.ActionUpdate}}, }, ) + + // Scope-level org deny must block a member-level allow, mirroring + // OrgDenyBlocksMember but through the scope's org/member permissions (the + // scope_org_member member_allow - org_deny fold). The roles allow both + // objects, so the scope is the deciding factor. + scopeAllowOrg := uuid.New() + scopeDenyOrg := uuid.New() + user = Subject{ + ID: "me", + Roles: Roles{ + must(RoleByName(RoleMember())), + { + Identifier: RoleIdentifier{Name: "member-allow-a", OrganizationID: scopeAllowOrg}, + ByOrgID: map[string]OrgPermissions{ + scopeAllowOrg.String(): { + Member: []Permission{{ResourceType: ResourceWorkspace.Type, Action: policy.ActionRead}}, + }, + }, + }, + { + Identifier: RoleIdentifier{Name: "member-allow-b", OrganizationID: scopeDenyOrg}, + ByOrgID: map[string]OrgPermissions{ + scopeDenyOrg.String(): { + Member: []Permission{{ResourceType: ResourceWorkspace.Type, Action: policy.ActionRead}}, + }, + }, + }, + }, + Scope: Scope{ + Role: Role{ + Identifier: RoleIdentifier{Name: "scope-org-deny"}, + ByOrgID: map[string]OrgPermissions{ + scopeAllowOrg.String(): { + Member: []Permission{{ResourceType: ResourceWorkspace.Type, Action: policy.ActionRead}}, + }, + scopeDenyOrg.String(): { + Org: []Permission{{Negate: true, ResourceType: ResourceWorkspace.Type, Action: policy.ActionRead}}, + Member: []Permission{{ResourceType: ResourceWorkspace.Type, Action: policy.ActionRead}}, + }, + }, + }, + AllowIDList: []AllowListElement{AllowListAll()}, + }, + } + + testAuthorize(t, "ScopeOrgDenyBlocksMember", user, + cases(func(c authTestCase) authTestCase { + c.actions = []policy.Action{policy.ActionRead} + return c + }, []authTestCase{ + // Scope member-allow permits the owned in-org object. + {resource: ResourceWorkspace.InOrg(scopeAllowOrg).WithOwner(user.ID), allow: true}, + // Scope org-level deny blocks it even though scope member allows. + {resource: ResourceWorkspace.InOrg(scopeDenyOrg).WithOwner(user.ID), allow: false}, + })) } func TestScopeAllowList(t *testing.T) { diff --git a/coderd/rbac/policy.rego b/coderd/rbac/policy.rego index 0a15955bad..ccd2ea4445 100644 --- a/coderd/rbac/policy.rego +++ b/coderd/rbac/policy.rego @@ -99,28 +99,23 @@ org_memberships := {org_id | # permissions. Adding a second set of org memberships might affect the partial # evaluation. This is being left until org scopes are used. -default org := 0 - -org := check_org_permissions(input.subject.roles, "org") - -default scope_org := 0 - -scope_org := check_org_permissions([input.subject.scope], "org") - # check_all_org_permissions creates a map from org ids to votes at each org # level, for each org that the subject is a member of. It doesn't actually check -# if the object is in the same org. Instead we look up the correct vote from -# this map based on the object's org id in `check_org_permissions`. -# For example, the `org_map` will look something like this: +# if the object is in the same org; the callers do that: +# - `org_ids_with_vote` picks the org ids with a given vote, and the known-org +# rules test the object's org id for membership in that set, and +# - the `any_org` clauses take the `max` vote. +# For example, the map will look something like this: # # {"": 1, "": 0, "": -1} # -# The caller then uses `output[input.object.org_owner]` to get the correct vote. -# -# We have to create this map, rather than just getting the vote of the object's -# org id because the org id _might_ be unknown. In order to make sure that this -# policy compresses down to simple queries we need to keep unknown values out of +# We build the whole map, rather than just the vote for the object's org, +# because the org id _might_ be unknown during partial evaluation. To keep this +# policy compressible to simple queries we need to keep unknown values out of # comprehensions. +# +# This is a helper function shared by the memoized vote-map rules below, so its +# per-call cost is paid at most once per (roles, key) combination. check_all_org_permissions(roles, key) := {org_id: vote | org_id := org_memberships[_] allow := {is_allowed | @@ -137,34 +132,62 @@ check_all_org_permissions(roles, key) := {org_id: vote | vote := to_vote(allow) } -# This check handles the case where the org id is known. -check_org_permissions(roles, key) := vote if { - # Disallow setting any_org at the same time as an org id. - not input.object.any_org +# The vote maps below are complete rules with no arguments, so OPA evaluates +# each once per query and caches the result. A function is instead re-evaluated +# at every call site, so reading org votes through these rules keeps the policy +# from rebuilding the same vote map for the org, member, and scope paths on +# every authorization check. +role_org_votes := check_all_org_permissions(input.subject.roles, "org") - allow_map := check_all_org_permissions(roles, key) +role_member_votes := check_all_org_permissions(input.subject.roles, "member") - # Return only the vote of the object's org. - vote := allow_map[input.object.org_owner] +scope_org_votes := check_all_org_permissions([input.subject.scope], "org") + +scope_member_votes := check_all_org_permissions([input.subject.scope], "member") + +# org_ids_with_vote returns the set of org ids in a vote map whose vote equals +# `wanted`. It depends only on the (fully known) vote map, never on the object's +# org id, so its result is ground during partial evaluation. The known-org +# rules test the object's org id for membership in this set, which lets the +# query compile to `organization_id = ANY(ARRAY[...])` instead of fanning out to +# one query per org. +org_ids_with_vote(votes, wanted) := {org_id | + some org_id, vote in votes + vote == wanted } -# This check handles the case where we want to know if the user has the -# appropriate permission for any organization, without needing to know which. -# This is used in several places in the UI to determine if certain parts of the -# app should be accessible. -# For example, can the user create a new template in any organization? If yes, -# then we should show the "New template" button. -check_org_permissions(roles, key) := vote if { - # Require `any_org` to be set +default org := 0 + +# Known org: only ever votes to allow. See POLICY.md "Known-org asymmetry". The +# count guard keeps an empty allow set from emitting an unsatisfiable +# `org_owner in set()` residual during partial evaluation (OPA drops the whole +# branch instead). +org := 1 if { + not input.object.any_org + allow := org_ids_with_vote(role_org_votes, 1) + count(allow) > 0 + input.object.org_owner in allow +} + +# any_org: the highest org-level vote across every org. Unlike the known-org +# clause this can vote -1, which the allow rules honor via `not org = -1`. +org := vote if { input.object.any_org + vote := max({v | some v in role_org_votes}) +} - allow_map := check_all_org_permissions(roles, key) +default scope_org := 0 - # Since we're checking if the subject has the permission in _any_ org, we're - # essentially trying to find the highest vote from any org. - vote := max({vote | - some vote in allow_map - }) +scope_org := 1 if { + not input.object.any_org + allow := org_ids_with_vote(scope_org_votes, 1) + count(allow) > 0 + input.object.org_owner in allow +} + +scope_org := vote if { + input.object.any_org + vote := max({v | some v in scope_org_votes}) } # is_org_member checks if the subject belong to the same organization as the @@ -190,25 +213,62 @@ is_org_member if { # the corresponding org. Permissions for objects which are not owned by an # organization instead defer to the user level rules. # -# The rules for this level are very similar to the rules for the organization -# level, and so we reuse the `check_org_permissions` function from those rules. +# The rules for this level mirror the organization level rules and read from the +# same memoized vote maps (`role_member_votes`, `scope_member_votes`, +# `role_org_votes`, `scope_org_votes`). default org_member := 0 +# Known org: allow when the subject owns the object and a member-level +# permission allows it. The allowed set folds in the org-level deny as a ground +# set difference (see POLICY.md "Known-org asymmetry"), and its value is fully +# known at partial-evaluation time, so the unknown org id appears in only one +# positive membership test and the decision never branches on it. The count +# guard keeps an empty set from emitting an unsatisfiable residual. +org_member := 1 if { + # Object must be jointly owned by the user + input.object.owner != "" + input.subject.id = input.object.owner + not input.object.any_org + + # Org-level deny is folded in as a ground set difference so a known org never + # needs an org-level -1 vote (see POLICY.md "Known-org asymmetry"). + allowed := org_ids_with_vote(role_member_votes, 1) - org_ids_with_vote(role_org_votes, -1) + count(allowed) > 0 + input.object.org_owner in allowed +} + +# any_org: the highest member-level vote across every org. Org-level deny is +# applied by the `not org = -1` gate in the allow rules rather than folded in +# here, because `org` votes -1 in the any_org case. org_member := vote if { # Object must be jointly owned by the user input.object.owner != "" input.subject.id = input.object.owner - vote := check_org_permissions(input.subject.roles, "member") + input.object.any_org + vote := max({v | some v in role_member_votes}) } default scope_org_member := 0 +# Known org: like org_member, scoped to the subject's current scope. +scope_org_member := 1 if { + # Object must be jointly owned by the user + input.object.owner != "" + input.subject.id = input.object.owner + not input.object.any_org + + allowed := org_ids_with_vote(scope_member_votes, 1) - org_ids_with_vote(scope_org_votes, -1) + count(allowed) > 0 + input.object.org_owner in allowed +} + scope_org_member := vote if { # Object must be jointly owned by the user input.object.owner != "" input.subject.id = input.object.owner - vote := check_org_permissions([input.subject.scope], "member") + input.object.any_org + vote := max({v | some v in scope_member_votes}) } #==============================================================================# @@ -243,6 +303,10 @@ role_allow if { # Org member authorization role_allow if { not site = -1 + + # For a known org this is always true: `org` never votes -1 for a known org, + # because org-level deny is folded into `org_member`. It only blocks here in + # the any_org case, where `org` can be -1 via `max`. not org = -1 org_member = 1 @@ -290,6 +354,9 @@ scope_allow if { # by the site or org. The object *must* be owned by an organization. object_is_included_in_scope_allow_list not scope_site = -1 + + # As with `not org = -1` above, this only blocks in the any_org case; for a + # known org, scope org-level deny is folded into `scope_org_member`. not scope_org = -1 scope_org_member = 1