Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit72f7d70

Browse files
authored
feat: allow TemplateAdmin to delete prebuilds via auth layer (#18333)
## DescriptionThis PR adds support for deleting prebuilt workspaces via theauthorization layer. It introduces special-case handling to ensure that`prebuilt_workspace` permissions are evaluated when attempting to deletea prebuilt workspace, falling back to the standard `workspace` resourceas needed.Prebuilt workspaces are a subset of workspaces, identified by having`owner_id` set to `PREBUILD_SYSTEM_USER`.This means:* A user with `prebuilt_workspace.delete` permission is allowed to**delete only prebuilt workspaces**.* A user with `workspace.delete` permission can **delete both normal andprebuilt workspaces**.⚠️ This implementation is scoped to **deletion operations only**. Noother operations are currently supported for the `prebuilt_workspace`resource.To delete a workspace, users must have the following permissions:* `workspace.read`: to read the current workspace state* `update`: to modify workspace metadata and related resources duringdeletion (e.g., updating the `deleted` field in the database)* `delete`: to perform the actual deletion of the workspace## Changes* Introduced `authorizeWorkspace()` helper to handle prebuilt workspaceauthorization logic.* Ensured both `prebuilt_workspace` and `workspace` permissions arechecked.* Added comments to clarify the current behavior and limitations.* Moved `SystemUserID` constant from the `prebuilds` package to the`database` package `PrebuildsSystemUserID` to resolve an import cycle(commitf24e4ab).* Update middleware `ExtractOrganizationMember` to include system usermembers.
1 parentd61353f commit72f7d70

29 files changed

+493
-63
lines changed

‎cli/delete_test.go

Lines changed: 230 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,225 @@ func TestDelete(t *testing.T) {
209218
cancel()
210219
<-doneChan
211220
})
221+
222+
t.Run("Prebuilt 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+
namestring
249+
client*codersdk.Client
250+
expectedPrebuiltDeleteErrMsgstring
251+
expectedWorkspaceDeleteErrMsgstring
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+
returnclient
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+
returnclient
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+
returnclient
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+
returnclient
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:=rangecases {
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 a 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+
userWorkspaceOwner,err:=client.User(context.Background(),"testUser")
300+
require.NoError(t,err)
301+
dbUserWorkspace:=setupTestDBWorkspace(t,clock,db,pb,orgID,userWorkspaceOwner.ID,template.ID,version.ID,preset.ID)
302+
303+
assertWorkspaceDelete:=func(
304+
runClient*codersdk.Client,
305+
workspace database.Workspace,
306+
workspaceOwnerstring,
307+
expectedErrstring,
308+
) {
309+
t.Helper()
310+
311+
// Attempt to delete the workspace as the test client
312+
inv,root:=clitest.New(t,"delete",workspaceOwner+"/"+workspace.Name,"-y")
313+
clitest.SetupConfig(t,runClient,root)
314+
doneChan:=make(chanstruct{})
315+
pty:=ptytest.New(t).Attach(inv)
316+
varrunErrerror
317+
gofunc() {
318+
deferclose(doneChan)
319+
runErr=inv.Run()
320+
}()
321+
322+
// Validate the result based on the expected error message
323+
ifexpectedErr!="" {
324+
<-doneChan
325+
require.Error(t,runErr)
326+
require.Contains(t,runErr.Error(),expectedErr)
327+
}else {
328+
pty.ExpectMatch("has been deleted")
329+
<-doneChan
330+
331+
// When running with the race detector on, we sometimes get an EOF.
332+
ifrunErr!=nil {
333+
assert.ErrorIs(t,runErr,io.EOF)
334+
}
335+
336+
// Verify that the workspace is now marked as deleted
337+
_,err:=client.Workspace(context.Background(),workspace.ID)
338+
require.ErrorContains(t,err,"was deleted")
339+
}
340+
}
341+
342+
// Ensure at least one prebuilt workspace is reported as running in the database
343+
testutil.Eventually(ctx,t,func(ctx context.Context) (donebool) {
344+
running,err:=db.GetRunningPrebuiltWorkspaces(ctx)
345+
if!assert.NoError(t,err)||!assert.GreaterOrEqual(t,len(running),1) {
346+
returnfalse
347+
}
348+
returntrue
349+
},testutil.IntervalMedium,"running prebuilt workspaces timeout")
350+
351+
runningWorkspaces,err:=db.GetRunningPrebuiltWorkspaces(ctx)
352+
require.NoError(t,err)
353+
require.GreaterOrEqual(t,len(runningWorkspaces),1)
354+
355+
// Get the full prebuilt workspace object from the DB
356+
prebuiltWorkspace,err:=db.GetWorkspaceByID(ctx,dbPrebuiltWorkspace.ID)
357+
require.NoError(t,err)
358+
359+
// Assert the prebuilt workspace deletion
360+
assertWorkspaceDelete(tc.client,prebuiltWorkspace,"prebuilds",tc.expectedPrebuiltDeleteErrMsg)
361+
362+
// Get the full user workspace object from the DB
363+
userWorkspace,err:=db.GetWorkspaceByID(ctx,dbUserWorkspace.ID)
364+
require.NoError(t,err)
365+
366+
// Assert the user workspace deletion
367+
assertWorkspaceDelete(tc.client,userWorkspace,userWorkspaceOwner.Username,tc.expectedWorkspaceDeleteErrMsg)
368+
})
369+
}
370+
})
371+
}
372+
373+
funcsetupTestDBPreset(
374+
t*testing.T,
375+
db database.Store,
376+
templateVersionID uuid.UUID,
377+
) database.TemplateVersionPreset {
378+
t.Helper()
379+
380+
preset:=dbgen.Preset(t,db, database.InsertPresetParams{
381+
TemplateVersionID:templateVersionID,
382+
Name:"preset-test",
383+
DesiredInstances: sql.NullInt32{
384+
Valid:true,
385+
Int32:1,
386+
},
387+
})
388+
dbgen.PresetParameter(t,db, database.InsertPresetParametersParams{
389+
TemplateVersionPresetID:preset.ID,
390+
Names: []string{"test"},
391+
Values: []string{"test"},
392+
})
393+
394+
returnpreset
395+
}
396+
397+
funcsetupTestDBWorkspace(
398+
t*testing.T,
399+
clock quartz.Clock,
400+
db database.Store,
401+
ps pubsub.Pubsub,
402+
orgID uuid.UUID,
403+
ownerID uuid.UUID,
404+
templateID uuid.UUID,
405+
templateVersionID uuid.UUID,
406+
presetID uuid.UUID,
407+
) database.WorkspaceTable {
408+
t.Helper()
409+
410+
workspace:=dbgen.Workspace(t,db, database.WorkspaceTable{
411+
TemplateID:templateID,
412+
OrganizationID:orgID,
413+
OwnerID:ownerID,
414+
Deleted:false,
415+
CreatedAt:time.Now().Add(-time.Hour*2),
416+
})
417+
job:=dbgen.ProvisionerJob(t,db,ps, database.ProvisionerJob{
418+
InitiatorID:ownerID,
419+
CreatedAt:time.Now().Add(-time.Hour*2),
420+
StartedAt: sql.NullTime{Time:clock.Now().Add(-time.Hour*2),Valid:true},
421+
CompletedAt: sql.NullTime{Time:clock.Now().Add(-time.Hour),Valid:true},
422+
OrganizationID:orgID,
423+
})
424+
workspaceBuild:=dbgen.WorkspaceBuild(t,db, database.WorkspaceBuild{
425+
WorkspaceID:workspace.ID,
426+
InitiatorID:ownerID,
427+
TemplateVersionID:templateVersionID,
428+
JobID:job.ID,
429+
TemplateVersionPresetID: uuid.NullUUID{UUID:presetID,Valid:true},
430+
Transition:database.WorkspaceTransitionStart,
431+
CreatedAt:clock.Now(),
432+
})
433+
dbgen.WorkspaceBuildParameters(t,db, []database.WorkspaceBuildParameter{
434+
{
435+
WorkspaceBuildID:workspaceBuild.ID,
436+
Name:"test",
437+
Value:"test",
438+
},
439+
})
440+
441+
returnworkspace
212442
}

