feat: configurable default org member roles by Emyrk · Pull Request #25994 · coder/coder

Emyrk · 2026-06-02T16:29:35Z

Refs #25936. Stacks on #25929.

Adds a configurable per-org default member role set. Unioned into each member's effective roles at read time.

Default {organization-workspace-access} preserves behavior.
PATCH gated by the minimum-implicit-member experiment.
Only built-in role names accepted; custom or malformed names rejected with 400.
Diff routed through canAssignRoles. Caller must be permitted to assign each added or removed role.
Per-org, not per-user.
Resolve-at-read in GetAuthorizationUserRoles. No backfill.

Implementation notes

Experiment constant lands here so the write-gate compiles. Floor-shrink behavior is a follow-up.
Off-deployment PATCH error: Gateway Accounts are not enabled on this deployment.
Per-user demotion is intentionally unsupported. To gate a gateway-accounts org, an admin shrinks the org's default.
Migration 000516 adds the column nullable, backfills with ARRAY['organization-workspace-access'], then sets NOT NULL.
No SQL DEFAULT. rbac.DefaultOrgMemberRoles() is the sole source; every InsertOrganization site must supply it. NOT NULL is the fail-closed backstop.
Single union point: GetAuthorizationUserRoles joins organizations and array_cats the column into the unnest. Applies to regular users and service accounts.
rbac.DefaultOrgMemberRoles() is a function, not a var, so callers cannot mutate the shared default.
Audit table tracks the new column.
UpdateOrganizationParams (sqlc-gen) preserves the existing value when the request omits the field.

_{Coder Agents on behalf of @Emyrk.}

github-actions · 2026-06-02T16:29:50Z

Docs preview

📖 View docs preview for docs/admin/security/audit-logs.md

Emyrk · 2026-06-02T18:40:55Z

Design change: dropped the SQL DEFAULT on default_org_member_roles; the Go const rbac.DefaultOrgMemberRoles is now the single source of truth.

Migration 000515 adds the column nullable, backfills existing orgs with ARRAY['organization-workspace-access'], then sets NOT NULL. No persistent DEFAULT.
InsertOrganization SQL query now requires default_org_member_roles. All call sites pass rbac.DefaultOrgMemberRoles (production: enterprise/coderd/organizations.go; test fixture: dbgen.Organization; 5 direct test sites).
Dropped TestDefaultOrgMemberRolesMatchesSQLDefault (no SQL DEFAULT to drift from).
Test expectations in coderd/httpmw/authorize_test.go and coderd/provisionerdserver/provisionerdserver_test.go updated to include organization-workspace-access in the owner role list, since GetAuthorizationUserRoles now correctly unions it in via the default.

Rationale: the SQL DEFAULT only did real work once (the ALTER TABLE backfill); every production INSERT goes through sqlc, which lists every column, so the DEFAULT was dead weight afterward. Removing it eliminates the drift surface entirely and forces conscious INSERTs.

_{Coder Agents on behalf of @Emyrk.}

Emyrk · 2026-06-02T20:31:34Z

+ALTER TABLE organizations
+    ADD COLUMN default_org_member_roles text[];
+
+UPDATE organizations
+SET default_org_member_roles = ARRAY['organization-workspace-access']::text[];
+
+ALTER TABLE organizations
+    ALTER COLUMN default_org_member_roles SET NOT NULL;
+
+COMMENT ON COLUMN organizations.default_org_member_roles IS
+    'Roles granted to every member of this organization at request time. '
+    'The set is unioned into each member''s effective roles when '
+    'GetAuthorizationUserRoles runs, so changes propagate to all members '
+    'on the next request. Deployments can use this column to revoke '
+    'capabilities that would otherwise be considered normal organization '
+    'member permissions.';


Existing orgs are backfilled this way

Emyrk · 2026-06-02T20:33:36Z

/coder-agents-review

coder-agents-review · 2026-06-02T20:33:39Z

