Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitf44969b

Browse files
authored
chore: reorder prebuilt workspace authorization logic (#18506)
## DescriptionFollow-up from PR#18333Related with:#18333 (comment)This changes the authorization logic to first try the normal workspaceauthorization check, and only if the resource is a prebuilt workspace,fall back to the prebuilt workspace authorization check. Since prebuiltworkspaces are a subset of workspaces, the normal workspace check ismore likely to succeed. This is a small optimization to reduceunnecessary prebuilt authorization calls.
1 parent341b54e commitf44969b

File tree

5 files changed

+55
-29
lines changed

5 files changed

+55
-29
lines changed

‎coderd/database/dbauthz/dbauthz.go‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -151,26 +151,28 @@ func (q *querier) authorizeContext(ctx context.Context, action policy.Action, ob
151151

152152
// authorizePrebuiltWorkspace handles authorization for workspace resource types.
153153
// prebuilt_workspaces are a subset of workspaces, currently limited to
154-
// supporting delete operations. Therefore, if the action is delete or
155-
// update and the workspace is a prebuild, a prebuilt-specific authorization
156-
// is attempted first. If that fails, it falls back to normal workspace
157-
// authorization.
154+
// supporting delete operations. This function first attempts normal workspace
155+
// authorization. If that fails, the action is delete or update and the workspace
156+
// is a prebuild, a prebuilt-specific authorization is attempted.
158157
// Note: Delete operations of workspaces requires both update and delete
159158
// permissions.
160159
func (q*querier)authorizePrebuiltWorkspace(ctx context.Context,action policy.Action,workspace database.Workspace)error {
161-
varprebuiltErrerror
162-
// Special handling for prebuilt_workspace deletion authorization check
160+
// Try default workspace authorization first
161+
varworkspaceErrerror
162+
ifworkspaceErr=q.authorizeContext(ctx,action,workspace);workspaceErr==nil {
163+
returnnil
164+
}
165+
166+
// Special handling for prebuilt workspace deletion
163167
if (action==policy.ActionUpdate||action==policy.ActionDelete)&&workspace.IsPrebuild() {
164-
// Try prebuilt-specific authorization first
168+
varprebuiltErrerror
165169
ifprebuiltErr=q.authorizeContext(ctx,action,workspace.AsPrebuild());prebuiltErr==nil {
166170
returnnil
167171
}
172+
returnxerrors.Errorf("authorize context failed for workspace (%v) and prebuilt (%w)",workspaceErr,prebuiltErr)
168173
}
169-
// Fallback to normal workspace authorization check
170-
iferr:=q.authorizeContext(ctx,action,workspace);err!=nil {
171-
returnxerrors.Errorf("authorize context: %w",errors.Join(prebuiltErr,err))
172-
}
173-
returnnil
174+
175+
returnxerrors.Errorf("authorize context: %w",workspaceErr)
174176
}
175177

176178
typeauthContextKeystruct{}

‎coderd/database/dbauthz/dbauthz_test.go‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5650,7 +5650,17 @@ func (s *MethodTestSuite) TestAuthorizePrebuiltWorkspace() {
56505650
Reason:database.BuildReasonInitiator,
56515651
TemplateVersionID:tv.ID,
56525652
JobID:pj.ID,
5653-
}).Asserts(w.AsPrebuild(),policy.ActionDelete)
5653+
}).
5654+
// Simulate a fallback authorization flow:
5655+
// - First, the default workspace authorization fails (simulated by returning an error).
5656+
// - Then, authorization is retried using the prebuilt workspace object, which succeeds.
5657+
// The test asserts that both authorization attempts occur in the correct order.
5658+
WithSuccessAuthorizer(func(ctx context.Context,subject rbac.Subject,action policy.Action,obj rbac.Object)error {
5659+
ifobj.Type==rbac.ResourceWorkspace.Type {
5660+
returnxerrors.Errorf("not authorized for workspace type")
5661+
}
5662+
returnnil
5663+
}).Asserts(w,policy.ActionDelete,w.AsPrebuild(),policy.ActionDelete)
56545664
}))
56555665
s.Run("PrebuildUpdate/InsertWorkspaceBuildParameters",s.Subtest(func(db database.Store,check*expects) {
56565666
u:=dbgen.User(s.T(),db, database.User{})
@@ -5679,6 +5689,16 @@ func (s *MethodTestSuite) TestAuthorizePrebuiltWorkspace() {
56795689
})
56805690
check.Args(database.InsertWorkspaceBuildParametersParams{
56815691
WorkspaceBuildID:wb.ID,
5682-
}).Asserts(w.AsPrebuild(),policy.ActionUpdate)
5692+
}).
5693+
// Simulate a fallback authorization flow:
5694+
// - First, the default workspace authorization fails (simulated by returning an error).
5695+
// - Then, authorization is retried using the prebuilt workspace object, which succeeds.
5696+
// The test asserts that both authorization attempts occur in the correct order.
5697+
WithSuccessAuthorizer(func(ctx context.Context,subject rbac.Subject,action policy.Action,obj rbac.Object)error {
5698+
ifobj.Type==rbac.ResourceWorkspace.Type {
5699+
returnxerrors.Errorf("not authorized for workspace type")
5700+
}
5701+
returnnil
5702+
}).Asserts(w,policy.ActionUpdate,w.AsPrebuild(),policy.ActionUpdate)
56835703
}))
56845704
}

