fix(auth/seed): relocate Purchaser group to free UUID + recover admin-backfill (closes #942)

frontend/src/permissions.ts

@@ -77,13 +77,14 @@ export type Resource =
 export const ADMINISTRATORS_GROUP_ID = '00000000-0000-5000-8000-000000000001';
 
 /**
- * Well-known group UUID for the Purchaser group seeded by migration
- * 000058 (issue #923). The three money-spending verbs
- * (execute:purchases, approve-any:purchases, retry-any:purchases) are
- * carved out of the admin:* wildcard and require explicit membership
- * in this group (or a custom group that grants the same verbs).
+ * Well-known group UUID for the Purchaser group relocated by migration
+ * 000064 (issue #942; originally seeded for issue #923). The three
+ * money-spending verbs (execute:purchases, approve-any:purchases,
+ * retry-any:purchases) are carved out of the admin:* wildcard and
+ * require explicit membership in this group (or a custom group that
+ * grants the same verbs).
  */
-export const PURCHASER_GROUP_ID = '00000000-0000-5000-8000-000000000005';
+export const PURCHASER_GROUP_ID = '00000000-0000-5000-8000-000000000007';
 
 /**
  * The set of (action, resource) pairs carved out of the admin:*

internal/auth/types.go

@@ -310,10 +310,11 @@ const (
 // group so the group card shows members on a fresh install.
 const DefaultAdminGroupID = "00000000-0000-5000-8000-000000000001"
 
-// DefaultPurchaserGroupID is the fixed UUID of the Purchaser group seeded
-// by migration 000054. It holds the three money-spending verbs carved out
-// of the admin:* wildcard (issue #923).
-const DefaultPurchaserGroupID = "00000000-0000-5000-8000-000000000005"
+// DefaultPurchaserGroupID is the fixed UUID of the Purchaser group, relocated
+// by migration 000064 to resolve the UUID collision with "Standard Users"
+// (issue #942). It holds the three money-spending verbs carved out of the
+// admin:* wildcard (issue #923).
+const DefaultPurchaserGroupID = "00000000-0000-5000-8000-000000000007"
 
 // GroupPurchaser is the canonical name of the system-managed Purchaser
 // group. MUST match the literal name inserted by migration

internal/database/postgres/migrations/000064_relocate_purchaser_group.down.sql

@@ -0,0 +1,13 @@
+-- Intentionally a no-op.
+--
+-- Reversing the UUID relocation of the Purchaser group is not safely
+-- possible: external references (user group_ids, audit logs, API tokens,
+-- and application constants) may have been written against the new UUID
+-- (00000000-0000-5000-8000-000000000007) since this migration applied.
+-- Rolling back to the old UUID (00000000-0000-5000-8000-000000000005) would
+-- silently break every one of those references.
+--
+-- To revert manually: remove Purchaser memberships from all users, delete
+-- the row at 00000000-0000-5000-8000-000000000007, and revert the constants
+-- in internal/auth/types.go and frontend/src/permissions.ts.
+SELECT 1;

internal/database/postgres/migrations/000064_relocate_purchaser_group.up.sql

@@ -0,0 +1,102 @@
+-- Relocate the Purchaser system-managed group to a free UUID.
+--
+-- Root cause (issue #942): migration 000057_drop_user_role_to_groups.up.sql
+-- claimed UUID 00000000-0000-5000-8000-000000000005 for "Standard Users".
+-- Migration 000059_seed_purchaser_group.up.sql tried to seed "Purchaser"
+-- at the same UUID and ended with ON CONFLICT (id) DO NOTHING, so the
+-- Purchaser INSERT was silently no-op'd on every database that ran both
+-- migrations. The admin-backfill in 000059 then attached admins to
+-- "Standard Users" (the row at ...000005), not to Purchaser, leaving full
+-- admins without execute:purchases / approve-any:purchases / retry-any:purchases
+-- despite holding admin:*.
+--
+-- Fix: assign Purchaser to the next free UUID in the seeded namespace:
+--   00000000-0000-5000-8000-000000000007
+-- (000005 = Standard Users, 000006 = Read-Only Users, 000007 = Purchaser)
+-- Update DefaultPurchaserGroupID in internal/auth/types.go and
+-- PURCHASER_GROUP_ID in frontend/src/permissions.ts to match.
+
+DO $$
+DECLARE
+    v_new_uuid UUID := '00000000-0000-5000-8000-000000000007';
+    v_old_uuid UUID;
+    v_occupant TEXT;
+BEGIN
+    -- Guard: fail hard if the target UUID is already taken by a
+    -- non-Purchaser row.  This prevents the same class of silent UUID
+    -- collision that caused the original bug.
+    SELECT name INTO v_occupant
+    FROM groups
+    WHERE id = v_new_uuid AND name <> 'Purchaser';
+
+    IF FOUND THEN
+        RAISE EXCEPTION
+            'migration 000064: UUID % is already claimed by group ''%''; '
+            'choose a different UUID for Purchaser before applying this migration',
+            v_new_uuid, v_occupant;
+    END IF;
+
+    -- Case 1: "Purchaser" exists but at a non-target UUID (e.g. a partial
+    -- repair attempt).  Capture the current id, swap user references, then
+    -- relocate the group row to the canonical UUID.
+    SELECT id INTO v_old_uuid
+    FROM groups
+    WHERE name = 'Purchaser' AND id <> v_new_uuid;
+
+    IF FOUND THEN
+        -- Swap the stale UUID for the canonical one in every user's
+        -- group_ids array before updating the groups PK to avoid FK issues.
+        UPDATE users
+        SET group_ids = ARRAY(
+            SELECT DISTINCT unnest(
+                array_remove(COALESCE(group_ids, '{}'), v_old_uuid) ||
+                ARRAY[v_new_uuid]
+            )
+        )
+        WHERE v_old_uuid = ANY(COALESCE(group_ids, '{}'));
+
+        UPDATE groups SET id = v_new_uuid WHERE id = v_old_uuid;
+
+    -- Case 2: No "Purchaser" row exists at all (the common bug-case on
+    -- any DB that ran 000057 before 000059).  Insert a fresh one.
+    ELSIF NOT EXISTS (SELECT 1 FROM groups WHERE name = 'Purchaser') THEN
+        INSERT INTO groups (id, name, description, permissions, allowed_accounts, system_managed)
+        VALUES (
+            v_new_uuid,
+            'Purchaser',
+            'Execute, approve, and retry purchases. Membership is required even for admins to spend money (separation of duties, issue #923).',
+            '[
+               {"action":"execute","resource":"purchases"},
+               {"action":"approve-any","resource":"purchases"},
+               {"action":"retry-any","resource":"purchases"},
+               {"action":"view","resource":"recommendations"},
+               {"action":"view","resource":"plans"},
+               {"action":"view","resource":"purchases"},
+               {"action":"view","resource":"history"}
+            ]'::jsonb,
+            ARRAY['*'],
+            TRUE
+        );
+
+    END IF;
+
+    -- Case 3: "Purchaser" already sits at v_new_uuid.
+    -- Nothing to relocate; fall through to the backfill below.
+
+    -- Admin-backfill: ensure every member of Administrators
+    -- (00000000-0000-5000-8000-000000000001) is also in Purchaser.
+    -- Idempotent: the NOT (v_new_uuid = ANY(...)) guard skips rows that are
+    -- already members; DISTINCT(unnest) deduplicates any remaining overlap.
+    -- The EXISTS guard skips the UPDATE entirely if Purchaser still does not
+    -- exist for some reason, leaving no orphaned array entries.
+    UPDATE users
+    SET group_ids = ARRAY(
+        SELECT DISTINCT unnest(
+            COALESCE(group_ids, '{}') || ARRAY[v_new_uuid]
+        )
+    )
+    WHERE '00000000-0000-5000-8000-000000000001'::UUID = ANY(COALESCE(group_ids, '{}'))
+      AND NOT (v_new_uuid = ANY(COALESCE(group_ids, '{}')))
+      AND EXISTS (SELECT 1 FROM groups WHERE id = v_new_uuid);
+
+END $$;

internal/database/postgres/migrations/000064_relocate_purchaser_group_test.go

@@ -0,0 +1,134 @@
+//go:build integration
+// +build integration
+
+package migrations_test
+
+import (
+	"context"
+	"sort"
+	"testing"
+
+	"github.com/LeanerCloud/CUDly/internal/database/postgres/migrations"
+	"github.com/LeanerCloud/CUDly/internal/database/postgres/testhelpers"
+	"github.com/jackc/pgx/v5/pgxpool"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	// purchaserGroupIDTest is the new UUID assigned to the Purchaser group
+	// by migration 000064 (relocated from 000005 to fix issue #942).
+	purchaserGroupIDTest = "00000000-0000-5000-8000-000000000007"
+	// adminGroupIDForPurchaserTest mirrors defaultAdminGroupIDTest; copied
+	// here to make the test self-contained without depending on declaration
+	// order across test files in the same package.
+	adminGroupIDForPurchaserTest = "00000000-0000-5000-8000-000000000001"
+)
+
+// queryGroupIDsByName returns the sorted group_ids of a user with the given
+// email, as UUID strings.
+func queryGroupIDsByEmail(t *testing.T, ctx context.Context, pool *pgxpool.Pool, email string) []string {
+	t.Helper()
+	var ids []string
+	err := pool.QueryRow(ctx, `
+		SELECT COALESCE(ARRAY(SELECT id::text FROM unnest(group_ids) AS id), '{}')
+		FROM users WHERE email = $1
+	`, email).Scan(&ids)
+	require.NoError(t, err, "user row for %s must exist", email)
+	sort.Strings(ids)
+	return ids
+}
+
+// TestMigration_RelocatePurchaserGroup covers issue #942: migration 000064
+// must ensure the Purchaser group exists at UUID 00000000-0000-5000-8000-000000000007
+// on databases where migration 000059 was silently no-op'd due to the UUID
+// collision with Standard Users (000057), and must backfill all Administrators
+// group members into Purchaser.
+func TestMigration_RelocatePurchaserGroup(t *testing.T) {
+	ctx := context.Background()
+	migrationsPath := getMigrationsPath()
+
+	t.Run("Purchaser seeded at new UUID after full migration run", func(t *testing.T) {
+		container, err := testhelpers.SetupPostgresContainer(ctx, t)
+		require.NoError(t, err)
+		defer container.Cleanup(ctx)
+		pool := container.DB.Pool()
+
+		require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", ""))
+
+		// Purchaser must exist at the new UUID.
+		var name string
+		err = pool.QueryRow(ctx, `SELECT name FROM groups WHERE id = $1`, purchaserGroupIDTest).Scan(&name)
+		require.NoError(t, err, "Purchaser group must exist at UUID %s", purchaserGroupIDTest)
+		assert.Equal(t, "Purchaser", name, "group at new UUID must be named 'Purchaser'")
+
+		// Round-trip: lookup by name must return the new UUID.
+		var id string
+		err = pool.QueryRow(ctx, `SELECT id FROM groups WHERE name = 'Purchaser'`).Scan(&id)
+		require.NoError(t, err, "a group named 'Purchaser' must exist")
+		assert.Equal(t, purchaserGroupIDTest, id, "Purchaser name must resolve to the new UUID")
+	})
+
+	t.Run("admin-backfill: Administrators members land in Purchaser", func(t *testing.T) {
+		container, err := testhelpers.SetupPostgresContainer(ctx, t)
+		require.NoError(t, err)
+		defer container.Cleanup(ctx)
+		pool := container.DB.Pool()
+
+		// Apply all migrations, then roll back to before 000064 so we
+		// can seed an admin and verify the backfill fires on re-apply.
+		require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", ""))
+		require.NoError(t, migrations.RollbackMigrations(ctx, pool, migrationsPath, 1))
+
+		// Seed an admin user in the Administrators group but NOT in Purchaser.
+		const adminEmail = "admin-purchaser-test@test.example"
+		_, err = pool.Exec(ctx, `
+			INSERT INTO users (id, email, password_hash, salt, active, group_ids, created_at, updated_at)
+			VALUES (gen_random_uuid(), $1, '', '', true,
+			        ARRAY[$2::uuid], NOW(), NOW())
+		`, adminEmail, adminGroupIDForPurchaserTest)
+		require.NoError(t, err)
+
+		ids := queryGroupIDsByEmail(t, ctx, pool, adminEmail)
+		require.NotContains(t, ids, purchaserGroupIDTest,
+			"test setup: admin must not be in Purchaser before migration 000064 runs")
+
+		// Re-apply 000064.
+		require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", ""))
+
+		// Admin must now be in Purchaser.
+		ids = queryGroupIDsByEmail(t, ctx, pool, adminEmail)
+		assert.Contains(t, ids, purchaserGroupIDTest,
+			"migration 000064 admin-backfill must add Administrators members to Purchaser")
+	})
+
+	t.Run("idempotent: re-running migrations does not duplicate Purchaser membership", func(t *testing.T) {
+		container, err := testhelpers.SetupPostgresContainer(ctx, t)
+		require.NoError(t, err)
+		defer container.Cleanup(ctx)
+		pool := container.DB.Pool()
+
+		// Seed an admin before first run so 000064 backfill fires.
+		require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", ""))
+
+		const adminEmail = "idempotent-test@test.example"
+		_, err = pool.Exec(ctx, `
+			INSERT INTO users (id, email, password_hash, salt, active, group_ids, created_at, updated_at)
+			VALUES (gen_random_uuid(), $1, '', '', true,
+			        ARRAY[$2::uuid, $3::uuid], NOW(), NOW())
+		`, adminEmail, adminGroupIDForPurchaserTest, purchaserGroupIDTest)
+		require.NoError(t, err)
+
+		// Run migrations again (no-op at DB level, but 000064's DO block runs again).
+		require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", ""))
+
+		ids := queryGroupIDsByEmail(t, ctx, pool, adminEmail)
+		count := 0
+		for _, id := range ids {
+			if id == purchaserGroupIDTest {
+				count++
+			}
+		}
+		assert.Equal(t, 1, count, "Purchaser UUID must appear exactly once in group_ids after two runs")
+	})
+}