Fix/validate context (#1282)

* fix: add context guards

* fix: change error message

* fix: remove console log

src/lib/routes/admin-api/feature.ts

@@ -196,7 +196,12 @@ class FeatureController extends Controller {
         const projectId = await this.service.getProjectId(featureName);
         const value = await featureSchema.validateAsync(updatedFeature);
 
-        await this.service.updateFeatureToggle(projectId, value, userName);
+        await this.service.updateFeatureToggle(
+            projectId,
+            value,
+            userName,
+            featureName,
+        );
 
         await this.service.removeAllStrategiesForEnv(featureName);
 

src/lib/routes/admin-api/project/features.ts

@@ -178,16 +178,22 @@ export default class ProjectFeaturesController extends Controller {
     }
 
     async updateFeature(
-        req: IAuthRequest<ProjectParam, any, FeatureToggleDTO, any>,
+        req: IAuthRequest<
+            { projectId: string; featureName: string },
+            any,
+            FeatureToggleDTO,
+            any
+        >,
         res: Response,
     ): Promise<void> {
-        const { projectId } = req.params;
+        const { projectId, featureName } = req.params;
         const data = req.body;
         const userName = extractUsername(req);
         const created = await this.featureService.updateFeatureToggle(
             projectId,
             data,
             userName,
+            featureName,
         );
         res.status(200).json(created);
     }

src/lib/services/feature-toggle-service.ts

@@ -118,7 +118,7 @@ class FeatureToggleService {
         const id = await this.featureToggleStore.getProjectId(featureName);
         if (id !== projectId) {
             throw new InvalidOperationError(
-                'You can not change the projectId for an activation strategy.',
+                'Project id does not match the project that the feature belongs to',
             );
         }
     }
@@ -162,6 +162,7 @@ class FeatureToggleService {
             project,
             newDocument,
             createdBy,
+            featureName,
         );
 
         if (featureToggle.stale !== newDocument.stale) {
@@ -559,8 +560,10 @@ class FeatureToggleService {
         projectId: string,
         updatedFeature: FeatureToggleDTO,
         userName: string,
+        featureName: string,
     ): Promise<FeatureToggle> {
-        const featureName = updatedFeature.name;
+        await this.validateFeatureContext({ featureName, projectId });
+
         this.logger.info(`${userName} updates feature toggle ${featureName}`);
 
         const featureData = await featureMetadataSchema.validateAsync(
@@ -569,10 +572,11 @@ class FeatureToggleService {
 
         const preData = await this.featureToggleStore.get(featureName);
 
-        const featureToggle = await this.featureToggleStore.update(
+        const featureToggle = await this.featureToggleStore.update(projectId, {
-            projectId,
+            ...featureData,
-            featureData,
+            name: featureName,
-        );
+        });
+
         const tags = await this.tagStore.getAllTagsForFeature(featureName);
 
         await this.eventStore.store(

src/test/e2e/api/admin/feature.e2e.test.ts

@@ -732,7 +732,6 @@ test('should hit validate and tags endpoint if legacy api is disabled', async ()
     await appWithDisabledLegacyFeatures.request
         .get(`/api/admin/features/${feature.name}/tags`)
         .expect((res) => {
-            console.log(res.body);
             expect(res.body.tags[0].value).toBe('TeamGreen');
         });
 

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

@@ -1914,3 +1914,72 @@ test(`a feature's variants should be sorted by name in increasing order`, async
             expect(res.body.variants[2].name).toBe('z');
         });
 });
+
+test('should validate context when calling update with PUT', async () => {
+    const name = 'new.toggle.validate.context';
+    await app.request
+        .post('/api/admin/projects/default/features')
+        .send({ name, description: 'some', type: 'release' })
+        .expect(201);
+
+    await app.request
+        .put(`/api/admin/projects/another-project/features/${name}`)
+        .send({ name, description: 'updated', type: 'kill-switch' })
+        .expect((res) => {
+            expect(res.body.name).toBe('InvalidOperationError');
+        })
+        .expect(403);
+});
+
+test('should validate context when calling update with PATCH', async () => {
+    const name = 'new.toggle.validate.context2';
+    await app.request
+        .post('/api/admin/projects/default/features')
+        .send({ name, description: 'some', type: 'release' })
+        .expect(201);
+
+    await app.request
+        .patch(`/api/admin/projects/another-project/features/${name}`)
+        .send([])
+        .expect((res) => {
+            expect(res.body.name).toBe('InvalidOperationError');
+        })
+        .expect(403);
+});
+
+test('should not update project with PUT', async () => {
+    const name = 'new.toggle.validate.update.project.put';
+    await app.request
+        .post('/api/admin/projects/default/features')
+        .send({ name, description: 'some', type: 'release' })
+        .expect(201);
+
+    await app.request
+        .put(`/api/admin/projects/default/features/${name}`)
+        .send({
+            name,
+            description: 'updated',
+            type: 'kill-switch',
+            project: 'new-project',
+        })
+        .expect((res) => {
+            expect(res.body.project).toBe('default');
+        })
+        .expect(200);
+});
+
+test('should not update project with PATCH', async () => {
+    const name = 'new.toggle.validate.update.project.patch';
+    await app.request
+        .post('/api/admin/projects/default/features')
+        .send({ name, description: 'some', type: 'release' })
+        .expect(201);
+
+    await app.request
+        .patch(`/api/admin/projects/default/features/${name}`)
+        .send([{ op: 'replace', path: '/project', value: 'new-project' }])
+        .expect((res) => {
+            expect(res.body.project).toBe('default');
+        })
+        .expect(200);
+});

src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts

@@ -111,3 +111,41 @@ test('Should be able to get strategy by id', async () => {
     const fetchedConfig = await service.getStrategy(createdConfig.id);
     expect(fetchedConfig).toEqual(createdConfig);
 });
+
+test('should ignore name in the body when updating feature toggle', async () => {
+    const featureName = 'body-name-update';
+    const projectId = 'default';
+
+    const userName = 'strategy';
+    const secondFeatureName = 'body-name-update2';
+
+    await service.createFeatureToggle(
+        projectId,
+        {
+            name: featureName,
+            description: 'First toggle',
+        },
+        userName,
+    );
+
+    await service.createFeatureToggle(
+        projectId,
+        {
+            name: secondFeatureName,
+            description: 'Second toggle',
+        },
+        userName,
+    );
+
+    const update = {
+        name: secondFeatureName,
+        description: "I'm changed",
+    };
+
+    await service.updateFeatureToggle(projectId, update, userName, featureName);
+    const featureOne = await service.getFeature(featureName);
+    const featureTwo = await service.getFeature(secondFeatureName);
+
+    expect(featureOne.description).toBe(`I'm changed`);
+    expect(featureTwo.description).toBe('Second toggle');
+});