Skip to content

Commit f03cfaf

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

File tree

5 files changed

+60
-108
lines changed

5 files changed

+60
-108
lines changed

coderd/coderd.go

Lines changed: 0 additions & 7 deletions
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

Lines changed: 0 additions & 47 deletions
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.Role) (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.Role) (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.Role
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

Lines changed: 11 additions & 10 deletions
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

Lines changed: 48 additions & 43 deletions
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,125 +10,131 @@ 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.Role) (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

80-
if role.OrganizationID != orgID.String() {
85+
if req.OrganizationID != organization.ID.String() {
8186
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
8287
Message: "Invalid request, organization in role and url must match",
83-
Detail: fmt.Sprintf("role organization=%q does not match URL=%q", role.OrganizationID, orgID.String()),
88+
Detail: fmt.Sprintf("role organization=%q does not match URL=%q", req.OrganizationID, organization.ID.String()),
8489
})
85-
return codersdk.Role{}, false
90+
return
8691
}
8792

8893
originalRoles, err := db.CustomRoles(ctx, database.CustomRolesParams{
8994
LookupRoles: []database.NameOrganizationPair{
9095
{
91-
Name: role.Name,
92-
OrganizationID: orgID,
96+
Name: req.Name,
97+
OrganizationID: organization.ID,
9398
},
9499
},
95100
ExcludeOrgRoles: false,
96-
OrganizationID: orgID,
101+
OrganizationID: organization.ID,
97102
})
98103
// If it is a 404 (not found) error, ignore it.
99104
if err != nil && !httpapi.Is404Error(err) {
100105
httpapi.InternalServerError(rw, err)
101-
return codersdk.Role{}, false
106+
return
102107
}
103108
if len(originalRoles) == 1 {
104109
// For auditing changes to a role.
105110
aReq.Old = originalRoles[0]
106111
}
107112

108113
inserted, err := db.UpsertCustomRole(ctx, database.UpsertCustomRoleParams{
109-
Name: role.Name,
110-
DisplayName: role.DisplayName,
114+
Name: req.Name,
115+
DisplayName: req.DisplayName,
111116
OrganizationID: uuid.NullUUID{
112-
UUID: orgID,
117+
UUID: organization.ID,
113118
Valid: true,
114119
},
115-
SitePermissions: db2sdk.List(role.SitePermissions, sdkPermissionToDB),
116-
OrgPermissions: db2sdk.List(role.OrganizationPermissions, sdkPermissionToDB),
117-
UserPermissions: db2sdk.List(role.UserPermissions, sdkPermissionToDB),
120+
SitePermissions: db2sdk.List(req.SitePermissions, sdkPermissionToDB),
121+
OrgPermissions: db2sdk.List(req.OrganizationPermissions, sdkPermissionToDB),
122+
UserPermissions: db2sdk.List(req.UserPermissions, sdkPermissionToDB),
118123
})
119124
if httpapi.Is404Error(err) {
120125
httpapi.ResourceNotFound(rw)
121-
return codersdk.Role{}, false
126+
return
122127
}
123128
if err != nil {
124129
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
125130
Message: "Failed to update role permissions",
126131
Detail: err.Error(),
127132
})
128-
return codersdk.Role{}, false
133+
return
129134
}
130135
aReq.New = inserted
131136

132-
return db2sdk.Role(inserted), true
137+
httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Role(inserted))
133138
}
134139

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

enterprise/coderd/roles_test.go

Lines changed: 1 addition & 1 deletion
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, first.OrganizationID, 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})

0 commit comments

Comments
 (0)