Skip to content

Commit be5b0b3

Browse files
EmyrkEmyrk
committed
Rbac users endpoints
Skip workspace agent endpoints
1 parent 01b2c94 commit be5b0b3

File tree

6 files changed

+162
-48
lines changed

6 files changed

+162
-48
lines changed

‎coderd/coderd.go

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func New(options *Options) (http.Handler, func()) {
133133
httpmw.RateLimitPerMinute(12),
134134
// TODO: @emyrk (rbac) Currently files are owned by the site?
135135
// Should files be org scoped? User scoped?
136-
httpmw.WithRBACObject(rbac.ResourceTypeFile),
136+
httpmw.WithRBACObject(rbac.ResourceFile),
137137
)
138138
r.Get("/{hash}", authorize(api.fileByHash, rbac.ActionRead))
139139
r.Post("/", authorize(api.postFile, rbac.ActionCreate, rbac.ActionUpdate))
@@ -237,36 +237,55 @@ func New(options *Options) (http.Handler, func()) {
237237
apiKeyMiddleware,
238238
authRolesMiddleware,
239239
)
240-
r.Post("/", api.postUser)
241-
r.Get("/", api.users)
240+
r.Group(func(r chi.Router) {
241+
// Site wide, all users
242+
r.Use(httpmw.WithRBACObject(rbac.ResourceUser))
243+
r.Post("/", authorize(api.postUser, rbac.ActionCreate))
244+
r.Get("/", authorize(api.users, rbac.ActionRead))
245+
})
242246
// These routes query information about site wide roles.
243247
r.Route("/roles", func(r chi.Router) {
244248
r.Use(httpmw.WithRBACObject(rbac.ResourceUserRole))
245249
r.Get("/", authorize(api.assignableSiteRoles, rbac.ActionRead))
246250
})
247251
r.Route("/{user}", func(r chi.Router) {
248252
r.Use(httpmw.ExtractUserParam(options.Database))
249-
r.Get("/", api.userByName)
250-
r.Put("/profile", api.putUserProfile)
251-
r.Put("/suspend", api.putUserSuspend)
252-
r.Route("/password", func(r chi.Router) {
253-
r.Use(httpmw.WithRBACObject(rbac.ResourceUserPasswordRole))
254-
r.Put("/", authorize(api.putUserPassword, rbac.ActionUpdate))
253+
r.Group(func(r chi.Router) {
254+
r.Use(httpmw.WithRBACObject(rbac.ResourceUser))
255+
r.Get("/", authorize(api.userByName, rbac.ActionRead))
256+
r.Put("/profile", authorize(api.putUserProfile, rbac.ActionUpdate))
257+
// suspension is deleting for a user
258+
r.Put("/suspend", authorize(api.putUserSuspend, rbac.ActionDelete))
259+
r.Route("/password", func(r chi.Router) {
260+
r.Put("/", authorize(api.putUserPassword, rbac.ActionUpdate))
261+
})
262+
// This route technically also fetches the organization member struct, but only
263+
// returns the roles.
264+
r.Get("/roles", authorize(api.userRoles, rbac.ActionRead))
265+
266+
// This has 2 authorize calls. The second is explicitly called
267+
// in the handler.
268+
r.Put("/roles", authorize(api.putUserRoles, rbac.ActionUpdate))
269+
270+
// For now, just use the "user" role for their ssh keys.
271+
// We can always split this out to it's own resource if we need to.
272+
r.Get("/gitsshkey", authorize(api.gitSSHKey, rbac.ActionRead))
273+
r.Put("/gitsshkey", authorize(api.regenerateGitSSHKey, rbac.ActionUpdate))
255274
})
256-
r.Get("/organizations", api.organizationsByUser)
257-
r.Post("/organizations", api.postOrganizationsByUser)
258-
// These roles apply to the site wide permissions.
259-
r.Put("/roles", api.putUserRoles)
260-
r.Get("/roles", api.userRoles)
261275

262-
r.Post("/keys", api.postAPIKey)
276+
r.With(httpmw.WithRBACObject(rbac.ResourceAPIKey)).Post("/keys", authorize(api.postAPIKey, rbac.ActionCreate))
277+
263278
r.Route("/organizations", func(r chi.Router) {
279+
// TODO: @emyrk This creates an organization, so why is it nested under {user}?
280+
// Shouldn't this be outside the {user} param subpath? Maybe in the organizations/
281+
// path?
264282
r.Post("/", api.postOrganizationsByUser)
283+
265284
r.Get("/", api.organizationsByUser)
285+
286+
// TODO: @emyrk why is this nested under {user} when the user param is not used?
266287
r.Get("/{organizationname}", api.organizationByUserAndName)
267288
})
268-
r.Get("/gitsshkey", api.gitSSHKey)
269-
r.Put("/gitsshkey", api.regenerateGitSSHKey)
270289
r.Get("/workspaces", api.workspacesByUser)
271290
})
272291
})

