Skip to content

Commit b873065

Browse files
EmyrkEmyrk
committed
chore: audit custom role changes
1 parent 8f62311 commit b873065

File tree

18 files changed

+107
-23
lines changed

18 files changed

+107
-23
lines changed

‎coderd/audit/diff.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ type Auditable interface {
2121
database.AuditOAuthConvertState |
2222
database.HealthSettings |
2323
database.OAuth2ProviderApp |
24-
database.OAuth2ProviderAppSecret
24+
database.OAuth2ProviderAppSecret |
25+
database.CustomRole
2526
}
2627

2728
// Map is a map of changed fields in an audited resource. It maps field names to

‎coderd/audit/request.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ func ResourceTarget[T Auditable](tgt T) string {
103103
return typed.Name
104104
case database.OAuth2ProviderAppSecret:
105105
return typed.DisplaySecret
106+
case database.CustomRole:
107+
return typed.Name
106108
default:
107109
panic(fmt.Sprintf("unknown resource %T for ResourceTarget", tgt))
108110
}
@@ -140,6 +142,8 @@ func ResourceID[T Auditable](tgt T) uuid.UUID {
140142
return typed.ID
141143
case database.OAuth2ProviderAppSecret:
142144
return typed.ID
145+
case database.CustomRole:
146+
return typed.ID
143147
default:
144148
panic(fmt.Sprintf("unknown resource %T for ResourceID", tgt))
145149
}
@@ -175,6 +179,8 @@ func ResourceType[T Auditable](tgt T) database.ResourceType {
175179
return database.ResourceTypeOauth2ProviderApp
176180
case database.OAuth2ProviderAppSecret:
177181
return database.ResourceTypeOauth2ProviderAppSecret
182+
case database.CustomRole:
183+
return database.ResourceTypeCustomRole
178184
default:
179185
panic(fmt.Sprintf("unknown resource %T for ResourceType", typed))
180186
}
@@ -211,6 +217,8 @@ func ResourceRequiresOrgID[T Auditable]() bool {
211217
return false
212218
case database.OAuth2ProviderAppSecret:
213219
return false
220+
case database.CustomRole:
221+
return true
214222
default:
215223
panic(fmt.Sprintf("unknown resource %T for ResourceRequiresOrgID", tgt))
216224
}

