don't list prebuild user in org api queries

Author: SasSwart

coderd/database/dbauthz/dbauthz.go

@@ -2373,6 +2373,9 @@ func (q *querier) GetTemplateParameterInsights(ctx context.Context, arg database
 }
 
 func (q *querier) GetTemplatePresetsWithPrebuilds(ctx context.Context, templateID uuid.NullUUID) ([]database.GetTemplatePresetsWithPrebuildsRow, error) {
+	// Although this fetches presets. It filters them by prebuilds and is only of use to the prebuild system.
+	// As such, we authorize this in line with other prebuild queries, not with other preset queries.
+
 	if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceTemplate); err != nil {
 		return nil, err
 	}

coderd/database/migrations/000302_prebuilds.down.sql

@@ -4,5 +4,6 @@ DROP VIEW IF EXISTS workspace_prebuilds;
 DROP VIEW IF EXISTS workspace_latest_build;
 
 -- Revert user operations
+-- c42fdf75-3097-471c-8c33-fb52454d81c0 is the identifier for the system user responsible for prebuilds.
 DELETE FROM user_status_changes WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0';
 DELETE FROM users WHERE id = 'c42fdf75-3097-471c-8c33-fb52454d81c0';

coderd/database/migrations/000302_prebuilds.up.sql

@@ -11,7 +11,7 @@ WITH default_org AS (SELECT id
 INSERT
 INTO organization_members (organization_id, user_id, created_at, updated_at)
 SELECT default_org.id,
-	   'c42fdf75-3097-471c-8c33-fb52454d81c0',
+	   'c42fdf75-3097-471c-8c33-fb52454d81c0', -- The system user responsible for prebuilds.
 	   NOW(),
 	   NOW()
 FROM default_org;
@@ -34,14 +34,14 @@ WITH
 	-- All workspaces owned by the "prebuilds" user.
 	all_prebuilds AS (SELECT w.*
 					  FROM workspaces w
-					  WHERE w.owner_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'),
+					  WHERE w.owner_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'), -- The system user responsible for prebuilds.
 	-- All workspace agents belonging to the workspaces owned by the "prebuilds" user.
 	workspace_agents AS (SELECT w.id AS workspace_id, wa.id AS agent_id, wa.lifecycle_state, wa.ready_at
 						 FROM workspaces w
 								  INNER JOIN workspace_latest_build wlb ON wlb.workspace_id = w.id
 								  INNER JOIN workspace_resources wr ON wr.job_id = wlb.job_id
 								  INNER JOIN workspace_agents wa ON wa.resource_id = wr.id
-						 WHERE w.owner_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'
+						 WHERE w.owner_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0' -- The system user responsible for prebuilds.
 						 GROUP BY w.id, wa.id),
 	-- We can't rely on the template_version_preset_id in the workspace_builds table because this value is only set on the
 	-- initial workspace creation. Subsequent stop/start transitions will not have a value for template_version_preset_id,
@@ -64,7 +64,7 @@ WITH
 								wb.workspace_id = wbmax.workspace_id
 									AND wb.build_number = wbmax.max_build_number
 								)) lps ON lps.workspace_id = w.id
-						WHERE w.owner_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0')
+						WHERE w.owner_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0') -- The system user responsible for prebuilds.
 SELECT p.*, a.agent_id, a.lifecycle_state, a.ready_at, cp.template_version_preset_id AS current_preset_id
 FROM all_prebuilds p
 		 LEFT JOIN workspace_agents a ON a.workspace_id = p.id
@@ -73,4 +73,4 @@ FROM all_prebuilds p
 CREATE VIEW workspace_prebuild_builds AS
 SELECT *
 FROM workspace_builds
-WHERE initiator_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0';
+WHERE initiator_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'; -- The system user responsible for prebuilds.

coderd/database/queries.sql.go

@@ -22,7 +22,9 @@ WHERE
 		WHEN @user_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN
 			user_id = @user_id
 		ELSE true
-	END;
+	END
+	-- Filter system users
+	AND (users.is_system IS NULL OR users.is_system = false);
 
 -- name: InsertOrganizationMember :one
 INSERT INTO

coderd/database/queries/organizationmembers.sql

@@ -1,3 +1,20 @@
+-- name: GetTemplatePresetsWithPrebuilds :many
+SELECT t.id                        AS template_id,
+	   t.name                      AS template_name,
+	   tv.id                       AS template_version_id,
+	   tv.name                     AS template_version_name,
+	   tv.id = t.active_version_id AS using_active_version,
+	   tvpp.preset_id,
+	   tvp.name,
+	   tvpp.desired_instances      AS desired_instances,
+	   t.deleted,
+	   t.deprecated != ''          AS deprecated
+FROM templates t
+		 INNER JOIN template_versions tv ON tv.template_id = t.id
+		 INNER JOIN template_version_presets tvp ON tvp.template_version_id = tv.id
+		 INNER JOIN template_version_preset_prebuilds tvpp ON tvpp.preset_id = tvp.id
+WHERE (t.id = sqlc.narg('template_id')::uuid OR sqlc.narg('template_id') IS NULL);
+
 -- name: GetRunningPrebuilds :many
 SELECT p.id               AS workspace_id,
 	   p.name             AS workspace_name,
