Skip to content

Commit 1f7d5d3

Browse files
EmyrkEmyrk
committed
Handle assigning/removing roles
1 parent 44423b3 commit 1f7d5d3

File tree

6 files changed

+77
-14
lines changed

6 files changed

+77
-14
lines changed

‎coderd/coderd_test.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -542,9 +542,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
542542
if routeAssertions.AssertObject.OrgID != "" {
543543
assert.Equal(t, routeAssertions.AssertObject.OrgID, authorizer.Called.Object.OrgID, "resource org")
544544
}
545-
if routeAssertions.AssertObject.ResourceID != "" {
546-
assert.Equal(t, routeAssertions.AssertObject.ResourceID, authorizer.Called.Object.ResourceID, "resource ID")
547-
}
548545
}
549546
} else {
550547
assert.Nil(t, authorizer.Called, "authorize not expected")

‎coderd/members.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
2121
organization := httpmw.OrganizationParam(r)
2222
member := httpmw.OrganizationMemberParam(r)
2323
apiKey := httpmw.APIKey(r)
24+
actorRoles := httpmw.AuthorizationUserRoles(r)
2425

2526
if apiKey.UserID == member.UserID {
2627
httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
@@ -41,17 +42,24 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
4142
// TODO: Handle added and removed roles.
4243

4344
// Assigning a role requires the create permission.
44-
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceOrgRoleAssignment.InOrg(organization.ID)) {
45+
if len(added) > 0 && !api.Authorize(r, rbac.ActionCreate, rbac.ResourceOrgRoleAssignment.InOrg(organization.ID)) {
4546
httpapi.Forbidden(rw)
4647
return
4748
}
4849

4950
// Removing a role requires the delete permission.
50-
if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceOrgRoleAssignment.InOrg(organization.ID)) {
51+
if len(removed) > 0 && !api.Authorize(r, rbac.ActionDelete, rbac.ResourceOrgRoleAssignment.InOrg(organization.ID)) {
5152
httpapi.Forbidden(rw)
5253
return
5354
}
5455

56+
// Just treat adding & removing as "assigning" for now.
57+
for _, roleName := range append(added, removed...) {
58+
if !rbac.CanAssignRole(actorRoles.Roles, roleName) {
59+
httpapi.Forbidden(rw)
60+
}
61+
}
62+
5563
updatedUser, err := api.updateOrganizationMemberRoles(r.Context(), database.UpdateMemberRolesParams{
5664
GrantedRoles: params.Roles,
5765
UserID: user.ID,

‎coderd/rbac/builtin.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,58 @@ var (
145145
}
146146
)
147147

148+
var (
149+
// assignRoles is a map of roles that can be assigned if a user has a given
150+
// role.
151+
// The first key is the actor role, the second is the roles they can assign.
152+
// map[actor_role][assign_role]<can_assign>
153+
assignRoles = map[string]map[string]bool{
154+
admin: {
155+
admin: true,
156+
auditor: true,
157+
member: true,
158+
orgAdmin: true,
159+
orgMember: true,
160+
},
161+
orgAdmin: {
162+
orgAdmin: true,
163+
orgMember: true,
164+
},
165+
}
166+
)
167+
168+
// CanAssignRole is a helper function that returns true if the user can assign
169+
// the specified role. This also can be used for removing a role.
170+
// This is a simple implementation for now.
171+
func CanAssignRole(roles []string, assignedRole string) bool {
172+
assigned, assignedOrg, err := roleSplit(assignedRole)
173+
if err != nil {
174+
return false
175+
}
176+
177+
for _, longRole := range roles {
178+
role, orgID, err := roleSplit(longRole)
179+
if err != nil {
180+
continue
181+
}
182+
183+
if orgID != "" && orgID != assignedOrg {
184+
// Org roles only apply to the org they are assigned to.
185+
continue
186+
}
187+
188+
allowed, ok := assignRoles[role]
189+
if !ok {
190+
continue
191+
}
192+
193+
if allowed[assigned] {
194+
return true
195+
}
196+
}
197+
return false
198+
}
199+
148200
// RoleByName returns the permissions associated with a given role name.
149201
// This allows just the role names to be stored and expanded when required.
150202
func RoleByName(name string) (Role, error) {

‎coderd/rbac/object.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ var (
8888
}
8989

9090
// ResourceOrganizationMember is a user's membership in an organization.
91-
// Has ONLY an organization owner. The resource ID is the user's ID
91+
// Has ONLY an organization owner.
9292
// create/delete = Create/delete member from org.
9393
// update = Update organization member
9494
// read = View member

‎coderd/roles.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,9 @@ func (api *API) checkPermissions(rw http.ResponseWriter, r *http.Request) {
7474
}
7575
err := api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, rbac.Action(v.Action),
7676
rbac.Object{
77-
ResourceID: v.Object.ResourceID,
78-
Owner: v.Object.OwnerID,
79-
OrgID: v.Object.OrganizationID,
80-
Type: v.Object.ResourceType,
77+
Owner: v.Object.OwnerID,
78+
OrgID: v.Object.OrganizationID,
79+
Type: v.Object.ResourceType,
8180
})
8281
response[k] = err == nil
8382
}

‎coderd/users.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,7 @@ func (api *API) userRoles(rw http.ResponseWriter, r *http.Request) {
504504
func (api *API) putUserRoles(rw http.ResponseWriter, r *http.Request) {
505505
// User is the user to modify.
506506
user := httpmw.UserParam(r)
507-
roles := httpmw.AuthorizationUserRoles(r)
507+
actorRoles := httpmw.AuthorizationUserRoles(r)
508508
apiKey := httpmw.APIKey(r)
509509

510510
if apiKey.UserID == user.ID {
@@ -526,21 +526,28 @@ func (api *API) putUserRoles(rw http.ResponseWriter, r *http.Request) {
526526

527527
// The member role is always implied.
528528
impliedTypes := append(params.Roles, rbac.RoleMember())
529-
added, removed := rbac.ChangeRoleSet(roles.Roles, impliedTypes)
529+
added, removed := rbac.ChangeRoleSet(user.RBACRoles, impliedTypes)
530530
// TODO: Handle added and removed roles.
531531

532532
// Assigning a role requires the create permission.
533-
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceRoleAssignment) {
533+
if len(added) > 0 && !api.Authorize(r, rbac.ActionCreate, rbac.ResourceRoleAssignment) {
534534
httpapi.Forbidden(rw)
535535
return
536536
}
537537

538538
// Removing a role requires the delete permission.
539-
if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceRoleAssignment) {
539+
if len(removed) > 0 && !api.Authorize(r, rbac.ActionDelete, rbac.ResourceRoleAssignment) {
540540
httpapi.Forbidden(rw)
541541
return
542542
}
543543

544+
// Just treat adding & removing as "assigning" for now.
545+
for _, roleName := range append(added, removed...) {
546+
if !rbac.CanAssignRole(actorRoles.Roles, roleName) {
547+
httpapi.Forbidden(rw)
548+
}
549+
}
550+
544551
updatedUser, err := api.updateSiteUserRoles(r.Context(), database.UpdateUserRolesParams{
545552
GrantedRoles: params.Roles,
546553
ID: user.ID,

0 commit comments

Comments
 (0)