From 2e5d81cb89e3fafdd75d065b0361a15f4685f0d0 Mon Sep 17 00:00:00 2001 From: David Leek Date: Tue, 9 Jul 2024 13:49:26 +0200 Subject: [PATCH] chore: delete project api tokens when last mapped project is removed (#7503) Deletes API tokens bound to specific projects when the last project they're mapped to is deleted. --------- Co-authored-by: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com> Co-authored-by: Thomas Heartman --- .../__snapshots__/create-config.test.ts.snap | 1 + .../features/project/createProjectService.ts | 12 ++ .../project/project-service.e2e.test.ts | 104 +++++++++++++++++- src/lib/features/project/project-service.ts | 27 ++++- src/lib/types/experimental.ts | 9 +- 5 files changed, 148 insertions(+), 5 deletions(-) diff --git a/src/lib/__snapshots__/create-config.test.ts.snap b/src/lib/__snapshots__/create-config.test.ts.snap index 481faa66c6f9..0064124bf73d 100644 --- a/src/lib/__snapshots__/create-config.test.ts.snap +++ b/src/lib/__snapshots__/create-config.test.ts.snap @@ -82,6 +82,7 @@ exports[`should create default config 1`] = ` "automatedActions": false, "caseInsensitiveInOperators": false, "celebrateUnleash": false, + "cleanApiTokenWhenOrphaned": false, "collectTrafficDataUsage": false, "commandBarUI": false, "demo": false, diff --git a/src/lib/features/project/createProjectService.ts b/src/lib/features/project/createProjectService.ts index 6744b34698f9..599df4a9db43 100644 --- a/src/lib/features/project/createProjectService.ts +++ b/src/lib/features/project/createProjectService.ts @@ -45,6 +45,8 @@ import { ProjectOwnersReadModel } from './project-owners-read-model'; import { FakeProjectOwnersReadModel } from './fake-project-owners-read-model'; import { FakeProjectFlagCreatorsReadModel } from './fake-project-flag-creators-read-model'; import { ProjectFlagCreatorsReadModel } from './project-flag-creators-read-model'; +import FakeApiTokenStore from '../../../test/fixtures/fake-api-token-store'; +import { ApiTokenStore } from '../../db/api-token-store'; export const createProjectService = ( db: Db, @@ -109,6 +111,13 @@ export const createProjectService = ( eventService, ); + const apiTokenStore = new ApiTokenStore( + db, + eventBus, + getLogger, + flagResolver, + ); + const privateProjectChecker = createPrivateProjectChecker(db, config); return new ProjectService( @@ -123,6 +132,7 @@ export const createProjectService = ( projectStatsStore, projectOwnersReadModel, projectFlagCreatorsReadModel, + apiTokenStore, }, config, accessService, @@ -153,6 +163,7 @@ export const createFakeProjectService = ( const { featureToggleService } = createFakeFeatureToggleService(config); const favoriteFeaturesStore = new FakeFavoriteFeaturesStore(); const favoriteProjectsStore = new FakeFavoriteProjectsStore(); + const apiTokenStore = new FakeApiTokenStore(); const eventService = new EventService( { eventStore, @@ -188,6 +199,7 @@ export const createFakeProjectService = ( featureTypeStore, accountStore, projectStatsStore, + apiTokenStore, }, config, accessService, diff --git a/src/lib/features/project/project-service.e2e.test.ts b/src/lib/features/project/project-service.e2e.test.ts index 3dcd277d92ee..f243d54adae4 100644 --- a/src/lib/features/project/project-service.e2e.test.ts +++ b/src/lib/features/project/project-service.e2e.test.ts @@ -9,7 +9,7 @@ import { RoleName } from '../../types/model'; import { randomId } from '../../util/random-id'; import EnvironmentService from '../project-environments/environment-service'; import IncompatibleProjectError from '../../error/incompatible-project-error'; -import { EventService } from '../../services'; +import { ApiTokenService, EventService } from '../../services'; import { FeatureEnvironmentEvent } from '../../types/events'; import { addDays, subDays } from 'date-fns'; import { @@ -28,7 +28,8 @@ import { } from '../../types'; import type { User } from '../../server-impl'; import { BadDataError, InvalidOperationError } from '../../error'; -import { extractAuditInfoFromUser } from '../../util'; +import { DEFAULT_ENV, extractAuditInfoFromUser } from '../../util'; +import { ApiTokenType } from '../../types/models/api-token'; let stores: IUnleashStores; let db: ITestDb; @@ -40,6 +41,7 @@ let environmentService: EnvironmentService; let featureToggleService: FeatureToggleService; let user: User; // many methods in this test use User instead of IUser let auditUser: IAuditUser; +let apiTokenService: ApiTokenService; let opsUser: IUser; let group: IGroup; @@ -89,6 +91,7 @@ beforeAll(async () => { environmentService = new EnvironmentService(stores, config, eventService); projectService = createProjectService(db.rawDatabase, config); + apiTokenService = new ApiTokenService(stores, config, eventService); }); beforeEach(async () => { await stores.accessStore.addUserToRole(opsUser.id, 1, ''); @@ -2488,6 +2491,103 @@ test('deleting a project with archived flags should result in any remaining arch expect(flags.find((t) => t.name === flagName)).toBeUndefined(); }); +test('should also delete api tokens that were only bound to deleted project', async () => { + const project = 'some'; + const tokenName = 'test'; + + await projectService.createProject( + { + id: project, + name: 'Test Project 1', + }, + user, + auditUser, + ); + + const token = await apiTokenService.createApiToken({ + type: ApiTokenType.CLIENT, + tokenName, + environment: DEFAULT_ENV, + project: project, + }); + + await projectService.deleteProject(project, user, auditUser); + const deletedToken = await apiTokenService.getToken(token.secret); + expect(deletedToken).toBeUndefined(); +}); + +test('should not delete project-bound api tokens still bound to project', async () => { + const project1 = 'token-deleted-project'; + const project2 = 'token-not-deleted-project'; + const tokenName = 'test'; + + await projectService.createProject( + { + id: project1, + name: 'Test Project 1', + }, + user, + auditUser, + ); + + await projectService.createProject( + { + id: project2, + name: 'Test Project 2', + }, + user, + auditUser, + ); + + const token = await apiTokenService.createApiToken({ + type: ApiTokenType.CLIENT, + tokenName, + environment: DEFAULT_ENV, + projects: [project1, project2], + }); + + await projectService.deleteProject(project1, user, auditUser); + const fetchedToken = await apiTokenService.getToken(token.secret); + expect(fetchedToken).not.toBeUndefined(); + expect(fetchedToken.project).toBe(project2); +}); + +test('should delete project-bound api tokens when all projects they belong to are deleted', async () => { + const project1 = 'token-deleted-project-1'; + const project2 = 'token-deleted-project-2'; + const tokenName = 'test'; + + await projectService.createProject( + { + id: project1, + name: 'Test Project 1', + }, + user, + auditUser, + ); + + await projectService.createProject( + { + id: project2, + name: 'Test Project 2', + }, + user, + auditUser, + ); + + const token = await apiTokenService.createApiToken({ + type: ApiTokenType.CLIENT, + tokenName, + environment: DEFAULT_ENV, + projects: [project1, project2], + }); + + await projectService.deleteProject(project1, user, auditUser); + await projectService.deleteProject(project2, user, auditUser); + const fetchedToken = await apiTokenService.getToken(token.secret); + expect(fetchedToken).toBeUndefined(); +}); + test('deleting a project with no archived flags should not result in an error', async () => { const project = { id: 'project-with-nothing', diff --git a/src/lib/features/project/project-service.ts b/src/lib/features/project/project-service.ts index c55e2c6d3c37..46ba74576bfc 100644 --- a/src/lib/features/project/project-service.ts +++ b/src/lib/features/project/project-service.ts @@ -53,6 +53,7 @@ import { type ProjectCreated, type IProjectOwnersReadModel, ADMIN, + type IApiTokenStore, } from '../../types'; import type { IProjectAccessModel, @@ -144,6 +145,8 @@ export default class ProjectService { private accountStore: IAccountStore; + private apiTokenStore: IApiTokenStore; + private favoritesService: FavoritesService; private eventService: EventService; @@ -168,6 +171,7 @@ export default class ProjectService { featureTypeStore, accountStore, projectStatsStore, + apiTokenStore, }: Pick< IUnleashStores, | 'projectStore' @@ -180,6 +184,7 @@ export default class ProjectService { | 'accountStore' | 'projectStatsStore' | 'featureTypeStore' + | 'apiTokenStore' >, config: IUnleashConfig, accessService: AccessService, @@ -198,6 +203,7 @@ export default class ProjectService { this.eventStore = eventStore; this.featureToggleStore = featureToggleStore; this.featureTypeStore = featureTypeStore; + this.apiTokenStore = apiTokenStore; this.featureToggleService = featureToggleService; this.favoritesService = favoriteService; this.privateProjectChecker = privateProjectChecker; @@ -558,7 +564,26 @@ export default class ProjectService { auditUser, ); - await this.projectStore.delete(id); + if (this.flagResolver.isEnabled('cleanApiTokenWhenOrphaned')) { + const allTokens = await this.apiTokenStore.getAll(); + const projectTokens = allTokens.filter( + (token) => + (token.projects && + token.projects.length === 1 && + token.projects[0] === id) || + token.project === id, + ); + + await this.projectStore.delete(id); + + await Promise.all( + projectTokens.map((token) => + this.apiTokenStore.delete(token.secret), + ), + ); + } else { + await this.projectStore.delete(id); + } await this.eventService.storeEvent( new ProjectDeletedEvent({ diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index 75e6f76b58ea..98b6f938668a 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -63,8 +63,9 @@ export type IFlagKey = | 'flagCreator' | 'anonymizeProjectOwners' | 'resourceLimits' - | 'allowOrphanedWildcardTokens' - | 'extendedMetrics'; + | 'extendedMetrics' + | 'cleanApiTokenWhenOrphaned' + | 'allowOrphanedWildcardTokens'; export type IFlags = Partial<{ [key in IFlagKey]: boolean | Variant }>; @@ -309,6 +310,10 @@ const flags: IFlags = { process.env.UNLEASH_EXPERIMENTAL_EXTENDED_METRICS, false, ), + cleanApiTokenWhenOrphaned: parseEnvVarBoolean( + process.env.UNLEASH_EXPERIMENTAL_CLEAN_API_TOKEN_WHEN_ORPHANED, + false, + ), }; export const defaultExperimentalOptions: IExperimentalOptions = {