mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat: promote MinimumImplicitMember experiment to GA (#27472)
Promotes the `minimum-implicit-member` experiment to GA and removes it.
## What changes
- The `minimum-implicit-member` experiment constant, its
`RoleOptions.MinimumImplicitMember` toggle, and the global
`rbac.MinimumImplicitMember()` accessor are deleted. The minimal-member
behavior is now the only behavior: `organization-member` and
`organization-service-account` carry only the floor (read-self records,
notifications, and similar) and grant **no workspace permissions**.
Workspace access lives exclusively on the
`organization-workspace-access` role.
- The experiment gate on customizing `default_org_member_roles` (`PATCH
/organizations/{org}`) is removed; the built-in-roles-only validation
remains.
- The dashboard's Default Roles section and the implied-roles display on
the members page are no longer experiment-gated.
- Admin docs: new "Default member roles" section in
`docs/admin/users/organizations.md`, cross-linked from
`groups-roles.md`.
## Why this is safe for existing deployments
Migration `000516` (shipped earlier) backfilled
`default_org_member_roles` with `['organization-workspace-access']` on
every organization. Members therefore keep exactly the effective
permissions they had with the experiment off; the workspace elevation
flows through the default role instead of being baked into
`organization-member`.
**Rollback caveat:** rolling back past this release restores the bundled
elevation, silently re-granting workspace access to members of
organizations that cleared their default roles.
## Review
Deep-review R1 findings are addressed in `chore: address deep-review
findings` (copy fixes, read-only Default Roles for viewers, removable
overlapping explicit grants, RBAC prose restoration, test
de-tautologizing, docs). Point-by-point disposition is in the PR
comments.
---
Generated by Coder Agents on behalf of @Emyrk.
This commit is contained in:
@@ -307,6 +307,7 @@ func TestAuthorizeDomain(t *testing.T) {
|
||||
Roles: Roles{
|
||||
must(RoleByName(RoleMember())),
|
||||
orgMemberRole(defOrg),
|
||||
must(RoleByName(ScopedRoleOrgWorkspaceAccess(defOrg))),
|
||||
},
|
||||
}
|
||||
|
||||
@@ -467,6 +468,7 @@ func TestAuthorizeDomain(t *testing.T) {
|
||||
Roles: Roles{
|
||||
must(RoleByName(ScopedRoleOrgAdmin(defOrg))),
|
||||
orgMemberRole(defOrg),
|
||||
must(RoleByName(ScopedRoleOrgWorkspaceAccess(defOrg))),
|
||||
must(RoleByName(RoleMember())),
|
||||
},
|
||||
}
|
||||
@@ -545,6 +547,7 @@ func TestAuthorizeDomain(t *testing.T) {
|
||||
Scope: must(ExpandScope(ScopeApplicationConnect)),
|
||||
Roles: Roles{
|
||||
orgMemberRole(defOrg),
|
||||
must(RoleByName(ScopedRoleOrgWorkspaceAccess(defOrg))),
|
||||
must(RoleByName(RoleMember())),
|
||||
},
|
||||
}
|
||||
@@ -1049,6 +1052,7 @@ func TestAuthorizeScope(t *testing.T) {
|
||||
Roles: Roles{
|
||||
must(RoleByName(RoleMember())),
|
||||
orgMemberRole(defOrg),
|
||||
must(RoleByName(ScopedRoleOrgWorkspaceAccess(defOrg))),
|
||||
},
|
||||
Scope: must(ExpandScope(ScopeApplicationConnect)),
|
||||
}
|
||||
@@ -1085,6 +1089,7 @@ func TestAuthorizeScope(t *testing.T) {
|
||||
Roles: Roles{
|
||||
must(RoleByName(RoleMember())),
|
||||
orgMemberRole(defOrg),
|
||||
must(RoleByName(ScopedRoleOrgWorkspaceAccess(defOrg))),
|
||||
},
|
||||
Scope: Scope{
|
||||
Role: Role{
|
||||
@@ -1174,6 +1179,7 @@ func TestAuthorizeScope(t *testing.T) {
|
||||
Roles: Roles{
|
||||
must(RoleByName(RoleMember())),
|
||||
orgMemberRole(defOrg),
|
||||
must(RoleByName(ScopedRoleOrgWorkspaceAccess(defOrg))),
|
||||
},
|
||||
Scope: Scope{
|
||||
Role: Role{
|
||||
@@ -1229,6 +1235,7 @@ func TestAuthorizeScope(t *testing.T) {
|
||||
Roles: Roles{
|
||||
must(RoleByName(RoleMember())),
|
||||
orgMemberRole(defOrg),
|
||||
must(RoleByName(ScopedRoleOrgWorkspaceAccess(defOrg))),
|
||||
},
|
||||
Scope: must(ScopeNoUserData.Expand()),
|
||||
}
|
||||
|
||||
@@ -267,16 +267,3 @@ func SetChatACLDisabled(v bool) {
|
||||
func ChatACLDisabled() bool {
|
||||
return chatACLDisabled.Load()
|
||||
}
|
||||
|
||||
// minimumImplicitMember mirrors RoleOptions.MinimumImplicitMember.
|
||||
// Stored as a global because OrgMemberPermissions and
|
||||
// OrgServiceAccountPermissions are called from rolestore without
|
||||
// access to api instance state.
|
||||
var minimumImplicitMember atomic.Bool
|
||||
|
||||
// MinimumImplicitMember reports whether the workspace-ops elevation
|
||||
// has been stripped from organization-member and
|
||||
// organization-service-account. See RoleOptions.MinimumImplicitMember.
|
||||
func MinimumImplicitMember() bool {
|
||||
return minimumImplicitMember.Load()
|
||||
}
|
||||
|
||||
+23
-39
@@ -4,7 +4,6 @@ import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"slices"
|
||||
"sort"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
@@ -223,9 +222,14 @@ func DefaultOrgMemberRoles() []string {
|
||||
return []string{orgWorkspaceAccess}
|
||||
}
|
||||
|
||||
// OrgWorkspaceAccessMemberPerms returns the elevation perms granted by the
|
||||
// organization-workspace-access role.
|
||||
func OrgWorkspaceAccessMemberPerms() []Permission {
|
||||
// orgWorkspaceAccessMemberPerms returns the member-scoped permissions
|
||||
// granted by the organization-workspace-access role: the ability to
|
||||
// create and operate your own workspaces in the organization. The
|
||||
// organization-member role intentionally does not include these
|
||||
// permissions (see OrgMemberPermissions), so workspace access is only
|
||||
// held by members that have this role, typically through the
|
||||
// organization's default_org_member_roles.
|
||||
func orgWorkspaceAccessMemberPerms() []Permission {
|
||||
return Permissions(map[string][]policy.Action{
|
||||
ResourceWorkspace.Type: ResourceWorkspace.AvailableActions(),
|
||||
|
||||
@@ -340,14 +344,6 @@ type RoleOptions struct {
|
||||
NoOwnerWorkspaceExec bool
|
||||
NoWorkspaceSharing bool
|
||||
NoChatSharing bool
|
||||
|
||||
// MinimumImplicitMember removes the workspace-ops elevation
|
||||
// (OrgWorkspaceAccessMemberPerms) from organization-member and
|
||||
// organization-service-account. With it set, those two roles carry
|
||||
// only the floor, and the elevation must be granted explicitly via
|
||||
// the organization-workspace-access role (typically attached
|
||||
// through default_org_member_roles).
|
||||
MinimumImplicitMember bool
|
||||
}
|
||||
|
||||
// ReservedRoleName exists because the database should only allow unique role
|
||||
@@ -369,8 +365,6 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
|
||||
opts = &RoleOptions{}
|
||||
}
|
||||
|
||||
minimumImplicitMember.Store(opts.MinimumImplicitMember)
|
||||
|
||||
denyPermissions := []Permission{}
|
||||
if opts.NoWorkspaceSharing {
|
||||
denyPermissions = append(denyPermissions, Permission{
|
||||
@@ -728,7 +722,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
|
||||
ByOrgID: map[string]OrgPermissions{
|
||||
organizationID.String(): {
|
||||
Org: []Permission{},
|
||||
Member: OrgWorkspaceAccessMemberPerms(),
|
||||
Member: orgWorkspaceAccessMemberPerms(),
|
||||
},
|
||||
},
|
||||
}
|
||||
@@ -1074,8 +1068,8 @@ func Permissions(perms map[string][]policy.Action) []Permission {
|
||||
}
|
||||
}
|
||||
// Deterministic ordering of permissions
|
||||
sort.Slice(list, func(i, j int) bool {
|
||||
return list[i].ResourceType < list[j].ResourceType
|
||||
slices.SortFunc(list, func(a, b Permission) int {
|
||||
return strings.Compare(a.ResourceType, b.ResourceType)
|
||||
})
|
||||
return list
|
||||
}
|
||||
@@ -1144,6 +1138,16 @@ type OrgRolePermissions struct {
|
||||
// OrgMemberPermissions returns the permissions for the organization-member
|
||||
// system role, which can vary based on the organization's workspace sharing
|
||||
// settings.
|
||||
//
|
||||
// organization-member carries only the "floor": the minimum permission
|
||||
// set every member of an organization holds (read-self records,
|
||||
// notifications, and similar). It deliberately grants no workspace
|
||||
// access. The ability to create and use workspaces lives exclusively on
|
||||
// the organization-workspace-access role (see
|
||||
// orgWorkspaceAccessMemberPerms), which organizations attach to members
|
||||
// through default_org_member_roles or explicit assignment. This is what
|
||||
// makes restricted "gateway account" members possible: clear the
|
||||
// default roles and members keep the floor but cannot touch workspaces.
|
||||
func OrgMemberPermissions(org OrgSettings) OrgRolePermissions {
|
||||
// Organization-level permissions that all org members get.
|
||||
orgPermMap := map[string][]policy.Action{
|
||||
@@ -1184,7 +1188,7 @@ func OrgMemberPermissions(org OrgSettings) OrgRolePermissions {
|
||||
|
||||
// Chat access requires the agents-access role and is intentionally
|
||||
// not granted in the floor.
|
||||
floor := Permissions(map[string][]policy.Action{
|
||||
memberPerms := Permissions(map[string][]policy.Action{
|
||||
// Read-self org-member record.
|
||||
ResourceOrganizationMember.Type: {policy.ActionRead},
|
||||
|
||||
@@ -1207,19 +1211,6 @@ func OrgMemberPermissions(org OrgSettings) OrgRolePermissions {
|
||||
ResourceInboxNotification.Type: ResourceInboxNotification.AvailableActions(),
|
||||
})
|
||||
|
||||
// Workspace-ops elevation. When MinimumImplicitMember is off, the
|
||||
// elevation is bundled into organization-member here. When on, the
|
||||
// elevation lives exclusively on organization-workspace-access; a
|
||||
// user without that role then has only the floor. See
|
||||
// OrgWorkspaceAccessMemberPerms for the perm set and the
|
||||
// "Intentionally omitted" rationale.
|
||||
var elevation []Permission
|
||||
if !MinimumImplicitMember() {
|
||||
elevation = OrgWorkspaceAccessMemberPerms()
|
||||
}
|
||||
|
||||
memberPerms := slices.Concat(elevation, floor)
|
||||
|
||||
if org.ShareableWorkspaceOwners != ShareableWorkspaceOwnersEveryone {
|
||||
memberPerms = append(memberPerms, Permission{
|
||||
Negate: true,
|
||||
@@ -1265,7 +1256,7 @@ func OrgServiceAccountPermissions(org OrgSettings) OrgRolePermissions {
|
||||
})
|
||||
}
|
||||
|
||||
floor := Permissions(map[string][]policy.Action{
|
||||
memberPerms := Permissions(map[string][]policy.Action{
|
||||
// Read-self org-member record.
|
||||
ResourceOrganizationMember.Type: {policy.ActionRead},
|
||||
|
||||
@@ -1289,12 +1280,5 @@ func OrgServiceAccountPermissions(org OrgSettings) OrgRolePermissions {
|
||||
ResourceInboxNotification.Type: ResourceInboxNotification.AvailableActions(),
|
||||
})
|
||||
|
||||
var elevation []Permission
|
||||
if !MinimumImplicitMember() {
|
||||
elevation = OrgWorkspaceAccessMemberPerms()
|
||||
}
|
||||
|
||||
memberPerms := slices.Concat(elevation, floor)
|
||||
|
||||
return OrgRolePermissions{Org: orgPerms, Member: memberPerms}
|
||||
}
|
||||
|
||||
+25
-44
@@ -203,60 +203,41 @@ func TestOwnerExec(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
// TestMinimumImplicitMember verifies the floor/elevation gate on
|
||||
// organization-member and organization-service-account. When the option
|
||||
// is off (default), both roles carry the workspace-ops elevation. When
|
||||
// on, both roles carry only the floor and the elevation must be
|
||||
// granted explicitly via organization-workspace-access.
|
||||
// TestMemberRolesExcludeWorkspacePerms verifies that organization-member
|
||||
// and organization-service-account grant no workspace permissions, and
|
||||
// that the registered organization-workspace-access role is what carries
|
||||
// them.
|
||||
//
|
||||
//nolint:tparallel,paralleltest
|
||||
func TestMinimumImplicitMember(t *testing.T) {
|
||||
// Reads the global builtin role registry via RoleByName, which sibling
|
||||
// tests reload, so it must run serially.
|
||||
//
|
||||
//nolint:paralleltest
|
||||
func TestMemberRolesExcludeWorkspacePerms(t *testing.T) {
|
||||
orgSettings := rbac.OrgSettings{
|
||||
ShareableWorkspaceOwners: rbac.ShareableWorkspaceOwnersEveryone,
|
||||
}
|
||||
|
||||
hasResource := func(perms []rbac.Permission, resource string) bool {
|
||||
for _, p := range perms {
|
||||
if p.ResourceType == resource && !p.Negate {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
return slices.ContainsFunc(perms, func(p rbac.Permission) bool {
|
||||
return p.ResourceType == resource && !p.Negate
|
||||
})
|
||||
}
|
||||
|
||||
// ResourceWorkspace is granted by the elevation
|
||||
// (OrgWorkspaceAccessMemberPerms) and not by the floor, so it acts as
|
||||
// a witness for whether the elevation is bundled in.
|
||||
elevationWitness := rbac.ResourceWorkspace.Type
|
||||
// ResourceOrganizationMember is part of the floor; floor must remain
|
||||
// regardless of the option.
|
||||
floorWitness := rbac.ResourceOrganizationMember.Type
|
||||
member := rbac.OrgMemberPermissions(orgSettings).Member
|
||||
require.False(t, hasResource(member, rbac.ResourceWorkspace.Type), "organization-member must not grant workspace permissions")
|
||||
require.True(t, hasResource(member, rbac.ResourceOrganizationMember.Type), "organization-member should grant read-self")
|
||||
|
||||
t.Run("Off", func(t *testing.T) {
|
||||
rbac.ReloadBuiltinRoles(nil)
|
||||
t.Cleanup(func() { rbac.ReloadBuiltinRoles(nil) })
|
||||
sa := rbac.OrgServiceAccountPermissions(orgSettings).Member
|
||||
require.False(t, hasResource(sa, rbac.ResourceWorkspace.Type), "organization-service-account must not grant workspace permissions")
|
||||
require.True(t, hasResource(sa, rbac.ResourceOrganizationMember.Type), "organization-service-account should grant read-self")
|
||||
|
||||
member := rbac.OrgMemberPermissions(orgSettings).Member
|
||||
require.True(t, hasResource(member, elevationWitness), "organization-member should include the elevation when MinimumImplicitMember is off")
|
||||
require.True(t, hasResource(member, floorWitness), "organization-member should include the floor")
|
||||
|
||||
sa := rbac.OrgServiceAccountPermissions(orgSettings).Member
|
||||
require.True(t, hasResource(sa, elevationWitness), "organization-service-account should include the elevation when MinimumImplicitMember is off")
|
||||
require.True(t, hasResource(sa, floorWitness), "organization-service-account should include the floor")
|
||||
})
|
||||
|
||||
t.Run("On", func(t *testing.T) {
|
||||
rbac.ReloadBuiltinRoles(&rbac.RoleOptions{MinimumImplicitMember: true})
|
||||
t.Cleanup(func() { rbac.ReloadBuiltinRoles(nil) })
|
||||
|
||||
member := rbac.OrgMemberPermissions(orgSettings).Member
|
||||
require.False(t, hasResource(member, elevationWitness), "organization-member should drop the elevation when MinimumImplicitMember is on")
|
||||
require.True(t, hasResource(member, floorWitness), "organization-member should still include the floor")
|
||||
|
||||
sa := rbac.OrgServiceAccountPermissions(orgSettings).Member
|
||||
require.False(t, hasResource(sa, elevationWitness), "organization-service-account should drop the elevation when MinimumImplicitMember is on")
|
||||
require.True(t, hasResource(sa, floorWitness), "organization-service-account should still include the floor")
|
||||
})
|
||||
// The registered organization-workspace-access role is the grant
|
||||
// path for workspace permissions.
|
||||
orgID := uuid.New()
|
||||
wsAccess, err := rbac.RoleByName(rbac.ScopedRoleOrgWorkspaceAccess(orgID))
|
||||
require.NoError(t, err)
|
||||
require.True(t, hasResource(wsAccess.ByOrgID[orgID.String()].Member, rbac.ResourceWorkspace.Type),
|
||||
"organization-workspace-access should grant workspace permissions")
|
||||
}
|
||||
|
||||
// These were "pared down" in https://github.com/coder/coder/pull/21359 to avoid
|
||||
|
||||
Reference in New Issue
Block a user