Chat: Review posted | View chat
Requested: 2026-06-02 20:33 UTC by @Emyrk
Spend: $43.03 / $100.00

Review history

R1 (2026-06-02): 12 reviewers, 4 Nit, 3 Note, 1 P1, 4 P3, REQUEST_CHANGES. Review

deep-review v0.6.1 | Round 1 | 6aef7c0..1451f52

Last posted: Round 1, 12 findings (1 P1, 4 P3, 4 Nit, 3 Note), REQUEST_CHANGES. Review

Finding inventory

Finding Inventory

Findings

#	Sev	Status	Location	Summary	Round	Reviewer	Posted
CRF-1	P1	Open	enterprise/coderd/organizations.go:108	No validation of role names in default_org_member_roles; privilege escalation and DoS	R1	Hisoka P1, Kurapika P1, Mafuuu P2, Meruem P2, Knov P2, Knuckle P2, Razor P2, Pariston P3, Bisky P3	Yes
CRF-2	P3	Open	coderd/rbac/roles.go:219	DefaultOrgMemberRoles is an exported mutable var slice	R1	Hisoka, Kurapika, Meruem, Knov, Razor, Gon, Mafuuu	Yes
CRF-3	P3	Open	coderd/database/dbgen/dbgen.go:1043	takeFirstSlice swallows intentional empty DefaultOrgMemberRoles	R1	Bisky, Mafuuu, Meruem, Knuckle	Yes
CRF-4	P3	Open	coderd/rbac/roles.go:216	DefaultOrgMemberRoles comment omits sync invariant and authoritative-source doc	R1	Gon, Leorio	Yes
CRF-5	P3	Open	coderd/database/querier_test.go:3043	No test that service accounts also receive default_org_member_roles	R1	Bisky	Yes
CRF-6	Nit	Open	coderd/database/queries/users.sql:618	Endash (U+2013) in SQL comment adjacent to new code	R1	Bisky, Leorio, Kurapika	Yes
CRF-7	Nit	Open	codersdk/deployment.go:5006	Experiment naming: slug names mechanism not feature; display name includes jargon	R1	Gon, Leorio	Yes
CRF-8	Nit	Open	enterprise/coderd/organizations.go:65	Guard comment restates the if-condition	R1	Gon	Yes
CRF-9	Nit	Open	codersdk/organizations.go:120	Doc comment produces confusing generated API docs	R1	Leorio	Yes
CRF-10	Note	Open	coderd/database/queries/users.sql:633	Service accounts receive default_org_member_roles uniformly; design intent should be explicit	R1	Pariston, Meruem, Razor	Yes
CRF-11	Note	Open	coderd/database/migrations/000515_org_default_member_roles.up.sql:1	PR description claims migration adds DEFAULT clause; it does not	R1	Leorio, Hisoka, Mafuuu, Meruem, Pariston, Razor	Yes
CRF-12	Note	Open	coderd/rbac/roles.go:1186	Shrinking default_org_member_roles is a no-op until follow-up removes elevation from org-member	R1	Knov	Yes

Round log

Round 1

Panel (12 reviewers: Bisky, Hisoka, Mafuuu, Pariston, Gon, Leorio, Knuckle, Kurapika, Meruem, Killua, Knov, Razor). Netero: 0 findings. 1 P1, 4 P3, 4 Nit, 3 Note. Reviewed against 6aef7c0..1451f52.

About deep-review

CRF = Coder Review Finding (P0-P4, Nit, Note)

Reviewer	Focus
Bisky	tests
Chopper	ops/errors
Churn-guard	change verification
Ging	language modernization
Gon	naming
Hisoka	edge cases
Killua	perf
Kite	change integrity
Knov	contracts
Knuckle	SQL
Kurapika	security
Law	decomposition
Leorio	docs
Luffy	product
Mafu-san	process
Mafuuu	contracts
Melody	dispatch/pairing
Meruem	structural
Nami	frontend
Netero	mechanical checks
Pariston	premise testing
Pen-botter	product gaps
Razor	verification
Robin	duplication
Ryosuke	Go arch
Takumi	concurrency
Zoro	shape

