fix: use system context for querying workspaces when deleting users (#19211) (#19227)

Backport of #19211 to our ESR, v2.24, since this is a kind of bad
(albeit rare) bug and very easy to fix.

### Original PR

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.
This commit is contained in:
Ethan
2025-08-07 22:32:09 +10:00
committed by GitHub
parent 5ff749617b
commit c219a9a748
2 changed files with 41 additions and 1 deletions
+4 -1
View File
@@ -542,7 +542,10 @@ func (api *API) deleteUser(rw http.ResponseWriter, r *http.Request) {
return
}
workspaces, err := api.Database.GetWorkspaces(ctx, database.GetWorkspacesParams{
// This query is ONLY done to get the workspace count, so we use a system
// context to return ALL workspaces. Not just workspaces the user can view.
// nolint:gocritic
workspaces, err := api.Database.GetWorkspaces(dbauthz.AsSystemRestricted(ctx), database.GetWorkspacesParams{
OwnerID: user.ID,
})
if err != nil {
+37
View File
@@ -378,6 +378,43 @@ func TestDeleteUser(t *testing.T) {
require.ErrorAs(t, err, &apiErr, "should be a coderd error")
require.Equal(t, http.StatusForbidden, apiErr.StatusCode(), "should be forbidden")
})
t.Run("CountCheckIncludesAllWorkspaces", func(t *testing.T) {
t.Parallel()
client, _ := coderdtest.NewWithProvisionerCloser(t, nil)
firstUser := coderdtest.CreateFirstUser(t, client)
// Create a target user who will own a workspace
targetUserClient, targetUser := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID)
// Create a User Admin who should not have permission to see the target user's workspace
userAdminClient, userAdmin := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID)
// Grant User Admin role to the userAdmin
userAdmin, err := client.UpdateUserRoles(context.Background(), userAdmin.ID.String(), codersdk.UpdateRoles{
Roles: []string{rbac.RoleUserAdmin().String()},
})
require.NoError(t, err)
// Create a template and workspace owned by the target user
version := coderdtest.CreateTemplateVersion(t, client, firstUser.OrganizationID, nil)
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, firstUser.OrganizationID, version.ID)
_ = coderdtest.CreateWorkspace(t, targetUserClient, template.ID)
workspaces, err := userAdminClient.Workspaces(context.Background(), codersdk.WorkspaceFilter{
Owner: targetUser.Username,
})
require.NoError(t, err)
require.Len(t, workspaces.Workspaces, 0)
// Attempt to delete the target user - this should fail because the
// user has a workspace not visible to the deleting user.
err = userAdminClient.DeleteUser(context.Background(), targetUser.ID)
var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusExpectationFailed, apiErr.StatusCode())
require.Contains(t, apiErr.Message, "has workspaces")
})
}
func TestNotifyUserStatusChanged(t *testing.T) {