diff --git a/src/lib/error/incompatible-project-error.ts b/src/lib/error/incompatible-project-error.ts new file mode 100644 index 0000000000..da912ae663 --- /dev/null +++ b/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; + } +} diff --git a/src/lib/routes/util.ts b/src/lib/routes/util.ts index 9903b41aba..0d2ec2ce23 100644 --- a/src/lib/routes/util.ts +++ b/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(); diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts index 6999e2af49..fd9bcc8fb8 100644 --- a/src/lib/services/project-service.ts +++ b/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 { + 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', diff --git a/src/test/e2e/api/admin/project/features.e2e.test.ts b/src/test/e2e/api/admin/project/features.e2e.test.ts index 3302fcbf83..835742813a 100644 --- a/src/test/e2e/api/admin/project/features.e2e.test.ts +++ b/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; +});