From 3a8107ce6eb0e2e22a5fea21f05334fa490ae4a2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= <ivar@getunleash.ai>
Date: Wed, 4 Jan 2023 12:24:34 +0100
Subject: [PATCH] fix: state-service should always keep api keys (#2552)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We have experienced side-effects where the import was unexpected and
resulted in environments thought to be removed. This had the unexpected
side-effect of also deleting API keys for some environments not part of
the import file.

This commit removes the ability of the state-service to mutate api keys
directly. There is no compelling reasons why we should remove API keys
as part of an import query.

Co-authored-by: Gastón Fournier <gaston@getunleash.ai>
Co-authored-by: Thomas Heartman <thomas@getunleash.ai>
---
 src/lib/services/state-service.ts             | 15 +-------
 src/test/e2e/api/admin/state.e2e.test.ts      | 34 -------------------
 .../docs/reference/deploy/import-export.md    |  4 ++-
 3 files changed, 4 insertions(+), 49 deletions(-)

diff --git a/src/lib/services/state-service.ts b/src/lib/services/state-service.ts
index 73ef43b6e5..8dba2a3ed8 100644
--- a/src/lib/services/state-service.ts
+++ b/src/lib/services/state-service.ts
@@ -46,11 +46,10 @@ import { IFeatureStrategiesStore } from '../types/stores/feature-strategies-stor
 import { IEnvironmentStore } from '../types/stores/environment-store';
 import { IFeatureEnvironmentStore } from '../types/stores/feature-environment-store';
 import { IUnleashStores } from '../types/stores';
-import { ALL_ENVS, DEFAULT_ENV } from '../util/constants';
+import { DEFAULT_ENV } from '../util/constants';
 import { GLOBAL_ENV } from '../types/environment';
 import { ISegmentStore } from '../types/stores/segment-store';
 import { PartialSome } from '../types/partial';
-import { IApiTokenStore } from 'lib/types/stores/api-token-store';
 import { IFlagResolver } from 'lib/types';
 
 export interface IBackupOption {
@@ -94,8 +93,6 @@ export default class StateService {
 
     private segmentStore: ISegmentStore;
 
-    private apiTokenStore: IApiTokenStore;
-
     private flagResolver: IFlagResolver;
 
     constructor(
@@ -116,7 +113,6 @@ export default class StateService {
         this.featureTagStore = stores.featureTagStore;
         this.environmentStore = stores.environmentStore;
         this.segmentStore = stores.segmentStore;
-        this.apiTokenStore = stores.apiTokenStore;
         this.flagResolver = flagResolver;
         this.logger = getLogger('services/state-service.js');
     }
@@ -479,15 +475,6 @@ export default class StateService {
                 data: env,
             }));
             await this.eventStore.batchStore(importedEnvironmentEvents);
-
-            const apiTokens = await this.apiTokenStore.getAll();
-            const envNames = importedEnvs.map((env) => env.name);
-            apiTokens
-                .filter((apiToken) => !(apiToken.environment === ALL_ENVS))
-                .filter((apiToken) => !envNames.includes(apiToken.environment))
-                .forEach((apiToken) =>
-                    this.apiTokenStore.delete(apiToken.secret),
-                );
         }
         return importedEnvs;
     }
diff --git a/src/test/e2e/api/admin/state.e2e.test.ts b/src/test/e2e/api/admin/state.e2e.test.ts
index cfd142f4c5..5b28412e58 100644
--- a/src/test/e2e/api/admin/state.e2e.test.ts
+++ b/src/test/e2e/api/admin/state.e2e.test.ts
@@ -416,40 +416,6 @@ test(`should not delete api_tokens on import when drop-flag is set`, async () =>
     expect(apiTokens[0].username).toBe(apiTokenName);
 });
 
-test(`should clean apitokens for not existing environment after import with drop`, async () => {
-    const projectId = 'not-reimported-project';
-    const environment = 'not-reimported-environment';
-    const apiTokenName = 'dropped-token';
-
-    await db.stores.environmentStore.create({
-        name: environment,
-        type: 'test',
-    });
-    await db.stores.projectStore.create({
-        name: projectId,
-        id: projectId,
-        description: 'Project for export',
-    });
-    await app.services.environmentService.addEnvironmentToProject(
-        environment,
-        projectId,
-    );
-    await app.services.apiTokenService.createApiTokenWithProjects({
-        username: apiTokenName,
-        type: ApiTokenType.CLIENT,
-        environment: environment,
-        projects: [projectId],
-    });
-
-    await app.request
-        .post('/api/admin/state/import?drop=true')
-        .attach('file', 'src/test/examples/v3-minimal.json')
-        .expect(202);
-
-    const apiTokens = await app.services.apiTokenService.getAllTokens();
-    expect(apiTokens.length).toEqual(0);
-});
-
 test(`should not show environment on feature toggle, when environment is disabled`, async () => {
     await app.request
         .post('/api/admin/state/import?drop=true')
diff --git a/website/docs/reference/deploy/import-export.md b/website/docs/reference/deploy/import-export.md
index e7cf5e6e66..d1421e2371 100644
--- a/website/docs/reference/deploy/import-export.md
+++ b/website/docs/reference/deploy/import-export.md
@@ -64,7 +64,9 @@ For example if you want to download just feature-toggles as yaml:
 
 ### API Import {#api-import}
 
-:::caution Importing environments
+:::caution Importing environments in Unleash 4.19 and below
+
+This is only relevant if you use **Unleash 4.19 or earlier**:
 
 If you import an environment into an instance that already has that environment defined, Unleash will delete any API keys created specifically for that environment. This is to prevent unexpected access to the newly imported environments.