Skip to content

Commit 4379230

Browse files
geokatgeokat
authored
feat: add deployment-wide option to disable workspace sharing (#21172)
Adds `--disable-workspace-sharing` option. Workspace sharing is disabled by not including user and group ACLs in the workspace RBAC object, which prevents ACL-based authz. Closes coder/internal#1072 The commit also adds saving of workspace user/group ACLs in the test DB data generator.
1 parent e31578d commit 4379230

File tree

16 files changed

+202
-2
lines changed

16 files changed

+202
-2
lines changed

‎cli/testdata/coder_server_--help.golden‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,13 @@ OPTIONS:
4646
the workspace serves malicious JavaScript. This is recommended for
4747
security purposes if a --wildcard-access-url is configured.
4848

49+
--disable-workspace-sharing bool, $CODER_DISABLE_WORKSPACE_SHARING
50+
Disable workspace sharing (requires the "workspace-sharing" experiment
51+
to be enabled). Workspace ACL checking is disabled and only owners can
52+
have ssh, apps and terminal access to workspaces. Access based on the
53+
'owner' role is also allowed unless disabled via
54+
--disable-owner-workspace-access.
55+
4956
--swagger-enable bool, $CODER_SWAGGER_ENABLE
5057
Expose the swagger endpoint via /swagger.
5158

‎cli/testdata/server-config.yaml.golden‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,12 @@ disablePathApps: false
497497
# workspaces.
498498
# (default: <unset>, type: bool)
499499
disableOwnerWorkspaceAccess: false
500+
# Disable workspace sharing (requires the "workspace-sharing" experiment to be
501+
# enabled). Workspace ACL checking is disabled and only owners can have ssh, apps
502+
# and terminal access to workspaces. Access based on the 'owner' role is also
503+
# allowed unless disabled via --disable-owner-workspace-access.
504+
# (default: <unset>, type: bool)
505+
disableWorkspaceSharing: false
500506
# These options change the behavior of how clients interact with the Coder.
501507
# Clients include the Coder CLI, Coder Desktop, IDE extensions, and the web UI.
502508
client:

‎coderd/apidoc/docs.go‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/coderd.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,10 @@ func New(options *Options) *API {
333333
})
334334
}
335335

336+
if options.DeploymentValues.DisableWorkspaceSharing {
337+
rbac.SetWorkspaceACLDisabled(true)
338+
}
339+
336340
if options.PrometheusRegistry == nil {
337341
options.PrometheusRegistry = prometheus.NewRegistry()
338342
}

‎coderd/database/dbgen/dbgen.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,16 @@ func Workspace(t testing.TB, db database.Store, orig database.WorkspaceTable) da
439439
require.NoError(t, err, "set workspace as dormant")
440440
workspace.DormantAt = orig.DormantAt
441441
}
442+
if len(orig.UserACL) > 0 || len(orig.GroupACL) > 0 {
443+
err = db.UpdateWorkspaceACLByID(genCtx, database.UpdateWorkspaceACLByIDParams{
444+
ID: workspace.ID,
445+
UserACL: orig.UserACL,
446+
GroupACL: orig.GroupACL,
447+
})
448+
require.NoError(t, err, "set workspace ACL")
449+
workspace.UserACL = orig.UserACL
450+
workspace.GroupACL = orig.GroupACL
451+
}
442452
return workspace
443453
}
444454

‎coderd/database/modelmethods.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,9 +430,16 @@ func (w WorkspaceTable) RBACObject() rbac.Object {
430430
return w.DormantRBAC()
431431
}
432432

433-
return rbac.ResourceWorkspace.WithID(w.ID).
433+
obj := rbac.ResourceWorkspace.
434+
WithID(w.ID).
434435
InOrg(w.OrganizationID).
435-
WithOwner(w.OwnerID.String()).
436+
WithOwner(w.OwnerID.String())
437+
438+
if rbac.WorkspaceACLDisabled() {
439+
return obj
440+
}
441+
442+
return obj.
436443
WithGroupACL(w.GroupACL.RBACACL()).
437444
WithACLUserList(w.UserACL.RBACACL())
438445
}

‎coderd/database/modelmethods_internal_test.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,45 @@ func TestAPIKeyScopesExpand(t *testing.T) {
143143
})
144144
}
145145

