chore: reorder prebuilt workspace authorization logic

ssncferreira · ssncferreira · commit a4b2e8d97c91 · 2025-06-23T16:25:20.000Z
diff --git 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: %w", errors.Join(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", errors.Join(workspaceErr))
 }
 
 type authContextKey struct{}
diff --git 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/workspacebuilds.go b/coderd/workspacebuilds.go
@@ -393,17 +393,16 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
 			ctx,
 			tx,
 			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 workspaceObj, ok := object.(database.Workspace); 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
@@ -963,14 +963,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 {
