From 2ba15c516126482aedf9ce1ad87ce757e39adb61 Mon Sep 17 00:00:00 2001 From: Susana Ferreira Date: Wed, 11 Jun 2025 19:10:55 +0000 Subject: [PATCH 1/9] feat: POC for allowing TemplateAdmin to delete prebuild workspaces via auth layer --- coderd/apidoc/docs.go | 2 + coderd/apidoc/swagger.json | 2 + coderd/database/dbauthz/dbauthz.go | 92 +++++++++------ coderd/database/modelmethods.go | 17 +++ coderd/rbac/object_gen.go | 10 ++ coderd/rbac/policy/policy.go | 7 ++ coderd/rbac/roles.go | 12 +- coderd/workspacebuilds.go | 10 ++ codersdk/rbacresources_gen.go | 2 + docs/reference/api/members.md | 5 + docs/reference/api/schemas.md | 1 + enterprise/coderd/prebuilds/claim_test.go | 24 ++-- enterprise/coderd/prebuilds/reconcile_test.go | 108 ++++++++++++++++++ site/src/api/rbacresourcesGenerated.ts | 5 + site/src/api/typesGenerated.ts | 2 + 15 files changed, 245 insertions(+), 54 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index d11a0635d6f52..e6b33957222d8 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -15259,6 +15259,7 @@ const docTemplate = `{ "oauth2_app_secret", "organization", "organization_member", + "prebuilt_workspace", "provisioner_daemon", "provisioner_jobs", "replicas", @@ -15298,6 +15299,7 @@ const docTemplate = `{ "ResourceOauth2AppSecret", "ResourceOrganization", "ResourceOrganizationMember", + "ResourcePrebuiltWorkspace", "ResourceProvisionerDaemon", "ResourceProvisionerJobs", "ResourceReplicas", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index aabe0b9b12672..c7c9c9873cf00 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -13851,6 +13851,7 @@ "oauth2_app_secret", "organization", "organization_member", + "prebuilt_workspace", "provisioner_daemon", "provisioner_jobs", "replicas", @@ -13890,6 +13891,7 @@ "ResourceOauth2AppSecret", "ResourceOrganization", "ResourceOrganizationMember", + "ResourcePrebuiltWorkspace", "ResourceProvisionerDaemon", "ResourceProvisionerJobs", "ResourceReplicas", diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 5bfa015af3d78..2fefa396fec6d 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -412,6 +412,9 @@ var ( policy.ActionCreate, policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop, }, + rbac.ResourcePrebuiltWorkspace.Type: { + policy.ActionRead, policy.ActionUpdate, policy.ActionDelete, + }, // Should be able to add the prebuilds system user as a member to any organization that needs prebuilds. rbac.ResourceOrganizationMember.Type: { policy.ActionCreate, @@ -527,9 +530,9 @@ func As(ctx context.Context, actor rbac.Subject) context.Context { // running the insertFunc. The insertFunc is expected to return the object that // was inserted. func insert[ - ObjectType any, - ArgumentType any, - Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), +ObjectType any, +ArgumentType any, +Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -540,9 +543,9 @@ func insert[ } func insertWithAction[ - ObjectType any, - ArgumentType any, - Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), +ObjectType any, +ArgumentType any, +Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -569,10 +572,10 @@ func insertWithAction[ } func deleteQ[ - ObjectType rbac.Objecter, - ArgumentType any, - Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), - Delete func(ctx context.Context, arg ArgumentType) error, +ObjectType rbac.Objecter, +ArgumentType any, +Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), +Delete func(ctx context.Context, arg ArgumentType) error, ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -584,10 +587,10 @@ func deleteQ[ } func updateWithReturn[ - ObjectType rbac.Objecter, - ArgumentType any, - Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), - UpdateQuery func(ctx context.Context, arg ArgumentType) (ObjectType, error), +ObjectType rbac.Objecter, +ArgumentType any, +Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), +UpdateQuery func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -598,10 +601,10 @@ func updateWithReturn[ } func update[ - ObjectType rbac.Objecter, - ArgumentType any, - Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), - Exec func(ctx context.Context, arg ArgumentType) error, +ObjectType rbac.Objecter, +ArgumentType any, +Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), +Exec func(ctx context.Context, arg ArgumentType) error, ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -619,9 +622,9 @@ func update[ // user cannot read the resource. This is because the resource details are // required to run a proper authorization check. func fetchWithAction[ - ArgumentType any, - ObjectType rbac.Objecter, - DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), +ArgumentType any, +ObjectType rbac.Objecter, +DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -652,9 +655,9 @@ func fetchWithAction[ } func fetch[ - ArgumentType any, - ObjectType rbac.Objecter, - DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), +ArgumentType any, +ObjectType rbac.Objecter, +DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -667,10 +670,10 @@ func fetch[ // from SQL 'exec' functions which only return an error. // See fetchAndQuery for more information. func fetchAndExec[ - ObjectType rbac.Objecter, - ArgumentType any, - Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), - Exec func(ctx context.Context, arg ArgumentType) error, +ObjectType rbac.Objecter, +ArgumentType any, +Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), +Exec func(ctx context.Context, arg ArgumentType) error, ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -693,10 +696,10 @@ func fetchAndExec[ // **before** the query runs. The returns from the fetch are only used to // assert rbac. The final return of this function comes from the Query function. func fetchAndQuery[ - ObjectType rbac.Objecter, - ArgumentType any, - Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), - Query func(ctx context.Context, arg ArgumentType) (ObjectType, error), +ObjectType rbac.Objecter, +ArgumentType any, +Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), +Query func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -730,9 +733,9 @@ func fetchAndQuery[ // fetchWithPostFilter is like fetch, but works with lists of objects. // SQL filters are much more optimal. func fetchWithPostFilter[ - ArgumentType any, - ObjectType rbac.Objecter, - DatabaseFunc func(ctx context.Context, arg ArgumentType) ([]ObjectType, error), +ArgumentType any, +ObjectType rbac.Objecter, +DatabaseFunc func(ctx context.Context, arg ArgumentType) ([]ObjectType, error), ]( authorizer rbac.Authorizer, action policy.Action, @@ -3909,7 +3912,14 @@ func (q *querier) InsertWorkspaceBuild(ctx context.Context, arg database.InsertW action = policy.ActionWorkspaceStop } - if err = q.authorizeContext(ctx, action, w); err != nil { + if action == policy.ActionDelete && w.IsPrebuild() { + if err := q.authorizeContext(ctx, action, w.PrebuildRBAC()); err != nil { + // Fallback to normal workspace auth check + if err = q.authorizeContext(ctx, action, w); err != nil { + return xerrors.Errorf("authorize context: %w", err) + } + } + } else if err = q.authorizeContext(ctx, action, w); err != nil { return xerrors.Errorf("authorize context: %w", err) } @@ -3949,7 +3959,15 @@ func (q *querier) InsertWorkspaceBuildParameters(ctx context.Context, arg databa return err } - err = q.authorizeContext(ctx, policy.ActionUpdate, workspace) + if workspace.IsPrebuild() { + err = q.authorizeContext(ctx, policy.ActionUpdate, workspace.PrebuildRBAC()) + // Fallback to normal workspace auth check + if err != nil { + err = q.authorizeContext(ctx, policy.ActionUpdate, workspace) + } + } else { + err = q.authorizeContext(ctx, policy.ActionUpdate, workspace) + } if err != nil { return err } diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index b3f6deed9eff0..b25d7a438cbe2 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -226,9 +226,26 @@ func (w Workspace) WorkspaceTable() WorkspaceTable { } func (w Workspace) RBACObject() rbac.Object { + // if w.IsPrebuild() { + // return w.PrebuildRBAC() + //} return w.WorkspaceTable().RBACObject() } +func (w Workspace) IsPrebuild() bool { + // TODO: avoid import cycle + return w.OwnerID == uuid.MustParse("c42fdf75-3097-471c-8c33-fb52454d81c0") +} + +func (w Workspace) PrebuildRBAC() rbac.Object { + if w.IsPrebuild() { + return rbac.ResourcePrebuiltWorkspace.WithID(w.ID). + InOrg(w.OrganizationID). + WithOwner(w.OwnerID.String()) + } + return w.RBACObject() +} + func (w WorkspaceTable) RBACObject() rbac.Object { if w.DormantAt.Valid { return w.DormantRBAC() diff --git a/coderd/rbac/object_gen.go b/coderd/rbac/object_gen.go index f19d90894dd55..3361e41d29922 100644 --- a/coderd/rbac/object_gen.go +++ b/coderd/rbac/object_gen.go @@ -222,6 +222,15 @@ var ( Type: "organization_member", } + // ResourcePrebuiltWorkspace + // Valid Actions + // - "ActionDelete" :: delete prebuilt workspace + // - "ActionRead" :: read prebuilt workspace + // - "ActionUpdate" :: update prebuilt workspace + ResourcePrebuiltWorkspace = Object{ + Type: "prebuilt_workspace", + } + // ResourceProvisionerDaemon // Valid Actions // - "ActionCreate" :: create a provisioner daemon/key @@ -389,6 +398,7 @@ func AllResources() []Objecter { ResourceOauth2AppSecret, ResourceOrganization, ResourceOrganizationMember, + ResourcePrebuiltWorkspace, ResourceProvisionerDaemon, ResourceProvisionerJobs, ResourceReplicas, diff --git a/coderd/rbac/policy/policy.go b/coderd/rbac/policy/policy.go index 160062283f857..c7f0d4befbe43 100644 --- a/coderd/rbac/policy/policy.go +++ b/coderd/rbac/policy/policy.go @@ -102,6 +102,13 @@ var RBACPermissions = map[string]PermissionDefinition{ "workspace_dormant": { Actions: workspaceActions, }, + "prebuilt_workspace": { + Actions: map[Action]ActionDefinition{ + ActionRead: actDef("read prebuilt workspace"), + ActionUpdate: actDef("update prebuilt workspace"), + ActionDelete: actDef("delete prebuilt workspace"), + }, + }, "workspace_proxy": { Actions: map[Action]ActionDefinition{ ActionCreate: actDef("create a workspace proxy"), diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 28ddc38462ce9..439a2ac12eb06 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -335,8 +335,9 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceAssignOrgRole.Type: {policy.ActionRead}, ResourceTemplate.Type: ResourceTemplate.AvailableActions(), // CRUD all files, even those they did not upload. - ResourceFile.Type: {policy.ActionCreate, policy.ActionRead}, - ResourceWorkspace.Type: {policy.ActionRead}, + ResourceFile.Type: {policy.ActionCreate, policy.ActionRead}, + ResourceWorkspace.Type: {policy.ActionRead}, + ResourcePrebuiltWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, // CRUD to provisioner daemons for now. ResourceProvisionerDaemon.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, // Needs to read all organizations since @@ -493,9 +494,10 @@ func ReloadBuiltinRoles(opts *RoleOptions) { 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}, + ResourceTemplate.Type: ResourceTemplate.AvailableActions(), + ResourceFile.Type: {policy.ActionCreate, policy.ActionRead}, + ResourceWorkspace.Type: {policy.ActionRead}, + ResourcePrebuiltWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, // Assigning template perms requires this permission. ResourceOrganization.Type: {policy.ActionRead}, ResourceOrganizationMember.Type: {policy.ActionRead}, diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index c01004653f86e..da6da04ad0afc 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -404,6 +404,16 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { ctx, tx, func(action policy.Action, object rbac.Objecter) bool { + if object.RBACObject().Type == rbac.ResourceWorkspace.Type && action == policy.ActionDelete { + workspaceObj, ok := object.(database.Workspace) + if ok { + prebuild := workspaceObj.PrebuildRBAC() + // Fallback to normal workspace auth check + if auth := api.Authorize(r, action, prebuild); auth { + return auth + } + } + } return api.Authorize(r, action, object) }, audit.WorkspaceBuildBaggageFromRequest(r), diff --git a/codersdk/rbacresources_gen.go b/codersdk/rbacresources_gen.go index 95792bb8e2a7b..b3f3db99eae21 100644 --- a/codersdk/rbacresources_gen.go +++ b/codersdk/rbacresources_gen.go @@ -28,6 +28,7 @@ const ( ResourceOauth2AppSecret RBACResource = "oauth2_app_secret" ResourceOrganization RBACResource = "organization" ResourceOrganizationMember RBACResource = "organization_member" + ResourcePrebuiltWorkspace RBACResource = "prebuilt_workspace" ResourceProvisionerDaemon RBACResource = "provisioner_daemon" ResourceProvisionerJobs RBACResource = "provisioner_jobs" ResourceReplicas RBACResource = "replicas" @@ -91,6 +92,7 @@ var RBACResourceActions = map[RBACResource][]RBACAction{ ResourceOauth2AppSecret: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceOrganization: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceOrganizationMember: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourcePrebuiltWorkspace: {ActionDelete, ActionRead, ActionUpdate}, ResourceProvisionerDaemon: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceProvisionerJobs: {ActionCreate, ActionRead, ActionUpdate}, ResourceReplicas: {ActionRead}, diff --git a/docs/reference/api/members.md b/docs/reference/api/members.md index 6b5d124753bc0..40921e40b70ee 100644 --- a/docs/reference/api/members.md +++ b/docs/reference/api/members.md @@ -206,6 +206,7 @@ Status Code **200** | `resource_type` | `oauth2_app_secret` | | `resource_type` | `organization` | | `resource_type` | `organization_member` | +| `resource_type` | `prebuilt_workspace` | | `resource_type` | `provisioner_daemon` | | `resource_type` | `provisioner_jobs` | | `resource_type` | `replicas` | @@ -375,6 +376,7 @@ Status Code **200** | `resource_type` | `oauth2_app_secret` | | `resource_type` | `organization` | | `resource_type` | `organization_member` | +| `resource_type` | `prebuilt_workspace` | | `resource_type` | `provisioner_daemon` | | `resource_type` | `provisioner_jobs` | | `resource_type` | `replicas` | @@ -544,6 +546,7 @@ Status Code **200** | `resource_type` | `oauth2_app_secret` | | `resource_type` | `organization` | | `resource_type` | `organization_member` | +| `resource_type` | `prebuilt_workspace` | | `resource_type` | `provisioner_daemon` | | `resource_type` | `provisioner_jobs` | | `resource_type` | `replicas` | @@ -682,6 +685,7 @@ Status Code **200** | `resource_type` | `oauth2_app_secret` | | `resource_type` | `organization` | | `resource_type` | `organization_member` | +| `resource_type` | `prebuilt_workspace` | | `resource_type` | `provisioner_daemon` | | `resource_type` | `provisioner_jobs` | | `resource_type` | `replicas` | @@ -1042,6 +1046,7 @@ Status Code **200** | `resource_type` | `oauth2_app_secret` | | `resource_type` | `organization` | | `resource_type` | `organization_member` | +| `resource_type` | `prebuilt_workspace` | | `resource_type` | `provisioner_daemon` | | `resource_type` | `provisioner_jobs` | | `resource_type` | `replicas` | diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 4191ab8970e92..a2f718b983913 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -6327,6 +6327,7 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith | `oauth2_app_secret` | | `organization` | | `organization_member` | +| `prebuilt_workspace` | | `provisioner_daemon` | | `provisioner_jobs` | | `replicas` | diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index 83933f3a98cd3..fba9043187d89 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -415,18 +415,18 @@ func templateWithAgentAndPresetsWithPrebuilds(desiredInstances int32) *echo.Resp Instances: desiredInstances, }, }, - { - Name: "preset-b", - Parameters: []*proto.PresetParameter{ - { - Name: "k1", - Value: "v2", - }, - }, - Prebuild: &proto.Prebuild{ - Instances: desiredInstances, - }, - }, + //{ + // Name: "preset-b", + // Parameters: []*proto.PresetParameter{ + // { + // Name: "k1", + // Value: "v2", + // }, + // }, + // Prebuild: &proto.Prebuild{ + // Instances: desiredInstances, + // }, + // }, }, }, }, diff --git a/enterprise/coderd/prebuilds/reconcile_test.go b/enterprise/coderd/prebuilds/reconcile_test.go index d2827999ba843..5db0870fec34b 100644 --- a/enterprise/coderd/prebuilds/reconcile_test.go +++ b/enterprise/coderd/prebuilds/reconcile_test.go @@ -8,6 +8,12 @@ import ( "testing" "time" + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" + "github.com/coder/coder/v2/enterprise/coderd/license" + "github.com/coder/coder/v2/provisionersdk" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" "golang.org/x/xerrors" @@ -420,6 +426,108 @@ func TestPrebuildReconciliation(t *testing.T) { } } +func TestTemplateAdminDelete(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres") + } + + t.Run("template admin delete prebuilds", func(t *testing.T) { + t.Parallel() + + clock := quartz.NewMock(t) + + // Setup. + ctx := testutil.Context(t, testutil.WaitSuperLong) + db, pubsub := dbtestutil.NewDB(t) + + spy := newStoreSpy(db, nil) + + logger := testutil.Logger(t) + client, _, api, owner := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + Database: spy, + Pubsub: pubsub, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureExternalProvisionerDaemons: 1, + }, + }, + + EntitlementsUpdateInterval: time.Second, + }) + + orgID := owner.OrganizationID + + provisionerCloser := coderdenttest.NewExternalProvisionerDaemon(t, client, orgID, map[string]string{ + provisionersdk.TagScope: provisionersdk.ScopeOrganization, + }) + defer provisionerCloser.Close() + + reconciler := prebuilds.NewStoreReconciler(spy, pubsub, codersdk.PrebuildsConfig{}, logger, clock, prometheus.NewRegistry(), newNoopEnqueuer()) + var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(spy) + api.AGPL.PrebuildsClaimer.Store(&claimer) + + version := coderdtest.CreateTemplateVersion(t, client, orgID, templateWithAgentAndPresetsWithPrebuilds(2)) + _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, orgID, version.ID) + presets, err := client.TemplateVersionPresets(ctx, version.ID) + require.NoError(t, err) + require.Len(t, presets, 1) + preset := setupTestDBPreset(t, db, version.ID, 2, "b0rked") + + templateAdminClient, _ := coderdtest.CreateAnotherUser(t, client, orgID, rbac.RoleTemplateAdmin()) + + state, err := reconciler.SnapshotState(ctx, spy) + require.NoError(t, err) + require.Len(t, state.Presets, 2) + + for _, preset := range presets { + ps, err := state.FilterByPreset(preset.ID) + require.NoError(t, err) + require.NotNil(t, ps) + actions, err := reconciler.CalculateActions(ctx, *ps) + require.NoError(t, err) + require.NotNil(t, actions) + + require.NoError(t, reconciler.ReconcilePreset(ctx, *ps)) + } + + workspace, _ := setupTestDBPrebuild( + t, + clock, + db, + pubsub, + database.WorkspaceTransitionStart, + database.ProvisionerJobStatusSucceeded, + orgID, + preset, + template.ID, + version.ID, + ) + + require.NoError(t, reconciler.ReconcileAll(ctx)) + + runningWorkspaces, err := db.GetRunningPrebuiltWorkspaces(ctx) + require.NoError(t, err) + + prebuiltWorkspace, err := db.GetWorkspaceByID(ctx, runningWorkspaces[0].ID) + require.NoError(t, err) + + build, err := templateAdminClient.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + Transition: codersdk.WorkspaceTransitionDelete, + }) + require.NoError(t, err, "delete the workspace") + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, build.ID) + + workspaceNew, err := client.DeletedWorkspace(ctx, prebuiltWorkspace.ID) + require.NoError(t, err) + require.Equal(t, prebuiltWorkspace.ID, workspaceNew.ID) + }) +} + // brokenPublisher is used to validate that Publish() calls which always fail do not affect the reconciler's behavior, // since the messages published are not essential but merely advisory. type brokenPublisher struct { diff --git a/site/src/api/rbacresourcesGenerated.ts b/site/src/api/rbacresourcesGenerated.ts index 885f603c1eb82..7edb47f8e8eff 100644 --- a/site/src/api/rbacresourcesGenerated.ts +++ b/site/src/api/rbacresourcesGenerated.ts @@ -123,6 +123,11 @@ export const RBACResourceActions: Partial< read: "read member", update: "update an organization member", }, + prebuilt_workspace: { + delete: "delete prebuilt workspace", + read: "read prebuilt workspace", + update: "update prebuilt workspace", + }, provisioner_daemon: { create: "create a provisioner daemon/key", delete: "delete a provisioner daemon/key", diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index c662b27386401..cb0ccdf784f02 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -2192,6 +2192,7 @@ export type RBACResource = | "oauth2_app_secret" | "organization" | "organization_member" + | "prebuilt_workspace" | "provisioner_daemon" | "provisioner_jobs" | "replicas" @@ -2231,6 +2232,7 @@ export const RBACResources: RBACResource[] = [ "oauth2_app_secret", "organization", "organization_member", + "prebuilt_workspace", "provisioner_daemon", "provisioner_jobs", "replicas", From a043f92d8230ed97148292ed796218f0461d6349 Mon Sep 17 00:00:00 2001 From: Susana Ferreira Date: Thu, 12 Jun 2025 10:54:51 +0000 Subject: [PATCH 2/9] fix: role permissions tests --- coderd/database/dbauthz/dbauthz.go | 70 +++++++++++++++--------------- coderd/rbac/roles_test.go | 9 ++++ 2 files changed, 44 insertions(+), 35 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 2fefa396fec6d..949dae873494e 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -530,9 +530,9 @@ func As(ctx context.Context, actor rbac.Subject) context.Context { // running the insertFunc. The insertFunc is expected to return the object that // was inserted. func insert[ -ObjectType any, -ArgumentType any, -Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), + ObjectType any, + ArgumentType any, + Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -543,9 +543,9 @@ Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), } func insertWithAction[ -ObjectType any, -ArgumentType any, -Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), + ObjectType any, + ArgumentType any, + Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -572,10 +572,10 @@ Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error), } func deleteQ[ -ObjectType rbac.Objecter, -ArgumentType any, -Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), -Delete func(ctx context.Context, arg ArgumentType) error, + ObjectType rbac.Objecter, + ArgumentType any, + Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), + Delete func(ctx context.Context, arg ArgumentType) error, ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -587,10 +587,10 @@ Delete func(ctx context.Context, arg ArgumentType) error, } func updateWithReturn[ -ObjectType rbac.Objecter, -ArgumentType any, -Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), -UpdateQuery func(ctx context.Context, arg ArgumentType) (ObjectType, error), + ObjectType rbac.Objecter, + ArgumentType any, + Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), + UpdateQuery func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -601,10 +601,10 @@ UpdateQuery func(ctx context.Context, arg ArgumentType) (ObjectType, error), } func update[ -ObjectType rbac.Objecter, -ArgumentType any, -Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), -Exec func(ctx context.Context, arg ArgumentType) error, + ObjectType rbac.Objecter, + ArgumentType any, + Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), + Exec func(ctx context.Context, arg ArgumentType) error, ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -622,9 +622,9 @@ Exec func(ctx context.Context, arg ArgumentType) error, // user cannot read the resource. This is because the resource details are // required to run a proper authorization check. func fetchWithAction[ -ArgumentType any, -ObjectType rbac.Objecter, -DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), + ArgumentType any, + ObjectType rbac.Objecter, + DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -655,9 +655,9 @@ DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), } func fetch[ -ArgumentType any, -ObjectType rbac.Objecter, -DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), + ArgumentType any, + ObjectType rbac.Objecter, + DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -670,10 +670,10 @@ DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error), // from SQL 'exec' functions which only return an error. // See fetchAndQuery for more information. func fetchAndExec[ -ObjectType rbac.Objecter, -ArgumentType any, -Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), -Exec func(ctx context.Context, arg ArgumentType) error, + ObjectType rbac.Objecter, + ArgumentType any, + Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), + Exec func(ctx context.Context, arg ArgumentType) error, ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -696,10 +696,10 @@ Exec func(ctx context.Context, arg ArgumentType) error, // **before** the query runs. The returns from the fetch are only used to // assert rbac. The final return of this function comes from the Query function. func fetchAndQuery[ -ObjectType rbac.Objecter, -ArgumentType any, -Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), -Query func(ctx context.Context, arg ArgumentType) (ObjectType, error), + ObjectType rbac.Objecter, + ArgumentType any, + Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error), + Query func(ctx context.Context, arg ArgumentType) (ObjectType, error), ]( logger slog.Logger, authorizer rbac.Authorizer, @@ -733,9 +733,9 @@ Query func(ctx context.Context, arg ArgumentType) (ObjectType, error), // fetchWithPostFilter is like fetch, but works with lists of objects. // SQL filters are much more optimal. func fetchWithPostFilter[ -ArgumentType any, -ObjectType rbac.Objecter, -DatabaseFunc func(ctx context.Context, arg ArgumentType) ([]ObjectType, error), + ArgumentType any, + ObjectType rbac.Objecter, + DatabaseFunc func(ctx context.Context, arg ArgumentType) ([]ObjectType, error), ]( authorizer rbac.Authorizer, action policy.Action, diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index 5738edfe8caa2..7bc132a362b21 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -496,6 +496,15 @@ func TestRolePermissions(t *testing.T) { false: {setOtherOrg, userAdmin, templateAdmin, memberMe, orgTemplateAdmin, orgUserAdmin, orgAuditor}, }, }, + { + Name: "PrebuiltWorkspace", + Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, + Resource: rbac.ResourcePrebuiltWorkspace.WithID(uuid.New()).InOrg(orgID).WithOwner(memberMe.Actor.ID), + AuthorizeMap: map[bool][]hasAuthSubjects{ + true: {owner, orgAdmin, orgMemberMe, templateAdmin, orgTemplateAdmin}, + false: {setOtherOrg, userAdmin, memberMe, orgUserAdmin, orgAuditor}, + }, + }, // Some admin style resources { Name: "Licenses", From 6cae7699273a56ff12cb2bdbde56539d76804adc Mon Sep 17 00:00:00 2001 From: Susana Ferreira Date: Thu, 12 Jun 2025 11:11:41 +0000 Subject: [PATCH 3/9] fix: exclude prebuiltWorkspace permissions from orgAdmin role --- coderd/rbac/roles.go | 2 +- coderd/rbac/roles_test.go | 4 ++-- enterprise/coderd/prebuilds/claim_test.go | 24 +++++++++---------- enterprise/coderd/prebuilds/reconcile_test.go | 17 +------------ 4 files changed, 16 insertions(+), 31 deletions(-) diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 439a2ac12eb06..6bc7ccfd3aa9d 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -414,7 +414,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { }), Org: map[string][]Permission{ // Org admins should not have workspace exec perms. - organizationID.String(): append(allPermsExcept(ResourceWorkspace, ResourceWorkspaceDormant, ResourceAssignRole), Permissions(map[string][]policy.Action{ + organizationID.String(): append(allPermsExcept(ResourceWorkspace, ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceAssignRole), 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), })...), diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index 7bc132a362b21..e497d74948d48 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -501,8 +501,8 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, Resource: rbac.ResourcePrebuiltWorkspace.WithID(uuid.New()).InOrg(orgID).WithOwner(memberMe.Actor.ID), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgAdmin, orgMemberMe, templateAdmin, orgTemplateAdmin}, - false: {setOtherOrg, userAdmin, memberMe, orgUserAdmin, orgAuditor}, + true: {owner, orgMemberMe, templateAdmin, orgTemplateAdmin}, + false: {setOtherOrg, userAdmin, memberMe, orgAdmin, orgUserAdmin, orgAuditor}, }, }, // Some admin style resources diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index fba9043187d89..83933f3a98cd3 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -415,18 +415,18 @@ func templateWithAgentAndPresetsWithPrebuilds(desiredInstances int32) *echo.Resp Instances: desiredInstances, }, }, - //{ - // Name: "preset-b", - // Parameters: []*proto.PresetParameter{ - // { - // Name: "k1", - // Value: "v2", - // }, - // }, - // Prebuild: &proto.Prebuild{ - // Instances: desiredInstances, - // }, - // }, + { + Name: "preset-b", + Parameters: []*proto.PresetParameter{ + { + Name: "k1", + Value: "v2", + }, + }, + Prebuild: &proto.Prebuild{ + Instances: desiredInstances, + }, + }, }, }, }, diff --git a/enterprise/coderd/prebuilds/reconcile_test.go b/enterprise/coderd/prebuilds/reconcile_test.go index 5db0870fec34b..92f903357a1d3 100644 --- a/enterprise/coderd/prebuilds/reconcile_test.go +++ b/enterprise/coderd/prebuilds/reconcile_test.go @@ -475,26 +475,11 @@ func TestTemplateAdminDelete(t *testing.T) { template := coderdtest.CreateTemplate(t, client, orgID, version.ID) presets, err := client.TemplateVersionPresets(ctx, version.ID) require.NoError(t, err) - require.Len(t, presets, 1) + require.Len(t, presets, 2) preset := setupTestDBPreset(t, db, version.ID, 2, "b0rked") templateAdminClient, _ := coderdtest.CreateAnotherUser(t, client, orgID, rbac.RoleTemplateAdmin()) - state, err := reconciler.SnapshotState(ctx, spy) - require.NoError(t, err) - require.Len(t, state.Presets, 2) - - for _, preset := range presets { - ps, err := state.FilterByPreset(preset.ID) - require.NoError(t, err) - require.NotNil(t, ps) - actions, err := reconciler.CalculateActions(ctx, *ps) - require.NoError(t, err) - require.NotNil(t, actions) - - require.NoError(t, reconciler.ReconcilePreset(ctx, *ps)) - } - workspace, _ := setupTestDBPrebuild( t, clock, From afc5359c41ff95b3af207586a2d4d67463275b2b Mon Sep 17 00:00:00 2001 From: Susana Ferreira Date: Thu, 12 Jun 2025 12:33:10 +0000 Subject: [PATCH 4/9] fix: explicitly set prebuild_workspace permissions --- coderd/database/dbauthz/dbauthz.go | 3 +++ coderd/rbac/roles.go | 12 ++++++++++-- coderd/rbac/roles_test.go | 4 ++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 949dae873494e..b048338045059 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -412,6 +412,9 @@ var ( policy.ActionCreate, policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop, }, + // PrebuiltWorkspaces are a subset of Workspaces. + // Explicitly setting PrebuiltWorkspace permissions for clarity. + // Note: even without PrebuiltWorkspace permissions, access is still granted via Workspace permissions. rbac.ResourcePrebuiltWorkspace.Type: { policy.ActionRead, policy.ActionUpdate, policy.ActionDelete, }, diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 6bc7ccfd3aa9d..7623e1dc3af21 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -270,11 +270,15 @@ func ReloadBuiltinRoles(opts *RoleOptions) { Site: append( // Workspace dormancy and workspace are omitted. // Workspace is specifically handled based on the opts.NoOwnerWorkspaceExec - allPermsExcept(ResourceWorkspaceDormant, ResourceWorkspace), + allPermsExcept(ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceWorkspace), // This adds back in the Workspace permissions. Permissions(map[string][]policy.Action{ ResourceWorkspace.Type: ownerWorkspaceActions, ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate, policy.ActionWorkspaceStop, policy.ActionCreateAgent, policy.ActionDeleteAgent}, + // 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.ActionRead, policy.ActionUpdate, policy.ActionDelete}, })...), Org: map[string][]Permission{}, User: []Permission{}, @@ -290,7 +294,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceWorkspaceProxy.Type: {policy.ActionRead}, }), Org: map[string][]Permission{}, - User: append(allPermsExcept(ResourceWorkspaceDormant, ResourceUser, ResourceOrganizationMember), + User: append(allPermsExcept(ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceUser, ResourceOrganizationMember), Permissions(map[string][]policy.Action{ // Reduced permission set on dormant workspaces. No build, ssh, or exec ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate, policy.ActionWorkspaceStop, policy.ActionCreateAgent, policy.ActionDeleteAgent}, @@ -417,6 +421,10 @@ func ReloadBuiltinRoles(opts *RoleOptions) { organizationID.String(): append(allPermsExcept(ResourceWorkspace, ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceAssignRole), 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.ActionRead, policy.ActionUpdate, policy.ActionDelete}, })...), }, User: []Permission{}, diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index e497d74948d48..4e0116a5614d4 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -501,8 +501,8 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, Resource: rbac.ResourcePrebuiltWorkspace.WithID(uuid.New()).InOrg(orgID).WithOwner(memberMe.Actor.ID), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgMemberMe, templateAdmin, orgTemplateAdmin}, - false: {setOtherOrg, userAdmin, memberMe, orgAdmin, orgUserAdmin, orgAuditor}, + true: {owner, orgAdmin, templateAdmin, orgTemplateAdmin}, + false: {setOtherOrg, userAdmin, memberMe, orgUserAdmin, orgAuditor, orgMemberMe}, }, }, // Some admin style resources From b2c7bdd86ac4bff084037b23d6bf11a16cd040e4 Mon Sep 17 00:00:00 2001 From: Susana Ferreira Date: Tue, 17 Jun 2025 18:18:51 +0000 Subject: [PATCH 5/9] chore: improve code quality and add clarifying comments --- coderd/database/dbauthz/dbauthz.go | 48 ++++++++++++++++---------- coderd/database/modelmethods.go | 9 +++-- coderd/rbac/object_gen.go | 4 +-- coderd/rbac/policy/policy.go | 12 +++++-- coderd/workspacebuilds.go | 10 +++--- site/src/api/rbacresourcesGenerated.ts | 4 +-- 6 files changed, 54 insertions(+), 33 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index b048338045059..b6dabb3bc58c5 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -150,6 +150,30 @@ func (q *querier) authorizeContext(ctx context.Context, action policy.Action, ob return nil } +// authorizeWorkspace handles authorization for workspace resource types. +// prebuilt_workspaces are a subset of workspaces, currently limited to +// supporting delete operations. Therefore, if the action is delete or +// update and the workspace is a prebuild, a prebuilt-specific authorization +// is attempted first. If that fails, it falls back to normal workspace +// authorization. +// Note: Delete operations of workspaces requires both update and delete +// permissions. +func (q *querier) authorizeWorkspace(ctx context.Context, action policy.Action, workspace database.Workspace) error { + var prebuiltErr error + // Special handling for prebuilt_workspace deletion authorization check + if (action == policy.ActionUpdate || action == policy.ActionDelete) && workspace.IsPrebuild() { + // Try prebuilt-specific authorization first + if prebuiltErr = q.authorizeContext(ctx, action, workspace.AsPrebuild()); prebuiltErr == nil { + return nil + } + } + // Fallback to normal workspace authorization check + if err := q.authorizeContext(ctx, action, workspace); err != nil { + return xerrors.Errorf("authorize context: %w", errors.Join(prebuiltErr, err)) + } + return nil +} + type authContextKey struct{} // ActorFromContext returns the authorization subject from the context. @@ -3915,15 +3939,9 @@ func (q *querier) InsertWorkspaceBuild(ctx context.Context, arg database.InsertW action = policy.ActionWorkspaceStop } - if action == policy.ActionDelete && w.IsPrebuild() { - if err := q.authorizeContext(ctx, action, w.PrebuildRBAC()); err != nil { - // Fallback to normal workspace auth check - if err = q.authorizeContext(ctx, action, w); err != nil { - return xerrors.Errorf("authorize context: %w", err) - } - } - } else if err = q.authorizeContext(ctx, action, w); err != nil { - return xerrors.Errorf("authorize context: %w", err) + // Special handling for prebuilt workspace deletion + if err := q.authorizeWorkspace(ctx, action, w); err != nil { + return err } // If we're starting a workspace we need to check the template. @@ -3962,16 +3980,8 @@ func (q *querier) InsertWorkspaceBuildParameters(ctx context.Context, arg databa return err } - if workspace.IsPrebuild() { - err = q.authorizeContext(ctx, policy.ActionUpdate, workspace.PrebuildRBAC()) - // Fallback to normal workspace auth check - if err != nil { - err = q.authorizeContext(ctx, policy.ActionUpdate, workspace) - } - } else { - err = q.authorizeContext(ctx, policy.ActionUpdate, workspace) - } - if err != nil { + // Special handling for prebuilt workspace deletion + if err := q.authorizeWorkspace(ctx, policy.ActionUpdate, workspace); err != nil { return err } diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index b25d7a438cbe2..adb4c0bddd172 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -227,17 +227,22 @@ func (w Workspace) WorkspaceTable() WorkspaceTable { func (w Workspace) RBACObject() rbac.Object { // if w.IsPrebuild() { - // return w.PrebuildRBAC() + // return w.AsPrebuild() //} return w.WorkspaceTable().RBACObject() } +// IsPrebuild returns true if the workspace is a prebuild workspace. +// A workspace is considered a prebuild if its owner is the prebuild system user. func (w Workspace) IsPrebuild() bool { // TODO: avoid import cycle return w.OwnerID == uuid.MustParse("c42fdf75-3097-471c-8c33-fb52454d81c0") } -func (w Workspace) PrebuildRBAC() rbac.Object { +// AsPrebuild returns the RBAC object corresponding to the workspace type. +// If the workspace is a prebuild, it returns a prebuilt_workspace RBAC object. +// Otherwise, it returns a normal workspace RBAC object. +func (w Workspace) AsPrebuild() rbac.Object { if w.IsPrebuild() { return rbac.ResourcePrebuiltWorkspace.WithID(w.ID). InOrg(w.OrganizationID). diff --git a/coderd/rbac/object_gen.go b/coderd/rbac/object_gen.go index 3361e41d29922..1a27c4b9c7d89 100644 --- a/coderd/rbac/object_gen.go +++ b/coderd/rbac/object_gen.go @@ -225,8 +225,8 @@ var ( // ResourcePrebuiltWorkspace // Valid Actions // - "ActionDelete" :: delete prebuilt workspace - // - "ActionRead" :: read prebuilt workspace - // - "ActionUpdate" :: update prebuilt workspace + // - "ActionRead" :: read prebuilt workspace data + // - "ActionUpdate" :: update prebuilt workspace settings ResourcePrebuiltWorkspace = Object{ Type: "prebuilt_workspace", } diff --git a/coderd/rbac/policy/policy.go b/coderd/rbac/policy/policy.go index c7f0d4befbe43..5d1b180533f2b 100644 --- a/coderd/rbac/policy/policy.go +++ b/coderd/rbac/policy/policy.go @@ -103,9 +103,15 @@ var RBACPermissions = map[string]PermissionDefinition{ Actions: workspaceActions, }, "prebuilt_workspace": { - Actions: map[Action]ActionDefinition{ - ActionRead: actDef("read prebuilt workspace"), - ActionUpdate: actDef("update prebuilt workspace"), + // Prebuilt_workspace actions currently apply only to delete operations. + // To successfully delete a workspace, a user must have the following permissions: + // * read: to read the current workspace state + // * update: to modify workspace metadata and related resources during deletion + // (e.g., updating the deleted field in the database) + // * delete: to perform the actual deletion of the workspace + Actions: map[Action]ActionDefinition{ + ActionRead: actDef("read prebuilt workspace data"), + ActionUpdate: actDef("update prebuilt workspace settings"), ActionDelete: actDef("delete prebuilt workspace"), }, }, diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index da6da04ad0afc..3b02d38b28b8a 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -404,16 +404,16 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { ctx, tx, func(action policy.Action, object rbac.Objecter) bool { + // Special handling for prebuilt workspace deletion if object.RBACObject().Type == rbac.ResourceWorkspace.Type && action == policy.ActionDelete { - workspaceObj, ok := object.(database.Workspace) - if ok { - prebuild := workspaceObj.PrebuildRBAC() - // Fallback to normal workspace auth check - if auth := api.Authorize(r, action, prebuild); auth { + if workspaceObj, ok := object.(database.Workspace); ok { + // Try prebuilt-specific authorization first + if auth := api.Authorize(r, action, workspaceObj.AsPrebuild()); auth { return auth } } } + // Fallback to default authorization return api.Authorize(r, action, object) }, audit.WorkspaceBuildBaggageFromRequest(r), diff --git a/site/src/api/rbacresourcesGenerated.ts b/site/src/api/rbacresourcesGenerated.ts index 7edb47f8e8eff..bee1d70704687 100644 --- a/site/src/api/rbacresourcesGenerated.ts +++ b/site/src/api/rbacresourcesGenerated.ts @@ -125,8 +125,8 @@ export const RBACResourceActions: Partial< }, prebuilt_workspace: { delete: "delete prebuilt workspace", - read: "read prebuilt workspace", - update: "update prebuilt workspace", + read: "read prebuilt workspace data", + update: "update prebuilt workspace settings", }, provisioner_daemon: { create: "create a provisioner daemon/key", From f24e4ab4b6f0a56726fd04be2d7302c9fdb52d53 Mon Sep 17 00:00:00 2001 From: Susana Ferreira Date: Wed, 18 Jun 2025 08:58:39 +0000 Subject: [PATCH 6/9] refactor: move PrebuildsSystemUserID constant to database package to resolve import cycle --- coderd/database/constants.go | 5 +++ coderd/database/dbauthz/dbauthz.go | 3 +- coderd/database/dbmem/dbmem.go | 6 ++-- coderd/database/modelmethods.go | 3 +- coderd/database/querier_test.go | 3 +- coderd/prebuilds/id.go | 5 --- enterprise/coderd/groups_test.go | 4 +-- enterprise/coderd/prebuilds/claim.go | 2 +- .../coderd/prebuilds/membership_test.go | 13 ++++---- .../coderd/prebuilds/metricscollector_test.go | 33 +++++++++---------- enterprise/coderd/prebuilds/reconcile.go | 8 ++--- enterprise/coderd/prebuilds/reconcile_test.go | 2 +- enterprise/coderd/workspaces_test.go | 3 +- 13 files changed, 40 insertions(+), 50 deletions(-) create mode 100644 coderd/database/constants.go delete mode 100644 coderd/prebuilds/id.go diff --git a/coderd/database/constants.go b/coderd/database/constants.go new file mode 100644 index 0000000000000..931e0d7e0983d --- /dev/null +++ b/coderd/database/constants.go @@ -0,0 +1,5 @@ +package database + +import "github.com/google/uuid" + +var PrebuildsSystemUserID = uuid.MustParse("c42fdf75-3097-471c-8c33-fb52454d81c0") diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index b6dabb3bc58c5..1774ae7998512 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -21,7 +21,6 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/httpapi/httpapiconstraints" "github.com/coder/coder/v2/coderd/httpmw/loggermw" - "github.com/coder/coder/v2/coderd/prebuilds" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/coderd/rbac/rolestore" @@ -423,7 +422,7 @@ var ( subjectPrebuildsOrchestrator = rbac.Subject{ Type: rbac.SubjectTypePrebuildsOrchestrator, FriendlyName: "Prebuilds Orchestrator", - ID: prebuilds.SystemUserID.String(), + ID: database.PrebuildsSystemUserID.String(), Roles: rbac.Roles([]rbac.Role{ { Identifier: rbac.RoleIdentifier{Name: "prebuilds-orchestrator"}, diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index f838a93d24c78..69b45a3bdaf6b 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -23,11 +23,9 @@ import ( "golang.org/x/exp/maps" "golang.org/x/xerrors" - "github.com/coder/coder/v2/coderd/notifications/types" - "github.com/coder/coder/v2/coderd/prebuilds" - "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/notifications/types" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac/regosql" "github.com/coder/coder/v2/coderd/util/slice" @@ -159,7 +157,7 @@ func New() database.Store { q.mutex.Lock() // We can't insert this user using the interface, because it's a system user. q.data.users = append(q.data.users, database.User{ - ID: prebuilds.SystemUserID, + ID: database.PrebuildsSystemUserID, Email: "prebuilds@coder.com", Username: "prebuilds", CreatedAt: dbtime.Now(), diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index adb4c0bddd172..fe136a05093d2 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -235,8 +235,7 @@ func (w Workspace) RBACObject() rbac.Object { // IsPrebuild returns true if the workspace is a prebuild workspace. // A workspace is considered a prebuild if its owner is the prebuild system user. func (w Workspace) IsPrebuild() bool { - // TODO: avoid import cycle - return w.OwnerID == uuid.MustParse("c42fdf75-3097-471c-8c33-fb52454d81c0") + return w.OwnerID == PrebuildsSystemUserID } // AsPrebuild returns the RBAC object corresponding to the workspace type. diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 74ac5b0a20caf..600fb8269909a 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -27,7 +27,6 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/database/migrations" "github.com/coder/coder/v2/coderd/httpmw" - "github.com/coder/coder/v2/coderd/prebuilds" "github.com/coder/coder/v2/coderd/provisionerdserver" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac/policy" @@ -1418,7 +1417,7 @@ func TestGetUsers_IncludeSystem(t *testing.T) { for _, u := range users { if u.IsSystem { foundSystemUser = true - require.Equal(t, prebuilds.SystemUserID, u.ID) + require.Equal(t, database.PrebuildsSystemUserID, u.ID) } else { foundRegularUser = true require.Equalf(t, other.ID.String(), u.ID.String(), "found unexpected regular user") diff --git a/coderd/prebuilds/id.go b/coderd/prebuilds/id.go deleted file mode 100644 index 7c2bbe79b7a6f..0000000000000 --- a/coderd/prebuilds/id.go +++ /dev/null @@ -1,5 +0,0 @@ -package prebuilds - -import "github.com/google/uuid" - -var SystemUserID = uuid.MustParse("c42fdf75-3097-471c-8c33-fb52454d81c0") diff --git a/enterprise/coderd/groups_test.go b/enterprise/coderd/groups_test.go index 028aa3328535f..f87a9193f5fa4 100644 --- a/enterprise/coderd/groups_test.go +++ b/enterprise/coderd/groups_test.go @@ -6,8 +6,6 @@ import ( "testing" "time" - "github.com/coder/coder/v2/coderd/prebuilds" - "github.com/google/uuid" "github.com/stretchr/testify/require" @@ -833,7 +831,7 @@ func TestGroup(t *testing.T) { ctx := testutil.Context(t, testutil.WaitLong) // nolint:gocritic // "This client is operating as the owner user" is fine in this case. - prebuildsUser, err := client.User(ctx, prebuilds.SystemUserID.String()) + prebuildsUser, err := client.User(ctx, database.PrebuildsSystemUserID.String()) require.NoError(t, err) // The 'Everyone' group always has an ID that matches the organization ID. group, err := userAdminClient.Group(ctx, user.OrganizationID) diff --git a/enterprise/coderd/prebuilds/claim.go b/enterprise/coderd/prebuilds/claim.go index f040ee756e678..b6a85ae1fc094 100644 --- a/enterprise/coderd/prebuilds/claim.go +++ b/enterprise/coderd/prebuilds/claim.go @@ -47,7 +47,7 @@ func (c EnterpriseClaimer) Claim( } func (EnterpriseClaimer) Initiator() uuid.UUID { - return prebuilds.SystemUserID + return database.PrebuildsSystemUserID } var _ prebuilds.Claimer = &EnterpriseClaimer{} diff --git a/enterprise/coderd/prebuilds/membership_test.go b/enterprise/coderd/prebuilds/membership_test.go index 6caa7178d9d60..229814e5bf764 100644 --- a/enterprise/coderd/prebuilds/membership_test.go +++ b/enterprise/coderd/prebuilds/membership_test.go @@ -12,7 +12,6 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" - agplprebuilds "github.com/coder/coder/v2/coderd/prebuilds" "github.com/coder/coder/v2/enterprise/coderd/prebuilds" ) @@ -74,14 +73,14 @@ func TestReconcileAll(t *testing.T) { // dbmem doesn't ensure membership to the default organization dbgen.OrganizationMember(t, db, database.OrganizationMember{ OrganizationID: defaultOrg.ID, - UserID: agplprebuilds.SystemUserID, + UserID: database.PrebuildsSystemUserID, }) } - dbgen.OrganizationMember(t, db, database.OrganizationMember{OrganizationID: unrelatedOrg.ID, UserID: agplprebuilds.SystemUserID}) + dbgen.OrganizationMember(t, db, database.OrganizationMember{OrganizationID: unrelatedOrg.ID, UserID: database.PrebuildsSystemUserID}) if tc.preExistingMembership { // System user already a member of both orgs. - dbgen.OrganizationMember(t, db, database.OrganizationMember{OrganizationID: targetOrg.ID, UserID: agplprebuilds.SystemUserID}) + dbgen.OrganizationMember(t, db, database.OrganizationMember{OrganizationID: targetOrg.ID, UserID: database.PrebuildsSystemUserID}) } presets := []database.GetTemplatePresetsWithPrebuildsRow{newPresetRow(unrelatedOrg.ID)} @@ -91,7 +90,7 @@ func TestReconcileAll(t *testing.T) { // Verify memberships before reconciliation. preReconcileMemberships, err := db.GetOrganizationsByUserID(ctx, database.GetOrganizationsByUserIDParams{ - UserID: agplprebuilds.SystemUserID, + UserID: database.PrebuildsSystemUserID, }) require.NoError(t, err) expectedMembershipsBefore := []uuid.UUID{defaultOrg.ID, unrelatedOrg.ID} @@ -102,11 +101,11 @@ func TestReconcileAll(t *testing.T) { // Reconcile reconciler := prebuilds.NewStoreMembershipReconciler(db, clock) - require.NoError(t, reconciler.ReconcileAll(ctx, agplprebuilds.SystemUserID, presets)) + require.NoError(t, reconciler.ReconcileAll(ctx, database.PrebuildsSystemUserID, presets)) // Verify memberships after reconciliation. postReconcileMemberships, err := db.GetOrganizationsByUserID(ctx, database.GetOrganizationsByUserIDParams{ - UserID: agplprebuilds.SystemUserID, + UserID: database.PrebuildsSystemUserID, }) require.NoError(t, err) expectedMembershipsAfter := expectedMembershipsBefore diff --git a/enterprise/coderd/prebuilds/metricscollector_test.go b/enterprise/coderd/prebuilds/metricscollector_test.go index dce9e07dd110f..7befffe9d3a05 100644 --- a/enterprise/coderd/prebuilds/metricscollector_test.go +++ b/enterprise/coderd/prebuilds/metricscollector_test.go @@ -20,7 +20,6 @@ import ( "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" - agplprebuilds "github.com/coder/coder/v2/coderd/prebuilds" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/coderd/prebuilds" "github.com/coder/coder/v2/testutil" @@ -55,8 +54,8 @@ func TestMetricsCollector(t *testing.T) { name: "prebuild provisioned but not completed", transitions: allTransitions, jobStatuses: allJobStatusesExcept(database.ProvisionerJobStatusPending, database.ProvisionerJobStatusRunning, database.ProvisionerJobStatusCanceling), - initiatorIDs: []uuid.UUID{agplprebuilds.SystemUserID}, - ownerIDs: []uuid.UUID{agplprebuilds.SystemUserID}, + initiatorIDs: []uuid.UUID{database.PrebuildsSystemUserID}, + ownerIDs: []uuid.UUID{database.PrebuildsSystemUserID}, metrics: []metricCheck{ {prebuilds.MetricCreatedCount, ptr.To(1.0), true}, {prebuilds.MetricClaimedCount, ptr.To(0.0), true}, @@ -72,8 +71,8 @@ func TestMetricsCollector(t *testing.T) { name: "prebuild running", transitions: []database.WorkspaceTransition{database.WorkspaceTransitionStart}, jobStatuses: []database.ProvisionerJobStatus{database.ProvisionerJobStatusSucceeded}, - initiatorIDs: []uuid.UUID{agplprebuilds.SystemUserID}, - ownerIDs: []uuid.UUID{agplprebuilds.SystemUserID}, + initiatorIDs: []uuid.UUID{database.PrebuildsSystemUserID}, + ownerIDs: []uuid.UUID{database.PrebuildsSystemUserID}, metrics: []metricCheck{ {prebuilds.MetricCreatedCount, ptr.To(1.0), true}, {prebuilds.MetricClaimedCount, ptr.To(0.0), true}, @@ -89,8 +88,8 @@ func TestMetricsCollector(t *testing.T) { name: "prebuild failed", transitions: allTransitions, jobStatuses: []database.ProvisionerJobStatus{database.ProvisionerJobStatusFailed}, - initiatorIDs: []uuid.UUID{agplprebuilds.SystemUserID}, - ownerIDs: []uuid.UUID{agplprebuilds.SystemUserID, uuid.New()}, + initiatorIDs: []uuid.UUID{database.PrebuildsSystemUserID}, + ownerIDs: []uuid.UUID{database.PrebuildsSystemUserID, uuid.New()}, metrics: []metricCheck{ {prebuilds.MetricCreatedCount, ptr.To(1.0), true}, {prebuilds.MetricFailedCount, ptr.To(1.0), true}, @@ -105,8 +104,8 @@ func TestMetricsCollector(t *testing.T) { name: "prebuild eligible", transitions: []database.WorkspaceTransition{database.WorkspaceTransitionStart}, jobStatuses: []database.ProvisionerJobStatus{database.ProvisionerJobStatusSucceeded}, - initiatorIDs: []uuid.UUID{agplprebuilds.SystemUserID}, - ownerIDs: []uuid.UUID{agplprebuilds.SystemUserID}, + initiatorIDs: []uuid.UUID{database.PrebuildsSystemUserID}, + ownerIDs: []uuid.UUID{database.PrebuildsSystemUserID}, metrics: []metricCheck{ {prebuilds.MetricCreatedCount, ptr.To(1.0), true}, {prebuilds.MetricClaimedCount, ptr.To(0.0), true}, @@ -122,8 +121,8 @@ func TestMetricsCollector(t *testing.T) { name: "prebuild ineligible", transitions: allTransitions, jobStatuses: allJobStatusesExcept(database.ProvisionerJobStatusSucceeded), - initiatorIDs: []uuid.UUID{agplprebuilds.SystemUserID}, - ownerIDs: []uuid.UUID{agplprebuilds.SystemUserID}, + initiatorIDs: []uuid.UUID{database.PrebuildsSystemUserID}, + ownerIDs: []uuid.UUID{database.PrebuildsSystemUserID}, metrics: []metricCheck{ {prebuilds.MetricCreatedCount, ptr.To(1.0), true}, {prebuilds.MetricClaimedCount, ptr.To(0.0), true}, @@ -139,7 +138,7 @@ func TestMetricsCollector(t *testing.T) { name: "prebuild claimed", transitions: allTransitions, jobStatuses: allJobStatuses, - initiatorIDs: []uuid.UUID{agplprebuilds.SystemUserID}, + initiatorIDs: []uuid.UUID{database.PrebuildsSystemUserID}, ownerIDs: []uuid.UUID{uuid.New()}, metrics: []metricCheck{ {prebuilds.MetricCreatedCount, ptr.To(1.0), true}, @@ -169,8 +168,8 @@ func TestMetricsCollector(t *testing.T) { name: "deleted templates should not be included in exported metrics", transitions: allTransitions, jobStatuses: allJobStatuses, - initiatorIDs: []uuid.UUID{agplprebuilds.SystemUserID}, - ownerIDs: []uuid.UUID{agplprebuilds.SystemUserID, uuid.New()}, + initiatorIDs: []uuid.UUID{database.PrebuildsSystemUserID}, + ownerIDs: []uuid.UUID{database.PrebuildsSystemUserID, uuid.New()}, metrics: nil, templateDeleted: []bool{true}, eligible: []bool{false}, @@ -209,7 +208,7 @@ func TestMetricsCollector(t *testing.T) { reconciler := prebuilds.NewStoreReconciler(db, pubsub, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) ctx := testutil.Context(t, testutil.WaitLong) - createdUsers := []uuid.UUID{agplprebuilds.SystemUserID} + createdUsers := []uuid.UUID{database.PrebuildsSystemUserID} for _, user := range slices.Concat(test.ownerIDs, test.initiatorIDs) { if !slices.Contains(createdUsers, user) { dbgen.User(t, db, database.User{ @@ -327,8 +326,8 @@ func TestMetricsCollector_DuplicateTemplateNames(t *testing.T) { test := testCase{ transition: database.WorkspaceTransitionStart, jobStatus: database.ProvisionerJobStatusSucceeded, - initiatorID: agplprebuilds.SystemUserID, - ownerID: agplprebuilds.SystemUserID, + initiatorID: database.PrebuildsSystemUserID, + ownerID: database.PrebuildsSystemUserID, metrics: []metricCheck{ {prebuilds.MetricCreatedCount, ptr.To(1.0), true}, {prebuilds.MetricClaimedCount, ptr.To(0.0), true}, diff --git a/enterprise/coderd/prebuilds/reconcile.go b/enterprise/coderd/prebuilds/reconcile.go index 3a1ab66d009a7..57490adb83974 100644 --- a/enterprise/coderd/prebuilds/reconcile.go +++ b/enterprise/coderd/prebuilds/reconcile.go @@ -265,7 +265,7 @@ func (c *StoreReconciler) ReconcileAll(ctx context.Context) error { } membershipReconciler := NewStoreMembershipReconciler(c.store, c.clock) - err = membershipReconciler.ReconcileAll(ctx, prebuilds.SystemUserID, snapshot.Presets) + err = membershipReconciler.ReconcileAll(ctx, database.PrebuildsSystemUserID, snapshot.Presets) if err != nil { return xerrors.Errorf("reconcile prebuild membership: %w", err) } @@ -667,7 +667,7 @@ func (c *StoreReconciler) createPrebuiltWorkspace(ctx context.Context, prebuiltW ID: prebuiltWorkspaceID, CreatedAt: now, UpdatedAt: now, - OwnerID: prebuilds.SystemUserID, + OwnerID: database.PrebuildsSystemUserID, OrganizationID: template.OrganizationID, TemplateID: template.ID, Name: name, @@ -709,7 +709,7 @@ func (c *StoreReconciler) deletePrebuiltWorkspace(ctx context.Context, prebuiltW return xerrors.Errorf("failed to get template: %w", err) } - if workspace.OwnerID != prebuilds.SystemUserID { + if workspace.OwnerID != database.PrebuildsSystemUserID { return xerrors.Errorf("prebuilt workspace is not owned by prebuild user anymore, probably it was claimed") } @@ -752,7 +752,7 @@ func (c *StoreReconciler) provision( builder := wsbuilder.New(workspace, transition). Reason(database.BuildReasonInitiator). - Initiator(prebuilds.SystemUserID). + Initiator(database.PrebuildsSystemUserID). MarkPrebuild() if transition != database.WorkspaceTransitionDelete { diff --git a/enterprise/coderd/prebuilds/reconcile_test.go b/enterprise/coderd/prebuilds/reconcile_test.go index 92f903357a1d3..8ed8fe1f3c3c5 100644 --- a/enterprise/coderd/prebuilds/reconcile_test.go +++ b/enterprise/coderd/prebuilds/reconcile_test.go @@ -1668,7 +1668,7 @@ func setupTestDBPrebuild( templateVersionID uuid.UUID, ) (database.WorkspaceTable, database.WorkspaceBuild) { t.Helper() - return setupTestDBWorkspace(t, clock, db, ps, transition, prebuildStatus, orgID, preset, templateID, templateVersionID, agplprebuilds.SystemUserID, agplprebuilds.SystemUserID) + return setupTestDBWorkspace(t, clock, db, ps, transition, prebuildStatus, orgID, preset, templateID, templateVersionID, database.PrebuildsSystemUserID, database.PrebuildsSystemUserID) } func setupTestDBWorkspace( diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 226232f37bf7f..f52678b297c53 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -32,7 +32,6 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/notifications" - "github.com/coder/coder/v2/coderd/prebuilds" "github.com/coder/coder/v2/coderd/provisionerdserver" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac/policy" @@ -496,7 +495,7 @@ func TestCreateUserWorkspace(t *testing.T) { }).Do() r := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ - OwnerID: prebuilds.SystemUserID, + OwnerID: database.PrebuildsSystemUserID, TemplateID: tv.Template.ID, }).Seed(database.WorkspaceBuild{ TemplateVersionID: tv.TemplateVersion.ID, From 98576c6fea47a61005c5943ab29dafc270da75d7 Mon Sep 17 00:00:00 2001 From: Susana Ferreira Date: Wed, 18 Jun 2025 09:09:33 +0000 Subject: [PATCH 7/9] chore: remove unnecessary comment --- coderd/database/modelmethods.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index fe136a05093d2..0e17335d9ab3f 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -226,9 +226,6 @@ func (w Workspace) WorkspaceTable() WorkspaceTable { } func (w Workspace) RBACObject() rbac.Object { - // if w.IsPrebuild() { - // return w.AsPrebuild() - //} return w.WorkspaceTable().RBACObject() } From d461a3de4ba7dee9a1bad5b68cc51bf5f3e39e86 Mon Sep 17 00:00:00 2001 From: Susana Ferreira Date: Wed, 18 Jun 2025 11:49:03 +0000 Subject: [PATCH 8/9] test: fix roles_test PrebuiltWorkspace to have the PrebuildsSystemUserID as owner --- coderd/rbac/roles_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index 4e0116a5614d4..df72c5d9127f5 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -3,6 +3,7 @@ package rbac_test import ( "context" "fmt" + "github.com/coder/coder/v2/coderd/database" "testing" "github.com/google/uuid" @@ -499,7 +500,7 @@ func TestRolePermissions(t *testing.T) { { Name: "PrebuiltWorkspace", Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, - Resource: rbac.ResourcePrebuiltWorkspace.WithID(uuid.New()).InOrg(orgID).WithOwner(memberMe.Actor.ID), + Resource: rbac.ResourcePrebuiltWorkspace.WithID(uuid.New()).InOrg(orgID).WithOwner(database.PrebuildsSystemUserID.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAdmin, templateAdmin, orgTemplateAdmin}, false: {setOtherOrg, userAdmin, memberMe, orgUserAdmin, orgAuditor, orgMemberMe}, From 5f896f0b3a01011695397e312cd7d59ae979dedd Mon Sep 17 00:00:00 2001 From: Susana Ferreira Date: Wed, 18 Jun 2025 18:35:14 +0000 Subject: [PATCH 9/9] test: improve end-to-end permission tests for deletion of prebuilt workspaces --- cli/delete_test.go | 219 ++++++++++++++++++ coderd/database/dbauthz/dbauthz.go | 10 +- coderd/rbac/object_gen.go | 1 - coderd/rbac/policy/policy.go | 7 +- coderd/rbac/roles.go | 8 +- coderd/rbac/roles_test.go | 5 +- coderd/wsbuilder/wsbuilder.go | 13 +- codersdk/rbacresources_gen.go | 2 +- enterprise/coderd/prebuilds/reconcile_test.go | 94 -------- site/src/api/rbacresourcesGenerated.ts | 1 - 10 files changed, 248 insertions(+), 112 deletions(-) diff --git a/cli/delete_test.go b/cli/delete_test.go index 1d4dc8dfb40ad..a705f084675c8 100644 --- a/cli/delete_test.go +++ b/cli/delete_test.go @@ -2,9 +2,18 @@ package cli_test import ( "context" + "database/sql" "fmt" "io" "testing" + "time" + + "github.com/google/uuid" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/pubsub" + "github.com/coder/quartz" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -209,4 +218,214 @@ func TestDelete(t *testing.T) { cancel() <-doneChan }) + + t.Run("Workspace delete permissions", func(t *testing.T) { + t.Parallel() + if !dbtestutil.WillUsePostgres() { + t.Skip("this test requires postgres") + } + + clock := quartz.NewMock(t) + ctx := testutil.Context(t, testutil.WaitSuperLong) + + // Setup + db, pb := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure()) + client, _ := coderdtest.NewWithProvisionerCloser(t, &coderdtest.Options{ + Database: db, + Pubsub: pb, + IncludeProvisionerDaemon: true, + }) + owner := coderdtest.CreateFirstUser(t, client) + orgID := owner.OrganizationID + + // Given a template version with a preset and a template + version := coderdtest.CreateTemplateVersion(t, client, orgID, nil) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + preset := setupTestDBPreset(t, db, version.ID) + template := coderdtest.CreateTemplate(t, client, orgID, version.ID) + + cases := []struct { + name string + client *codersdk.Client + expectedPrebuiltDeleteErrMsg string + expectedWorkspaceDeleteErrMsg string + }{ + // Users with the OrgAdmin role should be able to delete both normal and prebuilt workspaces + { + name: "OrgAdmin", + client: func() *codersdk.Client { + client, _ := coderdtest.CreateAnotherUser(t, client, orgID, rbac.ScopedRoleOrgAdmin(orgID)) + return client + }(), + }, + // Users with the TemplateAdmin role should be able to delete prebuilt workspaces, but not normal workspaces + { + name: "TemplateAdmin", + client: func() *codersdk.Client { + client, _ := coderdtest.CreateAnotherUser(t, client, orgID, rbac.RoleTemplateAdmin()) + return client + }(), + expectedWorkspaceDeleteErrMsg: "unexpected status code 403: You do not have permission to delete this workspace.", + }, + // Users with the OrgTemplateAdmin role should be able to delete prebuilt workspaces, but not normal workspaces + { + name: "OrgTemplateAdmin", + client: func() *codersdk.Client { + client, _ := coderdtest.CreateAnotherUser(t, client, orgID, rbac.ScopedRoleOrgTemplateAdmin(orgID)) + return client + }(), + expectedWorkspaceDeleteErrMsg: "unexpected status code 403: You do not have permission to delete this workspace.", + }, + // Users with the Member role should not be able to delete prebuilt or normal workspaces + { + name: "Member", + client: func() *codersdk.Client { + client, _ := coderdtest.CreateAnotherUser(t, client, orgID, rbac.RoleMember()) + return client + }(), + expectedPrebuiltDeleteErrMsg: "unexpected status code 404: Resource not found or you do not have access to this resource", + expectedWorkspaceDeleteErrMsg: "unexpected status code 404: Resource not found or you do not have access to this resource", + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + // Create one prebuilt workspace (owned by system user) and one normal workspace (owned by the owner user) + // Each workspace is persisted in the DB along with associated workspace jobs and builds. + dbPrebuiltWorkspace := setupTestDBWorkspace(t, clock, db, pb, orgID, database.PrebuildsSystemUserID, template.ID, version.ID, preset.ID) + dbUserWorkspace := setupTestDBWorkspace(t, clock, db, pb, orgID, owner.UserID, template.ID, version.ID, preset.ID) + + // Ensure at least one prebuilt workspace is reported as running in the database + testutil.Eventually(ctx, t, func(ctx context.Context) (done bool) { + running, err := db.GetRunningPrebuiltWorkspaces(ctx) + if !assert.NoError(t, err) || !assert.GreaterOrEqual(t, len(running), 1) { + return false + } + return true + }, testutil.IntervalMedium, "running prebuilt workspaces timeout") + + runningWorkspaces, err := db.GetRunningPrebuiltWorkspaces(ctx) + require.NoError(t, err) + require.GreaterOrEqual(t, len(runningWorkspaces), 1) + + // Get the full prebuilt workspace object from the DB + prebuiltWorkspace, err := db.GetWorkspaceByID(ctx, runningWorkspaces[0].ID) + require.NoError(t, err) + + // Attempt to delete the prebuilt workspace as the test client + build, err := tc.client.CreateWorkspaceBuild(ctx, dbPrebuiltWorkspace.ID, codersdk.CreateWorkspaceBuildRequest{ + Transition: codersdk.WorkspaceTransitionDelete, + }) + // Validate the result based on the expected error message + if tc.expectedPrebuiltDeleteErrMsg != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectedPrebuiltDeleteErrMsg) + } else { + require.NoError(t, err, "delete the prebuilt workspace") + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, build.ID) + + // Verify that the prebuilt workspace is now marked as deleted + deletedWorkspace, err := client.DeletedWorkspace(ctx, prebuiltWorkspace.ID) + require.NoError(t, err) + require.Equal(t, prebuiltWorkspace.ID, deletedWorkspace.ID) + } + + // Get the full user workspace object from the DB + userWorkspace, err := db.GetWorkspaceByID(ctx, dbUserWorkspace.ID) + require.NoError(t, err) + + // Attempt to delete the prebuilt workspace as the test client + build, err = tc.client.CreateWorkspaceBuild(ctx, dbUserWorkspace.ID, codersdk.CreateWorkspaceBuildRequest{ + Transition: codersdk.WorkspaceTransitionDelete, + }) + // Validate the result based on the expected error message + if tc.expectedWorkspaceDeleteErrMsg != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectedWorkspaceDeleteErrMsg) + } else { + require.NoError(t, err, "delete the user Workspace") + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, build.ID) + + // Verify that the user workspace is now marked as deleted + deletedWorkspace, err := client.DeletedWorkspace(ctx, userWorkspace.ID) + require.NoError(t, err) + require.Equal(t, userWorkspace.ID, deletedWorkspace.ID) + } + }) + } + }) +} + +func setupTestDBPreset( + t *testing.T, + db database.Store, + templateVersionID uuid.UUID, +) database.TemplateVersionPreset { + t.Helper() + + preset := dbgen.Preset(t, db, database.InsertPresetParams{ + TemplateVersionID: templateVersionID, + Name: "preset-test", + DesiredInstances: sql.NullInt32{ + Valid: true, + Int32: 1, + }, + }) + dbgen.PresetParameter(t, db, database.InsertPresetParametersParams{ + TemplateVersionPresetID: preset.ID, + Names: []string{"test"}, + Values: []string{"test"}, + }) + + return preset +} + +func setupTestDBWorkspace( + t *testing.T, + clock quartz.Clock, + db database.Store, + ps pubsub.Pubsub, + orgID uuid.UUID, + ownerID uuid.UUID, + templateID uuid.UUID, + templateVersionID uuid.UUID, + presetID uuid.UUID, +) database.WorkspaceTable { + t.Helper() + + workspace := dbgen.Workspace(t, db, database.WorkspaceTable{ + TemplateID: templateID, + OrganizationID: orgID, + OwnerID: ownerID, + Deleted: false, + CreatedAt: time.Now().Add(-time.Hour * 2), + }) + job := dbgen.ProvisionerJob(t, db, ps, database.ProvisionerJob{ + InitiatorID: ownerID, + CreatedAt: time.Now().Add(-time.Hour * 2), + StartedAt: sql.NullTime{Time: clock.Now().Add(-time.Hour * 2), Valid: true}, + CompletedAt: sql.NullTime{Time: clock.Now().Add(-time.Hour), Valid: true}, + OrganizationID: orgID, + }) + workspaceBuild := dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ + WorkspaceID: workspace.ID, + InitiatorID: ownerID, + TemplateVersionID: templateVersionID, + JobID: job.ID, + TemplateVersionPresetID: uuid.NullUUID{UUID: presetID, Valid: true}, + Transition: database.WorkspaceTransitionStart, + CreatedAt: clock.Now(), + }) + dbgen.WorkspaceBuildParameters(t, db, []database.WorkspaceBuildParameter{ + { + WorkspaceBuildID: workspaceBuild.ID, + Name: "test", + Value: "test", + }, + }) + + return workspace } diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index d8441c01e693d..e8a846519e07a 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -149,7 +149,7 @@ func (q *querier) authorizeContext(ctx context.Context, action policy.Action, ob return nil } -// authorizeWorkspace handles authorization for workspace resource types. +// authorizePrebuiltWorkspace handles authorization for workspace resource types. // prebuilt_workspaces are a subset of workspaces, currently limited to // supporting delete operations. Therefore, if the action is delete or // update and the workspace is a prebuild, a prebuilt-specific authorization @@ -157,7 +157,7 @@ func (q *querier) authorizeContext(ctx context.Context, action policy.Action, ob // authorization. // Note: Delete operations of workspaces requires both update and delete // permissions. -func (q *querier) authorizeWorkspace(ctx context.Context, action policy.Action, workspace database.Workspace) error { +func (q *querier) authorizePrebuiltWorkspace(ctx context.Context, action policy.Action, workspace database.Workspace) error { var prebuiltErr error // Special handling for prebuilt_workspace deletion authorization check if (action == policy.ActionUpdate || action == policy.ActionDelete) && workspace.IsPrebuild() { @@ -439,7 +439,7 @@ var ( // Explicitly setting PrebuiltWorkspace permissions for clarity. // Note: even without PrebuiltWorkspace permissions, access is still granted via Workspace permissions. rbac.ResourcePrebuiltWorkspace.Type: { - policy.ActionRead, policy.ActionUpdate, policy.ActionDelete, + policy.ActionUpdate, policy.ActionDelete, }, // Should be able to add the prebuilds system user as a member to any organization that needs prebuilds. rbac.ResourceOrganizationMember.Type: { @@ -3962,7 +3962,7 @@ func (q *querier) InsertWorkspaceBuild(ctx context.Context, arg database.InsertW } // Special handling for prebuilt workspace deletion - if err := q.authorizeWorkspace(ctx, action, w); err != nil { + if err := q.authorizePrebuiltWorkspace(ctx, action, w); err != nil { return err } @@ -4003,7 +4003,7 @@ func (q *querier) InsertWorkspaceBuildParameters(ctx context.Context, arg databa } // Special handling for prebuilt workspace deletion - if err := q.authorizeWorkspace(ctx, policy.ActionUpdate, workspace); err != nil { + if err := q.authorizePrebuiltWorkspace(ctx, policy.ActionUpdate, workspace); err != nil { return err } diff --git a/coderd/rbac/object_gen.go b/coderd/rbac/object_gen.go index 1a27c4b9c7d89..a5c696fb2a491 100644 --- a/coderd/rbac/object_gen.go +++ b/coderd/rbac/object_gen.go @@ -225,7 +225,6 @@ var ( // ResourcePrebuiltWorkspace // Valid Actions // - "ActionDelete" :: delete prebuilt workspace - // - "ActionRead" :: read prebuilt workspace data // - "ActionUpdate" :: update prebuilt workspace settings ResourcePrebuiltWorkspace = Object{ Type: "prebuilt_workspace", diff --git a/coderd/rbac/policy/policy.go b/coderd/rbac/policy/policy.go index 5d1b180533f2b..733a70bcafd0e 100644 --- a/coderd/rbac/policy/policy.go +++ b/coderd/rbac/policy/policy.go @@ -104,13 +104,14 @@ var RBACPermissions = map[string]PermissionDefinition{ }, "prebuilt_workspace": { // Prebuilt_workspace actions currently apply only to delete operations. - // To successfully delete a workspace, a user must have the following permissions: - // * read: to read the current workspace state + // To successfully delete a prebuilt workspace, a user must have the following permissions: + // * workspace.read: to read the current workspace state // * update: to modify workspace metadata and related resources during deletion // (e.g., updating the deleted field in the database) // * delete: to perform the actual deletion of the workspace + // If the user lacks prebuilt_workspace update or delete permissions, + // the authorization will always fall back to the corresponding permissions on workspace. Actions: map[Action]ActionDefinition{ - ActionRead: actDef("read prebuilt workspace data"), ActionUpdate: actDef("update prebuilt workspace settings"), ActionDelete: actDef("delete prebuilt workspace"), }, diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 7623e1dc3af21..8acdf7486ddd2 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -278,7 +278,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // 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.ActionRead, policy.ActionUpdate, policy.ActionDelete}, + ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete}, })...), Org: map[string][]Permission{}, User: []Permission{}, @@ -341,7 +341,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // CRUD all files, even those they did not upload. ResourceFile.Type: {policy.ActionCreate, policy.ActionRead}, ResourceWorkspace.Type: {policy.ActionRead}, - ResourcePrebuiltWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, + ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete}, // CRUD to provisioner daemons for now. ResourceProvisionerDaemon.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, // Needs to read all organizations since @@ -424,7 +424,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // 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.ActionRead, policy.ActionUpdate, policy.ActionDelete}, + ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete}, })...), }, User: []Permission{}, @@ -505,7 +505,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceTemplate.Type: ResourceTemplate.AvailableActions(), ResourceFile.Type: {policy.ActionCreate, policy.ActionRead}, ResourceWorkspace.Type: {policy.ActionRead}, - ResourcePrebuiltWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, + ResourcePrebuiltWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete}, // Assigning template perms requires this permission. ResourceOrganization.Type: {policy.ActionRead}, ResourceOrganizationMember.Type: {policy.ActionRead}, diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index df72c5d9127f5..a1b7c7c15d03a 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -3,9 +3,10 @@ package rbac_test import ( "context" "fmt" - "github.com/coder/coder/v2/coderd/database" "testing" + "github.com/coder/coder/v2/coderd/database" + "github.com/google/uuid" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" @@ -499,7 +500,7 @@ func TestRolePermissions(t *testing.T) { }, { Name: "PrebuiltWorkspace", - Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, + Actions: []policy.Action{policy.ActionUpdate, policy.ActionDelete}, Resource: rbac.ResourcePrebuiltWorkspace.WithID(uuid.New()).InOrg(orgID).WithOwner(database.PrebuildsSystemUserID.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAdmin, templateAdmin, orgTemplateAdmin}, diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 9605df58014de..b52a20ac1e9db 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -918,7 +918,18 @@ func (b *Builder) authorize(authFunc func(action policy.Action, object rbac.Obje msg := fmt.Sprintf("Transition %q not supported.", b.trans) return BuildError{http.StatusBadRequest, msg, xerrors.New(msg)} } - if !authFunc(action, b.workspace) { + + // Special handling for prebuilt workspace deletion + authorized := false + if action == policy.ActionDelete && b.workspace.IsPrebuild() && authFunc(action, b.workspace.AsPrebuild()) { + authorized = true + } + // Fallback to default authorization + if !authorized && authFunc(action, b.workspace) { + authorized = true + } + + if !authorized { if authFunc(policy.ActionRead, b.workspace) { // If the user can read the workspace, but not delete/create/update. Show // a more helpful error. They are allowed to know the workspace exists. diff --git a/codersdk/rbacresources_gen.go b/codersdk/rbacresources_gen.go index b3f3db99eae21..1304218ad7bea 100644 --- a/codersdk/rbacresources_gen.go +++ b/codersdk/rbacresources_gen.go @@ -92,7 +92,7 @@ var RBACResourceActions = map[RBACResource][]RBACAction{ ResourceOauth2AppSecret: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceOrganization: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceOrganizationMember: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourcePrebuiltWorkspace: {ActionDelete, ActionRead, ActionUpdate}, + ResourcePrebuiltWorkspace: {ActionDelete, ActionUpdate}, ResourceProvisionerDaemon: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceProvisionerJobs: {ActionCreate, ActionRead, ActionUpdate}, ResourceReplicas: {ActionRead}, diff --git a/enterprise/coderd/prebuilds/reconcile_test.go b/enterprise/coderd/prebuilds/reconcile_test.go index 8f3a734784dde..70d25e84beb58 100644 --- a/enterprise/coderd/prebuilds/reconcile_test.go +++ b/enterprise/coderd/prebuilds/reconcile_test.go @@ -9,12 +9,6 @@ import ( "testing" "time" - "github.com/coder/coder/v2/coderd/coderdtest" - "github.com/coder/coder/v2/coderd/rbac" - "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" - "github.com/coder/coder/v2/enterprise/coderd/license" - "github.com/coder/coder/v2/provisionersdk" - "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" "golang.org/x/xerrors" @@ -39,7 +33,6 @@ import ( "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/pubsub" - agplprebuilds "github.com/coder/coder/v2/coderd/prebuilds" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/coderd/prebuilds" "github.com/coder/coder/v2/testutil" @@ -427,93 +420,6 @@ func TestPrebuildReconciliation(t *testing.T) { } } -func TestTemplateAdminDelete(t *testing.T) { - t.Parallel() - - if !dbtestutil.WillUsePostgres() { - t.Skip("This test requires postgres") - } - - t.Run("template admin delete prebuilds", func(t *testing.T) { - t.Parallel() - - clock := quartz.NewMock(t) - - // Setup. - ctx := testutil.Context(t, testutil.WaitSuperLong) - db, pubsub := dbtestutil.NewDB(t) - - spy := newStoreSpy(db, nil) - - logger := testutil.Logger(t) - client, _, api, owner := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ - Options: &coderdtest.Options{ - Database: spy, - Pubsub: pubsub, - }, - LicenseOptions: &coderdenttest.LicenseOptions{ - Features: license.Features{ - codersdk.FeatureExternalProvisionerDaemons: 1, - }, - }, - - EntitlementsUpdateInterval: time.Second, - }) - - orgID := owner.OrganizationID - - provisionerCloser := coderdenttest.NewExternalProvisionerDaemon(t, client, orgID, map[string]string{ - provisionersdk.TagScope: provisionersdk.ScopeOrganization, - }) - defer provisionerCloser.Close() - - reconciler := prebuilds.NewStoreReconciler(spy, pubsub, codersdk.PrebuildsConfig{}, logger, clock, prometheus.NewRegistry(), newNoopEnqueuer()) - var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(spy) - api.AGPL.PrebuildsClaimer.Store(&claimer) - - version := coderdtest.CreateTemplateVersion(t, client, orgID, templateWithAgentAndPresetsWithPrebuilds(2)) - _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) - template := coderdtest.CreateTemplate(t, client, orgID, version.ID) - presets, err := client.TemplateVersionPresets(ctx, version.ID) - require.NoError(t, err) - require.Len(t, presets, 2) - preset := setupTestDBPreset(t, db, version.ID, 2, "b0rked") - - templateAdminClient, _ := coderdtest.CreateAnotherUser(t, client, orgID, rbac.RoleTemplateAdmin()) - - workspace, _ := setupTestDBPrebuild( - t, - clock, - db, - pubsub, - database.WorkspaceTransitionStart, - database.ProvisionerJobStatusSucceeded, - orgID, - preset, - template.ID, - version.ID, - ) - - require.NoError(t, reconciler.ReconcileAll(ctx)) - - runningWorkspaces, err := db.GetRunningPrebuiltWorkspaces(ctx) - require.NoError(t, err) - - prebuiltWorkspace, err := db.GetWorkspaceByID(ctx, runningWorkspaces[0].ID) - require.NoError(t, err) - - build, err := templateAdminClient.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ - Transition: codersdk.WorkspaceTransitionDelete, - }) - require.NoError(t, err, "delete the workspace") - coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, build.ID) - - workspaceNew, err := client.DeletedWorkspace(ctx, prebuiltWorkspace.ID) - require.NoError(t, err) - require.Equal(t, prebuiltWorkspace.ID, workspaceNew.ID) - }) -} - // brokenPublisher is used to validate that Publish() calls which always fail do not affect the reconciler's behavior, // since the messages published are not essential but merely advisory. type brokenPublisher struct { diff --git a/site/src/api/rbacresourcesGenerated.ts b/site/src/api/rbacresourcesGenerated.ts index bee1d70704687..3ec6a3accee32 100644 --- a/site/src/api/rbacresourcesGenerated.ts +++ b/site/src/api/rbacresourcesGenerated.ts @@ -125,7 +125,6 @@ export const RBACResourceActions: Partial< }, prebuilt_workspace: { delete: "delete prebuilt workspace", - read: "read prebuilt workspace data", update: "update prebuilt workspace settings", }, provisioner_daemon: {