feat: add migration (#8891)

Unleash · Jan 6, 2025 · 2363d99 · 2363d99
1 parent 8bf1b78
commit 2363d99
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 75 deletions.
diff --git a/src/lib/features/feature-search/feature.search.e2e.test.ts b/src/lib/features/feature-search/feature.search.e2e.test.ts
@@ -6,7 +6,12 @@ import {
 } from '../../../test/e2e/helpers/test-helper';
 import getLogger from '../../../test/fixtures/no-logger';
 import type { FeatureSearchQueryParameters } from '../../openapi/spec/feature-search-query-parameters';
-import { DEFAULT_PROJECT, type IUnleashStores } from '../../types';
+import {
+    CREATE_FEATURE_STRATEGY,
+    DEFAULT_PROJECT,
+    type IUnleashStores,
+    UPDATE_FEATURE_ENVIRONMENT,
+} from '../../types';
 import { DEFAULT_ENV } from '../../util';
 
 let app: IUnleashTest;
@@ -29,7 +34,7 @@ beforeAll(async () => {
     );
     stores = db.stores;
 
-    await app.request
+    const { body } = await app.request
         .post(`/auth/demo/login`)
         .send({
             email: '[email protected]',
@@ -43,12 +48,30 @@ beforeAll(async () => {
 
     await app.linkProjectToEnvironment('default', 'development');
 
+    await stores.accessStore.addPermissionsToRole(
+        body.rootRole,
+        [
+            { name: UPDATE_FEATURE_ENVIRONMENT },
+            { name: CREATE_FEATURE_STRATEGY },
+        ],
+        'development',
+    );
+
     await stores.environmentStore.create({
         name: 'production',
         type: 'production',
     });
 
     await app.linkProjectToEnvironment('default', 'production');
+
+    await stores.accessStore.addPermissionsToRole(
+        body.rootRole,
+        [
+            { name: UPDATE_FEATURE_ENVIRONMENT },
+            { name: CREATE_FEATURE_STRATEGY },
+        ],
+        'production',
+    );
 });
 
 afterAll(async () => {

diff --git a/src/lib/features/project/project-service.e2e.test.ts b/src/lib/features/project/project-service.e2e.test.ts
@@ -805,6 +805,11 @@ describe('Managing Project access', () => {
             mode: 'open' as const,
             defaultStickiness: 'clientId',
         };
+        await db.stores.environmentStore.create({
+            name: 'production',
+            type: 'production',
+            enabled: true,
+        });
 
         const auditUser = extractAuditInfoFromUser(user);
         await projectService.createProject(project, user, auditUser);

diff --git a/src/migrations/20241129102205-add-foreign-key-to-role-permissions.js b/src/migrations/20241129102205-add-foreign-key-to-role-permissions.js
@@ -0,0 +1,20 @@
+exports.up = function (db, cb) {
+	db.runSql(
+		`
+        UPDATE role_permission SET environment = null where environment = '';
+        DELETE FROM role_permission WHERE environment IS NOT NULL AND environment NOT IN (SELECT name FROM environments);
+        ALTER TABLE role_permission ADD CONSTRAINT fk_role_permission_environment FOREIGN KEY (environment) REFERENCES environments(name) ON DELETE CASCADE;
+        `,
+		cb,
+	);
+};
+
+exports.down = function (db, cb) {
+	db.runSql(
+		`
+        ALTER TABLE role_permission
+        DROP CONSTRAINT IF EXISTS fk_role_permission_environment;
+        `,
+		cb,
+	);
+};
diff --git a/src/test/e2e/helpers/database-init.ts b/src/test/e2e/helpers/database-init.ts
@@ -21,9 +21,19 @@ delete process.env.DATABASE_URL;
 // because of db-migrate bug (https://github.com/Unleash/unleash/issues/171)
 process.setMaxListeners(0);
 
+async function getDefaultEnvRolePermissions(knex) {
+    return knex.table('role_permission').whereIn('environment', ['default']);
+}
+
+async function restoreRolePermissions(knex, rolePermissions) {
+    await knex.table('role_permission').insert(rolePermissions);
+}
+
 async function resetDatabase(knex) {
     return Promise.all([
-        knex.table('environments').del(),
+        knex
+            .table('environments')
+            .del(), // deletes role permissions transitively
         knex.table('strategies').del(),
         knex.table('features').del(),
         knex.table('client_applications').del(),
@@ -110,15 +120,20 @@ export default async function init(
     const testDb = createDb(config);
     const stores = await createStores(config, testDb);
     stores.eventStore.setMaxListeners(0);
+    const defaultRolePermissions = await getDefaultEnvRolePermissions(testDb);
     await resetDatabase(testDb);
     await setupDatabase(stores);
+    await restoreRolePermissions(testDb, defaultRolePermissions);
 
     return {
         rawDatabase: testDb,
         stores,
         reset: async () => {
+            const defaultRolePermissions =
+                await getDefaultEnvRolePermissions(testDb);
             await resetDatabase(testDb);
             await setupDatabase(stores);
+            await restoreRolePermissions(testDb, defaultRolePermissions);
         },
         destroy: async () => {
             return new Promise<void>((resolve, reject) => {

diff --git a/src/test/e2e/services/access-service.e2e.test.ts b/src/test/e2e/services/access-service.e2e.test.ts
@@ -94,8 +94,6 @@ const createRole = async (rolePermissions: PermissionRef[]) => {
 
 const hasCommonProjectAccess = async (user, projectName, condition) => {
     const defaultEnv = 'default';
-    const developmentEnv = 'development';
-    const productionEnv = 'production';
 
     const {
         CREATE_FEATURE,
@@ -155,70 +153,6 @@ const hasCommonProjectAccess = async (user, projectName, condition) => {
             defaultEnv,
         ),
     ).toBe(condition);
-    expect(
-        await accessService.hasPermission(
-            user,
-            CREATE_FEATURE_STRATEGY,
-            projectName,
-            developmentEnv,
-        ),
-    ).toBe(condition);
-    expect(
-        await accessService.hasPermission(
-            user,
-            UPDATE_FEATURE_STRATEGY,
-            projectName,
-            developmentEnv,
-        ),
-    ).toBe(condition);
-    expect(
-        await accessService.hasPermission(
-            user,
-            DELETE_FEATURE_STRATEGY,
-            projectName,
-            developmentEnv,
-        ),
-    ).toBe(condition);
-    expect(
-        await accessService.hasPermission(
-            user,
-            UPDATE_FEATURE_ENVIRONMENT,
-            projectName,
-            developmentEnv,
-        ),
-    ).toBe(condition);
-    expect(
-        await accessService.hasPermission(
-            user,
-            CREATE_FEATURE_STRATEGY,
-            projectName,
-            productionEnv,
-        ),
-    ).toBe(condition);
-    expect(
-        await accessService.hasPermission(
-            user,
-            UPDATE_FEATURE_STRATEGY,
-            projectName,
-            productionEnv,
-        ),
-    ).toBe(condition);
-    expect(
-        await accessService.hasPermission(
-            user,
-            DELETE_FEATURE_STRATEGY,
-            projectName,
-            productionEnv,
-        ),
-    ).toBe(condition);
-    expect(
-        await accessService.hasPermission(
-            user,
-            UPDATE_FEATURE_ENVIRONMENT,
-            projectName,
-            productionEnv,
-        ),
-    ).toBe(condition);
 };
 
 const hasFullProjectAccess = async (user, projectName: string, condition) => {
@@ -378,7 +312,7 @@ test('should remove CREATE_FEATURE on default environment', async () => {
     await accessService.addPermissionToRole(
         editRole.id,
         permissions.CREATE_FEATURE,
-        '*',
+        'default',
     );
 
     // TODO: to validate the remove works, we should make sure that we had permission before removing it
@@ -637,15 +571,15 @@ test('should support permission with "ALL" environment requirement', async () =>
     await accessStore.addPermissionsToRole(
         customRole.id,
         [{ name: CREATE_FEATURE_STRATEGY }],
-        'production',
+        'default',
     );
     await accessStore.addUserToRole(user.id, customRole.id, ALL_PROJECTS);
 
     const hasAccess = await accessService.hasPermission(
         user,
         CREATE_FEATURE_STRATEGY,
         'default',
-        'production',
+        'default',
     );
 
     expect(hasAccess).toBe(true);
@@ -667,7 +601,7 @@ test('Should have access to create a strategy in an environment', async () => {
             user,
             CREATE_FEATURE_STRATEGY,
             'default',
-            'development',
+            'default',
         ),
     ).toBe(true);
 });
@@ -693,7 +627,7 @@ test('Should have access to edit a strategy in an environment', async () => {
             user,
             UPDATE_FEATURE_STRATEGY,
             'default',
-            'development',
+            'default',
         ),
     ).toBe(true);
 });
@@ -706,7 +640,7 @@ test('Should have access to delete a strategy in an environment', async () => {
             user,
             DELETE_FEATURE_STRATEGY,
             'default',
-            'development',
+            'default',
         ),
     ).toBe(true);
 });