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

Commit08f3271

Browse files
committed
chore: do not require site wide permission to use workspace apis
Workspace apis scoped to an org should not require site wide perms.Site wide perms should still work if the owner is not in an organzation
1 parenteaf7a21 commit08f3271

File tree

6 files changed

+231
-77
lines changed

6 files changed

+231
-77
lines changed

‎coderd/coderd.go‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,7 +1076,7 @@ func New(options *Options) *API {
10761076

10771077
r.Group(func(r chi.Router) {
10781078
r.Use(
1079-
httpmw.ExtractOrganizationMemberParam(options.Database),
1079+
httpmw.ExtractOrganizationMemberParam(options.Database,api.HTTPAuth.Authorize),
10801080
)
10811081
r.Delete("/",api.deleteOrganizationMember)
10821082
r.Put("/roles",api.putMemberRoles)
@@ -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: 121 additions & 25 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 {
@@ -107,39 +118,27 @@ type OrganizationMember struct {
107118

108119
// ExtractOrganizationMemberParam grabs a user membership from the "organization" and "user" URL parameter.
109120
// This middleware requires the ExtractUser and ExtractOrganization middleware higher in the stack
110-
funcExtractOrganizationMemberParam(db database.Store)func(http.Handler) http.Handler {
121+
funcExtractOrganizationMemberParam(db database.Store,authfunc(r*http.Request,action policy.Action,object rbac.Objecter)bool)func(http.Handler) http.Handler {
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,auth,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,105 @@ 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+
ifauth(r,policy.ActionRead,user) {
198+
return&user,organizationMembers,true
199+
}
200+
201+
// If the user cannot be read and 0 memberships exist, throw a 404 to not
202+
// leak the user existance.
203+
iflen(organizationMembers)==0 {
204+
httpapi.ResourceNotFound(rw)
205+
returnnil,nil,true
206+
}
207+
208+
returnnil,organizationMembers,false
209+
}
210+
211+
typeOrganizationMembersstruct {
212+
User*database.User
213+
Memberships []OrganizationMember
214+
}
215+
216+
func (omOrganizationMembers)UserID() uuid.UUID {
217+
ifom.User!=nil {
218+
returnom.User.ID
219+
}
220+
221+
iflen(om.Memberships)>0 {
222+
returnom.Memberships[0].UserID
223+
}
224+
returnuuid.Nil
225+
}
226+
227+
// ExtractOrganizationMembersParam grabs all user organization memberships.
228+
// Only requires the "user" URL parameter.
229+
funcExtractOrganizationMembersParam(db database.Store,authfunc(r*http.Request,action policy.Action,object rbac.Objecter)bool)func(http.Handler) http.Handler {
230+
returnfunc(next http.Handler) http.Handler {
231+
returnhttp.HandlerFunc(func(rw http.ResponseWriter,r*http.Request) {
232+
ctx:=r.Context()
233+
234+
// Fetch all memberships
235+
user,members,done:=ExtractOrganizationMember(ctx,auth,rw,r,db,uuid.Nil)
236+
ifdone {
237+
return
238+
}
239+
240+
orgMembers:=make([]OrganizationMember,0,len(members))
241+
for_,organizationMember:=rangemembers {
242+
orgMembers=append(orgMembers,OrganizationMember{
243+
OrganizationMember:organizationMember.OrganizationMember,
244+
Username:organizationMember.Username,
245+
AvatarURL:organizationMember.AvatarURL,
246+
})
247+
}
248+
249+
ctx=context.WithValue(ctx,organizationMembersParamContextKey{},OrganizationMembers{
250+
User:user,
251+
Memberships:orgMembers,
156252
})
157253
next.ServeHTTP(rw,r.WithContext(ctx))
158254
})

‎coderd/httpmw/organizationparam_test.go‎

Lines changed: 15 additions & 2 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
)
@@ -129,7 +131,9 @@ func TestOrganizationParam(t *testing.T) {
129131
}),
130132
httpmw.ExtractUserParam(db),
131133
httpmw.ExtractOrganizationParam(db),
132-
httpmw.ExtractOrganizationMemberParam(db),
134+
httpmw.ExtractOrganizationMemberParam(db,func(r*http.Request,_ policy.Action,_ rbac.Objecter)bool {
135+
returntrue
136+
}),
133137
)
134138
rtr.Get("/",nil)
135139
rtr.ServeHTTP(rw,r)
@@ -166,7 +170,12 @@ func TestOrganizationParam(t *testing.T) {
166170
}),
167171
httpmw.ExtractOrganizationParam(db),
168172
httpmw.ExtractUserParam(db),
169-
httpmw.ExtractOrganizationMemberParam(db),
173+
httpmw.ExtractOrganizationMemberParam(db,func(r*http.Request,_ policy.Action,_ rbac.Objecter)bool {
174+
returntrue
175+
}),
176+
httpmw.ExtractOrganizationMembersParam(db,func(r*http.Request,_ policy.Action,_ rbac.Objecter)bool {
177+
returntrue
178+
}),
170179
)
171180
rtr.Get("/",func(rw http.ResponseWriter,r*http.Request) {
172181
org:=httpmw.OrganizationParam(r)
@@ -190,6 +199,10 @@ func TestOrganizationParam(t *testing.T) {
190199
assert.NotEmpty(t,orgMem.OrganizationMember.UpdatedAt)
191200
assert.NotEmpty(t,orgMem.OrganizationMember.UserID)
192201
assert.NotEmpty(t,orgMem.OrganizationMember.Roles)
202+
203+
orgMems:=httpmw.OrganizationMembersParam(r)
204+
assert.NotZero(t,orgMems)
205+
assert.Equal(t,orgMem.UserID,orgMems[0].UserID)
193206
})
194207

195208
// Try by ID

‎coderd/httpmw/userparam.go‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/google/uuid"
1010

1111
"github.com/coder/coder/v2/coderd/database"
12+
"github.com/coder/coder/v2/coderd/database/dbauthz"
1213
"github.com/coder/coder/v2/coderd/httpapi"
1314
"github.com/coder/coder/v2/codersdk"
1415
)
@@ -128,3 +129,57 @@ func ExtractUserContext(ctx context.Context, db database.Store, rw http.Response
128129
}
129130
returnuser,true
130131
}
132+
133+
// ExtractUserID will work if the requester can access any OrganizationMember that
134+
// belongs to the user.
135+
funcExtractUserID(db database.Store)func(http.Handler) http.Handler {
136+
returnfunc(next http.Handler) http.Handler {
137+
returnhttp.HandlerFunc(func(rw http.ResponseWriter,r*http.Request) {
138+
ctx:=r.Context()
139+
// We need to resolve the `{user}` URL parameter so that we can get the userID and
140+
// username. We do this as SystemRestricted since the caller might have permission
141+
// to access the OrganizationMember object, but *not* the User object. So, it is
142+
// very important that we do not add the User object to the request context or otherwise
143+
// leak it to the API handler.
144+
// nolint:gocritic
145+
user,ok:=ExtractUserContext(dbauthz.AsSystemRestricted(ctx),db,rw,r)
146+
if!ok {
147+
return
148+
}
149+
organization:=OrganizationParam(r)
150+
151+
organizationMember,err:=database.ExpectOne(db.OrganizationMembers(ctx, database.OrganizationMembersParams{
152+
OrganizationID:organization.ID,
153+
UserID:user.ID,
154+
IncludeSystem:false,
155+
}))
156+
ifhttpapi.Is404Error(err) {
157+
httpapi.ResourceNotFound(rw)
158+
return
159+
}
160+
iferr!=nil {
161+
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
162+
Message:"Internal error fetching organization member.",
163+
Detail:err.Error(),
164+
})
165+
return
166+
}
167+
168+
ctx=context.WithValue(ctx,organizationMemberParamContextKey{},OrganizationMember{
169+
OrganizationMember:organizationMember.OrganizationMember,
170+
// Here we're making two exceptions to the rule about not leaking data about the user
171+
// to the API handler, which is to include the username and avatar URL.
172+
// If the caller has permission to read the OrganizationMember, then we're explicitly
173+
// saying here that they also have permission to see the member's username and avatar.
174+
// This is OK!
175+
//
176+
// API handlers need this information for audit logging and returning the owner's
177+
// username in response to creating a workspace. Additionally, the frontend consumes
178+
// the Avatar URL and this allows the FE to avoid an extra request.
179+
Username:user.Username,
180+
AvatarURL:user.AvatarURL,
181+
})
182+
next.ServeHTTP(rw,r.WithContext(ctx))
183+
})
184+
}
185+
}

‎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) {

0 commit comments

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