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

Commitd5360a6

Browse files
authored
chore: fetch workspaces by username with organization permissions (#17707)
Closes#17691`ExtractOrganizationMembersParam` will allow fetching a user with onlyorganization permissions. If the user belongs to 0 orgs, then the user "does not exist" from an org perspective. But if you are a site-wide admin, then the user does exist.
1 parentd93a9cf commitd5360a6

File tree

6 files changed

+185
-74
lines changed

6 files changed

+185
-74
lines changed

‎coderd/coderd.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,26 +1189,32 @@ func New(options *Options) *API {
11891189
})
11901190
r.Route("/{user}",func(r chi.Router) {
11911191
r.Group(func(r chi.Router) {
1192-
r.Use(httpmw.ExtractUserParamOptional(options.Database))
1192+
r.Use(httpmw.ExtractOrganizationMembersParam(options.Database,api.HTTPAuth.Authorize))
11931193
// Creating workspaces does not require permissions on the user, only the
11941194
// organization member. This endpoint should match the authz story of
11951195
// postWorkspacesByOrganization
11961196
r.Post("/workspaces",api.postUserWorkspaces)
1197+
r.Route("/workspace/{workspacename}",func(r chi.Router) {
1198+
r.Get("/",api.workspaceByOwnerAndName)
1199+
r.Get("/builds/{buildnumber}",api.workspaceBuildByBuildNumber)
1200+
})
1201+
})
1202+
1203+
r.Group(func(r chi.Router) {
1204+
r.Use(httpmw.ExtractUserParam(options.Database))
11971205

11981206
// Similarly to creating a workspace, evaluating parameters for a
11991207
// new workspace should also match the authz story of
12001208
// postWorkspacesByOrganization
1209+
// TODO: Do not require site wide read user permission. Make this work
1210+
// with org member permissions.
12011211
r.Route("/templateversions/{templateversion}",func(r chi.Router) {
12021212
r.Use(
12031213
httpmw.ExtractTemplateVersionParam(options.Database),
12041214
httpmw.RequireExperiment(api.Experiments,codersdk.ExperimentDynamicParameters),
12051215
)
12061216
r.Get("/parameters",api.templateVersionDynamicParameters)
12071217
})
1208-
})
1209-
1210-
r.Group(func(r chi.Router) {
1211-
r.Use(httpmw.ExtractUserParam(options.Database))
12121218

12131219
r.Post("/convert-login",api.postConvertLoginType)
12141220
r.Delete("/",api.deleteUser)
@@ -1250,10 +1256,7 @@ func New(options *Options) *API {
12501256
r.Get("/",api.organizationsByUser)
12511257
r.Get("/{organizationname}",api.organizationByUserAndName)
12521258
})
1253-
r.Route("/workspace/{workspacename}",func(r chi.Router) {
1254-
r.Get("/",api.workspaceByOwnerAndName)
1255-
r.Get("/builds/{buildnumber}",api.workspaceBuildByBuildNumber)
1256-
})
1259+
12571260
r.Get("/gitsshkey",api.gitSSHKey)
12581261
r.Put("/gitsshkey",api.regenerateGitSSHKey)
12591262
r.Route("/notifications",func(r chi.Router) {

‎coderd/httpmw/organizationparam.go

Lines changed: 128 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,15 @@ import (
1111
"github.com/coder/coder/v2/coderd/database"
1212
"github.com/coder/coder/v2/coderd/database/dbauthz"
1313
"github.com/coder/coder/v2/coderd/httpapi"
14+
"github.com/coder/coder/v2/coderd/rbac"
15+
"github.com/coder/coder/v2/coderd/rbac/policy"
1416
"github.com/coder/coder/v2/codersdk"
1517
)
1618

1719
type (
18-
organizationParamContextKeystruct{}
19-
organizationMemberParamContextKeystruct{}
20+
organizationParamContextKeystruct{}
21+
organizationMemberParamContextKeystruct{}
22+
organizationMembersParamContextKeystruct{}
2023
)
2124

2225
// OrganizationParam returns the organization from the ExtractOrganizationParam handler.
@@ -38,6 +41,14 @@ func OrganizationMemberParam(r *http.Request) OrganizationMember {
3841
returnorganizationMember
3942
}
4043

44+
funcOrganizationMembersParam(r*http.Request)OrganizationMembers {
45+
organizationMembers,ok:=r.Context().Value(organizationMembersParamContextKey{}).(OrganizationMembers)
46+
if!ok {
47+
panic("developer error: organization members param middleware not provided")
48+
}
49+
returnorganizationMembers
50+
}
51+
4152
// ExtractOrganizationParam grabs an organization from the "organization" URL parameter.
4253
// This middleware requires the API key middleware higher in the call stack for authentication.
4354
funcExtractOrganizationParam(db database.Store)func(http.Handler) http.Handler {
@@ -111,35 +122,23 @@ func ExtractOrganizationMemberParam(db database.Store) func(http.Handler) http.H
111122
returnfunc(next http.Handler) http.Handler {
112123
returnhttp.HandlerFunc(func(rw http.ResponseWriter,r*http.Request) {
113124
ctx:=r.Context()
114-
// We need to resolve the `{user}` URL parameter so that we can get the userID and
115-
// username. We do this as SystemRestricted since the caller might have permission
116-
// to access the OrganizationMember object, but *not* the User object. So, it is
117-
// very important that we do not add the User object to the request context or otherwise
118-
// leak it to the API handler.
119-
// nolint:gocritic
120-
user,ok:=ExtractUserContext(dbauthz.AsSystemRestricted(ctx),db,rw,r)
121-
if!ok {
122-
return
123-
}
124125
organization:=OrganizationParam(r)
125-
126-
organizationMember,err:=database.ExpectOne(db.OrganizationMembers(ctx, database.OrganizationMembersParams{
127-
OrganizationID:organization.ID,
128-
UserID:user.ID,
129-
IncludeSystem:false,
130-
}))
131-
ifhttpapi.Is404Error(err) {
132-
httpapi.ResourceNotFound(rw)
126+
_,members,done:=ExtractOrganizationMember(ctx,nil,rw,r,db,organization.ID)
127+
ifdone {
133128
return
134129
}
135-
iferr!=nil {
130+
131+
iflen(members)!=1 {
136132
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
137133
Message:"Internal error fetching organization member.",
138-
Detail:err.Error(),
134+
// This is a developer error and should never happen.
135+
Detail:fmt.Sprintf("Expected exactly one organization member, but got %d.",len(members)),
139136
})
140137
return
141138
}
142139

140+
organizationMember:=members[0]
141+
143142
ctx=context.WithValue(ctx,organizationMemberParamContextKey{},OrganizationMember{
144143
OrganizationMember:organizationMember.OrganizationMember,
145144
// Here we're making two exceptions to the rule about not leaking data about the user
@@ -151,8 +150,113 @@ func ExtractOrganizationMemberParam(db database.Store) func(http.Handler) http.H
151150
// API handlers need this information for audit logging and returning the owner's
152151
// username in response to creating a workspace. Additionally, the frontend consumes
153152
// the Avatar URL and this allows the FE to avoid an extra request.
154-
Username:user.Username,
155-
AvatarURL:user.AvatarURL,
153+
Username:organizationMember.Username,
154+
AvatarURL:organizationMember.AvatarURL,
155+
})
156+
157+
next.ServeHTTP(rw,r.WithContext(ctx))
158+
})
159+
}
160+
}
161+
162+
// ExtractOrganizationMember extracts all user memberships from the "user" URL
163+
// parameter. If orgID is uuid.Nil, then it will return all memberships for the
164+
// user, otherwise it will only return memberships to the org.
165+
//
166+
// If `user` is returned, that means the caller can use the data. This is returned because
167+
// it is possible to have a user with 0 organizations. So the user != nil, with 0 memberships.
168+
funcExtractOrganizationMember(ctx context.Context,authfunc(r*http.Request,action policy.Action,object rbac.Objecter)bool,rw http.ResponseWriter,r*http.Request,db database.Store,orgID uuid.UUID) (*database.User, []database.OrganizationMembersRow,bool) {
169+
// We need to resolve the `{user}` URL parameter so that we can get the userID and
170+
// username. We do this as SystemRestricted since the caller might have permission
171+
// to access the OrganizationMember object, but *not* the User object. So, it is
172+
// very important that we do not add the User object to the request context or otherwise
173+
// leak it to the API handler.
174+
// nolint:gocritic
175+
user,ok:=ExtractUserContext(dbauthz.AsSystemRestricted(ctx),db,rw,r)
176+
if!ok {
177+
returnnil,nil,true
178+
}
179+
180+
organizationMembers,err:=db.OrganizationMembers(ctx, database.OrganizationMembersParams{
181+
OrganizationID:orgID,
182+
UserID:user.ID,
183+
IncludeSystem:false,
184+
})
185+
ifhttpapi.Is404Error(err) {
186+
httpapi.ResourceNotFound(rw)
187+
returnnil,nil,true
188+
}
189+
iferr!=nil {
190+
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
191+
Message:"Internal error fetching organization member.",
192+
Detail:err.Error(),
193+
})
194+
returnnil,nil,true
195+
}
196+
197+
// Only return the user data if the caller can read the user object.
198+
ifauth!=nil&&auth(r,policy.ActionRead,user) {
199+
return&user,organizationMembers,false
200+
}
201+
202+
// If the user cannot be read and 0 memberships exist, throw a 404 to not
203+
// leak the user existence.
204+
iflen(organizationMembers)==0 {
205+
httpapi.ResourceNotFound(rw)
206+
returnnil,nil,true
207+
}
208+
209+
returnnil,organizationMembers,false
210+
}
211+
212+
typeOrganizationMembersstruct {
213+
// User is `nil` if the caller is not allowed access to the site wide
214+
// user object.
215+
User*database.User
216+
// Memberships can only be length 0 if `user != nil`. If `user == nil`, then
217+
// memberships will be at least length 1.
218+
Memberships []OrganizationMember
219+
}
220+
221+
func (omOrganizationMembers)UserID() uuid.UUID {
222+
ifom.User!=nil {
223+
returnom.User.ID
224+
}
225+
226+
iflen(om.Memberships)>0 {
227+
returnom.Memberships[0].UserID
228+
}
229+
returnuuid.Nil
230+
}
231+
232+
// ExtractOrganizationMembersParam grabs all user organization memberships.
233+
// Only requires the "user" URL parameter.
234+
//
235+
// Use this if you want to grab as much information for a user as you can.
236+
// From an organization context, site wide user information might not available.
237+
funcExtractOrganizationMembersParam(db database.Store,authfunc(r*http.Request,action policy.Action,object rbac.Objecter)bool)func(http.Handler) http.Handler {
238+
returnfunc(next http.Handler) http.Handler {
239+
returnhttp.HandlerFunc(func(rw http.ResponseWriter,r*http.Request) {
240+
ctx:=r.Context()
241+
242+
// Fetch all memberships
243+
user,members,done:=ExtractOrganizationMember(ctx,auth,rw,r,db,uuid.Nil)
244+
ifdone {
245+
return
246+
}
247+
248+
orgMembers:=make([]OrganizationMember,0,len(members))
249+
for_,organizationMember:=rangemembers {
250+
orgMembers=append(orgMembers,OrganizationMember{
251+
OrganizationMember:organizationMember.OrganizationMember,
252+
Username:organizationMember.Username,
253+
AvatarURL:organizationMember.AvatarURL,
254+
})
255+
}
256+
257+
ctx=context.WithValue(ctx,organizationMembersParamContextKey{},OrganizationMembers{
258+
User:user,
259+
Memberships:orgMembers,
156260
})
157261
next.ServeHTTP(rw,r.WithContext(ctx))
158262
})

‎coderd/httpmw/organizationparam_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import (
1616
"github.com/coder/coder/v2/coderd/database/dbmem"
1717
"github.com/coder/coder/v2/coderd/database/dbtime"
1818
"github.com/coder/coder/v2/coderd/httpmw"
19+
"github.com/coder/coder/v2/coderd/rbac"
20+
"github.com/coder/coder/v2/coderd/rbac/policy"
1921
"github.com/coder/coder/v2/codersdk"
2022
"github.com/coder/coder/v2/testutil"
2123
)
@@ -167,6 +169,10 @@ func TestOrganizationParam(t *testing.T) {
167169
httpmw.ExtractOrganizationParam(db),
168170
httpmw.ExtractUserParam(db),
169171
httpmw.ExtractOrganizationMemberParam(db),
172+
httpmw.ExtractOrganizationMembersParam(db,func(r*http.Request,_ policy.Action,_ rbac.Objecter)bool {
173+
// Assume the caller cannot read the member
174+
returnfalse
175+
}),
170176
)
171177
rtr.Get("/",func(rw http.ResponseWriter,r*http.Request) {
172178
org:=httpmw.OrganizationParam(r)
@@ -190,6 +196,11 @@ func TestOrganizationParam(t *testing.T) {
190196
assert.NotEmpty(t,orgMem.OrganizationMember.UpdatedAt)
191197
assert.NotEmpty(t,orgMem.OrganizationMember.UserID)
192198
assert.NotEmpty(t,orgMem.OrganizationMember.Roles)
199+
200+
orgMems:=httpmw.OrganizationMembersParam(r)
201+
assert.NotZero(t,orgMems)
202+
assert.Equal(t,orgMem.UserID,orgMems.Memberships[0].UserID)
203+
assert.Nil(t,orgMems.User,"user data should not be available, hard coded false authorize")
193204
})
194205

195206
// Try by ID

‎coderd/workspacebuilds.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ func (api *API) workspaceBuilds(rw http.ResponseWriter, r *http.Request) {
232232
// @Router /users/{user}/workspace/{workspacename}/builds/{buildnumber} [get]
233233
func (api*API)workspaceBuildByBuildNumber(rw http.ResponseWriter,r*http.Request) {
234234
ctx:=r.Context()
235-
owner:=httpmw.UserParam(r)
235+
mems:=httpmw.OrganizationMembersParam(r)
236236
workspaceName:=chi.URLParam(r,"workspacename")
237237
buildNumber,err:=strconv.ParseInt(chi.URLParam(r,"buildnumber"),10,32)
238238
iferr!=nil {
@@ -244,7 +244,7 @@ func (api *API) workspaceBuildByBuildNumber(rw http.ResponseWriter, r *http.Requ
244244
}
245245

246246
workspace,err:=api.Database.GetWorkspaceByOwnerIDAndName(ctx, database.GetWorkspaceByOwnerIDAndNameParams{
247-
OwnerID:owner.ID,
247+
OwnerID:mems.UserID(),
248248
Name:workspaceName,
249249
})
250250
ifhttpapi.Is404Error(err) {

‎coderd/workspaces.go

Lines changed: 25 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,8 @@ func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) {
253253
// @Router /users/{user}/workspace/{workspacename} [get]
254254
func (api*API)workspaceByOwnerAndName(rw http.ResponseWriter,r*http.Request) {
255255
ctx:=r.Context()
256-
owner:=httpmw.UserParam(r)
256+
257+
mems:=httpmw.OrganizationMembersParam(r)
257258
workspaceName:=chi.URLParam(r,"workspacename")
258259
apiKey:=httpmw.APIKey(r)
259260

@@ -273,12 +274,12 @@ func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
273274
}
274275

275276
workspace,err:=api.Database.GetWorkspaceByOwnerIDAndName(ctx, database.GetWorkspaceByOwnerIDAndNameParams{
276-
OwnerID:owner.ID,
277+
OwnerID:mems.UserID(),
277278
Name:workspaceName,
278279
})
279280
ifincludeDeleted&&errors.Is(err,sql.ErrNoRows) {
280281
workspace,err=api.Database.GetWorkspaceByOwnerIDAndName(ctx, database.GetWorkspaceByOwnerIDAndNameParams{
281-
OwnerID:owner.ID,
282+
OwnerID:mems.UserID(),
282283
Name:workspaceName,
283284
Deleted:includeDeleted,
284285
})
@@ -408,6 +409,7 @@ func (api *API) postUserWorkspaces(rw http.ResponseWriter, r *http.Request) {
408409
ctx=r.Context()
409410
apiKey=httpmw.APIKey(r)
410411
auditor=api.Auditor.Load()
412+
mems=httpmw.OrganizationMembersParam(r)
411413
)
412414

413415
varreq codersdk.CreateWorkspaceRequest
@@ -416,17 +418,16 @@ func (api *API) postUserWorkspaces(rw http.ResponseWriter, r *http.Request) {
416418
}
417419

418420
varownerworkspaceOwner
419-
// This user fetch is an optimization path for the most common case of creating a
420-
// workspace for 'Me'.
421-
//
422-
// This is also required to allow `owners` to create workspaces for users
423-
// that are not in an organization.
424-
user,ok:=httpmw.UserParamOptional(r)
425-
ifok {
421+
ifmems.User!=nil {
422+
// This user fetch is an optimization path for the most common case of creating a
423+
// workspace for 'Me'.
424+
//
425+
// This is also required to allow `owners` to create workspaces for users
426+
// that are not in an organization.
426427
owner=workspaceOwner{
427-
ID:user.ID,
428-
Username:user.Username,
429-
AvatarURL:user.AvatarURL,
428+
ID:mems.User.ID,
429+
Username:mems.User.Username,
430+
AvatarURL:mems.User.AvatarURL,
430431
}
431432
}else {
432433
// A workspace can still be created if the caller can read the organization
@@ -443,35 +444,21 @@ func (api *API) postUserWorkspaces(rw http.ResponseWriter, r *http.Request) {
443444
return
444445
}
445446

446-
// We need to fetch the original user as a system user to fetch the
447-
// user_id. 'ExtractUserContext' handles all cases like usernames,
448-
// 'Me', etc.
449-
// nolint:gocritic // The user_id needs to be fetched. This handles all those cases.
450-
user,ok:=httpmw.ExtractUserContext(dbauthz.AsSystemRestricted(ctx),api.Database,rw,r)
451-
if!ok {
452-
return
453-
}
454-
455-
organizationMember,err:=database.ExpectOne(api.Database.OrganizationMembers(ctx, database.OrganizationMembersParams{
456-
OrganizationID:template.OrganizationID,
457-
UserID:user.ID,
458-
IncludeSystem:false,
459-
}))
460-
ifhttpapi.Is404Error(err) {
447+
// If the caller can find the organization membership in the same org
448+
// as the template, then they can continue.
449+
orgIndex:=slices.IndexFunc(mems.Memberships,func(mem httpmw.OrganizationMember)bool {
450+
returnmem.OrganizationID==template.OrganizationID
451+
})
452+
iforgIndex==-1 {
461453
httpapi.ResourceNotFound(rw)
462454
return
463455
}
464-
iferr!=nil {
465-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
466-
Message:"Internal error fetching organization member.",
467-
Detail:err.Error(),
468-
})
469-
return
470-
}
456+
457+
member:=mems.Memberships[orgIndex]
471458
owner=workspaceOwner{
472-
ID:organizationMember.OrganizationMember.UserID,
473-
Username:organizationMember.Username,
474-
AvatarURL:organizationMember.AvatarURL,
459+
ID:member.UserID,
460+
Username:member.Username,
461+
AvatarURL:member.AvatarURL,
475462
}
476463
}
477464

0 commit comments

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