‎coderd/coderdtest/coderdtest.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -758,6 +758,8 @@ func createAnotherUserRetry(t testing.TB, client *codersdk.Client, organizationI
758758
roleName, _, err = rbac.RoleSplit(roleName)
759759
require.NoError(t, err, "split org role name")
760760
if ok {
761+
roleName, _, err = rbac.RoleSplit(roleName)
762+
require.NoError(t, err, "split rolename")
761763
orgRoles[orgID] = append(orgRoles[orgID], roleName)
762764
} else {
763765
siteRoles = append(siteRoles, roleName)

‎coderd/database/dbmem/dbmem.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1187,11 +1187,7 @@ func (q *FakeQuerier) CustomRoles(_ context.Context, arg database.CustomRolesPar
11871187
role := role
11881188
if len(arg.LookupRoles) > 0 {
11891189
if !slices.ContainsFunc(arg.LookupRoles, func(s string) bool {
1190-
roleName := rbac.RoleName(role.Name, "")
1191-
if role.OrganizationID.UUID != uuid.Nil {
1192-
roleName = rbac.RoleName(role.Name, role.OrganizationID.UUID.String())
1193-
}
1194-
return strings.EqualFold(s, roleName)
1190+
return strings.EqualFold(s, role.Name)
11951191
}) {
11961192
continue
11971193
}
@@ -8405,6 +8401,7 @@ func (q *FakeQuerier) UpsertCustomRole(_ context.Context, arg database.UpsertCus
84058401
}
84068402

84078403
role := database.CustomRole{
8404+
ID: uuid.New(),
84088405
Name: arg.Name,
84098406
DisplayName: arg.DisplayName,
84108407
OrganizationID: arg.OrganizationID,

‎coderd/database/dump.sql

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

‎coderd/database/migrations/000215_org_custom_role_audit.down.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
DROP INDEX idx_custom_roles_id;
2+
ALTER TABLE custom_roles DROP COLUMN id;

‎coderd/database/migrations/000215_org_custom_role_audit.up.sql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
-- (name) is the primary key, this column is almost exclusively for auditing.
2+
ALTER TABLE custom_roles ADD COLUMN id uuid DEFAULT gen_random_uuid() NOT NULL;
3+
4+
-- Ensure unique uuids.
5+
CREATE INDEX idx_custom_roles_id ON custom_roles (id);
6+
ALTER TYPE resource_type ADD VALUE IF NOT EXISTS 'custom_role';

‎coderd/database/models.go

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

‎coderd/database/querier.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

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

‎coderd/database/queries/roles.sql

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,7 @@ WHERE
88
-- Lookup roles filter expects the role names to be in the rbac package
99
-- format. Eg: name[:<organization_id>]
1010
AND CASE WHEN array_length(@lookup_roles :: text[], 1) > 0 THEN
11-
-- Case insensitive lookup with org_id appended (if non-null).
12-
-- This will return just the name if org_id is null. It'll append
13-
-- the org_id if not null
14-
concat(name, NULLIF(concat(':', organization_id), ':')) ILIKE ANY(@lookup_roles :: text [])
11+
name ILIKE ANY(@lookup_roles :: text [])
1512
ELSE true
1613
END
1714
-- Org scoping filter, to only fetch site wide roles

‎coderd/roles.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ import (
2020
// roles. Ideally only included in the enterprise package, but the routes are
2121
// intermixed with AGPL endpoints.
2222
type CustomRoleHandler interface {
23-
PatchOrganizationRole(ctx context.Context, db database.Store, rw http.ResponseWriter, orgID uuid.UUID, role codersdk.Role) (codersdk.Role, bool)
23+
PatchOrganizationRole(ctx context.Context, rw http.ResponseWriter, r *http.Request, orgID uuid.UUID, role codersdk.Role) (codersdk.Role, bool)
2424
}
2525

2626
type agplCustomRoleHandler struct{}
2727

28-
func (agplCustomRoleHandler) PatchOrganizationRole(ctx context.Context, _ database.Store, rw http.ResponseWriter, _ uuid.UUID, _ codersdk.Role) (codersdk.Role, bool) {
28+
func (agplCustomRoleHandler) PatchOrganizationRole(ctx context.Context, rw http.ResponseWriter, _ *http.Request, _ uuid.UUID, _ codersdk.Role) (codersdk.Role, bool) {
2929
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
3030
Message: "Creating and updating custom roles is an Enterprise feature. Contact sales!",
3131
})
@@ -54,7 +54,7 @@ func (api *API) patchOrgRoles(rw http.ResponseWriter, r *http.Request) {
5454
return
5555
}
5656

57-
updated, ok := handler.PatchOrganizationRole(ctx, api.Database, rw, organization.ID, req)
57+
updated, ok := handler.PatchOrganizationRole(ctx, rw, r, organization.ID, req)
5858
if !ok {
5959
return
6060
}

‎codersdk/audit.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ const (
3030
ResourceTypeOAuth2ProviderApp ResourceType = "oauth2_provider_app"
3131
// nolint:gosec // This is not a secret.
3232
ResourceTypeOAuth2ProviderAppSecret ResourceType = "oauth2_provider_app_secret"
33+
ResourceTypeCustomRole ResourceType = "custom_role"
3334
)
3435

3536
func (r ResourceType) FriendlyString() string {
@@ -66,6 +67,8 @@ func (r ResourceType) FriendlyString() string {
6667
return "oauth2 app"
6768
case ResourceTypeOAuth2ProviderAppSecret:
6869
return "oauth2 app secret"
70+
case ResourceTypeCustomRole:
71+
return "custom role"
6972
default:
7073
return "unknown"
7174
}

‎enterprise/audit/diff.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,19 @@ func convertDiffType(left, right any) (newLeft, newRight any, changed bool) {
144144
return leftInt64Ptr, rightInt64Ptr, true
145145
case database.TemplateACL:
146146
return fmt.Sprintf("%+v", left), fmt.Sprintf("%+v", right), true
147+
case database.CustomRolePermissions:
148+
leftArr := make([]string, 0)
149+
rightArr := make([]string, 0)
150+
for _, p := range typedLeft {
151+
leftArr = append(leftArr, p.String())
152+
}
153+
for _, p := range right.(database.CustomRolePermissions) {
154+
rightArr = append(rightArr, p.String())
155+
}
156+
157+
// String representation is much easier to visually inspect
158+
//typedRight := right.(database.CustomRolePermission)
159+
return leftArr, rightArr, true
147160
default:
148161
return left, right, false
149162
}

‎enterprise/audit/table.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,18 @@ type Table map[string]map[string]Action
5050
var AuditableResources = auditMap(auditableResourcesTypes)
5151

5252
var auditableResourcesTypes = map[any]map[string]Action{
53+
&database.CustomRole{}: {
54+
"name": ActionTrack,
55+
"display_name": ActionTrack,
56+
"site_permissions": ActionTrack,
57+
"org_permissions": ActionTrack,
58+
"user_permissions": ActionTrack,
59+
"organization_id": ActionTrack,
60+
61+
"id": ActionIgnore,
62+
"created_at": ActionIgnore,
63+
"updated_at": ActionIgnore,
64+
},
5365
&database.GitSSHKey{}: {
5466
"user_id": ActionTrack,
5567
"created_at": ActionIgnore, // Never changes, but is implicit and not helpful in a diff.

‎enterprise/coderd/coderd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -746,7 +746,7 @@ func (api *API) updateEntitlements(ctx context.Context) error {
746746
}
747747

748748
if initial, changed, enabled := featureChanged(codersdk.FeatureCustomRoles); shouldUpdate(initial, changed, enabled) {
749-
var handler coderd.CustomRoleHandler = &enterpriseCustomRoleHandler{Enabled: enabled}
749+
var handler coderd.CustomRoleHandler = &enterpriseCustomRoleHandler{API: api, Enabled: enabled}
750750
api.AGPL.CustomRoleHandler.Store(&handler)
751751
}
752752

‎enterprise/coderd/roles.go

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/google/uuid"
99

10+
"github.com/coder/coder/v2/coderd/audit"
1011
"github.com/coder/coder/v2/coderd/database"
1112
"github.com/coder/coder/v2/coderd/database/db2sdk"
1213
"github.com/coder/coder/v2/coderd/httpapi"
@@ -15,17 +16,31 @@ import (
1516
)
1617

1718
type enterpriseCustomRoleHandler struct {
19+
API *API
1820
Enabled bool
1921
}
2022

21-
func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context, db database.Store, rw http.ResponseWriter, orgID uuid.UUID, role codersdk.Role) (codersdk.Role, bool) {
23+
func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context, rw http.ResponseWriter, r *http.Request, orgID uuid.UUID, role codersdk.Role) (codersdk.Role, bool) {
2224
if !h.Enabled {
2325
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
2426
Message: "Custom roles are not enabled",
2527
})
2628
return codersdk.Role{}, false
2729
}
2830

31+
var (
32+
db = h.API.Database
33+
auditor = h.API.AGPL.Auditor.Load()
34+
aReq, commitAudit = audit.InitRequest[database.CustomRole](rw, &audit.RequestParams{
35+
Audit: *auditor,
36+
Log: h.API.Logger,
37+
Request: r,
38+
Action: database.AuditActionWrite,
39+
OrganizationID: orgID,
40+
})
41+
)
42+
defer commitAudit()
43+
2944
if err := httpapi.NameValid(role.Name); err != nil {
3045
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
3146
Message: "Invalid role name",
@@ -59,6 +74,21 @@ func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context,
5974
return codersdk.Role{}, false
6075
}
6176

77+
originalRoles, err := db.CustomRoles(ctx, database.CustomRolesParams{
78+
LookupRoles: []string{role.Name},
79+
ExcludeOrgRoles: false,
80+
OrganizationID: orgID,
81+
})
82+
// If it is a 404 (not found) error, ignore it.
83+
if err != nil && !httpapi.Is404Error(err) {
84+
httpapi.InternalServerError(rw, err)
85+
return codersdk.Role{}, false
86+
}
87+
if len(originalRoles) == 1 {
88+
// For auditing changes to a role.
89+
aReq.Old = originalRoles[1]
90+
}
91+
6292
inserted, err := db.UpsertCustomRole(ctx, database.UpsertCustomRoleParams{
6393
Name: role.Name,
6494
DisplayName: role.DisplayName,
@@ -81,6 +111,7 @@ func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context,
81111
})
82112
return codersdk.Role{}, false
83113
}
114+
aReq.New = inserted
84115

85116
return db2sdk.Role(inserted), true
86117
}

‎site/src/api/typesGenerated.ts

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

0 commit comments

Comments
 (0)