From c4b697b57d2d8527771d47e2ab5eff476b1ca1af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Wed, 15 Sep 2021 20:28:10 +0200 Subject: [PATCH] Feat/api key scoping (#941) Co-authored-by: Christopher Kolstad --- src/lib/db/api-token-store.ts | 13 +- src/lib/db/client-metrics-store.ts | 7 +- .../middleware/api-token-middleware.test.ts | 45 +++- src/lib/middleware/api-token-middleware.ts | 12 +- src/lib/middleware/demo-authentication.ts | 2 +- src/lib/middleware/oss-authentication.ts | 8 +- src/lib/middleware/rbac-middleware.test.ts | 7 + src/lib/middleware/rbac-middleware.ts | 4 +- .../routes/admin-api/api-token-controller.ts | 47 +---- src/lib/routes/client-api/feature.ts | 82 +++++--- src/lib/routes/client-api/metrics.test.ts | 7 +- src/lib/routes/client-api/metrics.ts | 19 +- src/lib/schema/api-token-schema.ts | 17 ++ src/lib/server-impl.ts | 4 +- src/lib/services/api-token-service.ts | 66 ++++-- .../client-metrics/client-metrics-schema.ts | 1 + src/lib/types/api-user.ts | 27 ++- src/lib/types/models/api-token.ts | 22 ++ src/lib/types/stores/api-token-store.ts | 18 +- src/lib/util/graceful-shutdown.ts | 2 +- .../20210913103159-api-keys-scoping.js | 19 ++ .../e2e/api/admin/api-token.auth.e2e.test.ts | 2 +- src/test/e2e/api/admin/api-token.e2e.test.ts | 160 +++++++++++++- .../client/feature.token.access.e2e.test.ts | 199 ++++++++++++++++++ .../api/client/metrics.e2e.access.e2e.test.ts | 45 ++++ .../services/api-token-service.e2e.test.ts | 29 ++- src/test/fixtures/fake-api-token-store.ts | 8 +- 27 files changed, 716 insertions(+), 156 deletions(-) create mode 100644 src/lib/schema/api-token-schema.ts create mode 100644 src/lib/types/models/api-token.ts create mode 100644 src/migrations/20210913103159-api-keys-scoping.js create mode 100644 src/test/e2e/api/client/feature.token.access.e2e.test.ts create mode 100644 src/test/e2e/api/client/metrics.e2e.access.e2e.test.ts diff --git a/src/lib/db/api-token-store.ts b/src/lib/db/api-token-store.ts index 34eecb6892..cfa8a2c014 100644 --- a/src/lib/db/api-token-store.ts +++ b/src/lib/db/api-token-store.ts @@ -4,15 +4,17 @@ import metricsHelper from '../util/metrics-helper'; import { DB_TIME } from '../metric-events'; import { Logger, LogProvider } from '../logger'; import NotFoundError from '../error/notfound-error'; +import { IApiTokenStore } from '../types/stores/api-token-store'; import { ApiTokenType, IApiToken, IApiTokenCreate, - IApiTokenStore, -} from '../types/stores/api-token-store'; +} from '../types/models/api-token'; const TABLE = 'api_tokens'; +const ALL = '*'; + interface ITokenTable { id: number; secret: string; @@ -21,12 +23,17 @@ interface ITokenTable { expires_at?: Date; created_at: Date; seen_at?: Date; + environment: string; + project: string; } const toRow = (newToken: IApiTokenCreate) => ({ username: newToken.username, secret: newToken.secret, type: newToken.type, + project: newToken.project === ALL ? undefined : newToken.project, + environment: + newToken.environment === ALL ? undefined : newToken.environment, expires_at: newToken.expiresAt, }); @@ -34,6 +41,8 @@ const toToken = (row: ITokenTable): IApiToken => ({ secret: row.secret, username: row.username, type: row.type, + environment: row.environment ? row.environment : ALL, + project: row.project ? row.project : ALL, expiresAt: row.expires_at, createdAt: row.created_at, }); diff --git a/src/lib/db/client-metrics-store.ts b/src/lib/db/client-metrics-store.ts index 7f39b3afc2..6e11993491 100644 --- a/src/lib/db/client-metrics-store.ts +++ b/src/lib/db/client-metrics-store.ts @@ -71,7 +71,12 @@ export class ClientMetricsStore } } - // Insert new client metrics + /** + * Insert client metrics. In the future we will isolate "appName" and "environment" + * in separate columns in the database to make it easier to query the data. + * + * @param metrics sent from the client SDK. + */ async insert(metrics: IClientMetric): Promise { const stopTimer = this.startTimer('insert'); diff --git a/src/lib/middleware/api-token-middleware.test.ts b/src/lib/middleware/api-token-middleware.test.ts index 317e0339c2..764bab52d5 100644 --- a/src/lib/middleware/api-token-middleware.test.ts +++ b/src/lib/middleware/api-token-middleware.test.ts @@ -3,6 +3,7 @@ import getLogger from '../../test/fixtures/no-logger'; import { CLIENT } from '../types/permissions'; import { createTestConfig } from '../../test/config/test-config'; import ApiUser from '../types/api-user'; +import { ALL, ApiTokenType } from '../types/models/api-token'; let config: any; @@ -55,10 +56,13 @@ test('should not add user if unknown token', async () => { expect(req.user).toBeFalsy(); }); -test('should add user if unknown token', async () => { +test('should add user if known token', async () => { const apiUser = new ApiUser({ username: 'default', permissions: [CLIENT], + project: ALL, + environment: ALL, + type: ApiTokenType.CLIENT, }); const apiTokenService = { getUserForToken: jest.fn().mockReturnValue(apiUser), @@ -71,6 +75,7 @@ test('should add user if unknown token', async () => { const req = { header: jest.fn().mockReturnValue('some-known-token'), user: undefined, + path: '/api/client', }; await func(req, undefined, cb); @@ -80,10 +85,47 @@ test('should add user if unknown token', async () => { expect(req.user).toBe(apiUser); }); +test('should not add user if not /api/client', async () => { + const apiUser = new ApiUser({ + username: 'default', + permissions: [CLIENT], + project: ALL, + environment: ALL, + type: ApiTokenType.CLIENT, + }); + const apiTokenService = { + getUserForToken: jest.fn().mockReturnValue(apiUser), + }; + + const func = apiTokenMiddleware(config, { apiTokenService }); + + const cb = jest.fn(); + + const res = { + sendStatus: jest.fn(), + }; + + const req = { + header: jest.fn().mockReturnValue('some-known-token'), + user: undefined, + path: '/api/admin', + }; + + await func(req, res, cb); + + expect(cb).not.toHaveBeenCalled(); + expect(req.header).toHaveBeenCalled(); + expect(req.user).toBeUndefined(); + expect(res.sendStatus).toHaveBeenCalledWith(403); +}); + test('should not add user if disabled', async () => { const apiUser = new ApiUser({ username: 'default', permissions: [CLIENT], + project: ALL, + environment: ALL, + type: ApiTokenType.CLIENT, }); const apiTokenService = { getUserForToken: jest.fn().mockReturnValue(apiUser), @@ -136,6 +178,7 @@ test('should call next if apiTokenService throws', async () => { }); test('should call next if apiTokenService throws x2', async () => { + jest.spyOn(global.console, 'error').mockImplementation(() => jest.fn()); const apiTokenService = { getUserForToken: () => { throw new Error('hi there, i am stupid'); diff --git a/src/lib/middleware/api-token-middleware.ts b/src/lib/middleware/api-token-middleware.ts index ef597eea06..10c82a3380 100644 --- a/src/lib/middleware/api-token-middleware.ts +++ b/src/lib/middleware/api-token-middleware.ts @@ -1,6 +1,11 @@ /* eslint-disable @typescript-eslint/explicit-module-boundary-types */ +import { ApiTokenType } from '../types/models/api-token'; import { IUnleashConfig } from '../types/option'; +const isClientApi = ({ path }) => { + return path && path.startsWith('/api/client'); +}; + const apiAccessMiddleware = ( { getLogger, @@ -9,14 +14,14 @@ const apiAccessMiddleware = ( { apiTokenService }: any, ): any => { const logger = getLogger('/middleware/api-token.ts'); - logger.info('Enabling api-token middleware'); + logger.debug('Enabling api-token middleware'); if (!authentication.enableApiToken) { return (req, res, next) => next(); } return (req, res, next) => { - if (req.apiUser) { + if (req.user) { return next(); } @@ -24,6 +29,9 @@ const apiAccessMiddleware = ( const apiToken = req.header('authorization'); const apiUser = apiTokenService.getUserForToken(apiToken); if (apiUser) { + if (apiUser.type === ApiTokenType.CLIENT && !isClientApi(req)) { + return res.sendStatus(403); + } req.user = apiUser; } } catch (error) { diff --git a/src/lib/middleware/demo-authentication.ts b/src/lib/middleware/demo-authentication.ts index e061006050..ff8e8db30f 100644 --- a/src/lib/middleware/demo-authentication.ts +++ b/src/lib/middleware/demo-authentication.ts @@ -30,7 +30,7 @@ function demoAuthentication( next(); }); - app.use(`${basePath}/api/admin/`, (req, res, next) => { + app.use(`${basePath}/api`, (req, res, next) => { // @ts-ignore if (req.user) { return next(); diff --git a/src/lib/middleware/oss-authentication.ts b/src/lib/middleware/oss-authentication.ts index 01a33e8692..8828399aaf 100644 --- a/src/lib/middleware/oss-authentication.ts +++ b/src/lib/middleware/oss-authentication.ts @@ -1,4 +1,5 @@ -import { Application, NextFunction, Request, Response } from 'express'; +import { Application, NextFunction, Response } from 'express'; +import { IAuthRequest } from '../routes/unleash-types'; import AuthenticationRequired from '../types/authentication-required'; function ossAuthHook(app: Application, baseUriPath: string): void { @@ -11,14 +12,11 @@ function ossAuthHook(app: Application, baseUriPath: string): void { app.use( `${baseUriPath}/api`, - async (req: Request, res: Response, next: NextFunction) => { - // @ts-ignore + async (req: IAuthRequest, res: Response, next: NextFunction) => { if (req.session && req.session.user) { - // @ts-ignore req.user = req.session.user; return next(); } - // @ts-ignore if (req.user) { return next(); } diff --git a/src/lib/middleware/rbac-middleware.test.ts b/src/lib/middleware/rbac-middleware.test.ts index d9cade938c..82bdd90f05 100644 --- a/src/lib/middleware/rbac-middleware.test.ts +++ b/src/lib/middleware/rbac-middleware.test.ts @@ -6,6 +6,7 @@ import { createTestConfig } from '../../test/config/test-config'; import ApiUser from '../types/api-user'; import { IFeatureToggleStore } from '../types/stores/feature-toggle-store'; import FakeFeatureToggleStore from '../../test/fixtures/fake-feature-toggle-store'; +import { ApiTokenType } from '../types/models/api-token'; let config: IUnleashConfig; let featureToggleStore: IFeatureToggleStore; @@ -46,6 +47,9 @@ test('should give api-user ADMIN permission', async () => { user: new ApiUser({ username: 'api', permissions: [perms.ADMIN], + project: '*', + environment: '*', + type: ApiTokenType.ADMIN, }), }; @@ -68,6 +72,9 @@ test('should not give api-user ADMIN permission', async () => { user: new ApiUser({ username: 'api', permissions: [perms.CLIENT], + project: '*', + environment: '*', + type: ApiTokenType.CLIENT, }), }; diff --git a/src/lib/middleware/rbac-middleware.ts b/src/lib/middleware/rbac-middleware.ts index d91152c486..c5da8c6f07 100644 --- a/src/lib/middleware/rbac-middleware.ts +++ b/src/lib/middleware/rbac-middleware.ts @@ -22,8 +22,8 @@ const rbacMiddleware = ( { featureToggleStore }: Pick, accessService: PermissionChecker, ): any => { - const logger = config.getLogger('/middleware/rbac-middleware.js'); - logger.info('Enabling RBAC'); + const logger = config.getLogger('/middleware/rbac-middleware.ts'); + logger.debug('Enabling RBAC middleware'); return (req, res, next) => { req.checkRbac = async (permission: string) => { diff --git a/src/lib/routes/admin-api/api-token-controller.ts b/src/lib/routes/admin-api/api-token-controller.ts index a016355579..bc212bb561 100644 --- a/src/lib/routes/admin-api/api-token-controller.ts +++ b/src/lib/routes/admin-api/api-token-controller.ts @@ -13,7 +13,8 @@ import { AccessService } from '../../services/access-service'; import { IAuthRequest } from '../unleash-types'; import User from '../../types/user'; import { IUnleashConfig } from '../../types/option'; -import { ApiTokenType } from '../../types/stores/api-token-store'; +import { ApiTokenType } from '../../types/models/api-token'; +import { createApiToken } from '../../schema/api-token-schema'; interface IServices { apiTokenService: ApiTokenService; @@ -64,41 +65,16 @@ class ApiTokenController extends Controller { } async createApiToken(req: IAuthRequest, res: Response): Promise { - const { username, type, expiresAt } = req.body; - - if (!username || !type) { - this.logger.error(req.body); - return res.status(400).send(); - } - - const tokenType = - type.toLowerCase() === 'admin' - ? ApiTokenType.ADMIN - : ApiTokenType.CLIENT; - - try { - const token = await this.apiTokenService.creteApiToken({ - type: tokenType, - username, - expiresAt, - }); - return res.status(201).json(token); - } catch (error) { - this.logger.error('error creating api-token', error); - return res.status(500); - } + const createToken = await createApiToken.validateAsync(req.body); + const token = await this.apiTokenService.createApiToken(createToken); + return res.status(201).json(token); } async deleteApiToken(req: IAuthRequest, res: Response): Promise { const { token } = req.params; - try { - await this.apiTokenService.delete(token); - res.status(200).end(); - } catch (error) { - this.logger.error('error creating api-token', error); - res.status(500); - } + await this.apiTokenService.delete(token); + res.status(200).end(); } async updateApiToken(req: IAuthRequest, res: Response): Promise { @@ -110,13 +86,8 @@ class ApiTokenController extends Controller { return res.status(400).send(); } - try { - await this.apiTokenService.updateExpiry(token, expiresAt); - return res.status(200).end(); - } catch (error) { - this.logger.error('error creating api-token', error); - return res.status(500); - } + await this.apiTokenService.updateExpiry(token, expiresAt); + return res.status(200).end(); } } diff --git a/src/lib/routes/client-api/feature.ts b/src/lib/routes/client-api/feature.ts index b9003aad22..e4e363a6a3 100644 --- a/src/lib/routes/client-api/feature.ts +++ b/src/lib/routes/client-api/feature.ts @@ -1,5 +1,5 @@ import memoizee from 'memoizee'; -import { Request, Response } from 'express'; +import { Response } from 'express'; import Controller from '../controller'; import { IUnleashServices } from '../../types/services'; import { IUnleashConfig } from '../../types/option'; @@ -8,9 +8,17 @@ import { Logger } from '../../logger'; import { querySchema } from '../../schema/feature-schema'; import { IFeatureToggleQuery } from '../../types/model'; import NotFoundError from '../../error/notfound-error'; +import { IAuthRequest } from '../unleash-types'; +import ApiUser from '../../types/api-user'; +import { ALL } from '../../types/models/api-token'; const version = 2; +interface QueryOverride { + project?: string[]; + environment?: string; +} + export default class FeatureController extends Controller { private readonly logger: Logger; @@ -50,20 +58,34 @@ export default class FeatureController extends Controller { } } - async getAll(req: Request, res: Response): Promise { - const query = await this.prepQuery(req.query); - let features; - if (this.cache) { - features = await this.cachedFeatures(query); - } else { - features = await this.featureToggleServiceV2.getClientFeatures( - query, - ); + private async resolveQuery( + req: IAuthRequest, + ): Promise { + const { user, query } = req; + + const override: QueryOverride = {}; + if (user instanceof ApiUser) { + if (user.project !== ALL) { + override.project = [user.project]; + } + if (user.environment !== ALL) { + override.environment = user.environment; + } } - res.json({ version, features }); + + const q = { ...query, ...override }; + return this.prepQuery(q); } - async prepQuery({ + // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types + private paramToArray(param: any) { + if (!param) { + return param; + } + return Array.isArray(param) ? param : [param]; + } + + private async prepQuery({ tag, project, namePrefix, @@ -86,29 +108,25 @@ export default class FeatureController extends Controller { return query; } - // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types - paramToArray(param: any) { - if (!param) { - return param; + async getAll(req: IAuthRequest, res: Response): Promise { + const featureQuery = await this.resolveQuery(req); + let features; + if (this.cache) { + features = await this.cachedFeatures(featureQuery); + } else { + features = await this.featureToggleServiceV2.getClientFeatures( + featureQuery, + ); } - return Array.isArray(param) ? param : [param]; + res.json({ version, features, query: featureQuery }); } - async getFeatureToggle( - req: Request< - { featureName: string }, - any, - any, - { environment?: string } - >, - res: Response, - ): Promise { + async getFeatureToggle(req: IAuthRequest, res: Response): Promise { const name = req.params.featureName; - const { environment } = req.query; - const toggles = await this.featureToggleServiceV2.getClientFeatures({ - namePrefix: name, - environment, - }); + const featureQuery = await this.resolveQuery(req); + const q = { ...featureQuery, namePrefix: name }; + const toggles = await this.featureToggleServiceV2.getClientFeatures(q); + const toggle = toggles.find((t) => t.name === name); if (!toggle) { throw new NotFoundError(`Could not find feature toggle ${name}`); @@ -116,5 +134,3 @@ export default class FeatureController extends Controller { res.json(toggle).end(); } } - -module.exports = FeatureController; diff --git a/src/lib/routes/client-api/metrics.test.ts b/src/lib/routes/client-api/metrics.test.ts index 58cf59dee9..4cbb1bb55a 100644 --- a/src/lib/routes/client-api/metrics.test.ts +++ b/src/lib/routes/client-api/metrics.test.ts @@ -43,7 +43,6 @@ afterEach(() => { }); test('should validate client metrics', () => { - expect.assertions(0); return request .post('/api/client/metrics') .send({ random: 'blush' }) @@ -51,7 +50,6 @@ test('should validate client metrics', () => { }); test('should accept empty client metrics', () => { - expect.assertions(0); return request .post('/api/client/metrics') .send({ @@ -67,7 +65,6 @@ test('should accept empty client metrics', () => { }); test('should accept client metrics with yes/no', () => { - expect.assertions(0); return request .post('/api/client/metrics') .send({ @@ -88,7 +85,6 @@ test('should accept client metrics with yes/no', () => { }); test('should accept client metrics with variants', () => { - expect.assertions(0); return request .post('/api/client/metrics') .send({ @@ -113,7 +109,6 @@ test('should accept client metrics with variants', () => { }); test('should accept client metrics without yes/no', () => { - expect.assertions(0); return request .post('/api/client/metrics') .send({ @@ -133,7 +128,7 @@ test('should accept client metrics without yes/no', () => { .expect(202); }); -test('shema allow empty strings', () => { +test('schema allow empty strings', () => { const data = { appName: 'java-test', instanceId: 'instance y', diff --git a/src/lib/routes/client-api/metrics.ts b/src/lib/routes/client-api/metrics.ts index 1732c9b78c..5b043eb948 100644 --- a/src/lib/routes/client-api/metrics.ts +++ b/src/lib/routes/client-api/metrics.ts @@ -1,9 +1,12 @@ -import { Request, Response } from 'express'; +import { Response } from 'express'; import Controller from '../controller'; import { IUnleashServices } from '../../types'; import { IUnleashConfig } from '../../types/option'; import ClientMetricsService from '../../services/client-metrics'; import { Logger } from '../../logger'; +import { IAuthRequest } from '../unleash-types'; +import ApiUser from '../../types/api-user'; +import { ALL } from '../../types/models/api-token'; export default class ClientMetricsController extends Controller { logger: Logger; @@ -23,13 +26,13 @@ export default class ClientMetricsController extends Controller { this.post('/', this.registerMetrics); } - async registerMetrics( - req: Request, - res: Response, - ): Promise { - const data = req.body; - const clientIp = req.ip; - + async registerMetrics(req: IAuthRequest, res: Response): Promise { + const { body: data, ip: clientIp, user } = req; + if (user instanceof ApiUser) { + if (user.environment !== ALL) { + data.environment = user.environment; + } + } await this.metrics.registerClientMetrics(data, clientIp); return res.status(202).end(); } diff --git a/src/lib/schema/api-token-schema.ts b/src/lib/schema/api-token-schema.ts new file mode 100644 index 0000000000..ed653b9286 --- /dev/null +++ b/src/lib/schema/api-token-schema.ts @@ -0,0 +1,17 @@ +import joi from 'joi'; +import { ALL, ApiTokenType } from '../types/models/api-token'; + +export const createApiToken = joi + .object() + .keys({ + username: joi.string().required(), + type: joi + .string() + .lowercase() + .required() + .valid(ApiTokenType.ADMIN, ApiTokenType.CLIENT), + expiresAt: joi.date().optional(), + project: joi.string().optional().default(ALL), + environment: joi.string().optional().default(ALL), + }) + .options({ stripUnknown: true, allowUnknown: false, abortEarly: false }); diff --git a/src/lib/server-impl.ts b/src/lib/server-impl.ts index 688ca9400c..1613365242 100644 --- a/src/lib/server-impl.ts +++ b/src/lib/server-impl.ts @@ -115,9 +115,9 @@ async function start(opts: IUnleashOptions = {}): Promise { if (config.db.disableMigration) { logger.info('DB migration: disabled'); } else { - logger.info('DB migration: start'); + logger.debug('DB migration: start'); await migrateDb(config); - logger.info('DB migration: end'); + logger.debug('DB migration: end'); } } catch (err) { logger.error('Failed to migrate db', err); diff --git a/src/lib/services/api-token-service.ts b/src/lib/services/api-token-service.ts index 5ae12ee7d3..3f9ca0901b 100644 --- a/src/lib/services/api-token-service.ts +++ b/src/lib/services/api-token-service.ts @@ -5,19 +5,17 @@ import { IUnleashStores } from '../types/stores'; import { IUnleashConfig } from '../types/option'; import ApiUser from '../types/api-user'; import { + ALL, ApiTokenType, IApiToken, - IApiTokenStore, -} from '../types/stores/api-token-store'; + IApiTokenCreate, +} from '../types/models/api-token'; +import { IApiTokenStore } from '../types/stores/api-token-store'; +import { FOREIGN_KEY_VIOLATION } from '../error/db-error'; +import BadDataError from '../error/bad-data-error'; const ONE_MINUTE = 60_000; -interface CreateTokenRequest { - username: string; - type: ApiTokenType; - expiresAt?: Date; -} - export class ApiTokenService { private store: IApiTokenStore; @@ -66,6 +64,9 @@ export class ApiTokenService { return new ApiUser({ username: token.username, permissions, + project: token.project, + environment: token.environment, + type: token.type, }); } return undefined; @@ -82,16 +83,49 @@ export class ApiTokenService { return this.store.delete(secret); } - public async creteApiToken( - creteTokenRequest: CreateTokenRequest, - ): Promise { - const secret = this.generateSecretKey(); - const createNewToken = { ...creteTokenRequest, secret }; - return this.store.insert(createNewToken); + private validateAdminToken({ type, project, environment }) { + if (type === ApiTokenType.ADMIN && project !== ALL) { + throw new BadDataError( + 'Admin token cannot be scoped to single project', + ); + } + + if (type === ApiTokenType.ADMIN && environment !== ALL) { + throw new BadDataError( + 'Admin token cannot be scoped to single environment', + ); + } } - private generateSecretKey() { - return crypto.randomBytes(32).toString('hex'); + public async createApiToken( + newToken: Omit, + ): Promise { + this.validateAdminToken(newToken); + + const secret = this.generateSecretKey(newToken); + const createNewToken = { ...newToken, secret }; + + try { + const token = await this.store.insert(createNewToken); + this.activeTokens.push(token); + return token; + } catch (error) { + if (error.code === FOREIGN_KEY_VIOLATION) { + let { message } = error; + if (error.constraint === 'api_tokens_project_fkey') { + message = `Project=${newToken.project} does not exist`; + } else if (error.constraint === 'api_tokens_environment_fkey') { + message = `Environment=${newToken.environment} does not exist`; + } + throw new BadDataError(message); + } + throw error; + } + } + + private generateSecretKey({ project, environment }) { + const randomStr = crypto.randomBytes(28).toString('hex'); + return `${project}:${environment}.${randomStr}`; } destroy(): void { diff --git a/src/lib/services/client-metrics/client-metrics-schema.ts b/src/lib/services/client-metrics/client-metrics-schema.ts index ae10825bca..f0bd754923 100644 --- a/src/lib/services/client-metrics/client-metrics-schema.ts +++ b/src/lib/services/client-metrics/client-metrics-schema.ts @@ -13,6 +13,7 @@ export const clientMetricsSchema = joi .object() .options({ stripUnknown: true }) .keys({ + environment: joi.string().optional(), appName: joi.string().required(), instanceId: joi.string().required(), bucket: joi diff --git a/src/lib/types/api-user.ts b/src/lib/types/api-user.ts index b115d83a39..28e0bb8d0c 100644 --- a/src/lib/types/api-user.ts +++ b/src/lib/types/api-user.ts @@ -1,23 +1,42 @@ +import { ApiTokenType } from './models/api-token'; import { CLIENT } from './permissions'; interface IApiUserData { username: string; permissions?: string[]; + project: string; + environment: string; + type: ApiTokenType; } export default class ApiUser { - isAPI: boolean = true; + readonly isAPI: boolean = true; - username: string; + readonly username: string; - permissions: string[]; + readonly permissions: string[]; - constructor({ username, permissions = [CLIENT] }: IApiUserData) { + readonly project: string; + + readonly environment: string; + + readonly type: ApiTokenType; + + constructor({ + username, + permissions = [CLIENT], + project, + environment, + type, + }: IApiUserData) { if (!username) { throw new TypeError('username is required'); } this.username = username; this.permissions = permissions; + this.project = project; + this.environment = environment; + this.type = type; } } diff --git a/src/lib/types/models/api-token.ts b/src/lib/types/models/api-token.ts new file mode 100644 index 0000000000..20075a614f --- /dev/null +++ b/src/lib/types/models/api-token.ts @@ -0,0 +1,22 @@ +export const ALL = '*'; + +export enum ApiTokenType { + CLIENT = 'client', + ADMIN = 'admin', +} + +export interface IApiTokenCreate { + secret: string; + username: string; + type: ApiTokenType; + environment: string; + project: string; + expiresAt?: Date; +} + +export interface IApiToken extends IApiTokenCreate { + createdAt: Date; + seenAt?: Date; + environment: string; + project: string; +} diff --git a/src/lib/types/stores/api-token-store.ts b/src/lib/types/stores/api-token-store.ts index 9d89c36c88..b48271423f 100644 --- a/src/lib/types/stores/api-token-store.ts +++ b/src/lib/types/stores/api-token-store.ts @@ -1,22 +1,6 @@ +import { IApiToken, IApiTokenCreate } from '../models/api-token'; import { Store } from './store'; -export enum ApiTokenType { - CLIENT = 'client', - ADMIN = 'admin', -} - -export interface IApiTokenCreate { - secret: string; - username: string; - type: ApiTokenType; - expiresAt?: Date; -} - -export interface IApiToken extends IApiTokenCreate { - createdAt: Date; - seenAt?: Date; -} - export interface IApiTokenStore extends Store { getAllActive(): Promise; insert(newToken: IApiTokenCreate): Promise; diff --git a/src/lib/util/graceful-shutdown.ts b/src/lib/util/graceful-shutdown.ts index f5cb519ff6..66405efcc1 100644 --- a/src/lib/util/graceful-shutdown.ts +++ b/src/lib/util/graceful-shutdown.ts @@ -14,7 +14,7 @@ function registerGracefulShutdown(unleash: IUnleash, logger: Logger): void { } }; - logger.info('Registering graceful shutdown'); + logger.debug('Registering graceful shutdown'); process.on('SIGINT', unleashCloser('SIGINT')); process.on('SIGHUP', unleashCloser('SIGHUP')); diff --git a/src/migrations/20210913103159-api-keys-scoping.js b/src/migrations/20210913103159-api-keys-scoping.js new file mode 100644 index 0000000000..68ab413c9c --- /dev/null +++ b/src/migrations/20210913103159-api-keys-scoping.js @@ -0,0 +1,19 @@ +exports.up = function (db, cb) { + db.runSql( + ` + ALTER TABLE api_tokens ADD COLUMN project VARCHAR REFERENCES PROJECTS(id) ON DELETE CASCADE;; + ALTER TABLE api_tokens ADD COLUMN environment VARCHAR REFERENCES environments(name) ON DELETE CASCADE;; + `, + cb, + ); +}; + +exports.down = function (db, cb) { + db.runSql( + ` + ALTER TABLE api_tokens DROP COLUMN project; + ALTER TABLE api_tokens DROP COLUMN environment; + `, + cb, + ); +}; diff --git a/src/test/e2e/api/admin/api-token.auth.e2e.test.ts b/src/test/e2e/api/admin/api-token.auth.e2e.test.ts index 482784d9a4..a48117c040 100644 --- a/src/test/e2e/api/admin/api-token.auth.e2e.test.ts +++ b/src/test/e2e/api/admin/api-token.auth.e2e.test.ts @@ -1,7 +1,7 @@ import { setupAppWithCustomAuth } from '../../helpers/test-helper'; import dbInit from '../../helpers/database-init'; import getLogger from '../../../fixtures/no-logger'; -import { ApiTokenType } from '../../../../lib/types/stores/api-token-store'; +import { ApiTokenType } from '../../../../lib/types/models/api-token'; import { RoleName } from '../../../../lib/types/model'; let stores; 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 a6f5b8db14..7d79431500 100644 --- a/src/test/e2e/api/admin/api-token.e2e.test.ts +++ b/src/test/e2e/api/admin/api-token.e2e.test.ts @@ -1,7 +1,7 @@ import { setupApp } from '../../helpers/test-helper'; import dbInit from '../../helpers/database-init'; import getLogger from '../../../fixtures/no-logger'; -import { ApiTokenType } from '../../../../lib/types/stores/api-token-store'; +import { ALL, ApiTokenType } from '../../../../lib/types/models/api-token'; let db; let app; @@ -70,6 +70,25 @@ 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({ + username: 'default-admin', + type: 'ADMIN', + }) + .set('Content-Type', 'application/json') + .expect(201) + .expect((res) => { + expect(res.body.username).toBe('default-admin'); + expect(res.body.type).toBe('admin'); + expect(res.body.createdAt).toBeTruthy(); + expect(res.body.expiresAt).toBeFalsy(); + expect(res.body.secret.length > 16).toBe(true); + }); +}); + test('creates new admin token with expiry', async () => { expect.assertions(1); const expiresAt = new Date(); @@ -170,3 +189,142 @@ test('removes api token', async () => { expect(res.body.tokens.length).toBe(0); }); }); + +test('creates new client token: project & environment defaults to "*"', async () => { + return app.request + .post('/api/admin/api-tokens') + .send({ + username: 'default-client', + type: 'client', + }) + .set('Content-Type', 'application/json') + .expect(201) + .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.project).toBe(ALL); + }); +}); + +test('creates new client token with project & environment set', async () => { + return app.request + .post('/api/admin/api-tokens') + .send({ + username: 'default-client', + type: 'client', + project: 'default', + environment: ':global:', + }) + .set('Content-Type', 'application/json') + .expect(201) + .expect((res) => { + expect(res.body.type).toBe('client'); + expect(res.body.secret.length > 16).toBe(true); + expect(res.body.environment).toBe(':global:'); + expect(res.body.project).toBe('default'); + }); +}); + +test('should prefix default token with "*:*."', async () => { + return app.request + .post('/api/admin/api-tokens') + .send({ + username: 'default-client', + type: 'client', + }) + .set('Content-Type', 'application/json') + .expect(201) + .expect((res) => { + expect(res.body.secret).toMatch(/\*:\*\..*/); + }); +}); + +test('should prefix token with "project:environment."', async () => { + return app.request + .post('/api/admin/api-tokens') + .send({ + username: 'default-client', + type: 'client', + project: 'default', + environment: ':global:', + }) + .set('Content-Type', 'application/json') + .expect(201) + .expect((res) => { + expect(res.body.secret).toMatch(/default::global:\..*/); + }); +}); + +test('should not create token for invalid projectId', async () => { + return app.request + .post('/api/admin/api-tokens') + .send({ + username: 'default-client', + type: 'client', + project: 'bogus-project-something', + }) + .set('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body.details[0].message).toMatch( + /bogus-project-something/, + ); + }); +}); + +test('should not create token for invalid environment', async () => { + return app.request + .post('/api/admin/api-tokens') + .send({ + username: 'default-client', + type: 'client', + environment: 'bogus-environment-something', + }) + .set('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body.details[0].message).toMatch( + /bogus-environment-something/, + ); + }); +}); + +test('should not create token for invalid project & environment', async () => { + return app.request + .post('/api/admin/api-tokens') + .send({ + username: 'default-admin', + type: 'admin', + project: 'bogus-project-something', + environment: 'bogus-environment-something', + }) + .set('Content-Type', 'application/json') + .expect(400); +}); + +test('admin token only supports ALL projects', async () => { + return app.request + .post('/api/admin/api-tokens') + .send({ + username: 'default-admin', + type: 'admin', + project: 'default', + environment: '*', + }) + .set('Content-Type', 'application/json') + .expect(400); +}); + +test('admin token only supports ALL environments', async () => { + return app.request + .post('/api/admin/api-tokens') + .send({ + username: 'default-admin', + type: 'admin', + project: '*', + environment: ':global:', + }) + .set('Content-Type', 'application/json') + .expect(400); +}); diff --git a/src/test/e2e/api/client/feature.token.access.e2e.test.ts b/src/test/e2e/api/client/feature.token.access.e2e.test.ts new file mode 100644 index 0000000000..119ec44a0d --- /dev/null +++ b/src/test/e2e/api/client/feature.token.access.e2e.test.ts @@ -0,0 +1,199 @@ +import { IUnleashTest, setupAppWithAuth } from '../../helpers/test-helper'; +import dbInit, { ITestDb } from '../../helpers/database-init'; +import getLogger from '../../../fixtures/no-logger'; +import { ApiTokenService } from '../../../../lib/services/api-token-service'; +import { ApiTokenType } from '../../../../lib/types/models/api-token'; + +let app: IUnleashTest; +let db: ITestDb; + +let apiTokenService: ApiTokenService; + +const environment = 'testing'; +const project = 'default'; +const project2 = 'some'; +const username = 'test'; +const feature1 = 'f1.token.access'; +const feature2 = 'f2.token.access'; +const feature3 = 'f3.p2.token.access'; + +beforeAll(async () => { + db = await dbInit('feature_api_api_access_client', getLogger); + app = await setupAppWithAuth(db.stores); + apiTokenService = app.services.apiTokenService; + + const { featureToggleServiceV2, environmentService } = app.services; + const { environmentStore, projectStore } = db.stores; + + await environmentStore.create({ + name: environment, + displayName: '', + type: 'test', + }); + + await projectStore.create({ + id: project2, + name: 'Test Project 2', + description: '', + }); + + await environmentService.addEnvironmentToProject(environment, project); + await environmentService.addEnvironmentToProject(environment, project2); + + await featureToggleServiceV2.createFeatureToggle( + project, + { + name: feature1, + description: 'the #1 feature', + }, + username, + ); + + await featureToggleServiceV2.createStrategy( + { + name: 'default', + constraints: [], + parameters: {}, + }, + project, + feature1, + ); + await featureToggleServiceV2.createStrategy( + { + name: 'custom-testing', + constraints: [], + parameters: {}, + }, + project, + feature1, + environment, + ); + + // create feature 2 + await featureToggleServiceV2.createFeatureToggle( + project, + { + name: feature2, + }, + username, + ); + await featureToggleServiceV2.createStrategy( + { + name: 'default', + constraints: [], + parameters: {}, + }, + project, + feature2, + environment, + ); + + // create feature 3 + await featureToggleServiceV2.createFeatureToggle( + project2, + { + name: feature3, + }, + username, + ); + await featureToggleServiceV2.createStrategy( + { + name: 'default', + constraints: [], + parameters: {}, + }, + project2, + feature3, + environment, + ); +}); + +afterAll(async () => { + await app.destroy(); + await db.destroy(); +}); + +test('returns feature toggle with :global: config', async () => { + const token = await apiTokenService.createApiToken({ + type: ApiTokenType.CLIENT, + username, + environment: ':global:', + project, + }); + await app.request + .get('/api/client/features') + .set('Authorization', token.secret) + .expect('Content-Type', /json/) + .expect(200) + .expect((res) => { + const { features } = res.body; + const f1 = features.find((f) => f.name === feature1); + const f2 = features.find((f) => f.name === feature2); + expect(features).toHaveLength(2); + expect(f1.strategies).toHaveLength(1); + expect(f2.strategies).toHaveLength(0); + }); +}); + +test('returns feature toggle with :global: config', async () => { + const token = await apiTokenService.createApiToken({ + type: ApiTokenType.CLIENT, + username, + environment, + project, + }); + await app.request + .get('/api/client/features') + .set('Authorization', token.secret) + .expect('Content-Type', /json/) + .expect(200) + .expect((res) => { + const { features, query } = res.body; + const f1 = features.find((f) => f.name === feature1); + const f2 = features.find((f) => f.name === feature2); + + expect(features).toHaveLength(2); + expect(f1.strategies).toHaveLength(2); + expect(f2.strategies).toHaveLength(1); + expect(query.project[0]).toBe(project); + expect(query.environment).toBe(environment); + }); +}); + +test('returns feature toggle for project2', async () => { + const token = await apiTokenService.createApiToken({ + type: ApiTokenType.CLIENT, + username, + environment, + project: project2, + }); + await app.request + .get('/api/client/features') + .set('Authorization', token.secret) + .expect('Content-Type', /json/) + .expect(200) + .expect((res) => { + const { features } = res.body; + const f3 = features.find((f) => f.name === feature3); + expect(features).toHaveLength(1); + expect(f3.strategies).toHaveLength(1); + }); +}); + +test('returns feature toggle for all projects', async () => { + const token = await apiTokenService.createApiToken({ + type: ApiTokenType.CLIENT, + username, + environment, + project: '*', + }); + await app.request + .get('/api/client/features') + .set('Authorization', token.secret) + .expect('Content-Type', /json/) + .expect(200) + .expect((res) => { + const { features } = res.body; + expect(features).toHaveLength(3); + }); +}); diff --git a/src/test/e2e/api/client/metrics.e2e.access.e2e.test.ts b/src/test/e2e/api/client/metrics.e2e.access.e2e.test.ts new file mode 100644 index 0000000000..347fa7b6da --- /dev/null +++ b/src/test/e2e/api/client/metrics.e2e.access.e2e.test.ts @@ -0,0 +1,45 @@ +import { setupAppWithAuth } from '../../helpers/test-helper'; +import metricsExample from '../../../examples/client-metrics.json'; +import dbInit from '../../helpers/database-init'; +import getLogger from '../../../fixtures/no-logger'; +import { ApiTokenType } from '../../../../lib/types/models/api-token'; + +let app; +let db; + +beforeAll(async () => { + db = await dbInit('metrics_api_client', getLogger); + app = await setupAppWithAuth(db.stores); +}); + +afterAll(async () => { + await app.destroy(); + await db.destroy(); +}); + +test('should enrich metrics with environment from api-token', async () => { + const { apiTokenService } = app.services; + const { environmentStore, clientMetricsStore } = db.stores; + + await environmentStore.create({ + name: 'some', + displayName: '', + type: 'test', + }); + + const token = await apiTokenService.createApiToken({ + type: ApiTokenType.CLIENT, + username: 'test', + environment: 'some', + project: '*', + }); + + await app.request + .post('/api/client/metrics') + .set('Authorization', token.secret) + .send(metricsExample) + .expect(202); + + const all = await clientMetricsStore.getAll(); + expect(all[0].metrics.environment).toBe('some'); +}); diff --git a/src/test/e2e/services/api-token-service.e2e.test.ts b/src/test/e2e/services/api-token-service.e2e.test.ts index cdf5195695..8c6e297e05 100644 --- a/src/test/e2e/services/api-token-service.e2e.test.ts +++ b/src/test/e2e/services/api-token-service.e2e.test.ts @@ -2,10 +2,7 @@ import dbInit from '../helpers/database-init'; import getLogger from '../../fixtures/no-logger'; import { ApiTokenService } from '../../../lib/services/api-token-service'; import { createTestConfig } from '../../config/test-config'; -import { - ApiTokenType, - IApiToken, -} from '../../../lib/types/stores/api-token-store'; +import { ApiTokenType, IApiToken } from '../../../lib/types/models/api-token'; let db; let stores; @@ -42,9 +39,11 @@ test('should have empty list of tokens', async () => { }); test('should create client token', async () => { - const token = await apiTokenService.creteApiToken({ + const token = await apiTokenService.createApiToken({ username: 'default-client', type: ApiTokenType.CLIENT, + project: '*', + environment: '*', }); const allTokens = await apiTokenService.getAllTokens(); @@ -56,9 +55,11 @@ test('should create client token', async () => { }); test('should create admin token', async () => { - const token = await apiTokenService.creteApiToken({ + const token = await apiTokenService.createApiToken({ username: 'admin', type: ApiTokenType.ADMIN, + project: '*', + environment: '*', }); expect(token.secret.length > 32).toBe(true); @@ -67,10 +68,12 @@ test('should create admin token', async () => { test('should set expiry of token', async () => { const time = new Date('2022-01-01'); - await apiTokenService.creteApiToken({ + await apiTokenService.createApiToken({ username: 'default-client', type: ApiTokenType.CLIENT, expiresAt: time, + project: '*', + environment: '*', }); const [token] = await apiTokenService.getAllTokens(); @@ -82,10 +85,12 @@ test('should update expiry of token', async () => { const time = new Date('2022-01-01'); const newTime = new Date('2023-01-01'); - const token = await apiTokenService.creteApiToken({ + const token = await apiTokenService.createApiToken({ username: 'default-client', type: ApiTokenType.CLIENT, expiresAt: time, + project: '*', + environment: '*', }); await apiTokenService.updateExpiry(token.secret, newTime); @@ -99,16 +104,20 @@ test('should only return valid tokens', async () => { const today = new Date(); const tomorrow = new Date(today.getTime() + 24 * 60 * 60 * 1000); - await apiTokenService.creteApiToken({ + await apiTokenService.createApiToken({ username: 'default-expired', type: ApiTokenType.CLIENT, expiresAt: new Date('2021-01-01'), + project: '*', + environment: '*', }); - const activeToken = await apiTokenService.creteApiToken({ + const activeToken = await apiTokenService.createApiToken({ username: 'default-valid', type: ApiTokenType.CLIENT, expiresAt: tomorrow, + project: '*', + environment: '*', }); const tokens = await apiTokenService.getAllActiveTokens(); diff --git a/src/test/fixtures/fake-api-token-store.ts b/src/test/fixtures/fake-api-token-store.ts index 1299431ee9..59e2de84c3 100644 --- a/src/test/fixtures/fake-api-token-store.ts +++ b/src/test/fixtures/fake-api-token-store.ts @@ -1,8 +1,6 @@ -import { - IApiToken, - IApiTokenCreate, - IApiTokenStore, -} from '../../lib/types/stores/api-token-store'; +import { IApiTokenStore } from '../../lib/types/stores/api-token-store'; +import { IApiToken, IApiTokenCreate } from '../../lib/types/models/api-token'; + import NotFoundError from '../../lib/error/notfound-error'; export default class FakeApiTokenStore implements IApiTokenStore {