fix: dedupe any duplicate permissions (#5397)

https://linear.app/unleash/issue/2-1656/create-db-migration-that-ensures-correct-state-of-permissions

This adds a migration that dedupes any duplicate permissions.

src/migrations/20231122121456-dedupe-any-duplicate-permissions.js

@@ -0,0 +1,72 @@
+exports.up = function (db, cb) {
+    db.runSql(
+        `
+-- STEP 1: Update references in the role_permission table, setting the permission to the lowest ID for each duplicate permission
+
+WITH Duplicates AS (
+    SELECT
+        MIN(id) AS min_id,
+        permission
+    FROM
+        permissions
+    GROUP BY
+        permission
+    HAVING
+        COUNT(*) > 1
+)
+
+UPDATE
+    role_permission
+SET
+    permission_id = d.min_id
+FROM
+    Duplicates d
+JOIN
+    permissions p ON d.permission = p.permission
+WHERE
+    role_permission.permission_id = p.id
+AND
+    role_permission.permission_id != d.min_id;
+
+
+-- STEP 2: Delete redundant role_permission entries
+
+DELETE FROM role_permission
+WHERE ctid IN (
+    SELECT ctid
+    FROM (
+        SELECT ctid, ROW_NUMBER() OVER (PARTITION BY role_id, permission_id, environment ORDER BY created_at) as rn
+        FROM role_permission
+    ) t
+    WHERE t.rn > 1
+);
+
+-- STEP 3: Delete the duplicate permissions, keeping the ones with the lowest ID
+
+WITH Duplicates AS (
+    SELECT
+        MIN(id) AS min_id,
+        permission
+    FROM
+        permissions
+    GROUP BY
+        permission
+    HAVING
+        COUNT(*) > 1
+)
+
+DELETE FROM permissions
+WHERE id NOT IN (
+    SELECT min_id FROM Duplicates
+)
+AND permission IN (
+    SELECT permission FROM Duplicates
+);
+        `,
+        cb
+    );
+};
+
+exports.down = function (db, callback) {
+    callback();
+};

src/test/e2e/dedupe-permissions.e2e.test.ts

@@ -0,0 +1,173 @@
+import { getDbConfig } from './helpers/database-config';
+import { createTestConfig } from '../config/test-config';
+import { getInstance } from 'db-migrate';
+import { log } from 'db-migrate-shared';
+import { Client } from 'pg';
+import { IDBOption } from 'lib/types';
+
+log.setLogLevel('error');
+
+async function initSchema(db: IDBOption): Promise<void> {
+    const client = new Client(db);
+    await client.connect();
+    await client.query(`DROP SCHEMA IF EXISTS ${db.schema} CASCADE`);
+    await client.query(`CREATE SCHEMA IF NOT EXISTS ${db.schema}`);
+    await client.end();
+}
+
+test('Dedupe permissions migration correctly dedupes permissions', async () => {
+    jest.setTimeout(15000);
+    const config = createTestConfig({
+        db: {
+            ...getDbConfig(),
+            pool: { min: 1, max: 4 },
+            schema: 'dedupe_permissions_test',
+            ssl: false,
+        },
+    });
+
+    await initSchema(config.db);
+
+    const e2e = {
+        ...config.db,
+        connectionTimeoutMillis: 2000,
+    };
+
+    // disable Intellij/WebStorm from setting verbose CLI argument to db-migrator
+    process.argv = process.argv.filter((it) => !it.includes('--verbose'));
+    const dbm = getInstance(true, {
+        cwd: `${__dirname}/../../`, // relative to src/test/e2e
+        config: { e2e },
+        env: 'e2e',
+    });
+
+    // Run all migrations up to, and including, this one, the last one before the dedupe migration
+    await dbm.up('20231121153304-add-permission-create-tag-type.js');
+
+    // Set up the test data
+    const client = new Client(config.db);
+    await client.connect();
+    await client.query(`
+        DELETE FROM "dedupe_permissions_test"."role_permission";
+
+        INSERT INTO "dedupe_permissions_test"."roles" (id, name, description, type) VALUES (101, 'Role 1', 'A test role', 'custom');
+        INSERT INTO "dedupe_permissions_test"."roles" (id, name, description, type) VALUES (102, 'Role 2', 'A test role', 'custom');
+        INSERT INTO "dedupe_permissions_test"."roles" (id, name, description, type) VALUES (103, 'Role 3', 'A test role', 'custom');
+        INSERT INTO "dedupe_permissions_test"."roles" (id, name, description, type) VALUES (104, 'Role 4', 'A test role', 'custom');
+    `);
+    await client.query(`
+        DELETE FROM "dedupe_permissions_test"."permissions";
+
+        INSERT INTO "dedupe_permissions_test"."permissions" (id, permission, display_name, type) VALUES (101, 'TEST_PERMISSION_1', 'A test permission', 'root');
+        INSERT INTO "dedupe_permissions_test"."permissions" (id, permission, display_name, type) VALUES (102, 'TEST_PERMISSION_DUPLICATE_1', 'A test permission', 'root');
+        INSERT INTO "dedupe_permissions_test"."permissions" (id, permission, display_name, type) VALUES (103, 'TEST_PERMISSION_DUPLICATE_1', 'A test permission', 'root');
+        INSERT INTO "dedupe_permissions_test"."permissions" (id, permission, display_name, type) VALUES (104, 'TEST_PERMISSION_2', 'A test permission', 'root');
+        INSERT INTO "dedupe_permissions_test"."permissions" (id, permission, display_name, type) VALUES (105, 'TEST_PERMISSION_3', 'A test permission', 'root');
+        INSERT INTO "dedupe_permissions_test"."permissions" (id, permission, display_name, type) VALUES (106, 'TEST_PERMISSION_DUPLICATE_2', 'A test permission', 'root');
+        INSERT INTO "dedupe_permissions_test"."permissions" (id, permission, display_name, type) VALUES (107, 'TEST_PERMISSION_DUPLICATE_2', 'A test permission', 'root');
+        INSERT INTO "dedupe_permissions_test"."permissions" (id, permission, display_name, type) VALUES (108, 'TEST_PERMISSION_4', 'A test permission', 'root');
+        INSERT INTO "dedupe_permissions_test"."permissions" (id, permission, display_name, type) VALUES (109, 'TEST_PERMISSION_DUPLICATE_3', 'A test permission', 'root');
+        INSERT INTO "dedupe_permissions_test"."permissions" (id, permission, display_name, type) VALUES (110, 'TEST_PERMISSION_DUPLICATE_3', 'A test permission', 'root');
+    `);
+    await client.query(`
+        DELETE FROM "dedupe_permissions_test"."role_permission";
+
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (101, 101);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (101, 102);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (101, 103);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (101, 104);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (101, 105);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (101, 106);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (101, 107);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (101, 108);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (101, 109);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (101, 110);
+
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (102, 102);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (102, 103);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (102, 106);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (102, 107);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (102, 109);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (102, 110);
+
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (103, 101);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (103, 104);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (103, 105);
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, permission_id) VALUES (103, 108);
+
+        -- Duplicate permission assignments, where role_id, permission_id, and environment are the same, as they should be deduped to the min created_at
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, created_at, permission_id, environment) VALUES (104, '2021-01-01T00:00:00Z', 102, 'dev');
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, created_at, permission_id, environment) VALUES (104, '2021-01-02T00:00:00Z', 102, 'dev');
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, created_at, permission_id, environment) VALUES (104, '2021-01-02T00:00:00Z', 102, 'prod');
+        INSERT INTO "dedupe_permissions_test"."role_permission" (role_id, created_at, permission_id, environment) VALUES (104, '2021-01-01T00:00:00Z', 102, 'prod');
+    `);
+
+    // Run the dedupe migration
+    await dbm.up('20231122121456-dedupe-any-duplicate-permissions.js');
+
+    // Check the results
+    const { rows: resultsPermissions } = await client.query(`
+        SELECT permission FROM "dedupe_permissions_test"."permissions" ORDER BY id;
+    `);
+
+    const { rows: resultsRolePermission } = await client.query(`
+        SELECT role_id, permission_id FROM "dedupe_permissions_test"."role_permission" WHERE role_id IN (101, 102, 103) ORDER BY role_id, permission_id;
+    `);
+
+    expect(resultsPermissions.length).toEqual(7);
+    expect(resultsPermissions).toEqual([
+        { permission: 'TEST_PERMISSION_1' },
+        { permission: 'TEST_PERMISSION_DUPLICATE_1' },
+        { permission: 'TEST_PERMISSION_2' },
+        { permission: 'TEST_PERMISSION_3' },
+        { permission: 'TEST_PERMISSION_DUPLICATE_2' },
+        { permission: 'TEST_PERMISSION_4' },
+        { permission: 'TEST_PERMISSION_DUPLICATE_3' },
+    ]);
+
+    expect(resultsRolePermission.length).toEqual(14);
+    expect(resultsRolePermission).toEqual([
+        // Role 101, we set all permissions and expect to see all permissions, where the duplicate permissions have been deduped to the min ID of each duplicate permission
+        { role_id: 101, permission_id: 101 },
+        { role_id: 101, permission_id: 102 },
+        { role_id: 101, permission_id: 104 },
+        { role_id: 101, permission_id: 105 },
+        { role_id: 101, permission_id: 106 },
+        { role_id: 101, permission_id: 108 },
+        { role_id: 101, permission_id: 109 },
+        // Role 102, we set all duplicate permissions and expect to see the deduped permissions to the min ID of each duplicate permission
+        { role_id: 102, permission_id: 102 },
+        { role_id: 102, permission_id: 106 },
+        { role_id: 102, permission_id: 109 },
+        // Role 103, we set all unique permissions and expect to see no changes
+        { role_id: 103, permission_id: 101 },
+        { role_id: 103, permission_id: 104 },
+        { role_id: 103, permission_id: 105 },
+        { role_id: 103, permission_id: 108 },
+    ]);
+
+    // Test duplicate permission assignments, where role_id, permission_id, and environment are the same, as they should be deduped to the min created_at
+    const { rows: resultsDedupedRolePermissionAssignments } =
+        await client.query(`
+        SELECT role_id, created_at, permission_id, environment FROM "dedupe_permissions_test"."role_permission" WHERE role_id = 104 ORDER BY role_id, permission_id;
+    `);
+
+    expect(resultsDedupedRolePermissionAssignments.length).toEqual(2);
+    expect(resultsDedupedRolePermissionAssignments).toEqual([
+        {
+            role_id: 104,
+            created_at: new Date('2021-01-01T00:00:00.000Z'),
+            permission_id: 102,
+            environment: 'dev',
+        },
+        {
+            role_id: 104,
+            created_at: new Date('2021-01-01T00:00:00.000Z'),
+            permission_id: 102,
+            environment: 'prod',
+        },
+    ]);
+
+    await client.end();
+    await dbm.reset();
+});