diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go index 7d9622b316..f28fa00407 100644 --- a/coderd/database/db2sdk/db2sdk.go +++ b/coderd/database/db2sdk/db2sdk.go @@ -693,13 +693,13 @@ func SlimRoleFromName(name string) codersdk.SlimRole { func RBACRole(role rbac.Role) codersdk.Role { slim := SlimRole(role) - orgPerms := role.Org[slim.OrganizationID] + orgPerms := role.ByOrgID[slim.OrganizationID] return codersdk.Role{ Name: slim.Name, OrganizationID: slim.OrganizationID, DisplayName: slim.DisplayName, SitePermissions: List(role.Site, RBACPermission), - OrganizationPermissions: List(orgPerms, RBACPermission), + OrganizationPermissions: List(orgPerms.Org, RBACPermission), UserPermissions: List(role.User, RBACPermission), } } diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 6df0168612..4f17a89703 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -232,8 +232,8 @@ var ( // Provisionerd creates usage events rbac.ResourceUsageEvent.Type: {policy.ActionCreate}, }), - Org: map[string][]rbac.Permission{}, - User: []rbac.Permission{}, + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, }, }), Scope: rbac.ScopeAll, @@ -257,8 +257,8 @@ var ( rbac.ResourceWorkspace.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop}, rbac.ResourceWorkspaceDormant.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStop}, }), - Org: map[string][]rbac.Permission{}, - User: []rbac.Permission{}, + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, }, }), Scope: rbac.ScopeAll, @@ -280,8 +280,8 @@ var ( rbac.ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionUpdate}, rbac.ResourceProvisionerJobs.Type: {policy.ActionRead, policy.ActionUpdate}, }), - Org: map[string][]rbac.Permission{}, - User: []rbac.Permission{}, + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, }, }), Scope: rbac.ScopeAll, @@ -299,8 +299,8 @@ var ( Site: rbac.Permissions(map[string][]policy.Action{ rbac.ResourceCryptoKey.Type: {policy.WildcardSymbol}, }), - Org: map[string][]rbac.Permission{}, - User: []rbac.Permission{}, + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, }, }), Scope: rbac.ScopeAll, @@ -318,8 +318,8 @@ var ( Site: rbac.Permissions(map[string][]policy.Action{ rbac.ResourceCryptoKey.Type: {policy.WildcardSymbol}, }), - Org: map[string][]rbac.Permission{}, - User: []rbac.Permission{}, + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, }, }), Scope: rbac.ScopeAll, @@ -336,8 +336,8 @@ var ( Site: rbac.Permissions(map[string][]policy.Action{ rbac.ResourceConnectionLog.Type: {policy.ActionUpdate, policy.ActionRead}, }), - Org: map[string][]rbac.Permission{}, - User: []rbac.Permission{}, + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, }, }), Scope: rbac.ScopeAll, @@ -357,8 +357,8 @@ var ( rbac.ResourceWebpushSubscription.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, rbac.ResourceDeploymentConfig.Type: {policy.ActionRead, policy.ActionUpdate}, // To read and upsert VAPID keys }), - Org: map[string][]rbac.Permission{}, - User: []rbac.Permission{}, + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, }, }), Scope: rbac.ScopeAll, @@ -376,8 +376,8 @@ var ( // The workspace monitor needs to be able to update monitors rbac.ResourceWorkspaceAgentResourceMonitor.Type: {policy.ActionUpdate}, }), - Org: map[string][]rbac.Permission{}, - User: []rbac.Permission{}, + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, }, }), Scope: rbac.ScopeAll, @@ -393,12 +393,12 @@ var ( Identifier: rbac.RoleIdentifier{Name: "subagentapi"}, DisplayName: "Sub Agent API", Site: []rbac.Permission{}, - Org: map[string][]rbac.Permission{ - orgID.String(): {}, - }, User: rbac.Permissions(map[string][]policy.Action{ rbac.ResourceWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionCreateAgent, policy.ActionDeleteAgent}, }), + ByOrgID: map[string]rbac.OrgPermissions{ + orgID.String(): {}, + }, }, }), Scope: rbac.ScopeAll, @@ -437,8 +437,8 @@ var ( rbac.ResourceOauth2App.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, rbac.ResourceOauth2AppSecret.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, }), - Org: map[string][]rbac.Permission{}, - User: []rbac.Permission{}, + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, }, }), Scope: rbac.ScopeAll, @@ -455,8 +455,8 @@ var ( Site: rbac.Permissions(map[string][]policy.Action{ rbac.ResourceProvisionerDaemon.Type: {policy.ActionRead}, }), - Org: map[string][]rbac.Permission{}, - User: []rbac.Permission{}, + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, }, }), Scope: rbac.ScopeAll, @@ -532,8 +532,8 @@ var ( Site: rbac.Permissions(map[string][]policy.Action{ rbac.ResourceFile.Type: {policy.ActionRead}, }), - Org: map[string][]rbac.Permission{}, - User: []rbac.Permission{}, + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, }, }), Scope: rbac.ScopeAll, @@ -553,8 +553,8 @@ var ( // reads/processes them. rbac.ResourceUsageEvent.Type: {policy.ActionRead, policy.ActionUpdate}, }), - Org: map[string][]rbac.Permission{}, - User: []rbac.Permission{}, + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, }, }), Scope: rbac.ScopeAll, @@ -577,8 +577,8 @@ var ( rbac.ResourceApiKey.Type: {policy.ActionRead}, // Validate API keys. rbac.ResourceAibridgeInterception.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate}, }), - Org: map[string][]rbac.Permission{}, - User: []rbac.Permission{}, + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, }, }), Scope: rbac.ScopeAll, @@ -1254,13 +1254,13 @@ func (q *querier) customRoleCheck(ctx context.Context, role database.CustomRole) return xerrors.Errorf("invalid role: %w", err) } - if len(rbacRole.Org) > 0 && len(rbacRole.Site) > 0 { + if len(rbacRole.ByOrgID) > 0 && len(rbacRole.Site) > 0 { // This is a choice to keep roles simple. If we allow mixing site and org scoped perms, then knowing who can // do what gets more complicated. return xerrors.Errorf("invalid custom role, cannot assign both org and site permissions at the same time") } - if len(rbacRole.Org) > 1 { + if len(rbacRole.ByOrgID) > 1 { // Again to avoid more complexity in our roles return xerrors.Errorf("invalid custom role, cannot assign permissions to more than 1 org at a time") } @@ -1273,8 +1273,8 @@ func (q *querier) customRoleCheck(ctx context.Context, role database.CustomRole) } } - for orgID, perms := range rbacRole.Org { - for _, orgPerm := range perms { + for orgID, perms := range rbacRole.ByOrgID { + for _, orgPerm := range perms.Org { err := q.customRoleEscalationCheck(ctx, act, orgPerm, rbac.Object{OrgID: orgID, Type: orgPerm.ResourceType}) if err != nil { return xerrors.Errorf("org=%q: %w", orgID, err) diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index 2ac62215b4..b6d10569a7 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -176,8 +176,8 @@ func (s APIKeyScopes) expandRBACScope() (rbac.Scope, error) { // Identifier is informational; not used in policy evaluation. Identifier: rbac.RoleIdentifier{Name: "Scope_Multiple"}, Site: nil, - Org: map[string][]rbac.Permission{}, User: nil, + ByOrgID: map[string]rbac.OrgPermissions{}, } // Collect allow lists for a union after expanding all scopes. @@ -191,8 +191,10 @@ func (s APIKeyScopes) expandRBACScope() (rbac.Scope, error) { // Merge role permissions: union by simple concatenation. merged.Site = append(merged.Site, expanded.Site...) - for orgID, perms := range expanded.Org { - merged.Org[orgID] = append(merged.Org[orgID], perms...) + for orgID, perms := range expanded.ByOrgID { + orgPerms := merged.ByOrgID[orgID] + orgPerms.Org = append(orgPerms.Org, perms.Org...) + merged.ByOrgID[orgID] = orgPerms } merged.User = append(merged.User, expanded.User...) @@ -201,10 +203,11 @@ func (s APIKeyScopes) expandRBACScope() (rbac.Scope, error) { // De-duplicate permissions across Site/Org/User merged.Site = rbac.DeduplicatePermissions(merged.Site) - for orgID, perms := range merged.Org { - merged.Org[orgID] = rbac.DeduplicatePermissions(perms) - } merged.User = rbac.DeduplicatePermissions(merged.User) + for orgID, perms := range merged.ByOrgID { + perms.Org = rbac.DeduplicatePermissions(perms.Org) + merged.ByOrgID[orgID] = perms + } union, err := rbac.UnionAllowLists(allowLists...) if err != nil { diff --git a/coderd/rbac/astvalue.go b/coderd/rbac/astvalue.go index a125b6bf7a..d207ae888a 100644 --- a/coderd/rbac/astvalue.go +++ b/coderd/rbac/astvalue.go @@ -157,23 +157,30 @@ func (role Role) regoValue() ast.Value { if role.cachedRegoValue != nil { return role.cachedRegoValue } - orgMap := ast.NewObject() - for k, p := range role.Org { - orgMap.Insert(ast.StringTerm(k), ast.NewTerm(regoSlice(p))) + byOrgIDMap := ast.NewObject() + for k, p := range role.ByOrgID { + byOrgIDMap.Insert(ast.StringTerm(k), ast.NewTerm( + ast.NewObject( + [2]*ast.Term{ + ast.StringTerm("org"), + ast.NewTerm(regoSlice(p.Org)), + }, + ), + )) } return ast.NewObject( [2]*ast.Term{ ast.StringTerm("site"), ast.NewTerm(regoSlice(role.Site)), }, - [2]*ast.Term{ - ast.StringTerm("org"), - ast.NewTerm(orgMap), - }, [2]*ast.Term{ ast.StringTerm("user"), ast.NewTerm(regoSlice(role.User)), }, + [2]*ast.Term{ + ast.StringTerm("by_org_id"), + ast.NewTerm(byOrgIDMap), + }, ) } diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index 284045b11f..1d52304ba0 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -633,13 +633,6 @@ func TestAuthorizeDomain(t *testing.T) { { Identifier: RoleIdentifier{Name: "ReadOnlyOrgAndUser"}, Site: []Permission{}, - Org: map[string][]Permission{ - defOrg.String(): {{ - Negate: false, - ResourceType: "*", - Action: policy.ActionRead, - }}, - }, User: []Permission{ { Negate: false, @@ -647,6 +640,15 @@ func TestAuthorizeDomain(t *testing.T) { Action: policy.ActionRead, }, }, + ByOrgID: map[string]OrgPermissions{ + defOrg.String(): { + Org: []Permission{{ + Negate: false, + ResourceType: "*", + Action: policy.ActionRead, + }}, + }, + }, }, }, } @@ -726,12 +728,14 @@ func TestAuthorizeLevels(t *testing.T) { must(RoleByName(RoleOwner())), { Identifier: RoleIdentifier{Name: "org-deny:", OrganizationID: defOrg}, - Org: map[string][]Permission{ + ByOrgID: map[string]OrgPermissions{ defOrg.String(): { - { - Negate: true, - ResourceType: "*", - Action: "*", + Org: []Permission{ + { + Negate: true, + ResourceType: "*", + Action: "*", + }, }, }, }, @@ -926,8 +930,8 @@ func TestAuthorizeScope(t *testing.T) { // Only read access for workspaces. ResourceWorkspace.Type: {policy.ActionRead}, }), - Org: map[string][]Permission{}, - User: []Permission{}, + User: []Permission{}, + ByOrgID: map[string]OrgPermissions{}, }, AllowIDList: []AllowListElement{{Type: ResourceWorkspace.Type, ID: workspaceID.String()}}, }, @@ -1015,8 +1019,8 @@ func TestAuthorizeScope(t *testing.T) { // Only read access for workspaces. ResourceWorkspace.Type: {policy.ActionCreate}, }), - Org: map[string][]Permission{}, - User: []Permission{}, + User: []Permission{}, + ByOrgID: map[string]OrgPermissions{}, }, // Empty string allow_list is allowed for actions like 'create' AllowIDList: []AllowListElement{{ @@ -1138,14 +1142,16 @@ func TestAuthorizeScope(t *testing.T) { }, 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}, }), + ByOrgID: map[string]OrgPermissions{ + defOrg.String(): { + Org: Permissions(map[string][]policy.Action{ + ResourceWorkspace.Type: {policy.ActionRead}, + }), + }, + }, }, AllowIDList: []AllowListElement{AllowListAll()}, }, diff --git a/coderd/rbac/policy.rego b/coderd/rbac/policy.rego index eb9187338c..0d11ea4cea 100644 --- a/coderd/rbac/policy.rego +++ b/coderd/rbac/policy.rego @@ -114,16 +114,16 @@ site_allow(roles) := num if { # 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] + input.subject.roles[_].by_org_id[orgID] } # 'org' is the same as 'site' except we need to iterate over each organization # that the actor is a member of. default org := 0 -org := org_allow(input.subject.roles) +org := org_allow(input.subject.roles, "org") default scope_org := 0 -scope_org := org_allow([input.subject.scope]) +scope_org := org_allow([input.subject.scope], "org") # 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 @@ -135,12 +135,12 @@ scope_org := org_allow([input.subject.scope]) # The reason we calculate this for all orgs, and not just the input.object.org_owner # is that sometimes the input.object.org_owner is unknown. In those cases # we have a list of org_ids that can we use in a SQL 'WHERE' clause. -org_allow_set(roles) := allow_set if { +org_allow_set(roles, key) := allow_set if { allow_set := {id: num | id := org_members[_] set := {is_allowed | # Iterate over all org permissions in all roles - perm := roles[_].org[id][_] + perm := roles[_].by_org_id[id][key][_] perm.action in [input.action, "*"] perm.resource_type in [input.object.type, "*"] @@ -151,11 +151,11 @@ org_allow_set(roles) := allow_set if { } } -org_allow(roles) := num if { +org_allow(roles, key) := num if { # If the object has "any_org" set to true, then use the other # org_allow block. not input.object.any_org - allow := org_allow_set(roles) + allow := org_allow_set(roles, key) # Return only the org value of the input's org. # The reason why we do not do this up front, is that we need to make sure @@ -171,9 +171,9 @@ org_allow(roles) := num if { # This is useful for UI elements when we want to conclude, "Can the user create # a new template in any organization?" # It is easier than iterating over every organization the user is apart of. -org_allow(roles) := num if { +org_allow(roles, key) := num if { input.object.any_org # if this is false, this code block is not used - allow := org_allow_set(roles) + allow := org_allow_set(roles, key) # allow is a map of {"": }. We only care about values # that are 1, and ignore the rest. diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index e5f822b91e..27162b7230 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -282,8 +282,8 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // Note: even without PrebuiltWorkspace permissions, access is still granted via Workspace permissions. ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete}, })...), - Org: map[string][]Permission{}, - User: []Permission{}, + User: []Permission{}, + ByOrgID: map[string]OrgPermissions{}, }.withCachedRegoValue() memberRole := Role{ @@ -295,7 +295,6 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceOauth2App.Type: {policy.ActionRead}, ResourceWorkspaceProxy.Type: {policy.ActionRead}, }), - Org: map[string][]Permission{}, User: append(allPermsExcept(ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceUser, ResourceOrganizationMember), Permissions(map[string][]policy.Action{ // Reduced permission set on dormant workspaces. No build, ssh, or exec @@ -309,6 +308,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceProvisionerDaemon.Type: {policy.ActionRead, policy.ActionCreate, policy.ActionRead, policy.ActionUpdate}, })..., ), + ByOrgID: map[string]OrgPermissions{}, }.withCachedRegoValue() auditorRole := Role{ @@ -331,8 +331,8 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // Allow auditors to query aibridge interceptions. ResourceAibridgeInterception.Type: {policy.ActionRead}, }), - Org: map[string][]Permission{}, - User: []Permission{}, + User: []Permission{}, + ByOrgID: map[string]OrgPermissions{}, }.withCachedRegoValue() templateAdminRole := Role{ @@ -354,8 +354,8 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceOrganization.Type: {policy.ActionRead}, ResourceOrganizationMember.Type: {policy.ActionRead}, }), - Org: map[string][]Permission{}, - User: []Permission{}, + User: []Permission{}, + ByOrgID: map[string]OrgPermissions{}, }.withCachedRegoValue() userAdminRole := Role{ @@ -378,8 +378,8 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // Manage org membership based on OIDC claims ResourceIdpsyncSettings.Type: {policy.ActionRead, policy.ActionUpdate}, }), - Org: map[string][]Permission{}, - User: []Permission{}, + User: []Permission{}, + ByOrgID: map[string]OrgPermissions{}, }.withCachedRegoValue() builtInRoles = map[string]func(orgID uuid.UUID) Role{ @@ -419,18 +419,20 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // users at the site wide to know they exist. ResourceUser.Type: {policy.ActionRead}, }), - Org: map[string][]Permission{ - // Org admins should not have workspace exec perms. - organizationID.String(): append(allPermsExcept(ResourceWorkspace, ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceAssignRole, ResourceUserSecret), Permissions(map[string][]policy.Action{ - ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate, policy.ActionWorkspaceStop, policy.ActionCreateAgent, policy.ActionDeleteAgent}, - ResourceWorkspace.Type: slice.Omit(ResourceWorkspace.AvailableActions(), policy.ActionApplicationConnect, policy.ActionSSH), - // PrebuiltWorkspaces are a subset of Workspaces. - // Explicitly setting PrebuiltWorkspace permissions for clarity. - // Note: even without PrebuiltWorkspace permissions, access is still granted via Workspace permissions. - ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete}, - })...), - }, User: []Permission{}, + ByOrgID: map[string]OrgPermissions{ + // Org admins should not have workspace exec perms. + organizationID.String(): { + Org: append(allPermsExcept(ResourceWorkspace, ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceAssignRole, ResourceUserSecret), Permissions(map[string][]policy.Action{ + ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate, policy.ActionWorkspaceStop, policy.ActionCreateAgent, policy.ActionDeleteAgent}, + ResourceWorkspace.Type: slice.Omit(ResourceWorkspace.AvailableActions(), policy.ActionApplicationConnect, policy.ActionSSH), + // PrebuiltWorkspaces are a subset of Workspaces. + // Explicitly setting PrebuiltWorkspace permissions for clarity. + // Note: even without PrebuiltWorkspace permissions, access is still granted via Workspace permissions. + ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete}, + })...), + }, + }, } }, @@ -440,18 +442,20 @@ func ReloadBuiltinRoles(opts *RoleOptions) { Identifier: RoleIdentifier{Name: orgMember, OrganizationID: organizationID}, DisplayName: "", Site: []Permission{}, - Org: map[string][]Permission{ - organizationID.String(): Permissions(map[string][]policy.Action{ - // All users can see the provisioner daemons for workspace - // creation. - ResourceProvisionerDaemon.Type: {policy.ActionRead}, - // All org members can read the organization - ResourceOrganization.Type: {policy.ActionRead}, - // Can read available roles. - ResourceAssignOrgRole.Type: {policy.ActionRead}, - }), + User: []Permission{}, + ByOrgID: map[string]OrgPermissions{ + organizationID.String(): { + Org: Permissions(map[string][]policy.Action{ + // All users can see the provisioner daemons for workspace + // creation. + ResourceProvisionerDaemon.Type: {policy.ActionRead}, + // All org members can read the organization + ResourceOrganization.Type: {policy.ActionRead}, + // Can read available roles. + ResourceAssignOrgRole.Type: {policy.ActionRead}, + }), + }, }, - User: []Permission{}, } }, orgAuditor: func(organizationID uuid.UUID) Role { @@ -459,19 +463,21 @@ func ReloadBuiltinRoles(opts *RoleOptions) { Identifier: RoleIdentifier{Name: orgAuditor, OrganizationID: organizationID}, DisplayName: "Organization Auditor", Site: []Permission{}, - Org: map[string][]Permission{ - organizationID.String(): Permissions(map[string][]policy.Action{ - ResourceAuditLog.Type: {policy.ActionRead}, - ResourceConnectionLog.Type: {policy.ActionRead}, - // Allow auditors to see the resources that audit logs reflect. - ResourceTemplate.Type: {policy.ActionRead, policy.ActionViewInsights}, - ResourceGroup.Type: {policy.ActionRead}, - ResourceGroupMember.Type: {policy.ActionRead}, - ResourceOrganization.Type: {policy.ActionRead}, - ResourceOrganizationMember.Type: {policy.ActionRead}, - }), + User: []Permission{}, + ByOrgID: map[string]OrgPermissions{ + organizationID.String(): { + Org: Permissions(map[string][]policy.Action{ + ResourceAuditLog.Type: {policy.ActionRead}, + ResourceConnectionLog.Type: {policy.ActionRead}, + // Allow auditors to see the resources that audit logs reflect. + ResourceTemplate.Type: {policy.ActionRead, policy.ActionViewInsights}, + ResourceGroup.Type: {policy.ActionRead}, + ResourceGroupMember.Type: {policy.ActionRead}, + ResourceOrganization.Type: {policy.ActionRead}, + ResourceOrganizationMember.Type: {policy.ActionRead}, + }), + }, }, - User: []Permission{}, } }, orgUserAdmin: func(organizationID uuid.UUID) Role { @@ -484,18 +490,20 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // users at the site wide to know they exist. ResourceUser.Type: {policy.ActionRead}, }), - Org: map[string][]Permission{ - organizationID.String(): Permissions(map[string][]policy.Action{ - // Assign, remove, and read roles in the organization. - ResourceAssignOrgRole.Type: {policy.ActionAssign, policy.ActionUnassign, policy.ActionRead}, - ResourceOrganization.Type: {policy.ActionRead}, - ResourceOrganizationMember.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, - ResourceGroup.Type: ResourceGroup.AvailableActions(), - ResourceGroupMember.Type: ResourceGroupMember.AvailableActions(), - ResourceIdpsyncSettings.Type: {policy.ActionRead, policy.ActionUpdate}, - }), - }, User: []Permission{}, + ByOrgID: map[string]OrgPermissions{ + organizationID.String(): { + Org: Permissions(map[string][]policy.Action{ + // Assign, remove, and read roles in the organization. + ResourceAssignOrgRole.Type: {policy.ActionAssign, policy.ActionUnassign, policy.ActionRead}, + ResourceOrganization.Type: {policy.ActionRead}, + ResourceOrganizationMember.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, + ResourceGroup.Type: ResourceGroup.AvailableActions(), + ResourceGroupMember.Type: ResourceGroupMember.AvailableActions(), + ResourceIdpsyncSettings.Type: {policy.ActionRead, policy.ActionUpdate}, + }), + }, + }, } }, orgTemplateAdmin: func(organizationID uuid.UUID) Role { @@ -504,25 +512,27 @@ func ReloadBuiltinRoles(opts *RoleOptions) { Identifier: RoleIdentifier{Name: orgTemplateAdmin, OrganizationID: organizationID}, DisplayName: "Organization Template Admin", Site: []Permission{}, - Org: map[string][]Permission{ - organizationID.String(): Permissions(map[string][]policy.Action{ - ResourceTemplate.Type: ResourceTemplate.AvailableActions(), - ResourceFile.Type: {policy.ActionCreate, policy.ActionRead}, - ResourceWorkspace.Type: {policy.ActionRead}, - ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete}, - // Assigning template perms requires this permission. - ResourceOrganization.Type: {policy.ActionRead}, - ResourceOrganizationMember.Type: {policy.ActionRead}, - ResourceGroup.Type: {policy.ActionRead}, - ResourceGroupMember.Type: {policy.ActionRead}, - // Since templates have to correlate with provisioners, - // the ability to create templates and provisioners has - // a lot of overlap. - ResourceProvisionerDaemon.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, - ResourceProvisionerJobs.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionCreate}, - }), + User: []Permission{}, + ByOrgID: map[string]OrgPermissions{ + organizationID.String(): { + Org: Permissions(map[string][]policy.Action{ + ResourceTemplate.Type: ResourceTemplate.AvailableActions(), + ResourceFile.Type: {policy.ActionCreate, policy.ActionRead}, + ResourceWorkspace.Type: {policy.ActionRead}, + ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete}, + // Assigning template perms requires this permission. + ResourceOrganization.Type: {policy.ActionRead}, + ResourceOrganizationMember.Type: {policy.ActionRead}, + ResourceGroup.Type: {policy.ActionRead}, + ResourceGroupMember.Type: {policy.ActionRead}, + // Since templates have to correlate with provisioners, + // the ability to create templates and provisioners has + // a lot of overlap. + ResourceProvisionerDaemon.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, + ResourceProvisionerJobs.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionCreate}, + }), + }, }, - User: []Permission{}, } }, // orgWorkspaceCreationBan prevents creating & deleting workspaces. This @@ -533,31 +543,33 @@ func ReloadBuiltinRoles(opts *RoleOptions) { Identifier: RoleIdentifier{Name: orgWorkspaceCreationBan, OrganizationID: organizationID}, DisplayName: "Organization Workspace Creation Ban", Site: []Permission{}, - Org: map[string][]Permission{ + User: []Permission{}, + ByOrgID: map[string]OrgPermissions{ organizationID.String(): { - { - Negate: true, - ResourceType: ResourceWorkspace.Type, - Action: policy.ActionCreate, - }, - { - Negate: true, - ResourceType: ResourceWorkspace.Type, - Action: policy.ActionDelete, - }, - { - Negate: true, - ResourceType: ResourceWorkspace.Type, - Action: policy.ActionCreateAgent, - }, - { - Negate: true, - ResourceType: ResourceWorkspace.Type, - Action: policy.ActionDeleteAgent, + Org: []Permission{ + { + Negate: true, + ResourceType: ResourceWorkspace.Type, + Action: policy.ActionCreate, + }, + { + Negate: true, + ResourceType: ResourceWorkspace.Type, + Action: policy.ActionDelete, + }, + { + Negate: true, + ResourceType: ResourceWorkspace.Type, + Action: policy.ActionCreateAgent, + }, + { + Negate: true, + ResourceType: ResourceWorkspace.Type, + Action: policy.ActionDeleteAgent, + }, }, }, }, - User: []Permission{}, } }, } @@ -680,17 +692,20 @@ type Role struct { // that means the UI should never display it. DisplayName string `json:"display_name"` Site []Permission `json:"site"` - // Org is a map of orgid to permissions. We represent orgid as a string. - // We scope the organizations in the role so we can easily combine all the - // roles. - Org map[string][]Permission `json:"org"` - User []Permission `json:"user"` + User []Permission `json:"user"` + // ByOrgID is a map of organization IDs to permissions. Grouping by + // organization makes roles easy to combine. + ByOrgID map[string]OrgPermissions `json:"by_org_id"` // cachedRegoValue can be used to cache the rego value for this role. // This is helpful for static roles that never change. cachedRegoValue ast.Value } +type OrgPermissions struct { + Org []Permission `json:"org"` +} + // Valid will check all it's permissions and ensure they are all correct // according to the policy. This verifies every action specified make sense // for the given resource. @@ -702,8 +717,8 @@ func (role Role) Valid() error { } } - for orgID, permissions := range role.Org { - for _, perm := range permissions { + for orgID, orgPermissions := range role.ByOrgID { + for _, perm := range orgPermissions.Org { if err := perm.Valid(); err != nil { errs = append(errs, xerrors.Errorf("org=%q: %w", orgID, err)) } @@ -774,7 +789,7 @@ func RoleByName(name RoleIdentifier) (Role, error) { // Ensure all org roles are properly scoped a non-empty organization id. // This is just some defensive programming. role := roleFunc(name.OrganizationID) - if len(role.Org) > 0 && name.OrganizationID == uuid.Nil { + if len(role.ByOrgID) > 0 && name.OrganizationID == uuid.Nil { return Role{}, xerrors.Errorf("expect a org id for role %q", name.String()) } diff --git a/coderd/rbac/roles_internal_test.go b/coderd/rbac/roles_internal_test.go index 5c7a00d899..5f18cac44a 100644 --- a/coderd/rbac/roles_internal_test.go +++ b/coderd/rbac/roles_internal_test.go @@ -270,17 +270,17 @@ func TestDeduplicatePermissions(t *testing.T) { require.Equal(t, want, got) } -// SameAs compares 2 roles for equality. +// equalRoles compares 2 roles for equality. func equalRoles(t *testing.T, a, b Role) { require.Equal(t, a.Identifier, b.Identifier, "role names") require.Equal(t, a.DisplayName, b.DisplayName, "role display names") require.ElementsMatch(t, a.Site, b.Site, "site permissions") require.ElementsMatch(t, a.User, b.User, "user permissions") - require.Equal(t, len(a.Org), len(b.Org), "same number of org roles") + require.Equal(t, len(a.ByOrgID), len(b.ByOrgID), "same number of org roles") - for ak, av := range a.Org { - bv, ok := b.Org[ak] + for ak, av := range a.ByOrgID { + bv, ok := b.ByOrgID[ak] require.True(t, ok, "org permissions missing: %s", ak) - require.ElementsMatchf(t, av, bv, "org %s permissions", ak) + require.ElementsMatchf(t, av.Org, bv.Org, "org %s permissions", ak) } } diff --git a/coderd/rbac/rolestore/rolestore.go b/coderd/rbac/rolestore/rolestore.go index 610b04c06a..c2189c13b0 100644 --- a/coderd/rbac/rolestore/rolestore.go +++ b/coderd/rbac/rolestore/rolestore.go @@ -124,7 +124,6 @@ func ConvertDBRole(dbRole database.CustomRole) (rbac.Role, error) { Identifier: dbRole.RoleIdentifier(), DisplayName: dbRole.DisplayName, Site: convertPermissions(dbRole.SitePermissions), - Org: nil, User: convertPermissions(dbRole.UserPermissions), } @@ -134,8 +133,10 @@ func ConvertDBRole(dbRole database.CustomRole) (rbac.Role, error) { } if dbRole.OrganizationID.UUID != uuid.Nil { - role.Org = map[string][]rbac.Permission{ - dbRole.OrganizationID.UUID.String(): convertPermissions(dbRole.OrgPermissions), + role.ByOrgID = map[string]rbac.OrgPermissions{ + dbRole.OrganizationID.UUID.String(): { + Org: convertPermissions(dbRole.OrgPermissions), + }, } } diff --git a/coderd/rbac/scopes.go b/coderd/rbac/scopes.go index ff8f08adb1..5c8c803056 100644 --- a/coderd/rbac/scopes.go +++ b/coderd/rbac/scopes.go @@ -78,8 +78,8 @@ var builtinScopes = map[ScopeName]Scope{ Site: Permissions(map[string][]policy.Action{ ResourceWildcard.Type: {policy.WildcardSymbol}, }), - Org: map[string][]Permission{}, - User: []Permission{}, + User: []Permission{}, + ByOrgID: map[string]OrgPermissions{}, }, AllowIDList: []AllowListElement{AllowListAll()}, }, @@ -91,8 +91,8 @@ var builtinScopes = map[ScopeName]Scope{ Site: Permissions(map[string][]policy.Action{ ResourceWorkspace.Type: {policy.ActionApplicationConnect}, }), - Org: map[string][]Permission{}, - User: []Permission{}, + User: []Permission{}, + ByOrgID: map[string]OrgPermissions{}, }, AllowIDList: []AllowListElement{AllowListAll()}, }, @@ -102,8 +102,8 @@ var builtinScopes = map[ScopeName]Scope{ Identifier: RoleIdentifier{Name: fmt.Sprintf("Scope_%s", ScopeNoUserData)}, DisplayName: "Scope without access to user data", Site: allPermsExcept(ResourceUser), - Org: map[string][]Permission{}, User: []Permission{}, + ByOrgID: map[string]OrgPermissions{}, }, AllowIDList: []AllowListElement{AllowListAll()}, }, @@ -232,8 +232,8 @@ func ExpandScope(scope ScopeName) (Scope, error) { Identifier: RoleIdentifier{Name: fmt.Sprintf("Scope_%s", scope)}, DisplayName: string(scope), Site: site, - Org: map[string][]Permission{}, User: []Permission{}, + ByOrgID: map[string]OrgPermissions{}, }, // Composites are site-level; allow-list empty by default AllowIDList: []AllowListElement{{Type: policy.WildcardSymbol, ID: policy.WildcardSymbol}}, @@ -289,8 +289,8 @@ func expandLowLevel(resource string, action policy.Action) Scope { Identifier: RoleIdentifier{Name: fmt.Sprintf("Scope_%s:%s", resource, action)}, DisplayName: fmt.Sprintf("%s:%s", resource, action), Site: []Permission{{ResourceType: resource, Action: action}}, - Org: map[string][]Permission{}, User: []Permission{}, + ByOrgID: map[string]OrgPermissions{}, }, // Low-level scopes intentionally return a wildcard allow list. AllowIDList: []AllowListElement{{Type: policy.WildcardSymbol, ID: policy.WildcardSymbol}}, diff --git a/coderd/rbac/scopes_catalog_internal_test.go b/coderd/rbac/scopes_catalog_internal_test.go index 4530447e99..37de001fae 100644 --- a/coderd/rbac/scopes_catalog_internal_test.go +++ b/coderd/rbac/scopes_catalog_internal_test.go @@ -36,7 +36,7 @@ func TestExternalScopeNames(t *testing.T) { expected, ok := CompositeSitePermissions(ScopeName(name)) require.Truef(t, ok, "expected composite scope definition: %s", name) require.ElementsMatchf(t, expected, s.Site, "unexpected expanded permissions for %s", name) - require.Empty(t, s.Org) + require.Empty(t, s.ByOrgID) require.Empty(t, s.User) continue } @@ -50,7 +50,7 @@ func TestExternalScopeNames(t *testing.T) { require.Len(t, s.Site, 1) require.Equal(t, res, s.Site[0].ResourceType) require.Equal(t, act, s.Site[0].Action) - require.Empty(t, s.Org) + require.Empty(t, s.ByOrgID) require.Empty(t, s.User) } } diff --git a/coderd/rbac/scopes_test.go b/coderd/rbac/scopes_test.go index a6f50b0326..270f6ff028 100644 --- a/coderd/rbac/scopes_test.go +++ b/coderd/rbac/scopes_test.go @@ -34,7 +34,7 @@ func TestExpandScope(t *testing.T) { 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.Org) + require.Empty(t, s.ByOrgID) require.Empty(t, s.User) require.Equal(t, []rbac.AllowListElement{rbac.AllowListAll()}, s.AllowIDList) diff --git a/coderd/workspaceapps/db.go b/coderd/workspaceapps/db.go index 9e26a28c71..337f4fab52 100644 --- a/coderd/workspaceapps/db.go +++ b/coderd/workspaceapps/db.go @@ -355,21 +355,20 @@ func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *rbac.Subj return true, []string{}, nil } case database.AppSharingLevelOrganization: - // Check if the user is a member of the same organization as the workspace // First check if they have permission to connect to their own workspace (enforces scopes) err := p.Authorizer.Authorize(ctx, *roles, rbacAction, rbacResourceOwned) if err != nil { return false, warnings, nil } - // Check if the user is a member of the workspace's organization + // Check if the user is a member of the same organization as the workspace workspaceOrgID := dbReq.Workspace.OrganizationID expandedRoles, err := roles.Roles.Expand() if err != nil { return false, warnings, xerrors.Errorf("expand roles: %w", err) } for _, role := range expandedRoles { - if _, ok := role.Org[workspaceOrgID.String()]; ok { + if _, ok := role.ByOrgID[workspaceOrgID.String()]; ok { return true, []string{}, nil } } diff --git a/enterprise/coderd/coderd_test.go b/enterprise/coderd/coderd_test.go index 860c4135e0..00bb3e59e6 100644 --- a/enterprise/coderd/coderd_test.go +++ b/enterprise/coderd/coderd_test.go @@ -739,8 +739,8 @@ func testDBAuthzRole(ctx context.Context) context.Context { Site: rbac.Permissions(map[string][]policy.Action{ rbac.ResourceWildcard.Type: {policy.WildcardSymbol}, }), - Org: map[string][]rbac.Permission{}, - User: []rbac.Permission{}, + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, }, }), Scope: rbac.ScopeAll, diff --git a/enterprise/tailnet/pgcoord.go b/enterprise/tailnet/pgcoord.go index 32bd896669..79e5c13c93 100644 --- a/enterprise/tailnet/pgcoord.go +++ b/enterprise/tailnet/pgcoord.go @@ -106,8 +106,8 @@ var pgCoordSubject = rbac.Subject{ Site: rbac.Permissions(map[string][]policy.Action{ rbac.ResourceTailnetCoordinator.Type: {policy.WildcardSymbol}, }), - Org: map[string][]rbac.Permission{}, - User: []rbac.Permission{}, + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, }, }), Scope: rbac.ScopeAll,