fix(coderd/rbac): reinstate deployment-wide workspace.share permission for owner role (#21620)

The removal of that permission from the role broke valid use cases (e.g.
a site owner user creating a workspace owned by a system account and
then trying to share it with another user).

The bulk of the PR is made up of the rollbacks of the previously
introduced test updates necessitated by the removal.

Related to: https://github.com/coder/internal/issues/1285
This commit is contained in:
George K
2026-01-22 08:12:15 -08:00
committed by GitHub
parent 859099f1f2
commit d29a168785
8 changed files with 46 additions and 53 deletions
+3 -3
View File
@@ -197,7 +197,7 @@ func TestSharingStatus(t *testing.T) {
ctx = testutil.Context(t, testutil.WaitMedium)
)
err := workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
err := client.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 := workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
err := client.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 := workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
err := client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
UserRoles: map[string]codersdk.WorkspaceRole{
toRemoveUser2.ID.String(): codersdk.WorkspaceRoleUse,
toRemoveUser1.ID.String(): codersdk.WorkspaceRoleUse,
+2 -10
View File
@@ -42,16 +42,8 @@ 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(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},
}),
},
)),
ID: "owner",
Roles: rbac.Roles(must(rbac.RoleIdentifiers{rbac.RoleOwner()}.Expand())),
Groups: []string{},
Scope: rbac.ExpandableScope(rbac.ScopeAll),
})
+12 -13
View File
@@ -496,33 +496,32 @@ 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: siteAdminWorkspaceActions, allow: true},
{resource: ResourceWorkspace.InOrg(defOrg), actions: siteAdminWorkspaceActions, allow: true},
{resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), actions: ResourceWorkspace.AvailableActions(), allow: true},
{resource: ResourceWorkspace.InOrg(defOrg), actions: ResourceWorkspace.AvailableActions(), allow: true},
{resource: ResourceWorkspace.WithOwner(user.ID), actions: siteAdminWorkspaceActions, allow: true},
{resource: ResourceWorkspace.WithOwner(user.ID), actions: ResourceWorkspace.AvailableActions(), allow: true},
{resource: ResourceWorkspace.All(), actions: siteAdminWorkspaceActions, allow: true},
{resource: ResourceWorkspace.All(), actions: ResourceWorkspace.AvailableActions(), allow: true},
// Other org + me
{resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.ID), actions: siteAdminWorkspaceActions, allow: true},
{resource: ResourceWorkspace.InOrg(unusedID), actions: siteAdminWorkspaceActions, allow: true},
{resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.ID), actions: ResourceWorkspace.AvailableActions(), allow: true},
{resource: ResourceWorkspace.InOrg(unusedID), actions: ResourceWorkspace.AvailableActions(), allow: true},
// Other org + other user
{resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me"), actions: siteAdminWorkspaceActions, allow: true},
{resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me"), actions: ResourceWorkspace.AvailableActions(), allow: true},
{resource: ResourceWorkspace.WithOwner("not-me"), actions: siteAdminWorkspaceActions, allow: true},
{resource: ResourceWorkspace.WithOwner("not-me"), actions: ResourceWorkspace.AvailableActions(), allow: true},
// Other org + other use
{resource: ResourceWorkspace.InOrg(unusedID).WithOwner("not-me"), actions: siteAdminWorkspaceActions, allow: true},
{resource: ResourceWorkspace.InOrg(unusedID), actions: siteAdminWorkspaceActions, allow: true},
{resource: ResourceWorkspace.InOrg(unusedID).WithOwner("not-me"), actions: ResourceWorkspace.AvailableActions(), allow: true},
{resource: ResourceWorkspace.InOrg(unusedID), actions: ResourceWorkspace.AvailableActions(), allow: true},
{resource: ResourceWorkspace.WithOwner("not-me"), actions: siteAdminWorkspaceActions, allow: true},
{resource: ResourceWorkspace.WithOwner("not-me"), actions: ResourceWorkspace.AvailableActions(), allow: true},
})
user = Subject{
@@ -757,7 +756,7 @@ func TestAuthorizeLevels(t *testing.T) {
testAuthorize(t, "AdminAlwaysAllow", user,
cases(func(c authTestCase) authTestCase {
c.actions = slice.Omit(ResourceWorkspace.AvailableActions(), policy.ActionShare)
c.actions = ResourceWorkspace.AvailableActions()
c.allow = true
return c
}, []authTestCase{
+1 -1
View File
@@ -267,7 +267,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
opts = &RoleOptions{}
}
ownerWorkspaceActions := slice.Omit(ResourceWorkspace.AvailableActions(), policy.ActionShare)
ownerWorkspaceActions := ResourceWorkspace.AvailableActions()
if opts.NoOwnerWorkspaceExec {
// Remove ssh and application connect from the owner role. This
// prevents owners from have exec access to all workspaces.
+2 -2
View File
@@ -302,9 +302,9 @@ func TestRolePermissions(t *testing.T) {
InOrg(orgID).
WithOwner(currentUser.String()),
AuthorizeMap: map[bool][]hasAuthSubjects{
true: {orgAdmin, orgAdminBanWorkspace},
true: {owner, orgAdmin, orgAdminBanWorkspace},
false: {
owner, memberMe, setOtherOrg,
memberMe, setOtherOrg,
templateAdmin, userAdmin,
orgTemplateAdmin, orgUserAdmin, orgAuditor,
},
+3 -3
View File
@@ -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 = workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
err = client.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 = workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
err = client.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 = workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
err = client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
GroupRoles: map[string]codersdk.WorkspaceRole{
group1.ID.String(): codersdk.WorkspaceRoleUse,
group2.ID.String(): codersdk.WorkspaceRoleUse,
+20 -20
View File
@@ -3656,7 +3656,7 @@ func TestWorkspacesFiltering(t *testing.T) {
},
})
workspaceOwnerClient, workspaceOwner := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAuditor(owner.OrganizationID))
_, workspaceOwner := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAuditor(owner.OrganizationID))
sharedWorkspace := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
OwnerID: workspaceOwner.ID,
@@ -3676,7 +3676,7 @@ func TestWorkspacesFiltering(t *testing.T) {
})
require.NoError(t, err, "create group")
err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{
err = ownerClient.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{
GroupRoles: map[string]codersdk.WorkspaceRole{
group.ID.String(): codersdk.WorkspaceRoleUse,
},
@@ -3709,8 +3709,8 @@ func TestWorkspacesFiltering(t *testing.T) {
},
})
workspaceOwnerClient, workspaceOwner = coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAuditor(owner.OrganizationID))
sharedWorkspace = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
_, 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
@@ -3727,7 +3727,7 @@ func TestWorkspacesFiltering(t *testing.T) {
})
require.NoError(t, err, "create group")
err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{
err = ownerClient.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{
UserRoles: map[string]codersdk.WorkspaceRole{
toShareWithUser.ID.String(): codersdk.WorkspaceRoleUse,
},
@@ -3762,8 +3762,8 @@ func TestWorkspacesFiltering(t *testing.T) {
},
},
})
workspaceOwnerClient, workspaceOwner = coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAuditor(owner.OrganizationID))
sharedWorkspace = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
_, 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
@@ -3779,7 +3779,7 @@ func TestWorkspacesFiltering(t *testing.T) {
})
require.NoError(t, err, "create group")
err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{
err = ownerClient.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{
GroupRoles: map[string]codersdk.WorkspaceRole{
group.ID.String(): codersdk.WorkspaceRoleUse,
},
@@ -3811,8 +3811,8 @@ func TestWorkspacesFiltering(t *testing.T) {
},
},
})
workspaceOwnerClient, workspaceOwner = coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAuditor(owner.OrganizationID))
sharedWorkspace = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
_, 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
@@ -3827,7 +3827,7 @@ func TestWorkspacesFiltering(t *testing.T) {
Name: "wibble",
})
require.NoError(t, err, "create group")
err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{
err = ownerClient.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{
GroupRoles: map[string]codersdk.WorkspaceRole{
group.ID.String(): codersdk.WorkspaceRoleUse,
},
@@ -3859,8 +3859,8 @@ func TestWorkspacesFiltering(t *testing.T) {
},
},
})
workspaceOwnerClient, workspaceOwner = coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAuditor(owner.OrganizationID))
sharedWorkspace = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
_, 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
@@ -3875,7 +3875,7 @@ func TestWorkspacesFiltering(t *testing.T) {
Name: "wibble",
})
require.NoError(t, err, "create group")
err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{
err = ownerClient.UpdateWorkspaceACL(ctx, sharedWorkspace.ID, codersdk.UpdateWorkspaceACL{
GroupRoles: map[string]codersdk.WorkspaceRole{
group.ID.String(): codersdk.WorkspaceRoleUse,
},
@@ -4463,7 +4463,7 @@ func TestDeleteWorkspaceACL(t *testing.T) {
Name: "wibble",
})
require.NoError(t, err)
err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
err = client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
GroupRoles: map[string]codersdk.WorkspaceRole{
group.ID.String(): codersdk.WorkspaceRoleUse,
},
@@ -4512,7 +4512,7 @@ func TestDeleteWorkspaceACL(t *testing.T) {
AddUsers: []string{toShareWithUser.ID.String()},
})
require.NoError(t, err)
err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
err = client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
GroupRoles: map[string]codersdk.WorkspaceRole{
group.ID.String(): codersdk.WorkspaceRoleUse,
},
@@ -4548,7 +4548,7 @@ func TestWorkspacesSharedWith(t *testing.T) {
},
})
workspaceOwnerClient, workspaceOwner := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
_, workspaceOwner := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
workspace := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
OwnerID: workspaceOwner.ID,
@@ -4576,7 +4576,7 @@ func TestWorkspacesSharedWith(t *testing.T) {
require.NoError(t, err)
// Share workspace with user and group
err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
err = client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
UserRoles: map[string]codersdk.WorkspaceRole{
sharedWithUser.ID.String(): codersdk.WorkspaceRoleUse,
},
@@ -4636,7 +4636,7 @@ func TestWorkspacesSharedWith(t *testing.T) {
},
})
workspaceOwnerClient, workspaceOwner := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
_, workspaceOwner := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
workspace := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
OwnerID: workspaceOwner.ID,
@@ -4664,7 +4664,7 @@ func TestWorkspacesSharedWith(t *testing.T) {
require.NoError(t, err)
// Share workspace with user and group
err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
err = client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
UserRoles: map[string]codersdk.WorkspaceRole{
sharedWithUser.ID.String(): codersdk.WorkspaceRoleUse,
},
+3 -1
View File
@@ -194,7 +194,9 @@ func TestWorkspaceSharingDisabled(t *testing.T) {
require.Equal(t, "Workspace sharing is disabled for this organization.", apiErr.Message)
}
err = workspaceOwnerClient.UpdateWorkspaceACL(ctx, ws.ID, codersdk.UpdateWorkspaceACL{
// Despite the site-wide workspace.share permission for the owner,
// the endpoint should return an authz error.
err = client.UpdateWorkspaceACL(ctx, ws.ID, codersdk.UpdateWorkspaceACL{
UserRoles: map[string]codersdk.WorkspaceRole{
uuid.NewString(): codersdk.WorkspaceRoleUse,
},