fix: add constraint for changing project. (#1049)

- In order for a feature toggle to be allowed to change project, the
  target project must have the same enabled environments.
- If the feature toggle has an environment which is not in use that does
  not exist in target project, this is ok.

Co-authored-by: Fredrik Strand Oseberg <fredrik.no@gmail.com>

Co-authored-by: Fredrik Strand Oseberg <fredrik.no@gmail.com>

src/lib/error/incompatible-project-error.ts

@@ -0,0 +1,23 @@
+export default class IncompatibleProjectError extends Error {
+    constructor(targetProject: string) {
+        super();
+        Error.captureStackTrace(this, this.constructor);
+
+        this.name = this.constructor.name;
+        this.message = `${targetProject} is not a compatible target`;
+    }
+
+    toJSON(): any {
+        const obj = {
+            isJoi: true,
+            name: this.constructor.name,
+            details: [
+                {
+                    validationErrors: [],
+                    message: this.message,
+                },
+            ],
+        };
+        return obj;
+    }
+}

src/lib/routes/util.ts

@@ -52,6 +52,8 @@ export const handleErrors: (
             return res.status(400).json(error).end();
         case 'PasswordUndefinedError':
             return res.status(400).json(error).end();
+        case 'IncompatibleProjectError':
+            return res.status(403).json(error).end();
         default:
             logger.error('Server failed executing request', error);
             return res.status(500).end();

src/lib/services/project-service.ts

@@ -18,6 +18,7 @@ import {
     IProjectOverview,
     IProjectWithCount,
     IUserWithRole,
+    FeatureToggle,
     RoleName,
 } from '../types/model';
 import { IEnvironmentStore } from '../types/stores/environment-store';
@@ -30,6 +31,7 @@ import { IEventStore } from '../types/stores/event-store';
 import FeatureToggleServiceV2 from './feature-toggle-service-v2';
 import { CREATE_FEATURE, UPDATE_FEATURE } from '../types/permissions';
 import NoAccessError from '../error/no-access-error';
+import IncompatibleProjectError from '../error/incompatible-project-error';
 
 const getCreatedBy = (user: User) => user.email || user.username;
 
@@ -173,6 +175,21 @@ export default class ProjectService {
         });
     }
 
+    async checkProjectsCompatibility(
+        feature: FeatureToggle,
+        newProjectId: string,
+    ): Promise<boolean> {
+        const featureEnvs = await this.featureEnvironmentStore.getAll({
+            feature_name: feature.name,
+        });
+        const newEnvs = await this.store.getEnvironmentsForProject(
+            newProjectId,
+        );
+        return featureEnvs.every(
+            (e) => !e.enabled || newEnvs.includes(e.environment),
+        );
+    }
+
     async changeProject(
         newProjectId: string,
         featureName: string,
@@ -184,7 +201,6 @@ export default class ProjectService {
         if (feature.project !== currentProjectId) {
             throw new NoAccessError(UPDATE_FEATURE);
         }
-
         const project = await this.getProject(newProjectId);
 
         if (!project) {
@@ -201,6 +217,11 @@ export default class ProjectService {
             throw new NoAccessError(CREATE_FEATURE);
         }
 
+        const isCompatibleWithTargetProject =
+            await this.checkProjectsCompatibility(feature, newProjectId);
+        if (!isCompatibleWithTargetProject) {
+            throw new IncompatibleProjectError(newProjectId);
+        }
         const updatedFeature = await this.featureToggleService.updateField(
             featureName,
             'project',

src/test/e2e/api/admin/project/features.e2e.test.ts

@@ -10,6 +10,9 @@ import {
     FEATURE_STALE_ON,
     FEATURE_STRATEGY_REMOVE,
 } from '../../../../../lib/types/events';
+import ApiUser from '../../../../../lib/types/api-user';
+import { ApiTokenType } from '../../../../../lib/types/models/api-token';
+import IncompatibleProjectError from '../../../../../lib/error/incompatible-project-error';
 
 let app: IUnleashTest;
 let db: ITestDb;
@@ -1484,3 +1487,171 @@ test('should clone feature toggle without replacing groupId', async () => {
             expect(env.strategies[0].parameters.groupId).toBe(featureName);
         });
 });
+
+test('Should not allow changing project to target project without the same enabled environments', async () => {
+    const envNameNotInBoth = 'not-in-both';
+    const featureName = 'feature.dont.allow.change.project';
+    const project = 'default';
+    const targetProject = 'target-for-disallowed-change';
+    await db.stores.projectStore.create({
+        name: 'Project to be moved to',
+        id: targetProject,
+        description: '',
+    });
+
+    await db.stores.environmentStore.create({
+        name: envNameNotInBoth,
+        type: 'test',
+        enabled: true,
+        sortOrder: 500,
+    });
+
+    await db.stores.projectStore.addEnvironmentToProject(
+        'default',
+        envNameNotInBoth,
+    );
+    await db.stores.projectStore.addEnvironmentToProject(
+        targetProject,
+        'default',
+    );
+
+    await app.request
+        .post(`/api/admin/projects/${project}/features`)
+        .send({
+            name: featureName,
+        })
+        .expect(201);
+    await app.request
+        .post(
+            `/api/admin/projects/${project}/features/${featureName}/environments/default/strategies`,
+        )
+        .send({
+            name: 'flexibleRollout',
+            parameters: {
+                groupId: featureName,
+            },
+        })
+        .expect(200);
+    await app.request
+        .post(
+            `/api/admin/projects/${project}/features/${featureName}/environments/${envNameNotInBoth}/strategies`,
+        )
+        .send({
+            name: 'flexibleRollout',
+            parameters: {
+                groupId: featureName,
+            },
+        })
+        .expect(200);
+    await app.request
+        .post(
+            `/api/admin/projects/${project}/features/${featureName}/environments/default/on`,
+        )
+        .send({})
+        .expect(200);
+    await app.request
+        .post(
+            `/api/admin/projects/${project}/features/${featureName}/environments/${envNameNotInBoth}/on`,
+        )
+        .send({})
+        .expect(200);
+    const user = new ApiUser({
+        username: 'project-changer',
+        permissions: ['ADMIN'],
+        project: '*',
+        type: ApiTokenType.ADMIN,
+        environment: '*',
+    });
+    await expect(async () =>
+        app.services.projectService.changeProject(
+            targetProject,
+            featureName,
+            //@ts-ignore
+            user,
+            'default',
+        ),
+    ).rejects.toThrow(new IncompatibleProjectError(targetProject));
+});
+
+test('Should allow changing project to target project with the same enabled environments', async () => {
+    const inBoth = 'in-both';
+    const featureName = 'feature.change.project';
+    const project = 'default';
+    const targetProject = 'target-for-change';
+    await db.stores.projectStore.create({
+        name: 'Project to be moved to',
+        id: targetProject,
+        description: '',
+    });
+
+    await db.stores.environmentStore.create({
+        name: inBoth,
+        type: 'test',
+        enabled: true,
+        sortOrder: 500,
+    });
+
+    await db.stores.projectStore.addEnvironmentToProject('default', inBoth);
+    await db.stores.projectStore.addEnvironmentToProject(
+        targetProject,
+        'default',
+    );
+    await db.stores.projectStore.addEnvironmentToProject(targetProject, inBoth);
+
+    await app.request
+        .post(`/api/admin/projects/${project}/features`)
+        .send({
+            name: featureName,
+        })
+        .expect(201);
+    await app.request
+        .post(
+            `/api/admin/projects/${project}/features/${featureName}/environments/default/strategies`,
+        )
+        .send({
+            name: 'flexibleRollout',
+            parameters: {
+                groupId: featureName,
+            },
+        })
+        .expect(200);
+    await app.request
+        .post(
+            `/api/admin/projects/${project}/features/${featureName}/environments/${inBoth}/strategies`,
+        )
+        .send({
+            name: 'flexibleRollout',
+            parameters: {
+                groupId: featureName,
+            },
+        })
+        .expect(200);
+    await app.request
+        .post(
+            `/api/admin/projects/${project}/features/${featureName}/environments/default/on`,
+        )
+        .send({})
+        .expect(200);
+    await app.request
+        .post(
+            `/api/admin/projects/${project}/features/${featureName}/environments/${inBoth}/on`,
+        )
+        .send({})
+        .expect(200);
+    const user = new ApiUser({
+        username: 'project-changer',
+        permissions: ['ADMIN'],
+        project: '*',
+        type: ApiTokenType.ADMIN,
+        environment: '*',
+    });
+    await expect(async () =>
+        app.services.projectService.changeProject(
+            targetProject,
+            featureName,
+            //@ts-ignore
+            user,
+            'default',
+        ),
+    ).resolves;
+});