diff --git a/cli/sharing_test.go b/cli/sharing_test.go index 19e1853470..90df1fa992 100644 --- a/cli/sharing_test.go +++ b/cli/sharing_test.go @@ -197,7 +197,7 @@ func TestSharingStatus(t *testing.T) { ctx = testutil.Context(t, testutil.WaitMedium) ) - err := client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ + err := workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ UserRoles: map[string]codersdk.WorkspaceRole{ toShareWithUser.ID.String(): codersdk.WorkspaceRoleUse, }, @@ -248,7 +248,7 @@ func TestSharingRemove(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) // Share the workspace with a user to later remove - err := client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ + err := workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ UserRoles: map[string]codersdk.WorkspaceRole{ toShareWithUser.ID.String(): codersdk.WorkspaceRoleUse, toRemoveUser.ID.String(): codersdk.WorkspaceRoleUse, @@ -309,7 +309,7 @@ func TestSharingRemove(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) // Share the workspace with a user to later remove - err := client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ + err := workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ UserRoles: map[string]codersdk.WorkspaceRole{ toRemoveUser2.ID.String(): codersdk.WorkspaceRoleUse, toRemoveUser1.ID.String(): codersdk.WorkspaceRoleUse, diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index f6735f0212..f450fe05b0 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -3349,8 +3349,8 @@ const docTemplate = `{ "tags": [ "Members" ], - "summary": "Upsert a custom organization role", - "operationId": "upsert-a-custom-organization-role", + "summary": "Update a custom organization role", + "operationId": "update-a-custom-organization-role", "parameters": [ { "type": "string", @@ -3361,7 +3361,7 @@ const docTemplate = `{ "required": true }, { - "description": "Upsert role request", + "description": "Update role request", "name": "request", "in": "body", "required": true, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 11a167f62e..fab51b33bf 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -2935,8 +2935,8 @@ "consumes": ["application/json"], "produces": ["application/json"], "tags": ["Members"], - "summary": "Upsert a custom organization role", - "operationId": "upsert-a-custom-organization-role", + "summary": "Update a custom organization role", + "operationId": "update-a-custom-organization-role", "parameters": [ { "type": "string", @@ -2947,7 +2947,7 @@ "required": true }, { - "description": "Upsert role request", + "description": "Update role request", "name": "request", "in": "body", "required": true, diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index 22a7a8c7f9..be6ccda582 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -1037,7 +1037,7 @@ func TestExecutorRequireActiveVersion(t *testing.T) { //nolint We need to set this in the database directly, because the API will return an error // letting you know that this feature requires an enterprise license. - err = db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(me, owner.OrganizationID)), database.UpdateTemplateAccessControlByIDParams{ + err = db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(me)), database.UpdateTemplateAccessControlByIDParams{ ID: template.ID, RequireActiveVersion: true, }) diff --git a/coderd/coderd.go b/coderd/coderd.go index a493a9da1a..351ddde805 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -568,6 +568,16 @@ func New(options *Options) *API { // bugs that may only occur when a key isn't precached in tests and the latency cost is minimal. cryptokeys.StartRotator(ctx, options.Logger, options.Database) + // Ensure all system role permissions are current. + //nolint:gocritic // Startup reconciliation reads/writes system roles. There is + // no user request context here, so use a system-restricted context. + err = rolestore.ReconcileSystemRoles(dbauthz.AsSystemRestricted(ctx), options.Logger, options.Database) + if err != nil { + // Not ideal, but not using Fatal here and just continuing + // after logging the error would be a potential security hole. + options.Logger.Fatal(ctx, "failed to reconcile system role permissions", slog.Error(err)) + } + // AGPL uses a no-op build usage checker as there are no license // entitlements to enforce. This is swapped out in // enterprise/coderd/coderd.go. diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index a167230c19..cd69cc2686 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -76,6 +76,7 @@ import ( "github.com/coder/coder/v2/coderd/provisionerdserver" "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/runtimeconfig" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/telemetry" @@ -768,8 +769,9 @@ func CreateAnotherUserMutators(t testing.TB, client *codersdk.Client, organizati return createAnotherUserRetry(t, client, []uuid.UUID{organizationID}, 5, roles, mutators...) } -// AuthzUserSubject does not include the user's groups. -func AuthzUserSubject(user codersdk.User, orgID uuid.UUID) rbac.Subject { +// AuthzUserSubject does not include the user's groups or the org-member role +// (which is a db-backed system role). +func AuthzUserSubject(user codersdk.User) rbac.Subject { roles := make(rbac.RoleIdentifiers, 0, len(user.Roles)) // Member role is always implied roles = append(roles, rbac.RoleMember()) @@ -780,8 +782,6 @@ func AuthzUserSubject(user codersdk.User, orgID uuid.UUID) rbac.Subject { OrganizationID: orgID, }) } - // We assume only 1 org exists - roles = append(roles, rbac.ScopedRoleOrgMember(orgID)) return rbac.Subject{ ID: user.ID.String(), @@ -791,6 +791,52 @@ func AuthzUserSubject(user codersdk.User, orgID uuid.UUID) rbac.Subject { } } +// AuthzUserSubjectWithDB is like AuthzUserSubject but adds db-backed roles +// (like organization-member). +func AuthzUserSubjectWithDB(ctx context.Context, t testing.TB, db database.Store, user codersdk.User) rbac.Subject { + t.Helper() + + roles := make(rbac.RoleIdentifiers, 0, len(user.Roles)+2) + // Member role is always implied + roles = append(roles, rbac.RoleMember()) + for _, r := range user.Roles { + parsedOrgID, _ := uuid.Parse(r.OrganizationID) // defaults to nil + roles = append(roles, rbac.RoleIdentifier{ + Name: r.Name, + OrganizationID: parsedOrgID, + }) + } + + //nolint:gocritic // We’re constructing the subject. The incoming ctx + // typically has no dbauthz actor yet, and using AuthzUserSubject(user) + // here would be circular (it lacks DB-backed org-member roles needed for + // organization:read). Use system-restricted ctx for the membership lookup. + orgs, err := db.GetOrganizationsByUserID(dbauthz.AsSystemRestricted(ctx), database.GetOrganizationsByUserIDParams{ + UserID: user.ID, + Deleted: sql.NullBool{ + Valid: true, + Bool: false, + }, + }) + require.NoError(t, err) + for _, org := range orgs { + roles = append(roles, rbac.ScopedRoleOrgMember(org.ID)) + } + + //nolint:gocritic // We need to expand DB-backed/system roles. The caller + // ctx may not have permission to read system roles, so use system-restricted + // context for the internal role lookup. + rbacRoles, err := rolestore.Expand(dbauthz.AsSystemRestricted(ctx), db, roles) + require.NoError(t, err) + + return rbac.Subject{ + ID: user.ID.String(), + Roles: rbacRoles, + Groups: []string{}, + Scope: rbac.ScopeAll, + }.WithCachedASTValue() +} + func createAnotherUserRetry(t testing.TB, client *codersdk.Client, organizationIDs []uuid.UUID, retries int, roles []rbac.RoleIdentifier, mutators ...func(r *codersdk.CreateUserRequestWithOrgs)) (*codersdk.Client, codersdk.User) { req := codersdk.CreateUserRequestWithOrgs{ Email: namesgenerator.UniqueName() + "@coder.com", diff --git a/coderd/database/dbauthz/customroles_test.go b/coderd/database/dbauthz/customroles_test.go index 52b905a04f..790541f47e 100644 --- a/coderd/database/dbauthz/customroles_test.go +++ b/coderd/database/dbauthz/customroles_test.go @@ -73,6 +73,7 @@ func TestInsertCustomRoles(t *testing.T) { site []codersdk.Permission org []codersdk.Permission user []codersdk.Permission + member []codersdk.Permission errorContains string }{ { @@ -171,6 +172,16 @@ func TestInsertCustomRoles(t *testing.T) { }), errorContains: "organization roles specify site or user permissions", }, + { + // Not allowing these at this time. + name: "member-permissions", + organizationID: orgID, + subject: merge(canCreateCustomRole), + member: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceWorkspace: {codersdk.ActionRead}, + }), + errorContains: "non-system roles specify member permissions", + }, { name: "site-escalation", organizationID: orgID, @@ -213,12 +224,13 @@ func TestInsertCustomRoles(t *testing.T) { ctx = dbauthz.As(ctx, subject) _, err := az.InsertCustomRole(ctx, database.InsertCustomRoleParams{ - Name: "test-role", - DisplayName: "", - OrganizationID: uuid.NullUUID{UUID: tc.organizationID, Valid: true}, - SitePermissions: db2sdk.List(tc.site, convertSDKPerm), - OrgPermissions: db2sdk.List(tc.org, convertSDKPerm), - UserPermissions: db2sdk.List(tc.user, convertSDKPerm), + Name: "test-role", + DisplayName: "", + OrganizationID: uuid.NullUUID{UUID: tc.organizationID, Valid: true}, + SitePermissions: db2sdk.List(tc.site, convertSDKPerm), + OrgPermissions: db2sdk.List(tc.org, convertSDKPerm), + UserPermissions: db2sdk.List(tc.user, convertSDKPerm), + MemberPermissions: db2sdk.List(tc.member, convertSDKPerm), }) if tc.errorContains != "" { require.ErrorContains(t, err, tc.errorContains) @@ -250,3 +262,220 @@ func convertSDKPerm(perm codersdk.Permission) database.CustomRolePermission { Action: policy.Action(perm.Action), } } + +func TestSystemRoles(t *testing.T) { + t.Parallel() + + orgID := uuid.New() + + canManageOrgRoles := rbac.Role{ + Identifier: rbac.RoleIdentifier{Name: "can-manage-org-roles"}, + DisplayName: "", + Site: rbac.Permissions(map[string][]policy.Action{ + rbac.ResourceAssignOrgRole.Type: {policy.ActionRead, policy.ActionCreate, policy.ActionUpdate}, + }), + } + + canCreateSystem := rbac.Role{ + Identifier: rbac.RoleIdentifier{Name: "can-create-system"}, + DisplayName: "", + Site: rbac.Permissions(map[string][]policy.Action{ + rbac.ResourceSystem.Type: {policy.ActionCreate}, + }), + } + + canUpdateSystem := rbac.Role{ + Identifier: rbac.RoleIdentifier{Name: "can-update-system"}, + DisplayName: "", + Site: rbac.Permissions(map[string][]policy.Action{ + rbac.ResourceSystem.Type: {policy.ActionUpdate}, + }), + } + + userID := uuid.New() + subjectNoSystemPerms := rbac.Subject{ + FriendlyName: "Test user", + ID: userID.String(), + Roles: rbac.Roles([]rbac.Role{canManageOrgRoles}), + Groups: nil, + Scope: rbac.ScopeAll, + } + subjectWithSystemCreatePerms := subjectNoSystemPerms + subjectWithSystemCreatePerms.Roles = rbac.Roles([]rbac.Role{canManageOrgRoles, canCreateSystem}) + subjectWithSystemUpdatePerms := subjectNoSystemPerms + subjectWithSystemUpdatePerms.Roles = rbac.Roles([]rbac.Role{canManageOrgRoles, canUpdateSystem}) + + db, _ := dbtestutil.NewDB(t) + rec := &coderdtest.RecordingAuthorizer{ + Wrapped: rbac.NewAuthorizer(prometheus.NewRegistry()), + } + az := dbauthz.New(db, rec, slog.Make(), coderdtest.AccessControlStorePointer()) + + t.Run("insert-requires-system-create", func(t *testing.T) { + t.Parallel() + + insertParamsTemplate := database.InsertCustomRoleParams{ + Name: "", + OrganizationID: uuid.NullUUID{ + UUID: orgID, + Valid: true, + }, + SitePermissions: database.CustomRolePermissions{}, + OrgPermissions: database.CustomRolePermissions{}, + UserPermissions: database.CustomRolePermissions{}, + MemberPermissions: database.CustomRolePermissions{}, + IsSystem: true, + } + + t.Run("deny-no-system-perms", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + insertParams := insertParamsTemplate + insertParams.Name = "test-system-role-" + uuid.NewString() + + ctx = dbauthz.As(ctx, subjectNoSystemPerms) + + _, err := az.InsertCustomRole(ctx, insertParams) + require.ErrorContains(t, err, "forbidden") + }) + + t.Run("deny-update-only", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + insertParams := insertParamsTemplate + insertParams.Name = "test-system-role-" + uuid.NewString() + + ctx = dbauthz.As(ctx, subjectWithSystemUpdatePerms) + + _, err := az.InsertCustomRole(ctx, insertParams) + require.ErrorContains(t, err, "forbidden") + }) + + t.Run("allow-create-only", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + insertParams := insertParamsTemplate + insertParams.Name = "test-system-role-" + uuid.NewString() + + ctx = dbauthz.As(ctx, subjectWithSystemCreatePerms) + + _, err := az.InsertCustomRole(ctx, insertParams) + require.NoError(t, err) + }) + }) + + t.Run("update-requires-system-update", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + ctx = dbauthz.As(ctx, subjectWithSystemCreatePerms) + + // Setup: create the role that we will attempt to update in + // subtests. One role for all is fine as we are only testing + // authz. + role, err := az.InsertCustomRole(ctx, database.InsertCustomRoleParams{ + Name: "test-system-role-" + uuid.NewString(), + OrganizationID: uuid.NullUUID{ + UUID: orgID, + Valid: true, + }, + SitePermissions: database.CustomRolePermissions{}, + OrgPermissions: database.CustomRolePermissions{}, + UserPermissions: database.CustomRolePermissions{}, + MemberPermissions: database.CustomRolePermissions{}, + IsSystem: true, + }) + require.NoError(t, err) + + // Use same params for all updates as we're only testing authz. + updateParams := database.UpdateCustomRoleParams{ + Name: role.Name, + OrganizationID: uuid.NullUUID{ + UUID: orgID, + Valid: true, + }, + DisplayName: "", + SitePermissions: database.CustomRolePermissions{}, + OrgPermissions: database.CustomRolePermissions{}, + UserPermissions: database.CustomRolePermissions{}, + MemberPermissions: database.CustomRolePermissions{}, + } + + t.Run("deny-no-system-perms", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + ctx = dbauthz.As(ctx, subjectNoSystemPerms) + + _, err := az.UpdateCustomRole(ctx, updateParams) + require.ErrorContains(t, err, "forbidden") + }) + + t.Run("deny-create-only", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + ctx = dbauthz.As(ctx, subjectWithSystemCreatePerms) + + _, err := az.UpdateCustomRole(ctx, updateParams) + require.ErrorContains(t, err, "forbidden") + }) + + t.Run("allow-update-only", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + ctx = dbauthz.As(ctx, subjectWithSystemUpdatePerms) + + _, err := az.UpdateCustomRole(ctx, updateParams) + require.NoError(t, err) + }) + }) + + t.Run("allow-member-permissions", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + ctx = dbauthz.As(ctx, subjectWithSystemCreatePerms) + + _, err := az.InsertCustomRole(ctx, database.InsertCustomRoleParams{ + Name: "test-system-role-member-perms", + OrganizationID: uuid.NullUUID{ + UUID: orgID, + Valid: true, + }, + SitePermissions: database.CustomRolePermissions{}, + OrgPermissions: database.CustomRolePermissions{}, + UserPermissions: database.CustomRolePermissions{}, + MemberPermissions: database.CustomRolePermissions{ + { + ResourceType: rbac.ResourceWorkspace.Type, + Action: policy.ActionRead, + }, + }, + IsSystem: true, + }) + require.NoError(t, err) + }) + + t.Run("allow-negative-permissions", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + ctx = dbauthz.As(ctx, subjectWithSystemCreatePerms) + + _, err := az.InsertCustomRole(ctx, database.InsertCustomRoleParams{ + Name: "test-system-role-negative", + OrganizationID: uuid.NullUUID{ + UUID: orgID, + Valid: true, + }, + SitePermissions: database.CustomRolePermissions{}, + OrgPermissions: database.CustomRolePermissions{ + { + Negate: true, + ResourceType: rbac.ResourceWorkspace.Type, + Action: policy.ActionShare, + }, + }, + UserPermissions: database.CustomRolePermissions{}, + MemberPermissions: database.CustomRolePermissions{}, + IsSystem: true, + }) + require.NoError(t, err) + }) +} diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 78b136d992..538a440a47 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1161,13 +1161,18 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID uuid.UUID, added, re for _, roleName := range grantedRoles { if _, isCustom := customRolesMap[roleName]; isCustom { - // 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. - if roleName.IsOrgRole() { - roleName = rbac.CustomOrganizationRole(roleName.OrganizationID) - } else { - roleName = rbac.CustomSiteRole() + // 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) { + // 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. + if roleName.IsOrgRole() { + roleName = rbac.CustomOrganizationRole(roleName.OrganizationID) + } else { + roleName = rbac.CustomSiteRole() + } } } @@ -1282,33 +1287,39 @@ func (q *querier) customRoleEscalationCheck(ctx context.Context, actor rbac.Subj // - Check custom roles are valid for their resource types + actions // - Check the actor can create the custom role // - Check the custom role does not grant perms the actor does not have -// - Prevent negative perms -// - Prevent roles with site and org permissions. -func (q *querier) customRoleCheck(ctx context.Context, role database.CustomRole) error { +// - Prevent negative perms for non-system roles +// - Prevent roles that have both organization scoped and non-organization scoped permissions +func (q *querier) customRoleCheck(ctx context.Context, role database.CustomRole, action policy.Action) error { act, ok := ActorFromContext(ctx) if !ok { return ErrNoActor } - // Org permissions require an org role - if role.OrganizationID.UUID == uuid.Nil && len(role.OrgPermissions) > 0 { - return xerrors.Errorf("organization permissions require specifying an organization id") + // Org and org member permissions require an org role. + if role.OrganizationID.UUID == uuid.Nil && (len(role.OrgPermissions) > 0 || len(role.MemberPermissions) > 0) { + return xerrors.Errorf("organization and member permissions require specifying an organization id") } - // Org roles can only specify org permissions + // Org roles can only specify org permissions; system roles can also specify orgMember ones. if role.OrganizationID.UUID != uuid.Nil && (len(role.SitePermissions) > 0 || len(role.UserPermissions) > 0) { return xerrors.Errorf("organization roles specify site or user permissions") } + // For now only system roles can specify orgMember permissions. + if !role.IsSystem && len(role.MemberPermissions) > 0 { + return xerrors.Errorf("non-system roles specify member permissions") + } + // The rbac.Role has a 'Valid()' function on it that will do a lot // of checks. rbacRole, err := rolestore.ConvertDBRole(database.CustomRole{ - Name: role.Name, - DisplayName: role.DisplayName, - SitePermissions: role.SitePermissions, - OrgPermissions: role.OrgPermissions, - UserPermissions: role.UserPermissions, - OrganizationID: role.OrganizationID, + Name: role.Name, + DisplayName: role.DisplayName, + SitePermissions: role.SitePermissions, + OrgPermissions: role.OrgPermissions, + UserPermissions: role.UserPermissions, + MemberPermissions: role.MemberPermissions, + OrganizationID: role.OrganizationID, }) if err != nil { return xerrors.Errorf("invalid args: %w", err) @@ -1333,6 +1344,16 @@ func (q *querier) customRoleCheck(ctx context.Context, role database.CustomRole) return xerrors.Errorf("invalid custom role, cannot assign permissions to more than 1 org at a time") } + // System roles are managed internally and may include permissions + // (including negative ones) that user-facing custom role APIs + // should reject. Still validate that the role shape and perms + // are internally consistent via rbacRole.Valid() above. + if role.IsSystem { + // Defensive programming: the caller should have checked that + // the action is authorized, but we double-check. + return q.authorizeContext(ctx, action, rbac.ResourceSystem) + } + // Prevent escalation for _, sitePerm := range rbacRole.Site { err := q.customRoleEscalationCheck(ctx, act, sitePerm, rbac.Object{Type: sitePerm.ResourceType}) @@ -4132,21 +4153,33 @@ func (q *querier) InsertCustomRole(ctx context.Context, arg database.InsertCusto if !arg.OrganizationID.Valid || arg.OrganizationID.UUID == uuid.Nil { return database.CustomRole{}, NotAuthorizedError{Err: xerrors.New("custom roles must belong to an organization")} } - if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil { + + rbacObj := rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID) + + if err := q.authorizeContext(ctx, policy.ActionCreate, rbacObj); err != nil { return database.CustomRole{}, err } + if arg.IsSystem { + err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem) + if err != nil { + return database.CustomRole{}, err + } + } + if err := q.customRoleCheck(ctx, database.CustomRole{ - Name: arg.Name, - DisplayName: arg.DisplayName, - SitePermissions: arg.SitePermissions, - OrgPermissions: arg.OrgPermissions, - UserPermissions: arg.UserPermissions, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - OrganizationID: arg.OrganizationID, - ID: uuid.New(), - }); err != nil { + Name: arg.Name, + DisplayName: arg.DisplayName, + SitePermissions: arg.SitePermissions, + OrgPermissions: arg.OrgPermissions, + UserPermissions: arg.UserPermissions, + MemberPermissions: arg.MemberPermissions, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + OrganizationID: arg.OrganizationID, + ID: uuid.New(), + IsSystem: arg.IsSystem, + }, policy.ActionCreate); err != nil { return database.CustomRole{}, err } return q.db.InsertCustomRole(ctx, arg) @@ -4886,21 +4919,48 @@ func (q *querier) UpdateCustomRole(ctx context.Context, arg database.UpdateCusto if !arg.OrganizationID.Valid || arg.OrganizationID.UUID == uuid.Nil { return database.CustomRole{}, NotAuthorizedError{Err: xerrors.New("custom roles must belong to an organization")} } - if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil { + + rbacObj := rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID) + + if err := q.authorizeContext(ctx, policy.ActionUpdate, rbacObj); err != nil { return database.CustomRole{}, err } + existing, err := database.ExpectOne(q.db.CustomRoles(ctx, database.CustomRolesParams{ + LookupRoles: []database.NameOrganizationPair{ + { + Name: arg.Name, + OrganizationID: arg.OrganizationID.UUID, + }, + }, + ExcludeOrgRoles: false, + OrganizationID: uuid.Nil, + IncludeSystemRoles: true, + })) + if err != nil { + return database.CustomRole{}, err + } + + if existing.IsSystem { + err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem) + if err != nil { + return database.CustomRole{}, err + } + } + if err := q.customRoleCheck(ctx, database.CustomRole{ - Name: arg.Name, - DisplayName: arg.DisplayName, - SitePermissions: arg.SitePermissions, - OrgPermissions: arg.OrgPermissions, - UserPermissions: arg.UserPermissions, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - OrganizationID: arg.OrganizationID, - ID: uuid.New(), - }); err != nil { + Name: arg.Name, + DisplayName: arg.DisplayName, + SitePermissions: arg.SitePermissions, + OrgPermissions: arg.OrgPermissions, + UserPermissions: arg.UserPermissions, + MemberPermissions: arg.MemberPermissions, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + OrganizationID: arg.OrganizationID, + ID: uuid.New(), + IsSystem: existing.IsSystem, + }, policy.ActionUpdate); err != nil { return database.CustomRole{}, err } return q.db.UpdateCustomRole(ctx, arg) diff --git a/coderd/database/dbauthz/groupsauth_test.go b/coderd/database/dbauthz/groupsauth_test.go index ae5cf650fd..0f60bb33c3 100644 --- a/coderd/database/dbauthz/groupsauth_test.go +++ b/coderd/database/dbauthz/groupsauth_test.go @@ -15,6 +15,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/coderd/rbac/rolestore" ) // nolint:tparallel @@ -109,8 +110,12 @@ func TestGroupsAuth(t *testing.T) { { Name: "GroupMember", Subject: rbac.Subject{ - ID: users[0].ID.String(), - Roles: rbac.Roles(must(rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(org.ID)}.Expand())), + ID: users[0].ID.String(), + Roles: must(rolestore.Expand( + context.Background(), + store, + []rbac.RoleIdentifier{rbac.RoleMember(), rbac.ScopedRoleOrgMember(org.ID)}, + )), Groups: []string{ group.ID.String(), }, diff --git a/coderd/database/dbauthz/setup_test.go b/coderd/database/dbauthz/setup_test.go index aaead40d00..d4ef606010 100644 --- a/coderd/database/dbauthz/setup_test.go +++ b/coderd/database/dbauthz/setup_test.go @@ -105,12 +105,51 @@ func (s *MethodTestSuite) TearDownSuite() { var testActorID = uuid.New() +type includeSystemRolesMatcher struct{} + +func (includeSystemRolesMatcher) Matches(x any) bool { + p, ok := x.(database.CustomRolesParams) + if !ok { + return false + } + return p.IncludeSystemRoles +} + +func (includeSystemRolesMatcher) String() string { + return "CustomRolesParams with IncludeSystemRoles=true" +} + // Mocked runs a subtest with a mocked database. Removing the overhead of a real // postgres database resulting in much faster tests. func (s *MethodTestSuite) Mocked(testCaseF func(dmb *dbmock.MockStore, faker *gofakeit.Faker, check *expects)) func() { t := s.T() mDB := dbmock.NewMockStore(gomock.NewController(t)) mDB.EXPECT().Wrappers().Return([]string{}).AnyTimes() + // dbauthz now expands DB-backed system roles (e.g. organization-member) + // during role-assignment validation, which triggers a CustomRoles lookup + // with IncludeSystemRoles=true. + mDB.EXPECT().CustomRoles(gomock.Any(), includeSystemRolesMatcher{}).DoAndReturn(func(_ context.Context, arg database.CustomRolesParams) ([]database.CustomRole, error) { + if len(arg.LookupRoles) == 0 { + return []database.CustomRole{}, nil + } + + out := make([]database.CustomRole, 0, len(arg.LookupRoles)) + + for _, pair := range arg.LookupRoles { + // Minimal set of fields that the tested code uses. + out = append(out, database.CustomRole{ + Name: pair.Name, + OrganizationID: uuid.NullUUID{ + UUID: pair.OrganizationID, + Valid: pair.OrganizationID != uuid.Nil, + }, + IsSystem: rbac.SystemRoleName(pair.Name), + ID: uuid.New(), + }) + } + + return out, nil + }).AnyTimes() // Use a constant seed to prevent flakes from random data generation. faker := gofakeit.New(0) diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 5906cc308b..f8f836b25f 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -29,6 +29,7 @@ import ( "github.com/coder/coder/v2/coderd/database/pubsub" "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/taskname" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/cryptorand" @@ -41,8 +42,16 @@ import ( // genCtx is to give all generator functions permission if the db is a dbauthz db. var genCtx = dbauthz.As(context.Background(), rbac.Subject{ - ID: "owner", - Roles: rbac.Roles(must(rbac.RoleIdentifiers{rbac.RoleOwner()}.Expand())), + ID: "owner", + Roles: rbac.Roles(append( + must(rbac.RoleIdentifiers{rbac.RoleOwner()}.Expand()), + rbac.Role{ + Identifier: rbac.RoleIdentifier{Name: "dbgen-workspace-sharer"}, + Site: rbac.Permissions(map[string][]policy.Action{ + rbac.ResourceWorkspace.Type: {policy.ActionShare}, + }), + }, + )), Groups: []string{}, Scope: rbac.ExpandableScope(rbac.ScopeAll), }) @@ -639,6 +648,36 @@ func Organization(t testing.TB, db database.Store, orig database.Organization) d UpdatedAt: takeFirst(orig.UpdatedAt, dbtime.Now()), }) 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 + // 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) + } + require.NoError(t, err, "reconcile organization-member role") + return org } @@ -1395,12 +1434,14 @@ func WorkspaceAgentVolumeResourceMonitor(t testing.TB, db database.Store, seed d func CustomRole(t testing.TB, db database.Store, seed database.CustomRole) database.CustomRole { role, err := db.InsertCustomRole(genCtx, database.InsertCustomRoleParams{ - Name: takeFirst(seed.Name, strings.ToLower(testutil.GetRandomName(t))), - DisplayName: testutil.GetRandomName(t), - OrganizationID: seed.OrganizationID, - SitePermissions: takeFirstSlice(seed.SitePermissions, []database.CustomRolePermission{}), - OrgPermissions: takeFirstSlice(seed.SitePermissions, []database.CustomRolePermission{}), - UserPermissions: takeFirstSlice(seed.SitePermissions, []database.CustomRolePermission{}), + Name: takeFirst(seed.Name, strings.ToLower(testutil.GetRandomName(t))), + DisplayName: testutil.GetRandomName(t), + OrganizationID: seed.OrganizationID, + SitePermissions: takeFirstSlice(seed.SitePermissions, []database.CustomRolePermission{}), + OrgPermissions: takeFirstSlice(seed.SitePermissions, []database.CustomRolePermission{}), + UserPermissions: takeFirstSlice(seed.SitePermissions, []database.CustomRolePermission{}), + MemberPermissions: takeFirstSlice(seed.MemberPermissions, []database.CustomRolePermission{}), + IsSystem: seed.IsSystem, }) require.NoError(t, err, "insert custom role") return role diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index c8e59cc452..ae4dcd728e 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -746,6 +746,37 @@ BEGIN END; $$; +CREATE FUNCTION insert_org_member_system_role() RETURNS trigger + LANGUAGE plpgsql + 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; +$$; + CREATE FUNCTION insert_user_links_fail_if_user_deleted() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -1203,6 +1234,8 @@ CREATE TABLE custom_roles ( updated_at timestamp with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, organization_id uuid, id uuid DEFAULT gen_random_uuid() NOT NULL, + is_system boolean DEFAULT false NOT NULL, + member_permissions jsonb DEFAULT '[]'::jsonb NOT NULL, CONSTRAINT organization_id_not_zero CHECK ((organization_id <> '00000000-0000-0000-0000-000000000000'::uuid)) ); @@ -1212,6 +1245,8 @@ COMMENT ON COLUMN custom_roles.organization_id IS 'Roles can optionally be scope COMMENT ON COLUMN custom_roles.id IS 'Custom roles ID is used purely for auditing purposes. Name is a better unique identifier.'; +COMMENT ON COLUMN custom_roles.is_system IS 'System roles are managed by Coder and cannot be modified or deleted by users.'; + CREATE TABLE dbcrypt_keys ( number integer NOT NULL, active_key_digest text, @@ -1595,7 +1630,8 @@ CREATE TABLE organizations ( is_default boolean DEFAULT false NOT NULL, display_name text NOT NULL, icon text DEFAULT ''::text NOT NULL, - deleted boolean DEFAULT false NOT NULL + deleted boolean DEFAULT false NOT NULL, + workspace_sharing_disabled boolean DEFAULT false NOT NULL ); CREATE TABLE parameter_schemas ( @@ -3546,6 +3582,8 @@ 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_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(); CREATE TRIGGER trigger_update_users AFTER INSERT OR UPDATE ON users FOR EACH ROW WHEN ((new.deleted = true)) EXECUTE FUNCTION delete_deleted_user_resources(); diff --git a/coderd/database/lock.go b/coderd/database/lock.go index e5091cdfd2..16aff01690 100644 --- a/coderd/database/lock.go +++ b/coderd/database/lock.go @@ -13,6 +13,7 @@ const ( LockIDNotificationsReportGenerator LockIDCryptoKeyRotation LockIDReconcilePrebuilds + LockIDReconcileSystemRoles ) // GenLockID generates a unique and consistent lock ID from a given string. diff --git a/coderd/database/migrations/000406_add_system_role_support.down.sql b/coderd/database/migrations/000406_add_system_role_support.down.sql new file mode 100644 index 0000000000..8198c7b1c7 --- /dev/null +++ b/coderd/database/migrations/000406_add_system_role_support.down.sql @@ -0,0 +1,3 @@ +ALTER TABLE custom_roles DROP COLUMN IF EXISTS member_permissions; + +ALTER TABLE custom_roles DROP COLUMN IF EXISTS is_system; diff --git a/coderd/database/migrations/000406_add_system_role_support.up.sql b/coderd/database/migrations/000406_add_system_role_support.up.sql new file mode 100644 index 0000000000..9fbc99afb8 --- /dev/null +++ b/coderd/database/migrations/000406_add_system_role_support.up.sql @@ -0,0 +1,10 @@ +-- Add is_system column to identify system-managed roles. +ALTER TABLE custom_roles + ADD COLUMN is_system boolean NOT NULL DEFAULT false; + +-- Add member_permissions column for member-scoped permissions within an organization. +ALTER TABLE custom_roles + ADD COLUMN member_permissions jsonb NOT NULL DEFAULT '[]'::jsonb; + +COMMENT ON COLUMN custom_roles.is_system IS + 'System roles are managed by Coder and cannot be modified or deleted by users.'; diff --git a/coderd/database/migrations/000407_add_workspace_sharing_disabled.down.sql b/coderd/database/migrations/000407_add_workspace_sharing_disabled.down.sql new file mode 100644 index 0000000000..cc35c25e86 --- /dev/null +++ b/coderd/database/migrations/000407_add_workspace_sharing_disabled.down.sql @@ -0,0 +1 @@ +ALTER TABLE organizations DROP COLUMN IF EXISTS workspace_sharing_disabled; diff --git a/coderd/database/migrations/000407_add_workspace_sharing_disabled.up.sql b/coderd/database/migrations/000407_add_workspace_sharing_disabled.up.sql new file mode 100644 index 0000000000..a5563107d6 --- /dev/null +++ b/coderd/database/migrations/000407_add_workspace_sharing_disabled.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE organizations + ADD COLUMN workspace_sharing_disabled boolean NOT NULL DEFAULT false; diff --git a/coderd/database/migrations/000408_create_org_member_system_roles.down.sql b/coderd/database/migrations/000408_create_org_member_system_roles.down.sql new file mode 100644 index 0000000000..9c352650b7 --- /dev/null +++ b/coderd/database/migrations/000408_create_org_member_system_roles.down.sql @@ -0,0 +1,6 @@ +-- Drop the trigger and function created by the up migration. +DROP TRIGGER IF EXISTS trigger_insert_org_member_system_role ON organizations; +DROP FUNCTION IF EXISTS insert_org_member_system_role; + +-- Remove organization-member system roles created by the up migration. +DELETE FROM custom_roles WHERE name = 'organization-member' AND is_system = true; diff --git a/coderd/database/migrations/000408_create_org_member_system_roles.up.sql b/coderd/database/migrations/000408_create_org_member_system_roles.up.sql new file mode 100644 index 0000000000..0aae03f9e0 --- /dev/null +++ b/coderd/database/migrations/000408_create_org_member_system_roles.up.sql @@ -0,0 +1,85 @@ +-- Create placeholder organization-member system roles for existing +-- organizations. Also add a trigger that creates the placeholder role +-- when an organization is created. Permissions will be empty until +-- populated by the reconciliation routine. +-- +-- Note: why do all this in the database (as opposed to coderd)? Less +-- room for race conditions. If the role doesn't exist when coderd +-- expects it, the only correct option is to panic. On the other hand, +-- a placeholder role with empty permissions is harmless and the +-- reconciliation process is idempotent. + +-- 'organization-member' is reserved and blocked from being created in +-- coderd, but let's do a delete just in case. +DELETE FROM custom_roles WHERE name = 'organization-member'; + +-- Create roles for the existing organizations. +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-member', -- reserved role name, so it doesn't exist in DB yet + '', + id, + '[]'::jsonb, + '[]'::jsonb, + '[]'::jsonb, + '[]'::jsonb, + true, + NOW(), + NOW() +FROM + organizations +WHERE + NOT EXISTS ( + SELECT 1 + FROM custom_roles + WHERE + custom_roles.name = 'organization-member' + AND custom_roles.organization_id = organizations.id + ); + +-- When we insert a new organization, we also want to create a +-- placeholder org-member system role for it. +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/models.go b/coderd/database/models.go index 7fde80a86d..2f0a14b2e5 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -3741,6 +3741,9 @@ type CustomRole struct { OrganizationID uuid.NullUUID `db:"organization_id" json:"organization_id"` // Custom roles ID is used purely for auditing purposes. Name is a better unique identifier. ID uuid.UUID `db:"id" json:"id"` + // System roles are managed by Coder and cannot be modified or deleted by users. + IsSystem bool `db:"is_system" json:"is_system"` + MemberPermissions CustomRolePermissions `db:"member_permissions" json:"member_permissions"` } // A table used to store the keys used to encrypt the database. @@ -4006,15 +4009,16 @@ 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"` + 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"` } type OrganizationMember struct { diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 9086a032dc..ac404541f1 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -2228,6 +2228,82 @@ func TestReadCustomRoles(t *testing.T) { } } +func TestDeleteCustomRoleDoesNotDeleteSystemRole(t *testing.T) { + t.Parallel() + + db, _ := dbtestutil.NewDB(t) + org := dbgen.Organization(t, db, database.Organization{}) + + ctx := testutil.Context(t, testutil.WaitShort) + + systemRole, err := db.InsertCustomRole(ctx, database.InsertCustomRoleParams{ + Name: "test-system-role", + DisplayName: "", + OrganizationID: uuid.NullUUID{ + UUID: org.ID, + Valid: true, + }, + SitePermissions: database.CustomRolePermissions{}, + OrgPermissions: database.CustomRolePermissions{}, + UserPermissions: database.CustomRolePermissions{}, + MemberPermissions: database.CustomRolePermissions{}, + IsSystem: true, + }) + require.NoError(t, err) + + nonSystemRole, err := db.InsertCustomRole(ctx, database.InsertCustomRoleParams{ + Name: "test-custom-role", + DisplayName: "", + OrganizationID: uuid.NullUUID{ + UUID: org.ID, + Valid: true, + }, + SitePermissions: database.CustomRolePermissions{}, + OrgPermissions: database.CustomRolePermissions{}, + UserPermissions: database.CustomRolePermissions{}, + MemberPermissions: database.CustomRolePermissions{}, + IsSystem: false, + }) + require.NoError(t, err) + + err = db.DeleteCustomRole(ctx, database.DeleteCustomRoleParams{ + Name: systemRole.Name, + OrganizationID: uuid.NullUUID{ + UUID: org.ID, + Valid: true, + }, + }) + require.NoError(t, err) + + err = db.DeleteCustomRole(ctx, database.DeleteCustomRoleParams{ + Name: nonSystemRole.Name, + OrganizationID: uuid.NullUUID{ + UUID: org.ID, + Valid: true, + }, + }) + require.NoError(t, err) + + roles, err := db.CustomRoles(ctx, database.CustomRolesParams{ + LookupRoles: []database.NameOrganizationPair{ + { + Name: systemRole.Name, + OrganizationID: org.ID, + }, + { + Name: nonSystemRole.Name, + OrganizationID: org.ID, + }, + }, + IncludeSystemRoles: true, + }) + require.NoError(t, err) + + require.Len(t, roles, 1) + require.Equal(t, systemRole.Name, roles[0].Name) + require.True(t, roles[0].IsSystem) +} + func TestAuthorizedAuditLogs(t *testing.T) { t.Parallel() diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 5b9346cb60..f08e50b333 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -7808,7 +7808,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 + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, workspace_sharing_disabled FROM organizations WHERE @@ -7830,13 +7830,14 @@ func (q *sqlQuerier) GetDefaultOrganization(ctx context.Context) (Organization, &i.DisplayName, &i.Icon, &i.Deleted, + &i.WorkspaceSharingDisabled, ) return i, err } const getOrganizationByID = `-- name: GetOrganizationByID :one SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon, deleted + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, workspace_sharing_disabled FROM organizations WHERE @@ -7856,13 +7857,14 @@ func (q *sqlQuerier) GetOrganizationByID(ctx context.Context, id uuid.UUID) (Org &i.DisplayName, &i.Icon, &i.Deleted, + &i.WorkspaceSharingDisabled, ) return i, err } const getOrganizationByName = `-- name: GetOrganizationByName :one SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon, deleted + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, workspace_sharing_disabled FROM organizations WHERE @@ -7891,6 +7893,7 @@ func (q *sqlQuerier) GetOrganizationByName(ctx context.Context, arg GetOrganizat &i.DisplayName, &i.Icon, &i.Deleted, + &i.WorkspaceSharingDisabled, ) return i, err } @@ -7961,7 +7964,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 + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, workspace_sharing_disabled FROM organizations WHERE @@ -8005,6 +8008,7 @@ func (q *sqlQuerier) GetOrganizations(ctx context.Context, arg GetOrganizationsP &i.DisplayName, &i.Icon, &i.Deleted, + &i.WorkspaceSharingDisabled, ); err != nil { return nil, err } @@ -8021,7 +8025,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 + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, workspace_sharing_disabled FROM organizations WHERE @@ -8066,6 +8070,7 @@ func (q *sqlQuerier) GetOrganizationsByUserID(ctx context.Context, arg GetOrgani &i.DisplayName, &i.Icon, &i.Deleted, + &i.WorkspaceSharingDisabled, ); err != nil { return nil, err } @@ -8085,7 +8090,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 + ($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 ` type InsertOrganizationParams struct { @@ -8119,6 +8124,7 @@ func (q *sqlQuerier) InsertOrganization(ctx context.Context, arg InsertOrganizat &i.DisplayName, &i.Icon, &i.Deleted, + &i.WorkspaceSharingDisabled, ) return i, err } @@ -8134,7 +8140,7 @@ SET icon = $5 WHERE id = $6 -RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted +RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted, workspace_sharing_disabled ` type UpdateOrganizationParams struct { @@ -8166,6 +8172,7 @@ func (q *sqlQuerier) UpdateOrganization(ctx context.Context, arg UpdateOrganizat &i.DisplayName, &i.Icon, &i.Deleted, + &i.WorkspaceSharingDisabled, ) return i, err } @@ -11927,7 +11934,7 @@ func (q *sqlQuerier) UpdateReplica(ctx context.Context, arg UpdateReplicaParams) const customRoles = `-- name: CustomRoles :many SELECT - name, display_name, site_permissions, org_permissions, user_permissions, created_at, updated_at, organization_id, id + name, display_name, site_permissions, org_permissions, user_permissions, created_at, updated_at, organization_id, id, is_system, member_permissions FROM custom_roles WHERE @@ -11950,16 +11957,30 @@ WHERE organization_id = $3 ELSE true END + -- Filter system roles. By default, system roles are excluded. + -- System roles are managed by Coder and should be hidden from user-facing APIs. + -- The authorization system uses @include_system_roles = true to load them. + AND CASE WHEN $4 :: boolean THEN + true + ELSE + is_system = false + END ` type CustomRolesParams struct { - LookupRoles []NameOrganizationPair `db:"lookup_roles" json:"lookup_roles"` - ExcludeOrgRoles bool `db:"exclude_org_roles" json:"exclude_org_roles"` - OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + LookupRoles []NameOrganizationPair `db:"lookup_roles" json:"lookup_roles"` + ExcludeOrgRoles bool `db:"exclude_org_roles" json:"exclude_org_roles"` + OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + IncludeSystemRoles bool `db:"include_system_roles" json:"include_system_roles"` } func (q *sqlQuerier) CustomRoles(ctx context.Context, arg CustomRolesParams) ([]CustomRole, error) { - rows, err := q.db.QueryContext(ctx, customRoles, pq.Array(arg.LookupRoles), arg.ExcludeOrgRoles, arg.OrganizationID) + rows, err := q.db.QueryContext(ctx, customRoles, + pq.Array(arg.LookupRoles), + arg.ExcludeOrgRoles, + arg.OrganizationID, + arg.IncludeSystemRoles, + ) if err != nil { return nil, err } @@ -11977,6 +11998,8 @@ func (q *sqlQuerier) CustomRoles(ctx context.Context, arg CustomRolesParams) ([] &i.UpdatedAt, &i.OrganizationID, &i.ID, + &i.IsSystem, + &i.MemberPermissions, ); err != nil { return nil, err } @@ -11997,6 +12020,9 @@ DELETE FROM WHERE name = lower($1) AND organization_id = $2 + -- Prevents accidental deletion of system roles even if the API + -- layer check is bypassed due to a bug. + AND is_system = false ` type DeleteCustomRoleParams struct { @@ -12018,6 +12044,8 @@ INSERT INTO site_permissions, org_permissions, user_permissions, + member_permissions, + is_system, created_at, updated_at ) @@ -12029,19 +12057,23 @@ VALUES ( $4, $5, $6, + $7, + $8, now(), now() ) -RETURNING name, display_name, site_permissions, org_permissions, user_permissions, created_at, updated_at, organization_id, id +RETURNING name, display_name, site_permissions, org_permissions, user_permissions, created_at, updated_at, organization_id, id, is_system, member_permissions ` type InsertCustomRoleParams struct { - Name string `db:"name" json:"name"` - DisplayName string `db:"display_name" json:"display_name"` - OrganizationID uuid.NullUUID `db:"organization_id" json:"organization_id"` - SitePermissions CustomRolePermissions `db:"site_permissions" json:"site_permissions"` - OrgPermissions CustomRolePermissions `db:"org_permissions" json:"org_permissions"` - UserPermissions CustomRolePermissions `db:"user_permissions" json:"user_permissions"` + Name string `db:"name" json:"name"` + DisplayName string `db:"display_name" json:"display_name"` + OrganizationID uuid.NullUUID `db:"organization_id" json:"organization_id"` + SitePermissions CustomRolePermissions `db:"site_permissions" json:"site_permissions"` + OrgPermissions CustomRolePermissions `db:"org_permissions" json:"org_permissions"` + UserPermissions CustomRolePermissions `db:"user_permissions" json:"user_permissions"` + MemberPermissions CustomRolePermissions `db:"member_permissions" json:"member_permissions"` + IsSystem bool `db:"is_system" json:"is_system"` } func (q *sqlQuerier) InsertCustomRole(ctx context.Context, arg InsertCustomRoleParams) (CustomRole, error) { @@ -12052,6 +12084,8 @@ func (q *sqlQuerier) InsertCustomRole(ctx context.Context, arg InsertCustomRoleP arg.SitePermissions, arg.OrgPermissions, arg.UserPermissions, + arg.MemberPermissions, + arg.IsSystem, ) var i CustomRole err := row.Scan( @@ -12064,6 +12098,8 @@ func (q *sqlQuerier) InsertCustomRole(ctx context.Context, arg InsertCustomRoleP &i.UpdatedAt, &i.OrganizationID, &i.ID, + &i.IsSystem, + &i.MemberPermissions, ) return i, err } @@ -12076,20 +12112,22 @@ SET site_permissions = $2, org_permissions = $3, user_permissions = $4, + member_permissions = $5, updated_at = now() WHERE - name = lower($5) - AND organization_id = $6 -RETURNING name, display_name, site_permissions, org_permissions, user_permissions, created_at, updated_at, organization_id, id + name = lower($6) + AND organization_id = $7 +RETURNING name, display_name, site_permissions, org_permissions, user_permissions, created_at, updated_at, organization_id, id, is_system, member_permissions ` type UpdateCustomRoleParams struct { - DisplayName string `db:"display_name" json:"display_name"` - SitePermissions CustomRolePermissions `db:"site_permissions" json:"site_permissions"` - OrgPermissions CustomRolePermissions `db:"org_permissions" json:"org_permissions"` - UserPermissions CustomRolePermissions `db:"user_permissions" json:"user_permissions"` - Name string `db:"name" json:"name"` - OrganizationID uuid.NullUUID `db:"organization_id" json:"organization_id"` + DisplayName string `db:"display_name" json:"display_name"` + SitePermissions CustomRolePermissions `db:"site_permissions" json:"site_permissions"` + OrgPermissions CustomRolePermissions `db:"org_permissions" json:"org_permissions"` + UserPermissions CustomRolePermissions `db:"user_permissions" json:"user_permissions"` + MemberPermissions CustomRolePermissions `db:"member_permissions" json:"member_permissions"` + Name string `db:"name" json:"name"` + OrganizationID uuid.NullUUID `db:"organization_id" json:"organization_id"` } func (q *sqlQuerier) UpdateCustomRole(ctx context.Context, arg UpdateCustomRoleParams) (CustomRole, error) { @@ -12098,6 +12136,7 @@ func (q *sqlQuerier) UpdateCustomRole(ctx context.Context, arg UpdateCustomRoleP arg.SitePermissions, arg.OrgPermissions, arg.UserPermissions, + arg.MemberPermissions, arg.Name, arg.OrganizationID, ) @@ -12112,6 +12151,8 @@ func (q *sqlQuerier) UpdateCustomRole(ctx context.Context, arg UpdateCustomRoleP &i.UpdatedAt, &i.OrganizationID, &i.ID, + &i.IsSystem, + &i.MemberPermissions, ) return i, err } diff --git a/coderd/database/queries/roles.sql b/coderd/database/queries/roles.sql index ee5d35d91a..8a2ed0cccc 100644 --- a/coderd/database/queries/roles.sql +++ b/coderd/database/queries/roles.sql @@ -23,6 +23,14 @@ WHERE organization_id = @organization_id ELSE true END + -- Filter system roles. By default, system roles are excluded. + -- System roles are managed by Coder and should be hidden from user-facing APIs. + -- The authorization system uses @include_system_roles = true to load them. + AND CASE WHEN @include_system_roles :: boolean THEN + true + ELSE + is_system = false + END ; -- name: DeleteCustomRole :exec @@ -31,6 +39,9 @@ DELETE FROM WHERE name = lower(@name) AND organization_id = @organization_id + -- Prevents accidental deletion of system roles even if the API + -- layer check is bypassed due to a bug. + AND is_system = false ; -- name: InsertCustomRole :one @@ -42,6 +53,8 @@ INSERT INTO site_permissions, org_permissions, user_permissions, + member_permissions, + is_system, created_at, updated_at ) @@ -53,6 +66,8 @@ VALUES ( @site_permissions, @org_permissions, @user_permissions, + @member_permissions, + @is_system, now(), now() ) @@ -66,6 +81,7 @@ SET site_permissions = @site_permissions, org_permissions = @org_permissions, user_permissions = @user_permissions, + member_permissions = @member_permissions, updated_at = now() WHERE name = lower(@name) diff --git a/coderd/database/sqlc.yaml b/coderd/database/sqlc.yaml index a7a821b758..d6a2269845 100644 --- a/coderd/database/sqlc.yaml +++ b/coderd/database/sqlc.yaml @@ -53,6 +53,9 @@ sql: - column: "custom_roles.user_permissions" go_type: type: "CustomRolePermissions" + - column: "custom_roles.member_permissions" + go_type: + type: "CustomRolePermissions" - column: "provisioner_daemons.tags" go_type: type: "StringMap" diff --git a/coderd/externalauth_test.go b/coderd/externalauth_test.go index 3eb7c2a002..4aa327313b 100644 --- a/coderd/externalauth_test.go +++ b/coderd/externalauth_test.go @@ -307,13 +307,13 @@ func TestExternalAuthManagement(t *testing.T) { gitlab.ExternalLogin(t, client) links, err := db.GetExternalAuthLinksByUserID( - dbauthz.As(ctx, coderdtest.AuthzUserSubject(user, ownerUser.OrganizationID)), user.ID) + dbauthz.As(ctx, coderdtest.AuthzUserSubject(user)), user.ID) require.NoError(t, err) require.Len(t, links, 2) // Expire the links for _, l := range links { - _, err := db.UpdateExternalAuthLink(dbauthz.As(ctx, coderdtest.AuthzUserSubject(user, ownerUser.OrganizationID)), database.UpdateExternalAuthLinkParams{ + _, err := db.UpdateExternalAuthLink(dbauthz.As(ctx, coderdtest.AuthzUserSubject(user)), database.UpdateExternalAuthLinkParams{ ProviderID: l.ProviderID, UserID: l.UserID, UpdatedAt: dbtime.Now(), diff --git a/coderd/members.go b/coderd/members.go index dd9ce73bba..c27245f98a 100644 --- a/coderd/members.go +++ b/coderd/members.go @@ -393,9 +393,10 @@ func convertOrganizationMembers(ctx context.Context, db database.Store, mems []d } customRoles, err := db.CustomRoles(ctx, database.CustomRolesParams{ - LookupRoles: roleLookup, - ExcludeOrgRoles: false, - OrganizationID: uuid.Nil, + LookupRoles: roleLookup, + ExcludeOrgRoles: false, + OrganizationID: uuid.Nil, + IncludeSystemRoles: false, }) if err != nil { // We are missing the display names, but that is not absolutely required. So just diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index c409655d0c..8c48e7acd6 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -168,7 +168,7 @@ func TestFilter(t *testing.T) { Name: "Admin", Actor: Subject{ ID: userIDs[0].String(), - Roles: RoleIdentifiers{ScopedRoleOrgMember(orgIDs[0]), RoleAuditor(), RoleOwner(), RoleMember()}, + Roles: RoleIdentifiers{RoleAuditor(), RoleOwner(), RoleMember()}, }, ObjectType: ResourceWorkspace.Type, Action: policy.ActionRead, @@ -177,7 +177,7 @@ func TestFilter(t *testing.T) { Name: "OrgAdmin", Actor: Subject{ ID: userIDs[0].String(), - Roles: RoleIdentifiers{ScopedRoleOrgMember(orgIDs[0]), ScopedRoleOrgAdmin(orgIDs[0]), RoleMember()}, + Roles: RoleIdentifiers{ScopedRoleOrgAdmin(orgIDs[0]), RoleMember()}, }, ObjectType: ResourceWorkspace.Type, Action: policy.ActionRead, @@ -186,7 +186,7 @@ func TestFilter(t *testing.T) { Name: "OrgMember", Actor: Subject{ ID: userIDs[0].String(), - Roles: RoleIdentifiers{ScopedRoleOrgMember(orgIDs[0]), ScopedRoleOrgMember(orgIDs[1]), RoleMember()}, + Roles: RoleIdentifiers{RoleMember()}, }, ObjectType: ResourceWorkspace.Type, Action: policy.ActionRead, @@ -196,11 +196,9 @@ func TestFilter(t *testing.T) { Actor: Subject{ ID: userIDs[0].String(), Roles: RoleIdentifiers{ - ScopedRoleOrgMember(orgIDs[0]), ScopedRoleOrgAdmin(orgIDs[0]), - ScopedRoleOrgMember(orgIDs[1]), ScopedRoleOrgAdmin(orgIDs[1]), - ScopedRoleOrgMember(orgIDs[2]), ScopedRoleOrgAdmin(orgIDs[2]), - ScopedRoleOrgMember(orgIDs[4]), - ScopedRoleOrgMember(orgIDs[5]), + ScopedRoleOrgAdmin(orgIDs[0]), + ScopedRoleOrgAdmin(orgIDs[1]), + ScopedRoleOrgAdmin(orgIDs[2]), RoleMember(), }, }, @@ -221,10 +219,6 @@ func TestFilter(t *testing.T) { Actor: Subject{ ID: userIDs[0].String(), Roles: RoleIdentifiers{ - ScopedRoleOrgMember(orgIDs[0]), - ScopedRoleOrgMember(orgIDs[1]), - ScopedRoleOrgMember(orgIDs[2]), - ScopedRoleOrgMember(orgIDs[3]), RoleMember(), }, }, @@ -235,7 +229,7 @@ func TestFilter(t *testing.T) { Name: "ScopeApplicationConnect", Actor: Subject{ ID: userIDs[0].String(), - Roles: RoleIdentifiers{ScopedRoleOrgMember(orgIDs[0]), RoleAuditor(), RoleOwner(), RoleMember()}, + Roles: RoleIdentifiers{RoleAuditor(), RoleOwner(), RoleMember()}, }, ObjectType: ResourceWorkspace.Type, Action: policy.ActionRead, @@ -312,7 +306,7 @@ func TestAuthorizeDomain(t *testing.T) { Groups: []string{allUsersGroup}, Roles: Roles{ must(RoleByName(RoleMember())), - must(RoleByName(ScopedRoleOrgMember(defOrg))), + orgMemberRole(defOrg), }, } @@ -456,7 +450,7 @@ func TestAuthorizeDomain(t *testing.T) { Scope: must(ExpandScope(ScopeAll)), Roles: Roles{ must(RoleByName(ScopedRoleOrgAdmin(defOrg))), - must(RoleByName(ScopedRoleOrgMember(defOrg))), + orgMemberRole(defOrg), must(RoleByName(RoleMember())), }, } @@ -502,39 +496,40 @@ func TestAuthorizeDomain(t *testing.T) { }, } + siteAdminWorkspaceActions := slice.Omit(ResourceWorkspace.AvailableActions(), policy.ActionShare) testAuthorize(t, "SiteAdmin", user, []authTestCase{ // Similar to an orphaned user, but has site level perms {resource: ResourceTemplate.AnyOrganization(), actions: []policy.Action{policy.ActionCreate}, allow: true}, // Org + me - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), actions: ResourceWorkspace.AvailableActions(), allow: true}, - {resource: ResourceWorkspace.InOrg(defOrg), actions: ResourceWorkspace.AvailableActions(), allow: true}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), actions: siteAdminWorkspaceActions, allow: true}, + {resource: ResourceWorkspace.InOrg(defOrg), actions: siteAdminWorkspaceActions, allow: true}, - {resource: ResourceWorkspace.WithOwner(user.ID), actions: ResourceWorkspace.AvailableActions(), allow: true}, + {resource: ResourceWorkspace.WithOwner(user.ID), actions: siteAdminWorkspaceActions, allow: true}, - {resource: ResourceWorkspace.All(), actions: ResourceWorkspace.AvailableActions(), allow: true}, + {resource: ResourceWorkspace.All(), actions: siteAdminWorkspaceActions, allow: true}, // Other org + me - {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.ID), actions: ResourceWorkspace.AvailableActions(), allow: true}, - {resource: ResourceWorkspace.InOrg(unusedID), actions: ResourceWorkspace.AvailableActions(), allow: true}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.ID), actions: siteAdminWorkspaceActions, allow: true}, + {resource: ResourceWorkspace.InOrg(unusedID), actions: siteAdminWorkspaceActions, allow: true}, // Other org + other user - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me"), actions: ResourceWorkspace.AvailableActions(), allow: true}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me"), actions: siteAdminWorkspaceActions, allow: true}, - {resource: ResourceWorkspace.WithOwner("not-me"), actions: ResourceWorkspace.AvailableActions(), allow: true}, + {resource: ResourceWorkspace.WithOwner("not-me"), actions: siteAdminWorkspaceActions, allow: true}, // Other org + other use - {resource: ResourceWorkspace.InOrg(unusedID).WithOwner("not-me"), actions: ResourceWorkspace.AvailableActions(), allow: true}, - {resource: ResourceWorkspace.InOrg(unusedID), actions: ResourceWorkspace.AvailableActions(), allow: true}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner("not-me"), actions: siteAdminWorkspaceActions, allow: true}, + {resource: ResourceWorkspace.InOrg(unusedID), actions: siteAdminWorkspaceActions, allow: true}, - {resource: ResourceWorkspace.WithOwner("not-me"), actions: ResourceWorkspace.AvailableActions(), allow: true}, + {resource: ResourceWorkspace.WithOwner("not-me"), actions: siteAdminWorkspaceActions, allow: true}, }) user = Subject{ ID: "me", Scope: must(ExpandScope(ScopeApplicationConnect)), Roles: Roles{ - must(RoleByName(ScopedRoleOrgMember(defOrg))), + orgMemberRole(defOrg), must(RoleByName(RoleMember())), }, } @@ -762,7 +757,7 @@ func TestAuthorizeLevels(t *testing.T) { testAuthorize(t, "AdminAlwaysAllow", user, cases(func(c authTestCase) authTestCase { - c.actions = ResourceWorkspace.AvailableActions() + c.actions = slice.Omit(ResourceWorkspace.AvailableActions(), policy.ActionShare) c.allow = true return c }, []authTestCase{ @@ -890,7 +885,7 @@ func TestAuthorizeScope(t *testing.T) { ID: "me", Roles: Roles{ must(RoleByName(RoleMember())), - must(RoleByName(ScopedRoleOrgMember(defOrg))), + orgMemberRole(defOrg), }, Scope: must(ExpandScope(ScopeApplicationConnect)), } @@ -926,7 +921,7 @@ func TestAuthorizeScope(t *testing.T) { ID: "me", Roles: Roles{ must(RoleByName(RoleMember())), - must(RoleByName(ScopedRoleOrgMember(defOrg))), + orgMemberRole(defOrg), }, Scope: Scope{ Role: Role{ @@ -1015,7 +1010,7 @@ func TestAuthorizeScope(t *testing.T) { ID: "me", Roles: Roles{ must(RoleByName(RoleMember())), - must(RoleByName(ScopedRoleOrgMember(defOrg))), + orgMemberRole(defOrg), }, Scope: Scope{ Role: Role{ @@ -1070,7 +1065,7 @@ func TestAuthorizeScope(t *testing.T) { ID: meID.String(), Roles: Roles{ must(RoleByName(RoleMember())), - must(RoleByName(ScopedRoleOrgMember(defOrg))), + orgMemberRole(defOrg), }, Scope: must(ScopeNoUserData.Expand()), } @@ -1138,7 +1133,7 @@ func TestAuthorizeScope(t *testing.T) { // This is odd behavior, as without this membership role, the test for // the workspace fails. Maybe scopes should just assume the user // is a member. - must(RoleByName(ScopedRoleOrgMember(defOrg))), + orgMemberRole(defOrg), }, Scope: Scope{ Role: Role{ @@ -1404,6 +1399,28 @@ func testAuthorize(t *testing.T, name string, subject Subject, sets ...[]authTes } } +// orgMemberRole returns an organization-member role for RBAC-only tests. +// +// organization-member is now a DB-backed system role (not a built-in role), so +// 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) + return Role{ + Identifier: ScopedRoleOrgMember(orgID), + DisplayName: "", + Site: []Permission{}, + User: []Permission{}, + ByOrgID: map[string]OrgPermissions{ + orgID.String(): { + Org: orgPerms, + Member: memberPerms, + }, + }, + } +} + func must[T any](value T, err error) T { if err != nil { panic(err) diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 8a693d2571..4c3376decc 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -229,15 +229,30 @@ 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 } // ReservedRoleName exists because the database should only allow unique role -// names, but some roles are built in. So these names are reserved +// names, but some roles are built in or generated at runtime. So these names +// are reserved func ReservedRoleName(name string) bool { - _, ok := builtInRoles[name] - return ok + _, isBuiltIn := builtInRoles[name] + _, isSystem := systemRoles[name] + return isBuiltIn || isSystem } // ReloadBuiltinRoles loads the static roles into the builtInRoles map. @@ -252,7 +267,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { opts = &RoleOptions{} } - ownerWorkspaceActions := ResourceWorkspace.AvailableActions() + ownerWorkspaceActions := slice.Omit(ResourceWorkspace.AvailableActions(), policy.ActionShare) if opts.NoOwnerWorkspaceExec { // Remove ssh and application connect from the owner role. This // prevents owners from have exec access to all workspaces. @@ -431,39 +446,6 @@ func ReloadBuiltinRoles(opts *RoleOptions) { }, } }, - - // orgMember is an implied role to any member in an organization. - orgMember: func(organizationID uuid.UUID) Role { - return Role{ - Identifier: RoleIdentifier{Name: orgMember, OrganizationID: organizationID}, - DisplayName: "", - Site: []Permission{}, - User: []Permission{}, - ByOrgID: map[string]OrgPermissions{ - organizationID.String(): { - Org: Permissions(map[string][]policy.Action{ - // All users can see the 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}, - }), - Member: 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}, - // Can read their own organization member record - ResourceOrganizationMember.Type: {policy.ActionRead}, - // Users can create provisioner daemons scoped to themselves. - ResourceProvisionerDaemon.Type: {policy.ActionRead, policy.ActionCreate, policy.ActionRead, policy.ActionUpdate}, - })..., - ), - }, - }, - } - }, orgAuditor: func(organizationID uuid.UUID) Role { return Role{ Identifier: RoleIdentifier{Name: orgAuditor, OrganizationID: organizationID}, @@ -915,3 +897,110 @@ func DeduplicatePermissions(perms []Permission) []Permission { } return deduped } + +// PermissionsEqual compares two permission slices as sets. Order and +// duplicate entries do not matter; it only checks that both slices +// contain the same unique permissions. +func PermissionsEqual(a, b []Permission) bool { + setA := make(map[Permission]struct{}, len(a)) + for _, p := range a { + setA[p] = struct{}{} + } + + setB := make(map[Permission]struct{}, len(b)) + for _, p := range b { + if _, ok := setA[p]; !ok { + return false + } + setB[p] = struct{}{} + } + + return len(setA) == len(setB) +} + +// 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, +) { + // Organization-level permissions that all org members 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, members need to see other org members + // and groups to share workspaces with them. + if !workspaceSharingDisabled { + orgPermMap[ResourceOrganizationMember.Type] = []policy.Action{policy.ActionRead} + orgPermMap[ResourceGroup.Type] = []policy.Action{policy.ActionRead} + } + + orgPerms = Permissions(orgPermMap) + + // Member-scoped permissions (resources owned by the member). + // 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, + }, + // Can read their own organization member record. + 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 { + // 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, + }) + } + + return orgPerms, memberPerms +} diff --git a/coderd/rbac/roles_internal_test.go b/coderd/rbac/roles_internal_test.go index b99791b5a1..c45760f653 100644 --- a/coderd/rbac/roles_internal_test.go +++ b/coderd/rbac/roles_internal_test.go @@ -1,6 +1,7 @@ package rbac import ( + "slices" "testing" "github.com/google/uuid" @@ -74,7 +75,7 @@ func TestRegoInputValue(t *testing.T) { // Expand all roles and make sure we have a good copy. // This is because these tests modify the roles, and we don't want to // modify the original roles. - roles, err := RoleIdentifiers{ScopedRoleOrgMember(uuid.New()), ScopedRoleOrgAdmin(uuid.New()), RoleMember()}.Expand() + roles, err := RoleIdentifiers{ScopedRoleOrgAuditor(uuid.New()), ScopedRoleOrgAdmin(uuid.New()), RoleMember()}.Expand() require.NoError(t, err, "failed to expand roles") for i := range roles { // If all cached values are nil, then the role will not use @@ -224,9 +225,9 @@ func TestRoleByName(t *testing.T) { {Role: builtInRoles[orgAdmin](uuid.New())}, {Role: builtInRoles[orgAdmin](uuid.New())}, - {Role: builtInRoles[orgMember](uuid.New())}, - {Role: builtInRoles[orgMember](uuid.New())}, - {Role: builtInRoles[orgMember](uuid.New())}, + {Role: builtInRoles[orgAuditor](uuid.New())}, + {Role: builtInRoles[orgAuditor](uuid.New())}, + {Role: builtInRoles[orgAuditor](uuid.New())}, } for _, c := range testCases { @@ -271,6 +272,62 @@ func TestDeduplicatePermissions(t *testing.T) { require.Equal(t, want, got) } +func TestPermissionsEqual(t *testing.T) { + t.Parallel() + + a := []Permission{ + {ResourceType: ResourceWorkspace.Type, Action: policy.ActionRead}, + {ResourceType: ResourceTemplate.Type, Action: policy.ActionUpdate}, + {ResourceType: ResourceWorkspace.Type, Action: policy.ActionShare, Negate: true}, + } + + t.Run("Order", func(t *testing.T) { + t.Parallel() + + b := []Permission{ + a[2], + a[0], + a[1], + } + require.True(t, PermissionsEqual(a, b)) + }) + + t.Run("SubsetAndSuperset", func(t *testing.T) { + t.Parallel() + + require.False(t, PermissionsEqual(a, a[:2])) + + b := append(slices.Clone(a), Permission{ResourceType: ResourceWorkspace.Type, Action: policy.ActionUpdate}) + require.False(t, PermissionsEqual(a, b)) + }) + + t.Run("Negate", func(t *testing.T) { + t.Parallel() + + b := slices.Clone(a) + b[0] = Permission{ + ResourceType: ResourceWorkspace.Type, Action: policy.ActionRead, Negate: true, + } + require.False(t, PermissionsEqual(a, b)) + }) + + t.Run("Duplicates", func(t *testing.T) { + t.Parallel() + + b := append(slices.Clone(a), a[0]) + require.True(t, PermissionsEqual(a, b), "equal sets with duplicates should compare equal even without pre-deduplication") + }) + + t.Run("NilEmpty", func(t *testing.T) { + t.Parallel() + + var nilSlice []Permission + emptySlice := []Permission{} + require.True(t, PermissionsEqual(nilSlice, emptySlice)) + require.True(t, PermissionsEqual(emptySlice, nilSlice)) + }) +} + // equalRoles compares 2 roles for equality. func equalRoles(t *testing.T, a, b Role) { require.Equal(t, a.Identifier, b.Identifier, "role names") diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index c1cbfd41f9..a0a9e541a9 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -3,6 +3,7 @@ package rbac_test import ( "context" "fmt" + "slices" "testing" "github.com/google/uuid" @@ -50,6 +51,56 @@ func TestBuiltInRoles(t *testing.T) { } } +func TestSystemRolesAreReservedRoleNames(t *testing.T) { + t.Parallel() + + require.True(t, rbac.ReservedRoleName(rbac.RoleOrgMember())) +} + +func TestOrgMemberPermissions(t *testing.T) { + t.Parallel() + + t.Run("WorkspaceSharingEnabled", func(t *testing.T) { + t.Parallel() + + orgPerms, _ := rbac.OrgMemberPermissions(false) + + 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, + })) + }) + + t.Run("WorkspaceSharingDisabled", func(t *testing.T) { + t.Parallel() + + orgPerms, _ := rbac.OrgMemberPermissions(true) + + 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, + })) + }) +} + //nolint:tparallel,paralleltest func TestOwnerExec(t *testing.T) { owner := rbac.Subject{ @@ -86,6 +137,19 @@ func TestOwnerExec(t *testing.T) { }) } +// These were "pared down" in https://github.com/coder/coder/pull/21359 to avoid +// using the now DB-backed organization-member role. As a result, they no longer +// model real-world org-scoped users (who also have organization-member). +// +// For example, `org_auditor` is now expected to be forbidden for +// `assign_org_role:read`, even though in production an org auditor can read +// available org roles via the org-member baseline. +// +// The tests are still useful for unit-testing the built-in roles in isolation. +// +// TODO(geokat): Add an integration test that includes organization-member to +// recover the old test coverage. +// // nolint:tparallel,paralleltest // subtests share a map, just run sequentially. func TestRolePermissions(t *testing.T) { t.Parallel() @@ -110,34 +174,30 @@ func TestRolePermissions(t *testing.T) { // Subjects to user memberMe := authSubject{Name: "member_me", Actor: rbac.Subject{ID: currentUser.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember()}}} - orgMemberMe := authSubject{Name: "org_member_me", Actor: rbac.Subject{ID: currentUser.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(orgID)}}} - orgMemberMeBanWorkspace := authSubject{Name: "org_member_me_workspace_ban", Actor: rbac.Subject{ID: currentUser.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(orgID), rbac.ScopedRoleOrgWorkspaceCreationBan(orgID)}}} - groupMemberMe := authSubject{Name: "group_member_me", Actor: rbac.Subject{ID: currentUser.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(orgID)}, Groups: []string{groupID.String()}}} owner := authSubject{Name: "owner", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.RoleOwner()}}} templateAdmin := authSubject{Name: "template-admin", Actor: rbac.Subject{ID: templateAdminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.RoleTemplateAdmin()}}} userAdmin := authSubject{Name: "user-admin", Actor: rbac.Subject{ID: userAdminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.RoleUserAdmin()}}} auditor := authSubject{Name: "auditor", Actor: rbac.Subject{ID: auditorID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.RoleAuditor()}}} - orgAdmin := authSubject{Name: "org_admin", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(orgID), rbac.ScopedRoleOrgAdmin(orgID)}}} - orgAuditor := authSubject{Name: "org_auditor", Actor: rbac.Subject{ID: auditorID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(orgID), rbac.ScopedRoleOrgAuditor(orgID)}}} - orgUserAdmin := authSubject{Name: "org_user_admin", Actor: rbac.Subject{ID: templateAdminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(orgID), rbac.ScopedRoleOrgUserAdmin(orgID)}}} - orgTemplateAdmin := authSubject{Name: "org_template_admin", Actor: rbac.Subject{ID: userAdminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(orgID), rbac.ScopedRoleOrgTemplateAdmin(orgID)}}} + orgAdmin := authSubject{Name: "org_admin", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgAdmin(orgID)}}} + orgAuditor := authSubject{Name: "org_auditor", Actor: rbac.Subject{ID: auditorID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgAuditor(orgID)}}} + orgUserAdmin := authSubject{Name: "org_user_admin", Actor: rbac.Subject{ID: templateAdminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgUserAdmin(orgID)}}} + orgTemplateAdmin := authSubject{Name: "org_template_admin", Actor: rbac.Subject{ID: userAdminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgTemplateAdmin(orgID)}}} + orgAdminBanWorkspace := authSubject{Name: "org_admin_workspace_ban", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgAdmin(orgID), rbac.ScopedRoleOrgWorkspaceCreationBan(orgID)}}} setOrgNotMe := authSubjectSet{orgAdmin, orgAuditor, orgUserAdmin, orgTemplateAdmin} - otherOrgMember := authSubject{Name: "org_member_other", Actor: rbac.Subject{ID: uuid.NewString(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(otherOrg)}}} - otherOrgAdmin := authSubject{Name: "org_admin_other", Actor: rbac.Subject{ID: uuid.NewString(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(otherOrg), rbac.ScopedRoleOrgAdmin(otherOrg)}}} - otherOrgAuditor := authSubject{Name: "org_auditor_other", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(otherOrg), rbac.ScopedRoleOrgAuditor(otherOrg)}}} - otherOrgUserAdmin := authSubject{Name: "org_user_admin_other", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(otherOrg), rbac.ScopedRoleOrgUserAdmin(otherOrg)}}} - otherOrgTemplateAdmin := authSubject{Name: "org_template_admin_other", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(otherOrg), rbac.ScopedRoleOrgTemplateAdmin(otherOrg)}}} - setOtherOrg := authSubjectSet{otherOrgMember, otherOrgAdmin, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin} + otherOrgAdmin := authSubject{Name: "org_admin_other", Actor: rbac.Subject{ID: uuid.NewString(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgAdmin(otherOrg)}}} + otherOrgAuditor := authSubject{Name: "org_auditor_other", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgAuditor(otherOrg)}}} + otherOrgUserAdmin := authSubject{Name: "org_user_admin_other", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgUserAdmin(otherOrg)}}} + otherOrgTemplateAdmin := authSubject{Name: "org_template_admin_other", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgTemplateAdmin(otherOrg)}}} + setOtherOrg := authSubjectSet{otherOrgAdmin, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin} // requiredSubjects are required to be asserted in each test case. This is // to make sure one is not forgotten. requiredSubjects := []authSubject{ memberMe, owner, - orgMemberMe, orgAdmin, - otherOrgAdmin, otherOrgMember, orgAuditor, orgUserAdmin, orgTemplateAdmin, + orgAdmin, otherOrgAdmin, orgAuditor, orgUserAdmin, orgTemplateAdmin, templateAdmin, userAdmin, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, } @@ -159,10 +219,10 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionRead}, Resource: rbac.ResourceUserObject(currentUser), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {orgMemberMe, owner, memberMe, templateAdmin, userAdmin, orgUserAdmin, otherOrgAdmin, otherOrgUserAdmin, orgAdmin}, + true: {owner, memberMe, templateAdmin, userAdmin, orgUserAdmin, otherOrgAdmin, otherOrgUserAdmin, orgAdmin}, false: { orgTemplateAdmin, orgAuditor, - otherOrgMember, otherOrgAuditor, otherOrgTemplateAdmin, + otherOrgAuditor, otherOrgTemplateAdmin, }, }, }, @@ -172,7 +232,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceUser, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, userAdmin}, - false: {setOtherOrg, setOrgNotMe, memberMe, orgMemberMe, templateAdmin}, + false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin}, }, }, { @@ -181,7 +241,7 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionRead}, Resource: rbac.ResourceWorkspace.WithID(workspaceID).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgMemberMe, orgAdmin, templateAdmin, orgTemplateAdmin, orgMemberMeBanWorkspace}, + true: {owner, orgAdmin, templateAdmin, orgTemplateAdmin, orgAdminBanWorkspace}, false: {setOtherOrg, memberMe, userAdmin, orgAuditor, orgUserAdmin}, }, }, @@ -191,7 +251,7 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionUpdate}, Resource: rbac.ResourceWorkspace.WithID(workspaceID).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgMemberMe, orgAdmin}, + true: {owner, orgAdmin, orgAdminBanWorkspace}, false: {setOtherOrg, memberMe, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor}, }, }, @@ -201,8 +261,8 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionCreate, policy.ActionDelete}, Resource: rbac.ResourceWorkspace.WithID(workspaceID).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgMemberMe, orgAdmin}, - false: {setOtherOrg, memberMe, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor, orgMemberMeBanWorkspace}, + true: {owner, orgAdmin}, + false: {setOtherOrg, memberMe, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor, orgAdminBanWorkspace}, }, }, { @@ -211,7 +271,7 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionSSH}, Resource: rbac.ResourceWorkspace.WithID(workspaceID).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgMemberMe}, + true: {owner}, false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin, userAdmin}, }, }, @@ -221,7 +281,7 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionApplicationConnect}, Resource: rbac.ResourceWorkspace.WithID(workspaceID).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgMemberMe}, + true: {owner}, false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin, userAdmin}, }, }, @@ -230,8 +290,8 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionCreateAgent, policy.ActionDeleteAgent}, Resource: rbac.ResourceWorkspace.WithID(workspaceID).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgMemberMe, orgAdmin}, - false: {setOtherOrg, memberMe, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor, orgMemberMeBanWorkspace}, + true: {owner, orgAdmin}, + false: {setOtherOrg, memberMe, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor, orgAdminBanWorkspace}, }, }, { @@ -242,9 +302,9 @@ func TestRolePermissions(t *testing.T) { InOrg(orgID). WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgMemberMe, orgAdmin, orgMemberMeBanWorkspace}, + true: {orgAdmin, orgAdminBanWorkspace}, false: { - memberMe, setOtherOrg, + owner, memberMe, setOtherOrg, templateAdmin, userAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor, }, @@ -260,10 +320,10 @@ func TestRolePermissions(t *testing.T) { AuthorizeMap: map[bool][]hasAuthSubjects{ true: {}, false: { - orgMemberMe, orgAdmin, owner, setOtherOrg, + orgAdmin, owner, setOtherOrg, userAdmin, memberMe, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor, - orgMemberMeBanWorkspace, + orgAdminBanWorkspace, }, }, }, @@ -273,7 +333,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceTemplate.WithID(templateID).InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAdmin, templateAdmin, orgTemplateAdmin}, - false: {setOtherOrg, orgUserAdmin, orgAuditor, memberMe, orgMemberMe, userAdmin}, + false: {setOtherOrg, orgUserAdmin, orgAuditor, memberMe, userAdmin}, }, }, { @@ -282,7 +342,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceTemplate.InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAuditor, orgAdmin, templateAdmin, orgTemplateAdmin}, - false: {setOtherOrg, orgUserAdmin, memberMe, userAdmin, orgMemberMe}, + false: {setOtherOrg, orgUserAdmin, memberMe, userAdmin}, }, }, { @@ -292,8 +352,8 @@ func TestRolePermissions(t *testing.T) { groupID.String(): {policy.ActionUse}, }), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgAdmin, templateAdmin, orgTemplateAdmin, groupMemberMe}, - false: {setOtherOrg, orgAuditor, orgUserAdmin, memberMe, userAdmin, orgMemberMe}, + true: {owner, orgAdmin, templateAdmin, orgTemplateAdmin}, + false: {setOtherOrg, orgAuditor, orgUserAdmin, memberMe, userAdmin}, }, }, { @@ -304,7 +364,7 @@ func TestRolePermissions(t *testing.T) { true: {owner, templateAdmin}, // Org template admins can only read org scoped files. // File scope is currently not org scoped :cry: - false: {setOtherOrg, orgTemplateAdmin, orgMemberMe, orgAdmin, memberMe, userAdmin, orgAuditor, orgUserAdmin}, + false: {setOtherOrg, orgTemplateAdmin, orgAdmin, memberMe, userAdmin, orgAuditor, orgUserAdmin}, }, }, { @@ -312,7 +372,7 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionCreate, policy.ActionRead}, Resource: rbac.ResourceFile.WithID(fileID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, memberMe, orgMemberMe, templateAdmin}, + true: {owner, memberMe, templateAdmin}, false: {setOtherOrg, setOrgNotMe, userAdmin}, }, }, @@ -322,7 +382,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceOrganization, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, - false: {setOtherOrg, setOrgNotMe, memberMe, orgMemberMe, templateAdmin, userAdmin}, + false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin, userAdmin}, }, }, { @@ -331,7 +391,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceOrganization.WithID(orgID).InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAdmin}, - false: {setOtherOrg, orgTemplateAdmin, orgUserAdmin, orgAuditor, memberMe, orgMemberMe, templateAdmin, userAdmin}, + false: {setOtherOrg, orgTemplateAdmin, orgUserAdmin, orgAuditor, memberMe, templateAdmin, userAdmin}, }, }, { @@ -339,7 +399,7 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionRead}, Resource: rbac.ResourceOrganization.WithID(orgID).InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgAdmin, orgMemberMe, templateAdmin, orgTemplateAdmin, auditor, orgAuditor, userAdmin, orgUserAdmin}, + true: {owner, orgAdmin, templateAdmin, orgTemplateAdmin, auditor, orgAuditor, userAdmin, orgUserAdmin}, false: {setOtherOrg, memberMe}, }, }, @@ -349,7 +409,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceAssignOrgRole, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, - false: {setOtherOrg, setOrgNotMe, userAdmin, orgMemberMe, memberMe, templateAdmin}, + false: {setOtherOrg, setOrgNotMe, userAdmin, memberMe, templateAdmin}, }, }, { @@ -358,7 +418,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceAssignRole, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, userAdmin}, - false: {setOtherOrg, setOrgNotMe, orgMemberMe, memberMe, templateAdmin}, + false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin}, }, }, { @@ -366,7 +426,7 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionRead}, Resource: rbac.ResourceAssignRole, AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {setOtherOrg, setOrgNotMe, owner, orgMemberMe, memberMe, templateAdmin, userAdmin}, + true: {setOtherOrg, setOrgNotMe, owner, memberMe, templateAdmin, userAdmin}, false: {}, }, }, @@ -376,7 +436,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceAssignOrgRole.InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAdmin, userAdmin, orgUserAdmin}, - false: {setOtherOrg, orgMemberMe, memberMe, templateAdmin, orgTemplateAdmin, orgAuditor}, + false: {setOtherOrg, memberMe, templateAdmin, orgTemplateAdmin, orgAuditor}, }, }, { @@ -385,7 +445,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceAssignOrgRole.InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAdmin}, - false: {setOtherOrg, orgUserAdmin, orgTemplateAdmin, orgAuditor, orgMemberMe, memberMe, templateAdmin, userAdmin}, + false: {setOtherOrg, orgUserAdmin, orgTemplateAdmin, orgAuditor, memberMe, templateAdmin, userAdmin}, }, }, { @@ -393,8 +453,8 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionRead}, Resource: rbac.ResourceAssignOrgRole.InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, setOrgNotMe, orgMemberMe, userAdmin, templateAdmin}, - false: {setOtherOrg, memberMe}, + true: {owner, orgAdmin, orgUserAdmin, userAdmin, templateAdmin}, + false: {setOtherOrg, memberMe, orgAuditor, orgTemplateAdmin}, }, }, { @@ -402,7 +462,7 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionCreate, policy.ActionRead, policy.ActionDelete, policy.ActionUpdate}, Resource: rbac.ResourceApiKey.WithID(apiKeyID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgMemberMe, memberMe}, + true: {owner, memberMe}, false: {setOtherOrg, setOrgNotMe, templateAdmin, userAdmin}, }, }, @@ -413,7 +473,7 @@ func TestRolePermissions(t *testing.T) { }, Resource: rbac.ResourceInboxNotification.WithID(uuid.New()).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgMemberMe, orgAdmin}, + true: {owner, orgAdmin}, false: {setOtherOrg, orgUserAdmin, orgTemplateAdmin, orgAuditor, templateAdmin, userAdmin, memberMe}, }, }, @@ -422,7 +482,7 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionReadPersonal, policy.ActionUpdatePersonal}, Resource: rbac.ResourceUserObject(currentUser), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgMemberMe, memberMe, userAdmin}, + true: {owner, memberMe, userAdmin}, false: {setOtherOrg, setOrgNotMe, templateAdmin}, }, }, @@ -432,7 +492,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceOrganizationMember.WithID(currentUser).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAdmin, userAdmin, orgUserAdmin}, - false: {setOtherOrg, orgTemplateAdmin, orgAuditor, orgMemberMe, memberMe, templateAdmin}, + false: {setOtherOrg, orgTemplateAdmin, orgAuditor, memberMe, templateAdmin}, }, }, { @@ -440,7 +500,7 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionRead}, Resource: rbac.ResourceOrganizationMember.WithID(currentUser).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgAuditor, orgAdmin, userAdmin, orgMemberMe, templateAdmin, orgUserAdmin, orgTemplateAdmin}, + true: {owner, orgAuditor, orgAdmin, userAdmin, templateAdmin, orgUserAdmin, orgTemplateAdmin}, false: {memberMe, setOtherOrg}, }, }, @@ -453,7 +513,7 @@ func TestRolePermissions(t *testing.T) { }), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgAdmin, orgMemberMe, templateAdmin, orgUserAdmin, orgTemplateAdmin, orgAuditor}, + true: {owner, orgAdmin, templateAdmin, orgUserAdmin, orgTemplateAdmin, orgAuditor}, false: {setOtherOrg, memberMe, userAdmin}, }, }, @@ -467,7 +527,7 @@ func TestRolePermissions(t *testing.T) { }), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAdmin, userAdmin, orgUserAdmin}, - false: {setOtherOrg, memberMe, orgMemberMe, templateAdmin, orgTemplateAdmin, groupMemberMe, orgAuditor}, + false: {setOtherOrg, memberMe, templateAdmin, orgTemplateAdmin, orgAuditor}, }, }, { @@ -479,8 +539,8 @@ func TestRolePermissions(t *testing.T) { }, }), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgAdmin, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, groupMemberMe, orgAuditor}, - false: {setOtherOrg, memberMe, orgMemberMe}, + true: {owner, orgAdmin, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor}, + false: {setOtherOrg, memberMe}, }, }, { @@ -488,7 +548,7 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionRead}, Resource: rbac.ResourceGroupMember.WithID(currentUser).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgAuditor, orgAdmin, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgMemberMe, groupMemberMe}, + true: {owner, orgAuditor, orgAdmin, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin}, false: {setOtherOrg, memberMe}, }, }, @@ -498,7 +558,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceGroupMember.WithID(adminID).InOrg(orgID).WithOwner(adminID.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAuditor, orgAdmin, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin}, - false: {setOtherOrg, memberMe, orgMemberMe, groupMemberMe}, + false: {setOtherOrg, memberMe}, }, }, { @@ -506,7 +566,7 @@ func TestRolePermissions(t *testing.T) { Actions: append(crud, policy.ActionWorkspaceStop, policy.ActionCreateAgent, policy.ActionDeleteAgent), Resource: rbac.ResourceWorkspaceDormant.WithID(uuid.New()).InOrg(orgID).WithOwner(memberMe.Actor.ID), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {orgMemberMe, orgAdmin, owner}, + true: {orgAdmin, owner}, false: {setOtherOrg, userAdmin, memberMe, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor}, }, }, @@ -516,7 +576,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceWorkspaceDormant.WithID(uuid.New()).InOrg(orgID).WithOwner(memberMe.Actor.ID), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {}, - false: {setOtherOrg, setOrgNotMe, memberMe, userAdmin, orgMemberMe, owner, templateAdmin}, + false: {setOtherOrg, setOrgNotMe, memberMe, userAdmin, owner, templateAdmin}, }, }, { @@ -524,7 +584,7 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionWorkspaceStart, policy.ActionWorkspaceStop}, Resource: rbac.ResourceWorkspace.WithID(uuid.New()).InOrg(orgID).WithOwner(memberMe.Actor.ID), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgAdmin, orgMemberMe}, + true: {owner, orgAdmin}, false: {setOtherOrg, userAdmin, templateAdmin, memberMe, orgTemplateAdmin, orgUserAdmin, orgAuditor}, }, }, @@ -534,7 +594,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourcePrebuiltWorkspace.WithID(uuid.New()).InOrg(orgID).WithOwner(database.PrebuildsSystemUserID.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAdmin, templateAdmin, orgTemplateAdmin}, - false: {setOtherOrg, userAdmin, memberMe, orgUserAdmin, orgAuditor, orgMemberMe}, + false: {setOtherOrg, userAdmin, memberMe, orgUserAdmin, orgAuditor}, }, }, { @@ -542,7 +602,7 @@ func TestRolePermissions(t *testing.T) { Actions: crud, Resource: rbac.ResourceTask.WithID(uuid.New()).InOrg(orgID).WithOwner(memberMe.Actor.ID), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgAdmin, orgMemberMe}, + true: {owner, orgAdmin}, false: {setOtherOrg, userAdmin, templateAdmin, memberMe, orgTemplateAdmin, orgUserAdmin, orgAuditor}, }, }, @@ -553,7 +613,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceLicense, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, - false: {setOtherOrg, setOrgNotMe, memberMe, orgMemberMe, templateAdmin, userAdmin}, + false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin, userAdmin}, }, }, { @@ -562,7 +622,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceDeploymentStats, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, - false: {setOtherOrg, setOrgNotMe, memberMe, orgMemberMe, templateAdmin, userAdmin}, + false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin, userAdmin}, }, }, { @@ -571,7 +631,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceDeploymentConfig, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, - false: {setOtherOrg, setOrgNotMe, memberMe, orgMemberMe, templateAdmin, userAdmin}, + false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin, userAdmin}, }, }, { @@ -580,7 +640,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceDebugInfo, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, - false: {setOtherOrg, setOrgNotMe, memberMe, orgMemberMe, templateAdmin, userAdmin}, + false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin, userAdmin}, }, }, { @@ -589,7 +649,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceReplicas, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, - false: {setOtherOrg, setOrgNotMe, memberMe, orgMemberMe, templateAdmin, userAdmin}, + false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin, userAdmin}, }, }, { @@ -598,7 +658,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceTailnetCoordinator, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, - false: {setOtherOrg, setOrgNotMe, memberMe, orgMemberMe, templateAdmin, userAdmin}, + false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin, userAdmin}, }, }, { @@ -607,7 +667,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceAuditLog, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, - false: {setOtherOrg, setOrgNotMe, memberMe, orgMemberMe, templateAdmin, userAdmin}, + false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin, userAdmin}, }, }, { @@ -616,7 +676,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceProvisionerDaemon.InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, templateAdmin, orgAdmin, orgTemplateAdmin}, - false: {setOtherOrg, orgAuditor, orgUserAdmin, memberMe, orgMemberMe, userAdmin}, + false: {setOtherOrg, orgAuditor, orgUserAdmin, memberMe, userAdmin}, }, }, { @@ -624,8 +684,8 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionRead}, Resource: rbac.ResourceProvisionerDaemon.InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, templateAdmin, setOrgNotMe, orgMemberMe}, - false: {setOtherOrg, memberMe, userAdmin}, + true: {owner, templateAdmin, orgAdmin, orgTemplateAdmin}, + false: {setOtherOrg, memberMe, userAdmin, orgAuditor, orgUserAdmin}, }, }, { @@ -633,7 +693,7 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete}, Resource: rbac.ResourceProvisionerDaemon.WithOwner(currentUser.String()).InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, templateAdmin, orgTemplateAdmin, orgMemberMe, orgAdmin}, + true: {owner, templateAdmin, orgTemplateAdmin, orgAdmin}, false: {setOtherOrg, memberMe, userAdmin, orgUserAdmin, orgAuditor}, }, }, @@ -643,7 +703,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceProvisionerJobs.InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgTemplateAdmin, orgAdmin}, - false: {setOtherOrg, memberMe, orgMemberMe, templateAdmin, userAdmin, orgUserAdmin, orgAuditor}, + false: {setOtherOrg, memberMe, templateAdmin, userAdmin, orgUserAdmin, orgAuditor}, }, }, { @@ -652,7 +712,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceSystem, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, - false: {setOtherOrg, setOrgNotMe, memberMe, orgMemberMe, templateAdmin, userAdmin}, + false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin, userAdmin}, }, }, { @@ -661,7 +721,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceOauth2App, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, - false: {setOtherOrg, setOrgNotMe, memberMe, orgMemberMe, templateAdmin, userAdmin}, + false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin, userAdmin}, }, }, { @@ -669,7 +729,7 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionRead}, Resource: rbac.ResourceOauth2App, AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, setOrgNotMe, setOtherOrg, memberMe, orgMemberMe, templateAdmin, userAdmin}, + true: {owner, setOrgNotMe, setOtherOrg, memberMe, templateAdmin, userAdmin}, false: {}, }, }, @@ -679,7 +739,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceOauth2AppSecret, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, - false: {setOrgNotMe, setOtherOrg, memberMe, orgMemberMe, templateAdmin, userAdmin}, + false: {setOrgNotMe, setOtherOrg, memberMe, templateAdmin, userAdmin}, }, }, { @@ -688,7 +748,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceOauth2AppCodeToken, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, - false: {setOrgNotMe, setOtherOrg, memberMe, orgMemberMe, templateAdmin, userAdmin}, + false: {setOrgNotMe, setOtherOrg, memberMe, templateAdmin, userAdmin}, }, }, { @@ -697,7 +757,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceWorkspaceProxy, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, - false: {setOrgNotMe, setOtherOrg, memberMe, orgMemberMe, templateAdmin, userAdmin}, + false: {setOrgNotMe, setOtherOrg, memberMe, templateAdmin, userAdmin}, }, }, { @@ -705,7 +765,7 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionRead}, Resource: rbac.ResourceWorkspaceProxy, AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, setOrgNotMe, setOtherOrg, memberMe, orgMemberMe, templateAdmin, userAdmin}, + true: {owner, setOrgNotMe, setOtherOrg, memberMe, templateAdmin, userAdmin}, false: {}, }, }, @@ -716,11 +776,11 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, Resource: rbac.ResourceNotificationPreference.WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {memberMe, orgMemberMe, owner}, + true: {memberMe, owner}, false: { userAdmin, orgUserAdmin, templateAdmin, orgAuditor, orgTemplateAdmin, - otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, + otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, orgAdmin, otherOrgAdmin, }, }, @@ -733,9 +793,9 @@ func TestRolePermissions(t *testing.T) { AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, false: { - memberMe, orgMemberMe, userAdmin, orgUserAdmin, templateAdmin, + memberMe, userAdmin, orgUserAdmin, templateAdmin, orgAuditor, orgTemplateAdmin, - otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, + otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, orgAdmin, otherOrgAdmin, }, }, @@ -747,7 +807,7 @@ func TestRolePermissions(t *testing.T) { AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, false: { - memberMe, orgMemberMe, otherOrgMember, + memberMe, orgAdmin, otherOrgAdmin, orgAuditor, otherOrgAuditor, templateAdmin, orgTemplateAdmin, otherOrgTemplateAdmin, @@ -767,8 +827,8 @@ func TestRolePermissions(t *testing.T) { false: { memberMe, templateAdmin, orgUserAdmin, userAdmin, orgAdmin, orgAuditor, orgTemplateAdmin, - otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, - otherOrgAdmin, orgMemberMe, + otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, + otherOrgAdmin, }, }, }, @@ -778,8 +838,8 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionCreate, policy.ActionRead, policy.ActionDelete}, Resource: rbac.ResourceWebpushSubscription.WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, memberMe, orgMemberMe}, - false: {otherOrgMember, orgAdmin, otherOrgAdmin, orgAuditor, otherOrgAuditor, templateAdmin, orgTemplateAdmin, otherOrgTemplateAdmin, userAdmin, orgUserAdmin, otherOrgUserAdmin}, + true: {owner, memberMe}, + false: {orgAdmin, otherOrgAdmin, orgAuditor, otherOrgAuditor, templateAdmin, orgTemplateAdmin, otherOrgTemplateAdmin, userAdmin, orgUserAdmin, otherOrgUserAdmin}, }, }, // AnyOrganization tests @@ -791,8 +851,8 @@ func TestRolePermissions(t *testing.T) { true: {owner, userAdmin, orgAdmin, otherOrgAdmin, orgUserAdmin, otherOrgUserAdmin}, false: { memberMe, templateAdmin, - orgTemplateAdmin, orgMemberMe, orgAuditor, - otherOrgMember, otherOrgAuditor, otherOrgTemplateAdmin, + orgTemplateAdmin, orgAuditor, + otherOrgAuditor, otherOrgTemplateAdmin, }, }, }, @@ -804,8 +864,8 @@ func TestRolePermissions(t *testing.T) { true: {owner, templateAdmin, orgTemplateAdmin, otherOrgTemplateAdmin, orgAdmin, otherOrgAdmin}, false: { userAdmin, memberMe, - orgMemberMe, orgAuditor, orgUserAdmin, - otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, + orgAuditor, orgUserAdmin, + otherOrgAuditor, otherOrgUserAdmin, }, }, }, @@ -814,11 +874,11 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionCreate}, Resource: rbac.ResourceWorkspace.AnyOrganization().WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgAdmin, otherOrgAdmin, orgMemberMe}, + true: {owner, orgAdmin, otherOrgAdmin}, false: { memberMe, userAdmin, templateAdmin, orgAuditor, orgUserAdmin, orgTemplateAdmin, - otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, + otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, }, }, }, @@ -828,7 +888,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceCryptoKey, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, - false: {setOtherOrg, setOrgNotMe, memberMe, orgMemberMe, templateAdmin, userAdmin}, + false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin, userAdmin}, }, }, { @@ -838,10 +898,10 @@ func TestRolePermissions(t *testing.T) { AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAdmin, orgUserAdmin, userAdmin}, false: { - orgMemberMe, otherOrgAdmin, + otherOrgAdmin, memberMe, templateAdmin, orgAuditor, orgTemplateAdmin, - otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, + otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, }, }, }, @@ -853,10 +913,10 @@ func TestRolePermissions(t *testing.T) { true: {owner, userAdmin}, false: { orgAdmin, orgUserAdmin, - orgMemberMe, otherOrgAdmin, + otherOrgAdmin, memberMe, templateAdmin, orgAuditor, orgTemplateAdmin, - otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, + otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, }, }, }, @@ -867,7 +927,7 @@ func TestRolePermissions(t *testing.T) { AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, false: { - memberMe, orgMemberMe, otherOrgMember, + memberMe, orgAdmin, otherOrgAdmin, orgAuditor, otherOrgAuditor, templateAdmin, orgTemplateAdmin, otherOrgTemplateAdmin, @@ -882,7 +942,7 @@ func TestRolePermissions(t *testing.T) { AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, false: { - memberMe, orgMemberMe, otherOrgMember, + memberMe, orgAdmin, otherOrgAdmin, orgAuditor, otherOrgAuditor, templateAdmin, orgTemplateAdmin, otherOrgTemplateAdmin, @@ -896,7 +956,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceConnectionLog, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, - false: {setOtherOrg, setOrgNotMe, memberMe, orgMemberMe, templateAdmin, userAdmin}, + false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin, userAdmin}, }, }, // Only the user themselves can access their own secrets — no one else. @@ -905,10 +965,10 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, Resource: rbac.ResourceUserSecret.WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {memberMe, orgMemberMe}, + true: {memberMe}, false: { owner, orgAdmin, - otherOrgAdmin, otherOrgMember, orgAuditor, orgUserAdmin, orgTemplateAdmin, + otherOrgAdmin, orgAuditor, orgUserAdmin, orgTemplateAdmin, templateAdmin, userAdmin, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, }, }, @@ -921,7 +981,7 @@ func TestRolePermissions(t *testing.T) { true: {}, false: { owner, - memberMe, orgMemberMe, otherOrgMember, + memberMe, orgAdmin, otherOrgAdmin, orgAuditor, otherOrgAuditor, templateAdmin, orgTemplateAdmin, otherOrgTemplateAdmin, @@ -934,9 +994,8 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionCreate, policy.ActionRead, policy.ActionUpdate}, Resource: rbac.ResourceAibridgeInterception.WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, memberMe, orgMemberMe}, + true: {owner, memberMe}, false: { - otherOrgMember, orgAdmin, otherOrgAdmin, orgAuditor, otherOrgAuditor, templateAdmin, orgTemplateAdmin, otherOrgTemplateAdmin, @@ -1096,7 +1155,6 @@ func TestListRoles(t *testing.T) { require.ElementsMatch(t, []string{ fmt.Sprintf("organization-admin:%s", orgID.String()), - fmt.Sprintf("organization-member:%s", orgID.String()), fmt.Sprintf("organization-auditor:%s", orgID.String()), fmt.Sprintf("organization-user-admin:%s", orgID.String()), fmt.Sprintf("organization-template-admin:%s", orgID.String()), diff --git a/coderd/rbac/rolestore/rolestore.go b/coderd/rbac/rolestore/rolestore.go index c2189c13b0..aef7992f16 100644 --- a/coderd/rbac/rolestore/rolestore.go +++ b/coderd/rbac/rolestore/rolestore.go @@ -7,6 +7,7 @@ import ( "github.com/google/uuid" "golang.org/x/xerrors" + "cdr.dev/slog/v3" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/util/syncmap" @@ -83,9 +84,10 @@ func Expand(ctx context.Context, db database.Store, names []rbac.RoleIdentifier) // the expansion. These roles are no-ops. Should we raise some kind of // warning when this happens? dbroles, err := db.CustomRoles(ctx, database.CustomRolesParams{ - LookupRoles: lookupArgs, - ExcludeOrgRoles: false, - OrganizationID: uuid.Nil, + LookupRoles: lookupArgs, + ExcludeOrgRoles: false, + OrganizationID: uuid.Nil, + IncludeSystemRoles: true, }) if err != nil { return nil, xerrors.Errorf("fetch custom roles: %w", err) @@ -105,7 +107,8 @@ func Expand(ctx context.Context, db database.Store, names []rbac.RoleIdentifier) return roles, nil } -func convertPermissions(dbPerms []database.CustomRolePermission) []rbac.Permission { +// ConvertDBPermissions converts database permissions to RBAC permissions. +func ConvertDBPermissions(dbPerms []database.CustomRolePermission) []rbac.Permission { n := make([]rbac.Permission, 0, len(dbPerms)) for _, dbPerm := range dbPerms { n = append(n, rbac.Permission{ @@ -117,14 +120,28 @@ func convertPermissions(dbPerms []database.CustomRolePermission) []rbac.Permissi return n } +// ConvertPermissionsToDB converts RBAC permissions to the database +// format. +func ConvertPermissionsToDB(perms []rbac.Permission) []database.CustomRolePermission { + dbPerms := make([]database.CustomRolePermission, 0, len(perms)) + for _, perm := range perms { + dbPerms = append(dbPerms, database.CustomRolePermission{ + Negate: perm.Negate, + ResourceType: perm.ResourceType, + Action: perm.Action, + }) + } + return dbPerms +} + // ConvertDBRole should not be used by any human facing apis. It is used // for authz purposes. func ConvertDBRole(dbRole database.CustomRole) (rbac.Role, error) { role := rbac.Role{ Identifier: dbRole.RoleIdentifier(), DisplayName: dbRole.DisplayName, - Site: convertPermissions(dbRole.SitePermissions), - User: convertPermissions(dbRole.UserPermissions), + Site: ConvertDBPermissions(dbRole.SitePermissions), + User: ConvertDBPermissions(dbRole.UserPermissions), } // Org permissions only make sense if an org id is specified. @@ -135,10 +152,158 @@ func ConvertDBRole(dbRole database.CustomRole) (rbac.Role, error) { if dbRole.OrganizationID.UUID != uuid.Nil { role.ByOrgID = map[string]rbac.OrgPermissions{ dbRole.OrganizationID.UUID.String(): { - Org: convertPermissions(dbRole.OrgPermissions), + Org: ConvertDBPermissions(dbRole.OrgPermissions), + Member: ConvertDBPermissions(dbRole.MemberPermissions), }, } } 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. +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 + // multiple coderd instances. Other instances will block here + // until we release the lock (when this transaction commits). + err := tx.AcquireLock(ctx, database.LockIDReconcileSystemRoles) + if err != nil { + return xerrors.Errorf("acquire system roles reconciliation lock: %w", err) + } + + orgs, err := tx.GetOrganizations(ctx, database.GetOrganizationsParams{}) + if err != nil { + return xerrors.Errorf("fetch organizations: %w", err) + } + + customRoles, err := tx.CustomRoles(ctx, database.CustomRolesParams{ + LookupRoles: nil, + ExcludeOrgRoles: false, + OrganizationID: uuid.Nil, + IncludeSystemRoles: true, + }) + if err != nil { + 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) + for _, role := range customRoles { + if role.IsSystem && role.Name == rbac.RoleOrgMember() && role.OrganizationID.Valid { + rolesByOrg[role.OrganizationID.UUID] = 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)) + + if err := CreateOrgMemberRole(ctx, tx, org); err != nil { + return xerrors.Errorf("create missing organization-member role for organization %s: %w", + org.ID, err) + } + + // 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) + } + } + + return nil + }, 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( + ctx context.Context, + tx database.Store, + in database.CustomRole, + workspaceSharingDisabled bool, +) ( + database.CustomRole, bool, error, +) { + // 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 = "" + + inOrgPerms := ConvertDBPermissions(in.OrgPermissions) + inMemberPerms := ConvertDBPermissions(in.MemberPermissions) + + outOrgPerms, outMemberPerms := rbac.OrgMemberPermissions(workspaceSharingDisabled) + + // Compare using set-based comparison (order doesn't matter). + match := rbac.PermissionsEqual(inOrgPerms, outOrgPerms) && + rbac.PermissionsEqual(inMemberPerms, outMemberPerms) + + if !match { + out.OrgPermissions = ConvertPermissionsToDB(outOrgPerms) + out.MemberPermissions = ConvertPermissionsToDB(outMemberPerms) + + _, err := tx.UpdateCustomRole(ctx, database.UpdateCustomRoleParams{ + Name: out.Name, + OrganizationID: out.OrganizationID, + DisplayName: out.DisplayName, + SitePermissions: out.SitePermissions, + UserPermissions: out.UserPermissions, + OrgPermissions: out.OrgPermissions, + 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, 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) + + _, err := tx.InsertCustomRole(ctx, database.InsertCustomRoleParams{ + Name: rbac.RoleOrgMember(), + DisplayName: "", + OrganizationID: uuid.NullUUID{UUID: org.ID, Valid: true}, + SitePermissions: database.CustomRolePermissions{}, + OrgPermissions: ConvertPermissionsToDB(orgPerms), + UserPermissions: database.CustomRolePermissions{}, + MemberPermissions: ConvertPermissionsToDB(memberPerms), + IsSystem: true, + }) + if err != nil { + return xerrors.Errorf("insert org-member role: %w", err) + } + + return nil +} diff --git a/coderd/rbac/rolestore/rolestore_test.go b/coderd/rbac/rolestore/rolestore_test.go index 47289704d8..175db71c77 100644 --- a/coderd/rbac/rolestore/rolestore_test.go +++ b/coderd/rbac/rolestore/rolestore_test.go @@ -1,11 +1,13 @@ package rolestore_test import ( + "database/sql" "testing" "github.com/google/uuid" "github.com/stretchr/testify/require" + "cdr.dev/slog/v3" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" @@ -39,3 +41,133 @@ func TestExpandCustomRoleRoles(t *testing.T) { require.NoError(t, err) require.Len(t, roles, 1, "role found") } + +func TestReconcileOrgMemberRole(t *testing.T) { + t.Parallel() + + 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) + + _, 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{} + + reconciled, didUpdate, err := rolestore.ReconcileOrgMemberRole(ctx, db, stale, org.WorkspaceSharingDisabled) + 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) + + 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)) + + _, 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") +} + +func TestReconcileSystemRoles(t *testing.T) { + t.Parallel() + + var sqlDB *sql.DB + db, _, sqlDB := dbtestutil.NewDBWithSQLDB(t) + + // The DB trigger will create system roles for the org. + org1 := dbgen.Organization(t, db, database.Organization{}) + org2 := dbgen.Organization(t, db, database.Organization{}) + + ctx := testutil.Context(t, testutil.WaitShort) + + _, err := sqlDB.ExecContext(ctx, "UPDATE organizations SET workspace_sharing_disabled = true WHERE id = $1", org2.ID) + require.NoError(t, err) + + // Simulate a missing system role by bypassing the application's + // safety check in DeleteCustomRole (which prevents deleting + // system roles). + res, err := sqlDB.ExecContext(ctx, + "DELETE FROM custom_roles WHERE name = lower($1) AND organization_id = $2", + rbac.RoleOrgMember(), + org1.ID, + ) + require.NoError(t, err) + affected, err := res.RowsAffected() + require.NoError(t, err) + require.Equal(t, int64(1), affected) + + // Not using testutil.Logger() here because it would fail on the + // CRITICAL log line due to the deleted custom role. + err = rolestore.ReconcileSystemRoles(ctx, slog.Make(), db) + require.NoError(t, err) + + orgs, err := db.GetOrganizations(ctx, database.GetOrganizationsParams{}) + require.NoError(t, err) + + orgByID := make(map[uuid.UUID]database.Organization, len(orgs)) + for _, org := range orgs { + orgByID[org.ID] = org + } + + assertOrgMemberRole := func(t *testing.T, orgID uuid.UUID) { + t.Helper() + + org := orgByID[orgID] + got, err := database.ExpectOne(db.CustomRoles(ctx, database.CustomRolesParams{ + LookupRoles: []database.NameOrganizationPair{ + { + Name: rbac.RoleOrgMember(), + OrganizationID: orgID, + }, + }, + IncludeSystemRoles: true, + })) + 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)) + } + + assertOrgMemberRole(t, org1.ID) + assertOrgMemberRole(t, org2.ID) +} diff --git a/coderd/roles.go b/coderd/roles.go index e9c911bc72..3d27ff666c 100644 --- a/coderd/roles.go +++ b/coderd/roles.go @@ -34,8 +34,9 @@ func (api *API) AssignableSiteRoles(rw http.ResponseWriter, r *http.Request) { dbCustomRoles, err := api.Database.CustomRoles(ctx, database.CustomRolesParams{ LookupRoles: nil, // Only site wide custom roles to be included - ExcludeOrgRoles: true, - OrganizationID: uuid.Nil, + ExcludeOrgRoles: true, + OrganizationID: uuid.Nil, + IncludeSystemRoles: false, }) if err != nil { httpapi.InternalServerError(rw, err) @@ -67,9 +68,10 @@ func (api *API) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) { roles := rbac.OrganizationRoles(organization.ID) dbCustomRoles, err := api.Database.CustomRoles(ctx, database.CustomRolesParams{ - LookupRoles: nil, - ExcludeOrgRoles: false, - OrganizationID: organization.ID, + LookupRoles: nil, + ExcludeOrgRoles: false, + OrganizationID: organization.ID, + IncludeSystemRoles: false, }) if err != nil { httpapi.InternalServerError(rw, err) diff --git a/coderd/templates_test.go b/coderd/templates_test.go index f99f5d07be..b425754866 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -480,7 +480,7 @@ func TestTemplates(t *testing.T) { // Deprecate bar template deprecationMessage := "Some deprecated message" - err := db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin, user.OrganizationID)), database.UpdateTemplateAccessControlByIDParams{ + err := db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin)), database.UpdateTemplateAccessControlByIDParams{ ID: bar.ID, RequireActiveVersion: false, Deprecated: deprecationMessage, @@ -522,13 +522,13 @@ func TestTemplates(t *testing.T) { // Deprecate foo and bar templates deprecationMessage := "Some deprecated message" - err := db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin, user.OrganizationID)), database.UpdateTemplateAccessControlByIDParams{ + err := db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin)), database.UpdateTemplateAccessControlByIDParams{ ID: foo.ID, RequireActiveVersion: false, Deprecated: deprecationMessage, }) require.NoError(t, err) - err = db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin, user.OrganizationID)), database.UpdateTemplateAccessControlByIDParams{ + err = db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin)), database.UpdateTemplateAccessControlByIDParams{ ID: bar.ID, RequireActiveVersion: false, Deprecated: deprecationMessage, @@ -637,7 +637,7 @@ func TestTemplates(t *testing.T) { // Deprecate bar template deprecationMessage := "Some deprecated message" - err := db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin, user.OrganizationID)), database.UpdateTemplateAccessControlByIDParams{ + err := db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin)), database.UpdateTemplateAccessControlByIDParams{ ID: bar.ID, RequireActiveVersion: false, Deprecated: deprecationMessage, @@ -650,7 +650,7 @@ func TestTemplates(t *testing.T) { require.Equal(t, deprecationMessage, updatedBar.DeprecationMessage) // Re-enable bar template - err = db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin, user.OrganizationID)), database.UpdateTemplateAccessControlByIDParams{ + err = db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin)), database.UpdateTemplateAccessControlByIDParams{ ID: bar.ID, RequireActiveVersion: false, Deprecated: "", @@ -793,7 +793,7 @@ func TestTemplatesByOrganization(t *testing.T) { // Deprecate bar template deprecationMessage := "Some deprecated message" - err := db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin, user.OrganizationID)), database.UpdateTemplateAccessControlByIDParams{ + err := db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin)), database.UpdateTemplateAccessControlByIDParams{ ID: bar.ID, RequireActiveVersion: false, Deprecated: deprecationMessage, @@ -1004,7 +1004,7 @@ func TestPatchTemplateMeta(t *testing.T) { ctx := testutil.Context(t, testutil.WaitLong) // nolint:gocritic // Setting up unit test data - err := db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin, user.OrganizationID)), database.UpdateTemplateAccessControlByIDParams{ + err := db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin)), database.UpdateTemplateAccessControlByIDParams{ ID: template.ID, RequireActiveVersion: false, Deprecated: "Some deprecated message", diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 17d77440ff..e836566367 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -182,7 +182,7 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) { admin, err := client.User(ctx, user.UserID.String()) require.NoError(t, err) - tvDB, err := db.GetTemplateVersionByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(admin, user.OrganizationID)), version.ID) + tvDB, err := db.GetTemplateVersionByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(admin)), version.ID) require.NoError(t, err) require.False(t, tvDB.SourceExampleID.Valid) }) @@ -232,7 +232,7 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) { admin, err := client.User(ctx, user.UserID.String()) require.NoError(t, err) - tvDB, err := db.GetTemplateVersionByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(admin, user.OrganizationID)), tv.ID) + tvDB, err := db.GetTemplateVersionByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(admin)), tv.ID) require.NoError(t, err) require.Equal(t, ls[0].ID, tvDB.SourceExampleID.String) diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go index da63fdea9c..f41fb65ee1 100644 --- a/coderd/userauth_test.go +++ b/coderd/userauth_test.go @@ -873,7 +873,8 @@ func TestUserOAuth2Github(t *testing.T) { }, }, }) - first := coderdtest.CreateFirstUser(t, owner) + + coderdtest.CreateFirstUser(t, owner) ctx := testutil.Context(t, testutil.WaitLong) ownerUser, err := owner.User(context.Background(), "me") @@ -890,7 +891,7 @@ func TestUserOAuth2Github(t *testing.T) { err = owner.DeleteUser(ctx, deleted.ID) require.NoError(t, err) // Check no user links for the user - links, err := db.GetUserLinksByUserID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(ownerUser, first.OrganizationID)), deleted.ID) + links, err := db.GetUserLinksByUserID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(ownerUser)), deleted.ID) require.NoError(t, err) require.Empty(t, links) diff --git a/coderd/workspaceagentportshare_test.go b/coderd/workspaceagentportshare_test.go index 201ba68f3d..f6cff2640d 100644 --- a/coderd/workspaceagentportshare_test.go +++ b/coderd/workspaceagentportshare_test.go @@ -31,7 +31,7 @@ func TestPostWorkspaceAgentPortShare(t *testing.T) { agents[0].Directory = tmpDir return agents }).Do() - agents, err := db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(user, owner.OrganizationID)), r.Workspace.ID) + agents, err := db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(dbauthz.As(ctx, coderdtest.AuthzUserSubjectWithDB(ctx, t, db, user)), r.Workspace.ID) require.NoError(t, err) // owner level should fail @@ -148,7 +148,7 @@ func TestGetWorkspaceAgentPortShares(t *testing.T) { agents[0].Directory = tmpDir return agents }).Do() - agents, err := db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(user, owner.OrganizationID)), r.Workspace.ID) + agents, err := db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(dbauthz.As(ctx, coderdtest.AuthzUserSubjectWithDB(ctx, t, db, user)), r.Workspace.ID) require.NoError(t, err) _, err = client.UpsertWorkspaceAgentPortShare(ctx, r.Workspace.ID, codersdk.UpsertWorkspaceAgentPortShareRequest{ @@ -184,7 +184,7 @@ func TestDeleteWorkspaceAgentPortShare(t *testing.T) { agents[0].Directory = tmpDir return agents }).Do() - agents, err := db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(user, owner.OrganizationID)), r.Workspace.ID) + agents, err := db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(dbauthz.As(ctx, coderdtest.AuthzUserSubjectWithDB(ctx, t, db, user)), r.Workspace.ID) require.NoError(t, err) // create @@ -211,7 +211,7 @@ func TestDeleteWorkspaceAgentPortShare(t *testing.T) { }) require.Error(t, err) - _, err = db.GetWorkspaceAgentPortShare(dbauthz.As(ctx, coderdtest.AuthzUserSubject(user, owner.OrganizationID)), database.GetWorkspaceAgentPortShareParams{ + _, err = db.GetWorkspaceAgentPortShare(dbauthz.As(ctx, coderdtest.AuthzUserSubjectWithDB(ctx, t, db, user)), database.GetWorkspaceAgentPortShareParams{ WorkspaceID: r.Workspace.ID, AgentName: agents[0].Name, Port: 8080, diff --git a/docs/admin/security/audit-logs.md b/docs/admin/security/audit-logs.md index d38f2e359b..31c6c45d64 100644 --- a/docs/admin/security/audit-logs.md +++ b/docs/admin/security/audit-logs.md @@ -19,7 +19,7 @@ We track the following resources: | AuditOAuthConvertState
| |
FieldTracked
created_attrue
expires_attrue
from_login_typetrue
to_login_typetrue
user_idtrue
| | Group
create, write, delete | |
FieldTracked
avatar_urltrue
display_nametrue
idtrue
memberstrue
nametrue
organization_idfalse
quota_allowancetrue
sourcefalse
| | AuditableOrganizationMember
| |
FieldTracked
created_attrue
organization_idfalse
rolestrue
updated_attrue
user_idtrue
usernametrue
| -| CustomRole
| |
FieldTracked
created_atfalse
display_nametrue
idfalse
nametrue
org_permissionstrue
organization_idfalse
site_permissionstrue
updated_atfalse
user_permissionstrue
| +| CustomRole
| |
FieldTracked
created_atfalse
display_nametrue
idfalse
is_systemfalse
member_permissionstrue
nametrue
org_permissionstrue
organization_idfalse
site_permissionstrue
updated_atfalse
user_permissionstrue
| | GitSSHKey
create | |
FieldTracked
created_atfalse
private_keytrue
public_keytrue
updated_atfalse
user_idtrue
| | GroupSyncSettings
| |
FieldTracked
auto_create_missing_groupstrue
fieldtrue
legacy_group_name_mappingfalse
mappingtrue
regex_filtertrue
| | HealthSettings
| |
FieldTracked
dismissed_healthcheckstrue
idfalse
| @@ -28,7 +28,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
| +| Organization
| |
FieldTracked
created_atfalse
deletedtrue
descriptiontrue
display_nametrue
icontrue
idfalse
is_defaulttrue
nametrue
updated_attrue
workspace_sharing_disabledtrue
| | OrganizationSyncSettings
| |
FieldTracked
assign_defaulttrue
fieldtrue
mappingtrue
| | PrebuildsSettings
| |
FieldTracked
idfalse
reconciliation_pausedtrue
| | RoleSyncSettings
| |
FieldTracked
fieldtrue
mappingtrue
| diff --git a/docs/reference/api/members.md b/docs/reference/api/members.md index e05399e84d..aa091bb094 100644 --- a/docs/reference/api/members.md +++ b/docs/reference/api/members.md @@ -179,7 +179,7 @@ Status Code **200** To perform this operation, you must be authenticated. [Learn more](authentication.md). -## Upsert a custom organization role +## Update a custom organization role ### Code samples @@ -235,7 +235,7 @@ curl -X PUT http://coder-server:8080/api/v2/organizations/{organization}/members | Name | In | Type | Required | Description | |----------------|------|--------------------------------------------------------------------|----------|---------------------| | `organization` | path | string(uuid) | true | Organization ID | -| `body` | body | [codersdk.CustomRoleRequest](schemas.md#codersdkcustomrolerequest) | true | Upsert role request | +| `body` | body | [codersdk.CustomRoleRequest](schemas.md#codersdkcustomrolerequest) | true | Update role request | ### Example responses @@ -285,7 +285,7 @@ curl -X PUT http://coder-server:8080/api/v2/organizations/{organization}/members |--------|---------------------------------------------------------|-------------|---------------------------------------------------| | 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | array of [codersdk.Role](schemas.md#codersdkrole) | -

