diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 3c6f691616d52..c0da1abd7db42 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -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{} diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index f5077b7bdcda9..95d1d22b64044 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -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{}) @@ -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) })) } diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index cb16d8c4995b6..725e45c268d72 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -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. diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index 850e9dd457dd8..c3c4aba0c378c 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -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), ) diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index f327e071c3946..3076d2056fdca 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -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 {