‎coderd/coderd_test.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,22 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
5555
"POST:/api/v2/users/logout": {NoAuthorize: true},
5656
"GET:/api/v2/users/authmethods": {NoAuthorize: true},
5757

58+
// All workspaceagents endpoints do not use rbac
59+
"POST:/api/v2/workspaceagents/aws-instance-identity": {NoAuthorize: true},
60+
"POST:/api/v2/workspaceagents/azure-instance-identity": {NoAuthorize: true},
61+
"POST:/api/v2/workspaceagents/google-instance-identity": {NoAuthorize: true},
62+
"GET:/api/v2/workspaceagents/me/gitsshkey": {NoAuthorize: true},
63+
"GET:/api/v2/workspaceagents/me/iceservers": {NoAuthorize: true},
64+
"GET:/api/v2/workspaceagents/me/listen": {NoAuthorize: true},
65+
"GET:/api/v2/workspaceagents/me/metadata": {NoAuthorize: true},
66+
"GET:/api/v2/workspaceagents/me/turn": {NoAuthorize: true},
67+
"GET:/api/v2/workspaceagents/{workspaceagent}": {NoAuthorize: true},
68+
"GET:/api/v2/workspaceagents/{workspaceagent}/": {NoAuthorize: true},
69+
"GET:/api/v2/workspaceagents/{workspaceagent}/dial": {NoAuthorize: true},
70+
"GET:/api/v2/workspaceagents/{workspaceagent}/iceservers": {NoAuthorize: true},
71+
"GET:/api/v2/workspaceagents/{workspaceagent}/pty": {NoAuthorize: true},
72+
"GET:/api/v2/workspaceagents/{workspaceagent}/turn": {NoAuthorize: true},
73+
5874
// TODO: @emyrk these need to be fixed by adding authorize calls
5975
"/api/v2/organizations/{organization}/provisionerdaemons": {NoAuthorize: true},
6076
"GET:/api/v2/organizations/{organization}": {AssertObject: rbac.ResourceOrganization.InOrg(admin.OrganizationID)},
@@ -71,8 +87,9 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
7187
routeAssertions = routeCheck{}
7288
}
7389

74-
// Replace all url params with expected
90+
// Replace all url params with known values
7591
route = strings.ReplaceAll(route, "{organization}", admin.OrganizationID.String())
92+
route = strings.ReplaceAll(route, "{user}", admin.UserID.String())
7693

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

