feat: Protect archive feature (#5003)

Unleash · Oct 12, 2023 · cfcf9de · cfcf9de
1 parent 2754c26
commit cfcf9de
Show file tree

Hide file tree

Showing 10 changed files with 69 additions and 28 deletions.
diff --git a/src/lib/features/dependent-features/dependent-features-service.ts b/src/lib/features/dependent-features/dependent-features-service.ts
@@ -182,7 +182,7 @@ export class DependentFeaturesService {
         projectId: string,
         user: string,
     ): Promise<void> {
-        await this.dependentFeaturesStore.deleteAll(feature);
+        await this.dependentFeaturesStore.deleteAll([feature]);
         await this.eventService.storeEvent({
             type: 'feature-dependencies-removed',
             project: projectId,

diff --git a/src/lib/features/dependent-features/dependent-features-store-type.ts b/src/lib/features/dependent-features/dependent-features-store-type.ts
@@ -3,5 +3,5 @@ import { FeatureDependency, FeatureDependencyId } from './dependent-features';
 export interface IDependentFeaturesStore {
     upsert(featureDependency: FeatureDependency): Promise<void>;
     delete(dependency: FeatureDependencyId): Promise<void>;
-    deleteAll(child?: string): Promise<void>;
+    deleteAll(children?: string[]): Promise<void>;
 }
diff --git a/src/lib/features/dependent-features/dependent-features-store.ts b/src/lib/features/dependent-features/dependent-features-store.ts
@@ -41,10 +41,10 @@ export class DependentFeaturesStore implements IDependentFeaturesStore {
             .del();
     }
 
-    async deleteAll(feature: string | undefined): Promise<void> {
-        if (feature) {
+    async deleteAll(features: string[] | undefined): Promise<void> {
+        if (features) {
             await this.db('dependent_features')
-                .andWhere('child', feature)
+                .whereIn('child', features)
                 .del();
         } else {
             await this.db('dependent_features').del();

diff --git a/src/lib/features/feature-toggle/feature-toggle-controller.ts b/src/lib/features/feature-toggle/feature-toggle-controller.ts
@@ -777,10 +777,9 @@ export default class ProjectFeaturesController extends Controller {
         res: Response<void>,
     ): Promise<void> {
         const { featureName, projectId } = req.params;
-        const userName = extractUsername(req);
         await this.featureService.archiveToggle(
             featureName,
-            userName,
+            req.user,
             projectId,
         );
         res.status(202).send();

diff --git a/src/lib/features/feature-toggle/feature-toggle-service.ts b/src/lib/features/feature-toggle/feature-toggle-service.ts
@@ -71,6 +71,7 @@ import {
 import {
     DATE_OPERATORS,
     DEFAULT_ENV,
+    extractUsernameFromUser,
     NUM_OPERATORS,
     SEMVER_OPERATORS,
     STRING_OPERATORS,
@@ -1465,6 +1466,25 @@ class FeatureToggleService {
     }
 
     async archiveToggle(
+        featureName: string,
+        user: User,
+        projectId?: string,
+    ): Promise<void> {
+        if (projectId) {
+            await this.stopWhenChangeRequestsEnabled(
+                projectId,
+                undefined,
+                user,
+            );
+        }
+        await this.unprotectedArchiveToggle(
+            featureName,
+            extractUsernameFromUser(user),
+            projectId,
+        );
+    }
+
+    async unprotectedArchiveToggle(
         featureName: string,
         createdBy: string,
         projectId?: string,
@@ -1476,6 +1496,7 @@ class FeatureToggleService {
                 featureName,
                 projectId,
             });
+            await this.validateNoOrphanParents([featureName]);
         }
 
         await this.validateNoChildren(featureName);
@@ -1492,12 +1513,27 @@ class FeatureToggleService {
     }
 
     async archiveToggles(
+        featureNames: string[],
+        user: User,
+        projectId: string,
+    ): Promise<void> {
+        await this.stopWhenChangeRequestsEnabled(projectId, undefined, user);
+        await this.unprotectedArchiveToggles(
+            featureNames,
+            extractUsernameFromUser(user),
+            projectId,
+        );
+    }
+
+    async unprotectedArchiveToggles(
         featureNames: string[],
         createdBy: string,
         projectId: string,
     ): Promise<void> {
-        await this.validateFeaturesContext(featureNames, projectId);
-        await this.validateNoOrphanParents(featureNames);
+        await Promise.all([
+            this.validateFeaturesContext(featureNames, projectId),
+            this.validateNoOrphanParents(featureNames),
+        ]);
 
         const features = await this.featureToggleStore.getAllByNames(
             featureNames,
@@ -2176,15 +2212,19 @@ class FeatureToggleService {
 
     private async stopWhenChangeRequestsEnabled(
         project: string,
-        environment: string,
+        environment?: string,
         user?: User,
     ) {
-        const canBypass =
-            await this.changeRequestAccessReadModel.canBypassChangeRequest(
-                project,
-                environment,
-                user,
-            );
+        const canBypass = environment
+            ? await this.changeRequestAccessReadModel.canBypassChangeRequest(
+                  project,
+                  environment,
+                  user,
+              )
+            : await this.changeRequestAccessReadModel.canBypassChangeRequestForProject(
+                  project,
+                  user,
+              );
         if (!canBypass) {
             throw new PermissionError(SKIP_CHANGE_REQUEST);
         }

diff --git a/src/lib/features/feature-toggle/legacy/feature-toggle-legacy-controller.ts b/src/lib/features/feature-toggle/legacy/feature-toggle-legacy-controller.ts
@@ -489,9 +489,8 @@ class FeatureController extends Controller {
 
     async archiveToggle(req: IAuthRequest, res: Response): Promise<void> {
         const { featureName } = req.params;
-        const userName = extractUsername(req);
 
-        await this.service.archiveToggle(featureName, userName);
+        await this.service.archiveToggle(featureName, req.user);
         res.status(200).end();
     }
 }

diff --git a/src/lib/routes/admin-api/project/project-archive.ts b/src/lib/routes/admin-api/project/project-archive.ts
@@ -140,9 +140,8 @@ export default class ProjectArchiveController extends Controller {
     ): Promise<void> {
         const { features } = req.body;
         const { projectId } = req.params;
-        const userName = extractUsername(req);
 
-        await this.featureService.archiveToggles(features, userName, projectId);
+        await this.featureService.archiveToggles(features, req.user, projectId);
         res.status(202).end();
     }
 }

diff --git a/src/test/e2e/api/client/feature.e2e.test.ts b/src/test/e2e/api/client/feature.e2e.test.ts
@@ -5,9 +5,11 @@ import {
 import dbInit, { ITestDb } from '../../helpers/database-init';
 import getLogger from '../../../fixtures/no-logger';
 import { DEFAULT_ENV } from '../../../../lib/util/constants';
+import User from '../../../../lib/types/user';
 
 let app: IUnleashTest;
 let db: ITestDb;
+const testUser = { name: 'test' } as User;
 
 beforeAll(async () => {
     db = await dbInit('feature_api_client', getLogger, {
@@ -69,7 +71,7 @@ beforeAll(async () => {
 
     await app.services.featureToggleServiceV2.archiveToggle(
         'featureArchivedX',
-        'test',
+        testUser,
     );
 
     await app.services.featureToggleServiceV2.createFeatureToggle(
@@ -83,7 +85,7 @@ beforeAll(async () => {
 
     await app.services.featureToggleServiceV2.archiveToggle(
         'featureArchivedY',
-        'test',
+        testUser,
     );
     await app.services.featureToggleServiceV2.createFeatureToggle(
         'default',
@@ -95,7 +97,7 @@ beforeAll(async () => {
     );
     await app.services.featureToggleServiceV2.archiveToggle(
         'featureArchivedZ',
-        'test',
+        testUser,
     );
     await app.services.featureToggleServiceV2.createFeatureToggle(
         'default',

diff --git a/src/test/e2e/api/client/feature.optimal304.e2e.test.ts b/src/test/e2e/api/client/feature.optimal304.e2e.test.ts
@@ -4,10 +4,12 @@ import {
 } from '../../helpers/test-helper';
 import dbInit, { ITestDb } from '../../helpers/database-init';
 import getLogger from '../../../fixtures/no-logger';
+import User from '../../../../lib/types/user';
 // import { DEFAULT_ENV } from '../../../../lib/util/constants';
 
 let app: IUnleashTest;
 let db: ITestDb;
+const testUser = { name: 'test' } as User;
 
 beforeAll(async () => {
     db = await dbInit('feature_304_api_client', getLogger);
@@ -55,7 +57,7 @@ beforeAll(async () => {
 
     await app.services.featureToggleServiceV2.archiveToggle(
         'featureArchivedX',
-        'test',
+        testUser,
     );
 
     await app.services.featureToggleServiceV2.createFeatureToggle(
@@ -69,7 +71,7 @@ beforeAll(async () => {
 
     await app.services.featureToggleServiceV2.archiveToggle(
         'featureArchivedY',
-        'test',
+        testUser,
     );
     await app.services.featureToggleServiceV2.createFeatureToggle(
         'default',
@@ -81,7 +83,7 @@ beforeAll(async () => {
     );
     await app.services.featureToggleServiceV2.archiveToggle(
         'featureArchivedZ',
-        'test',
+        testUser,
     );
     await app.services.featureToggleServiceV2.createFeatureToggle(
         'default',

diff --git a/src/test/e2e/services/project-service.e2e.test.ts b/src/test/e2e/services/project-service.e2e.test.ts
@@ -1241,7 +1241,7 @@ test('should only count active feature toggles for project', async () => {
         enabled: false,
     });
 
-    await featureToggleService.archiveToggle('only-active-t2', 'me');
+    await featureToggleService.archiveToggle('only-active-t2', user);
 
     const projects = await projectService.getProjects();
     const theProject = projects.find((p) => p.id === project.id);
@@ -1265,7 +1265,7 @@ test('should list projects with all features archived', async () => {
         enabled: false,
     });
 
-    await featureToggleService.archiveToggle('archived-toggle', 'me');
+    await featureToggleService.archiveToggle('archived-toggle', user);
 
     const projects = await projectService.getProjects();
     const theProject = projects.find((p) => p.id === project.id);