😬

coder · Emyrk · Aug 25, 2025 · Aug 15, 2025 · Aug 15, 2025 · Aug 15, 2025
commit 21da925fb39a7c9aaf650fd26e898edfdf212604
diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go
@@ -1883,6 +1883,20 @@ func (s *MethodTestSuite) TestWorkspace() {
 		// no asserts here because SQLFilter
 		check.Args([]uuid.UUID{}, emptyPreparedAuthorized{}).Asserts()
 	}))
+	s.Run("GetWorkspaceACLByID", s.Subtest(func(db database.Store, check *expects) {
+		u := dbgen.User(s.T(), db, database.User{})
+		o := dbgen.Organization(s.T(), db, database.Organization{})
+		tpl := dbgen.Template(s.T(), db, database.Template{
+			OrganizationID: o.ID,
+			CreatedBy:      u.ID,
+		})
+		ws := dbgen.Workspace(s.T(), db, database.WorkspaceTable{
+			OwnerID:        u.ID,
+			OrganizationID: o.ID,
+			TemplateID:     tpl.ID,
+		})
+		check.Args(ws.ID).Asserts(ws, policy.ActionCreate)
+	}))
 	s.Run("UpdateWorkspaceACLByID", s.Subtest(func(db database.Store, check *expects) {
 		u := dbgen.User(s.T(), db, database.User{})
 		o := dbgen.Organization(s.T(), db, database.Organization{})

diff --git a/coderd/workspaces.go b/coderd/workspaces.go
@@ -2155,18 +2155,31 @@ func (api *API) workspaceACL(rw http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// This is largely based on the template ACL implementation, and is far from
+	// ideal. Usually, when we use the System context it's because we need to
+	// run some query that won't actually be exposed to the user. That is not
+	// the case here. This data goes directly to an unauthorized user. We are
+	// just straight up breaking security promises.
+	//
+	// Fine for now while behind the shared-workspaces experiment, but needs to
+	// be fixed before GA.
+
 	// Fetch all of the users and their organization memberships
 	userIDs := make([]uuid.UUID, 0, len(workspaceACL.Users))
 	for userID := range workspaceACL.Users {
 		userIDs = append(userIDs, uuid.MustParse(userID))
 	}
-	dbUsers, err := api.Database.GetUsersByIDs(ctx, userIDs)
+	// For context see https://github.com/coder/coder/pull/19375
+	// nolint:gocritic
+	dbUsers, err := api.Database.GetUsersByIDs(dbauthz.AsSystemRestricted(ctx), userIDs)
 	if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
 		httpapi.InternalServerError(rw, err)
 		return
 	}
 
-	orgIDsByMemberIDsRows, err := api.Database.GetOrganizationIDsByMemberIDs(r.Context(), userIDs)
+	// For context see https://github.com/coder/coder/pull/19375
+	// nolint:gocritic
+	orgIDsByMemberIDsRows, err := api.Database.GetOrganizationIDsByMemberIDs(dbauthz.AsSystemRestricted(ctx), userIDs)
 	if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
 		httpapi.InternalServerError(rw, err)
 		return
@@ -2199,14 +2212,6 @@ func (api *API) workspaceACL(rw http.ResponseWriter, r *http.Request) {
 	groups := make([]codersdk.WorkspaceGroup, 0, len(dbGroups))
 	for _, it := range dbGroups {
 		var members []database.GroupMember
-		// This is taken from the template ACL implementation, and is far from
-		// ideal. Usually, when we use the System context it's because we need to
-		// run some query that won't actually be exposed to the user. That is not
-		// the case here. This data goes directly to an unauthorized user. We are
-		// just straight up breaking security promises.
-		//
-		// Fine for now while behind the shared-workspaces experiment, but needs to
-		// be fixed before GA.
 		// For context see https://github.com/coder/coder/pull/19375
 		// nolint:gocritic
 		members, err = api.Database.GetGroupMembersByGroupID(dbauthz.AsSystemRestricted(ctx), database.GetGroupMembersByGroupIDParams{

diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go
@@ -4844,6 +4844,12 @@ func TestUpdateWorkspaceACL(t *testing.T) {
 			},
 		})
 		require.NoError(t, err)
+
+		workspaceACL, err := client.WorkspaceACL(ctx, ws.ID)
+		require.NoError(t, err)
+		require.Len(t, workspaceACL.Users, 1)
+		require.Equal(t, workspaceACL.Users[0].ID, friend.ID)
+		require.Equal(t, workspaceACL.Users[0].Role, codersdk.WorkspaceRoleAdmin)
 	})
 
 	t.Run("UnknownUserID", func(t *testing.T) {
@@ -4914,4 +4920,40 @@ func TestUpdateWorkspaceACL(t *testing.T) {
 		require.Len(t, cerr.Validations, 1)
 		require.Equal(t, cerr.Validations[0].Field, "user_roles")
 	})
+
+	t.Run("DeletedUser", func(t *testing.T) {
+		t.Parallel()
+
+		dv := coderdtest.DeploymentValues(t)
+		dv.Experiments = []string{string(codersdk.ExperimentWorkspaceSharing)}
+		adminClient := coderdtest.New(t, &coderdtest.Options{
+			IncludeProvisionerDaemon: true,
+			DeploymentValues:         dv,
+		})
+		adminUser := coderdtest.CreateFirstUser(t, adminClient)
+		orgID := adminUser.OrganizationID
+		client, _ := coderdtest.CreateAnotherUser(t, adminClient, orgID)
+		_, mike := coderdtest.CreateAnotherUser(t, adminClient, orgID)
+
+		tv := coderdtest.CreateTemplateVersion(t, adminClient, orgID, nil)
+		coderdtest.AwaitTemplateVersionJobCompleted(t, adminClient, tv.ID)
+		template := coderdtest.CreateTemplate(t, adminClient, orgID, tv.ID)
+
+		ws := coderdtest.CreateWorkspace(t, client, template.ID)
+		coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID)
+
+		ctx := testutil.Context(t, testutil.WaitMedium)
+		err := adminClient.DeleteUser(ctx, mike.ID)
+		require.NoError(t, err)
+		err = client.UpdateWorkspaceACL(ctx, ws.ID, codersdk.UpdateWorkspaceACL{
+			UserRoles: map[string]codersdk.WorkspaceRole{
+				mike.ID.String(): codersdk.WorkspaceRoleAdmin,
+			},
+		})
+		require.Error(t, err)
+		cerr, ok := codersdk.AsError(err)
+		require.True(t, ok)
+		require.Len(t, cerr.Validations, 1)
+		require.Equal(t, cerr.Validations[0].Field, "user_roles")
+	})
 }
diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go
@@ -692,7 +692,7 @@ func (c *Client) WorkspaceACL(ctx context.Context, workspaceID uuid.UUID) (Works
 		return WorkspaceACL{}, err
 	}
 	defer res.Body.Close()
-	if res.StatusCode != http.StatusNoContent {
+	if res.StatusCode != http.StatusOK {
 		return WorkspaceACL{}, ReadBodyAsError(res)
 	}
 	var acl WorkspaceACL