chore: reorder prebuilt workspace authorization logic (#18506)

## Description

Follow-up from PR https://github.com/coder/coder/pull/18333
Related with:
https://github.com/coder/coder/pull/18333#discussion_r2159300881

This changes the authorization logic to first try the normal workspace
authorization check, and only if the resource is a prebuilt workspace,
fall back to the prebuilt workspace authorization check. Since prebuilt
workspaces are a subset of workspaces, the normal workspace check is
more likely to succeed. This is a small optimization to reduce
unnecessary prebuilt authorization calls.
This commit is contained in:
Susana Ferreira
2025-06-24 16:33:21 +01:00
committed by GitHub
parent 341b54e604
commit f44969b689
5 changed files with 55 additions and 29 deletions
+14 -12
View File
@@ -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{}
+22 -2
View File
@@ -5650,7 +5650,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{})
@@ -5679,6 +5689,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
View File
@@ -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.
+7 -8
View File
@@ -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),
)
+5 -7
View File
@@ -1049,14 +1049,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 {