‎coderd/database/modelmethods.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,13 @@ func (gm GroupMember) RBACObject() rbac.Object {
199199
returnrbac.ResourceGroupMember.WithID(gm.UserID).InOrg(gm.OrganizationID).WithOwner(gm.UserID.String())
200200
}
201201

202+
// PrebuiltWorkspaceResource defines the interface for types that can be identified as prebuilt workspaces
203+
// and converted to their corresponding prebuilt workspace RBAC object.
204+
typePrebuiltWorkspaceResourceinterface {
205+
IsPrebuild()bool
206+
AsPrebuild() rbac.Object
207+
}
208+
202209
// WorkspaceTable converts a Workspace to it's reduced version.
203210
// A more generalized solution is to use json marshaling to
204211
// consistently keep these two structs in sync.

‎coderd/workspacebuilds.go‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -391,17 +391,16 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
391391
tx,
392392
api.FileCache,
393393
func(action policy.Action,object rbac.Objecter)bool {
394+
ifauth:=api.Authorize(r,action,object);auth {
395+
returntrue
396+
}
394397
// Special handling for prebuilt workspace deletion
395-
ifobject.RBACObject().Type==rbac.ResourceWorkspace.Type&&action==policy.ActionDelete {
396-
ifworkspaceObj,ok:=object.(database.Workspace);ok {
397-
// Try prebuilt-specific authorization first
398-
ifauth:=api.Authorize(r,action,workspaceObj.AsPrebuild());auth {
399-
returnauth
400-
}
398+
ifaction==policy.ActionDelete {
399+
ifworkspaceObj,ok:=object.(database.PrebuiltWorkspaceResource);ok&&workspaceObj.IsPrebuild() {
400+
returnapi.Authorize(r,action,workspaceObj.AsPrebuild())
401401
}
402402
}
403-
// Fallback to default authorization
404-
returnapi.Authorize(r,action,object)
403+
returnfalse
405404
},
406405
audit.WorkspaceBuildBaggageFromRequest(r),
407406
)

‎coderd/wsbuilder/wsbuilder.go‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,14 +1049,12 @@ func (b *Builder) authorize(authFunc func(action policy.Action, object rbac.Obje
10491049
returnBuildError{http.StatusBadRequest,msg,xerrors.New(msg)}
10501050
}
10511051

1052+
// Try default workspace authorization first
1053+
authorized:=authFunc(action,b.workspace)
1054+
10521055
// Special handling for prebuilt workspace deletion
1053-
authorized:=false
1054-
ifaction==policy.ActionDelete&&b.workspace.IsPrebuild()&&authFunc(action,b.workspace.AsPrebuild()) {
1055-
authorized=true
1056-
}
1057-
// Fallback to default authorization
1058-
if!authorized&&authFunc(action,b.workspace) {
1059-
authorized=true
1056+
if!authorized&&action==policy.ActionDelete&&b.workspace.IsPrebuild() {
1057+
authorized=authFunc(action,b.workspace.AsPrebuild())
10601058
}
10611059

10621060
if!authorized {

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp