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

Commit99d75cc

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-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 we do for templates.
1 parent9edca92 commit99d75cc

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
@@ -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+
varapiErr*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
funcTestNotifyUserStatusChanged(t*testing.T) {

0 commit comments

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