Skip to content

Commit b78d140

Browse files
EmyrkEmyrk
committed
feat(coderd): gate org-member workspace elevation behind experiment
Adds RoleOptions.MinimumImplicitMember. When the minimum-implicit-member experiment is on, OrgMemberPermissions and OrgServiceAccountPermissions omit the workspace-ops elevation (OrgWorkspaceAccessMemberPerms). Members of the org then only have the floor unless granted organization-workspace-access via default_org_member_roles or direct assignment. Read once at startup from coderd.New. Flip the experiment, then restart coderd. Refs #25936.
1 parent 6c14675 commit b78d140

4 files changed

Lines changed: 102 additions & 14 deletions

File tree

‎coderd/coderd.go‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -348,11 +348,16 @@ func New(options *Options) *API {
348348
panic("developer error: options.PrometheusRegistry is nil and not running a unit test")
349349
}
350350

351-
if options.DeploymentValues.DisableOwnerWorkspaceExec || options.DeploymentValues.DisableWorkspaceSharing || options.DeploymentValues.DisableChatSharing {
351+
experiments := ReadExperiments(
352+
options.Logger, options.DeploymentValues.Experiments.Value(),
353+
)
354+
355+
if bool(options.DeploymentValues.DisableOwnerWorkspaceExec) || bool(options.DeploymentValues.DisableWorkspaceSharing) || bool(options.DeploymentValues.DisableChatSharing) || experiments.Enabled(codersdk.ExperimentMinimumImplicitMember) {
352356
rbac.ReloadBuiltinRoles(&rbac.RoleOptions{
353-
NoOwnerWorkspaceExec: bool(options.DeploymentValues.DisableOwnerWorkspaceExec),
354-
NoWorkspaceSharing: bool(options.DeploymentValues.DisableWorkspaceSharing),
355-
NoChatSharing: bool(options.DeploymentValues.DisableChatSharing),
357+
NoOwnerWorkspaceExec: bool(options.DeploymentValues.DisableOwnerWorkspaceExec),
358+
NoWorkspaceSharing: bool(options.DeploymentValues.DisableWorkspaceSharing),
359+
NoChatSharing: bool(options.DeploymentValues.DisableChatSharing),
360+
MinimumImplicitMember: experiments.Enabled(codersdk.ExperimentMinimumImplicitMember),
356361
})
357362
}
358363

@@ -391,9 +396,6 @@ func New(options *Options) *API {
391396
options.IDPSync = idpsync.NewAGPLSync(options.Logger, options.RuntimeConfig, idpsync.FromDeploymentValues(options.DeploymentValues))
392397
}
393398

394-
experiments := ReadExperiments(
395-
options.Logger, options.DeploymentValues.Experiments.Value(),
396-
)
397399
if options.AppHostname != "" && options.AppHostnameRegex == nil || options.AppHostname == "" && options.AppHostnameRegex != nil {
398400
panic("coderd: both AppHostname and AppHostnameRegex must be set or unset")
399401
}

‎coderd/rbac/object.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,3 +267,16 @@ func SetChatACLDisabled(v bool) {
267267
func ChatACLDisabled() bool {
268268
return chatACLDisabled.Load()
269269
}
270+
271+
// minimumImplicitMember mirrors RoleOptions.MinimumImplicitMember.
272+
// Stored as a global because OrgMemberPermissions and
273+
// OrgServiceAccountPermissions are called from rolestore without
274+
// access to api instance state.
275+
var minimumImplicitMember atomic.Bool
276+
277+
// MinimumImplicitMember reports whether the workspace-ops elevation
278+
// has been stripped from organization-member and
279+
// organization-service-account. See RoleOptions.MinimumImplicitMember.
280+
func MinimumImplicitMember() bool {
281+
return minimumImplicitMember.Load()
282+
}

‎coderd/rbac/roles.go‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,14 @@ type RoleOptions struct {
320320
NoOwnerWorkspaceExec bool
321321
NoWorkspaceSharing bool
322322
NoChatSharing bool
323+
324+
// MinimumImplicitMember removes the workspace-ops elevation
325+
// (OrgWorkspaceAccessMemberPerms) from organization-member and
326+
// organization-service-account. With it set, those two roles carry
327+
// only the floor, and the elevation must be granted explicitly via
328+
// the organization-workspace-access role (typically attached
329+
// through default_org_member_roles).
330+
MinimumImplicitMember bool
323331
}
324332

325333
// ReservedRoleName exists because the database should only allow unique role
@@ -341,6 +349,8 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
341349
opts = &RoleOptions{}
342350
}
343351

352+
minimumImplicitMember.Store(opts.MinimumImplicitMember)
353+
344354
denyPermissions := []Permission{}
345355
if opts.NoWorkspaceSharing {
346356
denyPermissions = append(denyPermissions, Permission{
@@ -1171,12 +1181,16 @@ func OrgMemberPermissions(org OrgSettings) OrgRolePermissions {
11711181
ResourceInboxNotification.Type: ResourceInboxNotification.AvailableActions(),
11721182
})
11731183

1174-
// Workspace-ops elevation. Today bundled into organization-member;
1175-
// the minimum-implicit-member experiment will move the binding
1176-
// exclusively onto organization-workspace-access so a user without
1177-
// that role has only the floor. See OrgWorkspaceAccessMemberPerms
1178-
// for the perm set and the "Intentionally omitted" rationale.
1179-
elevation := OrgWorkspaceAccessMemberPerms()
1184+
// Workspace-ops elevation. When MinimumImplicitMember is off, the
1185+
// elevation is bundled into organization-member here. When on, the
1186+
// elevation lives exclusively on organization-workspace-access; a
1187+
// user without that role then has only the floor. See
1188+
// OrgWorkspaceAccessMemberPerms for the perm set and the
1189+
// "Intentionally omitted" rationale.
1190+
var elevation []Permission
1191+
if !MinimumImplicitMember() {
1192+
elevation = OrgWorkspaceAccessMemberPerms()
1193+
}
11801194

11811195
memberPerms := slices.Concat(elevation, floor)
11821196

@@ -1249,7 +1263,10 @@ func OrgServiceAccountPermissions(org OrgSettings) OrgRolePermissions {
12491263
ResourceInboxNotification.Type: ResourceInboxNotification.AvailableActions(),
12501264
})
12511265

1252-
elevation := OrgWorkspaceAccessMemberPerms()
1266+
var elevation []Permission
1267+
if !MinimumImplicitMember() {
1268+
elevation = OrgWorkspaceAccessMemberPerms()
1269+
}
12531270

12541271
memberPerms := slices.Concat(elevation, floor)
12551272

‎coderd/rbac/roles_test.go‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,62 @@ func TestOwnerExec(t *testing.T) {
203203
})
204204
}
205205

