Skip to content

feat: implement organization "disable workspace sharing" option #21297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
geokat wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

feat: implement organization "disable workspace sharing" option #21297

geokat wants to merge 3 commits into from
+456 −100

Conversation

@geokat
Copy link
Contributor

@geokat geokat commented Dec 16, 2025 •
edited
Loading

wip: the PR is currently in draft mode to discuss the migration of the org-member role to the DB

Migrating Org-Member Role to Database

Problem

The organization-member role is hardcoded. This makes it easy to keep it in sync with resource types supported by Coder, but prevents per-org customization (e.g., workspace_sharing_disabled).

Solution: Database-Backed System Roles

Store organization-member roles per-org in the custom_roles table with an is_system flag. Create the roles during org creation and reconcile them at startup to keep permissions in sync with the codebase.

  1. Schema Changes
  • Added is_system boolean column to the custom_roles table — marks Coder-managed roles
  • Added member_permissions — for member-scoped permissions (resources owned by the user)
  • Added workspace_sharing_disabled to organizations — per-org setting
  1. Migration
  • Brings existing organizations up to date by creating empty placeholder organization-member system roles for all existing organizations
  • Permissions are empty initially (eventually backfilled by the startup reconciliation hook)

UPDATE: The migration has been removed. The startup hook (ReconcileOrgMemberRoles) already handles role creation with advisory locking. The migration only created empty placeholders with no permission effect.

  1. Permission Generation (coderd/rbac/roles.go)
  • OrgMemberPermissions(workspaceSharingDisabled bool) is the source of truth
  • Generates org-level and member-level permissions dynamically
  • Respects the workspace sharing setting (adds negation for ActionShare when disabled)
  1. Org Creation Hook (enterprise/coderd/organizations.go)
  • When a new org is created, the organization-member role is created in the same transaction
  • Uses OrgMemberPermissions(false) since new orgs have sharing enabled by default
  1. Startup Reconciliation (coderd/systemroles.go)
  • ReconcileOrgMemberRoles() runs at startup with an advisory lock
  • Creates missing roles
  • Compares expected vs stored permissions using set-based comparison
  • Updates roles if permissions differ (handles RBAC resource changes)
  • Blocking lock ensures each instance reconciles, catching orgs created by old instances

Key Design Decisions

Decision Rationale
Permissions as source of truth in code Moving them to the DB is a big project; duplicating them in the DB is error-prone
Introduce system roles is_system = true makes them transparent to the CRUD API
Startup reconciliation Resource type changes in code are propagated to roles stored in the DB
Set-based comparison Avoids unnecessary writes when permissions are unchanged
Blocking advisory lock Ensures correctness during rolling upgrades

closes: coder/internal#1073

@bpmct bpmct added the doc-check Assign this label to PRs to check for any doc changes. label Dec 16, 2025
@geokat
Copy link
Contributor Author

geokat commented Dec 16, 2025

@aslilac @Emyrk More work is needed to address all of the coder/internal#1073 requirements, but I wanted to run this approach for migrating the built-in role by you early on. The code isn't ready for review yet, but it should demonstrate the pros and cons of the chosen approach.

Other alternatives I can see--using the DB as the source of truth for permissions or duplicating them in the DB--seem to be more involved and error-prone than running reconciliation at startup. But please let me know if you disagree or have other ideas.

@geokat geokat force-pushed the geokat/internal-1073-organization-disable-workspace-sharing-option branch 5 times, most recently from 9614436 to 9b7cfc9 Compare December 17, 2025 23:37
@geokat geokat force-pushed the geokat/internal-1073-organization-disable-workspace-sharing-option branch from 9b7cfc9 to 764fd62 Compare December 18, 2025 04:06
@geokat
refactor(database): remove migration creating empty org-member system…
e6abc24 
… roles

The startup hook (ReconcileOrgMemberRoles) already handles role creation
with advisory locking. The migration only created empty placeholders with
no permission effect.
@geokat
chore: improve comment
5c8bf9b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Emyrk Emyrk Awaiting requested review from Emyrk Emyrk will be requested when the pull request is marked ready for review Emyrk is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@geokat geokat

Labels

doc-check Assign this label to PRs to check for any doc changes.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implement organization "disable workspace sharing" option

2 participants

@geokat @bpmct