Skip to content

chore: reorder prebuilt workspace authorization logic #18506

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions coderd/database/dbauthz/dbauthz.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,26 +151,28 @@ func (q *querier) authorizeContext(ctx context.Context, action policy.Action, ob

// 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
// is attempted first. If that fails, it falls back to normal workspace
// authorization.
// supporting delete operations. This function first attempts normal workspace
// authorization. If that fails, the action is delete or update and the workspace
// is a prebuild, a prebuilt-specific authorization is attempted.
// Note: Delete operations of workspaces requires both update and delete
// permissions.
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
// Try default workspace authorization first
var workspaceErr error
if workspaceErr = q.authorizeContext(ctx, action, workspace); workspaceErr == nil {
return nil
}

// Special handling for prebuilt workspace deletion
if (action == policy.ActionUpdate || action == policy.ActionDelete) && workspace.IsPrebuild() {
// Try prebuilt-specific authorization first
var prebuiltErr error
if prebuiltErr = q.authorizeContext(ctx, action, workspace.AsPrebuild()); prebuiltErr == nil {
return nil
}
return xerrors.Errorf("authorize context failed for workspace (%v) and prebuilt (%w)", workspaceErr, prebuiltErr)
}
// 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

return xerrors.Errorf("authorize context: %w", workspaceErr)
}

type authContextKey struct{}
Expand Down
24 changes: 22 additions & 2 deletions coderd/database/dbauthz/dbauthz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5590,7 +5590,17 @@ func (s *MethodTestSuite) TestAuthorizePrebuiltWorkspace() {
Reason: database.BuildReasonInitiator,
TemplateVersionID: tv.ID,
JobID: pj.ID,
}).Asserts(w.AsPrebuild(), policy.ActionDelete)
}).
// Simulate a fallback authorization flow:
// - First, the default workspace authorization fails (simulated by returning an error).
// - Then, authorization is retried using the prebuilt workspace object, which succeeds.
// The test asserts that both authorization attempts occur in the correct order.
WithSuccessAuthorizer(func(ctx context.Context, subject rbac.Subject, action policy.Action, obj rbac.Object) error {
if obj.Type == rbac.ResourceWorkspace.Type {
return xerrors.Errorf("not authorized for workspace type")
}
return nil
}).Asserts(w, policy.ActionDelete, w.AsPrebuild(), policy.ActionDelete)
}))
s.Run("PrebuildUpdate/InsertWorkspaceBuildParameters", s.Subtest(func(db database.Store, check *expects) {
u := dbgen.User(s.T(), db, database.User{})
Expand Down Expand Up @@ -5619,6 +5629,16 @@ func (s *MethodTestSuite) TestAuthorizePrebuiltWorkspace() {
})
check.Args(database.InsertWorkspaceBuildParametersParams{
WorkspaceBuildID: wb.ID,
}).Asserts(w.AsPrebuild(), policy.ActionUpdate)
}).
// Simulate a fallback authorization flow:
// - First, the default workspace authorization fails (simulated by returning an error).
// - Then, authorization is retried using the prebuilt workspace object, which succeeds.
// The test asserts that both authorization attempts occur in the correct order.
WithSuccessAuthorizer(func(ctx context.Context, subject rbac.Subject, action policy.Action, obj rbac.Object) error {
if obj.Type == rbac.ResourceWorkspace.Type {
return xerrors.Errorf("not authorized for workspace type")
}
return nil
}).Asserts(w, policy.ActionUpdate, w.AsPrebuild(), policy.ActionUpdate)
}))
}
7 changes: 7 additions & 0 deletions coderd/database/modelmethods.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,13 @@ func (gm GroupMember) RBACObject() rbac.Object {
return rbac.ResourceGroupMember.WithID(gm.UserID).InOrg(gm.OrganizationID).WithOwner(gm.UserID.String())
}

// PrebuiltWorkspaceResource defines the interface for types that can be identified as prebuilt workspaces
// and converted to their corresponding prebuilt workspace RBAC object.
type PrebuiltWorkspaceResource interface {
IsPrebuild() bool
AsPrebuild() rbac.Object
}

// WorkspaceTable converts a Workspace to it's reduced version.
// A more generalized solution is to use json marshaling to
// consistently keep these two structs in sync.
Expand Down
15 changes: 7 additions & 8 deletions coderd/workspacebuilds.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,17 +391,16 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
tx,
api.FileCache,
func(action policy.Action, object rbac.Objecter) bool {
if auth := api.Authorize(r, action, object); auth {
return true
}
// Special handling for prebuilt workspace deletion
if object.RBACObject().Type == rbac.ResourceWorkspace.Type && action == policy.ActionDelete {
if workspaceObj, ok := object.(database.Workspace); ok {
// Try prebuilt-specific authorization first
if auth := api.Authorize(r, action, workspaceObj.AsPrebuild()); auth {
return auth
}
if action == policy.ActionDelete {
if workspaceObj, ok := object.(database.PrebuiltWorkspaceResource); ok && workspaceObj.IsPrebuild() {
return api.Authorize(r, action, workspaceObj.AsPrebuild())
}
}
// Fallback to default authorization
return api.Authorize(r, action, object)
return false
},
audit.WorkspaceBuildBaggageFromRequest(r),
)
Expand Down
12 changes: 5 additions & 7 deletions coderd/wsbuilder/wsbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -1055,14 +1055,12 @@ func (b *Builder) authorize(authFunc func(action policy.Action, object rbac.Obje
return BuildError{http.StatusBadRequest, msg, xerrors.New(msg)}
}

// Try default workspace authorization first
authorized := 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 && action == policy.ActionDelete && b.workspace.IsPrebuild() {
authorized = authFunc(action, b.workspace.AsPrebuild())
}

if !authorized {
Expand Down
Loading