‎coderd/apidoc/docs.go

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/constants.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package database
2+
3+
import"github.com/google/uuid"
4+
5+
varPrebuildsSystemUserID=uuid.MustParse("c42fdf75-3097-471c-8c33-fb52454d81c0")

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"github.com/coder/coder/v2/coderd/database/dbtime"
2222
"github.com/coder/coder/v2/coderd/httpapi/httpapiconstraints"
2323
"github.com/coder/coder/v2/coderd/httpmw/loggermw"
24-
"github.com/coder/coder/v2/coderd/prebuilds"
2524
"github.com/coder/coder/v2/coderd/rbac"
2625
"github.com/coder/coder/v2/coderd/rbac/policy"
2726
"github.com/coder/coder/v2/coderd/rbac/rolestore"
@@ -150,6 +149,30 @@ func (q *querier) authorizeContext(ctx context.Context, action policy.Action, ob
150149
returnnil
151150
}
152151

152+
// authorizePrebuiltWorkspace handles authorization for workspace resource types.
153+
// prebuilt_workspaces are a subset of workspaces, currently limited to
154+
// supporting delete operations. Therefore, if the action is delete or
155+
// update and the workspace is a prebuild, a prebuilt-specific authorization
156+
// is attempted first. If that fails, it falls back to normal workspace
157+
// authorization.
158+
// Note: Delete operations of workspaces requires both update and delete
159+
// permissions.
160+
func (q*querier)authorizePrebuiltWorkspace(ctx context.Context,action policy.Action,workspace database.Workspace)error {
161+
varprebuiltErrerror
162+
// Special handling for prebuilt_workspace deletion authorization check
163+
if (action==policy.ActionUpdate||action==policy.ActionDelete)&&workspace.IsPrebuild() {
164+
// Try prebuilt-specific authorization first
165+
ifprebuiltErr=q.authorizeContext(ctx,action,workspace.AsPrebuild());prebuiltErr==nil {
166+
returnnil
167+
}
168+
}
169+
// Fallback to normal workspace authorization check
170+
iferr:=q.authorizeContext(ctx,action,workspace);err!=nil {
171+
returnxerrors.Errorf("authorize context: %w",errors.Join(prebuiltErr,err))
172+
}
173+
returnnil
174+
}
175+
153176
typeauthContextKeystruct{}
154177