146+
//nolint:tparallel,paralleltest
147+
func TestWorkspaceACLDisabled(t *testing.T) {
148+
uid := uuid.NewString()
149+
gid := uuid.NewString()
150+
151+
ws := WorkspaceTable{
152+
ID: uuid.New(),
153+
OrganizationID: uuid.New(),
154+
OwnerID: uuid.New(),
155+
UserACL: WorkspaceACL{
156+
uid: WorkspaceACLEntry{Permissions: []policy.Action{policy.ActionSSH}},
157+
},
158+
GroupACL: WorkspaceACL{
159+
gid: WorkspaceACLEntry{Permissions: []policy.Action{policy.ActionSSH}},
160+
},
161+
}
162+
163+
t.Run("ACLsOmittedWhenDisabled", func(t *testing.T) {
164+
rbac.SetWorkspaceACLDisabled(true)
165+
t.Cleanup(func() { rbac.SetWorkspaceACLDisabled(false) })
166+
167+
obj := ws.RBACObject()
168+
169+
require.Empty(t, obj.ACLUserList, "user ACLs should be empty when disabled")
170+
require.Empty(t, obj.ACLGroupList, "group ACLs should be empty when disabled")
171+
})
172+
173+
t.Run("ACLsIncludedWhenEnabled", func(t *testing.T) {
174+
rbac.SetWorkspaceACLDisabled(false)
175+
176+
obj := ws.RBACObject()
177+
178+
require.NotEmpty(t, obj.ACLUserList, "user ACLs should be present when enabled")
179+
require.NotEmpty(t, obj.ACLGroupList, "group ACLs should be present when enabled")
180+
require.Contains(t, obj.ACLUserList, uid)
181+
require.Contains(t, obj.ACLGroupList, gid)
182+
})
183+
}
184+
146185
// Helpers
147186
func requirePermission(t *testing.T, s rbac.Scope, resource string, action policy.Action) {
148187
t.Helper()

‎coderd/rbac/object.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,3 +236,19 @@ func (z Object) WithGroupACL(groups map[string][]policy.Action) Object {
236236
AnyOrgOwner: z.AnyOrgOwner,
237237
}
238238
}
239+
240+
// TODO(geokat): similar to builtInRoles, this should ideally be
241+
// scoped to a coderd rather than a global.
242+
var workspaceACLDisabled bool
243+
244+
// SetWorkspaceACLDisabled disables/enables workspace sharing for the
245+
// deployment.
246+
func SetWorkspaceACLDisabled(v bool) {
247+
workspaceACLDisabled = v
248+
}
249+
250+
// WorkspaceACLDisabled returns true if workspace sharing is disabled
251+
// for the deployment.
252+
func WorkspaceACLDisabled() bool {
253+
return workspaceACLDisabled
254+
}

‎coderd/workspaces_test.go‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5240,6 +5240,79 @@ func TestDeleteWorkspaceACL(t *testing.T) {
52405240
})
52415241
}
52425242

5243+
// nolint:tparallel,paralleltest // Subtests modify package global.
5244+
func TestWorkspaceSharingDisabled(t *testing.T) {
5245+
t.Run("CanAccessWhenEnabled", func(t *testing.T) {
5246+
var (
5247+
client, db = coderdtest.NewWithDatabase(t, &coderdtest.Options{
5248+
DeploymentValues: coderdtest.DeploymentValues(t, func(dv *codersdk.DeploymentValues) {
5249+
dv.Experiments = []string{string(codersdk.ExperimentWorkspaceSharing)}
5250+
// DisableWorkspaceSharing is false (default)
5251+
}),
5252+
})
5253+
admin = coderdtest.CreateFirstUser(t, client)
5254+
_, wsOwner = coderdtest.CreateAnotherUser(t, client, admin.OrganizationID)
5255+
userClient, user = coderdtest.CreateAnotherUser(t, client, admin.OrganizationID)
5256+
)
5257+
5258+
ctx := testutil.Context(t, testutil.WaitMedium)
5259+
5260+
// Create workspace with ACL granting access to user
5261+
ws := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
5262+
OwnerID: wsOwner.ID,
5263+
OrganizationID: admin.OrganizationID,
5264+
UserACL: database.WorkspaceACL{
5265+
user.ID.String(): database.WorkspaceACLEntry{
5266+
Permissions: []policy.Action{
5267+
policy.ActionRead, policy.ActionSSH, policy.ActionApplicationConnect,
5268+
},
5269+
},
5270+
},
5271+
}).Do().Workspace
5272+
5273+
// User SHOULD be able to access workspace when sharing is enabled
5274+
fetchedWs, err := userClient.Workspace(ctx, ws.ID)
5275+
require.NoError(t, err)
5276+
require.Equal(t, ws.ID, fetchedWs.ID)
5277+
})
5278+
5279+
t.Run("NoAccessWhenDisabled", func(t *testing.T) {
5280+
var (
5281+
client, db = coderdtest.NewWithDatabase(t, &coderdtest.Options{
5282+
DeploymentValues: coderdtest.DeploymentValues(t, func(dv *codersdk.DeploymentValues) {
5283+
dv.Experiments = []string{string(codersdk.ExperimentWorkspaceSharing)}
5284+
dv.DisableWorkspaceSharing = true
5285+
}),
5286+
})
5287+
admin = coderdtest.CreateFirstUser(t, client)
5288+
_, wsOwner = coderdtest.CreateAnotherUser(t, client, admin.OrganizationID)
5289+
userClient, user = coderdtest.CreateAnotherUser(t, client, admin.OrganizationID)
5290+
)
5291+
5292+
ctx := testutil.Context(t, testutil.WaitMedium)
5293+
5294+
// Create workspace with ACL granting access to user directly in DB
5295+
ws := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
5296+
OwnerID: wsOwner.ID,
5297+
OrganizationID: admin.OrganizationID,
5298+
UserACL: database.WorkspaceACL{
5299+
user.ID.String(): database.WorkspaceACLEntry{
5300+
Permissions: []policy.Action{
5301+
policy.ActionRead, policy.ActionSSH, policy.ActionApplicationConnect,
5302+
},
5303+
},
5304+
},
5305+
}).Do().Workspace
5306+
5307+
// User should NOT be able to access workspace when sharing is disabled
5308+
_, err := userClient.Workspace(ctx, ws.ID)
5309+
require.Error(t, err)
5310+
var sdkErr *codersdk.Error
5311+
require.ErrorAs(t, err, &sdkErr)
5312+
require.Equal(t, http.StatusNotFound, sdkErr.StatusCode())
5313+
})
5314+
}
5315+
52435316
func TestWorkspaceCreateWithImplicitPreset(t *testing.T) {
52445317
t.Parallel()
52455318

0 commit comments

Comments
 (0)