Skip to content

Commit afd9d3b

Browse files
EmyrkEmyrk
authored
feat: add api for patching custom org roles (coder#13357)
* chore: implement patching custom organization roles
1 parent b69f635 commit afd9d3b

File tree

16 files changed

+565
-464
lines changed

16 files changed

+565
-464
lines changed

‎coderd/apidoc/docs.go

Lines changed: 44 additions & 40 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 40 additions & 36 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/coderd.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,7 @@ func New(options *Options) *API {
424424
TemplateScheduleStore: options.TemplateScheduleStore,
425425
UserQuietHoursScheduleStore: options.UserQuietHoursScheduleStore,
426426
AccessControlStore: options.AccessControlStore,
427+
CustomRoleHandler: atomic.Pointer[CustomRoleHandler]{},
427428
Experiments: experiments,
428429
healthCheckGroup: &singleflight.Group[string, *healthsdk.HealthcheckReport]{},
429430
Acquirer: provisionerdserver.NewAcquirer(
@@ -436,6 +437,8 @@ func New(options *Options) *API {
436437
workspaceUsageTracker: options.WorkspaceUsageTracker,
437438
}
438439

440+
var customRoleHandler CustomRoleHandler = &agplCustomRoleHandler{}
441+
api.CustomRoleHandler.Store(&customRoleHandler)
439442
api.AppearanceFetcher.Store(&appearance.DefaultFetcher)
440443
api.PortSharer.Store(&portsharing.DefaultPortSharer)
441444
buildInfo := codersdk.BuildInfoResponse{
@@ -828,7 +831,12 @@ func New(options *Options) *API {
828831
})
829832
})
830833
r.Route("/members", func(r chi.Router) {
831-
r.Get("/roles", api.assignableOrgRoles)
834+
r.Route("/roles", func(r chi.Router) {
835+
r.Get("/", api.assignableOrgRoles)
836+
r.With(httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentCustomRoles)).
837+
Patch("/", api.patchOrgRoles)
838+
})
839+
832840
r.Route("/{user}", func(r chi.Router) {
833841
r.Use(
834842
httpmw.ExtractOrganizationMemberParam(options.Database),
@@ -1249,6 +1257,8 @@ type API struct {
12491257
// passed to dbauthz.
12501258
AccessControlStore *atomic.Pointer[dbauthz.AccessControlStore]
12511259
PortSharer atomic.Pointer[portsharing.PortSharer]
1260+
// CustomRoleHandler is the AGPL/Enterprise implementation for custom roles.
1261+
CustomRoleHandler atomic.Pointer[CustomRoleHandler]
12521262

12531263
HTTPAuth *HTTPAuthorizer
12541264

‎coderd/database/db2sdk/db2sdk.go

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -531,12 +531,16 @@ func Role(role rbac.Role) codersdk.Role {
531531
if err != nil {
532532
roleName = role.Name
533533
}
534+
534535
return codersdk.Role{
535-
Name: roleName,
536-
OrganizationID: orgIDStr,
537-
DisplayName: role.DisplayName,
538-
SitePermissions: List(role.Site, Permission),
539-
OrganizationPermissions: Map(role.Org, ListLazy(Permission)),
536+
Name: roleName,
537+
OrganizationID: orgIDStr,
538+
DisplayName: role.DisplayName,
539+
SitePermissions: List(role.Site, Permission),
540+
// This is not perfect. If there are organization permissions in another
541+
// organization, they will be omitted. This should not be allowed, so
542+
// should never happen.
543+
OrganizationPermissions: List(role.Org[orgIDStr], Permission),
540544
UserPermissions: List(role.User, Permission),
541545
}
542546
}
@@ -550,11 +554,18 @@ func Permission(permission rbac.Permission) codersdk.Permission {
550554
}
551555

552556
func RoleToRBAC(role codersdk.Role) rbac.Role {
557+
orgPerms := map[string][]rbac.Permission{}
558+
if role.OrganizationID != "" {
559+
orgPerms = map[string][]rbac.Permission{
560+
role.OrganizationID: List(role.OrganizationPermissions, PermissionToRBAC),
561+
}
562+
}
563+
553564
return rbac.Role{
554565
Name: rbac.RoleName(role.Name, role.OrganizationID),
555566
DisplayName: role.DisplayName,
556567
Site: List(role.SitePermissions, PermissionToRBAC),
557-
Org: Map(role.OrganizationPermissions, ListLazy(PermissionToRBAC)),
568+
Org: orgPerms,
558569
User: List(role.UserPermissions, PermissionToRBAC),
559570
}
560571
}

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -600,14 +600,29 @@ 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 orgID == nil {
618+
return xerrors.Errorf("should never happen, orgID is nil, but trying to assign an organization role")
619+
}
620+
621+
if roleOrgID != *orgID {
622+
return xerrors.Errorf("attempted to assign role from a different org, role %q to %q", r, orgID.String())
623+
}
624+
}
625+
611626
// All roles should be valid roles
612627
if _, err := rbac.RoleByName(r); err != nil {
613628
customRoles = append(customRoles, r)

‎coderd/members.go

Lines changed: 1 addition & 36 deletions
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,

0 commit comments

Comments
 (0)