Skip to content

Commit cccdf1e

Browse files
Emyrkjaaydenhaslilac
authored
feat: implement WorkspaceCreationBan org role (coder#16686)
Using negative permissions, this role prevents a user's ability to create & delete a workspace within a given organization. Workspaces are uniquely owned by an org and a user, so the org has to supercede the user permission with a negative permission. # Use case Organizations must be able to restrict a member's ability to create a workspace. This permission is implicitly granted (see coder#16546 (comment)). To revoke this permission, the solution chosen was to use negative permissions in a built in role called `WorkspaceCreationBan`. # Rational Using negative permissions is new territory, and not ideal. However, workspaces are in a unique position. Workspaces have 2 owners. The organization and the user. To prevent users from creating a workspace in another organization, an [implied negative permission](https://github.com/coder/coder/blob/36d9f5ddb3d98029fee07d004709e1e51022e979/coderd/rbac/policy.rego#L172-L192) is used. So the truth table looks like: _how to read this table [here](https://github.com/coder/coder/blob/36d9f5ddb3d98029fee07d004709e1e51022e979/coderd/rbac/README.md#roles)_ | Role (example) | Site | Org | User | Result | |-----------------|------|------|------|--------| | non-org-member | \_ | N | YN\_ | N | | user | \_ | \_ | Y | Y | | WorkspaceBan | \_ | N | Y | Y | | unauthenticated | \_ | \_ | \_ | N | This new role, `WorkspaceCreationBan` is the same truth table condition as if the user was not a member of the organization (when doing a workspace create/delete). So this behavior **is not entirely new**. <details> <summary>How to do it without a negative permission</summary> The alternate approach would be to remove the implied permission, and grant it via and organization role. However this would add new behavior that an organizational role has the ability to grant a user permissions on their own resources? It does not make sense for an org role to prevent user from changing their profile information for example. So the only option is to create a new truth table column for resources that are owned by both an organization and a user. | Role (example) | Site | Org |User+Org| User | Result | |-----------------|------|------|--------|------|--------| | non-org-member | \_ | N | \_ | \_ | N | | user | \_ | \_ | \_ | \_ | N | | WorkspaceAllow | \_ | \_ | Y | \_ | Y | | unauthenticated | \_ | \_ | \_ | \_ | N | Now a user has no opinion on if they can create a workspace, which feels a little wrong. A user should have the authority over what is theres. There is fundamental _philosophical_ question of "Who does a workspace belong to?". The user has some set of autonomy, yet it is the organization that controls it's existence. A head scratcher 🤔 </details> ## Will we need more negative built in roles? There are few resources that have shared ownership. Only `ResourceOrganizationMember` and `ResourceGroupMember`. Since negative permissions is intended to revoke access to a shared resource, then **no.** **This is the only one we need**. Classic resources like `ResourceTemplate` are entirely controlled by the Organization permissions. And resources entirely in the user control (like user profile) are only controlled by `User` permissions. ![Uploading Screenshot 2025-02-26 at 22.26.52.png…]() --------- Co-authored-by: Jaayden Halko <[email protected]> Co-authored-by: ケイラ <[email protected]>
1 parent 4ba5a8a commit cccdf1e

File tree

11 files changed

+261
-65
lines changed

11 files changed

+261
-65
lines changed

Diff for: ‎coderd/httpapi/httpapi.go

+6-4
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,13 @@ func ResourceNotFound(rw http.ResponseWriter) {
151151
Write(context.Background(), rw, http.StatusNotFound, ResourceNotFoundResponse)
152152
}
153153

154+
var ResourceForbiddenResponse = codersdk.Response{
155+
Message: "Forbidden.",
156+
Detail: "You don't have permission to view this content. If you believe this is a mistake, please contact your administrator or try signing in with different credentials.",
157+
}
158+
154159
func Forbidden(rw http.ResponseWriter) {
155-
Write(context.Background(), rw, http.StatusForbidden, codersdk.Response{
156-
Message: "Forbidden.",
157-
Detail: "You don't have permission to view this content. If you believe this is a mistake, please contact your administrator or try signing in with different credentials.",
158-
})
160+
Write(context.Background(), rw, http.StatusForbidden, ResourceForbiddenResponse)
159161
}
160162

161163
func InternalServerError(rw http.ResponseWriter, err error) {

Diff for: ‎coderd/rbac/roles.go

+72-35
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,12 @@ const (
2727
customSiteRole string = "custom-site-role"
2828
customOrganizationRole string = "custom-organization-role"
2929

30-
orgAdmin string = "organization-admin"
31-
orgMember string = "organization-member"
32-
orgAuditor string = "organization-auditor"
33-
orgUserAdmin string = "organization-user-admin"
34-
orgTemplateAdmin string = "organization-template-admin"
30+
orgAdmin string = "organization-admin"
31+
orgMember string = "organization-member"
32+
orgAuditor string = "organization-auditor"
33+
orgUserAdmin string = "organization-user-admin"
34+
orgTemplateAdmin string = "organization-template-admin"
35+
orgWorkspaceCreationBan string = "organization-workspace-creation-ban"
3536
)
3637

3738
func init() {
@@ -159,6 +160,10 @@ func RoleOrgTemplateAdmin() string {
159160
return orgTemplateAdmin
160161
}
161162

163+
func RoleOrgWorkspaceCreationBan() string {
164+
return orgWorkspaceCreationBan
165+
}
166+
162167
// ScopedRoleOrgAdmin is the org role with the organization ID
163168
func ScopedRoleOrgAdmin(organizationID uuid.UUID) RoleIdentifier {
164169
return RoleIdentifier{Name: RoleOrgAdmin(), OrganizationID: organizationID}
@@ -181,6 +186,10 @@ func ScopedRoleOrgTemplateAdmin(organizationID uuid.UUID) RoleIdentifier {
181186
return RoleIdentifier{Name: RoleOrgTemplateAdmin(), OrganizationID: organizationID}
182187
}
183188

189+
func ScopedRoleOrgWorkspaceCreationBan(organizationID uuid.UUID) RoleIdentifier {
190+
return RoleIdentifier{Name: RoleOrgWorkspaceCreationBan(), OrganizationID: organizationID}
191+
}
192+
184193
func allPermsExcept(excepts ...Objecter) []Permission {
185194
resources := AllResources()
186195
var perms []Permission
@@ -496,6 +505,31 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
496505
User: []Permission{},
497506
}
498507
},
508+
// orgWorkspaceCreationBan prevents creating & deleting workspaces. This
509+
// overrides any permissions granted by the org or user level. It accomplishes
510+
// this by using negative permissions.
511+
orgWorkspaceCreationBan: func(organizationID uuid.UUID) Role {
512+
return Role{
513+
Identifier: RoleIdentifier{Name: orgWorkspaceCreationBan, OrganizationID: organizationID},
514+
DisplayName: "Organization Workspace Creation Ban",
515+
Site: []Permission{},
516+
Org: map[string][]Permission{
517+
organizationID.String(): {
518+
{
519+
Negate: true,
520+
ResourceType: ResourceWorkspace.Type,
521+
Action: policy.ActionCreate,
522+
},
523+
{
524+
Negate: true,
525+
ResourceType: ResourceWorkspace.Type,
526+
Action: policy.ActionDelete,
527+
},
528+
},
529+
},
530+
User: []Permission{},
531+
}
532+
},
499533
}
500534
}
501535

@@ -506,44 +540,47 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
506540
// map[actor_role][assign_role]<can_assign>
507541
var assignRoles = map[string]map[string]bool{
508542
"system": {
509-
owner: true,
510-
auditor: true,
511-
member: true,
512-
orgAdmin: true,
513-
orgMember: true,
514-
orgAuditor: true,
515-
orgUserAdmin: true,
516-
orgTemplateAdmin: true,
517-
templateAdmin: true,
518-
userAdmin: true,
519-
customSiteRole: true,
520-
customOrganizationRole: true,
543+
owner: true,
544+
auditor: true,
545+
member: true,
546+
orgAdmin: true,
547+
orgMember: true,
548+
orgAuditor: true,
549+
orgUserAdmin: true,
550+
orgTemplateAdmin: true,
551+
orgWorkspaceCreationBan: true,
552+
templateAdmin: true,
553+
userAdmin: true,
554+
customSiteRole: true,
555+
customOrganizationRole: true,
521556
},
522557
owner: {
523-
owner: true,
524-
auditor: true,
525-
member: true,
526-
orgAdmin: true,
527-
orgMember: true,
528-
orgAuditor: true,
529-
orgUserAdmin: true,
530-
orgTemplateAdmin: true,
531-
templateAdmin: true,
532-
userAdmin: true,
533-
customSiteRole: true,
534-
customOrganizationRole: true,
558+
owner: true,
559+
auditor: true,
560+
member: true,
561+
orgAdmin: true,
562+
orgMember: true,
563+
orgAuditor: true,
564+
orgUserAdmin: true,
565+
orgTemplateAdmin: true,
566+
orgWorkspaceCreationBan: true,
567+
templateAdmin: true,
568+
userAdmin: true,
569+
customSiteRole: true,
570+
customOrganizationRole: true,
535571
},
536572
userAdmin: {
537573
member: true,
538574
orgMember: true,
539575
},
540576
orgAdmin: {
541-
orgAdmin: true,
542-
orgMember: true,
543-
orgAuditor: true,
544-
orgUserAdmin: true,
545-
orgTemplateAdmin: true,
546-
customOrganizationRole: true,
577+
orgAdmin: true,
578+
orgMember: true,
579+
orgAuditor: true,
580+
orgUserAdmin: true,
581+
orgTemplateAdmin: true,
582+
orgWorkspaceCreationBan: true,
583+
customOrganizationRole: true,
547584
},
548585
orgUserAdmin: {
549586
orgMember: true,

Diff for: ‎coderd/rbac/roles_test.go

+15-3
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ func TestRolePermissions(t *testing.T) {
112112
// Subjects to user
113113
memberMe := authSubject{Name: "member_me", Actor: rbac.Subject{ID: currentUser.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember()}}}
114114
orgMemberMe := authSubject{Name: "org_member_me", Actor: rbac.Subject{ID: currentUser.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(orgID)}}}
115+
orgMemberMeBanWorkspace := authSubject{Name: "org_member_me_workspace_ban", Actor: rbac.Subject{ID: currentUser.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(orgID), rbac.ScopedRoleOrgWorkspaceCreationBan(orgID)}}}
115116
groupMemberMe := authSubject{Name: "group_member_me", Actor: rbac.Subject{ID: currentUser.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(orgID)}, Groups: []string{groupID.String()}}}
116117

117118
owner := authSubject{Name: "owner", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.RoleOwner()}}}
@@ -181,20 +182,30 @@ func TestRolePermissions(t *testing.T) {
181182
Actions: []policy.Action{policy.ActionRead},
182183
Resource: rbac.ResourceWorkspace.WithID(workspaceID).InOrg(orgID).WithOwner(currentUser.String()),
183184
AuthorizeMap: map[bool][]hasAuthSubjects{
184-
true: {owner, orgMemberMe, orgAdmin, templateAdmin, orgTemplateAdmin},
185+
true: {owner, orgMemberMe, orgAdmin, templateAdmin, orgTemplateAdmin, orgMemberMeBanWorkspace},
185186
false: {setOtherOrg, memberMe, userAdmin, orgAuditor, orgUserAdmin},
186187
},
187188
},
188189
{
189-
Name: "C_RDMyWorkspaceInOrg",
190+
Name: "UpdateMyWorkspaceInOrg",
190191
// When creating the WithID won't be set, but it does not change the result.
191-
Actions: []policy.Action{policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete},
192+
Actions: []policy.Action{policy.ActionUpdate},
192193
Resource: rbac.ResourceWorkspace.WithID(workspaceID).InOrg(orgID).WithOwner(currentUser.String()),
193194
AuthorizeMap: map[bool][]hasAuthSubjects{
194195
true: {owner, orgMemberMe, orgAdmin},
195196
false: {setOtherOrg, memberMe, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor},
196197
},
197198
},
199+
{
200+
Name: "CreateDeleteMyWorkspaceInOrg",
201+
// When creating the WithID won't be set, but it does not change the result.
202+
Actions: []policy.Action{policy.ActionCreate, policy.ActionDelete},
203+
Resource: rbac.ResourceWorkspace.WithID(workspaceID).InOrg(orgID).WithOwner(currentUser.String()),
204+
AuthorizeMap: map[bool][]hasAuthSubjects{
205+
true: {owner, orgMemberMe, orgAdmin},
206+
false: {setOtherOrg, memberMe, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor, orgMemberMeBanWorkspace},
207+
},
208+
},
198209
{
199210
Name: "MyWorkspaceInOrgExecution",
200211
// When creating the WithID won't be set, but it does not change the result.
@@ -942,6 +953,7 @@ func TestListRoles(t *testing.T) {
942953
fmt.Sprintf("organization-auditor:%s", orgID.String()),
943954
fmt.Sprintf("organization-user-admin:%s", orgID.String()),
944955
fmt.Sprintf("organization-template-admin:%s", orgID.String()),
956+
fmt.Sprintf("organization-workspace-creation-ban:%s", orgID.String()),
945957
},
946958
orgRoleNames)
947959
}

