From 91ec0f1484cbb61ffbe81f17c831a7954e84ebb4 Mon Sep 17 00:00:00 2001 From: George K Date: Tue, 17 Mar 2026 12:16:43 -0700 Subject: [PATCH] feat: add service_accounts workspace sharing mode (#23093) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a three-way workspace sharing setting (none, everyone, service_accounts) replacing the boolean workspace_sharing_disabled. In service_accounts mode, only service account-owned workspaces can be shared while regular members' share permissions are removed. Adds a new organization-service-account system role with per-org permissions reconciled alongside the existing organization-member system role. Related to: https://linear.app/codercom/issue/PLAT-28/feat-service-accounts-sharing-mode-and-rbac-role --------- Co-authored-by: Steven Masley Co-authored-by: Kayla はな --- cli/organizationroles.go | 2 +- coderd/apidoc/docs.go | 45 +++- coderd/apidoc/swagger.json | 33 ++- coderd/coderdtest/coderdtest.go | 9 + coderd/database/dbauthz/dbauthz.go | 6 +- coderd/database/dbauthz/dbauthz_test.go | 11 +- coderd/database/dbauthz/setup_test.go | 3 +- coderd/database/dbgen/dbgen.go | 42 ++- coderd/database/dbmetrics/querymetrics.go | 6 +- coderd/database/dbmock/dbmock.go | 8 +- coderd/database/dump.sql | 29 +- ...ons_for_allowed_workspace_sharing.down.sql | 52 ++++ ...tions_for_allowed_workspace_sharing.up.sql | 101 +++++++ ...e_organization_service_account_role.up.sql | 28 ++ coderd/database/models.go | 82 +++++- coderd/database/querier.go | 2 +- coderd/database/querier_test.go | 225 ++++++++++++---- coderd/database/queries.sql.go | 73 +++-- coderd/database/queries/organizations.sql | 2 +- coderd/database/queries/users.sql | 16 +- coderd/database/queries/workspaces.sql | 8 +- coderd/rbac/authz_internal_test.go | 8 +- coderd/rbac/roles.go | 177 +++++++++---- coderd/rbac/roles_test.go | 92 ++++--- coderd/rbac/rolestore/rolestore.go | 164 +++++++----- coderd/rbac/rolestore/rolestore_test.go | 126 +++++---- coderd/workspaces.go | 8 +- codersdk/workspacesharing.go | 24 +- docs/admin/security/audit-logs.md | 2 +- docs/reference/api/enterprise.md | 23 +- docs/reference/api/schemas.md | 44 +++- enterprise/audit/table.go | 2 +- enterprise/coderd/groups_test.go | 6 +- enterprise/coderd/organizations.go | 20 +- enterprise/coderd/roles_test.go | 6 +- enterprise/coderd/workspacesharing.go | 95 +++++-- enterprise/coderd/workspacesharing_test.go | 249 +++++++++++++++++- site/src/api/typesGenerated.ts | 29 ++ 38 files changed, 1437 insertions(+), 421 deletions(-) create mode 100644 coderd/database/migrations/000443_three_options_for_allowed_workspace_sharing.down.sql create mode 100644 coderd/database/migrations/000443_three_options_for_allowed_workspace_sharing.up.sql create mode 100644 coderd/database/migrations/testdata/fixtures/000438_pre_organization_service_account_role.up.sql diff --git a/cli/organizationroles.go b/cli/organizationroles.go index 8e0bc5a121..37ce980378 100644 --- a/cli/organizationroles.go +++ b/cli/organizationroles.go @@ -214,7 +214,7 @@ func (r *RootCmd) createOrganizationRole(orgContext *OrganizationContext) *serpe } else { updated, err = client.CreateOrganizationRole(ctx, customRole) if err != nil { - return xerrors.Errorf("patch role: %w", err) + return xerrors.Errorf("create role: %w", err) } } diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index a4afb616e6..c67919e3be 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -4819,7 +4819,7 @@ const docTemplate = `{ "in": "body", "required": true, "schema": { - "$ref": "#/definitions/codersdk.WorkspaceSharingSettings" + "$ref": "#/definitions/codersdk.UpdateWorkspaceSharingSettingsRequest" } } ], @@ -4827,7 +4827,7 @@ const docTemplate = `{ "200": { "description": "OK", "schema": { - "$ref": "#/definitions/codersdk.UpdateWorkspaceSharingSettingsRequest" + "$ref": "#/definitions/codersdk.WorkspaceSharingSettings" } } } @@ -18720,6 +18720,19 @@ const docTemplate = `{ } } }, + "codersdk.ShareableWorkspaceOwners": { + "type": "string", + "enum": [ + "none", + "everyone", + "service_accounts" + ], + "x-enum-varnames": [ + "ShareableWorkspaceOwnersNone", + "ShareableWorkspaceOwnersEveryone", + "ShareableWorkspaceOwnersServiceAccounts" + ] + }, "codersdk.SharedWorkspaceActor": { "type": "object", "properties": { @@ -20328,7 +20341,21 @@ const docTemplate = `{ "codersdk.UpdateWorkspaceSharingSettingsRequest": { "type": "object", "properties": { + "shareable_workspace_owners": { + "description": "ShareableWorkspaceOwners controls whose workspaces can be shared\nwithin the organization.", + "enum": [ + "none", + "everyone", + "service_accounts" + ], + "allOf": [ + { + "$ref": "#/definitions/codersdk.ShareableWorkspaceOwners" + } + ] + }, "sharing_disabled": { + "description": "SharingDisabled is deprecated and left for backward compatibility\npurposes.\nDeprecated: use ` + "`" + `ShareableWorkspaceOwners` + "`" + ` instead", "type": "boolean" } } @@ -22169,7 +22196,21 @@ const docTemplate = `{ "codersdk.WorkspaceSharingSettings": { "type": "object", "properties": { + "shareable_workspace_owners": { + "description": "ShareableWorkspaceOwners controls whose workspaces can be shared\nwithin the organization.", + "enum": [ + "none", + "everyone", + "service_accounts" + ], + "allOf": [ + { + "$ref": "#/definitions/codersdk.ShareableWorkspaceOwners" + } + ] + }, "sharing_disabled": { + "description": "SharingDisabled is deprecated and left for backward compatibility\npurposes.\nDeprecated: use ` + "`" + `ShareableWorkspaceOwners` + "`" + ` instead", "type": "boolean" }, "sharing_globally_disabled": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 99dbcd6c51..c00b610772 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -4262,7 +4262,7 @@ "in": "body", "required": true, "schema": { - "$ref": "#/definitions/codersdk.WorkspaceSharingSettings" + "$ref": "#/definitions/codersdk.UpdateWorkspaceSharingSettingsRequest" } } ], @@ -4270,7 +4270,7 @@ "200": { "description": "OK", "schema": { - "$ref": "#/definitions/codersdk.UpdateWorkspaceSharingSettingsRequest" + "$ref": "#/definitions/codersdk.WorkspaceSharingSettings" } } } @@ -17109,6 +17109,15 @@ } } }, + "codersdk.ShareableWorkspaceOwners": { + "type": "string", + "enum": ["none", "everyone", "service_accounts"], + "x-enum-varnames": [ + "ShareableWorkspaceOwnersNone", + "ShareableWorkspaceOwnersEveryone", + "ShareableWorkspaceOwnersServiceAccounts" + ] + }, "codersdk.SharedWorkspaceActor": { "type": "object", "properties": { @@ -18645,7 +18654,17 @@ "codersdk.UpdateWorkspaceSharingSettingsRequest": { "type": "object", "properties": { + "shareable_workspace_owners": { + "description": "ShareableWorkspaceOwners controls whose workspaces can be shared\nwithin the organization.", + "enum": ["none", "everyone", "service_accounts"], + "allOf": [ + { + "$ref": "#/definitions/codersdk.ShareableWorkspaceOwners" + } + ] + }, "sharing_disabled": { + "description": "SharingDisabled is deprecated and left for backward compatibility\npurposes.\nDeprecated: use `ShareableWorkspaceOwners` instead", "type": "boolean" } } @@ -20384,7 +20403,17 @@ "codersdk.WorkspaceSharingSettings": { "type": "object", "properties": { + "shareable_workspace_owners": { + "description": "ShareableWorkspaceOwners controls whose workspaces can be shared\nwithin the organization.", + "enum": ["none", "everyone", "service_accounts"], + "allOf": [ + { + "$ref": "#/definitions/codersdk.ShareableWorkspaceOwners" + } + ] + }, "sharing_disabled": { + "description": "SharingDisabled is deprecated and left for backward compatibility\npurposes.\nDeprecated: use `ShareableWorkspaceOwners` instead", "type": "boolean" }, "sharing_globally_disabled": { diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 9d8adaa699..c0a0777ddc 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -879,6 +879,15 @@ func createAnotherUserRetry(t testing.TB, client *codersdk.Client, organizationI m(&req) } + // Service accounts cannot have a password or email and must + // use login_type=none. Enforce this after mutators so callers + // only need to set ServiceAccount=true. + if req.ServiceAccount { + req.Password = "" + req.Email = "" + req.UserLoginType = codersdk.LoginTypeNone + } + user, err := client.CreateUserWithOrgs(context.Background(), req) var apiError *codersdk.Error // If the user already exists by username or email conflict, try again up to "retries" times. diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 8eaa94d2df..de456e5026 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1264,7 +1264,7 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID uuid.UUID, added, re // System roles are stored in the database but have a fixed, code-defined // meaning. Do not rewrite the name for them so the static "who can assign // what" mapping applies. - if !rbac.SystemRoleName(roleName.Name) { + if !rolestore.IsSystemRoleName(roleName.Name) { // To support a dynamic mapping of what roles can assign what, we need // to store this in the database. For now, just use a static role so // owners and org admins can assign roles. @@ -2145,12 +2145,12 @@ func (q *querier) DeleteWorkspaceACLByID(ctx context.Context, id uuid.UUID) erro return fetchAndExec(q.log, q.auth, policy.ActionShare, fetch, q.db.DeleteWorkspaceACLByID)(ctx, id) } -func (q *querier) DeleteWorkspaceACLsByOrganization(ctx context.Context, organizationID uuid.UUID) error { +func (q *querier) DeleteWorkspaceACLsByOrganization(ctx context.Context, params database.DeleteWorkspaceACLsByOrganizationParams) error { // This is a system-only function. if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil { return err } - return q.db.DeleteWorkspaceACLsByOrganization(ctx, organizationID) + return q.db.DeleteWorkspaceACLsByOrganization(ctx, params) } func (q *querier) DeleteWorkspaceAgentPortShare(ctx context.Context, arg database.DeleteWorkspaceAgentPortShareParams) error { diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 6e82cd4932..761d9cabf3 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1485,7 +1485,7 @@ func (s *MethodTestSuite) TestOrganization() { org := testutil.Fake(s.T(), faker, database.Organization{}) arg := database.UpdateOrganizationWorkspaceSharingSettingsParams{ ID: org.ID, - WorkspaceSharingDisabled: true, + ShareableWorkspaceOwners: database.ShareableWorkspaceOwnersNone, } dbm.EXPECT().GetOrganizationByID(gomock.Any(), org.ID).Return(org, nil).AnyTimes() dbm.EXPECT().UpdateOrganizationWorkspaceSharingSettings(gomock.Any(), arg).Return(org, nil).AnyTimes() @@ -2404,9 +2404,12 @@ func (s *MethodTestSuite) TestWorkspace() { check.Args(w.ID).Asserts(w, policy.ActionShare) })) s.Run("DeleteWorkspaceACLsByOrganization", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { - orgID := uuid.New() - dbm.EXPECT().DeleteWorkspaceACLsByOrganization(gomock.Any(), orgID).Return(nil).AnyTimes() - check.Args(orgID).Asserts(rbac.ResourceSystem, policy.ActionUpdate) + arg := database.DeleteWorkspaceACLsByOrganizationParams{ + OrganizationID: uuid.New(), + ExcludeServiceAccounts: false, + } + dbm.EXPECT().DeleteWorkspaceACLsByOrganization(gomock.Any(), arg).Return(nil).AnyTimes() + check.Args(arg).Asserts(rbac.ResourceSystem, policy.ActionUpdate) })) s.Run("GetLatestWorkspaceBuildByWorkspaceID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { w := testutil.Fake(s.T(), faker, database.Workspace{}) diff --git a/coderd/database/dbauthz/setup_test.go b/coderd/database/dbauthz/setup_test.go index d4ef606010..7b305c2b10 100644 --- a/coderd/database/dbauthz/setup_test.go +++ b/coderd/database/dbauthz/setup_test.go @@ -29,6 +29,7 @@ import ( "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/coderd/rbac/regosql" + "github.com/coder/coder/v2/coderd/rbac/rolestore" "github.com/coder/coder/v2/coderd/util/slice" ) @@ -143,7 +144,7 @@ func (s *MethodTestSuite) Mocked(testCaseF func(dmb *dbmock.MockStore, faker *go UUID: pair.OrganizationID, Valid: pair.OrganizationID != uuid.Nil, }, - IsSystem: rbac.SystemRoleName(pair.Name), + IsSystem: rolestore.IsSystemRoleName(pair.Name), ID: uuid.New(), }) } diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 69d84fafdf..ac30be56c5 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -650,34 +650,26 @@ func Organization(t testing.TB, db database.Store, orig database.Organization) d }) require.NoError(t, err, "insert organization") - // Populate the placeholder organization-member system role (created by - // DB trigger/migration) so org members have expected permissions. - //nolint:gocritic // ReconcileOrgMemberRole needs the system:update + // Populate the placeholder system roles (created by DB + // trigger/migration) so org members have expected permissions. + //nolint:gocritic // ReconcileSystemRole needs the system:update // permission that `genCtx` does not have. sysCtx := dbauthz.AsSystemRestricted(genCtx) - _, _, err = rolestore.ReconcileOrgMemberRole(sysCtx, db, database.CustomRole{ - Name: rbac.RoleOrgMember(), - OrganizationID: uuid.NullUUID{ - UUID: org.ID, - Valid: true, - }, - }, org.WorkspaceSharingDisabled) - - if errors.Is(err, sql.ErrNoRows) { - // The trigger that creates the placeholder role didn't run (e.g., - // triggers were disabled in the test). Create the role manually. - err = rolestore.CreateOrgMemberRole(sysCtx, db, org) - require.NoError(t, err, "create organization-member role") - - _, _, err = rolestore.ReconcileOrgMemberRole(sysCtx, db, database.CustomRole{ - Name: rbac.RoleOrgMember(), - OrganizationID: uuid.NullUUID{ - UUID: org.ID, - Valid: true, - }, - }, org.WorkspaceSharingDisabled) + for roleName := range rolestore.SystemRoleNames { + role := database.CustomRole{ + Name: roleName, + OrganizationID: uuid.NullUUID{UUID: org.ID, Valid: true}, + } + _, _, err = rolestore.ReconcileSystemRole(sysCtx, db, role, org) + if errors.Is(err, sql.ErrNoRows) { + // The trigger that creates the placeholder role didn't run (e.g., + // triggers were disabled in the test). Create the role manually. + err = rolestore.CreateSystemRole(sysCtx, db, org, roleName) + require.NoError(t, err, "create role "+roleName) + _, _, err = rolestore.ReconcileSystemRole(sysCtx, db, role, org) + } + require.NoError(t, err, "reconcile role "+roleName) } - require.NoError(t, err, "reconcile organization-member role") return org } diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 471c549fd1..e6769a697b 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -696,10 +696,11 @@ func (m queryMetricsStore) DeleteWorkspaceACLByID(ctx context.Context, id uuid.U return r0 } -func (m queryMetricsStore) DeleteWorkspaceACLsByOrganization(ctx context.Context, organizationID uuid.UUID) error { +func (m queryMetricsStore) DeleteWorkspaceACLsByOrganization(ctx context.Context, arg database.DeleteWorkspaceACLsByOrganizationParams) error { start := time.Now() - r0 := m.s.DeleteWorkspaceACLsByOrganization(ctx, organizationID) + r0 := m.s.DeleteWorkspaceACLsByOrganization(ctx, arg) m.queryLatencies.WithLabelValues("DeleteWorkspaceACLsByOrganization").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "DeleteWorkspaceACLsByOrganization").Inc() return r0 } @@ -3971,6 +3972,7 @@ func (m queryMetricsStore) UpdateOrganizationWorkspaceSharingSettings(ctx contex start := time.Now() r0, r1 := m.s.UpdateOrganizationWorkspaceSharingSettings(ctx, arg) m.queryLatencies.WithLabelValues("UpdateOrganizationWorkspaceSharingSettings").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "UpdateOrganizationWorkspaceSharingSettings").Inc() return r0, r1 } diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 34f3a8131d..45b6e95f89 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -1155,17 +1155,17 @@ func (mr *MockStoreMockRecorder) DeleteWorkspaceACLByID(ctx, id any) *gomock.Cal } // DeleteWorkspaceACLsByOrganization mocks base method. -func (m *MockStore) DeleteWorkspaceACLsByOrganization(ctx context.Context, organizationID uuid.UUID) error { +func (m *MockStore) DeleteWorkspaceACLsByOrganization(ctx context.Context, arg database.DeleteWorkspaceACLsByOrganizationParams) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteWorkspaceACLsByOrganization", ctx, organizationID) + ret := m.ctrl.Call(m, "DeleteWorkspaceACLsByOrganization", ctx, arg) ret0, _ := ret[0].(error) return ret0 } // DeleteWorkspaceACLsByOrganization indicates an expected call of DeleteWorkspaceACLsByOrganization. -func (mr *MockStoreMockRecorder) DeleteWorkspaceACLsByOrganization(ctx, organizationID any) *gomock.Call { +func (mr *MockStoreMockRecorder) DeleteWorkspaceACLsByOrganization(ctx, arg any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteWorkspaceACLsByOrganization", reflect.TypeOf((*MockStore)(nil).DeleteWorkspaceACLsByOrganization), ctx, organizationID) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteWorkspaceACLsByOrganization", reflect.TypeOf((*MockStore)(nil).DeleteWorkspaceACLsByOrganization), ctx, arg) } // DeleteWorkspaceAgentPortShare mocks base method. diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index c289364b3a..ce24788cc4 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -512,6 +512,12 @@ CREATE TYPE resource_type AS ENUM ( 'ai_seat' ); +CREATE TYPE shareable_workspace_owners AS ENUM ( + 'none', + 'everyone', + 'service_accounts' +); + CREATE TYPE startup_script_behavior AS ENUM ( 'blocking', 'non-blocking' @@ -792,7 +798,7 @@ BEGIN END; $$; -CREATE FUNCTION insert_org_member_system_role() RETURNS trigger +CREATE FUNCTION insert_organization_system_roles() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN @@ -807,7 +813,8 @@ BEGIN is_system, created_at, updated_at - ) VALUES ( + ) VALUES + ( 'organization-member', '', NEW.id, @@ -818,6 +825,18 @@ BEGIN true, NOW(), NOW() + ), + ( + 'organization-service-account', + '', + NEW.id, + '[]'::jsonb, + '[]'::jsonb, + '[]'::jsonb, + '[]'::jsonb, + true, + NOW(), + NOW() ); RETURN NEW; END; @@ -1832,9 +1851,11 @@ CREATE TABLE organizations ( display_name text NOT NULL, icon text DEFAULT ''::text NOT NULL, deleted boolean DEFAULT false NOT NULL, - workspace_sharing_disabled boolean DEFAULT false NOT NULL + shareable_workspace_owners shareable_workspace_owners DEFAULT 'everyone'::shareable_workspace_owners NOT NULL ); +COMMENT ON COLUMN organizations.shareable_workspace_owners IS 'Controls whose workspaces can be shared: none, everyone, or service_accounts.'; + CREATE TABLE parameter_schemas ( id uuid NOT NULL, created_at timestamp with time zone NOT NULL, @@ -3863,7 +3884,7 @@ CREATE TRIGGER trigger_delete_oauth2_provider_app_token AFTER DELETE ON oauth2_p CREATE TRIGGER trigger_insert_apikeys BEFORE INSERT ON api_keys FOR EACH ROW EXECUTE FUNCTION insert_apikey_fail_if_user_deleted(); -CREATE TRIGGER trigger_insert_org_member_system_role AFTER INSERT ON organizations FOR EACH ROW EXECUTE FUNCTION insert_org_member_system_role(); +CREATE TRIGGER trigger_insert_organization_system_roles AFTER INSERT ON organizations FOR EACH ROW EXECUTE FUNCTION insert_organization_system_roles(); CREATE TRIGGER trigger_nullify_next_start_at_on_workspace_autostart_modificati AFTER UPDATE ON workspaces FOR EACH ROW EXECUTE FUNCTION nullify_next_start_at_on_workspace_autostart_modification(); diff --git a/coderd/database/migrations/000443_three_options_for_allowed_workspace_sharing.down.sql b/coderd/database/migrations/000443_three_options_for_allowed_workspace_sharing.down.sql new file mode 100644 index 0000000000..0a052076ce --- /dev/null +++ b/coderd/database/migrations/000443_three_options_for_allowed_workspace_sharing.down.sql @@ -0,0 +1,52 @@ +DELETE FROM custom_roles + WHERE name = 'organization-service-account' AND is_system = true; + +ALTER TABLE organizations + ADD COLUMN workspace_sharing_disabled boolean NOT NULL DEFAULT false; + +-- Migrate back: 'none' -> disabled, everything else -> enabled. +UPDATE organizations + SET workspace_sharing_disabled = true + WHERE shareable_workspace_owners = 'none'; + +ALTER TABLE organizations DROP COLUMN shareable_workspace_owners; + +DROP TYPE shareable_workspace_owners; + +-- Restore the original single-role trigger from migration 408. +DROP TRIGGER IF EXISTS trigger_insert_organization_system_roles ON organizations; +DROP FUNCTION IF EXISTS insert_organization_system_roles; + +CREATE OR REPLACE FUNCTION insert_org_member_system_role() RETURNS trigger AS $$ +BEGIN + INSERT INTO custom_roles ( + name, + display_name, + organization_id, + site_permissions, + org_permissions, + user_permissions, + member_permissions, + is_system, + created_at, + updated_at + ) VALUES ( + 'organization-member', + '', + NEW.id, + '[]'::jsonb, + '[]'::jsonb, + '[]'::jsonb, + '[]'::jsonb, + true, + NOW(), + NOW() + ); + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER trigger_insert_org_member_system_role + AFTER INSERT ON organizations + FOR EACH ROW + EXECUTE FUNCTION insert_org_member_system_role(); diff --git a/coderd/database/migrations/000443_three_options_for_allowed_workspace_sharing.up.sql b/coderd/database/migrations/000443_three_options_for_allowed_workspace_sharing.up.sql new file mode 100644 index 0000000000..ed6554ead5 --- /dev/null +++ b/coderd/database/migrations/000443_three_options_for_allowed_workspace_sharing.up.sql @@ -0,0 +1,101 @@ +CREATE TYPE shareable_workspace_owners AS ENUM ('none', 'everyone', 'service_accounts'); + +ALTER TABLE organizations + ADD COLUMN shareable_workspace_owners shareable_workspace_owners NOT NULL DEFAULT 'everyone'; + +COMMENT ON COLUMN organizations.shareable_workspace_owners IS 'Controls whose workspaces can be shared: none, everyone, or service_accounts.'; + +-- Migrate existing data from the boolean column. +UPDATE organizations + SET shareable_workspace_owners = 'none' + WHERE workspace_sharing_disabled = true; + +ALTER TABLE organizations DROP COLUMN workspace_sharing_disabled; + +-- Defensively rename any existing 'organization-service-account' roles +-- so they don't collide with the new system role. +UPDATE custom_roles + SET name = name || '-' || id::text + -- lower(name) is part of the existing unique index + WHERE lower(name) = 'organization-service-account'; + +-- Create skeleton organization-service-account system roles for all +-- existing organizations, mirroring what migration 408 did for +-- organization-member. +INSERT INTO custom_roles ( + name, + display_name, + organization_id, + site_permissions, + org_permissions, + user_permissions, + member_permissions, + is_system, + created_at, + updated_at +) +SELECT + 'organization-service-account', + '', + id, + '[]'::jsonb, + '[]'::jsonb, + '[]'::jsonb, + '[]'::jsonb, + true, + NOW(), + NOW() +FROM + organizations; + +-- Replace the single-role trigger with one that creates both system +-- roles when a new organization is inserted. +DROP TRIGGER IF EXISTS trigger_insert_org_member_system_role ON organizations; +DROP FUNCTION IF EXISTS insert_org_member_system_role; + +CREATE OR REPLACE FUNCTION insert_organization_system_roles() RETURNS trigger AS $$ +BEGIN + INSERT INTO custom_roles ( + name, + display_name, + organization_id, + site_permissions, + org_permissions, + user_permissions, + member_permissions, + is_system, + created_at, + updated_at + ) VALUES + ( + 'organization-member', + '', + NEW.id, + '[]'::jsonb, + '[]'::jsonb, + '[]'::jsonb, + '[]'::jsonb, + true, + NOW(), + NOW() + ), + ( + 'organization-service-account', + '', + NEW.id, + '[]'::jsonb, + '[]'::jsonb, + '[]'::jsonb, + '[]'::jsonb, + true, + NOW(), + NOW() + ); + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER trigger_insert_organization_system_roles + AFTER INSERT ON organizations + FOR EACH ROW + EXECUTE FUNCTION insert_organization_system_roles(); diff --git a/coderd/database/migrations/testdata/fixtures/000438_pre_organization_service_account_role.up.sql b/coderd/database/migrations/testdata/fixtures/000438_pre_organization_service_account_role.up.sql new file mode 100644 index 0000000000..9447573841 --- /dev/null +++ b/coderd/database/migrations/testdata/fixtures/000438_pre_organization_service_account_role.up.sql @@ -0,0 +1,28 @@ +-- Fixture for migration 000443_three_options_for_allowed_workspace_sharing. +-- Inserts a custom role named 'Organization-Service-Account' (mixed case) +-- to ensure the migration's case-insensitive rename catches it. +INSERT INTO custom_roles ( + name, + display_name, + organization_id, + site_permissions, + org_permissions, + user_permissions, + member_permissions, + is_system, + created_at, + updated_at +) +VALUES ( + 'Organization-Service-Account', + 'User-created role', + 'bb640d07-ca8a-4869-b6bc-ae61ebb2fda1', + '[]'::jsonb, + '[]'::jsonb, + '[]'::jsonb, + '[]'::jsonb, + false, + NOW(), + NOW() +) +ON CONFLICT DO NOTHING; diff --git a/coderd/database/models.go b/coderd/database/models.go index 3d2392054b..de88b21440 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -3131,6 +3131,67 @@ func AllResourceTypeValues() []ResourceType { } } +type ShareableWorkspaceOwners string + +const ( + ShareableWorkspaceOwnersNone ShareableWorkspaceOwners = "none" + ShareableWorkspaceOwnersEveryone ShareableWorkspaceOwners = "everyone" + ShareableWorkspaceOwnersServiceAccounts ShareableWorkspaceOwners = "service_accounts" +) + +func (e *ShareableWorkspaceOwners) Scan(src interface{}) error { + switch s := src.(type) { + case []byte: + *e = ShareableWorkspaceOwners(s) + case string: + *e = ShareableWorkspaceOwners(s) + default: + return fmt.Errorf("unsupported scan type for ShareableWorkspaceOwners: %T", src) + } + return nil +} + +type NullShareableWorkspaceOwners struct { + ShareableWorkspaceOwners ShareableWorkspaceOwners `json:"shareable_workspace_owners"` + Valid bool `json:"valid"` // Valid is true if ShareableWorkspaceOwners is not NULL +} + +// Scan implements the Scanner interface. +func (ns *NullShareableWorkspaceOwners) Scan(value interface{}) error { + if value == nil { + ns.ShareableWorkspaceOwners, ns.Valid = "", false + return nil + } + ns.Valid = true + return ns.ShareableWorkspaceOwners.Scan(value) +} + +// Value implements the driver Valuer interface. +func (ns NullShareableWorkspaceOwners) Value() (driver.Value, error) { + if !ns.Valid { + return nil, nil + } + return string(ns.ShareableWorkspaceOwners), nil +} + +func (e ShareableWorkspaceOwners) Valid() bool { + switch e { + case ShareableWorkspaceOwnersNone, + ShareableWorkspaceOwnersEveryone, + ShareableWorkspaceOwnersServiceAccounts: + return true + } + return false +} + +func AllShareableWorkspaceOwnersValues() []ShareableWorkspaceOwners { + return []ShareableWorkspaceOwners{ + ShareableWorkspaceOwnersNone, + ShareableWorkspaceOwnersEveryone, + ShareableWorkspaceOwnersServiceAccounts, + } +} + type StartupScriptBehavior string const ( @@ -4535,16 +4596,17 @@ type OAuth2ProviderAppToken struct { } type Organization struct { - ID uuid.UUID `db:"id" json:"id"` - Name string `db:"name" json:"name"` - Description string `db:"description" json:"description"` - CreatedAt time.Time `db:"created_at" json:"created_at"` - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` - IsDefault bool `db:"is_default" json:"is_default"` - DisplayName string `db:"display_name" json:"display_name"` - Icon string `db:"icon" json:"icon"` - Deleted bool `db:"deleted" json:"deleted"` - WorkspaceSharingDisabled bool `db:"workspace_sharing_disabled" json:"workspace_sharing_disabled"` + ID uuid.UUID `db:"id" json:"id"` + Name string `db:"name" json:"name"` + Description string `db:"description" json:"description"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + IsDefault bool `db:"is_default" json:"is_default"` + DisplayName string `db:"display_name" json:"display_name"` + Icon string `db:"icon" json:"icon"` + Deleted bool `db:"deleted" json:"deleted"` + // Controls whose workspaces can be shared: none, everyone, or service_accounts. + ShareableWorkspaceOwners ShareableWorkspaceOwners `db:"shareable_workspace_owners" json:"shareable_workspace_owners"` } type OrganizationMember struct { diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 92f5e18342..ea59c4775f 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -150,7 +150,7 @@ type sqlcQuerier interface { DeleteWebpushSubscriptionByUserIDAndEndpoint(ctx context.Context, arg DeleteWebpushSubscriptionByUserIDAndEndpointParams) error DeleteWebpushSubscriptions(ctx context.Context, ids []uuid.UUID) error DeleteWorkspaceACLByID(ctx context.Context, id uuid.UUID) error - DeleteWorkspaceACLsByOrganization(ctx context.Context, organizationID uuid.UUID) error + DeleteWorkspaceACLsByOrganization(ctx context.Context, arg DeleteWorkspaceACLsByOrganizationParams) error DeleteWorkspaceAgentPortShare(ctx context.Context, arg DeleteWorkspaceAgentPortShareParams) error DeleteWorkspaceAgentPortSharesByTemplate(ctx context.Context, templateID uuid.UUID) error DeleteWorkspaceSubAgentByID(ctx context.Context, id uuid.UUID) error diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 5588aa01f1..aaaa0f8022 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -2655,6 +2655,42 @@ func TestDeleteCustomRoleDoesNotDeleteSystemRole(t *testing.T) { require.True(t, roles[0].IsSystem) } +func TestGetAuthorizationUserRolesImpliedOrgRole(t *testing.T) { + t.Parallel() + + db, _ := dbtestutil.NewDB(t) + org := dbgen.Organization(t, db, database.Organization{}) + + regularUser := dbgen.User(t, db, database.User{}) + saUser := dbgen.User(t, db, database.User{IsServiceAccount: true}) + + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + OrganizationID: org.ID, + UserID: regularUser.ID, + }) + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + OrganizationID: org.ID, + UserID: saUser.ID, + }) + + ctx := testutil.Context(t, testutil.WaitShort) + + wantMember := rbac.RoleOrgMember() + ":" + org.ID.String() + wantSA := rbac.RoleOrgServiceAccount() + ":" + org.ID.String() + + // Regular users get the implied organization-member role. + regularRoles, err := db.GetAuthorizationUserRoles(ctx, regularUser.ID) + require.NoError(t, err) + require.Contains(t, regularRoles.Roles, wantMember) + require.NotContains(t, regularRoles.Roles, wantSA) + + // Service accounts get the implied organization-service-account role. + saRoles, err := db.GetAuthorizationUserRoles(ctx, saUser.ID) + require.NoError(t, err) + require.Contains(t, saRoles.Roles, wantSA) + require.NotContains(t, saRoles.Roles, wantMember) +} + func TestUpdateOrganizationWorkspaceSharingSettings(t *testing.T) { t.Parallel() @@ -2665,82 +2701,155 @@ func TestUpdateOrganizationWorkspaceSharingSettings(t *testing.T) { updated, err := db.UpdateOrganizationWorkspaceSharingSettings(ctx, database.UpdateOrganizationWorkspaceSharingSettingsParams{ ID: org.ID, - WorkspaceSharingDisabled: true, + ShareableWorkspaceOwners: database.ShareableWorkspaceOwnersNone, UpdatedAt: dbtime.Now(), }) require.NoError(t, err) - require.True(t, updated.WorkspaceSharingDisabled) + require.Equal(t, database.ShareableWorkspaceOwnersNone, updated.ShareableWorkspaceOwners) got, err := db.GetOrganizationByID(ctx, org.ID) require.NoError(t, err) - require.True(t, got.WorkspaceSharingDisabled) + require.Equal(t, database.ShareableWorkspaceOwnersNone, got.ShareableWorkspaceOwners) } func TestDeleteWorkspaceACLsByOrganization(t *testing.T) { t.Parallel() - db, _ := dbtestutil.NewDB(t) - org1 := dbgen.Organization(t, db, database.Organization{}) - org2 := dbgen.Organization(t, db, database.Organization{}) + t.Run("DeletesAll", func(t *testing.T) { + t.Parallel() - owner1 := dbgen.User(t, db, database.User{}) - owner2 := dbgen.User(t, db, database.User{}) - sharedUser := dbgen.User(t, db, database.User{}) - sharedGroup := dbgen.Group(t, db, database.Group{ - OrganizationID: org1.ID, + db, _ := dbtestutil.NewDB(t) + org1 := dbgen.Organization(t, db, database.Organization{}) + org2 := dbgen.Organization(t, db, database.Organization{}) + + owner1 := dbgen.User(t, db, database.User{}) + owner2 := dbgen.User(t, db, database.User{}) + sharedUser := dbgen.User(t, db, database.User{}) + sharedGroup := dbgen.Group(t, db, database.Group{ + OrganizationID: org1.ID, + }) + + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + OrganizationID: org1.ID, + UserID: owner1.ID, + }) + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + OrganizationID: org2.ID, + UserID: owner2.ID, + }) + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + OrganizationID: org1.ID, + UserID: sharedUser.ID, + }) + + ws1 := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: owner1.ID, + OrganizationID: org1.ID, + UserACL: database.WorkspaceACL{ + sharedUser.ID.String(): { + Permissions: []policy.Action{policy.ActionRead}, + }, + }, + GroupACL: database.WorkspaceACL{ + sharedGroup.ID.String(): { + Permissions: []policy.Action{policy.ActionRead}, + }, + }, + }).Do().Workspace + + ws2 := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: owner2.ID, + OrganizationID: org2.ID, + UserACL: database.WorkspaceACL{ + uuid.NewString(): { + Permissions: []policy.Action{policy.ActionRead}, + }, + }, + }).Do().Workspace + + ctx := testutil.Context(t, testutil.WaitShort) + + err := db.DeleteWorkspaceACLsByOrganization(ctx, database.DeleteWorkspaceACLsByOrganizationParams{ + OrganizationID: org1.ID, + ExcludeServiceAccounts: false, + }) + require.NoError(t, err) + + got1, err := db.GetWorkspaceByID(ctx, ws1.ID) + require.NoError(t, err) + require.Empty(t, got1.UserACL) + require.Empty(t, got1.GroupACL) + + got2, err := db.GetWorkspaceByID(ctx, ws2.ID) + require.NoError(t, err) + require.NotEmpty(t, got2.UserACL) }) - dbgen.OrganizationMember(t, db, database.OrganizationMember{ - OrganizationID: org1.ID, - UserID: owner1.ID, - }) - dbgen.OrganizationMember(t, db, database.OrganizationMember{ - OrganizationID: org2.ID, - UserID: owner2.ID, - }) - dbgen.OrganizationMember(t, db, database.OrganizationMember{ - OrganizationID: org1.ID, - UserID: sharedUser.ID, - }) + t.Run("ExcludesServiceAccounts", func(t *testing.T) { + t.Parallel() - ws1 := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ - OwnerID: owner1.ID, - OrganizationID: org1.ID, - UserACL: database.WorkspaceACL{ + db, _ := dbtestutil.NewDB(t) + org := dbgen.Organization(t, db, database.Organization{}) + + regularUser := dbgen.User(t, db, database.User{}) + saUser := dbgen.User(t, db, database.User{IsServiceAccount: true}) + sharedUser := dbgen.User(t, db, database.User{}) + + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + OrganizationID: org.ID, + UserID: regularUser.ID, + }) + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + OrganizationID: org.ID, + UserID: saUser.ID, + }) + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + OrganizationID: org.ID, + UserID: sharedUser.ID, + }) + + regularWS := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: regularUser.ID, + OrganizationID: org.ID, + UserACL: database.WorkspaceACL{ + sharedUser.ID.String(): { + Permissions: []policy.Action{policy.ActionRead}, + }, + }, + }).Do().Workspace + + saWS := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: saUser.ID, + OrganizationID: org.ID, + UserACL: database.WorkspaceACL{ + sharedUser.ID.String(): { + Permissions: []policy.Action{policy.ActionRead}, + }, + }, + }).Do().Workspace + + ctx := testutil.Context(t, testutil.WaitShort) + + err := db.DeleteWorkspaceACLsByOrganization(ctx, database.DeleteWorkspaceACLsByOrganizationParams{ + OrganizationID: org.ID, + ExcludeServiceAccounts: true, + }) + require.NoError(t, err) + + // Regular user workspace ACLs should be cleared. + gotRegular, err := db.GetWorkspaceByID(ctx, regularWS.ID) + require.NoError(t, err) + require.Empty(t, gotRegular.UserACL) + + // Service account workspace ACLs should be preserved. + gotSA, err := db.GetWorkspaceByID(ctx, saWS.ID) + require.NoError(t, err) + require.Equal(t, database.WorkspaceACL{ sharedUser.ID.String(): { Permissions: []policy.Action{policy.ActionRead}, }, - }, - GroupACL: database.WorkspaceACL{ - sharedGroup.ID.String(): { - Permissions: []policy.Action{policy.ActionRead}, - }, - }, - }).Do().Workspace - - ws2 := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ - OwnerID: owner2.ID, - OrganizationID: org2.ID, - UserACL: database.WorkspaceACL{ - uuid.NewString(): { - Permissions: []policy.Action{policy.ActionRead}, - }, - }, - }).Do().Workspace - - ctx := testutil.Context(t, testutil.WaitShort) - - err := db.DeleteWorkspaceACLsByOrganization(ctx, org1.ID) - require.NoError(t, err) - - got1, err := db.GetWorkspaceByID(ctx, ws1.ID) - require.NoError(t, err) - require.Empty(t, got1.UserACL) - require.Empty(t, got1.GroupACL) - - got2, err := db.GetWorkspaceByID(ctx, ws2.ID) - require.NoError(t, err) - require.NotEmpty(t, got2.UserACL) + }, gotSA.UserACL) + }) } func TestAuthorizedAuditLogs(t *testing.T) { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index ef125ac269..ad1eb7728d 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -11035,7 +11035,7 @@ func (q *sqlQuerier) UpdateMemberRoles(ctx context.Context, arg UpdateMemberRole const getDefaultOrganization = `-- name: GetDefaultOrganization :one SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, workspace_sharing_disabled + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, shareable_workspace_owners FROM organizations WHERE @@ -11057,14 +11057,14 @@ func (q *sqlQuerier) GetDefaultOrganization(ctx context.Context) (Organization, &i.DisplayName, &i.Icon, &i.Deleted, - &i.WorkspaceSharingDisabled, + &i.ShareableWorkspaceOwners, ) return i, err } const getOrganizationByID = `-- name: GetOrganizationByID :one SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, workspace_sharing_disabled + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, shareable_workspace_owners FROM organizations WHERE @@ -11084,14 +11084,14 @@ func (q *sqlQuerier) GetOrganizationByID(ctx context.Context, id uuid.UUID) (Org &i.DisplayName, &i.Icon, &i.Deleted, - &i.WorkspaceSharingDisabled, + &i.ShareableWorkspaceOwners, ) return i, err } const getOrganizationByName = `-- name: GetOrganizationByName :one SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, workspace_sharing_disabled + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, shareable_workspace_owners FROM organizations WHERE @@ -11120,7 +11120,7 @@ func (q *sqlQuerier) GetOrganizationByName(ctx context.Context, arg GetOrganizat &i.DisplayName, &i.Icon, &i.Deleted, - &i.WorkspaceSharingDisabled, + &i.ShareableWorkspaceOwners, ) return i, err } @@ -11191,7 +11191,7 @@ func (q *sqlQuerier) GetOrganizationResourceCountByID(ctx context.Context, organ const getOrganizations = `-- name: GetOrganizations :many SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, workspace_sharing_disabled + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, shareable_workspace_owners FROM organizations WHERE @@ -11235,7 +11235,7 @@ func (q *sqlQuerier) GetOrganizations(ctx context.Context, arg GetOrganizationsP &i.DisplayName, &i.Icon, &i.Deleted, - &i.WorkspaceSharingDisabled, + &i.ShareableWorkspaceOwners, ); err != nil { return nil, err } @@ -11252,7 +11252,7 @@ func (q *sqlQuerier) GetOrganizations(ctx context.Context, arg GetOrganizationsP const getOrganizationsByUserID = `-- name: GetOrganizationsByUserID :many SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, workspace_sharing_disabled + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, shareable_workspace_owners FROM organizations WHERE @@ -11297,7 +11297,7 @@ func (q *sqlQuerier) GetOrganizationsByUserID(ctx context.Context, arg GetOrgani &i.DisplayName, &i.Icon, &i.Deleted, - &i.WorkspaceSharingDisabled, + &i.ShareableWorkspaceOwners, ); err != nil { return nil, err } @@ -11317,7 +11317,7 @@ INSERT INTO organizations (id, "name", display_name, description, icon, created_at, updated_at, is_default) VALUES -- If no organizations exist, and this is the first, make it the default. - ($1, $2, $3, $4, $5, $6, $7, (SELECT TRUE FROM organizations LIMIT 1) IS NULL) RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, workspace_sharing_disabled + ($1, $2, $3, $4, $5, $6, $7, (SELECT TRUE FROM organizations LIMIT 1) IS NULL) RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, shareable_workspace_owners ` type InsertOrganizationParams struct { @@ -11351,7 +11351,7 @@ func (q *sqlQuerier) InsertOrganization(ctx context.Context, arg InsertOrganizat &i.DisplayName, &i.Icon, &i.Deleted, - &i.WorkspaceSharingDisabled, + &i.ShareableWorkspaceOwners, ) return i, err } @@ -11367,7 +11367,7 @@ SET icon = $5 WHERE id = $6 -RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, workspace_sharing_disabled +RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, shareable_workspace_owners ` type UpdateOrganizationParams struct { @@ -11399,7 +11399,7 @@ func (q *sqlQuerier) UpdateOrganization(ctx context.Context, arg UpdateOrganizat &i.DisplayName, &i.Icon, &i.Deleted, - &i.WorkspaceSharingDisabled, + &i.ShareableWorkspaceOwners, ) return i, err } @@ -11428,21 +11428,21 @@ const updateOrganizationWorkspaceSharingSettings = `-- name: UpdateOrganizationW UPDATE organizations SET - workspace_sharing_disabled = $1, + shareable_workspace_owners = $1, updated_at = $2 WHERE id = $3 -RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, workspace_sharing_disabled +RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, shareable_workspace_owners ` type UpdateOrganizationWorkspaceSharingSettingsParams struct { - WorkspaceSharingDisabled bool `db:"workspace_sharing_disabled" json:"workspace_sharing_disabled"` - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` - ID uuid.UUID `db:"id" json:"id"` + ShareableWorkspaceOwners ShareableWorkspaceOwners `db:"shareable_workspace_owners" json:"shareable_workspace_owners"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + ID uuid.UUID `db:"id" json:"id"` } func (q *sqlQuerier) UpdateOrganizationWorkspaceSharingSettings(ctx context.Context, arg UpdateOrganizationWorkspaceSharingSettingsParams) (Organization, error) { - row := q.db.QueryRowContext(ctx, updateOrganizationWorkspaceSharingSettings, arg.WorkspaceSharingDisabled, arg.UpdatedAt, arg.ID) + row := q.db.QueryRowContext(ctx, updateOrganizationWorkspaceSharingSettings, arg.ShareableWorkspaceOwners, arg.UpdatedAt, arg.ID) var i Organization err := row.Scan( &i.ID, @@ -11454,7 +11454,7 @@ func (q *sqlQuerier) UpdateOrganizationWorkspaceSharingSettings(ctx context.Cont &i.DisplayName, &i.Icon, &i.Deleted, - &i.WorkspaceSharingDisabled, + &i.ShareableWorkspaceOwners, ) return i, err } @@ -19373,9 +19373,21 @@ SELECT array_agg(org_roles || ':' || organization_members.organization_id::text) FROM organization_members, - -- All org_members get the organization-member role for their orgs + -- All org members get an implied role for their orgs. Most members + -- get organization-member, but service accounts will get + -- organization-service-account instead. They're largely the same, + -- but having them be distinct means we can allow configuring + -- service-accounts to have slightly broader permissions–such as + -- for workspace sharing. unnest( - array_append(roles, 'organization-member') + array_append( + roles, + CASE WHEN users.is_service_account THEN + 'organization-service-account' + ELSE + 'organization-member' + END + ) ) AS org_roles WHERE user_id = users.id @@ -25466,10 +25478,21 @@ SET user_acl = '{}'::jsonb WHERE organization_id = $1 + AND ( + NOT $2::boolean + OR owner_id NOT IN ( + SELECT id FROM users WHERE is_service_account = true + ) + ) ` -func (q *sqlQuerier) DeleteWorkspaceACLsByOrganization(ctx context.Context, organizationID uuid.UUID) error { - _, err := q.db.ExecContext(ctx, deleteWorkspaceACLsByOrganization, organizationID) +type DeleteWorkspaceACLsByOrganizationParams struct { + OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + ExcludeServiceAccounts bool `db:"exclude_service_accounts" json:"exclude_service_accounts"` +} + +func (q *sqlQuerier) DeleteWorkspaceACLsByOrganization(ctx context.Context, arg DeleteWorkspaceACLsByOrganizationParams) error { + _, err := q.db.ExecContext(ctx, deleteWorkspaceACLsByOrganization, arg.OrganizationID, arg.ExcludeServiceAccounts) return err } diff --git a/coderd/database/queries/organizations.sql b/coderd/database/queries/organizations.sql index c0e0de92d6..8f27330e9e 100644 --- a/coderd/database/queries/organizations.sql +++ b/coderd/database/queries/organizations.sql @@ -147,7 +147,7 @@ WHERE UPDATE organizations SET - workspace_sharing_disabled = @workspace_sharing_disabled, + shareable_workspace_owners = @shareable_workspace_owners, updated_at = @updated_at WHERE id = @id diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index b392385ab2..24a2271ca6 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -391,9 +391,21 @@ SELECT array_agg(org_roles || ':' || organization_members.organization_id::text) FROM organization_members, - -- All org_members get the organization-member role for their orgs + -- All org members get an implied role for their orgs. Most members + -- get organization-member, but service accounts will get + -- organization-service-account instead. They're largely the same, + -- but having them be distinct means we can allow configuring + -- service-accounts to have slightly broader permissions–such as + -- for workspace sharing. unnest( - array_append(roles, 'organization-member') + array_append( + roles, + CASE WHEN users.is_service_account THEN + 'organization-service-account' + ELSE + 'organization-member' + END + ) ) AS org_roles WHERE user_id = users.id diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index c2aa9b6742..5269ea8fba 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -955,7 +955,13 @@ SET group_acl = '{}'::jsonb, user_acl = '{}'::jsonb WHERE - organization_id = @organization_id; + organization_id = @organization_id + AND ( + NOT @exclude_service_accounts::boolean + OR owner_id NOT IN ( + SELECT id FROM users WHERE is_service_account = true + ) + ); -- name: GetRegularWorkspaceCreateMetrics :many -- Count regular workspaces: only those whose first successful 'start' build diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index 853fed8359..3d93306017 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -1404,8 +1404,8 @@ func testAuthorize(t *testing.T, name string, subject Subject, sets ...[]authTes // RoleByName won't resolve it here. Assume the default behavior: workspace // sharing enabled. func orgMemberRole(orgID uuid.UUID) Role { - workspaceSharingDisabled := false - orgPerms, memberPerms := OrgMemberPermissions(workspaceSharingDisabled) + settings := OrgSettings{ShareableWorkspaceOwners: ShareableWorkspaceOwnersEveryone} + perms := OrgMemberPermissions(settings) return Role{ Identifier: ScopedRoleOrgMember(orgID), DisplayName: "", @@ -1413,8 +1413,8 @@ func orgMemberRole(orgID uuid.UUID) Role { User: []Permission{}, ByOrgID: map[string]OrgPermissions{ orgID.String(): { - Org: orgPerms, - Member: memberPerms, + Org: perms.Org, + Member: perms.Member, }, }, } diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 1789b57786..03285bd6db 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -29,6 +29,7 @@ const ( orgAdmin string = "organization-admin" orgMember string = "organization-member" + orgServiceAccount string = "organization-service-account" orgAuditor string = "organization-auditor" orgUserAdmin string = "organization-user-admin" orgTemplateAdmin string = "organization-template-admin" @@ -150,6 +151,10 @@ func RoleOrgMember() string { return orgMember } +func RoleOrgServiceAccount() string { + return orgServiceAccount +} + func RoleOrgAuditor() string { return orgAuditor } @@ -229,31 +234,16 @@ func allPermsExcept(excepts ...Objecter) []Permission { // https://github.com/coder/coder/issues/1194 var builtInRoles map[string]func(orgID uuid.UUID) Role -// systemRoles are roles that have migrated from builtInRoles to -// database storage. This migration is partial - permissions are still -// generated at runtime and reconciled to the database, rather than -// the database being the source of truth. -var systemRoles = map[string]struct{}{ - RoleOrgMember(): {}, -} - -func SystemRoleName(name string) bool { - _, ok := systemRoles[name] - return ok -} - type RoleOptions struct { NoOwnerWorkspaceExec bool NoWorkspaceSharing bool } // ReservedRoleName exists because the database should only allow unique role -// names, but some roles are built in or generated at runtime. So these names -// are reserved +// names, but some roles are built in. So these names are reserved func ReservedRoleName(name string) bool { - _, isBuiltIn := builtInRoles[name] - _, isSystem := systemRoles[name] - return isBuiltIn || isSystem + _, ok := builtInRoles[name] + return ok } // ReloadBuiltinRoles loads the static roles into the builtInRoles map. @@ -938,21 +928,32 @@ func PermissionsEqual(a, b []Permission) bool { return len(setA) == len(setB) } +// OrgSettings carries organization-level settings that affect system +// role permissions. It lives in the rbac package to avoid a cyclic +// dependency with the database package. Callers in rolestore map +// database.Organization fields onto this struct. +type OrgSettings struct { + ShareableWorkspaceOwners ShareableWorkspaceOwners +} +type ShareableWorkspaceOwners string + +const ( + ShareableWorkspaceOwnersNone ShareableWorkspaceOwners = "none" + ShareableWorkspaceOwnersEveryone ShareableWorkspaceOwners = "everyone" + ShareableWorkspaceOwnersServiceAccounts ShareableWorkspaceOwners = "service_accounts" +) + +// OrgRolePermissions holds the two permission sets that make up a +// system role: org-wide permissions and member-scoped permissions. +type OrgRolePermissions struct { + Org []Permission + Member []Permission +} + // OrgMemberPermissions returns the permissions for the organization-member -// system role. The results are then stored in the database and can vary per -// organization based on the workspace_sharing_disabled setting. -// This is the source of truth for org-member permissions, used by: -// - the startup reconciliation routine, to keep permissions current with -// RBAC resources -// - the organization workspace sharing setting endpoint, when updating -// the setting -// - the org creation endpoint, when populating the organization-member -// system role created by the DB trigger -// -//nolint:revive // workspaceSharingDisabled is an org setting -func OrgMemberPermissions(workspaceSharingDisabled bool) ( - orgPerms, memberPerms []Permission, -) { +// system role, which can vary based on the organization's workspace sharing +// settings. +func OrgMemberPermissions(org OrgSettings) OrgRolePermissions { // Organization-level permissions that all org members get. orgPermMap := map[string][]policy.Action{ // All users can see provisioner daemons for workspace creation. @@ -963,18 +964,35 @@ func OrgMemberPermissions(workspaceSharingDisabled bool) ( ResourceAssignOrgRole.Type: {policy.ActionRead}, } - // When workspace sharing is enabled, members need to see other org members - // and groups to share workspaces with them. - if !workspaceSharingDisabled { + // In all modes of workspace sharing but `none`, members need to + // see other org members (including service accounts) to either + // share with them or get access to their shared workspaces, + // resolved through GET /users/{user}/workspace/{workspace} + if org.ShareableWorkspaceOwners != ShareableWorkspaceOwnersNone { orgPermMap[ResourceOrganizationMember.Type] = []policy.Action{policy.ActionRead} + } + + // When workspace sharing is open to members, they also need to + // see org groups to share with them. + if org.ShareableWorkspaceOwners == ShareableWorkspaceOwnersEveryone { orgPermMap[ResourceGroup.Type] = []policy.Action{policy.ActionRead} } - orgPerms = Permissions(orgPermMap) + orgPerms := Permissions(orgPermMap) + + if org.ShareableWorkspaceOwners == ShareableWorkspaceOwnersNone { + // Org-level negation blocks sharing on ANY workspace in the + // org. This overrides any positive permission from other + // roles, including org-admin. + orgPerms = append(orgPerms, Permission{ + Negate: true, + ResourceType: ResourceWorkspace.Type, + Action: policy.ActionShare, + }) + } - // Member-scoped permissions (resources owned by the member). // Uses allPermsExcept to automatically include permissions for new resources. - memberPerms = append( + memberPerms := append( allPermsExcept( ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, @@ -998,24 +1016,47 @@ func OrgMemberPermissions(workspaceSharingDisabled bool) ( ResourceOrganizationMember.Type: { policy.ActionRead, }, - // Users can create provisioner daemons scoped to themselves. - // - // TODO(geokat): copied from the original built-in role - // verbatim, but seems to be a no-op (not excepted above; - // plus no owner is set for the ProvisionerDaemon RBAC - // object). - ResourceProvisionerDaemon.Type: { - policy.ActionRead, - policy.ActionCreate, - policy.ActionUpdate, - }, })..., ) - if workspaceSharingDisabled { + if org.ShareableWorkspaceOwners != ShareableWorkspaceOwnersEveryone { + memberPerms = append(memberPerms, Permission{ + Negate: true, + ResourceType: ResourceWorkspace.Type, + Action: policy.ActionShare, + }) + } + + return OrgRolePermissions{Org: orgPerms, Member: memberPerms} +} + +// OrgServiceAccountPermissions returns the permissions for the +// organization-service-account system role, which can vary based on +// the organization's workspace sharing settings. +func OrgServiceAccountPermissions(org OrgSettings) OrgRolePermissions { + // Organization-level permissions that all org service accounts get. + orgPermMap := map[string][]policy.Action{ + // All users can see provisioner daemons for workspace creation. + ResourceProvisionerDaemon.Type: {policy.ActionRead}, + // All org members can read the organization. + ResourceOrganization.Type: {policy.ActionRead}, + // Can read available roles. + ResourceAssignOrgRole.Type: {policy.ActionRead}, + } + + // When workspace sharing is enabled, service accounts need to see + // other org members and groups to share workspaces with them. + if org.ShareableWorkspaceOwners != ShareableWorkspaceOwnersNone { + orgPermMap[ResourceOrganizationMember.Type] = []policy.Action{policy.ActionRead} + orgPermMap[ResourceGroup.Type] = []policy.Action{policy.ActionRead} + } + + orgPerms := Permissions(orgPermMap) + + if org.ShareableWorkspaceOwners == ShareableWorkspaceOwnersNone { // Org-level negation blocks sharing on ANY workspace in the - // org. This overrides any positive permission from other - // roles, including org-admin. + // org. If a service account has any other roles assigned, + // this negation will override any positive perms in them, too. orgPerms = append(orgPerms, Permission{ Negate: true, ResourceType: ResourceWorkspace.Type, @@ -1023,5 +1064,35 @@ func OrgMemberPermissions(workspaceSharingDisabled bool) ( }) } - return orgPerms, memberPerms + // service account-scoped permissions (resources owned by the + // service account). Uses allPermsExcept to automatically include + // permissions for new resources. + memberPerms := append( + allPermsExcept( + ResourceWorkspaceDormant, + ResourcePrebuiltWorkspace, + ResourceUser, + ResourceOrganizationMember, + ), + Permissions(map[string][]policy.Action{ + // Reduced permission set on dormant workspaces. No build, + // ssh, or exec. + ResourceWorkspaceDormant.Type: { + policy.ActionRead, + policy.ActionDelete, + policy.ActionCreate, + policy.ActionUpdate, + policy.ActionWorkspaceStop, + policy.ActionCreateAgent, + policy.ActionDeleteAgent, + policy.ActionUpdateAgent, + }, + // Can read their own organization member record. + ResourceOrganizationMember.Type: { + policy.ActionRead, + }, + })..., + ) + + return OrgRolePermissions{Org: orgPerms, Member: memberPerms} } diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index 76c87618be..16b14057e4 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -51,54 +51,68 @@ func TestBuiltInRoles(t *testing.T) { } } -func TestSystemRolesAreReservedRoleNames(t *testing.T) { - t.Parallel() - - require.True(t, rbac.ReservedRoleName(rbac.RoleOrgMember())) +// permissionGranted checks whether a permission list contains a +// matching entry for the target, accounting for wildcard actions. +// It does not evaluate negations that may override a positive grant. +func permissionGranted(perms []rbac.Permission, target rbac.Permission) bool { + return slices.ContainsFunc(perms, func(p rbac.Permission) bool { + return p.Negate == target.Negate && + p.ResourceType == target.ResourceType && + (p.Action == target.Action || p.Action == policy.WildcardSymbol) + }) } -func TestOrgMemberPermissions(t *testing.T) { +func TestOrgSharingPermissions(t *testing.T) { t.Parallel() - t.Run("WorkspaceSharingEnabled", func(t *testing.T) { - t.Parallel() + tests := []struct { + name string + permsFunc func(rbac.OrgSettings) rbac.OrgRolePermissions + mode rbac.ShareableWorkspaceOwners + orgReadMembers bool + orgReadGroups bool + orgNegateShare bool + memberNegateShare bool + }{ + {"Member/Everyone", rbac.OrgMemberPermissions, rbac.ShareableWorkspaceOwnersEveryone, true, true, false, false}, + {"Member/None", rbac.OrgMemberPermissions, rbac.ShareableWorkspaceOwnersNone, false, false, true, true}, + {"Member/ServiceAccounts", rbac.OrgMemberPermissions, rbac.ShareableWorkspaceOwnersServiceAccounts, true, false, false, true}, + {"ServiceAccount/Everyone", rbac.OrgServiceAccountPermissions, rbac.ShareableWorkspaceOwnersEveryone, true, true, false, false}, + {"ServiceAccount/None", rbac.OrgServiceAccountPermissions, rbac.ShareableWorkspaceOwnersNone, false, false, true, false}, + {"ServiceAccount/ServiceAccounts", rbac.OrgServiceAccountPermissions, rbac.ShareableWorkspaceOwnersServiceAccounts, true, true, false, false}, + } - orgPerms, _ := rbac.OrgMemberPermissions(false) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() - require.True(t, slices.Contains(orgPerms, rbac.Permission{ - ResourceType: rbac.ResourceOrganizationMember.Type, - Action: policy.ActionRead, - })) - require.True(t, slices.Contains(orgPerms, rbac.Permission{ - ResourceType: rbac.ResourceGroup.Type, - Action: policy.ActionRead, - })) - require.False(t, slices.Contains(orgPerms, rbac.Permission{ - Negate: true, - ResourceType: rbac.ResourceWorkspace.Type, - Action: policy.ActionShare, - })) - }) + perms := tt.permsFunc(rbac.OrgSettings{ + ShareableWorkspaceOwners: tt.mode, + }) - t.Run("WorkspaceSharingDisabled", func(t *testing.T) { - t.Parallel() + assert.Equal(t, tt.orgReadMembers, permissionGranted(perms.Org, rbac.Permission{ + ResourceType: rbac.ResourceOrganizationMember.Type, + Action: policy.ActionRead, + }), "org read members") - orgPerms, _ := rbac.OrgMemberPermissions(true) + assert.Equal(t, tt.orgReadGroups, permissionGranted(perms.Org, rbac.Permission{ + ResourceType: rbac.ResourceGroup.Type, + Action: policy.ActionRead, + }), "org read groups") - require.False(t, slices.Contains(orgPerms, rbac.Permission{ - ResourceType: rbac.ResourceOrganizationMember.Type, - Action: policy.ActionRead, - })) - require.False(t, slices.Contains(orgPerms, rbac.Permission{ - ResourceType: rbac.ResourceGroup.Type, - Action: policy.ActionRead, - })) - require.True(t, slices.Contains(orgPerms, rbac.Permission{ - Negate: true, - ResourceType: rbac.ResourceWorkspace.Type, - Action: policy.ActionShare, - })) - }) + assert.Equal(t, tt.orgNegateShare, permissionGranted(perms.Org, rbac.Permission{ + Negate: true, + ResourceType: rbac.ResourceWorkspace.Type, + Action: policy.ActionShare, + }), "org negate share") + + assert.Equal(t, tt.memberNegateShare, permissionGranted(perms.Member, rbac.Permission{ + Negate: true, + ResourceType: rbac.ResourceWorkspace.Type, + Action: policy.ActionShare, + }), "member negate share") + }) + } } //nolint:tparallel,paralleltest diff --git a/coderd/rbac/rolestore/rolestore.go b/coderd/rbac/rolestore/rolestore.go index aef7992f16..c246778995 100644 --- a/coderd/rbac/rolestore/rolestore.go +++ b/coderd/rbac/rolestore/rolestore.go @@ -2,6 +2,7 @@ package rolestore import ( "context" + "maps" "net/http" "github.com/google/uuid" @@ -161,13 +162,28 @@ func ConvertDBRole(dbRole database.CustomRole) (rbac.Role, error) { return role, nil } -// ReconcileSystemRoles ensures that every organization's org-member -// system role in the DB is up-to-date with permissions reflecting -// current RBAC resources and the organization's -// workspace_sharing_disabled setting. Uses PostgreSQL advisory lock -// (LockIDReconcileSystemRoles) to safely handle multi-instance -// deployments. Uses set-based comparison to avoid unnecessary -// database writes when permissions haven't changed. +// System roles are defined in code but stored in the database, +// allowing their permissions to be adjusted per-organization at +// runtime based on org settings (e.g. workspace sharing). +var systemRoles = map[string]permissionsFunc{ + rbac.RoleOrgMember(): rbac.OrgMemberPermissions, + rbac.RoleOrgServiceAccount(): rbac.OrgServiceAccountPermissions, +} + +// permissionsFunc produces the desired permissions for a system role +// given organization settings. +type permissionsFunc func(rbac.OrgSettings) rbac.OrgRolePermissions + +func IsSystemRoleName(name string) bool { + _, ok := systemRoles[name] + return ok +} + +var SystemRoleNames = maps.Keys(systemRoles) + +// ReconcileSystemRoles ensures that every organization's system roles +// in the DB are up-to-date with the current RBAC definitions and +// organization settings. func ReconcileSystemRoles(ctx context.Context, log slog.Logger, db database.Store) error { return db.InTx(func(tx database.Store) error { // Acquire advisory lock to prevent concurrent updates from @@ -193,36 +209,45 @@ func ReconcileSystemRoles(ctx context.Context, log slog.Logger, db database.Stor return xerrors.Errorf("fetch custom roles: %w", err) } - // Find org-member roles and index by organization ID for quick lookup. - rolesByOrg := make(map[uuid.UUID]database.CustomRole) + // Index system roles by (org ID, role name) for quick lookup. + type orgRoleKey struct { + OrgID uuid.UUID + RoleName string + } + roleIndex := make(map[orgRoleKey]database.CustomRole) for _, role := range customRoles { - if role.IsSystem && role.Name == rbac.RoleOrgMember() && role.OrganizationID.Valid { - rolesByOrg[role.OrganizationID.UUID] = role + if role.IsSystem && IsSystemRoleName(role.Name) && role.OrganizationID.Valid { + roleIndex[orgRoleKey{role.OrganizationID.UUID, role.Name}] = role } } for _, org := range orgs { - role, exists := rolesByOrg[org.ID] - if !exists { - // Something is very wrong: the role should have been created by the - // database trigger or migration. Log loudly and try creating it as - // a last-ditch effort before giving up. - log.Critical(ctx, "missing organization-member system role; trying to re-create", - slog.F("organization_id", org.ID)) + for roleName := range systemRoles { + role, exists := roleIndex[orgRoleKey{org.ID, roleName}] + if !exists { + // Something is very wrong: the role should have been + // created by the db trigger or migration. Log loudly and + // try creating it as a last-ditch effort before giving up. + log.Critical(ctx, "missing system role; trying to re-create", + slog.F("organization_id", org.ID), + slog.F("role_name", roleName)) - if err := CreateOrgMemberRole(ctx, tx, org); err != nil { - return xerrors.Errorf("create missing organization-member role for organization %s: %w", - org.ID, err) + err := CreateSystemRole(ctx, tx, org, roleName) + if err != nil { + return xerrors.Errorf("create missing %s system role for organization %s: %w", + roleName, org.ID, err) + } + + // Nothing more to do; the new role's permissions are + // up-to-date. + continue } - // Nothing more to do; the new role's permissions are up-to-date. - continue - } - - _, _, err := ReconcileOrgMemberRole(ctx, tx, role, org.WorkspaceSharingDisabled) - if err != nil { - return xerrors.Errorf("reconcile organization-member role for organization %s: %w", - org.ID, err) + _, _, err := ReconcileSystemRole(ctx, tx, role, org) + if err != nil { + return xerrors.Errorf("reconcile %s system role for organization %s: %w", + roleName, org.ID, err) + } } } @@ -230,28 +255,30 @@ func ReconcileSystemRoles(ctx context.Context, log slog.Logger, db database.Stor }, nil) } -// ReconcileOrgMemberRole ensures passed-in org-member role's perms -// are correct (current) and stored in the DB. Uses set-based -// comparison to avoid unnecessary database writes when permissions -// haven't changed. Returns the correct role and a boolean indicating -// whether the reconciliation was necessary. -// NOTE: Callers must acquire `database.LockIDReconcileSystemRoles` at -// the start of the transaction and hold it for the transaction’s -// duration. This prevents concurrent org-member reconciliation from -// racing and producing inconsistent writes. -func ReconcileOrgMemberRole( +// ReconcileSystemRole compares the given role's permissions against +// the desired permissions produced by the permissions function based +// on the organization's settings. If they differ, the DB row is +// updated. Uses set-based comparison so permission ordering doesn't +// matter. Returns the correct role and a boolean indicating whether +// the reconciliation was necessary. +// +// IMPORTANT: Callers must hold database.LockIDReconcileSystemRoles +// for the duration of the enclosing transaction. +func ReconcileSystemRole( ctx context.Context, tx database.Store, in database.CustomRole, - workspaceSharingDisabled bool, -) ( - database.CustomRole, bool, error, -) { + org database.Organization, +) (database.CustomRole, bool, error) { + permsFunc, ok := systemRoles[in.Name] + if !ok { + panic("dev error: no permissions function exists for role " + in.Name) + } + // All fields except OrgPermissions and MemberPermissions will be the same. out := in // Paranoia check: we don't use these in custom roles yet. - // TODO(geokat): Have these as check constraints in DB for now? out.SitePermissions = database.CustomRolePermissions{} out.UserPermissions = database.CustomRolePermissions{} out.DisplayName = "" @@ -259,15 +286,14 @@ func ReconcileOrgMemberRole( inOrgPerms := ConvertDBPermissions(in.OrgPermissions) inMemberPerms := ConvertDBPermissions(in.MemberPermissions) - outOrgPerms, outMemberPerms := rbac.OrgMemberPermissions(workspaceSharingDisabled) + outPerms := permsFunc(orgSettings(org)) - // Compare using set-based comparison (order doesn't matter). - match := rbac.PermissionsEqual(inOrgPerms, outOrgPerms) && - rbac.PermissionsEqual(inMemberPerms, outMemberPerms) + match := rbac.PermissionsEqual(inOrgPerms, outPerms.Org) && + rbac.PermissionsEqual(inMemberPerms, outPerms.Member) if !match { - out.OrgPermissions = ConvertPermissionsToDB(outOrgPerms) - out.MemberPermissions = ConvertPermissionsToDB(outMemberPerms) + out.OrgPermissions = ConvertPermissionsToDB(outPerms.Org) + out.MemberPermissions = ConvertPermissionsToDB(outPerms.Member) _, err := tx.UpdateCustomRole(ctx, database.UpdateCustomRoleParams{ Name: out.Name, @@ -279,30 +305,50 @@ func ReconcileOrgMemberRole( MemberPermissions: out.MemberPermissions, }) if err != nil { - return out, !match, xerrors.Errorf("update organization-member custom role for organization %s: %w", - in.OrganizationID.UUID, err) + return out, !match, xerrors.Errorf("update %s system role for organization %s: %w", + in.Name, in.OrganizationID.UUID, err) } } return out, !match, nil } -// CreateOrgMemberRole creates an org-member system role for an organization. -func CreateOrgMemberRole(ctx context.Context, tx database.Store, org database.Organization) error { - orgPerms, memberPerms := rbac.OrgMemberPermissions(org.WorkspaceSharingDisabled) +// orgSettings maps database.Organization fields to the +// rbac.OrgSettings struct, bridging the database and rbac packages +// without introducing a circular dependency. +func orgSettings(org database.Organization) rbac.OrgSettings { + return rbac.OrgSettings{ + ShareableWorkspaceOwners: rbac.ShareableWorkspaceOwners(org.ShareableWorkspaceOwners), + } +} + +// CreateSystemRole inserts a new system role into the database with +// permissions produced by permsFunc based on the organization's current +// settings. +func CreateSystemRole( + ctx context.Context, + tx database.Store, + org database.Organization, + roleName string, +) error { + permsFunc, ok := systemRoles[roleName] + if !ok { + panic("dev error: no permissions function exists for role " + roleName) + } + perms := permsFunc(orgSettings(org)) _, err := tx.InsertCustomRole(ctx, database.InsertCustomRoleParams{ - Name: rbac.RoleOrgMember(), + Name: roleName, DisplayName: "", OrganizationID: uuid.NullUUID{UUID: org.ID, Valid: true}, SitePermissions: database.CustomRolePermissions{}, - OrgPermissions: ConvertPermissionsToDB(orgPerms), + OrgPermissions: ConvertPermissionsToDB(perms.Org), UserPermissions: database.CustomRolePermissions{}, - MemberPermissions: ConvertPermissionsToDB(memberPerms), + MemberPermissions: ConvertPermissionsToDB(perms.Member), IsSystem: true, }) if err != nil { - return xerrors.Errorf("insert org-member role: %w", err) + return xerrors.Errorf("insert %s role: %w", roleName, err) } return nil diff --git a/coderd/rbac/rolestore/rolestore_test.go b/coderd/rbac/rolestore/rolestore_test.go index 175db71c77..80b6fb40f4 100644 --- a/coderd/rbac/rolestore/rolestore_test.go +++ b/coderd/rbac/rolestore/rolestore_test.go @@ -42,68 +42,84 @@ func TestExpandCustomRoleRoles(t *testing.T) { require.Len(t, roles, 1, "role found") } -func TestReconcileOrgMemberRole(t *testing.T) { +func TestReconcileSystemRole(t *testing.T) { t.Parallel() - db, _ := dbtestutil.NewDB(t) + tests := []struct { + name string + roleName string + permsFunc func(rbac.OrgSettings) rbac.OrgRolePermissions + }{ + {"OrgMember", rbac.RoleOrgMember(), rbac.OrgMemberPermissions}, + {"ServiceAccount", rbac.RoleOrgServiceAccount(), rbac.OrgServiceAccountPermissions}, + } - org := dbgen.Organization(t, db, database.Organization{}) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() - ctx := testutil.Context(t, testutil.WaitShort) + db, _ := dbtestutil.NewDB(t) + org := dbgen.Organization(t, db, database.Organization{}) + ctx := testutil.Context(t, testutil.WaitShort) - existing, err := database.ExpectOne(db.CustomRoles(ctx, database.CustomRolesParams{ - LookupRoles: []database.NameOrganizationPair{ - { - Name: rbac.RoleOrgMember(), - OrganizationID: org.ID, - }, - }, - IncludeSystemRoles: true, - })) - require.NoError(t, err) + existing, err := database.ExpectOne(db.CustomRoles(ctx, database.CustomRolesParams{ + LookupRoles: []database.NameOrganizationPair{ + { + Name: tt.roleName, + OrganizationID: org.ID, + }, + }, + IncludeSystemRoles: true, + })) + require.NoError(t, err) - _, err = db.UpdateCustomRole(ctx, database.UpdateCustomRoleParams{ - Name: existing.Name, - OrganizationID: uuid.NullUUID{ - UUID: org.ID, - Valid: true, - }, - DisplayName: "", - SitePermissions: database.CustomRolePermissions{}, - UserPermissions: database.CustomRolePermissions{}, - OrgPermissions: database.CustomRolePermissions{}, - MemberPermissions: database.CustomRolePermissions{}, - }) - require.NoError(t, err) + // Zero out permissions to simulate stale state. + _, err = db.UpdateCustomRole(ctx, database.UpdateCustomRoleParams{ + Name: existing.Name, + OrganizationID: uuid.NullUUID{ + UUID: org.ID, + Valid: true, + }, + DisplayName: "", + SitePermissions: database.CustomRolePermissions{}, + UserPermissions: database.CustomRolePermissions{}, + OrgPermissions: database.CustomRolePermissions{}, + MemberPermissions: database.CustomRolePermissions{}, + }) + require.NoError(t, err) - stale := existing - stale.OrgPermissions = database.CustomRolePermissions{} - stale.MemberPermissions = database.CustomRolePermissions{} + stale := existing + stale.OrgPermissions = database.CustomRolePermissions{} + stale.MemberPermissions = database.CustomRolePermissions{} - reconciled, didUpdate, err := rolestore.ReconcileOrgMemberRole(ctx, db, stale, org.WorkspaceSharingDisabled) - require.NoError(t, err) - require.True(t, didUpdate, "expected reconciliation to update stale permissions") + reconciled, didUpdate, err := rolestore.ReconcileSystemRole(ctx, db, stale, org) + require.NoError(t, err) + require.True(t, didUpdate, "expected reconciliation to update stale permissions") - got, err := database.ExpectOne(db.CustomRoles(ctx, database.CustomRolesParams{ - LookupRoles: []database.NameOrganizationPair{ - { - Name: rbac.RoleOrgMember(), - OrganizationID: org.ID, - }, - }, - IncludeSystemRoles: true, - })) - require.NoError(t, err) + dbstored, err := database.ExpectOne(db.CustomRoles(ctx, database.CustomRolesParams{ + LookupRoles: []database.NameOrganizationPair{ + { + Name: tt.roleName, + OrganizationID: org.ID, + }, + }, + IncludeSystemRoles: true, + })) + require.NoError(t, err) - wantOrg, wantMember := rbac.OrgMemberPermissions(org.WorkspaceSharingDisabled) - require.True(t, rbac.PermissionsEqual(rolestore.ConvertDBPermissions(got.OrgPermissions), wantOrg)) - require.True(t, rbac.PermissionsEqual(rolestore.ConvertDBPermissions(got.MemberPermissions), wantMember)) - require.True(t, rbac.PermissionsEqual(rolestore.ConvertDBPermissions(reconciled.OrgPermissions), wantOrg)) - require.True(t, rbac.PermissionsEqual(rolestore.ConvertDBPermissions(reconciled.MemberPermissions), wantMember)) + want := tt.permsFunc(rbac.OrgSettings{ + ShareableWorkspaceOwners: rbac.ShareableWorkspaceOwners(org.ShareableWorkspaceOwners), + }) + require.True(t, rbac.PermissionsEqual(rolestore.ConvertDBPermissions(dbstored.OrgPermissions), want.Org)) + require.True(t, rbac.PermissionsEqual(rolestore.ConvertDBPermissions(dbstored.MemberPermissions), want.Member)) + require.True(t, rbac.PermissionsEqual(rolestore.ConvertDBPermissions(reconciled.OrgPermissions), want.Org)) + require.True(t, rbac.PermissionsEqual(rolestore.ConvertDBPermissions(reconciled.MemberPermissions), want.Member)) - _, didUpdate, err = rolestore.ReconcileOrgMemberRole(ctx, db, reconciled, org.WorkspaceSharingDisabled) - require.NoError(t, err) - require.False(t, didUpdate, "expected no-op reconciliation when permissions are already current") + _, didUpdate, err = rolestore.ReconcileSystemRole(ctx, db, reconciled, org) + require.NoError(t, err) + require.False(t, didUpdate, "expected no-op reconciliation when permissions are already current") + }) + } } func TestReconcileSystemRoles(t *testing.T) { @@ -118,7 +134,7 @@ func TestReconcileSystemRoles(t *testing.T) { ctx := testutil.Context(t, testutil.WaitShort) - _, err := sqlDB.ExecContext(ctx, "UPDATE organizations SET workspace_sharing_disabled = true WHERE id = $1", org2.ID) + _, err := sqlDB.ExecContext(ctx, "UPDATE organizations SET shareable_workspace_owners = 'none' WHERE id = $1", org2.ID) require.NoError(t, err) // Simulate a missing system role by bypassing the application's @@ -163,9 +179,9 @@ func TestReconcileSystemRoles(t *testing.T) { require.NoError(t, err) require.True(t, got.IsSystem) - wantOrg, wantMember := rbac.OrgMemberPermissions(org.WorkspaceSharingDisabled) - require.True(t, rbac.PermissionsEqual(rolestore.ConvertDBPermissions(got.OrgPermissions), wantOrg)) - require.True(t, rbac.PermissionsEqual(rolestore.ConvertDBPermissions(got.MemberPermissions), wantMember)) + want := rbac.OrgMemberPermissions(rbac.OrgSettings{ShareableWorkspaceOwners: rbac.ShareableWorkspaceOwners(org.ShareableWorkspaceOwners)}) + require.True(t, rbac.PermissionsEqual(rolestore.ConvertDBPermissions(got.OrgPermissions), want.Org)) + require.True(t, rbac.PermissionsEqual(rolestore.ConvertDBPermissions(got.MemberPermissions), want.Member)) } assertOrgMemberRole(t, org1.ID) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 76b42b36ec..e13f6380c1 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -2487,7 +2487,11 @@ func (api *API) patchWorkspaceACL(rw http.ResponseWriter, r *http.Request) { return nil }, nil) if err != nil { - httpapi.InternalServerError(rw, err) + if dbauthz.IsNotAuthorizedError(err) { + httpapi.Forbidden(rw) + } else { + httpapi.InternalServerError(rw, err) + } return } @@ -2566,7 +2570,7 @@ func (api *API) allowWorkspaceSharing(ctx context.Context, rw http.ResponseWrite httpapi.InternalServerError(rw, err) return false } - if org.WorkspaceSharingDisabled { + if org.ShareableWorkspaceOwners == database.ShareableWorkspaceOwnersNone { httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ Message: "Workspace sharing is disabled for this organization.", }) diff --git a/codersdk/workspacesharing.go b/codersdk/workspacesharing.go index 0d635063b6..6f64af80db 100644 --- a/codersdk/workspacesharing.go +++ b/codersdk/workspacesharing.go @@ -7,19 +7,41 @@ import ( "net/http" ) +// ShareableWorkspaceOwners controls whose workspaces can be shared +// within an organization. +type ShareableWorkspaceOwners string + +const ( + ShareableWorkspaceOwnersNone ShareableWorkspaceOwners = "none" + ShareableWorkspaceOwnersEveryone ShareableWorkspaceOwners = "everyone" + ShareableWorkspaceOwnersServiceAccounts ShareableWorkspaceOwners = "service_accounts" +) + // WorkspaceSharingSettings represents workspace sharing settings affecting an // organization. type WorkspaceSharingSettings struct { // SharingGloballyDisabled is true if sharing has been disabled for this // organization because of a deployment-wide setting. SharingGloballyDisabled bool `json:"sharing_globally_disabled"` - SharingDisabled bool `json:"sharing_disabled"` + // SharingDisabled is deprecated and left for backward compatibility + // purposes. + // Deprecated: use `ShareableWorkspaceOwners` instead + SharingDisabled bool `json:"sharing_disabled"` + // ShareableWorkspaceOwners controls whose workspaces can be shared + // within the organization. + ShareableWorkspaceOwners ShareableWorkspaceOwners `json:"shareable_workspace_owners" enums:"none,everyone,service_accounts"` } // UpdateWorkspaceSharingSettingsRequest represents workspace sharing settings // that can be updated for an organization. type UpdateWorkspaceSharingSettingsRequest struct { + // SharingDisabled is deprecated and left for backward compatibility + // purposes. + // Deprecated: use `ShareableWorkspaceOwners` instead SharingDisabled bool `json:"sharing_disabled"` + // ShareableWorkspaceOwners controls whose workspaces can be shared + // within the organization. + ShareableWorkspaceOwners ShareableWorkspaceOwners `json:"shareable_workspace_owners,omitempty" enums:"none,everyone,service_accounts"` } // WorkspaceSharingSettings retrieves the workspace sharing settings for an organization. diff --git a/docs/admin/security/audit-logs.md b/docs/admin/security/audit-logs.md index 421eee64de..a0e745be60 100644 --- a/docs/admin/security/audit-logs.md +++ b/docs/admin/security/audit-logs.md @@ -29,7 +29,7 @@ We track the following resources: | NotificationsSettings
| |
FieldTracked
idfalse
notifier_pausedtrue
| | OAuth2ProviderApp
| |
FieldTracked
callback_urltrue
client_id_issued_atfalse
client_secret_expires_attrue
client_typetrue
client_uritrue
contactstrue
created_atfalse
dynamically_registeredtrue
grant_typestrue
icontrue
idfalse
jwkstrue
jwks_uritrue
logo_uritrue
nametrue
policy_uritrue
redirect_uristrue
registration_access_tokentrue
registration_client_uritrue
response_typestrue
scopetrue
software_idtrue
software_versiontrue
token_endpoint_auth_methodtrue
tos_uritrue
updated_atfalse
| | OAuth2ProviderAppSecret
| |
FieldTracked
app_idfalse
created_atfalse
display_secretfalse
hashed_secretfalse
idfalse
last_used_atfalse
secret_prefixfalse
| -| Organization
| |
FieldTracked
created_atfalse
deletedtrue
descriptiontrue
display_nametrue
icontrue
idfalse
is_defaulttrue
nametrue
updated_attrue
workspace_sharing_disabledtrue
| +| Organization
| |
FieldTracked
created_atfalse
deletedtrue
descriptiontrue
display_nametrue
icontrue
idfalse
is_defaulttrue
nametrue
shareable_workspace_ownerstrue
updated_attrue
| | OrganizationSyncSettings
| |
FieldTracked
assign_defaulttrue
fieldtrue
mappingtrue
| | PrebuildsSettings
| |
FieldTracked
idfalse
reconciliation_pausedtrue
| | RoleSyncSettings
| |
FieldTracked
fieldtrue
mappingtrue
| diff --git a/docs/reference/api/enterprise.md b/docs/reference/api/enterprise.md index 524bdad000..51d14a815f 100644 --- a/docs/reference/api/enterprise.md +++ b/docs/reference/api/enterprise.md @@ -2851,6 +2851,7 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/setting ```json { + "shareable_workspace_owners": "none", "sharing_disabled": true, "sharing_globally_disabled": true } @@ -2882,17 +2883,17 @@ curl -X PATCH http://coder-server:8080/api/v2/organizations/{organization}/setti ```json { - "sharing_disabled": true, - "sharing_globally_disabled": true + "shareable_workspace_owners": "none", + "sharing_disabled": true } ``` ### Parameters -| Name | In | Type | Required | Description | -|----------------|------|----------------------------------------------------------------------------------|----------|----------------------------| -| `organization` | path | string(uuid) | true | Organization ID | -| `body` | body | [codersdk.WorkspaceSharingSettings](schemas.md#codersdkworkspacesharingsettings) | true | Workspace sharing settings | +| Name | In | Type | Required | Description | +|----------------|------|------------------------------------------------------------------------------------------------------------|----------|----------------------------| +| `organization` | path | string(uuid) | true | Organization ID | +| `body` | body | [codersdk.UpdateWorkspaceSharingSettingsRequest](schemas.md#codersdkupdateworkspacesharingsettingsrequest) | true | Workspace sharing settings | ### Example responses @@ -2900,15 +2901,17 @@ curl -X PATCH http://coder-server:8080/api/v2/organizations/{organization}/setti ```json { - "sharing_disabled": true + "shareable_workspace_owners": "none", + "sharing_disabled": true, + "sharing_globally_disabled": true } ``` ### Responses -| Status | Meaning | Description | Schema | -|--------|---------------------------------------------------------|-------------|------------------------------------------------------------------------------------------------------------| -| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.UpdateWorkspaceSharingSettingsRequest](schemas.md#codersdkupdateworkspacesharingsettingsrequest) | +| Status | Meaning | Description | Schema | +|--------|---------------------------------------------------------|-------------|----------------------------------------------------------------------------------| +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.WorkspaceSharingSettings](schemas.md#codersdkworkspacesharingsettings) | To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 80a0ccec90..02a579d46b 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -8011,6 +8011,20 @@ Only certain features set these fields: - FeatureManagedAgentLimit| | `max_token_lifetime` | integer | false | | | | `refresh_default_duration` | integer | false | | Refresh default duration is the default lifetime for OAuth2 refresh tokens. This should generally be longer than access token lifetimes to allow refreshing after access token expiry. | +## codersdk.ShareableWorkspaceOwners + +```json +"none" +``` + +### Properties + +#### Enumerated Values + +| Value(s) | +|----------------------------------------| +| `everyone`, `none`, `service_accounts` | + ## codersdk.SharedWorkspaceActor ```json @@ -9829,15 +9843,23 @@ If the schedule is empty, the user will be updated to use the default schedule.| ```json { + "shareable_workspace_owners": "none", "sharing_disabled": true } ``` ### Properties -| Name | Type | Required | Restrictions | Description | -|--------------------|---------|----------|--------------|-------------| -| `sharing_disabled` | boolean | false | | | +| Name | Type | Required | Restrictions | Description | +|------------------------------|------------------------------------------------------------------------|----------|--------------|---------------------------------------------------------------------------------------------------------------------------------| +| `shareable_workspace_owners` | [codersdk.ShareableWorkspaceOwners](#codersdkshareableworkspaceowners) | false | | Shareable workspace owners controls whose workspaces can be shared within the organization. | +| `sharing_disabled` | boolean | false | | Sharing disabled is deprecated and left for backward compatibility purposes. Deprecated: use `ShareableWorkspaceOwners` instead | + +#### Enumerated Values + +| Property | Value(s) | +|------------------------------|----------------------------------------| +| `shareable_workspace_owners` | `everyone`, `none`, `service_accounts` | ## codersdk.UpdateWorkspaceTTLRequest @@ -12308,6 +12330,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| ```json { + "shareable_workspace_owners": "none", "sharing_disabled": true, "sharing_globally_disabled": true } @@ -12315,10 +12338,17 @@ If the schedule is empty, the user will be updated to use the default schedule.| ### Properties -| Name | Type | Required | Restrictions | Description | -|-----------------------------|---------|----------|--------------|----------------------------------------------------------------------------------------------------------------------------| -| `sharing_disabled` | boolean | false | | | -| `sharing_globally_disabled` | boolean | false | | Sharing globally disabled is true if sharing has been disabled for this organization because of a deployment-wide setting. | +| Name | Type | Required | Restrictions | Description | +|------------------------------|------------------------------------------------------------------------|----------|--------------|---------------------------------------------------------------------------------------------------------------------------------| +| `shareable_workspace_owners` | [codersdk.ShareableWorkspaceOwners](#codersdkshareableworkspaceowners) | false | | Shareable workspace owners controls whose workspaces can be shared within the organization. | +| `sharing_disabled` | boolean | false | | Sharing disabled is deprecated and left for backward compatibility purposes. Deprecated: use `ShareableWorkspaceOwners` instead | +| `sharing_globally_disabled` | boolean | false | | Sharing globally disabled is true if sharing has been disabled for this organization because of a deployment-wide setting. | + +#### Enumerated Values + +| Property | Value(s) | +|------------------------------|----------------------------------------| +| `shareable_workspace_owners` | `everyone`, `none`, `service_accounts` | ## codersdk.WorkspaceStatus diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 9d7901d5ac..fbf7fe1a47 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -324,7 +324,7 @@ var auditableResourcesTypes = map[any]map[string]Action{ "is_default": ActionTrack, "display_name": ActionTrack, "icon": ActionTrack, - "workspace_sharing_disabled": ActionTrack, + "shareable_workspace_owners": ActionTrack, }, &database.NotificationTemplate{}: { "id": ActionIgnore, diff --git a/enterprise/coderd/groups_test.go b/enterprise/coderd/groups_test.go index 4f300de766..43e71d646e 100644 --- a/enterprise/coderd/groups_test.go +++ b/enterprise/coderd/groups_test.go @@ -737,19 +737,19 @@ func TestGroup(t *testing.T) { }, }) ctx := testutil.Context(t, testutil.WaitLong) - _, err := sqlDB.ExecContext(ctx, "UPDATE organizations SET workspace_sharing_disabled = true WHERE id = $1", user.OrganizationID) + _, err := sqlDB.ExecContext(ctx, "UPDATE organizations SET shareable_workspace_owners = 'none' WHERE id = $1", user.OrganizationID) require.NoError(t, err) //nolint:gocritic // ReconcileOrgMemberRole needs the system:update // permission that the test context doesn't have. sysCtx := dbauthz.AsSystemRestricted(ctx) - _, _, err = rolestore.ReconcileOrgMemberRole(sysCtx, api.Database, database.CustomRole{ + _, _, err = rolestore.ReconcileSystemRole(sysCtx, api.Database, database.CustomRole{ Name: rbac.RoleOrgMember(), OrganizationID: uuid.NullUUID{ UUID: user.OrganizationID, Valid: true, }, - }, true) + }, database.Organization{ShareableWorkspaceOwners: database.ShareableWorkspaceOwnersNone}) require.NoError(t, err) client1, _ := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) diff --git a/enterprise/coderd/organizations.go b/enterprise/coderd/organizations.go index ff6861f847..76d5060be6 100644 --- a/enterprise/coderd/organizations.go +++ b/enterprise/coderd/organizations.go @@ -16,7 +16,6 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" - "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac/rolestore" "github.com/coder/coder/v2/codersdk" ) @@ -298,16 +297,15 @@ func (api *API) postOrganizations(rw http.ResponseWriter, r *http.Request) { //nolint:gocritic // ReconcileOrgMemberRole needs the system:update // permission that user doesn't have. sysCtx := dbauthz.AsSystemRestricted(ctx) - _, _, err = rolestore.ReconcileOrgMemberRole(sysCtx, tx, database.CustomRole{ - Name: rbac.RoleOrgMember(), - OrganizationID: uuid.NullUUID{ - UUID: organizationID, - Valid: true, - }, - }, organization.WorkspaceSharingDisabled) - if err != nil { - return xerrors.Errorf("reconcile organization-member role for organization %s: %w", - organizationID, err) + for roleName := range rolestore.SystemRoleNames { + _, _, err = rolestore.ReconcileSystemRole(sysCtx, tx, database.CustomRole{ + Name: roleName, + OrganizationID: uuid.NullUUID{UUID: organizationID, Valid: true}, + }, organization) + if err != nil { + return xerrors.Errorf("reconcile %s role for organization %s: %w", + roleName, organizationID, err) + } } _, err = tx.InsertOrganizationMember(ctx, database.InsertOrganizationMemberParams{ diff --git a/enterprise/coderd/roles_test.go b/enterprise/coderd/roles_test.go index ccacb10a22..e8b6ae9218 100644 --- a/enterprise/coderd/roles_test.go +++ b/enterprise/coderd/roles_test.go @@ -288,7 +288,8 @@ func TestCustomOrganizationRole(t *testing.T) { require.ErrorContains(t, err, "not allowed to assign organization member permissions for an organization role") }) - // Attempt to delete a system role, which is not allowed. + // System roles are stored in the DB but excluded from the custom + // roles API, so attempting to delete one returns 404. t.Run("DeleteSystemRole", func(t *testing.T) { t.Parallel() @@ -306,8 +307,7 @@ func TestCustomOrganizationRole(t *testing.T) { err := owner.DeleteOrganizationRole(ctx, first.OrganizationID, rbac.RoleOrgMember()) var apiErr *codersdk.Error require.ErrorAs(t, err, &apiErr) - require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) - require.ErrorContains(t, err, "Reserved role name") + require.Equal(t, http.StatusNotFound, apiErr.StatusCode()) }) t.Run("NotFound", func(t *testing.T) { diff --git a/enterprise/coderd/workspacesharing.go b/enterprise/coderd/workspacesharing.go index 2f5af4728f..dfe106d186 100644 --- a/enterprise/coderd/workspacesharing.go +++ b/enterprise/coderd/workspacesharing.go @@ -1,7 +1,9 @@ package coderd import ( + "fmt" "net/http" + "strings" "golang.org/x/xerrors" @@ -14,6 +16,7 @@ import ( "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/coderd/rbac/rolestore" + "github.com/coder/coder/v2/coderd/util/slice" "github.com/coder/coder/v2/codersdk" ) @@ -34,10 +37,16 @@ func (api *API) workspaceSharingSettings(rw http.ResponseWriter, r *http.Request return } + disabled := org.ShareableWorkspaceOwners == database.ShareableWorkspaceOwnersNone globallyDisabled := bool(api.DeploymentValues.DisableWorkspaceSharing) + owners := codersdk.ShareableWorkspaceOwners(org.ShareableWorkspaceOwners) + if globallyDisabled { + owners = codersdk.ShareableWorkspaceOwnersNone + } httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspaceSharingSettings{ - SharingGloballyDisabled: globallyDisabled, - SharingDisabled: org.WorkspaceSharingDisabled || globallyDisabled, + SharingGloballyDisabled: globallyDisabled, + SharingDisabled: disabled || globallyDisabled, + ShareableWorkspaceOwners: owners, }) } @@ -48,8 +57,8 @@ func (api *API) workspaceSharingSettings(rw http.ResponseWriter, r *http.Request // @Accept json // @Tags Enterprise // @Param organization path string true "Organization ID" format(uuid) -// @Param request body codersdk.WorkspaceSharingSettings true "Workspace sharing settings" -// @Success 200 {object} codersdk.UpdateWorkspaceSharingSettingsRequest +// @Param request body codersdk.UpdateWorkspaceSharingSettingsRequest true "Workspace sharing settings" +// @Success 200 {object} codersdk.WorkspaceSharingSettings // @Router /organizations/{organization}/settings/workspace-sharing [patch] func (api *API) patchWorkspaceSharingSettings(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() @@ -70,19 +79,44 @@ func (api *API) patchWorkspaceSharingSettings(rw http.ResponseWriter, r *http.Re return } - var req codersdk.WorkspaceSharingSettings + var req codersdk.UpdateWorkspaceSharingSettingsRequest if !httpapi.Read(ctx, rw, r, &req) { return } + // Resolve the effective enum value. Prefer the new field; fall + // back to the deprecated boolean for older clients (e.g + // tf-provider-coderd v0.0.16) + allowedOwners := req.ShareableWorkspaceOwners + if allowedOwners == "" { + if req.SharingDisabled { + allowedOwners = codersdk.ShareableWorkspaceOwnersNone + } else { + allowedOwners = codersdk.ShareableWorkspaceOwnersEveryone + } + } + + if !database.ShareableWorkspaceOwners(allowedOwners).Valid() { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Invalid shareable workspace owners value.", + Validations: []codersdk.ValidationError{{ + Field: "shareable_workspace_owners", + Detail: fmt.Sprintf("invalid value %q, must be one of [%s]", + allowedOwners, + strings.Join(slice.ToStrings(database.AllShareableWorkspaceOwnersValues()), ", ")), + }}, + }) + return + } + err := api.Database.InTx(func(tx database.Store) error { //nolint:gocritic // System context required to look up and reconcile the - // organization-member system role; callers only need `organization:update` + // system roles; callers only need `organization:update` sysCtx := dbauthz.AsSystemRestricted(ctx) // Serialize organization workspace-sharing updates with system role // reconciliation across coderd instances (e.g. during rolling restarts). - // This prevents conflicting writes to the organization-member system role. + // This prevents conflicting writes to the system roles. // TODO(geokat): Consider finer-grained locks as we add more system roles. err := tx.AcquireLock(ctx, database.LockIDReconcileSystemRoles) if err != nil { @@ -91,38 +125,54 @@ func (api *API) patchWorkspaceSharingSettings(rw http.ResponseWriter, r *http.Re org, err = tx.UpdateOrganizationWorkspaceSharingSettings(ctx, database.UpdateOrganizationWorkspaceSharingSettingsParams{ ID: org.ID, - WorkspaceSharingDisabled: req.SharingDisabled, + ShareableWorkspaceOwners: database.ShareableWorkspaceOwners(allowedOwners), UpdatedAt: dbtime.Now(), }) if err != nil { - return xerrors.Errorf("update organization workspace sharing settings: %w", err) + return xerrors.Errorf("update workspace sharing settings for organization %s: %w", + org.ID, err) } - role, err := database.ExpectOne(tx.CustomRoles(sysCtx, database.CustomRolesParams{ + roles, err := tx.CustomRoles(sysCtx, database.CustomRolesParams{ LookupRoles: []database.NameOrganizationPair{ { Name: rbac.RoleOrgMember(), OrganizationID: org.ID, }, + { + Name: rbac.RoleOrgServiceAccount(), + OrganizationID: org.ID, + }, }, // Satisfy linter that requires all fields to be set. OrganizationID: org.ID, ExcludeOrgRoles: false, IncludeSystemRoles: true, - })) - if err != nil { - return xerrors.Errorf("get organization-member role: %w", err) + }) + if err != nil || len(roles) != 2 { + return xerrors.Errorf("get member and service-account roles for organization %s: %w", + org.ID, err) } - _, _, err = rolestore.ReconcileOrgMemberRole(sysCtx, tx, role, req.SharingDisabled) - if err != nil { - return xerrors.Errorf("reconcile organization-member role: %w", err) - } - - if req.SharingDisabled { - err = tx.DeleteWorkspaceACLsByOrganization(sysCtx, org.ID) + for _, role := range roles { + _, _, err = rolestore.ReconcileSystemRole(sysCtx, tx, role, org) if err != nil { - return xerrors.Errorf("delete workspace ACLs by organization: %w", err) + return xerrors.Errorf("reconcile %s role for organization %s: %w", + role.Name, org.ID, err) + } + } + + // If sharing is not enabled, delete workspace ACLs to prevent + // ongoing shared use. In "service_accounts" mode, preserve + // ACLs on SA workspaces. + if org.ShareableWorkspaceOwners != database.ShareableWorkspaceOwnersEveryone { + err = tx.DeleteWorkspaceACLsByOrganization(sysCtx, database.DeleteWorkspaceACLsByOrganizationParams{ + OrganizationID: org.ID, + ExcludeServiceAccounts: org.ShareableWorkspaceOwners == database.ShareableWorkspaceOwnersServiceAccounts, + }) + if err != nil { + return xerrors.Errorf("delete workspace ACLs for organization %s: %w", + org.ID, err) } } @@ -138,6 +188,7 @@ func (api *API) patchWorkspaceSharingSettings(rw http.ResponseWriter, r *http.Re aReq.New = org httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspaceSharingSettings{ - SharingDisabled: org.WorkspaceSharingDisabled, + SharingDisabled: org.ShareableWorkspaceOwners == database.ShareableWorkspaceOwnersNone, + ShareableWorkspaceOwners: codersdk.ShareableWorkspaceOwners(org.ShareableWorkspaceOwners), }) } diff --git a/enterprise/coderd/workspacesharing_test.go b/enterprise/coderd/workspacesharing_test.go index 37acf21e19..ac37b58914 100644 --- a/enterprise/coderd/workspacesharing_test.go +++ b/enterprise/coderd/workspacesharing_test.go @@ -11,7 +11,9 @@ import ( "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbfake" + "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" "github.com/coder/coder/v2/enterprise/coderd/license" @@ -34,10 +36,13 @@ func TestWorkspaceSharingSettings(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) + // Use a regular user to make sure the setting is exposed to them. memberClient, _ := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) settings, err := memberClient.WorkspaceSharingSettings(ctx, first.OrganizationID.String()) require.NoError(t, err) + // Check the deprecated boolean field. require.False(t, settings.SharingDisabled) + require.Equal(t, codersdk.ShareableWorkspaceOwnersEveryone, settings.ShareableWorkspaceOwners) }) t.Run("DisabledTogglePersists", func(t *testing.T) { @@ -54,21 +59,59 @@ func TestWorkspaceSharingSettings(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) orgAdminClient, _ := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, rbac.ScopedRoleOrgAdmin(first.OrganizationID)) + + // Disable sharing via the deprecated boolean field. settings, err := orgAdminClient.PatchWorkspaceSharingSettings(ctx, first.OrganizationID.String(), codersdk.UpdateWorkspaceSharingSettingsRequest{ SharingDisabled: true, }) require.NoError(t, err) require.True(t, settings.SharingDisabled) + require.Equal(t, codersdk.ShareableWorkspaceOwnersNone, settings.ShareableWorkspaceOwners) settings, err = orgAdminClient.WorkspaceSharingSettings(ctx, first.OrganizationID.String()) require.NoError(t, err) require.True(t, settings.SharingDisabled) + require.Equal(t, codersdk.ShareableWorkspaceOwnersNone, settings.ShareableWorkspaceOwners) + // Switch to service_accounts mode via the new field. settings, err = orgAdminClient.PatchWorkspaceSharingSettings(ctx, first.OrganizationID.String(), codersdk.UpdateWorkspaceSharingSettingsRequest{ - SharingDisabled: false, + ShareableWorkspaceOwners: codersdk.ShareableWorkspaceOwnersServiceAccounts, }) require.NoError(t, err) require.False(t, settings.SharingDisabled) + require.Equal(t, codersdk.ShareableWorkspaceOwnersServiceAccounts, settings.ShareableWorkspaceOwners) + + settings, err = orgAdminClient.WorkspaceSharingSettings(ctx, first.OrganizationID.String()) + require.NoError(t, err) + require.Equal(t, codersdk.ShareableWorkspaceOwnersServiceAccounts, settings.ShareableWorkspaceOwners) + + // Re-enable full sharing. + settings, err = orgAdminClient.PatchWorkspaceSharingSettings(ctx, first.OrganizationID.String(), codersdk.UpdateWorkspaceSharingSettingsRequest{ + ShareableWorkspaceOwners: codersdk.ShareableWorkspaceOwnersEveryone, + }) + require.NoError(t, err) + require.False(t, settings.SharingDisabled) + require.Equal(t, codersdk.ShareableWorkspaceOwnersEveryone, settings.ShareableWorkspaceOwners) + + settings, err = orgAdminClient.WorkspaceSharingSettings(ctx, first.OrganizationID.String()) + require.NoError(t, err) + require.Equal(t, codersdk.ShareableWorkspaceOwnersEveryone, settings.ShareableWorkspaceOwners) + }) + + t.Run("InvalidValueRejected", func(t *testing.T) { + t.Parallel() + + client, first := coderdenttest.New(t, nil) + + ctx := testutil.Context(t, testutil.WaitMedium) + + orgAdminClient, _ := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, rbac.ScopedRoleOrgAdmin(first.OrganizationID)) + _, err := orgAdminClient.PatchWorkspaceSharingSettings(ctx, first.OrganizationID.String(), codersdk.UpdateWorkspaceSharingSettingsRequest{ + ShareableWorkspaceOwners: "invalid", + }) + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) }) t.Run("UpdateAuthz", func(t *testing.T) { @@ -153,7 +196,7 @@ func TestWorkspaceSharingDisabled(t *testing.T) { orgAdminClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.ScopedRoleOrgAdmin(owner.OrganizationID)) _, err := orgAdminClient.PatchWorkspaceSharingSettings(ctx, owner.OrganizationID.String(), codersdk.UpdateWorkspaceSharingSettingsRequest{ - SharingDisabled: true, + ShareableWorkspaceOwners: codersdk.ShareableWorkspaceOwnersNone, }) require.NoError(t, err) @@ -185,6 +228,132 @@ func TestWorkspaceSharingDisabled(t *testing.T) { assertSharingDisabled(t, err) }) + t.Run("ACLEndpointsForbiddenServiceAccountsMode", func(t *testing.T) { + t.Parallel() + + client, db, owner := coderdenttest.NewWithDatabase(t, nil) + + regularClient, regularUser := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) + regularWS := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: regularUser.ID, + OrganizationID: owner.OrganizationID, + }).Do().Workspace + + // Create an SA with a workspace. + saClient, saUser := coderdtest.CreateAnotherUserMutators(t, client, owner.OrganizationID, nil, func(r *codersdk.CreateUserRequestWithOrgs) { + r.ServiceAccount = true + }) + saWS := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: saUser.ID, + OrganizationID: owner.OrganizationID, + }).Do().Workspace + + ctx := testutil.Context(t, testutil.WaitMedium) + + orgAdminClient, orgAdmin := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.ScopedRoleOrgAdmin(owner.OrganizationID)) + _, err := orgAdminClient.PatchWorkspaceSharingSettings(ctx, owner.OrganizationID.String(), codersdk.UpdateWorkspaceSharingSettingsRequest{ + ShareableWorkspaceOwners: codersdk.ShareableWorkspaceOwnersServiceAccounts, + }) + require.NoError(t, err) + + // Regular member cannot share their own workspace. + err = regularClient.UpdateWorkspaceACL(ctx, regularWS.ID, codersdk.UpdateWorkspaceACL{ + UserRoles: map[string]codersdk.WorkspaceRole{ + orgAdmin.ID.String(): codersdk.WorkspaceRoleUse, + }, + }) + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusForbidden, apiErr.StatusCode()) + + // SA can share their own workspace. + err = saClient.UpdateWorkspaceACL(ctx, saWS.ID, codersdk.UpdateWorkspaceACL{ + UserRoles: map[string]codersdk.WorkspaceRole{ + regularUser.ID.String(): codersdk.WorkspaceRoleUse, + }, + }) + require.NoError(t, err) + }) + + // Future-proofing: if custom roles with member-scoped + // workspace:share are ever allowed, the member-level negation + // from the organization-member system role must block sharing in + // service_accounts mode even with such custom role. + t.Run("MemberCannotBypassWithCustomRole", func(t *testing.T) { + t.Parallel() + + rawDB, pubsub, sqlDB := dbtestutil.NewDBWithSQLDB(t) + client, _, _, owner := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + Database: rawDB, + Pubsub: pubsub, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureCustomRoles: 1, + codersdk.FeatureTemplateRBAC: 1, + }, + }, + }) + + ctx := testutil.Context(t, testutil.WaitMedium) + + // Create an empty custom role via the API, then add + // member-scoped workspace:share via raw SQL (the API and + // dbauthz both reject member permissions on custom roles). + //nolint:gocritic // owner context required for role creation + customRole, err := client.CreateOrganizationRole(ctx, codersdk.Role{ + Name: "workspace-share-granter", + OrganizationID: owner.OrganizationID.String(), + }) + require.NoError(t, err) + + _, err = sqlDB.ExecContext(ctx, + `UPDATE custom_roles SET member_permissions = $1 WHERE name = $2 AND organization_id = $3`, + database.CustomRolePermissions{{ + ResourceType: rbac.ResourceWorkspace.Type, + Action: policy.ActionShare, + }}, + customRole.Name, + owner.OrganizationID, + ) + require.NoError(t, err) + + // Create a member and assign the custom role. + memberClient, memberUser := coderdtest.CreateAnotherUserMutators( + t, client, owner.OrganizationID, + []rbac.RoleIdentifier{{ + Name: customRole.Name, + OrganizationID: owner.OrganizationID, + }}, + ) + memberWS := dbfake.WorkspaceBuild(t, rawDB, database.WorkspaceTable{ + OwnerID: memberUser.ID, + OrganizationID: owner.OrganizationID, + }).Do().Workspace + + _, sharedUser := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) + + // Switch to service_accounts mode. + orgAdminClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.ScopedRoleOrgAdmin(owner.OrganizationID)) + _, err = orgAdminClient.PatchWorkspaceSharingSettings(ctx, owner.OrganizationID.String(), codersdk.UpdateWorkspaceSharingSettingsRequest{ + ShareableWorkspaceOwners: codersdk.ShareableWorkspaceOwnersServiceAccounts, + }) + require.NoError(t, err) + + // Despite the custom role granting workspace:share at the + // member level, the negation from organization-member should + // block it. + err = memberClient.UpdateWorkspaceACL(ctx, memberWS.ID, codersdk.UpdateWorkspaceACL{ + UserRoles: map[string]codersdk.WorkspaceRole{ + sharedUser.ID.String(): codersdk.WorkspaceRoleUse, + }, + }) + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusForbidden, apiErr.StatusCode()) + }) + t.Run("ACLsPurged", func(t *testing.T) { t.Parallel() @@ -236,12 +405,12 @@ func TestWorkspaceSharingDisabled(t *testing.T) { orgAdminClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.ScopedRoleOrgAdmin(owner.OrganizationID)) _, err = orgAdminClient.PatchWorkspaceSharingSettings(ctx, owner.OrganizationID.String(), codersdk.UpdateWorkspaceSharingSettingsRequest{ - SharingDisabled: true, + ShareableWorkspaceOwners: codersdk.ShareableWorkspaceOwnersNone, }) require.NoError(t, err) _, err = orgAdminClient.PatchWorkspaceSharingSettings(ctx, owner.OrganizationID.String(), codersdk.UpdateWorkspaceSharingSettingsRequest{ - SharingDisabled: false, + ShareableWorkspaceOwners: codersdk.ShareableWorkspaceOwnersEveryone, }) require.NoError(t, err) @@ -263,4 +432,76 @@ func TestWorkspaceSharingDisabled(t *testing.T) { require.Len(t, acl.Users, 1) require.Equal(t, sharedUser.ID, acl.Users[0].ID) }) + + t.Run("ACLsPurgedExceptServiceAccounts", func(t *testing.T) { + t.Parallel() + + dv := coderdtest.DeploymentValues(t) + + client, db, owner := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + DeploymentValues: dv, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureTemplateRBAC: 1, + }, + }, + }) + + // Regular user with a workspace. + workspaceOwnerClient, workspaceOwner := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) + _, sharedUser := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) + + regularWS := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: workspaceOwner.ID, + OrganizationID: owner.OrganizationID, + }).Do().Workspace + + // Service account with a workspace. + _, saUser := coderdtest.CreateAnotherUserMutators(t, client, owner.OrganizationID, nil, func(r *codersdk.CreateUserRequestWithOrgs) { + r.ServiceAccount = true + }) + saWS := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: saUser.ID, + OrganizationID: owner.OrganizationID, + }).Do().Workspace + + ctx := testutil.Context(t, testutil.WaitMedium) + + // Share regular user's workspace with sharedUser. + err := workspaceOwnerClient.UpdateWorkspaceACL(ctx, regularWS.ID, codersdk.UpdateWorkspaceACL{ + UserRoles: map[string]codersdk.WorkspaceRole{ + sharedUser.ID.String(): codersdk.WorkspaceRoleUse, + }, + }) + require.NoError(t, err) + + // Use the owner client (site admin) to share the SA workspace, + // since the SA can't authenticate via the API. + err = client.UpdateWorkspaceACL(ctx, saWS.ID, codersdk.UpdateWorkspaceACL{ + UserRoles: map[string]codersdk.WorkspaceRole{ + sharedUser.ID.String(): codersdk.WorkspaceRoleUse, + }, + }) + require.NoError(t, err) + + // Switch to service_accounts mode. + orgAdminClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.ScopedRoleOrgAdmin(owner.OrganizationID)) + _, err = orgAdminClient.PatchWorkspaceSharingSettings(ctx, owner.OrganizationID.String(), codersdk.UpdateWorkspaceSharingSettingsRequest{ + ShareableWorkspaceOwners: codersdk.ShareableWorkspaceOwnersServiceAccounts, + }) + require.NoError(t, err) + + // Regular user workspace ACLs should be purged. + acl, err := workspaceOwnerClient.WorkspaceACL(ctx, regularWS.ID) + require.NoError(t, err) + require.Empty(t, acl.Users) + + // Service account workspace ACLs should be preserved. + acl, err = client.WorkspaceACL(ctx, saWS.ID) + require.NoError(t, err) + require.Len(t, acl.Users, 1) + require.Equal(t, sharedUser.ID, acl.Users[0].ID) + }) } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 7b2630af6c..1d0ff5e18c 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -5707,6 +5707,15 @@ export interface SessionLifetime { */ export const SessionTokenHeader = "Coder-Session-Token"; +// From codersdk/workspacesharing.go +export type ShareableWorkspaceOwners = "everyone" | "none" | "service_accounts"; + +export const ShareableWorkspaceOwnerses: ShareableWorkspaceOwners[] = [ + "everyone", + "none", + "service_accounts", +]; + // From codersdk/workspaces.go export interface SharedWorkspaceActor { readonly id: string; @@ -6891,7 +6900,17 @@ export interface UpdateWorkspaceRequest { * that can be updated for an organization. */ export interface UpdateWorkspaceSharingSettingsRequest { + /** + * SharingDisabled is deprecated and left for backward compatibility + * purposes. + * Deprecated: use `ShareableWorkspaceOwners` instead + */ readonly sharing_disabled: boolean; + /** + * ShareableWorkspaceOwners controls whose workspaces can be shared + * within the organization. + */ + readonly shareable_workspace_owners?: ShareableWorkspaceOwners; } // From codersdk/workspaces.go @@ -8043,7 +8062,17 @@ export interface WorkspaceSharingSettings { * organization because of a deployment-wide setting. */ readonly sharing_globally_disabled: boolean; + /** + * SharingDisabled is deprecated and left for backward compatibility + * purposes. + * Deprecated: use `ShareableWorkspaceOwners` instead + */ readonly sharing_disabled: boolean; + /** + * ShareableWorkspaceOwners controls whose workspaces can be shared + * within the organization. + */ + readonly shareable_workspace_owners: ShareableWorkspaceOwners; } // From codersdk/workspacebuilds.go