diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 8905c8a09cba5..5fe1e929d83bd 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -11330,7 +11330,8 @@ const docTemplate = `{ "workspace_proxy", "organization", "oauth2_provider_app", - "oauth2_provider_app_secret" + "oauth2_provider_app_secret", + "custom_role" ], "x-enum-varnames": [ "ResourceTypeTemplate", @@ -11347,7 +11348,8 @@ const docTemplate = `{ "ResourceTypeWorkspaceProxy", "ResourceTypeOrganization", "ResourceTypeOAuth2ProviderApp", - "ResourceTypeOAuth2ProviderAppSecret" + "ResourceTypeOAuth2ProviderAppSecret", + "ResourceTypeCustomRole" ] }, "codersdk.Response": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 3ab826d3920da..18eb052c3fd64 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -10219,7 +10219,8 @@ "workspace_proxy", "organization", "oauth2_provider_app", - "oauth2_provider_app_secret" + "oauth2_provider_app_secret", + "custom_role" ], "x-enum-varnames": [ "ResourceTypeTemplate", @@ -10236,7 +10237,8 @@ "ResourceTypeWorkspaceProxy", "ResourceTypeOrganization", "ResourceTypeOAuth2ProviderApp", - "ResourceTypeOAuth2ProviderAppSecret" + "ResourceTypeOAuth2ProviderAppSecret", + "ResourceTypeCustomRole" ] }, "codersdk.Response": { diff --git a/coderd/audit/diff.go b/coderd/audit/diff.go index a6835014d4fe2..dd5205c0afb42 100644 --- a/coderd/audit/diff.go +++ b/coderd/audit/diff.go @@ -21,7 +21,8 @@ type Auditable interface { database.AuditOAuthConvertState | database.HealthSettings | database.OAuth2ProviderApp | - database.OAuth2ProviderAppSecret + database.OAuth2ProviderAppSecret | + database.CustomRole } // Map is a map of changed fields in an audited resource. It maps field names to diff --git a/coderd/audit/request.go b/coderd/audit/request.go index e6d9d01fbfd27..20eb8185af53e 100644 --- a/coderd/audit/request.go +++ b/coderd/audit/request.go @@ -103,6 +103,8 @@ func ResourceTarget[T Auditable](tgt T) string { return typed.Name case database.OAuth2ProviderAppSecret: return typed.DisplaySecret + case database.CustomRole: + return typed.Name default: panic(fmt.Sprintf("unknown resource %T for ResourceTarget", tgt)) } @@ -140,6 +142,8 @@ func ResourceID[T Auditable](tgt T) uuid.UUID { return typed.ID case database.OAuth2ProviderAppSecret: return typed.ID + case database.CustomRole: + return typed.ID default: panic(fmt.Sprintf("unknown resource %T for ResourceID", tgt)) } @@ -175,6 +179,8 @@ func ResourceType[T Auditable](tgt T) database.ResourceType { return database.ResourceTypeOauth2ProviderApp case database.OAuth2ProviderAppSecret: return database.ResourceTypeOauth2ProviderAppSecret + case database.CustomRole: + return database.ResourceTypeCustomRole default: panic(fmt.Sprintf("unknown resource %T for ResourceType", typed)) } @@ -211,6 +217,8 @@ func ResourceRequiresOrgID[T Auditable]() bool { return false case database.OAuth2ProviderAppSecret: return false + case database.CustomRole: + return true default: panic(fmt.Sprintf("unknown resource %T for ResourceRequiresOrgID", tgt)) } diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 316683a9f1e65..3ebca07686d0e 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -758,6 +758,8 @@ func createAnotherUserRetry(t testing.TB, client *codersdk.Client, organizationI roleName, _, err = rbac.RoleSplit(roleName) require.NoError(t, err, "split org role name") if ok { + roleName, _, err = rbac.RoleSplit(roleName) + require.NoError(t, err, "split rolename") orgRoles[orgID] = append(orgRoles[orgID], roleName) } else { siteRoles = append(siteRoles, roleName) diff --git a/coderd/database/dbauthz/customroles_test.go b/coderd/database/dbauthz/customroles_test.go index b98af8fd23889..814ba88a1b18c 100644 --- a/coderd/database/dbauthz/customroles_test.go +++ b/coderd/database/dbauthz/customroles_test.go @@ -244,7 +244,7 @@ func TestUpsertCustomRoles(t *testing.T) { } else { require.NoError(t, err) - // Verify we can fetch the role + // Verify the role is fetched with the lookup filter. roles, err := az.CustomRoles(ctx, database.CustomRolesParams{ LookupRoles: []database.NameOrganizationPair{ { diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 3f9ef73048e6b..147eb8eca6a05 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -8415,6 +8415,7 @@ func (q *FakeQuerier) UpsertCustomRole(_ context.Context, arg database.UpsertCus } role := database.CustomRole{ + ID: uuid.New(), Name: arg.Name, DisplayName: arg.DisplayName, OrganizationID: arg.OrganizationID, diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index c6faa00c65fc5..83eea6e3583a6 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -147,7 +147,8 @@ CREATE TYPE resource_type AS ENUM ( 'convert_login', 'health_settings', 'oauth2_provider_app', - 'oauth2_provider_app_secret' + 'oauth2_provider_app_secret', + 'custom_role' ); CREATE TYPE startup_script_behavior AS ENUM ( @@ -417,13 +418,16 @@ CREATE TABLE custom_roles ( user_permissions jsonb DEFAULT '[]'::jsonb NOT NULL, created_at timestamp with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, updated_at timestamp with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, - organization_id uuid + organization_id uuid, + id uuid DEFAULT gen_random_uuid() NOT NULL ); COMMENT ON TABLE custom_roles IS 'Custom roles allow dynamic roles expanded at runtime'; COMMENT ON COLUMN custom_roles.organization_id IS 'Roles can optionally be scoped to an organization'; +COMMENT ON COLUMN custom_roles.id IS 'Custom roles ID is used purely for auditing purposes. Name is a better unique identifier.'; + CREATE TABLE dbcrypt_keys ( number integer NOT NULL, active_key_digest text, @@ -1642,6 +1646,8 @@ CREATE INDEX idx_audit_log_user_id ON audit_logs USING btree (user_id); CREATE INDEX idx_audit_logs_time_desc ON audit_logs USING btree ("time" DESC); +CREATE INDEX idx_custom_roles_id ON custom_roles USING btree (id); + CREATE UNIQUE INDEX idx_custom_roles_name_lower ON custom_roles USING btree (lower(name)); CREATE INDEX idx_organization_member_organization_id_uuid ON organization_members USING btree (organization_id); diff --git a/coderd/database/migrations/000218_org_custom_role_audit.down.sql b/coderd/database/migrations/000218_org_custom_role_audit.down.sql new file mode 100644 index 0000000000000..5ad6106f2fc26 --- /dev/null +++ b/coderd/database/migrations/000218_org_custom_role_audit.down.sql @@ -0,0 +1,2 @@ +DROP INDEX idx_custom_roles_id; +ALTER TABLE custom_roles DROP COLUMN id; diff --git a/coderd/database/migrations/000218_org_custom_role_audit.up.sql b/coderd/database/migrations/000218_org_custom_role_audit.up.sql new file mode 100644 index 0000000000000..a780f34960907 --- /dev/null +++ b/coderd/database/migrations/000218_org_custom_role_audit.up.sql @@ -0,0 +1,8 @@ +-- (name) is the primary key, this column is almost exclusively for auditing. +-- Audit logs require a uuid as the unique identifier for a resource. +ALTER TABLE custom_roles ADD COLUMN id uuid DEFAULT gen_random_uuid() NOT NULL; +COMMENT ON COLUMN custom_roles.id IS 'Custom roles ID is used purely for auditing purposes. Name is a better unique identifier.'; + +-- Ensure unique uuids. +CREATE INDEX idx_custom_roles_id ON custom_roles (id); +ALTER TYPE resource_type ADD VALUE IF NOT EXISTS 'custom_role'; diff --git a/coderd/database/models.go b/coderd/database/models.go index aa69054abc2aa..8a558f5beeb0b 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -1222,6 +1222,7 @@ const ( ResourceTypeHealthSettings ResourceType = "health_settings" ResourceTypeOauth2ProviderApp ResourceType = "oauth2_provider_app" ResourceTypeOauth2ProviderAppSecret ResourceType = "oauth2_provider_app_secret" + ResourceTypeCustomRole ResourceType = "custom_role" ) func (e *ResourceType) Scan(src interface{}) error { @@ -1275,7 +1276,8 @@ func (e ResourceType) Valid() bool { ResourceTypeConvertLogin, ResourceTypeHealthSettings, ResourceTypeOauth2ProviderApp, - ResourceTypeOauth2ProviderAppSecret: + ResourceTypeOauth2ProviderAppSecret, + ResourceTypeCustomRole: return true } return false @@ -1298,6 +1300,7 @@ func AllResourceTypeValues() []ResourceType { ResourceTypeHealthSettings, ResourceTypeOauth2ProviderApp, ResourceTypeOauth2ProviderAppSecret, + ResourceTypeCustomRole, } } @@ -1792,6 +1795,8 @@ type CustomRole struct { UpdatedAt time.Time `db:"updated_at" json:"updated_at"` // Roles can optionally be scoped to an organization OrganizationID uuid.NullUUID `db:"organization_id" json:"organization_id"` + // Custom roles ID is used purely for auditing purposes. Name is a better unique identifier. + ID uuid.UUID `db:"id" json:"id"` } // A table used to store the keys used to encrypt the database. diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 5dff8c05e05a7..823cf2cc45796 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5618,7 +5618,7 @@ func (q *sqlQuerier) UpdateReplica(ctx context.Context, arg UpdateReplicaParams) const customRoles = `-- name: CustomRoles :many SELECT - name, display_name, site_permissions, org_permissions, user_permissions, created_at, updated_at, organization_id + name, display_name, site_permissions, org_permissions, user_permissions, created_at, updated_at, organization_id, id FROM custom_roles WHERE @@ -5667,6 +5667,7 @@ func (q *sqlQuerier) CustomRoles(ctx context.Context, arg CustomRolesParams) ([] &i.CreatedAt, &i.UpdatedAt, &i.OrganizationID, + &i.ID, ); err != nil { return nil, err } @@ -5711,7 +5712,7 @@ ON CONFLICT (name) org_permissions = $5, user_permissions = $6, updated_at = now() -RETURNING name, display_name, site_permissions, org_permissions, user_permissions, created_at, updated_at, organization_id +RETURNING name, display_name, site_permissions, org_permissions, user_permissions, created_at, updated_at, organization_id, id ` type UpsertCustomRoleParams struct { @@ -5742,6 +5743,7 @@ func (q *sqlQuerier) UpsertCustomRole(ctx context.Context, arg UpsertCustomRoleP &i.CreatedAt, &i.UpdatedAt, &i.OrganizationID, + &i.ID, ) return i, err } diff --git a/coderd/roles.go b/coderd/roles.go index 94b121940ed45..1e7f1b1473b9a 100644 --- a/coderd/roles.go +++ b/coderd/roles.go @@ -20,12 +20,12 @@ import ( // roles. Ideally only included in the enterprise package, but the routes are // intermixed with AGPL endpoints. type CustomRoleHandler interface { - PatchOrganizationRole(ctx context.Context, db database.Store, rw http.ResponseWriter, orgID uuid.UUID, role codersdk.Role) (codersdk.Role, bool) + PatchOrganizationRole(ctx context.Context, rw http.ResponseWriter, r *http.Request, orgID uuid.UUID, role codersdk.Role) (codersdk.Role, bool) } type agplCustomRoleHandler struct{} -func (agplCustomRoleHandler) PatchOrganizationRole(ctx context.Context, _ database.Store, rw http.ResponseWriter, _ uuid.UUID, _ codersdk.Role) (codersdk.Role, bool) { +func (agplCustomRoleHandler) PatchOrganizationRole(ctx context.Context, rw http.ResponseWriter, _ *http.Request, _ uuid.UUID, _ codersdk.Role) (codersdk.Role, bool) { httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ Message: "Creating and updating custom roles is an Enterprise feature. Contact sales!", }) @@ -54,7 +54,7 @@ func (api *API) patchOrgRoles(rw http.ResponseWriter, r *http.Request) { return } - updated, ok := handler.PatchOrganizationRole(ctx, api.Database, rw, organization.ID, req) + updated, ok := handler.PatchOrganizationRole(ctx, rw, r, organization.ID, req) if !ok { return } diff --git a/codersdk/audit.go b/codersdk/audit.go index 553bd9cc2dbea..837ef729e4a58 100644 --- a/codersdk/audit.go +++ b/codersdk/audit.go @@ -30,6 +30,7 @@ const ( ResourceTypeOAuth2ProviderApp ResourceType = "oauth2_provider_app" // nolint:gosec // This is not a secret. ResourceTypeOAuth2ProviderAppSecret ResourceType = "oauth2_provider_app_secret" + ResourceTypeCustomRole ResourceType = "custom_role" ) func (r ResourceType) FriendlyString() string { @@ -66,6 +67,8 @@ func (r ResourceType) FriendlyString() string { return "oauth2 app" case ResourceTypeOAuth2ProviderAppSecret: return "oauth2 app secret" + case ResourceTypeCustomRole: + return "custom role" default: return "unknown" } diff --git a/docs/admin/audit-logs.md b/docs/admin/audit-logs.md index fada57f32065f..34c4e8c9a8dc3 100644 --- a/docs/admin/audit-logs.md +++ b/docs/admin/audit-logs.md @@ -13,6 +13,7 @@ We track the following resources: | APIKey
login, logout, register, create, delete |
FieldTracked
created_attrue
expires_attrue
hashed_secretfalse
idfalse
ip_addressfalse
last_usedtrue
lifetime_secondsfalse
login_typefalse
scopefalse
token_namefalse
updated_atfalse
user_idtrue
| | AuditOAuthConvertState
|
FieldTracked
created_attrue
expires_attrue
from_login_typetrue
to_login_typetrue
user_idtrue
| | Group
create, write, delete |
FieldTracked
avatar_urltrue
display_nametrue
idtrue
memberstrue
nametrue
organization_idfalse
quota_allowancetrue
sourcefalse
| +| CustomRole
|
FieldTracked
created_atfalse
display_nametrue
idfalse
nametrue
org_permissionstrue
organization_idtrue
site_permissionstrue
updated_atfalse
user_permissionstrue
| | GitSSHKey
create |
FieldTracked
created_atfalse
private_keytrue
public_keytrue
updated_atfalse
user_idtrue
| | HealthSettings
|
FieldTracked
dismissed_healthcheckstrue
idfalse
| | License
create, delete |
FieldTracked
exptrue
idfalse
jwtfalse
uploaded_attrue
uuidtrue
| diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 24af8aece05e6..55070fb629864 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -4323,6 +4323,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `organization` | | `oauth2_provider_app` | | `oauth2_provider_app_secret` | +| `custom_role` | ## codersdk.Response diff --git a/enterprise/audit/diff.go b/enterprise/audit/diff.go index 59780d2918418..007f475f6f5eb 100644 --- a/enterprise/audit/diff.go +++ b/enterprise/audit/diff.go @@ -144,6 +144,18 @@ func convertDiffType(left, right any) (newLeft, newRight any, changed bool) { return leftInt64Ptr, rightInt64Ptr, true case database.TemplateACL: return fmt.Sprintf("%+v", left), fmt.Sprintf("%+v", right), true + case database.CustomRolePermissions: + // String representation is much easier to visually inspect + leftArr := make([]string, 0) + rightArr := make([]string, 0) + for _, p := range typedLeft { + leftArr = append(leftArr, p.String()) + } + for _, p := range right.(database.CustomRolePermissions) { + rightArr = append(rightArr, p.String()) + } + + return leftArr, rightArr, true default: return left, right, false } diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index f26b4921aaace..e2788959e3275 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -50,6 +50,18 @@ type Table map[string]map[string]Action var AuditableResources = auditMap(auditableResourcesTypes) var auditableResourcesTypes = map[any]map[string]Action{ + &database.CustomRole{}: { + "name": ActionTrack, + "display_name": ActionTrack, + "site_permissions": ActionTrack, + "org_permissions": ActionTrack, + "user_permissions": ActionTrack, + "organization_id": ActionTrack, + + "id": ActionIgnore, + "created_at": ActionIgnore, + "updated_at": ActionIgnore, + }, &database.GitSSHKey{}: { "user_id": ActionTrack, "created_at": ActionIgnore, // Never changes, but is implicit and not helpful in a diff. diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 574d2c12dd2de..26fdab6ec1bfb 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -746,7 +746,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { } if initial, changed, enabled := featureChanged(codersdk.FeatureCustomRoles); shouldUpdate(initial, changed, enabled) { - var handler coderd.CustomRoleHandler = &enterpriseCustomRoleHandler{Enabled: enabled} + var handler coderd.CustomRoleHandler = &enterpriseCustomRoleHandler{API: api, Enabled: enabled} api.AGPL.CustomRoleHandler.Store(&handler) } diff --git a/enterprise/coderd/roles.go b/enterprise/coderd/roles.go index b3a24a8a7779f..3a162a1b5ea80 100644 --- a/enterprise/coderd/roles.go +++ b/enterprise/coderd/roles.go @@ -7,6 +7,7 @@ import ( "github.com/google/uuid" + "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/httpapi" @@ -15,10 +16,11 @@ import ( ) type enterpriseCustomRoleHandler struct { + API *API Enabled bool } -func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context, db database.Store, rw http.ResponseWriter, orgID uuid.UUID, role codersdk.Role) (codersdk.Role, bool) { +func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context, rw http.ResponseWriter, r *http.Request, orgID uuid.UUID, role codersdk.Role) (codersdk.Role, bool) { if !h.Enabled { httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ Message: "Custom roles are not enabled", @@ -26,6 +28,19 @@ func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context, return codersdk.Role{}, false } + var ( + db = h.API.Database + auditor = h.API.AGPL.Auditor.Load() + aReq, commitAudit = audit.InitRequest[database.CustomRole](rw, &audit.RequestParams{ + Audit: *auditor, + Log: h.API.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: orgID, + }) + ) + defer commitAudit() + if err := httpapi.NameValid(role.Name); err != nil { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Invalid role name", @@ -59,6 +74,26 @@ func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context, return codersdk.Role{}, false } + originalRoles, err := db.CustomRoles(ctx, database.CustomRolesParams{ + LookupRoles: []database.NameOrganizationPair{ + { + Name: role.Name, + OrganizationID: orgID, + }, + }, + ExcludeOrgRoles: false, + OrganizationID: orgID, + }) + // If it is a 404 (not found) error, ignore it. + if err != nil && !httpapi.Is404Error(err) { + httpapi.InternalServerError(rw, err) + return codersdk.Role{}, false + } + if len(originalRoles) == 1 { + // For auditing changes to a role. + aReq.Old = originalRoles[0] + } + inserted, err := db.UpsertCustomRole(ctx, database.UpsertCustomRoleParams{ Name: role.Name, DisplayName: role.DisplayName, @@ -81,6 +116,7 @@ func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context, }) return codersdk.Role{}, false } + aReq.New = inserted return db2sdk.Role(inserted), true } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index fae41504f1c34..a53717e3e0229 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -2181,6 +2181,7 @@ export const RBACResources: RBACResource[] = [ export type ResourceType = | "api_key" | "convert_login" + | "custom_role" | "git_ssh_key" | "group" | "health_settings" @@ -2197,6 +2198,7 @@ export type ResourceType = export const ResourceTypes: ResourceType[] = [ "api_key", "convert_login", + "custom_role", "git_ssh_key", "group", "health_settings",