Skip to content

Commit 7a3ffb0

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

21 files changed

+520
-65
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: 8 additions & 3 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/000404_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/000404_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/000405_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/000405_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/000406_create_org_member_system_roles.down.sql‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
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;
7+
8+
-- Restore the original unique constraint (name only, no organization_id).
9+
DROP INDEX IF EXISTS idx_custom_roles_name_lower_organization_id;
10+
11+
CREATE UNIQUE INDEX idx_custom_roles_name_lower ON custom_roles USING btree (lower(name));

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
-- Fix the unique constraint on custom_roles to allow the same role
2+
-- name in different organizations. The original constraint only
3+
-- covered name, but roles should be unique per (name,
4+
-- organization_id).
5+
DROP INDEX IF EXISTS idx_custom_roles_name_lower;
6+
7+
-- Use COALESCE to handle NULL organization_id. Site-wide custom roles
8+
-- are currently not used, but that can change in the future and this
9+
-- will be necessary. And there are no performance implications.
10+
CREATE UNIQUE INDEX idx_custom_roles_name_lower_organization_id ON custom_roles USING btree (
11+
lower(name),
12+
COALESCE(organization_id, '00000000-0000-0000-0000-000000000000'::uuid)
13+
);
14+
15+
-- Create placeholder organization-member system roles for existing
16+
-- organizations. Permissions are empty here and will be populated by
17+
-- the startup hook.
18+
INSERT INTO custom_roles (
19+
name,
20+
display_name,
21+
organization_id,
22+
site_permissions,
23+
org_permissions,
24+
user_permissions,
25+
member_permissions,
26+
is_system,
27+
created_at,
28+
updated_at
29+
)
30+
SELECT
31+
'organization-member', -- reserved role name, so it doesn't exist in DB yet
32+
'',
33+
id,
34+
'[]'::jsonb,
35+
'[]'::jsonb,
36+
'[]'::jsonb,
37+
'[]'::jsonb,
38+
true,
39+
NOW(),
40+
NOW()
41+
FROM
42+
organizations
43+
WHERE
44+
deleted = false
45+
AND NOT EXISTS (
46+
SELECT 1
47+
FROM custom_roles
48+
WHERE
49+
custom_roles.name = 'organization-member'
50+
AND custom_roles.organization_id = organizations.id
51+
);

0 commit comments

Comments
 (0)