From 007a3a1eb1b2fd6840d625df40dc44a811e53c2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Tue, 21 Sep 2021 13:27:18 +0200 Subject: [PATCH] fix: client tokens should be scoped to default env --- src/lib/schema/api-token-schema.ts | 7 ++++- src/lib/services/api-token-service.ts | 10 +++++-- ...0210921105032-client-api-tokens-default.js | 20 +++++++++++++ src/test/e2e/api/admin/api-token.e2e.test.ts | 28 ++++++++++--------- 4 files changed, 49 insertions(+), 16 deletions(-) create mode 100644 src/migrations/20210921105032-client-api-tokens-default.js diff --git a/src/lib/schema/api-token-schema.ts b/src/lib/schema/api-token-schema.ts index ed653b9286..be71638eb1 100644 --- a/src/lib/schema/api-token-schema.ts +++ b/src/lib/schema/api-token-schema.ts @@ -1,5 +1,6 @@ import joi from 'joi'; import { ALL, ApiTokenType } from '../types/models/api-token'; +import { DEFAULT_ENV } from '../util/constants'; export const createApiToken = joi .object() @@ -12,6 +13,10 @@ export const createApiToken = joi .valid(ApiTokenType.ADMIN, ApiTokenType.CLIENT), expiresAt: joi.date().optional(), project: joi.string().optional().default(ALL), - environment: joi.string().optional().default(ALL), + environment: joi.when('type', { + is: joi.string().valid(ApiTokenType.CLIENT), + then: joi.string().optional().default(DEFAULT_ENV), + otherwise: joi.string().optional().default(ALL), + }), }) .options({ stripUnknown: true, allowUnknown: false, abortEarly: false }); diff --git a/src/lib/services/api-token-service.ts b/src/lib/services/api-token-service.ts index 3f9ca0901b..6d69657b0f 100644 --- a/src/lib/services/api-token-service.ts +++ b/src/lib/services/api-token-service.ts @@ -83,7 +83,7 @@ export class ApiTokenService { return this.store.delete(secret); } - private validateAdminToken({ type, project, environment }) { + private validateNewApiToken({ type, project, environment }) { if (type === ApiTokenType.ADMIN && project !== ALL) { throw new BadDataError( 'Admin token cannot be scoped to single project', @@ -95,12 +95,18 @@ export class ApiTokenService { 'Admin token cannot be scoped to single environment', ); } + + if (type === ApiTokenType.CLIENT && environment === ALL) { + throw new BadDataError( + 'Client token cannot be scoped to all environments', + ); + } } public async createApiToken( newToken: Omit, ): Promise { - this.validateAdminToken(newToken); + this.validateNewApiToken(newToken); const secret = this.generateSecretKey(newToken); const createNewToken = { ...newToken, secret }; diff --git a/src/migrations/20210921105032-client-api-tokens-default.js b/src/migrations/20210921105032-client-api-tokens-default.js new file mode 100644 index 0000000000..f97490ab8e --- /dev/null +++ b/src/migrations/20210921105032-client-api-tokens-default.js @@ -0,0 +1,20 @@ +'use strict'; + +exports.up = function (db, cb) { + db.runSql( + ` + UPDATE api_tokens SET environment = 'default' WHERE environment = ':global:'; + UPDATE api_tokens SET environment = 'default' WHERE type='client' AND environment is null; + `, + cb, + ); +}; + +exports.down = function (db, cb) { + db.runSql( + ` + UPDATE api_tokens SET environment = null WHERE type='client' AND environment = 'default'; + `, + cb, + ); +}; diff --git a/src/test/e2e/api/admin/api-token.e2e.test.ts b/src/test/e2e/api/admin/api-token.e2e.test.ts index e3339cc93d..8bf0e28a91 100644 --- a/src/test/e2e/api/admin/api-token.e2e.test.ts +++ b/src/test/e2e/api/admin/api-token.e2e.test.ts @@ -24,7 +24,6 @@ afterEach(async () => { }); test('returns empty list of tokens', async () => { - expect.assertions(1); return app.request .get('/api/admin/api-tokens') .expect('Content-Type', /json/) @@ -35,7 +34,6 @@ test('returns empty list of tokens', async () => { }); test('creates new client token', async () => { - expect.assertions(4); return app.request .post('/api/admin/api-tokens') .send({ @@ -53,7 +51,6 @@ test('creates new client token', async () => { }); test('creates new admin token', async () => { - expect.assertions(5); return app.request .post('/api/admin/api-tokens') .send({ @@ -65,6 +62,7 @@ test('creates new admin token', async () => { .expect((res) => { expect(res.body.username).toBe('default-admin'); expect(res.body.type).toBe('admin'); + expect(res.body.environment).toBe(ALL); expect(res.body.createdAt).toBeTruthy(); expect(res.body.expiresAt).toBeFalsy(); expect(res.body.secret.length > 16).toBe(true); @@ -72,7 +70,6 @@ test('creates new admin token', async () => { }); test('creates new ADMIN token should fix casing', async () => { - expect.assertions(5); return app.request .post('/api/admin/api-tokens') .send({ @@ -91,7 +88,6 @@ test('creates new ADMIN token should fix casing', async () => { }); test('creates new admin token with expiry', async () => { - expect.assertions(1); const expiresAt = new Date(); const expiresAtAsISOStr = JSON.parse(JSON.stringify(expiresAt)); return app.request @@ -109,8 +105,6 @@ test('creates new admin token with expiry', async () => { }); test('update admin token with expiry', async () => { - expect.assertions(2); - const tokenSecret = 'random-secret-update'; await db.stores.apiTokenStore.insert({ @@ -138,8 +132,6 @@ test('update admin token with expiry', async () => { }); test('creates a lot of client tokens', async () => { - expect.assertions(4); - const requests = []; for (let i = 0; i < 10; i++) { @@ -167,8 +159,6 @@ test('creates a lot of client tokens', async () => { }); test('removes api token', async () => { - expect.assertions(1); - const tokenSecret = 'random-secret'; await db.stores.apiTokenStore.insert({ @@ -203,7 +193,7 @@ test('creates new client token: project & environment defaults to "*"', async () .expect((res) => { expect(res.body.type).toBe('client'); expect(res.body.secret.length > 16).toBe(true); - expect(res.body.environment).toBe(ALL); + expect(res.body.environment).toBe(DEFAULT_ENV); expect(res.body.project).toBe(ALL); }); }); @@ -237,7 +227,7 @@ test('should prefix default token with "*:*."', async () => { .set('Content-Type', 'application/json') .expect(201) .expect((res) => { - expect(res.body.secret).toMatch(/\*:\*\..*/); + expect(res.body.secret).toMatch(/\*:default\..*/); }); }); @@ -329,3 +319,15 @@ test('admin token only supports ALL environments', async () => { .set('Content-Type', 'application/json') .expect(400); }); + +test('client tokens cannot span all environments', async () => { + return app.request + .post('/api/admin/api-tokens') + .send({ + username: 'default-client', + type: 'client', + environment: ALL, + }) + .set('Content-Type', 'application/json') + .expect(400); +});