Diff for: ‎coderd/workspaces_test.go

+48
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,54 @@ func TestWorkspace(t *testing.T) {
375375
require.Error(t, err, "create workspace with archived version")
376376
require.ErrorContains(t, err, "Archived template versions cannot")
377377
})
378+
379+
t.Run("WorkspaceBan", func(t *testing.T) {
380+
t.Parallel()
381+
owner, _, _ := coderdtest.NewWithAPI(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
382+
first := coderdtest.CreateFirstUser(t, owner)
383+
384+
version := coderdtest.CreateTemplateVersion(t, owner, first.OrganizationID, nil)
385+
coderdtest.AwaitTemplateVersionJobCompleted(t, owner, version.ID)
386+
template := coderdtest.CreateTemplate(t, owner, first.OrganizationID, version.ID)
387+
388+
goodClient, _ := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID)
389+
390+
// When a user with workspace-creation-ban
391+
client, user := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.ScopedRoleOrgWorkspaceCreationBan(first.OrganizationID))
392+
393+
// Ensure a similar user can create a workspace
394+
coderdtest.CreateWorkspace(t, goodClient, template.ID)
395+
396+
ctx := testutil.Context(t, testutil.WaitLong)
397+
// Then: Cannot create a workspace
398+
_, err := client.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{
399+
TemplateID: template.ID,
400+
TemplateVersionID: uuid.UUID{},
401+
Name: "random",
402+
})
403+
require.Error(t, err)
404+
var apiError *codersdk.Error
405+
require.ErrorAs(t, err, &apiError)
406+
require.Equal(t, http.StatusForbidden, apiError.StatusCode())
407+
408+
// When: workspace-ban use has a workspace
409+
wrk, err := owner.CreateUserWorkspace(ctx, user.ID.String(), codersdk.CreateWorkspaceRequest{
410+
TemplateID: template.ID,
411+
TemplateVersionID: uuid.UUID{},
412+
Name: "random",
413+
})
414+
require.NoError(t, err)
415+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, wrk.LatestBuild.ID)
416+
417+
// Then: They cannot delete said workspace
418+
_, err = client.CreateWorkspaceBuild(ctx, wrk.ID, codersdk.CreateWorkspaceBuildRequest{
419+
Transition: codersdk.WorkspaceTransitionDelete,
420+
ProvisionerState: []byte{},
421+
})
422+
require.Error(t, err)
423+
require.ErrorAs(t, err, &apiError)
424+
require.Equal(t, http.StatusForbidden, apiError.StatusCode())
425+
})
378426
}
379427

