Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc219a9a

Browse files
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 PRCloses#19209.In `templates.go`, we do this to make sure we count ALL workspaces for atemplate before we try and delete that template:https://github.com/coder/coder/blob/dc598856e3be0926573dbbe2ec680e95a139093a/coderd/templates.go#L81-L99However, 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 wedo for templates.
1 parent5ff7496 commitc219a9a

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
iferr!=nil {

‎coderd/users_test.go‎

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

383420
funcTestNotifyUserStatusChanged(t*testing.T) {

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp