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

Commiteea8dc6

Browse files
authored
feat: Add rbac to templateversion+orgmember endpoints (#1713)
1 parentf8410de commiteea8dc6

17 files changed

+302
-66
lines changed

‎coderd/coderd.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ func newRouter(options *Options, a *api) chi.Router {
170170
r.Route("/{user}",func(r chi.Router) {
171171
r.Use(
172172
httpmw.ExtractUserParam(options.Database),
173+
httpmw.ExtractOrganizationMemberParam(options.Database),
173174
)
174175
r.Put("/roles",a.putMemberRoles)
175176
})
@@ -201,6 +202,7 @@ func newRouter(options *Options, a *api) chi.Router {
201202
r.Route("/templateversions/{templateversion}",func(r chi.Router) {
202203
r.Use(
203204
apiKeyMiddleware,
205+
authRolesMiddleware,
204206
httpmw.ExtractTemplateVersionParam(options.Database),
205207
)
206208

@@ -289,6 +291,7 @@ func newRouter(options *Options, a *api) chi.Router {
289291
r.Route("/workspaceresources/{workspaceresource}",func(r chi.Router) {
290292
r.Use(
291293
apiKeyMiddleware,
294+
authRolesMiddleware,
292295
httpmw.ExtractWorkspaceResourceParam(options.Database),
293296
httpmw.ExtractWorkspaceParam(options.Database),
294297
)

‎coderd/coderd_test.go

Lines changed: 74 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import (
1717
"github.com/coder/coder/coderd/coderdtest"
1818
"github.com/coder/coder/coderd/rbac"
1919
"github.com/coder/coder/codersdk"
20+
"github.com/coder/coder/provisioner/echo"
21+
"github.com/coder/coder/provisionersdk/proto"
2022
)
2123

2224
funcTestMain(m*testing.M) {
@@ -47,13 +49,32 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
4749
require.NoError(t,err,"fetch org")
4850

4951
// Setup some data in the database.
50-
version:=coderdtest.CreateTemplateVersion(t,client,admin.OrganizationID,nil)
52+
version:=coderdtest.CreateTemplateVersion(t,client,admin.OrganizationID,&echo.Responses{
53+
Parse:echo.ParseComplete,
54+
Provision: []*proto.Provision_Response{{
55+
Type:&proto.Provision_Response_Complete{
56+
Complete:&proto.Provision_Complete{
57+
// Return a workspace resource
58+
Resources: []*proto.Resource{{
59+
Name:"some",
60+
Type:"example",
61+
Agents: []*proto.Agent{{
62+
Id:"something",
63+
Auth:&proto.Agent_Token{},
64+
}},
65+
}},
66+
},
67+
},
68+
}},
69+
})
5170
coderdtest.AwaitTemplateVersionJob(t,client,version.ID)
5271
template:=coderdtest.CreateTemplate(t,client,admin.OrganizationID,version.ID)
5372
workspace:=coderdtest.CreateWorkspace(t,client,admin.OrganizationID,template.ID)
5473
coderdtest.AwaitWorkspaceBuildJob(t,client,workspace.LatestBuild.ID)
5574
file,err:=client.Upload(ctx,codersdk.ContentTypeTar,make([]byte,1024))
5675
require.NoError(t,err,"upload file")
76+
workspaceResources,err:=client.WorkspaceResourcesByBuild(ctx,workspace.LatestBuild.ID)
77+
require.NoError(t,err,"workspace resources")
5778

5879
// Always fail auth from this point forward
5980
authorizer.AlwaysReturn=rbac.ForbiddenWithInternal(xerrors.New("fake implementation"),nil,nil)
@@ -78,6 +99,9 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
7899
"POST:/api/v2/users/logout": {NoAuthorize:true},
79100
"GET:/api/v2/users/authmethods": {NoAuthorize:true},
80101

102+
// Has it's own auth
103+
"GET:/api/v2/users/oauth2/github/callback": {NoAuthorize:true},
104+
81105
// All workspaceagents endpoints do not use rbac
82106
"POST:/api/v2/workspaceagents/aws-instance-identity": {NoAuthorize:true},
83107
"POST:/api/v2/workspaceagents/azure-instance-identity": {NoAuthorize:true},
@@ -94,11 +118,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
94118
"GET:/api/v2/workspaceagents/{workspaceagent}/turn": {NoAuthorize:true},
95119

96120
// TODO: @emyrk these need to be fixed by adding authorize calls
97-
"GET:/api/v2/workspaceresources/{workspaceresource}": {NoAuthorize:true},
98-
99-
"GET:/api/v2/users/oauth2/github/callback": {NoAuthorize:true},
100-
101-
"PUT:/api/v2/organizations/{organization}/members/{user}/roles": {NoAuthorize:true},
102121
"GET:/api/v2/organizations/{organization}/provisionerdaemons": {NoAuthorize:true},
103122
"GET:/api/v2/organizations/{organization}/templates/{templatename}": {NoAuthorize:true},
104123
"POST:/api/v2/organizations/{organization}/templateversions": {NoAuthorize:true},
@@ -108,17 +127,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
108127
"GET:/api/v2/parameters/{scope}/{id}": {NoAuthorize:true},
109128
"DELETE:/api/v2/parameters/{scope}/{id}/{name}": {NoAuthorize:true},
110129

111-
"GET:/api/v2/templates/{template}/versions": {NoAuthorize:true},
112-
"PATCH:/api/v2/templates/{template}/versions": {NoAuthorize:true},
113-
"GET:/api/v2/templates/{template}/versions/{templateversionname}": {NoAuthorize:true},
114-
115-
"GET:/api/v2/templateversions/{templateversion}": {NoAuthorize:true},
116-
"PATCH:/api/v2/templateversions/{templateversion}/cancel": {NoAuthorize:true},
117-
"GET:/api/v2/templateversions/{templateversion}/logs": {NoAuthorize:true},
118-
"GET:/api/v2/templateversions/{templateversion}/parameters": {NoAuthorize:true},
119-
"GET:/api/v2/templateversions/{templateversion}/resources": {NoAuthorize:true},
120-
"GET:/api/v2/templateversions/{templateversion}/schema": {NoAuthorize:true},
121-
122130
"POST:/api/v2/users/{user}/organizations": {NoAuthorize:true},
123131

124132
"GET:/api/v2/workspaces/{workspace}/watch": {NoAuthorize:true},
@@ -164,6 +172,10 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
164172
AssertAction:rbac.ActionUpdate,
165173
AssertObject:workspaceRBACObj,
166174
},
175+
"GET:/api/v2/workspaceresources/{workspaceresource}": {
176+
AssertAction:rbac.ActionRead,
177+
AssertObject:workspaceRBACObj,
178+
},
167179
"PATCH:/api/v2/workspacebuilds/{workspacebuild}/cancel": {
168180
AssertAction:rbac.ActionUpdate,
169181
AssertObject:workspaceRBACObj,
@@ -199,12 +211,51 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
199211
AssertObject:rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
200212
},
201213
"POST:/api/v2/files": {AssertAction:rbac.ActionCreate,AssertObject:rbac.ResourceFile},
202-
"GET:/api/v2/files/{fileHash}": {AssertAction:rbac.ActionRead,
203-
AssertObject:rbac.ResourceFile.WithOwner(admin.UserID.String()).WithID(file.Hash)},
214+
"GET:/api/v2/files/{fileHash}": {
215+
AssertAction:rbac.ActionRead,
216+
AssertObject:rbac.ResourceFile.WithOwner(admin.UserID.String()).WithID(file.Hash),
217+
},
218+
"GET:/api/v2/templates/{template}/versions": {
219+
AssertAction:rbac.ActionRead,
220+
AssertObject:rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
221+
},
222+
"PATCH:/api/v2/templates/{template}/versions": {
223+
AssertAction:rbac.ActionUpdate,
224+
AssertObject:rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
225+
},
226+
"GET:/api/v2/templates/{template}/versions/{templateversionname}": {
227+
AssertAction:rbac.ActionRead,
228+
AssertObject:rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
229+
},
230+
"GET:/api/v2/templateversions/{templateversion}": {
231+
AssertAction:rbac.ActionRead,
232+
AssertObject:rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
233+
},
234+
"PATCH:/api/v2/templateversions/{templateversion}/cancel": {
235+
AssertAction:rbac.ActionUpdate,
236+
AssertObject:rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
237+
},
238+
"GET:/api/v2/templateversions/{templateversion}/logs": {
239+
AssertAction:rbac.ActionRead,
240+
AssertObject:rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
241+
},
242+
"GET:/api/v2/templateversions/{templateversion}/parameters": {
243+
AssertAction:rbac.ActionRead,
244+
AssertObject:rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
245+
},
246+
"GET:/api/v2/templateversions/{templateversion}/resources": {
247+
AssertAction:rbac.ActionRead,
248+
AssertObject:rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
249+
},
250+
"GET:/api/v2/templateversions/{templateversion}/schema": {
251+
AssertAction:rbac.ActionRead,
252+
AssertObject:rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
253+
},
204254

205255
// These endpoints need payloads to get to the auth part. Payloads will be required
206-
"PUT:/api/v2/users/{user}/roles": {StatusCode:http.StatusBadRequest,NoAuthorize:true},
207-
"POST:/api/v2/workspaces/{workspace}/builds": {StatusCode:http.StatusBadRequest,NoAuthorize:true},
256+
"PUT:/api/v2/users/{user}/roles": {StatusCode:http.StatusBadRequest,NoAuthorize:true},
257+
"PUT:/api/v2/organizations/{organization}/members/{user}/roles": {NoAuthorize:true},
258+
"POST:/api/v2/workspaces/{workspace}/builds": {StatusCode:http.StatusBadRequest,NoAuthorize:true},
208259
}
209260

210261
fork,v:=rangeassertRoute {
@@ -240,6 +291,8 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
240291
route=strings.ReplaceAll(route,"{workspacebuildname}",workspace.LatestBuild.Name)
241292
route=strings.ReplaceAll(route,"{template}",template.ID.String())
242293
route=strings.ReplaceAll(route,"{hash}",file.Hash)
294+
route=strings.ReplaceAll(route,"{workspaceresource}",workspaceResources[0].ID.String())
295+
route=strings.ReplaceAll(route,"{templateversion}",version.ID.String())
243296

244297
resp,err:=client.Request(context.Background(),method,route,nil)
245298
require.NoError(t,err,"do req")

‎coderd/coderdtest/coderdtest.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,7 @@ func CreateAnotherUser(t *testing.T, client *codersdk.Client, organizationID uui
276276
fororgID,roles:=rangeorgRoles {
277277
organizationID,err:=uuid.Parse(orgID)
278278
require.NoError(t,err,fmt.Sprintf("parse org id %q",orgID))
279-
// TODO: @Emyrk add the member to the organization if they do not already belong.
280-
_,err=other.UpdateOrganizationMemberRoles(context.Background(),organizationID,user.ID.String(),
279+
_,err=client.UpdateOrganizationMemberRoles(context.Background(),organizationID,user.ID.String(),
281280
codersdk.UpdateRoles{Roles:append(roles,rbac.RoleOrgMember(organizationID))})
282281
require.NoError(t,err,"update org membership roles")
283282
}

‎coderd/database/modelmethods.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ func (t Template) RBACObject() rbac.Object {
66
returnrbac.ResourceTemplate.InOrg(t.OrganizationID).WithID(t.ID.String())
77
}
88

9+
func (tTemplateVersion)RBACObject() rbac.Object {
10+
// Just use the parent template resource for controlling versions
11+
returnrbac.ResourceTemplate.InOrg(t.OrganizationID).WithID(t.TemplateID.UUID.String())
12+
}
13+
914
func (wWorkspace)RBACObject() rbac.Object {
1015
returnrbac.ResourceWorkspace.InOrg(w.OrganizationID).WithID(w.ID.String()).WithOwner(w.OwnerID.String())
1116
}

‎coderd/httpmw/organizationparam.go

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ func OrganizationParam(r *http.Request) database.Organization {
2828
funcOrganizationMemberParam(r*http.Request) database.OrganizationMember {
2929
organizationMember,ok:=r.Context().Value(organizationMemberParamContextKey{}).(database.OrganizationMember)
3030
if!ok {
31-
panic("developer error: organization param middleware not provided")
31+
panic("developer error: organizationmemberparam middleware not provided")
3232
}
3333
returnorganizationMember
3434
}
3535

36-
// ExtractOrganizationParam grabs an organizationand user membershipfrom the "organization" URL parameter.
36+
// ExtractOrganizationParam grabs an organization from the "organization" URL parameter.
3737
// This middleware requires the API key middleware higher in the call stack for authentication.
3838
funcExtractOrganizationParam(db database.Store)func(http.Handler) http.Handler {
3939
returnfunc(next http.Handler) http.Handler {
@@ -56,11 +56,23 @@ func ExtractOrganizationParam(db database.Store) func(http.Handler) http.Handler
5656
})
5757
return
5858
}
59+
ctx:=context.WithValue(r.Context(),organizationParamContextKey{},organization)
60+
next.ServeHTTP(rw,r.WithContext(ctx))
61+
})
62+
}
63+
}
64+
65+
// ExtractOrganizationMemberParam grabs a user membership from the "organization" and "user" URL parameter.
66+
// This middleware requires the ExtractUser and ExtractOrganization middleware higher in the stack
67+
funcExtractOrganizationMemberParam(db database.Store)func(http.Handler) http.Handler {
68+
returnfunc(next http.Handler) http.Handler {
69+
returnhttp.HandlerFunc(func(rw http.ResponseWriter,r*http.Request) {
70+
organization:=OrganizationParam(r)
71+
user:=UserParam(r)
5972

60-
apiKey:=APIKey(r)
6173
organizationMember,err:=db.GetOrganizationMemberByUserID(r.Context(), database.GetOrganizationMemberByUserIDParams{
6274
OrganizationID:organization.ID,
63-
UserID:apiKey.UserID,
75+
UserID:user.ID,
6476
})
6577
iferrors.Is(err,sql.ErrNoRows) {
6678
httpapi.Write(rw,http.StatusForbidden, httpapi.Response{
@@ -74,9 +86,8 @@ func ExtractOrganizationParam(db database.Store) func(http.Handler) http.Handler
7486
})
7587
return
7688
}
89+
ctx:=context.WithValue(r.Context(),organizationMemberParamContextKey{},organizationMember)
7790

78-
ctx:=context.WithValue(r.Context(),organizationParamContextKey{},organization)
79-
ctx=context.WithValue(ctx,organizationMemberParamContextKey{},organizationMember)
8091
next.ServeHTTP(rw,r.WithContext(ctx))
8192
})
8293
}

‎coderd/httpmw/organizationparam_test.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ func TestOrganizationParam(t *testing.T) {
122122
var (
123123
db=databasefake.New()
124124
rw=httptest.NewRecorder()
125-
r,_=setupAuthentication(db)
125+
r,u=setupAuthentication(db)
126126
rtr=chi.NewRouter()
127127
)
128128
organization,err:=db.InsertOrganization(r.Context(), database.InsertOrganizationParams{
@@ -133,9 +133,12 @@ func TestOrganizationParam(t *testing.T) {
133133
})
134134
require.NoError(t,err)
135135
chi.RouteContext(r.Context()).URLParams.Add("organization",organization.ID.String())
136+
chi.RouteContext(r.Context()).URLParams.Add("user",u.ID.String())
136137
rtr.Use(
137138
httpmw.ExtractAPIKey(db,nil),
139+
httpmw.ExtractUserParam(db),
138140
httpmw.ExtractOrganizationParam(db),
141+
httpmw.ExtractOrganizationMemberParam(db),
139142
)
140143
rtr.Get("/",nil)
141144
rtr.ServeHTTP(rw,r)
@@ -167,9 +170,12 @@ func TestOrganizationParam(t *testing.T) {
167170
})
168171
require.NoError(t,err)
169172
chi.RouteContext(r.Context()).URLParams.Add("organization",organization.ID.String())
173+
chi.RouteContext(r.Context()).URLParams.Add("user",user.ID.String())
170174
rtr.Use(
171175
httpmw.ExtractAPIKey(db,nil),
172176
httpmw.ExtractOrganizationParam(db),
177+
httpmw.ExtractUserParam(db),
178+
httpmw.ExtractOrganizationMemberParam(db),
173179
)
174180
rtr.Get("/",func(rw http.ResponseWriter,r*http.Request) {
175181
_=httpmw.OrganizationParam(r)

‎coderd/members.go

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,27 +17,29 @@ import (
1717
)
1818

1919
func (api*api)putMemberRoles(rw http.ResponseWriter,r*http.Request) {
20-
// User is the user to modify
21-
// TODO: Until rbac authorize is implemented, only be able to change your
22-
//own roles. This also means you can grant yourself whatever roles you want.
2320
user:=httpmw.UserParam(r)
24-
apiKey:=httpmw.APIKey(r)
2521
organization:=httpmw.OrganizationParam(r)
26-
// TODO: @emyrk add proper `Authorize()` check here instead of a uuid match.
27-
//Proper authorize should check the granted roles are able to given within
28-
//the selected organization. Until then, allow anarchy
29-
ifapiKey.UserID!=user.ID {
30-
httpapi.Write(rw,http.StatusUnauthorized, httpapi.Response{
31-
Message:"modifying other users is not supported at this time",
32-
})
33-
return
34-
}
22+
member:=httpmw.OrganizationMemberParam(r)
3523

3624
varparams codersdk.UpdateRoles
3725
if!httpapi.Read(rw,r,&params) {
3826
return
3927
}
4028

29+
added,removed:=rbac.ChangeRoleSet(member.Roles,params.Roles)
30+
for_,roleName:=rangeadded {
31+
// Assigning a role requires the create permission.
32+
if!api.Authorize(rw,r,rbac.ActionCreate,rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {
33+
return
34+
}
35+
}
36+
for_,roleName:=rangeremoved {
37+
// Removing a role requires the delete permission.
38+
if!api.Authorize(rw,r,rbac.ActionDelete,rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {
39+
return
40+
}
41+
}
42+
4143
updatedUser,err:=api.updateOrganizationMemberRoles(r.Context(), database.UpdateMemberRolesParams{
4244
GrantedRoles:params.Roles,
4345
UserID:user.ID,

‎coderd/rbac/authz.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package rbac
33
import (
44
"context"
55
_"embed"
6+
67
"golang.org/x/xerrors"
78

89
"github.com/open-policy-agent/opa/rego"

‎coderd/rbac/builtin.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,12 @@ var (
135135
Action:ActionRead,
136136
ResourceID:"*",
137137
},
138+
{
139+
// Can read available roles.
140+
ResourceType:ResourceOrgRoleAssignment.Type,
141+
ResourceID:"*",
142+
Action:ActionRead,
143+
},
138144
},
139145
},
140146
}
@@ -217,6 +223,37 @@ func SiteRoles() []Role {
217223
returnroles
218224
}
219225

226+
// ChangeRoleSet is a helper function that finds the difference of 2 sets of
227+
// roles. When setting a user's new roles, it is equivalent to adding and
228+
// removing roles. This set determines the changes, so that the appropriate
229+
// RBAC checks can be applied using "ActionCreate" and "ActionDelete" for
230+
// "added" and "removed" roles respectively.
231+
funcChangeRoleSet(from []string,to []string) (added []string,removed []string) {
232+
has:=make(map[string]struct{})
233+
for_,exists:=rangefrom {
234+
has[exists]=struct{}{}
235+
}
236+
237+
for_,roleName:=rangeto {
238+
// If the user already has the role assigned, we don't need to check the permission
239+
// to reassign it. Only run permission checks on the difference in the set of
240+
// roles.
241+
if_,ok:=has[roleName];ok {
242+
delete(has,roleName)
243+
continue
244+
}
245+
246+
added=append(added,roleName)
247+
}
248+
249+
// Remaining roles are the ones removed/deleted.
250+
forroleName:=rangehas {
251+
removed=append(removed,roleName)
252+
}
253+
254+
returnadded,removed
255+
}
256+
220257
// roleName is a quick helper function to return
221258
// role_name:scopeID
222259
// If no scopeID is required, only 'role_name' is returned

0 commit comments

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