From 856c7a358b0206a8d4df04c5f0ecf26b98496a31 Mon Sep 17 00:00:00 2001 From: Fredrik Strand Oseberg Date: Wed, 25 Aug 2021 13:38:00 +0200 Subject: [PATCH] Fix/switch project endpoint (#923) --- src/lib/services/feature-toggle-service-v2.ts | 147 +++++++++++------- src/lib/services/project-service.ts | 42 +++++ src/lib/types/events.ts | 1 + .../e2e/services/project-service.e2e.test.ts | 122 ++++++++++++++- 4 files changed, 253 insertions(+), 59 deletions(-) diff --git a/src/lib/services/feature-toggle-service-v2.ts b/src/lib/services/feature-toggle-service-v2.ts index 59dbb4880b..c271722678 100644 --- a/src/lib/services/feature-toggle-service-v2.ts +++ b/src/lib/services/feature-toggle-service-v2.ts @@ -17,7 +17,10 @@ import { } from '../types/events'; import { GLOBAL_ENV } from '../types/environment'; import NotFoundError from '../error/notfound-error'; -import { FeatureConfigurationClient, IFeatureStrategiesStore } from '../types/stores/feature-strategies-store'; +import { + FeatureConfigurationClient, + IFeatureStrategiesStore, +} from '../types/stores/feature-strategies-store'; import { IFeatureTypeStore } from '../types/stores/feature-type-store'; import { IEventStore } from '../types/stores/event-store'; import { IEnvironmentStore } from '../types/stores/environment-store'; @@ -65,15 +68,15 @@ class FeatureToggleServiceV2 { featureTypeStore, featureEnvironmentStore, }: Pick< - IUnleashStores, - | 'featureStrategiesStore' - | 'featureToggleStore' - | 'projectStore' - | 'eventStore' - | 'featureTagStore' - | 'environmentStore' - | 'featureTypeStore' - | 'featureEnvironmentStore' + IUnleashStores, + | 'featureStrategiesStore' + | 'featureToggleStore' + | 'projectStore' + | 'eventStore' + | 'featureTagStore' + | 'environmentStore' + | 'featureTypeStore' + | 'featureEnvironmentStore' >, { getLogger }: Pick, ) { @@ -95,16 +98,15 @@ class FeatureToggleServiceV2 { environment: string = GLOBAL_ENV, ): Promise { try { - const newFeatureStrategy = await this.featureStrategiesStore.createStrategyConfig( - { + const newFeatureStrategy = + await this.featureStrategiesStore.createStrategyConfig({ strategyName: strategyConfig.name, constraints: strategyConfig.constraints, parameters: strategyConfig.parameters, projectName, featureName, environment, - }, - ); + }); return { id: newFeatureStrategy.id, name: newFeatureStrategy.strategyName, @@ -150,11 +152,12 @@ class FeatureToggleServiceV2 { featureName, ); if (hasEnv) { - const featureStrategies = await this.featureStrategiesStore.getStrategiesForFeature( - projectName, - featureName, - environment, - ); + const featureStrategies = + await this.featureStrategiesStore.getStrategiesForFeature( + projectName, + featureName, + environment, + ); return featureStrategies.map((strat) => ({ id: strat.id, name: strat.strategyName, @@ -176,7 +179,10 @@ class FeatureToggleServiceV2 { featureName: string, archived: boolean = false, ): Promise { - return this.featureStrategiesStore.getFeatureToggleAdmin(featureName, archived); + return this.featureStrategiesStore.getFeatureToggleAdmin( + featureName, + archived, + ); } async getClientFeatures( @@ -203,7 +209,10 @@ class FeatureToggleServiceV2 { async getFeatureToggle( featureName: string, ): Promise { - return this.featureStrategiesStore.getFeatureToggleAdmin(featureName, false); + return this.featureStrategiesStore.getFeatureToggleAdmin( + featureName, + false, + ); } async createFeatureToggle( @@ -215,7 +224,9 @@ class FeatureToggleServiceV2 { await this.validateName(value.name); const exists = await this.projectStore.hasProject(projectId); if (exists) { - const featureData = await featureMetadataSchema.validateAsync(value); + const featureData = await featureMetadataSchema.validateAsync( + value, + ); const createdToggle = await this.featureToggleStore.createFeature( projectId, featureData, @@ -250,9 +261,10 @@ class FeatureToggleServiceV2 { projectId, updatedFeature, ); - const tags = (await this.featureTagStore.getAllTagsForFeature( - updatedFeature.name, - )) || []; + const tags = + (await this.featureTagStore.getAllTagsForFeature( + updatedFeature.name, + )) || []; await this.eventStore.store({ type: FEATURE_UPDATED, createdBy: userName, @@ -296,15 +308,17 @@ class FeatureToggleServiceV2 { environment: string, featureName: string, ): Promise { - const envMetadata = await this.featureEnvironmentStore.getEnvironmentMetaData( - environment, - featureName, - ); - const strategies = await this.featureStrategiesStore.getStrategiesForFeature( - project, - featureName, - environment, - ); + const envMetadata = + await this.featureEnvironmentStore.getEnvironmentMetaData( + environment, + featureName, + ); + const strategies = + await this.featureStrategiesStore.getStrategiesForFeature( + project, + featureName, + environment, + ); return { name: featureName, environment, @@ -321,7 +335,10 @@ class FeatureToggleServiceV2 { projectId, environment, ); - await this.projectStore.deleteEnvironmentForProject(projectId, environment); + await this.projectStore.deleteEnvironmentForProject( + projectId, + environment, + ); } /** Validations */ @@ -358,8 +375,9 @@ class FeatureToggleServiceV2 { ); feature.stale = isStale; await this.featureToggleStore.updateFeature(feature.project, feature); - const tags = (await this.featureTagStore.getAllTagsForFeature(featureName)) - || []; + const tags = + (await this.featureTagStore.getAllTagsForFeature(featureName)) || + []; await this.eventStore.store({ type: isStale ? FEATURE_STALE_ON : FEATURE_STALE_OFF, @@ -373,7 +391,8 @@ class FeatureToggleServiceV2 { async archiveToggle(name: string, userName: string): Promise { await this.featureToggleStore.hasFeature(name); await this.featureToggleStore.archiveFeature(name); - const tags = (await this.featureTagStore.getAllTagsForFeature(name)) || []; + const tags = + (await this.featureTagStore.getAllTagsForFeature(name)) || []; await this.eventStore.store({ type: FEATURE_ARCHIVED, createdBy: userName, @@ -388,22 +407,25 @@ class FeatureToggleServiceV2 { enabled: boolean, userName: string, ): Promise { - const hasEnvironment = await this.featureEnvironmentStore.featureHasEnvironment( - environment, - featureName, - ); - if (hasEnvironment) { - const newEnabled = await this.featureEnvironmentStore.toggleEnvironmentEnabledStatus( + const hasEnvironment = + await this.featureEnvironmentStore.featureHasEnvironment( environment, featureName, - enabled, ); + if (hasEnvironment) { + const newEnabled = + await this.featureEnvironmentStore.toggleEnvironmentEnabledStatus( + environment, + featureName, + enabled, + ); const feature = await this.featureToggleStore.getFeatureMetadata( featureName, ); - const tags = (await this.featureTagStore.getAllTagsForFeature( - featureName, - )) || []; + const tags = + (await this.featureTagStore.getAllTagsForFeature( + featureName, + )) || []; await this.eventStore.store({ type: FEATURE_UPDATED, createdBy: userName, @@ -424,10 +446,11 @@ class FeatureToggleServiceV2 { userName: string, ): Promise { await this.featureToggleStore.hasFeature(featureName); - const isEnabled = await this.featureEnvironmentStore.isEnvironmentEnabled( - featureName, - environment, - ); + const isEnabled = + await this.featureEnvironmentStore.isEnvironmentEnabled( + featureName, + environment, + ); return this.updateEnabled( featureName, environment, @@ -443,17 +466,19 @@ class FeatureToggleServiceV2 { // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types value: any, userName: string, + event?: string, ): Promise { const feature = await this.featureToggleStore.getFeatureMetadata( featureName, ); feature[field] = value; await this.featureToggleStore.updateFeature(feature.project, feature); - const tags = (await this.featureTagStore.getAllTagsForFeature(featureName)) - || []; + const tags = + (await this.featureTagStore.getAllTagsForFeature(featureName)) || + []; await this.eventStore.store({ - type: FEATURE_UPDATED, + type: event || FEATURE_UPDATED, createdBy: userName, data: feature, tags, @@ -478,7 +503,9 @@ class FeatureToggleServiceV2 { async reviveToggle(featureName: string, userName: string): Promise { const data = await this.featureToggleStore.reviveFeature(featureName); - const tags = await this.featureTagStore.getAllTagsForFeature(featureName); + const tags = await this.featureTagStore.getAllTagsForFeature( + featureName, + ); await this.eventStore.store({ type: FEATURE_REVIVED, createdBy: userName, @@ -487,12 +514,16 @@ class FeatureToggleServiceV2 { }); } - async getMetadataForAllFeatures(archived: boolean): Promise { + async getMetadataForAllFeatures( + archived: boolean, + ): Promise { return this.featureToggleStore.getFeatures(archived); } async getProjectId(name: string): Promise { - const { project } = await this.featureToggleStore.getFeatureMetadata(name); + const { project } = await this.featureToggleStore.getFeatureMetadata( + name, + ); return project; } } diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts index 469947a414..99bdbd2fe0 100644 --- a/src/lib/services/project-service.ts +++ b/src/lib/services/project-service.ts @@ -6,6 +6,7 @@ import { nameType } from '../routes/util'; import schema from './project-schema'; import NotFoundError from '../error/notfound-error'; import { + FEATURE_PROJECT_CHANGE, PROJECT_CREATED, PROJECT_DELETED, PROJECT_UPDATED, @@ -27,6 +28,8 @@ import { IProjectStore } from '../types/stores/project-store'; import { IRole } from '../types/stores/access-store'; 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'; const getCreatedBy = (user: User) => user.email || user.username; @@ -140,6 +143,45 @@ export default class ProjectService { }); } + async changeProject( + newProjectId: string, + featureName: string, + user: User, + currentProjectId: string, + ): Promise { + const feature = await this.featureToggleStore.get(featureName); + + if (feature.project !== currentProjectId) { + throw new NoAccessError(UPDATE_FEATURE); + } + + const project = await this.getProject(newProjectId); + + if (!project) { + throw new NotFoundError(`Project ${newProjectId} not found`); + } + + const authorized = await this.accessService.hasPermission( + user, + CREATE_FEATURE, + newProjectId, + ); + + if (!authorized) { + throw new NoAccessError(CREATE_FEATURE); + } + + const updatedFeature = await this.featureToggleService.updateField( + featureName, + 'project', + newProjectId, + user.username, + FEATURE_PROJECT_CHANGE, + ); + + return updatedFeature; + } + async deleteProject(id: string, user: User): Promise { if (id === DEFAULT_PROJECT) { throw new InvalidOperationError( diff --git a/src/lib/types/events.ts b/src/lib/types/events.ts index 0529e29cbe..293be6b745 100644 --- a/src/lib/types/events.ts +++ b/src/lib/types/events.ts @@ -2,6 +2,7 @@ export const APPLICATION_CREATED = 'application-created'; export const FEATURE_CREATED = 'feature-created'; export const FEATURE_DELETED = 'feature-deleted'; export const FEATURE_UPDATED = 'feature-updated'; +export const FEATURE_PROJECT_CHANGE = 'feature-project-change'; export const FEATURE_ARCHIVED = 'feature-archived'; export const FEATURE_REVIVED = 'feature-revived'; export const FEATURE_IMPORT = 'feature-import'; diff --git a/src/test/e2e/services/project-service.e2e.test.ts b/src/test/e2e/services/project-service.e2e.test.ts index d26103c879..0ae0496d98 100644 --- a/src/test/e2e/services/project-service.e2e.test.ts +++ b/src/test/e2e/services/project-service.e2e.test.ts @@ -3,7 +3,11 @@ import getLogger from '../../fixtures/no-logger'; import FeatureToggleServiceV2 from '../../../lib/services/feature-toggle-service-v2'; import ProjectService from '../../../lib/services/project-service'; import { AccessService } from '../../../lib/services/access-service'; -import { UPDATE_PROJECT } from '../../../lib/types/permissions'; +import { + CREATE_FEATURE, + UPDATE_FEATURE, + UPDATE_PROJECT, +} from '../../../lib/types/permissions'; import NotFoundError from '../../../lib/error/notfound-error'; import { createTestConfig } from '../../config/test-config'; import { RoleName } from '../../../lib/types/model'; @@ -390,3 +394,119 @@ test('should not remove user from the project', async () => { new Error('A project must have at least one owner'), ); }); + +test('should not change project if feature toggle project does not match current project id', async () => { + const project = { + id: 'test-change-project', + name: 'New project', + description: 'Blah', + }; + + const toggle = { name: 'test-toggle' }; + + await projectService.createProject(project, user); + await featureToggleService.createFeatureToggle(project.id, toggle, user); + + try { + await projectService.changeProject( + 'newProject', + toggle.name, + user, + 'wrong-project-id', + ); + } catch (err) { + expect(err.message).toBe( + `You need permission=${UPDATE_FEATURE} to perform this action`, + ); + } +}); + +test('should return 404 if no project is found with the project id', async () => { + const project = { + id: 'test-change-project-2', + name: 'New project', + description: 'Blah', + }; + + const toggle = { name: 'test-toggle-2' }; + + await projectService.createProject(project, user); + await featureToggleService.createFeatureToggle(project.id, toggle, user); + + try { + await projectService.changeProject( + 'newProject', + toggle.name, + user, + project.id, + ); + } catch (err) { + expect(err.message).toBe(`No project found`); + } +}); + +test('should fail if user is not authorized', async () => { + const project = { + id: 'test-change-project-3', + name: 'New project', + description: 'Blah', + }; + + const projectDestination = { + id: 'test-change-project-dest', + name: 'New project 2', + description: 'Blah', + }; + + const toggle = { name: 'test-toggle-3' }; + const projectAdmin1 = await stores.userStore.insert({ + name: 'test-change-project-creator', + email: 'admin-change-project@getunleash.io', + }); + + await projectService.createProject(project, user); + await projectService.createProject(projectDestination, projectAdmin1); + await featureToggleService.createFeatureToggle(project.id, toggle, user); + + try { + await projectService.changeProject( + projectDestination.id, + toggle.name, + user, + project.id, + ); + } catch (err) { + expect(err.message).toBe( + `You need permission=${CREATE_FEATURE} to perform this action`, + ); + } +}); + +test('should change project when checks pass', async () => { + const project = { + id: 'test-change-project-4', + name: 'New project', + description: 'Blah', + }; + + const projectDestination = { + id: 'test-change-project-dest-2', + name: 'New project 2', + description: 'Blah', + }; + + const toggle = { name: 'test-toggle-4' }; + + await projectService.createProject(project, user); + await projectService.createProject(projectDestination, user); + await featureToggleService.createFeatureToggle(project.id, toggle, user); + + const updatedFeature = await projectService.changeProject( + projectDestination.id, + toggle.name, + user, + project.id, + ); + + expect(updatedFeature.project).toBe(projectDestination.id); +});