Skip to content

Commit b2c7bdd

Browse files
ssncferreirassncferreira
committed
chore: improve code quality and add clarifying comments
1 parent afc5359 commit b2c7bdd

File tree

6 files changed

+54
-33
lines changed

6 files changed

+54
-33
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,30 @@ func (q *querier) authorizeContext(ctx context.Context, action policy.Action, ob
150150
return nil
151151
}
152152

153+
// authorizeWorkspace handles authorization for workspace resource types.
154+
// prebuilt_workspaces are a subset of workspaces, currently limited to
155+
// supporting delete operations. Therefore, if the action is delete or
156+
// update and the workspace is a prebuild, a prebuilt-specific authorization
157+
// is attempted first. If that fails, it falls back to normal workspace
158+
// authorization.
159+
// Note: Delete operations of workspaces requires both update and delete
160+
// permissions.
161+
func (q *querier) authorizeWorkspace(ctx context.Context, action policy.Action, workspace database.Workspace) error {
162+
var prebuiltErr error
163+
// Special handling for prebuilt_workspace deletion authorization check
164+
if (action == policy.ActionUpdate || action == policy.ActionDelete) && workspace.IsPrebuild() {
165+
// Try prebuilt-specific authorization first
166+
if prebuiltErr = q.authorizeContext(ctx, action, workspace.AsPrebuild()); prebuiltErr == nil {
167+
return nil
168+
}
169+
}
170+
// Fallback to normal workspace authorization check
171+
if err := q.authorizeContext(ctx, action, workspace); err != nil {
172+
return xerrors.Errorf("authorize context: %w", errors.Join(prebuiltErr, err))
173+
}
174+
return nil
175+
}
176+
153177
type authContextKey struct{}
154178

155179
// ActorFromContext returns the authorization subject from the context.
@@ -3915,15 +3939,9 @@ func (q *querier) InsertWorkspaceBuild(ctx context.Context, arg database.InsertW
39153939
action = policy.ActionWorkspaceStop
39163940
}
39173941

3918-
if action == policy.ActionDelete && w.IsPrebuild() {
3919-
if err := q.authorizeContext(ctx, action, w.PrebuildRBAC()); err != nil {
3920-
// Fallback to normal workspace auth check
3921-
if err = q.authorizeContext(ctx, action, w); err != nil {
3922-
return xerrors.Errorf("authorize context: %w", err)
3923-
}
3924-
}
3925-
} else if err = q.authorizeContext(ctx, action, w); err != nil {
3926-
return xerrors.Errorf("authorize context: %w", err)
3942+
// Special handling for prebuilt workspace deletion
3943+
if err := q.authorizeWorkspace(ctx, action, w); err != nil {
3944+
return err
39273945
}
39283946

39293947
// If we're starting a workspace we need to check the template.
@@ -3962,16 +3980,8 @@ func (q *querier) InsertWorkspaceBuildParameters(ctx context.Context, arg databa
39623980
return err
39633981
}
39643982

3965-
if workspace.IsPrebuild() {
3966-
err = q.authorizeContext(ctx, policy.ActionUpdate, workspace.PrebuildRBAC())
3967-
// Fallback to normal workspace auth check
3968-
if err != nil {
3969-
err = q.authorizeContext(ctx, policy.ActionUpdate, workspace)
3970-
}
3971-
} else {
3972-
err = q.authorizeContext(ctx, policy.ActionUpdate, workspace)
3973-
}
3974-
if err != nil {
3983+
// Special handling for prebuilt workspace deletion
3984+
if err := q.authorizeWorkspace(ctx, policy.ActionUpdate, workspace); err != nil {
39753985
return err
39763986
}
39773987

‎coderd/database/modelmethods.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,17 +227,22 @@ func (w Workspace) WorkspaceTable() WorkspaceTable {
227227

228228
func (w Workspace) RBACObject() rbac.Object {
229229
// if w.IsPrebuild() {
230-
// return w.PrebuildRBAC()
230+
// return w.AsPrebuild()
231231
//}
232232
return w.WorkspaceTable().RBACObject()
233233
}
234234

235+
// IsPrebuild returns true if the workspace is a prebuild workspace.
236+
// A workspace is considered a prebuild if its owner is the prebuild system user.
235237
func (w Workspace) IsPrebuild() bool {
236238
// TODO: avoid import cycle
237239
return w.OwnerID == uuid.MustParse("c42fdf75-3097-471c-8c33-fb52454d81c0")
238240
}
239241

240-
func (w Workspace) PrebuildRBAC() rbac.Object {
242+
// AsPrebuild returns the RBAC object corresponding to the workspace type.
243+
// If the workspace is a prebuild, it returns a prebuilt_workspace RBAC object.
244+
// Otherwise, it returns a normal workspace RBAC object.
245+
func (w Workspace) AsPrebuild() rbac.Object {
241246
if w.IsPrebuild() {
242247
return rbac.ResourcePrebuiltWorkspace.WithID(w.ID).
243248
InOrg(w.OrganizationID).

‎coderd/rbac/object_gen.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/rbac/policy/policy.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,15 @@ var RBACPermissions = map[string]PermissionDefinition{
103103
Actions: workspaceActions,
104104
},
105105
"prebuilt_workspace": {
106-
Actions: map[Action]ActionDefinition{
107-
ActionRead: actDef("read prebuilt workspace"),
108-
ActionUpdate: actDef("update prebuilt workspace"),
106+
// Prebuilt_workspace actions currently apply only to delete operations.
107+
// To successfully delete a workspace, a user must have the following permissions:
108+
// * read: to read the current workspace state
109+
// * update: to modify workspace metadata and related resources during deletion
110+
// (e.g., updating the deleted field in the database)
111+
// * delete: to perform the actual deletion of the workspace
112+
Actions: map[Action]ActionDefinition{
113+
ActionRead: actDef("read prebuilt workspace data"),
114+
ActionUpdate: actDef("update prebuilt workspace settings"),
109115
ActionDelete: actDef("delete prebuilt workspace"),
110116
},
111117
},

‎coderd/workspacebuilds.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -404,16 +404,16 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
404404
ctx,
405405
tx,
406406
func(action policy.Action, object rbac.Objecter) bool {
407+
// Special handling for prebuilt workspace deletion
407408
if object.RBACObject().Type == rbac.ResourceWorkspace.Type && action == policy.ActionDelete {
408-
workspaceObj, ok := object.(database.Workspace)
409-
if ok {
410-
prebuild := workspaceObj.PrebuildRBAC()
411-
// Fallback to normal workspace auth check
412-
if auth := api.Authorize(r, action, prebuild); auth {
409+
if workspaceObj, ok := object.(database.Workspace); ok {
410+
// Try prebuilt-specific authorization first
411+
if auth := api.Authorize(r, action, workspaceObj.AsPrebuild()); auth {
413412
return auth
414413
}
415414
}
416415
}
416+
// Fallback to default authorization
417417
return api.Authorize(r, action, object)
418418
},
419419
audit.WorkspaceBuildBaggageFromRequest(r),

‎site/src/api/rbacresourcesGenerated.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,8 @@ export const RBACResourceActions: Partial<
125125
},
126126
prebuilt_workspace: {
127127
delete: "delete prebuilt workspace",
128-
read: "read prebuilt workspace",
129-
update: "update prebuilt workspace",
128+
read: "read prebuilt workspace data",
129+
update: "update prebuilt workspace settings",
130130
},
131131
provisioner_daemon: {
132132
create: "create a provisioner daemon/key",

0 commit comments

Comments
 (0)