@@ -17,23 +34,6 @@ FROM workspace_prebuilds p
 WHERE (b.transition = 'start'::workspace_transition
 	AND pj.job_status = 'succeeded'::provisioner_job_status);
 
--- name: GetTemplatePresetsWithPrebuilds :many
-SELECT t.id                        AS template_id,
-	   t.name                      AS template_name,
-	   tv.id                       AS template_version_id,
-	   tv.name                     AS template_version_name,
-	   tv.id = t.active_version_id AS using_active_version,
-	   tvpp.preset_id,
-	   tvp.name,
-	   tvpp.desired_instances      AS desired_instances,
-	   t.deleted,
-	   t.deprecated != ''          AS deprecated
-FROM templates t
-		 INNER JOIN template_versions tv ON tv.template_id = t.id
-		 INNER JOIN template_version_presets tvp ON tvp.template_version_id = tv.id
-		 INNER JOIN template_version_preset_prebuilds tvpp ON tvpp.preset_id = tvp.id
-WHERE (t.id = sqlc.narg('template_id')::uuid OR sqlc.narg('template_id') IS NULL);
-
 -- name: GetPrebuildsInProgress :many
 SELECT t.id AS template_id, wpb.template_version_id, wpb.transition, COUNT(wpb.transition)::int AS count
 FROM workspace_latest_build wlb
@@ -107,7 +107,9 @@ SELECT
     tvp.name as preset_name,
     COUNT(*) as created_count,
     COUNT(*) FILTER (WHERE pj.job_status = 'failed'::provisioner_job_status) as failed_count,
-    COUNT(*) FILTER (WHERE w.owner_id != 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid) as claimed_count
+    COUNT(*) FILTER (
+			 WHERE w.owner_id != 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid -- The system user responsible for prebuilds.
+		) as claimed_count
 FROM workspaces w
 INNER JOIN workspace_prebuild_builds wpb ON wpb.workspace_id = w.id
 INNER JOIN templates t ON t.id = w.template_id

coderd/database/queries/prebuilds.sql

@@ -6,9 +6,6 @@ import (
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/require"
 
-	"github.com/coder/coder/v2/coderd/database/dbtestutil"
-	"github.com/coder/coder/v2/coderd/prebuilds"
-
 	"github.com/coder/coder/v2/coderd/coderdtest"
 	"github.com/coder/coder/v2/coderd/database/db2sdk"
 	"github.com/coder/coder/v2/coderd/rbac"
@@ -58,11 +55,6 @@ func TestListMembers(t *testing.T) {
 	t.Run("OK", func(t *testing.T) {
 		t.Parallel()
 
-		// TODO: we should not be returning the prebuilds user in OrganizationMembers, and this is not returned in dbmem.
-		if !dbtestutil.WillUsePostgres() {
-			t.Skip("This test requires postgres")
-		}
-
 		owner := coderdtest.New(t, nil)
 		first := coderdtest.CreateFirstUser(t, owner)
 
@@ -71,9 +63,9 @@ func TestListMembers(t *testing.T) {
 		ctx := testutil.Context(t, testutil.WaitShort)
 		members, err := client.OrganizationMembers(ctx, first.OrganizationID)
 		require.NoError(t, err)
-		require.Len(t, members, 3)
+		require.Len(t, members, 2)
 		require.ElementsMatch(t,
-			[]uuid.UUID{first.UserID, user.ID, prebuilds.OwnerID},
+			[]uuid.UUID{first.UserID, user.ID},
 			db2sdk.List(members, onlyIDs))
 	})
 }

coderd/members_test.go

@@ -8,7 +8,6 @@ import (
 	"testing"
 
 	"github.com/coder/coder/v2/coderd/database/dbtestutil"
-	"github.com/coder/coder/v2/coderd/prebuilds"
 
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/require"
@@ -369,9 +368,9 @@ func TestCustomOrganizationRole(t *testing.T) {
 		// Verify members have the custom role
 		originalMembers, err := orgAdmin.OrganizationMembers(ctx, first.OrganizationID)
 		require.NoError(t, err)
-		require.Len(t, originalMembers, 6) // 3 members + org admin + owner + prebuilds user
+		require.Len(t, originalMembers, 5) // 3 members + org admin + owner
 		for _, member := range originalMembers {
-			if member.UserID == orgAdminUser.ID || member.UserID == first.UserID || member.UserID == prebuilds.OwnerID {
+			if member.UserID == orgAdminUser.ID || member.UserID == first.UserID {
 				continue
 			}
 
@@ -386,7 +385,7 @@ func TestCustomOrganizationRole(t *testing.T) {
 		// Verify the role was removed from all members
 		members, err := orgAdmin.OrganizationMembers(ctx, first.OrganizationID)
 		require.NoError(t, err)
-		require.Len(t, members, 6) // 3 members + org admin + owner + prebuilds user
+		require.Len(t, members, 5) // 3 members + org admin + owner
 		for _, member := range members {
 			require.False(t, slices.ContainsFunc(member.Roles, func(role codersdk.SlimRole) bool {
 				return role.Name == customRoleIdentifier.Name