Response Schema

+

Response Schema

Status Code **200** diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 426a0a2381..6565d5e49c 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -62,12 +62,14 @@ var auditableResourcesTypes = map[any]map[string]Action{ "roles": ActionTrack, }, &database.CustomRole{}: { - "name": ActionTrack, - "display_name": ActionTrack, - "site_permissions": ActionTrack, - "org_permissions": ActionTrack, - "user_permissions": ActionTrack, - "organization_id": ActionIgnore, // Never changes. + "name": ActionTrack, + "display_name": ActionTrack, + "site_permissions": ActionTrack, + "org_permissions": ActionTrack, + "user_permissions": ActionTrack, + "member_permissions": ActionTrack, + "organization_id": ActionIgnore, // Never changes. + "is_system": ActionIgnore, // Never changes. "id": ActionIgnore, "created_at": ActionIgnore, @@ -309,15 +311,16 @@ var auditableResourcesTypes = map[any]map[string]Action{ "secret_prefix": ActionIgnore, }, &database.Organization{}: { - "id": ActionIgnore, - "name": ActionTrack, - "description": ActionTrack, - "deleted": ActionTrack, - "created_at": ActionIgnore, - "updated_at": ActionTrack, - "is_default": ActionTrack, - "display_name": ActionTrack, - "icon": ActionTrack, + "id": ActionIgnore, + "name": ActionTrack, + "description": ActionTrack, + "deleted": ActionTrack, + "created_at": ActionIgnore, + "updated_at": ActionTrack, + "is_default": ActionTrack, + "display_name": ActionTrack, + "icon": ActionTrack, + "workspace_sharing_disabled": ActionTrack, }, &database.NotificationTemplate{}: { "id": ActionIgnore, diff --git a/enterprise/cli/sharing_test.go b/enterprise/cli/sharing_test.go index 9e99b85886..c096b515e3 100644 --- a/enterprise/cli/sharing_test.go +++ b/enterprise/cli/sharing_test.go @@ -221,7 +221,7 @@ func TestSharingStatus(t *testing.T) { group, err := createGroupWithMembers(ctx, client, orgOwner.OrganizationID, "new-group", []uuid.UUID{orgMember.ID}) require.NoError(t, err) - err = client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ + err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ GroupRoles: map[string]codersdk.WorkspaceRole{ group.ID.String(): codersdk.WorkspaceRoleUse, }, @@ -284,7 +284,7 @@ func TestSharingRemove(t *testing.T) { require.NoError(t, err) // Share the workspace with a user to later remove - err = client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ + err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ GroupRoles: map[string]codersdk.WorkspaceRole{ group1.ID.String(): codersdk.WorkspaceRoleUse, group2.ID.String(): codersdk.WorkspaceRoleUse, @@ -357,7 +357,7 @@ func TestSharingRemove(t *testing.T) { require.NoError(t, err) // Share the workspace with a user to later remove - err = client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ + err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ GroupRoles: map[string]codersdk.WorkspaceRole{ group1.ID.String(): codersdk.WorkspaceRoleUse, group2.ID.String(): codersdk.WorkspaceRoleUse, diff --git a/enterprise/coderd/groups_test.go b/enterprise/coderd/groups_test.go index 568825adcd..967f927d60 100644 --- a/enterprise/coderd/groups_test.go +++ b/enterprise/coderd/groups_test.go @@ -13,7 +13,10 @@ import ( "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" + "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/coderd/rbac/rolestore" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" @@ -694,22 +697,75 @@ func TestGroup(t *testing.T) { t.Run("RegularUserReadGroup", func(t *testing.T) { t.Parallel() - client, user := coderdenttest.New(t, &coderdenttest.Options{LicenseOptions: &coderdenttest.LicenseOptions{ - Features: license.Features{ - codersdk.FeatureTemplateRBAC: 1, - }, - }}) - client1, _ := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) + t.Run("WorkspaceSharingEnabled", func(t *testing.T) { + t.Parallel() - ctx := testutil.Context(t, testutil.WaitLong) - //nolint:gocritic // test setup - group, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{ - Name: "hi", + client, user := coderdenttest.New(t, &coderdenttest.Options{LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureTemplateRBAC: 1, + }, + }}) + client1, _ := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) + + ctx := testutil.Context(t, testutil.WaitLong) + //nolint:gocritic // test setup + group, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{ + Name: "hi", + }) + require.NoError(t, err) + + ggroup, err := client1.Group(ctx, group.ID) + require.NoError(t, err, "regular users can read groups unless workspace sharing is disabled") + normalizeGroupMembers(&group) + normalizeGroupMembers(&ggroup) + require.Equal(t, group, ggroup) }) - require.NoError(t, err) - _, err = client1.Group(ctx, group.ID) - require.Error(t, err, "regular users cannot read groups") + t.Run("WorkspaceSharingDisabled", func(t *testing.T) { + t.Parallel() + + db, ps, sqlDB := dbtestutil.NewDBWithSQLDB(t) + client, _, api, user := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + Database: db, + Pubsub: ps, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureTemplateRBAC: 1, + }, + }, + }) + ctx := testutil.Context(t, testutil.WaitLong) + _, err := sqlDB.ExecContext(ctx, "UPDATE organizations SET workspace_sharing_disabled = true 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{ + Name: rbac.RoleOrgMember(), + OrganizationID: uuid.NullUUID{ + UUID: user.OrganizationID, + Valid: true, + }, + }, true) + require.NoError(t, err) + + client1, _ := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) + + //nolint:gocritic // test setup + group, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{ + Name: "hi", + }) + require.NoError(t, err) + + _, err = client1.Group(ctx, group.ID) + require.Error(t, err, "regular users cannot read groups when workspace sharing is disabled") + cerr, ok := codersdk.AsError(err) + require.True(t, ok) + require.Equal(t, http.StatusNotFound, cerr.StatusCode()) + }) }) t.Run("FilterDeletedUsers", func(t *testing.T) { @@ -947,22 +1003,30 @@ func TestGroups(t *testing.T) { // Query from the user's perspective user5View, err := user5Client.Groups(ctx, codersdk.GroupArguments{}) require.NoError(t, err) - normalizeAllGroups(user5Groups) + normalizeAllGroups(user5View) - // Everyone group and group 2 - require.Len(t, user5View, 2) + // Org members can read all groups when workspace sharing is not + // disabled, but group membership is limited to the requesting user. + // TODO(geokat): add another test with workspace sharing disabled. + require.Len(t, user5View, 3) user5ViewIDs := db2sdk.List(user5View, func(g codersdk.Group) uuid.UUID { return g.ID }) require.ElementsMatch(t, []uuid.UUID{ everyoneGroup.ID, + group1.ID, group2.ID, }, user5ViewIDs) for _, g := range user5View { - // Only expect the 1 member, themselves - require.Len(t, g.Members, 1) - require.Equal(t, user5.ReducedUser.ID, g.Members[0].MinimalUser.ID) + if g.ID == everyoneGroup.ID || g.ID == group2.ID { + // Only expect the 1 member, themselves. + require.Len(t, g.Members, 1) + require.Equal(t, user5.ReducedUser.ID, g.Members[0].MinimalUser.ID) + continue + } + + require.Empty(t, g.Members) } }) } diff --git a/enterprise/coderd/organizations.go b/enterprise/coderd/organizations.go index 5a7a4eb777..ff6861f847 100644 --- a/enterprise/coderd/organizations.go +++ b/enterprise/coderd/organizations.go @@ -12,9 +12,12 @@ import ( "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" + "github.com/coder/coder/v2/coderd/database/dbauthz" "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" ) @@ -265,6 +268,14 @@ func (api *API) postOrganizations(rw http.ResponseWriter, r *http.Request) { var organization database.Organization err = api.Database.InTx(func(tx database.Store) error { + // Serialize creation and reconciliation of the org-member + // system role across coderd instances (e.g. during rolling + // restarts). + err := tx.AcquireLock(ctx, database.LockIDReconcileSystemRoles) + if err != nil { + return xerrors.Errorf("acquire system roles reconciliation lock: %w", err) + } + if req.DisplayName == "" { req.DisplayName = req.Name } @@ -281,6 +292,24 @@ func (api *API) postOrganizations(rw http.ResponseWriter, r *http.Request) { if err != nil { return xerrors.Errorf("create organization: %w", err) } + + // Populate the placeholder system role(s) that the DB trigger + // created for us. + //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) + } + _, err = tx.InsertOrganizationMember(ctx, database.InsertOrganizationMemberParams{ OrganizationID: organization.ID, UserID: apiKey.UserID, diff --git a/enterprise/coderd/roles.go b/enterprise/coderd/roles.go index f103a8d1b0..5df6550495 100644 --- a/enterprise/coderd/roles.go +++ b/enterprise/coderd/roles.go @@ -65,6 +65,9 @@ func (api *API) postOrgRoles(rw http.ResponseWriter, r *http.Request) { SitePermissions: db2sdk.List(req.SitePermissions, sdkPermissionToDB), OrgPermissions: db2sdk.List(req.OrganizationPermissions, sdkPermissionToDB), UserPermissions: db2sdk.List(req.UserPermissions, sdkPermissionToDB), + // Satisfy the linter (we don't support member permissions in non-system roles). + MemberPermissions: database.CustomRolePermissions{}, + IsSystem: false, }) if httpapi.Is404Error(err) { httpapi.ResourceNotFound(rw) @@ -82,15 +85,15 @@ func (api *API) postOrgRoles(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Role(inserted)) } -// patchRole will allow creating a custom organization role +// putOrgRoles will allow updating a custom organization role // -// @Summary Upsert a custom organization role -// @ID upsert-a-custom-organization-role +// @Summary Update a custom organization role +// @ID update-a-custom-organization-role // @Security CoderSessionToken // @Accept json // @Produce json // @Param organization path string true "Organization ID" format(uuid) -// @Param request body codersdk.CustomRoleRequest true "Upsert role request" +// @Param request body codersdk.CustomRoleRequest true "Update role request" // @Tags Members // @Success 200 {array} codersdk.Role // @Router /organizations/{organization}/members/roles [put] @@ -126,8 +129,9 @@ func (api *API) putOrgRoles(rw http.ResponseWriter, r *http.Request) { OrganizationID: organization.ID, }, }, - ExcludeOrgRoles: false, - OrganizationID: organization.ID, + ExcludeOrgRoles: false, + OrganizationID: organization.ID, + IncludeSystemRoles: false, }) // If it is a 404 (not found) error, ignore it. if err != nil && !httpapi.Is404Error(err) { @@ -153,6 +157,8 @@ func (api *API) putOrgRoles(rw http.ResponseWriter, r *http.Request) { SitePermissions: db2sdk.List(filterInvalidPermissions(req.SitePermissions), sdkPermissionToDB), OrgPermissions: db2sdk.List(filterInvalidPermissions(req.OrganizationPermissions), sdkPermissionToDB), UserPermissions: db2sdk.List(filterInvalidPermissions(req.UserPermissions), sdkPermissionToDB), + // Satisfy the linter (we don't support member permissions in non-system roles). + MemberPermissions: database.CustomRolePermissions{}, }) if httpapi.Is404Error(err) { httpapi.ResourceNotFound(rw) @@ -197,6 +203,12 @@ func (api *API) deleteOrgRole(rw http.ResponseWriter, r *http.Request) { defer commitAudit() rolename := chi.URLParam(r, "roleName") + + // Catch requests that try to delete system roles. + if !validOrganizationRoleRequest(ctx, codersdk.CustomRoleRequest{Name: rolename}, rw) { + return + } + roles, err := api.Database.CustomRoles(ctx, database.CustomRolesParams{ LookupRoles: []database.NameOrganizationPair{ { @@ -204,7 +216,8 @@ func (api *API) deleteOrgRole(rw http.ResponseWriter, r *http.Request) { OrganizationID: organization.ID, }, }, - ExcludeOrgRoles: false, + ExcludeOrgRoles: false, + IncludeSystemRoles: false, // Linter requires all fields to be set. This field is not actually required. OrganizationID: organization.ID, }) diff --git a/enterprise/coderd/roles_test.go b/enterprise/coderd/roles_test.go index 70c432755f..8db6e59921 100644 --- a/enterprise/coderd/roles_test.go +++ b/enterprise/coderd/roles_test.go @@ -256,6 +256,59 @@ func TestCustomOrganizationRole(t *testing.T) { require.ErrorContains(t, err, "not allowed to assign user permissions") }) + // Attempt to add org member permissions, which is not allowed. + t.Run("MemberPermissions", func(t *testing.T) { + t.Parallel() + + owner, first := coderdenttest.New(t, &coderdenttest.Options{ + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureCustomRoles: 1, + }, + }, + }) + + ctx := testutil.Context(t, testutil.WaitMedium) + + role := templateAdminCustom(first.OrganizationID) + role.Name = "test-role-member-perms" + role.OrganizationMemberPermissions = []codersdk.Permission{ + { + ResourceType: codersdk.ResourceWorkspace, + Action: codersdk.ActionRead, + }, + } + + //nolint:gocritic // we want unrestricted permissions for the test + _, err := owner.CreateOrganizationRole(ctx, role) + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) + 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. + t.Run("DeleteSystemRole", func(t *testing.T) { + t.Parallel() + + owner, first := coderdenttest.New(t, &coderdenttest.Options{ + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureCustomRoles: 1, + }, + }, + }) + + ctx := testutil.Context(t, testutil.WaitMedium) + + //nolint:gocritic // we want unrestricted permissions for the test + 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") + }) + t.Run("NotFound", func(t *testing.T) { t.Parallel() owner, first := coderdenttest.New(t, &coderdenttest.Options{ diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 523f3fae1c..fe445ff749 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -3639,41 +3639,45 @@ func TestWorkspacesFiltering(t *testing.T) { dv := coderdtest.DeploymentValues(t) dv.Experiments = []string{string(codersdk.ExperimentWorkspaceSharing)} - var ( - client, db, orgOwner = coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ - Options: &coderdtest.Options{ - DeploymentValues: dv, + ownerClient, db, owner := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + DeploymentValues: dv, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureTemplateRBAC: 1, }, - LicenseOptions: &coderdenttest.LicenseOptions{ - Features: license.Features{ - codersdk.FeatureTemplateRBAC: 1, - }, - }, - }) - _, workspaceOwner = coderdtest.CreateAnotherUser(t, client, orgOwner.OrganizationID, rbac.ScopedRoleOrgAuditor(orgOwner.OrganizationID)) - sharedWorkspace = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ - OwnerID: workspaceOwner.ID, - OrganizationID: orgOwner.OrganizationID, - }).Do().Workspace - _ = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ - OwnerID: workspaceOwner.ID, - OrganizationID: orgOwner.OrganizationID, - }).Do().Workspace - ctx = testutil.Context(t, testutil.WaitMedium) - ) + }, + }) - group, err := client.CreateGroup(ctx, orgOwner.OrganizationID, codersdk.CreateGroupRequest{ + workspaceOwnerClient, workspaceOwner := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAuditor(owner.OrganizationID)) + + sharedWorkspace := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: workspaceOwner.ID, + OrganizationID: owner.OrganizationID, + }).Do().Workspace + + // Unshared workspace. + _ = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: workspaceOwner.ID, + OrganizationID: owner.OrganizationID, + }).Do().Workspace + + ctx := testutil.Context(t, testutil.WaitMedium) + + group, err := ownerClient.CreateGroup(ctx, owner.OrganizationID, codersdk.CreateGroupRequest{ Name: "wibble", }) require.NoError(t, err, "create group") - client.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{ + err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{ GroupRoles: map[string]codersdk.WorkspaceRole{ group.ID.String(): codersdk.WorkspaceRoleUse, }, }) + require.NoError(t, err, "update workspace ACL") - workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + workspaces, err := ownerClient.Workspaces(ctx, codersdk.WorkspaceFilter{ Shared: ptr.Ref(true), }) require.NoError(t, err, "fetch workspaces") @@ -3688,7 +3692,7 @@ func TestWorkspacesFiltering(t *testing.T) { dv.Experiments = []string{string(codersdk.ExperimentWorkspaceSharing)} var ( - client, db, orgOwner = coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ + ownerClient, db, owner = coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ DeploymentValues: dv, }, @@ -3698,25 +3702,26 @@ func TestWorkspacesFiltering(t *testing.T) { }, }, }) - _, workspaceOwner = coderdtest.CreateAnotherUser(t, client, orgOwner.OrganizationID, rbac.ScopedRoleOrgAuditor(orgOwner.OrganizationID)) - sharedWorkspace = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + + workspaceOwnerClient, workspaceOwner = coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAuditor(owner.OrganizationID)) + sharedWorkspace = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ OwnerID: workspaceOwner.ID, - OrganizationID: orgOwner.OrganizationID, + OrganizationID: owner.OrganizationID, }).Do().Workspace _ = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ OwnerID: workspaceOwner.ID, - OrganizationID: orgOwner.OrganizationID, + OrganizationID: owner.OrganizationID, }).Do().Workspace - _, toShareWithUser = coderdtest.CreateAnotherUser(t, client, orgOwner.OrganizationID) + _, toShareWithUser = coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID) ctx = testutil.Context(t, testutil.WaitMedium) ) - group, err := client.CreateGroup(ctx, orgOwner.OrganizationID, codersdk.CreateGroupRequest{ + group, err := ownerClient.CreateGroup(ctx, owner.OrganizationID, codersdk.CreateGroupRequest{ Name: "wibble", }) require.NoError(t, err, "create group") - client.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{ + err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{ UserRoles: map[string]codersdk.WorkspaceRole{ toShareWithUser.ID.String(): codersdk.WorkspaceRoleUse, }, @@ -3724,8 +3729,9 @@ func TestWorkspacesFiltering(t *testing.T) { group.ID.String(): codersdk.WorkspaceRoleUse, }, }) + require.NoError(t, err, "update workspace ACL") - workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + workspaces, err := ownerClient.Workspaces(ctx, codersdk.WorkspaceFilter{ Shared: ptr.Ref(true), }) require.NoError(t, err, "fetch workspaces") @@ -3740,7 +3746,7 @@ func TestWorkspacesFiltering(t *testing.T) { dv.Experiments = []string{string(codersdk.ExperimentWorkspaceSharing)} var ( - client, db, orgOwner = coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ + ownerClient, db, owner = coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ DeploymentValues: dv, }, @@ -3750,30 +3756,31 @@ func TestWorkspacesFiltering(t *testing.T) { }, }, }) - _, workspaceOwner = coderdtest.CreateAnotherUser(t, client, orgOwner.OrganizationID, rbac.ScopedRoleOrgAuditor(orgOwner.OrganizationID)) - sharedWorkspace = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + workspaceOwnerClient, workspaceOwner = coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAuditor(owner.OrganizationID)) + sharedWorkspace = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ OwnerID: workspaceOwner.ID, - OrganizationID: orgOwner.OrganizationID, + OrganizationID: owner.OrganizationID, }).Do().Workspace notSharedWorkspace = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ OwnerID: workspaceOwner.ID, - OrganizationID: orgOwner.OrganizationID, + OrganizationID: owner.OrganizationID, }).Do().Workspace ctx = testutil.Context(t, testutil.WaitMedium) ) - group, err := client.CreateGroup(ctx, orgOwner.OrganizationID, codersdk.CreateGroupRequest{ + group, err := ownerClient.CreateGroup(ctx, owner.OrganizationID, codersdk.CreateGroupRequest{ Name: "wibble", }) require.NoError(t, err, "create group") - client.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{ + err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{ GroupRoles: map[string]codersdk.WorkspaceRole{ group.ID.String(): codersdk.WorkspaceRoleUse, }, }) + require.NoError(t, err, "update workspace ACL") - workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + workspaces, err := ownerClient.Workspaces(ctx, codersdk.WorkspaceFilter{ Shared: ptr.Ref(false), }) require.NoError(t, err, "fetch workspaces") @@ -3788,7 +3795,7 @@ func TestWorkspacesFiltering(t *testing.T) { dv.Experiments = []string{string(codersdk.ExperimentWorkspaceSharing)} var ( - client, db, orgOwner = coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ + ownerClient, db, owner = coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ DeploymentValues: dv, }, @@ -3798,30 +3805,30 @@ func TestWorkspacesFiltering(t *testing.T) { }, }, }) - _, workspaceOwner = coderdtest.CreateAnotherUser(t, client, orgOwner.OrganizationID, rbac.ScopedRoleOrgAuditor(orgOwner.OrganizationID)) - sharedWorkspace = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + workspaceOwnerClient, workspaceOwner = coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAuditor(owner.OrganizationID)) + sharedWorkspace = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ OwnerID: workspaceOwner.ID, - OrganizationID: orgOwner.OrganizationID, + OrganizationID: owner.OrganizationID, }).Do().Workspace _ = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ OwnerID: workspaceOwner.ID, - OrganizationID: orgOwner.OrganizationID, + OrganizationID: owner.OrganizationID, }).Do().Workspace ctx = testutil.Context(t, testutil.WaitMedium) ) - group, err := client.CreateGroup(ctx, orgOwner.OrganizationID, codersdk.CreateGroupRequest{ + group, err := ownerClient.CreateGroup(ctx, owner.OrganizationID, codersdk.CreateGroupRequest{ Name: "wibble", }) require.NoError(t, err, "create group") - err = client.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{ + err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{ GroupRoles: map[string]codersdk.WorkspaceRole{ group.ID.String(): codersdk.WorkspaceRoleUse, }, }) require.NoError(t, err) - workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + workspaces, err := ownerClient.Workspaces(ctx, codersdk.WorkspaceFilter{ SharedWithGroup: group.ID.String(), }) require.NoError(t, err) @@ -3836,7 +3843,7 @@ func TestWorkspacesFiltering(t *testing.T) { dv.Experiments = []string{string(codersdk.ExperimentWorkspaceSharing)} var ( - client, db, orgOwner = coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ + ownerClient, db, owner = coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ DeploymentValues: dv, }, @@ -3846,44 +3853,44 @@ func TestWorkspacesFiltering(t *testing.T) { }, }, }) - _, workspaceOwner = coderdtest.CreateAnotherUser(t, client, orgOwner.OrganizationID, rbac.ScopedRoleOrgAuditor(orgOwner.OrganizationID)) - sharedWorkspace = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + workspaceOwnerClient, workspaceOwner = coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAuditor(owner.OrganizationID)) + sharedWorkspace = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ OwnerID: workspaceOwner.ID, - OrganizationID: orgOwner.OrganizationID, + OrganizationID: owner.OrganizationID, }).Do().Workspace _ = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ OwnerID: workspaceOwner.ID, - OrganizationID: orgOwner.OrganizationID, + OrganizationID: owner.OrganizationID, }).Do().Workspace ctx = testutil.Context(t, testutil.WaitMedium) ) - group, err := client.CreateGroup(ctx, orgOwner.OrganizationID, codersdk.CreateGroupRequest{ + group, err := ownerClient.CreateGroup(ctx, owner.OrganizationID, codersdk.CreateGroupRequest{ Name: "wibble", }) require.NoError(t, err, "create group") - err = client.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{ + err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{ GroupRoles: map[string]codersdk.WorkspaceRole{ group.ID.String(): codersdk.WorkspaceRoleUse, }, }) require.NoError(t, err) - workspacesByID, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + workspacesByID, err := ownerClient.Workspaces(ctx, codersdk.WorkspaceFilter{ SharedWithGroup: group.ID.String(), }) require.NoError(t, err) require.Equal(t, 1, workspacesByID.Count) require.Equal(t, sharedWorkspace.ID, workspacesByID.Workspaces[0].ID) - workspacesByName, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + workspacesByName, err := ownerClient.Workspaces(ctx, codersdk.WorkspaceFilter{ SharedWithGroup: group.Name, }) require.NoError(t, err) require.Equal(t, 1, workspacesByName.Count) require.Equal(t, sharedWorkspace.ID, workspacesByName.Workspaces[0].ID) - workspacesByOrgAndName, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + workspacesByOrgAndName, err := ownerClient.Workspaces(ctx, codersdk.WorkspaceFilter{ SharedWithGroup: fmt.Sprintf("coder/%s", group.Name), }) require.NoError(t, err) @@ -4535,7 +4542,7 @@ func TestWorkspacesSharedWith(t *testing.T) { }, }) - _, workspaceOwner := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) + workspaceOwnerClient, workspaceOwner := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) workspace := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ OwnerID: workspaceOwner.ID, @@ -4563,7 +4570,7 @@ func TestWorkspacesSharedWith(t *testing.T) { require.NoError(t, err) // Share workspace with user and group - err = client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ + err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ UserRoles: map[string]codersdk.WorkspaceRole{ sharedWithUser.ID.String(): codersdk.WorkspaceRoleUse, }, @@ -4623,7 +4630,7 @@ func TestWorkspacesSharedWith(t *testing.T) { }, }) - _, workspaceOwner := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) + workspaceOwnerClient, workspaceOwner := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) workspace := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ OwnerID: workspaceOwner.ID, @@ -4651,7 +4658,7 @@ func TestWorkspacesSharedWith(t *testing.T) { require.NoError(t, err) // Share workspace with user and group - err = client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ + err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{ UserRoles: map[string]codersdk.WorkspaceRole{ sharedWithUser.ID.String(): codersdk.WorkspaceRoleUse, },