Skip to content

Commit 764fd62

Browse files
geokatgeokat
committed
wip: (re-)implement organization-member as custom role
1 parent 3194bcf commit 764fd62

21 files changed

+502
-100
lines changed

‎coderd/coderd.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,17 @@ func New(options *Options) *API {
573573
// bugs that may only occur when a key isn't precached in tests and the latency cost is minimal.
574574
cryptokeys.StartRotator(ctx, options.Logger, options.Database)
575575

576+
// Ensure system role permissions are current for all organizations.
577+
// This backfills permissions for roles created by migration and
578+
// updates permissions when new RBAC resources are added, while
579+
// using advisory lock for multi-replica safety.
580+
err = ReconcileOrgMemberRoles(ctx, options.Logger, options.Database)
581+
if err != nil {
582+
// TODO:(geokat) Not using fatal here and just continuing after
583+
// logging the error would be a potential security hole, right?
584+
options.Logger.Fatal(ctx, "failed to reconcile orgMember role permissions", slog.Error(err))
585+
}
586+
576587
// AGPL uses a no-op build usage checker as there are no license
577588
// entitlements to enforce. This is swapped out in
578589
// enterprise/coderd/coderd.go.

‎coderd/database/dbauthz/dbauthz.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4110,6 +4110,8 @@ func (q *querier) InsertCustomRole(ctx context.Context, arg database.InsertCusto
41104110
return database.CustomRole{}, err
41114111
}
41124112

4113+
// TODO(geokat): should we add MemberPermissions validation too
4114+
// now that we have them in system roles?
41134115
if err := q.customRoleCheck(ctx, database.CustomRole{
41144116
Name: arg.Name,
41154117
DisplayName: arg.DisplayName,
@@ -4864,6 +4866,8 @@ func (q *querier) UpdateCustomRole(ctx context.Context, arg database.UpdateCusto
48644866
return database.CustomRole{}, err
48654867
}
48664868

4869+
// TODO(geokat): should we add MemberPermissions validation too
4870+
// now that we have them in system roles?
48674871
if err := q.customRoleCheck(ctx, database.CustomRole{
48684872
Name: arg.Name,
48694873
DisplayName: arg.DisplayName,

‎coderd/database/dump.sql‎

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

‎coderd/database/lock.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const (
1313
LockIDNotificationsReportGenerator
1414
LockIDCryptoKeyRotation
1515
LockIDReconcilePrebuilds
16+
LockIDReconcileOrgMemberRoles
1617
)
1718

1819
// GenLockID generates a unique and consistent lock ID from a given string.

‎coderd/database/migrations/000405_add_system_role_support.down.sql‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
ALTER TABLE custom_roles DROP COLUMN IF EXISTS member_permissions;
2+
3+
ALTER TABLE custom_roles DROP COLUMN IF EXISTS is_system;

‎coderd/database/migrations/000405_add_system_role_support.up.sql‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
-- Add is_system column to identify system-managed roles.
2+
ALTER TABLE custom_roles
3+
ADD COLUMN is_system boolean NOT NULL DEFAULT false;
4+
5+
-- Add member_permissions column for member-scoped permissions within an organization.
6+
ALTER TABLE custom_roles
7+
ADD COLUMN member_permissions jsonb NOT NULL DEFAULT '[]'::jsonb;
8+
9+
COMMENT ON COLUMN custom_roles.is_system IS
10+
'System roles are managed by Coder and cannot be modified or deleted by users.';

‎coderd/database/migrations/000406_add_workspace_sharing_disabled.down.sql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE organizations DROP COLUMN IF EXISTS workspace_sharing_disabled;

‎coderd/database/migrations/000406_add_workspace_sharing_disabled.up.sql‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TABLE organizations
2+
ADD COLUMN workspace_sharing_disabled boolean NOT NULL DEFAULT false;

‎coderd/database/migrations/000407_create_org_member_system_roles.down.sql‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
-- Remove organization-member system roles created by the up migration.
2+
DELETE FROM
3+
custom_roles
4+
WHERE
5+
name = 'organization-member'
6+
AND is_system = true;

‎coderd/database/migrations/000407_create_org_member_system_roles.up.sql‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
-- Create placeholder organization-member system roles for existing
2+
-- organizations. Permissions are empty here and will be populated by
3+
-- the startup hook.
4+
INSERT INTO custom_roles (
5+
name,
6+
display_name,
7+
organization_id,
8+
site_permissions,
9+
org_permissions,
10+
user_permissions,
11+
member_permissions,
12+
is_system,
13+
created_at,
14+
updated_at
15+
)
16+
SELECT
17+
'organization-member', -- reserved role name, so it doesn't exist in DB yet
18+
'',
19+
id,
20+
'[]'::jsonb,
21+
'[]'::jsonb,
22+
'[]'::jsonb,
23+
'[]'::jsonb,
24+
true,
25+
NOW(),
26+
NOW()
27+
FROM
28+
organizations
29+
WHERE
30+
deleted = false
31+
AND NOT EXISTS (
32+
SELECT 1
33+
FROM custom_roles
34+
WHERE
35+
custom_roles.name = 'organization-member'
36+
AND custom_roles.organization_id = organizations.id
37+
);

0 commit comments

Comments
 (0)