From 80a2e1b93f859636df1e7ab0ed59d4c8e4da8488 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Tue, 20 Jun 2023 14:57:59 +0200 Subject: [PATCH] fix: reject API admin tokens when importing features (#4016) This PR fixes an issue where trying to use an admin token to import features via the API resulted in a 500 (due to missing properties). The solution is to catch when the user is using and admin token in the controller, throw a 400, and tell them to use personal access tokens or service accounts. The PR includes a new test file for this specific use case. We don't really test these cases many other places, so it seemed the logical choice. --- .../export-import-api-usage.test.ts | 76 +++++++++++++++++++ .../export-import-controller.ts | 13 +++- 2 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 src/lib/features/export-import-toggles/export-import-api-usage.test.ts diff --git a/src/lib/features/export-import-toggles/export-import-api-usage.test.ts b/src/lib/features/export-import-toggles/export-import-api-usage.test.ts new file mode 100644 index 0000000000..5e8585d166 --- /dev/null +++ b/src/lib/features/export-import-toggles/export-import-api-usage.test.ts @@ -0,0 +1,76 @@ +import { setupAppWithCustomAuth } from '../../../test/e2e/helpers/test-helper'; +import dbInit, { ITestDb } from '../../../test/e2e/helpers/database-init'; +import getLogger from '../../../test/fixtures/no-logger'; +import { DEFAULT_PROJECT } from '../../types'; +import { DEFAULT_ENV } from '../../util'; +import { ImportTogglesSchema } from '../../openapi'; +import { ApiTokenType } from '../../types/models/api-token'; +import { ApiUser } from '../../server-impl'; + +let db: ITestDb; + +beforeAll(async () => { + db = await dbInit('export_import_api_admin', getLogger); +}); + +afterAll(async () => { + await db.destroy(); +}); + +const defaultImportPayload: ImportTogglesSchema = { + data: { + features: [ + { + project: 'old_project', + name: 'first_feature', + }, + ], + featureStrategies: [], + featureEnvironments: [], + featureTags: [], + tagTypes: [], + contextFields: [], + segments: [], + }, + project: DEFAULT_PROJECT, + environment: DEFAULT_ENV, +}; + +test('reject API imports with admin tokens', async () => { + const preHook = (app: any) => { + app.use('/api/admin/', async (req, res, next) => { + const user = new ApiUser({ + permissions: ['ADMIN'], + environment: '*', + type: ApiTokenType.ADMIN, + tokenName: 'tokenName', + secret: 'secret', + projects: ['*'], + }); + req.user = user; + next(); + }); + }; + + const { request, destroy } = await setupAppWithCustomAuth( + db.stores, + preHook, + ); + + const { body } = await request + .post('/api/admin/features-batch/import') + .send(defaultImportPayload) + .expect(400); + + expect(body).toMatchObject({ + message: + // it tells the user that they used an admin token + expect.stringContaining('admin') && + // it tells the user to use a personal access token + expect.stringContaining('personal access token') && + // it tells the user to use a service account + expect.stringContaining('service account'), + }); + + await destroy(); +}); diff --git a/src/lib/features/export-import-toggles/export-import-controller.ts b/src/lib/features/export-import-toggles/export-import-controller.ts index dab6d3a52d..0ea42b9ad8 100644 --- a/src/lib/features/export-import-toggles/export-import-controller.ts +++ b/src/lib/features/export-import-toggles/export-import-controller.ts @@ -21,7 +21,8 @@ import { } from '../../openapi'; import { IAuthRequest } from '../../routes/unleash-types'; import { extractUsername } from '../../util'; -import { InvalidOperationError } from '../../error'; +import { BadDataError, InvalidOperationError } from '../../error'; +import ApiUser from '../../types/api-user'; class ExportImportController extends Controller { private logger: Logger; @@ -156,8 +157,16 @@ class ExportImportController extends Controller { res: Response, ): Promise { this.verifyExportImportEnabled(); - const dto = req.body; const { user } = req; + + if (user instanceof ApiUser && user.type === 'admin') { + throw new BadDataError( + `You can't use an admin token to import features. Please use either a personal access token (https://docs.getunleash.io/reference/api-tokens-and-client-keys#personal-access-tokens) or a service account (https://docs.getunleash.io/reference/service-accounts).`, + ); + } + + const dto = req.body; + await this.startTransaction(async (tx) => this.transactionalExportImportService(tx).import(dto, user), );