380428
func TestResolveAutostart(t *testing.T) {

Diff for: ‎coderd/wsbuilder/wsbuilder.go

+9
Original file line numberDiff line numberDiff line change
@@ -790,6 +790,15 @@ func (b *Builder) authorize(authFunc func(action policy.Action, object rbac.Obje
790790
return BuildError{http.StatusBadRequest, msg, xerrors.New(msg)}
791791
}
792792
if !authFunc(action, b.workspace) {
793+
if authFunc(policy.ActionRead, b.workspace) {
794+
// If the user can read the workspace, but not delete/create/update. Show
795+
// a more helpful error. They are allowed to know the workspace exists.
796+
return BuildError{
797+
Status: http.StatusForbidden,
798+
Message: fmt.Sprintf("You do not have permission to %s this workspace.", action),
799+
Wrapped: xerrors.New(httpapi.ResourceForbiddenResponse.Detail),
800+
}
801+
}
793802
// We use the same wording as the httpapi to avoid leaking the existence of the workspace
794803
return BuildError{http.StatusNotFound, httpapi.ResourceNotFoundResponse.Message, xerrors.New(httpapi.ResourceNotFoundResponse.Message)}
795804
}

Diff for: ‎codersdk/rbacroles.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@ const (
88
RoleUserAdmin string = "user-admin"
99
RoleAuditor string = "auditor"
1010

11-
RoleOrganizationAdmin string = "organization-admin"
12-
RoleOrganizationMember string = "organization-member"
13-
RoleOrganizationAuditor string = "organization-auditor"
14-
RoleOrganizationTemplateAdmin string = "organization-template-admin"
15-
RoleOrganizationUserAdmin string = "organization-user-admin"
11+
RoleOrganizationAdmin string = "organization-admin"
12+
RoleOrganizationMember string = "organization-member"
13+
RoleOrganizationAuditor string = "organization-auditor"
14+
RoleOrganizationTemplateAdmin string = "organization-template-admin"
15+
RoleOrganizationUserAdmin string = "organization-user-admin"
16+
RoleOrganizationWorkspaceCreationBan string = "organization-workspace-creation-ban"
1617
)

Diff for: ‎enterprise/coderd/roles_test.go

+15-12
Original file line numberDiff line numberDiff line change
@@ -441,10 +441,11 @@ func TestListRoles(t *testing.T) {
441441
return member.ListOrganizationRoles(ctx, owner.OrganizationID)
442442
},
443443
ExpectedRoles: convertRoles(map[rbac.RoleIdentifier]bool{
444-
{Name: codersdk.RoleOrganizationAdmin, OrganizationID: owner.OrganizationID}: false,
445-
{Name: codersdk.RoleOrganizationAuditor, OrganizationID: owner.OrganizationID}: false,
446-
{Name: codersdk.RoleOrganizationTemplateAdmin, OrganizationID: owner.OrganizationID}: false,
447-
{Name: codersdk.RoleOrganizationUserAdmin, OrganizationID: owner.OrganizationID}: false,
444+
{Name: codersdk.RoleOrganizationAdmin, OrganizationID: owner.OrganizationID}: false,
445+
{Name: codersdk.RoleOrganizationAuditor, OrganizationID: owner.OrganizationID}: false,
446+
{Name: codersdk.RoleOrganizationTemplateAdmin, OrganizationID: owner.OrganizationID}: false,
447+
{Name: codersdk.RoleOrganizationUserAdmin, OrganizationID: owner.OrganizationID}: false,
448+
{Name: codersdk.RoleOrganizationWorkspaceCreationBan, OrganizationID: owner.OrganizationID}: false,
448449
}),
449450
},
450451
{
@@ -473,10 +474,11 @@ func TestListRoles(t *testing.T) {
473474
return orgAdmin.ListOrganizationRoles(ctx, owner.OrganizationID)
474475
},
475476
ExpectedRoles: convertRoles(map[rbac.RoleIdentifier]bool{
476-
{Name: codersdk.RoleOrganizationAdmin, OrganizationID: owner.OrganizationID}: true,
477-
{Name: codersdk.RoleOrganizationAuditor, OrganizationID: owner.OrganizationID}: true,
478-
{Name: codersdk.RoleOrganizationTemplateAdmin, OrganizationID: owner.OrganizationID}: true,
479-
{Name: codersdk.RoleOrganizationUserAdmin, OrganizationID: owner.OrganizationID}: true,
477+
{Name: codersdk.RoleOrganizationAdmin, OrganizationID: owner.OrganizationID}: true,
478+
{Name: codersdk.RoleOrganizationAuditor, OrganizationID: owner.OrganizationID}: true,
479+
{Name: codersdk.RoleOrganizationTemplateAdmin, OrganizationID: owner.OrganizationID}: true,
480+
{Name: codersdk.RoleOrganizationUserAdmin, OrganizationID: owner.OrganizationID}: true,
481+
{Name: codersdk.RoleOrganizationWorkspaceCreationBan, OrganizationID: owner.OrganizationID}: true,
480482
}),
481483
},
482484
{
@@ -505,10 +507,11 @@ func TestListRoles(t *testing.T) {
505507
return client.ListOrganizationRoles(ctx, owner.OrganizationID)
506508
},
507509
ExpectedRoles: convertRoles(map[rbac.RoleIdentifier]bool{
508-
{Name: codersdk.RoleOrganizationAdmin, OrganizationID: owner.OrganizationID}: true,
509-
{Name: codersdk.RoleOrganizationAuditor, OrganizationID: owner.OrganizationID}: true,
510-
{Name: codersdk.RoleOrganizationTemplateAdmin, OrganizationID: owner.OrganizationID}: true,
511-
{Name: codersdk.RoleOrganizationUserAdmin, OrganizationID: owner.OrganizationID}: true,
510+
{Name: codersdk.RoleOrganizationAdmin, OrganizationID: owner.OrganizationID}: true,
511+
{Name: codersdk.RoleOrganizationAuditor, OrganizationID: owner.OrganizationID}: true,
512+
{Name: codersdk.RoleOrganizationTemplateAdmin, OrganizationID: owner.OrganizationID}: true,
513+
{Name: codersdk.RoleOrganizationUserAdmin, OrganizationID: owner.OrganizationID}: true,
514+
{Name: codersdk.RoleOrganizationWorkspaceCreationBan, OrganizationID: owner.OrganizationID}: true,
512515
}),
513516
},
514517
}

Diff for: ‎site/src/api/typesGenerated.ts

+4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: ‎site/src/pages/OrganizationSettingsPage/UserTable/EditRolesButton.stories.tsx

+12
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
MockOwnerRole,
55
MockSiteRoles,
66
MockUserAdminRole,
7+
MockWorkspaceCreationBanRole,
78
} from "testHelpers/entities";
89
import { withDesktopViewport } from "testHelpers/storybook";
910
import { EditRolesButton } from "./EditRolesButton";
@@ -41,3 +42,14 @@ export const Loading: Story = {
4142
await userEvent.click(canvas.getByRole("button"));
4243
},
4344
};
45+
46+
export const AdvancedOpen: Story = {
47+
args: {
48+
selectedRoleNames: new Set([MockWorkspaceCreationBanRole.name]),
49+
roles: MockSiteRoles,
50+
},
51+
play: async ({ canvasElement }) => {
52+
const canvas = within(canvasElement);
53+
await userEvent.click(canvas.getByRole("button"));
54+
},
55+
};

0 commit comments

Comments
 (0)