206+
// TestMinimumImplicitMember verifies the floor/elevation gate on
207+
// organization-member and organization-service-account. When the option
208+
// is off (default), both roles carry the workspace-ops elevation. When
209+
// on, both roles carry only the floor and the elevation must be
210+
// granted explicitly via organization-workspace-access.
211+
//
212+
//nolint:tparallel,paralleltest
213+
func TestMinimumImplicitMember(t *testing.T) {
214+
orgSettings := rbac.OrgSettings{
215+
ShareableWorkspaceOwners: rbac.ShareableWorkspaceOwnersEveryone,
216+
}
217+
218+
hasResource := func(perms []rbac.Permission, resource string) bool {
219+
for _, p := range perms {
220+
if p.ResourceType == resource && !p.Negate {
221+
return true
222+
}
223+
}
224+
return false
225+
}
226+
227+
// ResourceWorkspace is granted by the elevation
228+
// (OrgWorkspaceAccessMemberPerms) and not by the floor, so it acts as
229+
// a witness for whether the elevation is bundled in.
230+
elevationWitness := rbac.ResourceWorkspace.Type
231+
// ResourceOrganizationMember is part of the floor; floor must remain
232+
// regardless of the option.
233+
floorWitness := rbac.ResourceOrganizationMember.Type
234+
235+
t.Run("Off", func(t *testing.T) {
236+
rbac.ReloadBuiltinRoles(nil)
237+
t.Cleanup(func() { rbac.ReloadBuiltinRoles(nil) })
238+
239+
member := rbac.OrgMemberPermissions(orgSettings).Member
240+
require.True(t, hasResource(member, elevationWitness), "organization-member should include the elevation when MinimumImplicitMember is off")
241+
require.True(t, hasResource(member, floorWitness), "organization-member should include the floor")
242+
243+
sa := rbac.OrgServiceAccountPermissions(orgSettings).Member
244+
require.True(t, hasResource(sa, elevationWitness), "organization-service-account should include the elevation when MinimumImplicitMember is off")
245+
require.True(t, hasResource(sa, floorWitness), "organization-service-account should include the floor")
246+
})
247+
248+
t.Run("On", func(t *testing.T) {
249+
rbac.ReloadBuiltinRoles(&rbac.RoleOptions{MinimumImplicitMember: true})
250+
t.Cleanup(func() { rbac.ReloadBuiltinRoles(nil) })
251+
252+
member := rbac.OrgMemberPermissions(orgSettings).Member
253+
require.False(t, hasResource(member, elevationWitness), "organization-member should drop the elevation when MinimumImplicitMember is on")
254+
require.True(t, hasResource(member, floorWitness), "organization-member should still include the floor")
255+
256+
sa := rbac.OrgServiceAccountPermissions(orgSettings).Member
257+
require.False(t, hasResource(sa, elevationWitness), "organization-service-account should drop the elevation when MinimumImplicitMember is on")
258+
require.True(t, hasResource(sa, floorWitness), "organization-service-account should still include the floor")
259+
})
260+
}
261+
206262
// These were "pared down" in https://github.com/coder/coder/pull/21359 to avoid
207263
// using the now DB-backed organization-member role. As a result, they no longer
208264
// model real-world org-scoped users (who also have organization-member).

0 commit comments

Comments
 (0)