🤖 Managed by Coder Agents.

coder-agents-review

Well-structured preparation PR. The resolve-at-read approach via GetAuthorizationUserRoles is a clean design: one SQL union point, no backfill needed, atomic propagation on next request. Test density is strong at 60%, and the three enterprise subtests cover the experiment gate thoroughly.

1 P1, 4 P3, 4 Nit, 3 Note.

The critical gap is the absence of any validation on role names written to default_org_member_roles. Nine of twelve reviewers converged on this independently. The P1 finding (CRF-1) traces two concrete attack paths: privilege escalation through built-in role injection, and DoS through malformed names that break RoleNameFromString and produce 500s for every org member. The experiment gate limits blast radius today but provides no structural protection.

Additional observations:

PR description inaccuracy (CRF-11): The implementation notes state the migration adds a column with DEFAULT ARRAY['organization-workspace-access']::text[]. The actual migration adds the column without a DEFAULT, backfills existing rows, then sets NOT NULL. The Go code supplies the value at every insertion point. Consider correcting the description to avoid misleading future readers.
PR title scope: The scope coderd does not contain all changed files (changes span coderd/, codersdk/, enterprise/, site/). Per project rules, either use a broader path scope or omit the scope: feat: add organizations.default_org_member_roles for Gateway Accounts.

"The assumption 'callers will only shrink, not escalate' is invisible." (Meruem)

coderd/rbac/roles.go:1186

Note [CRF-12] Shrinking default_org_member_roles to empty is currently a no-op. organization-member (line 1181) and organization-service-account (line 1258) still bundle OrgWorkspaceAccessMemberPerms(). An operator who enables the experiment and sets default_org_member_roles = [] gets a successful API response, but effective permissions don't change because the member role still grants the same workspace-ops set. The PR acknowledges this as staged delivery. Noting it here so the follow-up is not forgotten. (Knov)

🤖

🤖 This review was automatically generated with Coder Agents.

Emyrk · 2026-06-02T22:03:42Z

Per follow-up review on the audit: rolled the dbauthz fix into this PR rather than splitting it out.

