Skip to content

Commit 009b9fc

Browse files
EmyrkEmyrk
committed
chore: refactor patch custom organization route to live in enterprise
This was before realizing single routes could be overwritten
1 parent f3ff172 commit 009b9fc

File tree

5 files changed

+58
-106
lines changed

5 files changed

+58
-106
lines changed

‎coderd/coderd.go

-7
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,6 @@ func New(options *Options) *API {
464464
TemplateScheduleStore: options.TemplateScheduleStore,
465465
UserQuietHoursScheduleStore: options.UserQuietHoursScheduleStore,
466466
AccessControlStore: options.AccessControlStore,
467-
CustomRoleHandler: atomic.Pointer[CustomRoleHandler]{},
468467
Experiments: experiments,
469468
healthCheckGroup: &singleflight.Group[string, *healthsdk.HealthcheckReport]{},
470469
Acquirer: provisionerdserver.NewAcquirer(
@@ -476,8 +475,6 @@ func New(options *Options) *API {
476475
dbRolluper: options.DatabaseRolluper,
477476
}
478477

479-
var customRoleHandler CustomRoleHandler = &agplCustomRoleHandler{}
480-
api.CustomRoleHandler.Store(&customRoleHandler)
481478
api.AppearanceFetcher.Store(&appearance.DefaultFetcher)
482479
api.PortSharer.Store(&portsharing.DefaultPortSharer)
483480
buildInfo := codersdk.BuildInfoResponse{
@@ -887,8 +884,6 @@ func New(options *Options) *API {
887884
r.Get("/", api.listMembers)
888885
r.Route("/roles", func(r chi.Router) {
889886
r.Get("/", api.assignableOrgRoles)
890-
r.With(httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentCustomRoles)).
891-
Patch("/", api.patchOrgRoles)
892887
})
893888

894889
r.Route("/{user}", func(r chi.Router) {
@@ -1327,8 +1322,6 @@ type API struct {
13271322
// passed to dbauthz.
13281323
AccessControlStore *atomic.Pointer[dbauthz.AccessControlStore]
13291324
PortSharer atomic.Pointer[portsharing.PortSharer]
1330-
// CustomRoleHandler is the AGPL/Enterprise implementation for custom roles.
1331-
CustomRoleHandler atomic.Pointer[CustomRoleHandler]
13321325

13331326
HTTPAuth *HTTPAuthorizer
13341327

‎coderd/roles.go

-47
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package coderd
22

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

76
"github.com/google/uuid"
@@ -16,52 +15,6 @@ import (
1615
"github.com/coder/coder/v2/coderd/rbac"
1716
)
1817

19-
// CustomRoleHandler handles AGPL/Enterprise interface for handling custom
20-
// roles. Ideally only included in the enterprise package, but the routes are
21-
// intermixed with AGPL endpoints.
22-
type CustomRoleHandler interface {
23-
PatchOrganizationRole(ctx context.Context, rw http.ResponseWriter, r *http.Request, orgID uuid.UUID, role codersdk.PatchRoleRequest) (codersdk.Role, bool)
24-
}
25-
26-
type agplCustomRoleHandler struct{}
27-
28-
func (agplCustomRoleHandler) PatchOrganizationRole(ctx context.Context, rw http.ResponseWriter, _ *http.Request, _ uuid.UUID, _ codersdk.PatchRoleRequest) (codersdk.Role, bool) {
29-
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
30-
Message: "Creating and updating custom roles is an Enterprise feature. Contact sales!",
31-
})
32-
return codersdk.Role{}, false
33-
}
34-
35-
// patchRole will allow creating a custom organization role
36-
//
37-
// @Summary Upsert a custom organization role
38-
// @ID upsert-a-custom-organization-role
39-
// @Security CoderSessionToken
40-
// @Produce json
41-
// @Param organization path string true "Organization ID" format(uuid)
42-
// @Tags Members
43-
// @Success 200 {array} codersdk.Role
44-
// @Router /organizations/{organization}/members/roles [patch]
45-
func (api *API) patchOrgRoles(rw http.ResponseWriter, r *http.Request) {
46-
var (
47-
ctx = r.Context()
48-
handler = *api.CustomRoleHandler.Load()
49-
organization = httpmw.OrganizationParam(r)
50-
)
51-
52-
var req codersdk.PatchRoleRequest
53-
if !httpapi.Read(ctx, rw, r, &req) {
54-
return
55-
}
56-
57-
updated, ok := handler.PatchOrganizationRole(ctx, rw, r, organization.ID, req)
58-
if !ok {
59-
return
60-
}
61-
62-
httpapi.Write(ctx, rw, http.StatusOK, updated)
63-
}
64-
6518
// AssignableSiteRoles returns all site wide roles that can be assigned.
6619
//
6720
// @Summary Get site member roles

‎enterprise/coderd/coderd.go

+11-10
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,17 @@ func New(ctx context.Context, options *Options) (_ *API, err error) {
261261
r.Delete("/organizations/{organization}", api.deleteOrganization)
262262
})
263263

264+
r.Group(func(r chi.Router) {
265+
r.Use(
266+
apiKeyMiddleware,
267+
api.RequireFeatureMW(codersdk.FeatureCustomRoles),
268+
httpmw.RequireExperiment(api.AGPL.Experiments, codersdk.ExperimentCustomRoles),
269+
httpmw.ExtractOrganizationParam(api.Database),
270+
)
271+
r.Patch("/organizations/{organization}/members/roles", api.patchOrgRoles)
272+
273+
})
274+
264275
r.Route("/organizations/{organization}/groups", func(r chi.Router) {
265276
r.Use(
266277
apiKeyMiddleware,
@@ -787,16 +798,6 @@ func (api *API) updateEntitlements(ctx context.Context) error {
787798
api.AGPL.PortSharer.Store(&ps)
788799
}
789800

790-
if initial, changed, enabled := featureChanged(codersdk.FeatureCustomRoles); shouldUpdate(initial, changed, enabled) {
791-
var handler coderd.CustomRoleHandler = &enterpriseCustomRoleHandler{API: api, Enabled: enabled}
792-
api.AGPL.CustomRoleHandler.Store(&handler)
793-
}
794-
795-
if initial, changed, enabled := featureChanged(codersdk.FeatureMultipleOrganizations); shouldUpdate(initial, changed, enabled) {
796-
var handler coderd.CustomRoleHandler = &enterpriseCustomRoleHandler{API: api, Enabled: enabled}
797-
api.AGPL.CustomRoleHandler.Store(&handler)
798-
}
799-
800801
// External token encryption is soft-enforced
801802
featureExternalTokenEncryption := entitlements.Features[codersdk.FeatureExternalTokenEncryption]
802803
featureExternalTokenEncryption.Enabled = len(api.ExternalTokenEncryption) > 0

‎enterprise/coderd/roles.go

+45-40
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package coderd
22

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

@@ -11,117 +10,123 @@ import (
1110
"github.com/coder/coder/v2/coderd/database"
1211
"github.com/coder/coder/v2/coderd/database/db2sdk"
1312
"github.com/coder/coder/v2/coderd/httpapi"
13+
"github.com/coder/coder/v2/coderd/httpmw"
1414
"github.com/coder/coder/v2/coderd/rbac"
1515
"github.com/coder/coder/v2/coderd/rbac/policy"
1616
"github.com/coder/coder/v2/codersdk"
1717
)
1818

19-
type enterpriseCustomRoleHandler struct {
20-
API *API
21-
Enabled bool
22-
}
23-
24-
func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context, rw http.ResponseWriter, r *http.Request, orgID uuid.UUID, role codersdk.PatchRoleRequest) (codersdk.Role, bool) {
25-
if !h.Enabled {
26-
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
27-
Message: "Custom roles are not enabled",
28-
})
29-
return codersdk.Role{}, false
30-
}
31-
19+
// patchRole will allow creating a custom organization role
20+
//
21+
// @Summary Upsert a custom organization role
22+
// @ID upsert-a-custom-organization-role
23+
// @Security CoderSessionToken
24+
// @Produce json
25+
// @Param organization path string true "Organization ID" format(uuid)
26+
// @Tags Members
27+
// @Success 200 {array} codersdk.Role
28+
// @Router /organizations/{organization}/members/roles [patch]
29+
func (api *API) patchOrgRoles(rw http.ResponseWriter, r *http.Request) {
3230
var (
33-
db = h.API.Database
34-
auditor = h.API.AGPL.Auditor.Load()
31+
ctx = r.Context()
32+
db = api.Database
33+
auditor = api.AGPL.Auditor.Load()
34+
organization = httpmw.OrganizationParam(r)
3535
aReq, commitAudit = audit.InitRequest[database.CustomRole](rw, &audit.RequestParams{
3636
Audit: *auditor,
37-
Log: h.API.Logger,
37+
Log: api.Logger,
3838
Request: r,
3939
Action: database.AuditActionWrite,
40-
OrganizationID: orgID,
40+
OrganizationID: organization.ID,
4141
})
4242
)
4343
defer commitAudit()
4444

45+
var req codersdk.Role
46+
if !httpapi.Read(ctx, rw, r, &req) {
47+
return
48+
}
49+
4550
// This check is not ideal, but we cannot enforce a unique role name in the db against
4651
// the built-in role names.
47-
if rbac.ReservedRoleName(role.Name) {
52+
if rbac.ReservedRoleName(req.Name) {
4853
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
4954
Message: "Reserved role name",
50-
Detail: fmt.Sprintf("%q is a reserved role name, and not allowed to be used", role.Name),
55+
Detail: fmt.Sprintf("%q is a reserved role name, and not allowed to be used", req.Name),
5156
})
52-
return codersdk.Role{}, false
57+
return
5358
}
5459

55-
if err := httpapi.NameValid(role.Name); err != nil {
60+
if err := httpapi.NameValid(req.Name); err != nil {
5661
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
5762
Message: "Invalid role name",
5863
Detail: err.Error(),
5964
})
60-
return codersdk.Role{}, false
65+
return
6166
}
6267

6368
// Only organization permissions are allowed to be granted
64-
if len(role.SitePermissions) > 0 {
69+
if len(req.SitePermissions) > 0 {
6570
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
6671
Message: "Invalid request, not allowed to assign site wide permissions for an organization role.",
6772
Detail: "organization scoped roles may not contain site wide permissions",
6873
})
69-
return codersdk.Role{}, false
74+
return
7075
}
7176

72-
if len(role.UserPermissions) > 0 {
77+
if len(req.UserPermissions) > 0 {
7378
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
7479
Message: "Invalid request, not allowed to assign user permissions for an organization role.",
7580
Detail: "organization scoped roles may not contain user permissions",
7681
})
77-
return codersdk.Role{}, false
82+
return
7883
}
7984

8085
originalRoles, err := db.CustomRoles(ctx, database.CustomRolesParams{
8186
LookupRoles: []database.NameOrganizationPair{
8287
{
83-
Name: role.Name,
84-
OrganizationID: orgID,
88+
Name: req.Name,
89+
OrganizationID: organization.ID,
8590
},
8691
},
8792
ExcludeOrgRoles: false,
88-
OrganizationID: orgID,
93+
OrganizationID: organization.ID,
8994
})
9095
// If it is a 404 (not found) error, ignore it.
9196
if err != nil && !httpapi.Is404Error(err) {
9297
httpapi.InternalServerError(rw, err)
93-
return codersdk.Role{}, false
98+
return
9499
}
95100
if len(originalRoles) == 1 {
96101
// For auditing changes to a role.
97102
aReq.Old = originalRoles[0]
98103
}
99104

100105
inserted, err := db.UpsertCustomRole(ctx, database.UpsertCustomRoleParams{
101-
Name: role.Name,
102-
DisplayName: role.DisplayName,
106+
Name: req.Name,
107+
DisplayName: req.DisplayName,
103108
OrganizationID: uuid.NullUUID{
104-
UUID: orgID,
109+
UUID: organization.ID,
105110
Valid: true,
106111
},
107-
SitePermissions: db2sdk.List(role.SitePermissions, sdkPermissionToDB),
108-
OrgPermissions: db2sdk.List(role.OrganizationPermissions, sdkPermissionToDB),
109-
UserPermissions: db2sdk.List(role.UserPermissions, sdkPermissionToDB),
112+
SitePermissions: db2sdk.List(req.SitePermissions, sdkPermissionToDB),
113+
OrgPermissions: db2sdk.List(req.OrganizationPermissions, sdkPermissionToDB),
114+
UserPermissions: db2sdk.List(req.UserPermissions, sdkPermissionToDB),
110115
})
111116
if httpapi.Is404Error(err) {
112117
httpapi.ResourceNotFound(rw)
113-
return codersdk.Role{}, false
118+
return
114119
}
115120
if err != nil {
116121
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
117122
Message: "Failed to update role permissions",
118123
Detail: err.Error(),
119124
})
120-
return codersdk.Role{}, false
125+
return
121126
}
122127
aReq.New = inserted
123128

124-
return db2sdk.Role(inserted), true
129+
httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Role(inserted))
125130
}
126131

127132
func sdkPermissionToDB(p codersdk.Permission) database.CustomRolePermission {

‎enterprise/coderd/roles_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ func TestCustomOrganizationRole(t *testing.T) {
125125

126126
// Verify functionality is lost
127127
_, err = owner.PatchOrganizationRole(ctx, templateAdminCustom(first.OrganizationID))
128-
require.ErrorContains(t, err, "roles are not enabled")
128+
require.ErrorContains(t, err, "Custom Roles is an Enterprise feature")
129129

130130
// Assign the custom template admin role
131131
tmplAdmin, _ := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.RoleIdentifier{Name: role.Name, OrganizationID: first.OrganizationID})
@@ -308,7 +308,7 @@ func TestCustomOrganizationRole(t *testing.T) {
308308

309309
//nolint:gocritic // owner is required for this
310310
_, err := owner.PatchOrganizationRole(ctx, newRole)
311-
require.ErrorContains(t, err, "Resource not found")
311+
require.ErrorContains(t, err, "Invalid request")
312312
})
313313
}
314314

0 commit comments

Comments
 (0)