synced/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2025-07-12 00:14:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: allow TemplateAdmin to delete prebuilds via auth layer (#18333)

Browse Source 
## Description

This PR adds support for deleting prebuilt workspaces via the
authorization layer. It introduces special-case handling to ensure that
`prebuilt_workspace` permissions are evaluated when attempting to delete
a prebuilt workspace, falling back to the standard `workspace` resource
as 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 and
prebuilt workspaces**.

⚠️ This implementation is scoped to **deletion operations only**. No
other 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 during
deletion (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 workspace
authorization logic.
* Ensured both `prebuilt_workspace` and `workspace` permissions are
checked.
* 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
(commit
f24e4ab4b6).
* Update middleware `ExtractOrganizationMember` to include system user
members.
This commit is contained in:
Susana Ferreira
2025-06-20 17:36:32 +01:00
committed by GitHub
parent d61353f468
commit 72f7d70bab
29 changed files with 493 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
enterprise/coderd/groups_test.go
View File

@ -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)

2
enterprise/coderd/prebuilds/claim.go
View File

@ -47,7 +47,7 @@ func (c EnterpriseClaimer) Claim(
}
func (EnterpriseClaimer) Initiator() uuid.UUID {
return prebuilds.SystemUserID
return database.PrebuildsSystemUserID
}
var _ prebuilds.Claimer = &EnterpriseClaimer{}

13
enterprise/coderd/prebuilds/membership_test.go
View File

@ -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

33
enterprise/coderd/prebuilds/metricscollector_test.go
View File

@ -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},

8
enterprise/coderd/prebuilds/reconcile.go
View File

@ -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)
}
@ -676,7 +676,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,
@ -718,7 +718,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")
}
@ -761,7 +761,7 @@ func (c *StoreReconciler) provision(
builder := wsbuilder.New(workspace, transition).
Reason(database.BuildReasonInitiator).
Initiator(prebuilds.SystemUserID).
Initiator(database.PrebuildsSystemUserID).
MarkPrebuild()
if transition != database.WorkspaceTransitionDelete {

3
enterprise/coderd/prebuilds/reconcile_test.go
View File

@ -33,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"
@ -2021,7 +2020,7 @@ func setupTestDBPrebuild(
opts ...prebuildOption,
) (database.WorkspaceTable, database.WorkspaceBuild) {
t.Helper()
return setupTestDBWorkspace(t, clock, db, ps, transition, prebuildStatus, orgID, preset, templateID, templateVersionID, agplprebuilds.SystemUserID, agplprebuilds.SystemUserID, opts...)
return setupTestDBWorkspace(t, clock, db, ps, transition, prebuildStatus, orgID, preset, templateID, templateVersionID, database.PrebuildsSystemUserID, database.PrebuildsSystemUserID, opts...)
}
func setupTestDBWorkspace(

3
enterprise/coderd/workspaces_test.go
View File

@ -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,