InsertOrganizationMember and UpdateMemberRoles now fetch the org's default_org_member_roles and include them in canAssignRoles's implied set. The caller is required to be authorized to assign the full effective role set (explicit roles + organization-member + the org's defaults), matching what GetAuthorizationUserRoles will hand back to the new member at request time.

Two new q.db.GetOrganizationByID lookups (bypass dbauthz so the caller doesn't need org-read).
Reuses the existing convertToOrganizationRoles helper to scope role names to the org.
Comments updated to drop the "deferred to config" caveat.
coderd/rbac/roles.go adds orgWorkspaceAccess: true to prebuildsOrchestrator's assignable set. Without it, the orchestrator can't add the prebuild user to an org whose default contains organization-workspace-access, which broke TestReinitializeAgent/useDefaultOrg=false.
dbauthz_test.go for InsertOrganizationMember and UpdateMemberRoles now seed the fake org with DefaultOrgMemberRoles: []string{} and add a GetOrganizationByID mock; existing ActionAssign assertions stay one per call (canAssignRoles emits a single ActionAssign regardless of how many roles are in the diff).

Full test sweep: coderd/rbac, coderd/database/dbauthz, enterprise/coderd (TestPatchOrganizationsByUser, TestReinitializeAgent), coderd (TestAddMember) all clean.

_{Coder Agents on behalf of @Emyrk.}

Emyrk · 2026-06-02T22:11:21Z

Follow-up amend (c5afc1cee6): folded the idpsync filter fix from the audit into this PR so the diff stays consistent with the new resolve-at-read semantics.

Why: (AGPLIDPSync).SyncRoles only stripped the literal organization-member role from both sides of the IdP diff. With default_org_member_roles now in play, any role listed there would otherwise look like a diff against OrganizationMember.Roles (which never stores defaults), so sync would try to re-grant or remove it forever.

Change in coderd/idpsync/role.go:

Inside the per-org loop, fetch org.DefaultOrgMemberRoles via tx.GetOrganizationByID (ctx is already dbauthz.AsSystemRestricted).
Build an implicit set = {organization-member} ∪ default_org_member_roles.
Delete entries in that set from both validExpected and existingFound before the equality check.

Test: TestNoopNoDiff gained the new GetOrganizationByID expectation returning DefaultOrgMemberRoles: []string{} so existing semantics hold. TestRoleSyncTable already routes through dbgen.Organization, which now defaults to rbac.DefaultOrgMemberRoles; the suite passes because none of the table cases claim organization-workspace-access.

_{Coder Agents on behalf of @Emyrk.}

Emyrk · 2026-06-02T22:20:24Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

Emyrk · 2026-06-02T23:03:50Z

Bot review response (4d52fbe8b8):

#	Resolution
CRF-1 (P1) role validation	Done. Two-layer fix: API rejects non-built-in names (400 `Invalid default_org_member_roles entry.`); dbauthz `UpdateOrganization` routes default-role diffs through `canAssignRoles` so the caller must already be permitted to assign each added/removed role.
CRF-2 (P3) `var` → `func`	Done. 12 call sites updated. Doc now also covers the CRF-4 sync invariant.
CRF-3 (P3) `takeFirstSlice` empty trap	Not fixed in this PR. `dbgen` is test-only; existing dbauthz mocks already seed `[]string{}` explicitly. Revisit if a future test author hits the trap.
CRF-4 (P3) doc invariant	Folded into CRF-2.
CRF-5 (P3) service-account test	Done. `TestGetAuthorizationUserRolesUnionsDefaultOrgMemberRoles` now exercises both a regular user and `IsServiceAccount: true`.
CRF-6 (Nit) endash	Fixed `permissions–such as` → `permissions, such as`. `make gen` regenerated `queries.sql.go`.
CRF-7 (Nit) experiment slug rename	Keeping `minimum-implicit-member`; it names the merge-safety gate accurately.
CRF-8 (Nit) guard comment trim	Done, took your suggested wording.
CRF-9 (Nit) codersdk doc rewording	Not in this PR; will land with the next codersdk-touching change.
CRF-10 (Note) service accounts share defaults	Confirmed intended; CRF-5 test now covers it.
CRF-11 (Note) PR description stale	PR description updated.
CRF-12 (Note) empty defaults no-op today	Acknowledged staged delivery. Once `OrgWorkspaceAccessMemberPerms` is removed from `organization-member` and `organization-service-account` (follow-up PR), `default_org_member_roles = []` becomes the actual shrink mechanism.
Title scope	Dropped the `coderd` scope; changes span `coderd/`, `codersdk/`, `cli/`, `docs/`, `enterprise/`, `site/`.

_{Coder Agents on behalf of @Emyrk.}

Emyrk · 2026-06-03T15:32:03Z

 	addedRoles := append(orgRoles, rbac.ScopedRoleOrgMember(arg.OrganizationID))
+	addedRoles = append(addedRoles, defaultRoles...)
 	err = q.canAssignRoles(ctx, arg.OrganizationID, addedRoles, []rbac.RoleIdentifier{})
 	if err != nil {
 		return database.OrganizationMember{}, err
 	}


As this is written, you need perms to assign the default role to change an org members perms. Right now role assign is very global, but this could be an issue later if we were change that.

I'm leaving it as is today.

Emyrk · 2026-06-03T22:19:07Z

+UPDATE organizations
+SET default_org_member_roles = ARRAY['organization-workspace-access']::text[];


Backfill all existing organizations

Adds a new `default_org_member_roles` set attribute to the `coderd_organization` resource so callers can override the deployment-wide default member roles per organization. The attribute is set after `CreateOrganization` (the create endpoint doesn't accept it) and through `UpdateOrganization` thereafter. Also bumps coder/coder to a SHA that contains the gateway-accounts stack (coder/coder#25994) and reacts to coder/coder#24984, which migrated UpdateTemplateMeta fields from values to optional pointers.

geokat · 2026-06-05T17:39:58Z

+func DefaultOrgMemberRoles() []string {
+	return []string{orgWorkspaceAccess}
+}


Do we need to catch up existing organizations as this evolves?

There is a PR that backfills existing orgs. So this is covered

I see the one-time migration backfill for introducing default_org_member_roles, but I don’t see anything that would catch up existing orgs if DefaultOrgMemberRoles() changes later. Was your reply referring just to the introduction backfill, or is there another reconcile path I’m missing?

Ah. If we change DefaultOrgMemberRoles, you are right there is no resolution mechanism. I'll add a comment to this effect.

So it only catches up orgs to this state.

Changing it later is a bit tough to reconcile because after this feature goes live, it can be changed from the default set.

Adds a configurable per-org default member role set, unioned into each member's effective roles at read time via GetAuthorizationUserRoles. PATCH gated by the minimum-implicit-member experiment. Only built-in role names accepted. Diff routed through canAssignRoles. Per-org, not per-user. Default {organization-workspace-access} preserves behavior. Refs #25936.

- querier_test.go: the assertion only checks effective roles, so do not claim the test verifies that organization_members rows were untouched. - enterprise/coderd: replace the user-facing "Gateway Accounts are not enabled" message with a description tied to the actual API surface: changing default organization roles.

ai

github-actions Bot assigned Emyrk Jun 2, 2026

Emyrk commented Jun 2, 2026

View reviewed changes

Comment thread coderd/database/migrations/000515_org_default_member_roles.up.sql Outdated

Comment thread coderd/rbac/roles.go Outdated

Comment thread codersdk/organizations.go

Comment thread codersdk/organizations.go

Emyrk force-pushed the gateway-accounts/org-default-roles branch 2 times, most recently from 55edab1 to 1451f52 Compare June 2, 2026 18:40

Emyrk commented Jun 2, 2026

View reviewed changes

coder-agents-review Bot previously requested changes Jun 2, 2026

View reviewed changes

Emyrk force-pushed the gateway-accounts/org-default-roles branch from 1451f52 to 7511db4 Compare June 2, 2026 21:36

Emyrk mentioned this pull request Jun 2, 2026

userRoles handler should resolve effective roles via GetAuthorizationUserRoles #26002

Open

Emyrk force-pushed the gateway-accounts/org-default-roles branch from 7511db4 to 3efb4a4 Compare June 2, 2026 22:03

Emyrk force-pushed the gateway-accounts/org-default-roles branch from 3efb4a4 to c5afc1c Compare June 2, 2026 22:11

This was referenced Jun 2, 2026

test: include per-org default roles in rbac user subjects #26003

Merged

refactor: extract organization-workspace-access role #25929

Merged

refactor(coderd/rbac): enumerate org-member and org-service-account perms #25928

Merged

Emyrk force-pushed the gateway-accounts/org-default-roles branch from c5afc1c to 4d52fbe Compare June 2, 2026 23:02

Emyrk changed the title ~~feat(coderd): add organizations.default_org_member_roles for Gateway Accounts~~ feat: add organizations.default_org_member_roles for Gateway Accounts Jun 2, 2026

Emyrk force-pushed the gateway-accounts/org-default-roles branch 2 times, most recently from 286774c to e6dfeb2 Compare June 2, 2026 23:28

Emyrk force-pushed the gateway-accounts/extract-workspace-access branch 2 times, most recently from fb33230 to eafa7f5 Compare June 3, 2026 13:15

Emyrk force-pushed the gateway-accounts/org-default-roles branch 2 times, most recently from 85045f1 to d15a3f0 Compare June 3, 2026 13:58

Emyrk force-pushed the gateway-accounts/extract-workspace-access branch from eafa7f5 to 0d366d9 Compare June 3, 2026 13:58

Emyrk force-pushed the gateway-accounts/org-default-roles branch from 9edae41 to ad60bf1 Compare June 3, 2026 20:48

Emyrk changed the base branch from gateway-accounts/extract-workspace-access to graphite-base/25994 June 3, 2026 21:08

Emyrk force-pushed the gateway-accounts/org-default-roles branch 6 times, most recently from b96becd to 56ddb69 Compare June 3, 2026 22:20

Emyrk force-pushed the graphite-base/25994 branch from e9a3bd8 to c746ace Compare June 3, 2026 22:23

Emyrk marked this pull request as ready for review June 3, 2026 22:24

Emyrk force-pushed the gateway-accounts/org-default-roles branch from 56ddb69 to dce59fb Compare June 3, 2026 22:29

Emyrk force-pushed the graphite-base/25994 branch from c746ace to eeadd81 Compare June 3, 2026 22:29

Emyrk commented Jun 4, 2026

View reviewed changes

Emyrk requested a review from geokat June 4, 2026 14:42

Emyrk changed the base branch from graphite-base/25994 to gateway-accounts/extract-workspace-access June 4, 2026 14:49

Emyrk force-pushed the gateway-accounts/extract-workspace-access branch from eeadd81 to 010e350 Compare June 5, 2026 14:19

Emyrk force-pushed the gateway-accounts/org-default-roles branch from dce59fb to 662646e Compare June 5, 2026 14:19

Emyrk mentioned this pull request Jun 5, 2026

feat: add default_org_member_roles to coderd_organization coder/terraform-provider-coderd#355

Draft

Base automatically changed from gateway-accounts/extract-workspace-access to main June 5, 2026 15:30

Emyrk force-pushed the gateway-accounts/org-default-roles branch from 662646e to 16724f9 Compare June 5, 2026 15:32

Emyrk mentioned this pull request Jun 5, 2026

feat(site): show org default roles in member role editor #26107

Merged

geokat approved these changes Jun 5, 2026

View reviewed changes

Emyrk added 2 commits June 5, 2026 19:19

Emyrk force-pushed the gateway-accounts/org-default-roles branch from ed135cf to 0474e9f Compare June 5, 2026 19:20

Emyrk merged commit 938c208 into main Jun 5, 2026
31 checks passed

Emyrk deleted the gateway-accounts/org-default-roles branch June 5, 2026 19:33

Emyrk mentioned this pull request Jun 15, 2026

fix(coderd/rbac): guard builtInRoles with atomic.Pointer #26384

Merged

		UPDATE organizations
		SET default_org_member_roles = ARRAY['organization-workspace-access']::text[];

Conversation

Emyrk commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 2, 2026

Docs preview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Emyrk commented Jun 2, 2026

Uh oh!

Emyrk Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

Emyrk commented Jun 2, 2026

Uh oh!

coder-agents-review Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Finding Inventory

Findings

Round log

Round 1

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Emyrk commented Jun 2, 2026

Uh oh!

Emyrk commented Jun 2, 2026

Uh oh!

Emyrk commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Emyrk commented Jun 2, 2026

Uh oh!

Emyrk Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

Emyrk Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

geokat Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

Emyrk Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

geokat Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Emyrk Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Emyrk commented Jun 2, 2026 •

edited

Loading

coder-agents-review Bot commented Jun 2, 2026 •

edited

Loading

Emyrk commented Jun 2, 2026 •

edited

Loading

geokat Jun 5, 2026 •

edited

Loading