Skip to content

Commit 99d75cc

Browse files
fix: use system context for querying workspaces when deleting users (#19211)
Closes #19209. In `templates.go`, we do this to make sure we count ALL workspaces for a template before we try and delete that template: https://github.com/coder/coder/blob/dc598856e3be0926573dbbe2ec680e95a139093a/coderd/templates.go#L81-L99 However, we weren't doing the same when attempting to delete users, leading to the linked issue. We can solve the issue the same way as we do for templates.
1 parent 9edca92 commit 99d75cc

File tree

2 files changed

+41
-1
lines changed

2 files changed

+41
-1
lines changed

coderd/users.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,10 @@ func (api *API) deleteUser(rw http.ResponseWriter, r *http.Request) {
542542
return
543543
}
544544

545-
workspaces, err := api.Database.GetWorkspaces(ctx, database.GetWorkspacesParams{
545+
// This query is ONLY done to get the workspace count, so we use a system
546+
// context to return ALL workspaces. Not just workspaces the user can view.
547+
// nolint:gocritic
548+
workspaces, err := api.Database.GetWorkspaces(dbauthz.AsSystemRestricted(ctx), database.GetWorkspacesParams{
546549
OwnerID: user.ID,
547550
})
548551
if err != nil {

coderd/users_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,43 @@ func TestDeleteUser(t *testing.T) {
377377
require.ErrorAs(t, err, &apiErr, "should be a coderd error")
378378
require.Equal(t, http.StatusForbidden, apiErr.StatusCode(), "should be forbidden")
379379
})
380+
t.Run("CountCheckIncludesAllWorkspaces", func(t *testing.T) {
381+
t.Parallel()
382+
client, _ := coderdtest.NewWithProvisionerCloser(t, nil)
383+
firstUser := coderdtest.CreateFirstUser(t, client)
384+
385+
// Create a target user who will own a workspace
386+
targetUserClient, targetUser := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID)
387+
388+
// Create a User Admin who should not have permission to see the target user's workspace
389+
userAdminClient, userAdmin := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID)
390+
391+
// Grant User Admin role to the userAdmin
392+
userAdmin, err := client.UpdateUserRoles(context.Background(), userAdmin.ID.String(), codersdk.UpdateRoles{
393+
Roles: []string{rbac.RoleUserAdmin().String()},
394+
})
395+
require.NoError(t, err)
396+
397+
// Create a template and workspace owned by the target user
398+
version := coderdtest.CreateTemplateVersion(t, client, firstUser.OrganizationID, nil)
399+
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
400+
template := coderdtest.CreateTemplate(t, client, firstUser.OrganizationID, version.ID)
401+
_ = coderdtest.CreateWorkspace(t, targetUserClient, template.ID)
402+
403+
workspaces, err := userAdminClient.Workspaces(context.Background(), codersdk.WorkspaceFilter{
404+
Owner: targetUser.Username,
405+
})
406+
require.NoError(t, err)
407+
require.Len(t, workspaces.Workspaces, 0)
408+
409+
// Attempt to delete the target user - this should fail because the
410+
// user has a workspace not visible to the deleting user.
411+
err = userAdminClient.DeleteUser(context.Background(), targetUser.ID)
412+
var apiErr *codersdk.Error
413+
require.ErrorAs(t, err, &apiErr)
414+
require.Equal(t, http.StatusExpectationFailed, apiErr.StatusCode())
415+
require.Contains(t, apiErr.Message, "has workspaces")
416+
})
380417
}
381418

382419
func TestNotifyUserStatusChanged(t *testing.T) {

0 commit comments

Comments
 (0)