Skip to content

Commit f91ffe5

Browse files
EmyrkEmyrk
committed
fixup test to assign org role instead of site
1 parent 4a38c95 commit f91ffe5

File tree

7 files changed

+81
-183
lines changed

7 files changed

+81
-183
lines changed

‎coderd/coderd.go

+6-3
Original file line numberDiff line numberDiff line change
@@ -826,9 +826,12 @@ func New(options *Options) *API {
826826
})
827827
})
828828
r.Route("/members", func(r chi.Router) {
829-
r.Get("/roles", api.assignableOrgRoles)
830-
r.With(httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentCustomRoles)).
831-
Patch("/roles", api.patchOrgRoles)
829+
r.Route("/roles", func(r chi.Router) {
830+
r.Get("/", api.assignableOrgRoles)
831+
r.With(httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentCustomRoles)).
832+
Patch("/", api.patchOrgRoles)
833+
})
834+
832835
r.Route("/{user}", func(r chi.Router) {
833836
r.Use(
834837
httpmw.ExtractOrganizationMemberParam(options.Database),

‎coderd/database/dbauthz/dbauthz.go

+12-1
Original file line numberDiff line numberDiff line change
@@ -600,14 +600,25 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r
600600
customRoles := make([]string, 0)
601601
// Validate that the roles being assigned are valid.
602602
for _, r := range grantedRoles {
603-
_, isOrgRole := rbac.IsOrgRole(r)
603+
roleOrgIDStr, isOrgRole := rbac.IsOrgRole(r)
604604
if shouldBeOrgRoles && !isOrgRole {
605605
return xerrors.Errorf("Must only update org roles")
606606
}
607607
if !shouldBeOrgRoles && isOrgRole {
608608
return xerrors.Errorf("Must only update site wide roles")
609609
}
610610

611+
if shouldBeOrgRoles {
612+
roleOrgID, err := uuid.Parse(roleOrgIDStr)
613+
if err != nil {
614+
return xerrors.Errorf("role %q has invalid uuid for org: %w", r, err)
615+
}
616+
617+
if roleOrgID != *orgID {
618+
return xerrors.Errorf("attempted to assign role from a different org, role %q to %q", r, (*orgID).String())
619+
}
620+
}
621+
611622
// All roles should be valid roles
612623
if _, err := rbac.RoleByName(r); err != nil {
613624
customRoles = append(customRoles, r)

‎coderd/members.go

+1-36
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,8 @@
11
package coderd
22

33
import (
4-
"context"
54
"net/http"
65

7-
"github.com/google/uuid"
8-
9-
"golang.org/x/xerrors"
10-
116
"github.com/coder/coder/v2/coderd/database/db2sdk"
127
"github.com/coder/coder/v2/coderd/rbac"
138

@@ -48,7 +43,7 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
4843
return
4944
}
5045

51-
updatedUser, err := api.updateOrganizationMemberRoles(ctx, database.UpdateMemberRolesParams{
46+
updatedUser, err := api.Database.UpdateMemberRoles(ctx, database.UpdateMemberRolesParams{
5247
GrantedRoles: params.Roles,
5348
UserID: member.UserID,
5449
OrgID: organization.ID,
@@ -63,36 +58,6 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
6358
httpapi.Write(ctx, rw, http.StatusOK, convertOrganizationMember(updatedUser))
6459
}
6560

66-
func (api *API) updateOrganizationMemberRoles(ctx context.Context, args database.UpdateMemberRolesParams) (database.OrganizationMember, error) {
67-
// Enforce only site wide roles
68-
for _, r := range args.GrantedRoles {
69-
// Must be an org role for the org in the args
70-
orgID, ok := rbac.IsOrgRole(r)
71-
if !ok {
72-
return database.OrganizationMember{}, xerrors.Errorf("must only update organization roles")
73-
}
74-
75-
roleOrg, err := uuid.Parse(orgID)
76-
if err != nil {
77-
return database.OrganizationMember{}, xerrors.Errorf("Role must have proper UUIDs for organization, %q does not", r)
78-
}
79-
80-
if roleOrg != args.OrgID {
81-
return database.OrganizationMember{}, xerrors.Errorf("Must only pass roles for org %q", args.OrgID.String())
82-
}
83-
84-
if _, err := rbac.RoleByName(r); err != nil {
85-
return database.OrganizationMember{}, xerrors.Errorf("%q is not a supported organization role", r)
86-
}
87-
}
88-
89-
updatedUser, err := api.Database.UpdateMemberRoles(ctx, args)
90-
if err != nil {
91-
return database.OrganizationMember{}, xerrors.Errorf("Update site roles: %w", err)
92-
}
93-
return updatedUser, nil
94-
}
95-
9661
func convertOrganizationMember(mem database.OrganizationMember) codersdk.OrganizationMember {
9762
convertedMember := codersdk.OrganizationMember{
9863
UserID: mem.UserID,

‎codersdk/roles.go

+15-3
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,21 @@ type Role struct {
4444
UserPermissions []Permission `json:"user_permissions" table:"user_permissions"`
4545
}
4646

47-
// PatchRole will upsert a custom site wide role
48-
func (c *Client) PatchRole(ctx context.Context, req Role) (Role, error) {
49-
res, err := c.Request(ctx, http.MethodPatch, "/api/v2/users/roles", req)
47+
// FullName returns the role name scoped to the organization ID. This is useful if
48+
// printing a set of roles from different scopes, as duplicated names across multiple
49+
// scopes will become unique.
50+
// In practice, this is primarily used in testing.
51+
func (r Role) FullName() string {
52+
if r.OrganizationID == "" {
53+
return r.Name
54+
}
55+
return r.Name + ":" + r.OrganizationID
56+
}
57+
58+
// PatchOrganizationRole will upsert a custom organization role
59+
func (c *Client) PatchOrganizationRole(ctx context.Context, organizationID uuid.UUID, req Role) (Role, error) {
60+
res, err := c.Request(ctx, http.MethodPatch,
61+
fmt.Sprintf("/api/v2/organizations/%s/members/roles", organizationID.String()), req)
5062
if err != nil {
5163
return Role{}, err
5264
}

‎enterprise/coderd/coderd.go

-16
Original file line numberDiff line numberDiff line change
@@ -327,22 +327,6 @@ func New(ctx context.Context, options *Options) (_ *API, err error) {
327327
})
328328
})
329329

330-
r.Route("/users/roles", func(r chi.Router) {
331-
r.Use(
332-
apiKeyMiddleware,
333-
)
334-
r.Group(func(r chi.Router) {
335-
r.Use(
336-
api.customRolesEnabledMW,
337-
)
338-
r.Patch("/", api.patchRole)
339-
})
340-
// Unfortunate, but this r.Route overrides the AGPL roles route.
341-
// The AGPL does not have the entitlements to block the licensed
342-
// routes, so we need to duplicate the AGPL here.
343-
r.Get("/", api.AGPL.AssignableSiteRoles)
344-
})
345-
346330
r.Route("/users/{user}/quiet-hours", func(r chi.Router) {
347331
r.Use(
348332
api.autostopRequirementEnabledMW,

‎enterprise/coderd/roles.go

-77
Original file line numberDiff line numberDiff line change
@@ -107,80 +107,3 @@ func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context,
107107

108108
return db2sdk.Role(convertedInsert), true
109109
}
110-
111-
// patchRole will allow creating a custom role
112-
//
113-
// @Summary Upsert a custom site-wide role
114-
// @ID upsert-a-custom-site-wide-role
115-
// @Security CoderSessionToken
116-
// @Produce json
117-
// @Tags Members
118-
// @Success 200 {array} codersdk.Role
119-
// @Router /users/roles [patch]
120-
func (api *API) patchRole(rw http.ResponseWriter, r *http.Request) {
121-
ctx := r.Context()
122-
123-
var req codersdk.Role
124-
if !httpapi.Read(ctx, rw, r, &req) {
125-
return
126-
}
127-
128-
if err := httpapi.NameValid(req.Name); err != nil {
129-
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
130-
Message: "Invalid role name",
131-
Detail: err.Error(),
132-
})
133-
return
134-
}
135-
136-
if len(req.OrganizationPermissions) > 0 {
137-
// Org perms should be assigned only in org specific roles. Otherwise,
138-
// it gets complicated to keep track of who can do what.
139-
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
140-
Message: "Invalid request, not allowed to assign organization permissions for a site wide role.",
141-
Detail: "site wide roles may not contain organization specific permissions",
142-
})
143-
return
144-
}
145-
146-
// Make sure all permissions inputted are valid according to our policy.
147-
rbacRole := db2sdk.RoleToRBAC(req)
148-
args, err := rolestore.ConvertRoleToDB(rbacRole)
149-
if err != nil {
150-
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
151-
Message: "Invalid request",
152-
Detail: err.Error(),
153-
})
154-
return
155-
}
156-
157-
inserted, err := api.Database.UpsertCustomRole(ctx, database.UpsertCustomRoleParams{
158-
Name: args.Name,
159-
DisplayName: args.DisplayName,
160-
SitePermissions: args.SitePermissions,
161-
OrgPermissions: args.OrgPermissions,
162-
UserPermissions: args.UserPermissions,
163-
})
164-
if httpapi.Is404Error(err) {
165-
httpapi.ResourceNotFound(rw)
166-
return
167-
}
168-
if err != nil {
169-
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
170-
Message: "Failed to update role permissions",
171-
Detail: err.Error(),
172-
})
173-
return
174-
}
175-
176-
convertedInsert, err := rolestore.ConvertDBRole(inserted)
177-
if err != nil {
178-
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
179-
Message: "Permissions were updated, unable to read them back out of the database.",
180-
Detail: err.Error(),
181-
})
182-
return
183-
}
184-
185-
httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Role(convertedInsert))
186-
}

0 commit comments

Comments
 (0)