Skip to content

Commit

Permalink
feat: Prevent role changing on yourself. (#1931)
Browse files Browse the repository at this point in the history
* feat: Prevent role changing on yourself.

Only allow changing roles on other users. Not much value in self changing
at the moment
  • Loading branch information
Emyrk committed May 31, 2022
1 parent 4b0ed06 commit 7acb742
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 4 deletions.
8 changes: 8 additions & 0 deletions coderd/members.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
user := httpmw.UserParam(r)
organization := httpmw.OrganizationParam(r)
member := httpmw.OrganizationMemberParam(r)
apiKey := httpmw.APIKey(r)

if apiKey.UserID == member.UserID {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: "You cannot change your own organization roles.",
})
return
}

var params codersdk.UpdateRoles
if !httpapi.Read(rw, r, &params) {
Expand Down
8 changes: 8 additions & 0 deletions coderd/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,14 @@ func (api *API) putUserRoles(rw http.ResponseWriter, r *http.Request) {
// User is the user to modify.
user := httpmw.UserParam(r)
roles := httpmw.UserRoles(r)
apiKey := httpmw.APIKey(r)

if apiKey.UserID == user.ID {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: "You cannot change your own roles.",
})
return
}

var params codersdk.UpdateRoles
if !httpapi.Read(rw, r, &params) {
Expand Down
20 changes: 16 additions & 4 deletions coderd/users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -409,11 +409,11 @@ func TestGrantRoles(t *testing.T) {
t.Run("UpdateIncorrectRoles", func(t *testing.T) {
t.Parallel()
ctx := context.Background()
var err error

admin := coderdtest.New(t, nil)
first := coderdtest.CreateFirstUser(t, admin)
member := coderdtest.CreateAnotherUser(t, admin, first.OrganizationID)
memberUser, err := member.User(ctx, codersdk.Me)
require.NoError(t, err, "member user")

_, err = admin.UpdateUserRoles(ctx, codersdk.Me, codersdk.UpdateRoles{
Roles: []string{rbac.RoleOrgMember(first.OrganizationID)},
Expand Down Expand Up @@ -445,7 +445,7 @@ func TestGrantRoles(t *testing.T) {
require.Error(t, err, "member cannot change other's roles")
requireStatusCode(t, err, http.StatusForbidden)

_, err = member.UpdateUserRoles(ctx, memberUser.ID.String(), codersdk.UpdateRoles{
_, err = member.UpdateUserRoles(ctx, first.UserID.String(), codersdk.UpdateRoles{
Roles: []string{rbac.RoleMember()},
})
require.Error(t, err, "member cannot change any roles")
Expand All @@ -456,6 +456,18 @@ func TestGrantRoles(t *testing.T) {
})
require.Error(t, err, "member cannot change other's org roles")
requireStatusCode(t, err, http.StatusForbidden)

_, err = admin.UpdateUserRoles(ctx, first.UserID.String(), codersdk.UpdateRoles{
Roles: []string{},
})
require.Error(t, err, "admin cannot change self roles")
requireStatusCode(t, err, http.StatusBadRequest)

_, err = admin.UpdateOrganizationMemberRoles(ctx, first.OrganizationID, first.UserID.String(), codersdk.UpdateRoles{
Roles: []string{},
})
require.Error(t, err, "admin cannot change self org roles")
requireStatusCode(t, err, http.StatusBadRequest)
})

t.Run("FirstUserRoles", func(t *testing.T) {
Expand Down Expand Up @@ -508,7 +520,7 @@ func TestGrantRoles(t *testing.T) {
require.NoError(t, err, "grant member admin role")

// Promote to org admin
_, err = member.UpdateOrganizationMemberRoles(ctx, first.OrganizationID, codersdk.Me, codersdk.UpdateRoles{
_, err = admin.UpdateOrganizationMemberRoles(ctx, first.OrganizationID, memberUser.ID.String(), codersdk.UpdateRoles{
Roles: []string{
// Promote to org admin
rbac.RoleOrgMember(first.OrganizationID),
Expand Down

0 comments on commit 7acb742

Please sign in to comment.