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

ethanndickson · web-flow · commit c219a9a74872 · 2025-08-07T22:32:09.000+10:00
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.
diff --git a/coderd/users.go b/coderd/users.go
@@ -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 {
diff --git a/coderd/users_test.go b/coderd/users_test.go
@@ -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) {
