Skip to content

Commit 5ecd163

Browse files
EmyrkEmyrk
committed
chore: static role assignment mapping
Until a dynamic approach is created in the database, only org-admins can assign custom organization roles.
1 parent c587af7 commit 5ecd163

File tree

7 files changed

+126
-30
lines changed

7 files changed

+126
-30
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -239,10 +239,10 @@ var (
239239
rbac.ResourceApiKey.Type: rbac.ResourceApiKey.AvailableActions(),
240240
rbac.ResourceGroup.Type: {policy.ActionCreate, policy.ActionUpdate},
241241
rbac.ResourceAssignRole.Type: rbac.ResourceAssignRole.AvailableActions(),
242+
rbac.ResourceAssignOrgRole.Type: rbac.ResourceAssignOrgRole.AvailableActions(),
242243
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
243244
rbac.ResourceOrganization.Type: {policy.ActionCreate, policy.ActionRead},
244245
rbac.ResourceOrganizationMember.Type: {policy.ActionCreate},
245-
rbac.ResourceAssignOrgRole.Type: {policy.ActionRead, policy.ActionCreate, policy.ActionDelete},
246246
rbac.ResourceProvisionerDaemon.Type: {policy.ActionCreate, policy.ActionUpdate},
247247
rbac.ResourceUser.Type: rbac.ResourceUser.AvailableActions(),
248248
rbac.ResourceWorkspaceDormant.Type: {policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceStop},
@@ -622,7 +622,7 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r
622622
roleAssign := rbac.ResourceAssignRole
623623
shouldBeOrgRoles := false
624624
if orgID != nil {
625-
roleAssign = roleAssign.InOrg(*orgID)
625+
roleAssign = rbac.ResourceAssignOrgRole.InOrg(*orgID)
626626
shouldBeOrgRoles = true
627627
}
628628

@@ -697,8 +697,14 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r
697697

698698
for _, roleName := range grantedRoles {
699699
if _, isCustom := customRolesMap[roleName]; isCustom {
700-
// For now, use a constant name so our static assign map still works.
701-
roleName = rbac.CustomSiteRole()
700+
// To support a dynamic mapping of what roles can assign what, we need
701+
// to store this in the database. For now, just use a static role so
702+
// owners and org admins can assign roles.
703+
if roleName.IsOrgRole() {
704+
roleName = rbac.CustomOrganizationRole(roleName.OrganizationID)
705+
} else {
706+
roleName = rbac.CustomSiteRole()
707+
}
702708
}
703709

704710
if !rbac.CanAssignRole(actor.Roles, roleName) {
@@ -3476,9 +3482,15 @@ func (q *querier) UpsertCustomRole(ctx context.Context, arg database.UpsertCusto
34763482
return database.CustomRole{}, NoActorError
34773483
}
34783484

3479-
// TODO: If this is an org role, check the org assign role type.
3480-
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignRole); err != nil {
3481-
return database.CustomRole{}, err
3485+
// Org and site role upsert share the same query. So switch the assertion based on the org uuid.
3486+
if arg.OrganizationID.UUID != uuid.Nil {
3487+
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil {
3488+
return database.CustomRole{}, err
3489+
}
3490+
} else {
3491+
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignRole); err != nil {
3492+
return database.CustomRole{}, err
3493+
}
34823494
}
34833495

34843496
if arg.OrganizationID.UUID == uuid.Nil && len(arg.OrgPermissions) > 0 {

‎coderd/members.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,10 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
8787
UserID: member.UserID,
8888
OrgID: organization.ID,
8989
})
90+
if httpapi.Is404Error(err) {
91+
httpapi.Forbidden(rw)
92+
return
93+
}
9094
if err != nil {
9195
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
9296
Message: err.Error(),

‎coderd/rbac/policy/policy.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ var RBACPermissions = map[string]PermissionDefinition{
218218
ActionAssign: actDef("ability to assign org scoped roles"),
219219
ActionRead: actDef("view what roles are assignable"),
220220
ActionDelete: actDef("ability to delete org scoped roles"),
221+
ActionCreate: actDef("ability to create/delete/edit custom roles within an organization"),
221222
},
222223
},
223224
"oauth2_app": {

‎coderd/rbac/roles.go

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ const (
2424
// customSiteRole is a placeholder for all custom site roles.
2525
// This is used for what roles can assign other roles.
2626
// TODO: Make this more dynamic to allow other roles to grant.
27-
customSiteRole string = "custom-site-role"
27+
customSiteRole string = "custom-site-role"
28+
customOrganizationRole string = "custom-organization-role"
2829

2930
orgAdmin string = "organization-admin"
3031
orgMember string = "organization-member"
@@ -125,8 +126,11 @@ func (r *RoleIdentifier) UnmarshalJSON(data []byte) error {
125126
// Once we have a database implementation, the "default" roles can be defined on the
126127
// site and orgs, and these functions can be removed.
127128

128-
func RoleOwner() RoleIdentifier { return RoleIdentifier{Name: owner} }
129-
func CustomSiteRole() RoleIdentifier { return RoleIdentifier{Name: customSiteRole} }
129+
func RoleOwner() RoleIdentifier { return RoleIdentifier{Name: owner} }
130+
func CustomSiteRole() RoleIdentifier { return RoleIdentifier{Name: customSiteRole} }
131+
func CustomOrganizationRole(orgID uuid.UUID) RoleIdentifier {
132+
return RoleIdentifier{Name: customOrganizationRole, OrganizationID: orgID}
133+
}
130134
func RoleTemplateAdmin() RoleIdentifier { return RoleIdentifier{Name: templateAdmin} }
131135
func RoleUserAdmin() RoleIdentifier { return RoleIdentifier{Name: userAdmin} }
132136
func RoleMember() RoleIdentifier { return RoleIdentifier{Name: member} }
@@ -354,7 +358,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
354358
Site: []Permission{},
355359
Org: map[string][]Permission{
356360
// Org admins should not have workspace exec perms.
357-
organizationID.String(): append(allPermsExcept(ResourceWorkspace, ResourceWorkspaceDormant), Permissions(map[string][]policy.Action{
361+
organizationID.String(): append(allPermsExcept(ResourceWorkspace, ResourceWorkspaceDormant, ResourceAssignRole), Permissions(map[string][]policy.Action{
358362
ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate, policy.ActionWorkspaceStop},
359363
ResourceWorkspace.Type: slice.Omit(ResourceWorkspace.AvailableActions(), policy.ActionApplicationConnect, policy.ActionSSH),
360364
})...),
@@ -402,32 +406,35 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
402406
// map[actor_role][assign_role]<can_assign>
403407
var assignRoles = map[string]map[string]bool{
404408
"system": {
405-
owner: true,
406-
auditor: true,
407-
member: true,
408-
orgAdmin: true,
409-
orgMember: true,
410-
templateAdmin: true,
411-
userAdmin: true,
412-
customSiteRole: true,
409+
owner: true,
410+
auditor: true,
411+
member: true,
412+
orgAdmin: true,
413+
orgMember: true,
414+
templateAdmin: true,
415+
userAdmin: true,
416+
customSiteRole: true,
417+
customOrganizationRole: true,
413418
},
414419
owner: {
415-
owner: true,
416-
auditor: true,
417-
member: true,
418-
orgAdmin: true,
419-
orgMember: true,
420-
templateAdmin: true,
421-
userAdmin: true,
422-
customSiteRole: true,
420+
owner: true,
421+
auditor: true,
422+
member: true,
423+
orgAdmin: true,
424+
orgMember: true,
425+
templateAdmin: true,
426+
userAdmin: true,
427+
customSiteRole: true,
428+
customOrganizationRole: true,
423429
},
424430
userAdmin: {
425431
member: true,
426432
orgMember: true,
427433
},
428434
orgAdmin: {
429-
orgAdmin: true,
430-
orgMember: true,
435+
orgAdmin: true,
436+
orgMember: true,
437+
customOrganizationRole: true,
431438
},
432439
}
433440

@@ -589,6 +596,13 @@ func RoleByName(name RoleIdentifier) (Role, error) {
589596
return Role{}, xerrors.Errorf("expect a org id for role %q", name.String())
590597
}
591598

599+
// This can happen if a custom role shares the same name as a built-in role.
600+
// You could make an org role called "owner", and we should not return the
601+
// owner role itself.
602+
if name.OrganizationID != role.Identifier.OrganizationID {
603+
return Role{}, xerrors.Errorf("role %q not found", name.String())
604+
}
605+
592606
return role, nil
593607
}
594608

‎coderd/roles.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,14 @@ func assignableRoles(actorRoles rbac.ExpandableRoles, roles []rbac.Role, customR
144144
}
145145

146146
for _, role := range customRoles {
147+
canAssign := rbac.CanAssignRole(actorRoles, rbac.CustomSiteRole())
148+
if role.RoleIdentifier().IsOrgRole() {
149+
canAssign = rbac.CanAssignRole(actorRoles, rbac.CustomOrganizationRole(role.OrganizationID.UUID))
150+
}
151+
147152
assignable = append(assignable, codersdk.AssignableRoles{
148153
Role: db2sdk.Role(role),
149-
Assignable: rbac.CanAssignRole(actorRoles, role.RoleIdentifier()),
154+
Assignable: canAssign,
150155
BuiltIn: false,
151156
})
152157
}

‎enterprise/coderd/roles_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ func TestCustomOrganizationRole(t *testing.T) {
203203
_, err := owner.PatchOrganizationRole(ctx, first.OrganizationID, codersdk.Role{
204204
Name: "Bad_Name", // No underscores allowed
205205
DisplayName: "Testing Purposes",
206+
OrganizationID: first.OrganizationID.String(),
206207
SitePermissions: nil,
207208
OrganizationPermissions: nil,
208209
UserPermissions: nil,

‎enterprise/coderd/users_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/stretchr/testify/require"
1010

1111
"github.com/coder/coder/v2/coderd/coderdtest"
12+
"github.com/coder/coder/v2/coderd/rbac"
1213
"github.com/coder/coder/v2/coderd/schedule/cron"
1314
"github.com/coder/coder/v2/codersdk"
1415
"github.com/coder/coder/v2/enterprise/coderd/coderdenttest"
@@ -237,3 +238,61 @@ func TestCreateFirstUser_Entitlements_Trial(t *testing.T) {
237238
require.NoError(t, err)
238239
require.True(t, entitlements.Trial, "Trial license should be immediately active.")
239240
}
241+
242+
// TestAssignCustomOrgRoles verifies an organization admin (not just an owner) can create
243+
// a custom role and assign it to an organization user.
244+
func TestAssignCustomOrgRoles(t *testing.T) {
245+
t.Parallel()
246+
dv := coderdtest.DeploymentValues(t)
247+
dv.Experiments = []string{string(codersdk.ExperimentCustomRoles)}
248+
249+
ownerClient, owner := coderdenttest.New(t, &coderdenttest.Options{
250+
Options: &coderdtest.Options{
251+
DeploymentValues: dv,
252+
IncludeProvisionerDaemon: true,
253+
},
254+
LicenseOptions: &coderdenttest.LicenseOptions{
255+
Features: license.Features{
256+
codersdk.FeatureCustomRoles: 1,
257+
},
258+
},
259+
})
260+
261+
client, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAdmin(owner.OrganizationID))
262+
tv := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil)
263+
coderdtest.AwaitTemplateVersionJobCompleted(t, client, tv.ID)
264+
265+
ctx := testutil.Context(t, testutil.WaitShort)
266+
// Create a custom role as an organization admin that allows making templates.
267+
auditorRole, err := client.PatchOrganizationRole(ctx, owner.OrganizationID, codersdk.Role{
268+
Name: "template-admin",
269+
OrganizationID: owner.OrganizationID.String(),
270+
DisplayName: "Template Admin",
271+
SitePermissions: nil,
272+
OrganizationPermissions: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
273+
codersdk.ResourceTemplate: codersdk.RBACResourceActions[codersdk.ResourceTemplate], // All template perms
274+
}),
275+
UserPermissions: nil,
276+
})
277+
require.NoError(t, err)
278+
279+
createTemplateReq := codersdk.CreateTemplateRequest{
280+
Name: "name",
281+
DisplayName: "Template",
282+
VersionID: tv.ID,
283+
}
284+
memberClient, member := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID)
285+
// Check the member cannot create a template
286+
_, err = memberClient.CreateTemplate(ctx, owner.OrganizationID, createTemplateReq)
287+
require.Error(t, err)
288+
289+
// Assign new role to the member as the org admin
290+
_, err = client.UpdateOrganizationMemberRoles(ctx, owner.OrganizationID, member.ID.String(), codersdk.UpdateRoles{
291+
Roles: []string{auditorRole.Name},
292+
})
293+
require.NoError(t, err)
294+
295+
// Now the member can create the template
296+
_, err = memberClient.CreateTemplate(ctx, owner.OrganizationID, createTemplateReq)
297+
require.NoError(t, err)
298+
}

0 commit comments

Comments
 (0)