‎coderd/coderdtest/coderdtest.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,13 @@ import (
5555

5656
type Options struct {
5757
AWSCertificates awsidentity.Certificates
58+
Authorizer rbac.Authorizer
5859
AzureCertificates x509.VerifyOptions
5960
GithubOAuth2Config *coderd.GithubOAuth2Config
6061
GoogleTokenValidator *idtoken.Validator
6162
SSHKeygenAlgorithm gitsshkey.Algorithm
6263
APIRateLimit int
63-
Authorizer rbac.Authorizer
64+
LifecycleTicker <-chan time.Time
6465
}
6566

6667
// New constructs an in-memory coderd instance and returns

‎coderd/rbac/builtin.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ var (
6464
return Role{
6565
Name: member,
6666
DisplayName: "Member",
67+
Site: permissions(map[Object][]Action{
68+
// All users can read all organizations.
69+
// TODO: @emyrk is this ok?
70+
ResourceOrganization: {ActionRead},
71+
}),
6772
User: permissions(map[Object][]Action{
6873
ResourceWildcard: {WildcardSymbol},
6974
}),
@@ -111,7 +116,18 @@ var (
111116
Name: roleName(orgMember, organizationID),
112117
DisplayName: "Organization Member",
113118
Org: map[string][]Permission{
114-
organizationID: {},
119+
organizationID: {
120+
{
121+
// All org members can read the other members in their org.
122+
ResourceType: ResourceOrganizationMember.Type,
123+
Action: ActionRead,
124+
},
125+
{
126+
// All org members can read the organization
127+
ResourceType: ResourceOrganization.Type,
128+
Action: ActionRead,
129+
},
130+
},
115131
},
116132
}
117133
},

‎coderd/rbac/object.go

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,13 @@ var (
2121
Type: "template",
2222
}
2323

24-
ResourceTypeFile = Object{
24+
ResourceFile = Object{
2525
Type: "file",
2626
}
2727

28-
// ResourceOrganization CRUD. Org owner
28+
// ResourceOrganization CRUD. Always has an org owner.
2929
// create/delete = make or delete organizations
30-
// read = view org information
30+
// read = view org information (Can add user owner for read)
3131
// update = ??
3232
ResourceOrganization = Object{
3333
Type: "organization",
@@ -36,12 +36,38 @@ var (
3636
// ResourceUserRole might be expanded later to allow more granular permissions
3737
// to modifying roles. For now, this covers all possible roles, so having this permission
3838
// allows granting/deleting **ALL** roles.
39+
// create = Assign roles
40+
// update = ??
41+
// read = View available roles to assign
42+
// delete = Remove role
3943
ResourceUserRole = Object{
4044
Type: "user_role",
4145
}
4246

43-
ResourceUserPasswordRole = Object{
44-
Type: "user_password",
47+
// ResourceAPIKey is owned by a user.
48+
// create = Create a new api key for user
49+
// update = ??
50+
// read = View api key
51+
// delete = Delete api key
52+
ResourceAPIKey = Object{
53+
Type: "api_key",
54+
}
55+
56+
// ResourceUser is the user in the 'users' table.
57+
// create/delete = make or delete a new user.
58+
// read = view all user's settings
59+
// update = update all user field & settings
60+
ResourceUser = Object{
61+
Type: "user",
62+
}
63+
64+
// ResourceOrganizationMember is a user's membership in an organization.
65+
// Has ONLY an organization owner.
66+
// create/delete = Create/delete member from org.
67+
// update = Update organization member
68+
// read = View member
69+
ResourceOrganizationMember = Object{
70+
Type: "organization_member",
4571
}
4672

4773
// ResourceWildcard represents all resource types

‎coderd/users.go

Lines changed: 58 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -385,22 +385,51 @@ func (api *api) userRoles(rw http.ResponseWriter, r *http.Request) {
385385

386386
func (api *api) putUserRoles(rw http.ResponseWriter, r *http.Request) {
387387
// User is the user to modify
388-
// TODO: Until rbac authorize is implemented, only be able to change your
389-
// own roles. This also means you can grant yourself whatever roles you want.
390388
user := httpmw.UserParam(r)
391-
apiKey := httpmw.APIKey(r)
392-
if apiKey.UserID != user.ID {
393-
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
394-
Message: "modifying other users is not supported at this time",
395-
})
396-
return
397-
}
389+
roles := httpmw.UserRoles(r)
398390

399391
var params codersdk.UpdateRoles
400392
if !httpapi.Read(rw, r, &params) {
401393
return
402394
}
403395

396+
has := make(map[string]struct{})
397+
for _, exists := range roles.Roles {
398+
has[exists] = struct{}{}
399+
}
400+
401+
for _, roleName := range params.Roles {
402+
// If the user already has the role, we don't need to check the permission.
403+
if _, ok := has[roleName]; ok {
404+
delete(has, roleName)
405+
continue
406+
}
407+
408+
// Assigning a role requires the create permission. The middleware checks if
409+
// we can update this user, so the combination of the 2 permissions enables
410+
// assigning new roles.
411+
err := api.Authorizer.AuthorizeByRoleName(r.Context(), roles.ID.String(), roles.Roles,
412+
rbac.ActionCreate, rbac.ResourceUserRole.WithID(roleName))
413+
if err != nil {
414+
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
415+
Message: "unauthorized",
416+
})
417+
return
418+
}
419+
}
420+
421+
// Any roles that were removed also need to be checked.
422+
for roleName := range has {
423+
err := api.Authorizer.AuthorizeByRoleName(r.Context(), roles.ID.String(), roles.Roles,
424+
rbac.ActionDelete, rbac.ResourceUserRole.WithID(roleName))
425+
if err != nil {
426+
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
427+
Message: "unauthorized",
428+
})
429+
return
430+
}
431+
}
432+
404433
updatedUser, err := api.updateSiteUserRoles(r.Context(), database.UpdateUserRolesParams{
405434
GrantedRoles: params.Roles,
406435
ID: user.ID,
@@ -445,6 +474,7 @@ func (api *api) updateSiteUserRoles(ctx context.Context, args database.UpdateUse
445474
// Returns organizations the parameterized user has access to.
446475
func (api *api) organizationsByUser(rw http.ResponseWriter, r *http.Request) {
447476
user := httpmw.UserParam(r)
477+
roles := httpmw.UserRoles(r)
448478

449479
organizations, err := api.Database.GetOrganizationsByUserID(r.Context(), user.ID)
450480
if errors.Is(err, sql.ErrNoRows) {
@@ -460,14 +490,23 @@ func (api *api) organizationsByUser(rw http.ResponseWriter, r *http.Request) {
460490

461491
publicOrganizations := make([]codersdk.Organization, 0, len(organizations))
462492
for _, organization := range organizations {
463-
publicOrganizations = append(publicOrganizations, convertOrganization(organization))
493+
err := api.Authorizer.AuthorizeByRoleName(r.Context(), roles.ID.String(), roles.Roles, rbac.ActionRead,
494+
rbac.ResourceOrganization.
495+
WithID(organization.ID.String()).
496+
InOrg(organization.ID),
497+
)
498+
if err == nil {
499+
// Only return orgs the user can read
500+
publicOrganizations = append(publicOrganizations, convertOrganization(organization))
501+
}
464502
}
465503

466504
httpapi.Write(rw, http.StatusOK, publicOrganizations)
467505
}
468506

469507
func (api *api) organizationByUserAndName(rw http.ResponseWriter, r *http.Request) {
470-
user := httpmw.UserParam(r)
508+
roles := httpmw.UserRoles(r)
509+
471510
organizationName := chi.URLParam(r, "organizationname")
472511
organization, err := api.Database.GetOrganizationByName(r.Context(), organizationName)
473512
if errors.Is(err, sql.ErrNoRows) {
@@ -482,19 +521,15 @@ func (api *api) organizationByUserAndName(rw http.ResponseWriter, r *http.Reques
482521
})
483522
return
484523
}
485-
_, err = api.Database.GetOrganizationMemberByUserID(r.Context(), database.GetOrganizationMemberByUserIDParams{
486-
OrganizationID: organization.ID,
487-
UserID: user.ID,
488-
})
489-
if errors.Is(err, sql.ErrNoRows) {
490-
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
491-
Message: "you are not a member of that organization",
492-
})
493-
return
494-
}
524+
525+
err = api.Authorizer.AuthorizeByRoleName(r.Context(), roles.ID.String(), roles.Roles, rbac.ActionRead,
526+
rbac.ResourceOrganization.
527+
InOrg(organization.ID).
528+
WithID(organization.ID.String()),
529+
)
495530
if err != nil {
496-
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
497-
Message: fmt.Sprintf("get organization member: %s", err),
531+
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
532+
Message: err.Error(),
498533
})
499534
return
500535
}

0 commit comments

Comments
 (0)