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

Commit38124bd

Browse files
committed
fix: creating workspaces should not require site wide permissions
Creating a workspace requires `read` on site wide `user`.Added unit tests to assert this
1 parent44ddc9f commit38124bd

File tree

5 files changed

+198
-59
lines changed

5 files changed

+198
-59
lines changed

‎coderd/coderd.go

Lines changed: 63 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,64 +1147,74 @@ func New(options *Options) *API {
11471147
r.Get("/",api.AssignableSiteRoles)
11481148
})
11491149
r.Route("/{user}",func(r chi.Router) {
1150-
r.Use(httpmw.ExtractUserParam(options.Database))
1151-
r.Post("/convert-login",api.postConvertLoginType)
1152-
r.Delete("/",api.deleteUser)
1153-
r.Get("/",api.userByName)
1154-
r.Get("/autofill-parameters",api.userAutofillParameters)
1155-
r.Get("/login-type",api.userLoginType)
1156-
r.Put("/profile",api.putUserProfile)
1157-
r.Route("/status",func(r chi.Router) {
1158-
r.Put("/suspend",api.putSuspendUserAccount())
1159-
r.Put("/activate",api.putActivateUserAccount())
1150+
r.Group(func(r chi.Router) {
1151+
r.Use(httpmw.ExtractUserParam(options.Database))
1152+
// Creating workspaces does not require permissions on the user, only the
1153+
// organization member. This endpoint should match the authz story of
1154+
// postWorkspacesByOrganization
1155+
r.Post("/workspaces",api.postUserWorkspaces)
11601156
})
1161-
r.Get("/appearance",api.userAppearanceSettings)
1162-
r.Put("/appearance",api.putUserAppearanceSettings)
1163-
r.Route("/password",func(r chi.Router) {
1164-
r.Use(httpmw.RateLimit(options.LoginRateLimit,time.Minute))
1165-
r.Put("/",api.putUserPassword)
1166-
})
1167-
// These roles apply to the site wide permissions.
1168-
r.Put("/roles",api.putUserRoles)
1169-
r.Get("/roles",api.userRoles)
1170-
1171-
r.Route("/keys",func(r chi.Router) {
1172-
r.Post("/",api.postAPIKey)
1173-
r.Route("/tokens",func(r chi.Router) {
1174-
r.Post("/",api.postToken)
1175-
r.Get("/",api.tokens)
1176-
r.Get("/tokenconfig",api.tokenConfig)
1177-
r.Route("/{keyname}",func(r chi.Router) {
1178-
r.Get("/",api.apiKeyByName)
1179-
})
1157+
1158+
r.Group(func(r chi.Router) {
1159+
r.Use(httpmw.ExtractUserParam(options.Database))
1160+
1161+
r.Post("/convert-login",api.postConvertLoginType)
1162+
r.Delete("/",api.deleteUser)
1163+
r.Get("/",api.userByName)
1164+
r.Get("/autofill-parameters",api.userAutofillParameters)
1165+
r.Get("/login-type",api.userLoginType)
1166+
r.Put("/profile",api.putUserProfile)
1167+
r.Route("/status",func(r chi.Router) {
1168+
r.Put("/suspend",api.putSuspendUserAccount())
1169+
r.Put("/activate",api.putActivateUserAccount())
11801170
})
1181-
r.Route("/{keyid}",func(r chi.Router) {
1182-
r.Get("/",api.apiKeyByID)
1183-
r.Delete("/",api.deleteAPIKey)
1171+
r.Get("/appearance",api.userAppearanceSettings)
1172+
r.Put("/appearance",api.putUserAppearanceSettings)
1173+
r.Route("/password",func(r chi.Router) {
1174+
r.Use(httpmw.RateLimit(options.LoginRateLimit,time.Minute))
1175+
r.Put("/",api.putUserPassword)
1176+
})
1177+
// These roles apply to the site wide permissions.
1178+
r.Put("/roles",api.putUserRoles)
1179+
r.Get("/roles",api.userRoles)
1180+
1181+
r.Route("/keys",func(r chi.Router) {
1182+
r.Post("/",api.postAPIKey)
1183+
r.Route("/tokens",func(r chi.Router) {
1184+
r.Post("/",api.postToken)
1185+
r.Get("/",api.tokens)
1186+
r.Get("/tokenconfig",api.tokenConfig)
1187+
r.Route("/{keyname}",func(r chi.Router) {
1188+
r.Get("/",api.apiKeyByName)
1189+
})
1190+
})
1191+
r.Route("/{keyid}",func(r chi.Router) {
1192+
r.Get("/",api.apiKeyByID)
1193+
r.Delete("/",api.deleteAPIKey)
1194+
})
11841195
})
1185-
})
11861196

1187-
r.Route("/organizations",func(r chi.Router) {
1188-
r.Get("/",api.organizationsByUser)
1189-
r.Get("/{organizationname}",api.organizationByUserAndName)
1190-
})
1191-
r.Post("/workspaces",api.postUserWorkspaces)
1192-
r.Route("/workspace/{workspacename}",func(r chi.Router) {
1193-
r.Get("/",api.workspaceByOwnerAndName)
1194-
r.Get("/builds/{buildnumber}",api.workspaceBuildByBuildNumber)
1195-
})
1196-
r.Get("/gitsshkey",api.gitSSHKey)
1197-
r.Put("/gitsshkey",api.regenerateGitSSHKey)
1198-
r.Route("/notifications",func(r chi.Router) {
1199-
r.Route("/preferences",func(r chi.Router) {
1200-
r.Get("/",api.userNotificationPreferences)
1201-
r.Put("/",api.putUserNotificationPreferences)
1197+
r.Route("/organizations",func(r chi.Router) {
1198+
r.Get("/",api.organizationsByUser)
1199+
r.Get("/{organizationname}",api.organizationByUserAndName)
1200+
})
1201+
r.Route("/workspace/{workspacename}",func(r chi.Router) {
1202+
r.Get("/",api.workspaceByOwnerAndName)
1203+
r.Get("/builds/{buildnumber}",api.workspaceBuildByBuildNumber)
1204+
})
1205+
r.Get("/gitsshkey",api.gitSSHKey)
1206+
r.Put("/gitsshkey",api.regenerateGitSSHKey)
1207+
r.Route("/notifications",func(r chi.Router) {
1208+
r.Route("/preferences",func(r chi.Router) {
1209+
r.Get("/",api.userNotificationPreferences)
1210+
r.Put("/",api.putUserNotificationPreferences)
1211+
})
1212+
})
1213+
r.Route("/webpush",func(r chi.Router) {
1214+
r.Post("/subscription",api.postUserWebpushSubscription)
1215+
r.Delete("/subscription",api.deleteUserWebpushSubscription)
1216+
r.Post("/test",api.postUserPushNotificationTest)
12021217
})
1203-
})
1204-
r.Route("/webpush",func(r chi.Router) {
1205-
r.Post("/subscription",api.postUserWebpushSubscription)
1206-
r.Delete("/subscription",api.deleteUserWebpushSubscription)
1207-
r.Post("/test",api.postUserPushNotificationTest)
12081218
})
12091219
})
12101220
})

‎coderd/coderdtest/authorize.go

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func AssertRBAC(t *testing.T, api *coderd.API, client *codersdk.Client) RBACAsse
8181
// Note that duplicate rbac calls are handled by the rbac.Cacher(), but
8282
// will be recorded twice. So AllCalls() returns calls regardless if they
8383
// were returned from the cached or not.
84-
func (aRBACAsserter)AllCalls()[]AuthCall {
84+
func (aRBACAsserter)AllCalls()AuthCalls {
8585
returna.Recorder.AllCalls(&a.Subject)
8686
}
8787

@@ -140,9 +140,27 @@ func (a RBACAsserter) Reset() RBACAsserter {
140140
returna
141141
}
142142

143+
typeAuthCalls []AuthCall
144+
145+
func (cAuthCalls)Mutate(mutfunc(cAuthCall)AuthCall)AuthCalls {
146+
fori:=rangec {
147+
c[i]=mut(c[i])
148+
}
149+
returnc
150+
}
151+
152+
func (cAuthCalls)String()string {
153+
varstr strings.Builder
154+
for_,call:=rangec {
155+
str.WriteString(fmt.Sprintf("%s: %s.%s\n",call.Actor.FriendlyName,call.Action,call.Object.Type))
156+
}
157+
returnstr.String()
158+
}
159+
143160
typeAuthCallstruct {
144161
rbac.AuthCall
145162

163+
errerror
146164
assertedbool
147165
// callers is a small stack trace for debugging.
148166
callers []string
@@ -252,7 +270,7 @@ func (r *RecordingAuthorizer) AssertActor(t *testing.T, actor rbac.Subject, did
252270
}
253271

254272
// recordAuthorize is the internal method that records the Authorize() call.
255-
func (r*RecordingAuthorizer)recordAuthorize(subject rbac.Subject,action policy.Action,object rbac.Object) {
273+
func (r*RecordingAuthorizer)recordAuthorize(subject rbac.Subject,action policy.Action,object rbac.Object,errerror) {
256274
r.Lock()
257275
deferr.Unlock()
258276

@@ -262,6 +280,7 @@ func (r *RecordingAuthorizer) recordAuthorize(subject rbac.Subject, action polic
262280
Action:action,
263281
Object:object,
264282
},
283+
err:err,
265284
callers: []string{
266285
// This is a decent stack trace for debugging.
267286
// Some dbauthz calls are a bit nested, so we skip a few.
@@ -288,11 +307,12 @@ func caller(skip int) string {
288307
}
289308

290309
func (r*RecordingAuthorizer)Authorize(ctx context.Context,subject rbac.Subject,action policy.Action,object rbac.Object)error {
291-
r.recordAuthorize(subject,action,object)
292310
ifr.Wrapped==nil {
293311
panic("Developer error: RecordingAuthorizer.Wrapped is nil")
294312
}
295-
returnr.Wrapped.Authorize(ctx,subject,action,object)
313+
err:=r.Wrapped.Authorize(ctx,subject,action,object)
314+
r.recordAuthorize(subject,action,object,err)
315+
returnerr
296316
}
297317

298318
func (r*RecordingAuthorizer)Prepare(ctx context.Context,subject rbac.Subject,action policy.Action,objectTypestring) (rbac.PreparedAuthorized,error) {
@@ -339,10 +359,11 @@ func (s *PreparedRecorder) Authorize(ctx context.Context, object rbac.Object) er
339359
s.rw.Lock()
340360
defers.rw.Unlock()
341361

362+
err:=s.prepped.Authorize(ctx,object)
342363
if!s.usingSQL {
343-
s.rec.recordAuthorize(s.subject,s.action,object)
364+
s.rec.recordAuthorize(s.subject,s.action,object,err)
344365
}
345-
returns.prepped.Authorize(ctx,object)
366+
returnerr
346367
}
347368

348369
func (s*PreparedRecorder)CompileToSQL(ctx context.Context,cfg regosql.ConvertConfig) (string,error) {

‎coderd/rbac/object.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
package rbac
22

33
import (
4+
"fmt"
5+
"strings"
6+
47
"github.com/google/uuid"
58
"golang.org/x/xerrors"
69

710
"github.com/coder/coder/v2/coderd/rbac/policy"
11+
cstrings"github.com/coder/coder/v2/coderd/util/strings"
812
)
913

1014
// ResourceUserObject is a helper function to create a user object for authz checks.
@@ -37,6 +41,25 @@ type Object struct {
3741
ACLGroupListmap[string][]policy.Action` json:"acl_group_list"`
3842
}
3943

44+
// String is not perfect, but decent enough for human display
45+
func (zObject)String()string {
46+
varparts []string
47+
ifz.OrgID!="" {
48+
parts=append(parts,fmt.Sprintf("org:%s",cstrings.Truncate(z.OrgID,4)))
49+
}
50+
ifz.Owner!="" {
51+
parts=append(parts,fmt.Sprintf("owner:%s",cstrings.Truncate(z.Owner,4)))
52+
}
53+
parts=append(parts,z.Type)
54+
ifz.ID!="" {
55+
parts=append(parts,fmt.Sprintf("id:%s",cstrings.Truncate(z.ID,4)))
56+
}
57+
iflen(z.ACLGroupList)>0||len(z.ACLUserList)>0 {
58+
parts=append(parts,fmt.Sprintf("acl:%d",len(z.ACLUserList)+len(z.ACLGroupList)))
59+
}
60+
returnstrings.Join(parts,".")
61+
}
62+
4063
// ValidAction checks if the action is valid for the given object type.
4164
func (zObject)ValidAction(action policy.Action)error {
4265
perms,ok:=policy.RBACPermissions[z.Type]

‎coderd/workspaces_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,42 @@ func TestWorkspace(t *testing.T) {
423423
require.ErrorAs(t,err,&apiError)
424424
require.Equal(t,http.StatusForbidden,apiError.StatusCode())
425425
})
426+
427+
// Asserting some authz calls when creating a workspace.
428+
t.Run("AuthzStory",func(t*testing.T) {
429+
t.Parallel()
430+
owner,_,api:=coderdtest.NewWithAPI(t,&coderdtest.Options{IncludeProvisionerDaemon:true})
431+
first:=coderdtest.CreateFirstUser(t,owner)
432+
authz:=coderdtest.AssertRBAC(t,api,owner)
433+
434+
version:=coderdtest.CreateTemplateVersion(t,owner,first.OrganizationID,nil)
435+
coderdtest.AwaitTemplateVersionJobCompleted(t,owner,version.ID)
436+
template:=coderdtest.CreateTemplate(t,owner,first.OrganizationID,version.ID)
437+
_,userID:=coderdtest.CreateAnotherUser(t,owner,first.OrganizationID)
438+
439+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
440+
defercancel()
441+
442+
// Create a workspace with the current api.
443+
authz.Reset()// Reset all previous checks done in setup.
444+
_,err:=owner.CreateUserWorkspace(ctx,userID.ID.String(), codersdk.CreateWorkspaceRequest{
445+
TemplateID:template.ID,
446+
Name:"test-user",
447+
})
448+
require.NoError(t,err)
449+
450+
// Assert all authz properties
451+
t.Run("OnlyOrganizationAuthzCalls",func(t*testing.T) {
452+
// Creating workspaces is an organization action. So organization
453+
// permissions should be sufficient to complete the action.
454+
for_,call:=rangeauthz.AllCalls() {
455+
assert.Falsef(t,call.Object.OrgID=="",
456+
"call %q for object %q has no organization set. Site authz calls not expected here",
457+
call.Action,call.Object.String(),
458+
)
459+
}
460+
})
461+
})
426462
}
427463

428464
funcTestResolveAutostart(t*testing.T) {

‎enterprise/coderd/workspaces_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,55 @@ func TestCreateWorkspace(t *testing.T) {
245245
funcTestCreateUserWorkspace(t*testing.T) {
246246
t.Parallel()
247247

248+
// Create a custom role that can create workspaces for another user.
249+
t.Run("ForAnotherUser",func(t*testing.T) {
250+
t.Parallel()
251+
252+
owner,first:=coderdenttest.New(t,&coderdenttest.Options{
253+
Options:&coderdtest.Options{
254+
IncludeProvisionerDaemon:true,
255+
},
256+
LicenseOptions:&coderdenttest.LicenseOptions{
257+
Features: license.Features{
258+
codersdk.FeatureCustomRoles:1,
259+
codersdk.FeatureTemplateRBAC:1,
260+
},
261+
},
262+
})
263+
ctx:=testutil.Context(t,testutil.WaitShort)
264+
r,err:=owner.CreateOrganizationRole(ctx, codersdk.Role{
265+
Name:"creator",
266+
OrganizationID:first.OrganizationID.String(),
267+
DisplayName:"Creator",
268+
OrganizationPermissions:codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
269+
codersdk.ResourceWorkspace: {codersdk.ActionCreate},
270+
}),
271+
})
272+
require.NoError(t,err)
273+
274+
// use admin for setting up test
275+
admin,adminID:=coderdtest.CreateAnotherUser(t,owner,first.OrganizationID,rbac.RoleTemplateAdmin())
276+
277+
// try the test action with this user & custom role
278+
creator,_:=coderdtest.CreateAnotherUser(t,owner,first.OrganizationID,rbac.RoleMember(), rbac.RoleIdentifier{
279+
Name:r.Name,
280+
OrganizationID:first.OrganizationID,
281+
})
282+
283+
version:=coderdtest.CreateTemplateVersion(t,admin,first.OrganizationID,nil)
284+
coderdtest.AwaitTemplateVersionJobCompleted(t,admin,version.ID)
285+
template:=coderdtest.CreateTemplate(t,admin,first.OrganizationID,version.ID)
286+
287+
ctx=testutil.Context(t,testutil.WaitLong*1000)// Reset the context to avoid timeouts.
288+
289+
var_=creator
290+
_,err=owner.CreateUserWorkspace(ctx,adminID.ID.String(), codersdk.CreateWorkspaceRequest{
291+
TemplateID:template.ID,
292+
Name:"workspace",
293+
})
294+
require.NoError(t,err)
295+
})
296+
248297
t.Run("NoTemplateAccess",func(t*testing.T) {
249298
t.Parallel()
250299

0 commit comments

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