Skip to content

Commit 5f896f0

Browse files
ssncferreirassncferreira
committed
test: improve end-to-end permission tests for deletion of prebuilt workspaces
1 parent d461a3d commit 5f896f0

File tree

10 files changed

+248
-112
lines changed

10 files changed

+248
-112
lines changed

‎cli/delete_test.go

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,18 @@ package cli_test
22

33
import (
44
"context"
5+
"database/sql"
56
"fmt"
67
"io"
78
"testing"
9+
"time"
10+
11+
"github.com/google/uuid"
12+
13+
"github.com/coder/coder/v2/coderd/database"
14+
"github.com/coder/coder/v2/coderd/database/dbgen"
15+
"github.com/coder/coder/v2/coderd/database/pubsub"
16+
"github.com/coder/quartz"
817

918
"github.com/stretchr/testify/assert"
1019
"github.com/stretchr/testify/require"
@@ -209,4 +218,214 @@ func TestDelete(t *testing.T) {
209218
cancel()
210219
<-doneChan
211220
})
221+
222+
t.Run("Workspace delete permissions", func(t *testing.T) {
223+
t.Parallel()
224+
if !dbtestutil.WillUsePostgres() {
225+
t.Skip("this test requires postgres")
226+
}
227+
228+
clock := quartz.NewMock(t)
229+
ctx := testutil.Context(t, testutil.WaitSuperLong)
230+
231+
// Setup
232+
db, pb := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
233+
client, _ := coderdtest.NewWithProvisionerCloser(t, &coderdtest.Options{
234+
Database: db,
235+
Pubsub: pb,
236+
IncludeProvisionerDaemon: true,
237+
})
238+
owner := coderdtest.CreateFirstUser(t, client)
239+
orgID := owner.OrganizationID
240+
241+
// Given a template version with a preset and a template
242+
version := coderdtest.CreateTemplateVersion(t, client, orgID, nil)
243+
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
244+
preset := setupTestDBPreset(t, db, version.ID)
245+
template := coderdtest.CreateTemplate(t, client, orgID, version.ID)
246+
247+
cases := []struct {
248+
name string
249+
client *codersdk.Client
250+
expectedPrebuiltDeleteErrMsg string
251+
expectedWorkspaceDeleteErrMsg string
252+
}{
253+
// Users with the OrgAdmin role should be able to delete both normal and prebuilt workspaces
254+
{
255+
name: "OrgAdmin",
256+
client: func() *codersdk.Client {
257+
client, _ := coderdtest.CreateAnotherUser(t, client, orgID, rbac.ScopedRoleOrgAdmin(orgID))
258+
return client
259+
}(),
260+
},
261+
// Users with the TemplateAdmin role should be able to delete prebuilt workspaces, but not normal workspaces
262+
{
263+
name: "TemplateAdmin",
264+
client: func() *codersdk.Client {
265+
client, _ := coderdtest.CreateAnotherUser(t, client, orgID, rbac.RoleTemplateAdmin())
266+
return client
267+
}(),
268+
expectedWorkspaceDeleteErrMsg: "unexpected status code 403: You do not have permission to delete this workspace.",
269+
},
270+
// Users with the OrgTemplateAdmin role should be able to delete prebuilt workspaces, but not normal workspaces
271+
{
272+
name: "OrgTemplateAdmin",
273+
client: func() *codersdk.Client {
274+
client, _ := coderdtest.CreateAnotherUser(t, client, orgID, rbac.ScopedRoleOrgTemplateAdmin(orgID))
275+
return client
276+
}(),
277+
expectedWorkspaceDeleteErrMsg: "unexpected status code 403: You do not have permission to delete this workspace.",
278+
},
279+
// Users with the Member role should not be able to delete prebuilt or normal workspaces
280+
{
281+
name: "Member",
282+
client: func() *codersdk.Client {
283+
client, _ := coderdtest.CreateAnotherUser(t, client, orgID, rbac.RoleMember())
284+
return client
285+
}(),
286+
expectedPrebuiltDeleteErrMsg: "unexpected status code 404: Resource not found or you do not have access to this resource",
287+
expectedWorkspaceDeleteErrMsg: "unexpected status code 404: Resource not found or you do not have access to this resource",
288+
},
289+
}
290+
291+
for _, tc := range cases {
292+
tc := tc
293+
t.Run(tc.name, func(t *testing.T) {
294+
t.Parallel()
295+
296+
// Create one prebuilt workspace (owned by system user) and one normal workspace (owned by the owner user)
297+
// Each workspace is persisted in the DB along with associated workspace jobs and builds.
298+
dbPrebuiltWorkspace := setupTestDBWorkspace(t, clock, db, pb, orgID, database.PrebuildsSystemUserID, template.ID, version.ID, preset.ID)
299+
dbUserWorkspace := setupTestDBWorkspace(t, clock, db, pb, orgID, owner.UserID, template.ID, version.ID, preset.ID)
300+
301+
// Ensure at least one prebuilt workspace is reported as running in the database
302+
testutil.Eventually(ctx, t, func(ctx context.Context) (done bool) {
303+
running, err := db.GetRunningPrebuiltWorkspaces(ctx)
304+
if !assert.NoError(t, err) || !assert.GreaterOrEqual(t, len(running), 1) {
305+
return false
306+
}
307+
return true
308+
}, testutil.IntervalMedium, "running prebuilt workspaces timeout")
309+
310+
runningWorkspaces, err := db.GetRunningPrebuiltWorkspaces(ctx)
311+
require.NoError(t, err)
312+
require.GreaterOrEqual(t, len(runningWorkspaces), 1)
313+
314+
// Get the full prebuilt workspace object from the DB
315+
prebuiltWorkspace, err := db.GetWorkspaceByID(ctx, runningWorkspaces[0].ID)
316+
require.NoError(t, err)
317+
318+
// Attempt to delete the prebuilt workspace as the test client
319+
build, err := tc.client.CreateWorkspaceBuild(ctx, dbPrebuiltWorkspace.ID, codersdk.CreateWorkspaceBuildRequest{
320+
Transition: codersdk.WorkspaceTransitionDelete,
321+
})
322+
// Validate the result based on the expected error message
323+
if tc.expectedPrebuiltDeleteErrMsg != "" {
324+
require.Error(t, err)
325+
require.Contains(t, err.Error(), tc.expectedPrebuiltDeleteErrMsg)
326+
} else {
327+
require.NoError(t, err, "delete the prebuilt workspace")
328+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, build.ID)
329+
330+
// Verify that the prebuilt workspace is now marked as deleted
331+
deletedWorkspace, err := client.DeletedWorkspace(ctx, prebuiltWorkspace.ID)
332+
require.NoError(t, err)
333+
require.Equal(t, prebuiltWorkspace.ID, deletedWorkspace.ID)
334+
}
335+
336+
// Get the full user workspace object from the DB
337+
userWorkspace, err := db.GetWorkspaceByID(ctx, dbUserWorkspace.ID)
338+
require.NoError(t, err)
339+
340+
// Attempt to delete the prebuilt workspace as the test client
341+
build, err = tc.client.CreateWorkspaceBuild(ctx, dbUserWorkspace.ID, codersdk.CreateWorkspaceBuildRequest{
342+
Transition: codersdk.WorkspaceTransitionDelete,
343+
})
344+
// Validate the result based on the expected error message
345+
if tc.expectedWorkspaceDeleteErrMsg != "" {
346+
require.Error(t, err)
347+
require.Contains(t, err.Error(), tc.expectedWorkspaceDeleteErrMsg)
348+
} else {
349+
require.NoError(t, err, "delete the user Workspace")
350+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, build.ID)
351+
352+
// Verify that the user workspace is now marked as deleted
353+
deletedWorkspace, err := client.DeletedWorkspace(ctx, userWorkspace.ID)
354+
require.NoError(t, err)
355+
require.Equal(t, userWorkspace.ID, deletedWorkspace.ID)
356+
}
357+
})
358+
}
359+
})
360+
}
361+
362+
func setupTestDBPreset(
363+
t *testing.T,
364+
db database.Store,
365+
templateVersionID uuid.UUID,
366+
) database.TemplateVersionPreset {
367+
t.Helper()
368+
369+
preset := dbgen.Preset(t, db, database.InsertPresetParams{
370+
TemplateVersionID: templateVersionID,
371+
Name: "preset-test",
372+
DesiredInstances: sql.NullInt32{
373+
Valid: true,
374+
Int32: 1,
375+
},
376+
})
377+
dbgen.PresetParameter(t, db, database.InsertPresetParametersParams{
378+
TemplateVersionPresetID: preset.ID,
379+
Names: []string{"test"},
380+
Values: []string{"test"},
381+
})
382+
383+
return preset
384+
}
385+
386+
func setupTestDBWorkspace(
387+
t *testing.T,
388+
clock quartz.Clock,
389+
db database.Store,
390+
ps pubsub.Pubsub,
391+
orgID uuid.UUID,
392+
ownerID uuid.UUID,
393+
templateID uuid.UUID,
394+
templateVersionID uuid.UUID,
395+
presetID uuid.UUID,
396+
) database.WorkspaceTable {
397+
t.Helper()
398+
399+
workspace := dbgen.Workspace(t, db, database.WorkspaceTable{
400+
TemplateID: templateID,
401+
OrganizationID: orgID,
402+
OwnerID: ownerID,
403+
Deleted: false,
404+
CreatedAt: time.Now().Add(-time.Hour * 2),
405+
})
406+
job := dbgen.ProvisionerJob(t, db, ps, database.ProvisionerJob{
407+
InitiatorID: ownerID,
408+
CreatedAt: time.Now().Add(-time.Hour * 2),
409+
StartedAt: sql.NullTime{Time: clock.Now().Add(-time.Hour * 2), Valid: true},
410+
CompletedAt: sql.NullTime{Time: clock.Now().Add(-time.Hour), Valid: true},
411+
OrganizationID: orgID,
412+
})
413+
workspaceBuild := dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{
414+
WorkspaceID: workspace.ID,
415+
InitiatorID: ownerID,
416+
TemplateVersionID: templateVersionID,
417+
JobID: job.ID,
418+
TemplateVersionPresetID: uuid.NullUUID{UUID: presetID, Valid: true},
419+
Transition: database.WorkspaceTransitionStart,
420+
CreatedAt: clock.Now(),
421+
})
422+
dbgen.WorkspaceBuildParameters(t, db, []database.WorkspaceBuildParameter{
423+
{
424+
WorkspaceBuildID: workspaceBuild.ID,
425+
Name: "test",
426+
Value: "test",
427+
},
428+
})
429+
430+
return workspace
212431
}

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -149,15 +149,15 @@ func (q *querier) authorizeContext(ctx context.Context, action policy.Action, ob
149149
return nil
150150
}
151151

152-
// authorizeWorkspace handles authorization for workspace resource types.
152+
// authorizePrebuiltWorkspace handles authorization for workspace resource types.
153153
// prebuilt_workspaces are a subset of workspaces, currently limited to
154154
// supporting delete operations. Therefore, if the action is delete or
155155
// update and the workspace is a prebuild, a prebuilt-specific authorization
156156
// is attempted first. If that fails, it falls back to normal workspace
157157
// authorization.
158158
// Note: Delete operations of workspaces requires both update and delete
159159
// permissions.
160-
func (q *querier) authorizeWorkspace(ctx context.Context, action policy.Action, workspace database.Workspace) error {
160+
func (q *querier) authorizePrebuiltWorkspace(ctx context.Context, action policy.Action, workspace database.Workspace) error {
161161
var prebuiltErr error
162162
// Special handling for prebuilt_workspace deletion authorization check
163163
if (action == policy.ActionUpdate || action == policy.ActionDelete) && workspace.IsPrebuild() {
@@ -439,7 +439,7 @@ var (
439439
// Explicitly setting PrebuiltWorkspace permissions for clarity.
440440
// Note: even without PrebuiltWorkspace permissions, access is still granted via Workspace permissions.
441441
rbac.ResourcePrebuiltWorkspace.Type: {
442-
policy.ActionRead, policy.ActionUpdate, policy.ActionDelete,
442+
policy.ActionUpdate, policy.ActionDelete,
443443
},
444444
// Should be able to add the prebuilds system user as a member to any organization that needs prebuilds.
445445
rbac.ResourceOrganizationMember.Type: {
@@ -3962,7 +3962,7 @@ func (q *querier) InsertWorkspaceBuild(ctx context.Context, arg database.InsertW
39623962
}
39633963

39643964
// Special handling for prebuilt workspace deletion
3965-
if err := q.authorizeWorkspace(ctx, action, w); err != nil {
3965+
if err := q.authorizePrebuiltWorkspace(ctx, action, w); err != nil {
39663966
return err
39673967
}
39683968

@@ -4003,7 +4003,7 @@ func (q *querier) InsertWorkspaceBuildParameters(ctx context.Context, arg databa
40034003
}
40044004

40054005
// Special handling for prebuilt workspace deletion
4006-
if err := q.authorizeWorkspace(ctx, policy.ActionUpdate, workspace); err != nil {
4006+
if err := q.authorizePrebuiltWorkspace(ctx, policy.ActionUpdate, workspace); err != nil {
40074007
return err
40084008
}
40094009

‎coderd/rbac/object_gen.go

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/rbac/policy/policy.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,13 +104,14 @@ var RBACPermissions = map[string]PermissionDefinition{
104104
},
105105
"prebuilt_workspace": {
106106
// Prebuilt_workspace actions currently apply only to delete operations.
107-
// To successfully delete a workspace, a user must have the following permissions:
108-
// * read: to read the current workspace state
107+
// To successfully delete a prebuilt workspace, a user must have the following permissions:
108+
// * workspace.read: to read the current workspace state
109109
// * update: to modify workspace metadata and related resources during deletion
110110
// (e.g., updating the deleted field in the database)
111111
// * delete: to perform the actual deletion of the workspace
112+
// If the user lacks prebuilt_workspace update or delete permissions,
113+
// the authorization will always fall back to the corresponding permissions on workspace.
112114
Actions: map[Action]ActionDefinition{
113-
ActionRead: actDef("read prebuilt workspace data"),
114115
ActionUpdate: actDef("update prebuilt workspace settings"),
115116
ActionDelete: actDef("delete prebuilt workspace"),
116117
},

‎coderd/rbac/roles.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
278278
// PrebuiltWorkspaces are a subset of Workspaces.
279279
// Explicitly setting PrebuiltWorkspace permissions for clarity.
280280
// Note: even without PrebuiltWorkspace permissions, access is still granted via Workspace permissions.
281-
ResourcePrebuiltWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
281+
ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete},
282282
})...),
283283
Org: map[string][]Permission{},
284284
User: []Permission{},
@@ -341,7 +341,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
341341
// CRUD all files, even those they did not upload.
342342
ResourceFile.Type: {policy.ActionCreate, policy.ActionRead},
343343
ResourceWorkspace.Type: {policy.ActionRead},
344-
ResourcePrebuiltWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
344+
ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete},
345345
// CRUD to provisioner daemons for now.
346346
ResourceProvisionerDaemon.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
347347
// Needs to read all organizations since
@@ -424,7 +424,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
424424
// PrebuiltWorkspaces are a subset of Workspaces.
425425
// Explicitly setting PrebuiltWorkspace permissions for clarity.
426426
// Note: even without PrebuiltWorkspace permissions, access is still granted via Workspace permissions.
427-
ResourcePrebuiltWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
427+
ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete},
428428
})...),
429429
},
430430
User: []Permission{},
@@ -505,7 +505,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
505505
ResourceTemplate.Type: ResourceTemplate.AvailableActions(),
506506
ResourceFile.Type: {policy.ActionCreate, policy.ActionRead},
507507
ResourceWorkspace.Type: {policy.ActionRead},
508-
ResourcePrebuiltWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
508+
ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete},
509509
// Assigning template perms requires this permission.
510510
ResourceOrganization.Type: {policy.ActionRead},
511511
ResourceOrganizationMember.Type: {policy.ActionRead},

‎coderd/rbac/roles_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@ package rbac_test
33
import (
44
"context"
55
"fmt"
6-
"github.com/coder/coder/v2/coderd/database"
76
"testing"
87

8+
"github.com/coder/coder/v2/coderd/database"
9+
910
"github.com/google/uuid"
1011
"github.com/prometheus/client_golang/prometheus"
1112
"github.com/stretchr/testify/assert"
@@ -499,7 +500,7 @@ func TestRolePermissions(t *testing.T) {
499500
},
500501
{
501502
Name: "PrebuiltWorkspace",
502-
Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
503+
Actions: []policy.Action{policy.ActionUpdate, policy.ActionDelete},
503504
Resource: rbac.ResourcePrebuiltWorkspace.WithID(uuid.New()).InOrg(orgID).WithOwner(database.PrebuildsSystemUserID.String()),
504505
AuthorizeMap: map[bool][]hasAuthSubjects{
505506
true: {owner, orgAdmin, templateAdmin, orgTemplateAdmin},

‎coderd/wsbuilder/wsbuilder.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -918,7 +918,18 @@ func (b *Builder) authorize(authFunc func(action policy.Action, object rbac.Obje
918918
msg := fmt.Sprintf("Transition %q not supported.", b.trans)
919919
return BuildError{http.StatusBadRequest, msg, xerrors.New(msg)}
920920
}
921-
if !authFunc(action, b.workspace) {
921+
922+
// Special handling for prebuilt workspace deletion
923+
authorized := false
924+
if action == policy.ActionDelete && b.workspace.IsPrebuild() && authFunc(action, b.workspace.AsPrebuild()) {
925+
authorized = true
926+
}
927+
// Fallback to default authorization
928+
if !authorized && authFunc(action, b.workspace) {
929+
authorized = true
930+
}
931+
932+
if !authorized {
922933
if authFunc(policy.ActionRead, b.workspace) {
923934
// If the user can read the workspace, but not delete/create/update. Show
924935
// a more helpful error. They are allowed to know the workspace exists.

‎codersdk/rbacresources_gen.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)