155178
// ActorFromContext returns the authorization subject from the context.
@@ -399,7 +422,7 @@ var (
399422
subjectPrebuildsOrchestrator= rbac.Subject{
400423
Type:rbac.SubjectTypePrebuildsOrchestrator,
401424
FriendlyName:"Prebuilds Orchestrator",
402-
ID:prebuilds.SystemUserID.String(),
425+
ID:database.PrebuildsSystemUserID.String(),
403426
Roles:rbac.Roles([]rbac.Role{
404427
{
405428
Identifier: rbac.RoleIdentifier{Name:"prebuilds-orchestrator"},
@@ -412,6 +435,12 @@ var (
412435
policy.ActionCreate,policy.ActionDelete,policy.ActionRead,policy.ActionUpdate,
413436
policy.ActionWorkspaceStart,policy.ActionWorkspaceStop,
414437
},
438+
// PrebuiltWorkspaces are a subset of Workspaces.
439+
// Explicitly setting PrebuiltWorkspace permissions for clarity.
440+
// Note: even without PrebuiltWorkspace permissions, access is still granted via Workspace permissions.
441+
rbac.ResourcePrebuiltWorkspace.Type: {
442+
policy.ActionUpdate,policy.ActionDelete,
443+
},
415444
// Should be able to add the prebuilds system user as a member to any organization that needs prebuilds.
416445
rbac.ResourceOrganizationMember.Type: {
417446
policy.ActionCreate,
@@ -3953,8 +3982,9 @@ func (q *querier) InsertWorkspaceBuild(ctx context.Context, arg database.InsertW
39533982
action=policy.ActionWorkspaceStop
39543983
}
39553984

3956-
iferr=q.authorizeContext(ctx,action,w);err!=nil {
3957-
returnxerrors.Errorf("authorize context: %w",err)
3985+
// Special handling for prebuilt workspace deletion
3986+
iferr:=q.authorizePrebuiltWorkspace(ctx,action,w);err!=nil {
3987+
returnerr
39583988
}
39593989

39603990
// If we're starting a workspace we need to check the template.
@@ -3993,8 +4023,8 @@ func (q *querier) InsertWorkspaceBuildParameters(ctx context.Context, arg databa
39934023
returnerr
39944024
}
39954025

3996-
err=q.authorizeContext(ctx,policy.ActionUpdate,workspace)
3997-
iferr!=nil {
4026+
// Special handling for prebuiltworkspace deletion
4027+
iferr:=q.authorizePrebuiltWorkspace(ctx,policy.ActionUpdate,workspace);err!=nil {
39984028
returnerr
39994029
}
40004030

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp