diff --git a/src/lib/create-config.ts b/src/lib/create-config.ts index 9ab90b57c6..4ea2185262 100644 --- a/src/lib/create-config.ts +++ b/src/lib/create-config.ts @@ -21,7 +21,11 @@ import { defaultCustomAuthDenyAll } from './default-custom-auth-deny-all'; import { formatBaseUri } from './util/format-base-uri'; import { minutesToMilliseconds, secondsToMilliseconds } from 'date-fns'; import EventEmitter from 'events'; -import { ApiTokenType, validateApiToken } from './types/models/api-token'; +import { + ApiTokenType, + mapLegacyToken, + validateApiToken, +} from './types/models/api-token'; const safeToUpper = (s: string) => (s ? s.toUpperCase() : s); @@ -198,7 +202,7 @@ const loadTokensFromString = (tokenString: String, tokenType: ApiTokenType) => { type: tokenType, username: 'admin', }; - validateApiToken(token); + validateApiToken(mapLegacyToken(token)); return token; }); return tokens; diff --git a/src/lib/db/api-token-store.ts b/src/lib/db/api-token-store.ts index 04108d4ca1..18139fc4bb 100644 --- a/src/lib/db/api-token-store.ts +++ b/src/lib/db/api-token-store.ts @@ -9,13 +9,16 @@ import { ApiTokenType, IApiToken, IApiTokenCreate, + isAllProjects, } from '../types/models/api-token'; +import { ALL_PROJECTS } from '../../lib/services/access-service'; const TABLE = 'api_tokens'; +const API_LINK_TABLE = 'api_token_project'; const ALL = '*'; -interface ITokenTable { +interface ITokenInsert { id: number; secret: string; username: string; @@ -24,28 +27,50 @@ interface ITokenTable { created_at: Date; seen_at?: Date; environment: string; +} + +interface ITokenRow extends ITokenInsert { project: string; } +const tokenRowReducer = (acc, tokenRow) => { + const { project, ...token } = tokenRow; + if (!acc[tokenRow.secret]) { + acc[tokenRow.secret] = { + secret: token.secret, + username: token.username, + type: token.type, + project: ALL, + projects: [ALL], + environment: token.environment ? token.environment : ALL, + expiresAt: token.expires_at, + createdAt: token.created_at, + }; + } + const currentToken = acc[tokenRow.secret]; + if (tokenRow.project) { + if (isAllProjects(currentToken.projects)) { + currentToken.projects = []; + } + currentToken.projects.push(tokenRow.project); + currentToken.project = currentToken.projects.join(','); + } + return acc; +}; + 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, }); -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, -}); +const toTokens = (rows: any[]): IApiToken[] => { + const tokens = rows.reduce(tokenRowReducer, {}); + return Object.values(tokens); +}; export class ApiTokenStore implements IApiTokenStore { private logger: Logger; @@ -72,26 +97,64 @@ export class ApiTokenStore implements IApiTokenStore { async getAll(): Promise { const stopTimer = this.timer('getAll'); - const rows = await this.db(TABLE); + const rows = await this.makeTokenProjectQuery(); stopTimer(); - return rows.map(toToken); + return toTokens(rows); } async getAllActive(): Promise { const stopTimer = this.timer('getAllActive'); - const rows = await this.db(TABLE) + const rows = await this.makeTokenProjectQuery() .where('expires_at', 'IS', null) .orWhere('expires_at', '>', 'now()'); stopTimer(); - return rows.map(toToken); + return toTokens(rows); + } + + private makeTokenProjectQuery() { + return this.db(`${TABLE} as tokens`) + .leftJoin( + `${API_LINK_TABLE} as token_project_link`, + 'tokens.secret', + 'token_project_link.secret', + ) + .select( + 'tokens.secret', + 'username', + 'type', + 'expires_at', + 'created_at', + 'seen_at', + 'environment', + 'token_project_link.project', + ); } async insert(newToken: IApiTokenCreate): Promise { - const [row] = await this.db(TABLE).insert( - toRow(newToken), - ['created_at'], - ); - return { ...newToken, createdAt: row.created_at }; + const response = await this.db.transaction(async (tx) => { + const [row] = await tx(TABLE).insert( + toRow(newToken), + ['created_at'], + ); + + const updateProjectTasks = (newToken.projects || []) + .filter((project) => { + return project !== ALL_PROJECTS; + }) + .map((project) => { + return tx.raw( + `INSERT INTO ${API_LINK_TABLE} VALUES (?, ?)`, + [newToken.secret, project], + ); + }); + await Promise.all(updateProjectTasks); + return { + ...newToken, + project: newToken.projects?.join(',') || '*', + createdAt: row.created_at, + }; + }); + return response; } destroy(): void {} @@ -106,25 +169,25 @@ export class ApiTokenStore implements IApiTokenStore { } async get(key: string): Promise { - const row = await this.db(TABLE).where('secret', key).first(); - return toToken(row); + const row = await this.makeTokenProjectQuery().where('secret', key); + return toTokens(row)[0]; } async delete(secret: string): Promise { - return this.db(TABLE).where({ secret }).del(); + return this.db(TABLE).where({ secret }).del(); } async deleteAll(): Promise { - return this.db(TABLE).del(); + return this.db(TABLE).del(); } async setExpiry(secret: string, expiresAt: Date): Promise { - const rows = await this.db(TABLE) + const rows = await this.makeTokenProjectQuery() .update({ expires_at: expiresAt }) .where({ secret }) .returning('*'); if (rows.length > 0) { - return toToken(rows[0]); + return toTokens(rows)[0]; } throw new NotFoundError('Could not find api-token.'); } diff --git a/src/lib/routes/client-api/feature.ts b/src/lib/routes/client-api/feature.ts index 664eed7dc9..8274982df0 100644 --- a/src/lib/routes/client-api/feature.ts +++ b/src/lib/routes/client-api/feature.ts @@ -10,7 +10,7 @@ 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'; +import { ALL, isAllProjects } from '../../types/models/api-token'; const version = 2; @@ -65,8 +65,8 @@ export default class FeatureController extends Controller { const override: QueryOverride = {}; if (user instanceof ApiUser) { - if (user.project !== ALL) { - override.project = [user.project]; + if (!isAllProjects(user.projects)) { + override.project = user.projects; } if (user.environment !== ALL) { override.environment = user.environment; diff --git a/src/lib/schema/api-token-schema.test.ts b/src/lib/schema/api-token-schema.test.ts new file mode 100644 index 0000000000..cdd272ec51 --- /dev/null +++ b/src/lib/schema/api-token-schema.test.ts @@ -0,0 +1,44 @@ +import { ALL } from '../types/models/api-token'; +import { createApiToken } from './api-token-schema'; + +test('should reject token with projects and project', async () => { + expect.assertions(1); + try { + await createApiToken.validateAsync({ + username: 'test', + type: 'admin', + project: 'default', + projects: ['default'], + }); + } catch (error) { + expect(error.details[0].message).toEqual( + '"project" must not exist simultaneously with [projects]', + ); + } +}); + +test('should not have default project set if projects is present', async () => { + let token = await createApiToken.validateAsync({ + username: 'test', + type: 'admin', + projects: ['default'], + }); + expect(token.project).not.toBeDefined(); +}); + +test('should have project set to default if projects is missing', async () => { + let token = await createApiToken.validateAsync({ + username: 'test', + type: 'admin', + }); + expect(token.project).toBe(ALL); +}); + +test('should not have projects set if project is present', async () => { + let token = await createApiToken.validateAsync({ + username: 'test', + type: 'admin', + project: 'default', + }); + expect(token.projects).not.toBeDefined(); +}); diff --git a/src/lib/schema/api-token-schema.ts b/src/lib/schema/api-token-schema.ts index be71638eb1..c6725f67a2 100644 --- a/src/lib/schema/api-token-schema.ts +++ b/src/lib/schema/api-token-schema.ts @@ -12,11 +12,16 @@ export const createApiToken = joi .required() .valid(ApiTokenType.ADMIN, ApiTokenType.CLIENT), expiresAt: joi.date().optional(), - project: joi.string().optional().default(ALL), + project: joi.when('projects', { + not: joi.required(), + then: joi.string().optional().default(ALL), + }), + projects: joi.array().min(0).optional(), environment: joi.when('type', { is: joi.string().valid(ApiTokenType.CLIENT), then: joi.string().optional().default(DEFAULT_ENV), otherwise: joi.string().optional().default(ALL), }), }) + .nand('project', 'projects') .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 ec8f590e99..3916d5201e 100644 --- a/src/lib/services/api-token-service.ts +++ b/src/lib/services/api-token-service.ts @@ -7,9 +7,12 @@ import ApiUser from '../types/api-user'; import { ApiTokenType, IApiToken, + ILegacyApiTokenCreate, IApiTokenCreate, validateApiToken, validateApiTokenEnvironment, + mapLegacyToken, + mapLegacyTokenWithSecret, } from '../types/models/api-token'; import { IApiTokenStore } from '../types/stores/api-token-store'; import { FOREIGN_KEY_VIOLATION } from '../error/db-error'; @@ -67,13 +70,15 @@ export class ApiTokenService { return this.store.getAllActive(); } - private async initApiTokens(tokens: IApiTokenCreate[]) { + private async initApiTokens(tokens: ILegacyApiTokenCreate[]) { const tokenCount = await this.store.count(); if (tokenCount > 0) { return; } try { - const createAll = tokens.map((t) => this.insertNewApiToken(t)); + const createAll = tokens + .map(mapLegacyTokenWithSecret) + .map((t) => this.insertNewApiToken(t)); await Promise.all(createAll); } catch (e) { this.logger.error('Unable to create initial Admin API tokens'); @@ -89,7 +94,7 @@ export class ApiTokenService { return new ApiUser({ username: token.username, permissions, - project: token.project, + projects: token.projects, environment: token.environment, type: token.type, }); @@ -108,7 +113,17 @@ export class ApiTokenService { return this.store.delete(secret); } + /** + * @deprecated This may be removed in a future release, prefer createApiTokenWithProjects + */ public async createApiToken( + newToken: Omit, + ): Promise { + const token = mapLegacyToken(newToken); + return this.createApiTokenWithProjects(token); + } + + public async createApiTokenWithProjects( newToken: Omit, ): Promise { validateApiToken(newToken); @@ -131,8 +146,11 @@ export class ApiTokenService { } catch (error) { if (error.code === FOREIGN_KEY_VIOLATION) { let { message } = error; - if (error.constraint === 'api_tokens_project_fkey') { - message = `Project=${newApiToken.project} does not exist`; + if (error.constraint === 'api_token_project_project_fkey') { + message = `Project=${this.findInvalidProject( + error.detail, + newApiToken.projects, + )} does not exist`; } else if (error.constraint === 'api_tokens_environment_fkey') { message = `Environment=${newApiToken.environment} does not exist`; } @@ -142,9 +160,23 @@ export class ApiTokenService { } } - private generateSecretKey({ project, environment }) { + private findInvalidProject(errorDetails, projects) { + if (!errorDetails) { + return 'invalid'; + } + let invalidProject = projects.find((project) => { + return errorDetails.includes(`=(${project})`); + }); + return invalidProject || 'invalid'; + } + + private generateSecretKey({ projects, environment }) { const randomStr = crypto.randomBytes(28).toString('hex'); - return `${project}:${environment}.${randomStr}`; + if (projects.length > 1) { + return `[]:${environment}.${randomStr}`; + } else { + return `${projects[0]}:${environment}.${randomStr}`; + } } destroy(): void { diff --git a/src/lib/types/api-user.ts b/src/lib/types/api-user.ts index 79567f80a7..8514edd6d0 100644 --- a/src/lib/types/api-user.ts +++ b/src/lib/types/api-user.ts @@ -4,7 +4,8 @@ import { CLIENT } from './permissions'; interface IApiUserData { username: string; permissions?: string[]; - project: string; + projects?: string[]; + project?: string; environment: string; type: ApiTokenType; } @@ -16,7 +17,7 @@ export default class ApiUser { readonly permissions: string[]; - readonly project: string; + readonly projects: string[]; readonly environment: string; @@ -25,6 +26,7 @@ export default class ApiUser { constructor({ username, permissions = [CLIENT], + projects, project, environment, type, @@ -34,8 +36,12 @@ export default class ApiUser { } this.username = username; this.permissions = permissions; - this.project = project; this.environment = environment; this.type = type; + if (projects && projects.length > 0) { + this.projects = projects; + } else { + this.projects = [project]; + } } } diff --git a/src/lib/types/models/api-token.ts b/src/lib/types/models/api-token.ts index 486ee7de9a..b31948248e 100644 --- a/src/lib/types/models/api-token.ts +++ b/src/lib/types/models/api-token.ts @@ -8,12 +8,22 @@ export enum ApiTokenType { ADMIN = 'admin', } +export interface ILegacyApiTokenCreate { + secret: string; + username: string; + type: ApiTokenType; + environment: string; + project?: string; + projects?: string[]; + expiresAt?: Date; +} + export interface IApiTokenCreate { secret: string; username: string; type: ApiTokenType; environment: string; - project: string; + projects: string[]; expiresAt?: Date; } @@ -24,12 +34,58 @@ export interface IApiToken extends IApiTokenCreate { project: string; } +export const isAllProjects = (projects: string[]): boolean => { + return projects && projects.length === 1 && projects[0] === ALL; +}; + +export const mapLegacyProjects = ( + project?: string, + projects?: string[], +): string[] => { + let cleanedProjects; + if (project) { + cleanedProjects = [project]; + } else if (projects) { + cleanedProjects = projects; + if (cleanedProjects.includes('*')) { + cleanedProjects = ['*']; + } + } else { + throw new BadDataError( + 'API tokens must either contain a project or projects field', + ); + } + return cleanedProjects; +}; + +export const mapLegacyToken = ( + token: Omit, +): Omit => { + const cleanedProjects = mapLegacyProjects(token.project, token.projects); + return { + username: token.username, + type: token.type, + environment: token.environment, + projects: cleanedProjects, + expiresAt: token.expiresAt, + }; +}; + +export const mapLegacyTokenWithSecret = ( + token: ILegacyApiTokenCreate, +): IApiTokenCreate => { + return { + ...mapLegacyToken(token), + secret: token.secret, + }; +}; + export const validateApiToken = ({ type, - project, + projects, environment, }: Omit): void => { - if (type === ApiTokenType.ADMIN && project !== ALL) { + if (type === ApiTokenType.ADMIN && !isAllProjects(projects)) { throw new BadDataError( 'Admin token cannot be scoped to single project', ); diff --git a/src/lib/types/option.ts b/src/lib/types/option.ts index f0d6509fc4..c90c76ba8f 100644 --- a/src/lib/types/option.ts +++ b/src/lib/types/option.ts @@ -1,6 +1,6 @@ import EventEmitter from 'events'; import { LogLevel, LogProvider } from '../logger'; -import { IApiTokenCreate } from './models/api-token'; +import { ILegacyApiTokenCreate } from './models/api-token'; import { IExperimentalOptions } from '../experimental'; export type EventHook = (eventName: string, data: object) => void; @@ -55,7 +55,7 @@ export interface IAuthOption { type: IAuthType; customAuthHandler?: Function; createAdminUser: boolean; - initApiTokens: IApiTokenCreate[]; + initApiTokens: ILegacyApiTokenCreate[]; } export interface IImportOption { diff --git a/src/migrations/20220331085057-add-api-link-table.js b/src/migrations/20220331085057-add-api-link-table.js new file mode 100644 index 0000000000..7523e0c1cb --- /dev/null +++ b/src/migrations/20220331085057-add-api-link-table.js @@ -0,0 +1,41 @@ +'use strict'; + +exports.up = function (db, cb) { + db.runSql( + ` + CREATE TABLE IF NOT EXISTS api_token_project + ( + secret text NOT NULL, + project text NOT NULL, + FOREIGN KEY (secret) REFERENCES api_tokens (secret) ON DELETE CASCADE, + FOREIGN KEY (project) REFERENCES projects(id) ON DELETE CASCADE + ); + + INSERT INTO api_token_project SELECT secret, project FROM api_tokens WHERE project IS NOT NULL; + + ALTER TABLE api_tokens DROP COLUMN "project"; + `, + cb, + ); +}; + +//This is a lossy down migration, tokens with multiple projects are discarded +exports.down = function (db, cb) { + db.runSql( + ` + ALTER TABLE api_tokens ADD COLUMN project VARCHAR REFERENCES PROJECTS(id) ON DELETE CASCADE; + DELETE FROM api_tokens WHERE secret LIKE '[]%'; + + UPDATE api_tokens + SET project = subquery.project + FROM( + SELECT token.secret, link.project FROM api_tokens AS token LEFT JOIN api_token_project AS link ON + token.secret = link.secret + ) AS subquery + WHERE api_tokens.project = subquery.project; + + DROP TABLE api_token_project; +`, + 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 c01a5d1c4b..c71c3fb185 100644 --- a/src/test/e2e/api/admin/api-token.e2e.test.ts +++ b/src/test/e2e/api/admin/api-token.e2e.test.ts @@ -194,7 +194,7 @@ test('creates new client token: project & environment defaults to "*"', async () expect(res.body.type).toBe('client'); expect(res.body.secret.length > 16).toBe(true); expect(res.body.environment).toBe(DEFAULT_ENV); - expect(res.body.project).toBe(ALL); + expect(res.body.projects[0]).toBe(ALL); }); }); @@ -213,7 +213,7 @@ test('creates new client token with project & environment set', async () => { expect(res.body.type).toBe('client'); expect(res.body.secret.length > 16).toBe(true); expect(res.body.environment).toBe(DEFAULT_ENV); - expect(res.body.project).toBe('default'); + expect(res.body.projects[0]).toBe('default'); }); }); 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 55cb140a8e..44aa4b947f 100644 --- a/src/test/e2e/services/api-token-service.e2e.test.ts +++ b/src/test/e2e/services/api-token-service.e2e.test.ts @@ -5,10 +5,14 @@ import { createTestConfig } from '../../config/test-config'; import { ApiTokenType, IApiToken } from '../../../lib/types/models/api-token'; import { DEFAULT_ENV } from '../../../lib/util/constants'; import { addDays, subDays } from 'date-fns'; +import ProjectService from '../../../lib/services/project-service'; +import FeatureToggleService from '../../../lib/services/feature-toggle-service'; +import { AccessService } from '../../../lib/services/access-service'; let db; let stores; let apiTokenService: ApiTokenService; +let projectService: ProjectService; beforeAll(async () => { const config = createTestConfig({ @@ -16,7 +20,27 @@ beforeAll(async () => { }); db = await dbInit('api_token_service_serial', getLogger); stores = db.stores; - // projectStore = stores.projectStore; + const accessService = new AccessService(stores, config); + const featureToggleService = new FeatureToggleService(stores, config); + const project = { + id: 'test-project', + name: 'Test Project', + description: 'Fancy', + }; + const user = await stores.userStore.insert({ + name: 'Some Name', + email: 'test@getunleash.io', + }); + + projectService = new ProjectService( + stores, + config, + accessService, + featureToggleService, + ); + + await projectService.createProject(project, user); + apiTokenService = new ApiTokenService(stores, config); }); @@ -128,3 +152,75 @@ test('should only return valid tokens', async () => { expect(tokens.length).toBe(1); expect(activeToken.secret).toBe(tokens[0].secret); }); + +test('should create client token with project list', async () => { + const token = await apiTokenService.createApiToken({ + username: 'default-client', + type: ApiTokenType.CLIENT, + projects: ['default', 'test-project'], + environment: DEFAULT_ENV, + }); + + expect(token.secret.slice(0, 2)).toEqual('[]'); + expect(token.projects).toStrictEqual(['default', 'test-project']); +}); + +test('should strip all other projects if ALL_PROJECTS is present', async () => { + const token = await apiTokenService.createApiToken({ + username: 'default-client', + type: ApiTokenType.CLIENT, + projects: ['*', 'default'], + environment: DEFAULT_ENV, + }); + + expect(token.projects).toStrictEqual(['*']); +}); + +test('should return user with multiple projects', async () => { + const now = Date.now(); + const tomorrow = addDays(now, 1); + + await apiTokenService.createApiToken({ + username: 'default-valid', + type: ApiTokenType.CLIENT, + expiresAt: tomorrow, + projects: ['test-project', 'default'], + environment: DEFAULT_ENV, + }); + + await apiTokenService.createApiToken({ + username: 'default-also-valid', + type: ApiTokenType.CLIENT, + expiresAt: tomorrow, + projects: ['test-project'], + environment: DEFAULT_ENV, + }); + + const tokens = await apiTokenService.getAllActiveTokens(); + const multiProjectUser = await apiTokenService.getUserForToken( + tokens[0].secret, + ); + const singleProjectUser = await apiTokenService.getUserForToken( + tokens[1].secret, + ); + + expect(multiProjectUser.projects).toStrictEqual([ + 'test-project', + 'default', + ]); + expect(singleProjectUser.projects).toStrictEqual(['test-project']); +}); + +test('should not partially create token if projects are invalid', async () => { + try { + await apiTokenService.createApiTokenWithProjects({ + username: 'default-client', + type: ApiTokenType.CLIENT, + projects: ['non-existent-project'], + environment: DEFAULT_ENV, + }); + } catch (e) {} + const allTokens = await apiTokenService.getAllTokens(); + + expect(allTokens.length).toBe(0); +}); diff --git a/src/test/fixtures/fake-api-token-store.ts b/src/test/fixtures/fake-api-token-store.ts index d4fc077e41..d1d24274ad 100644 --- a/src/test/fixtures/fake-api-token-store.ts +++ b/src/test/fixtures/fake-api-token-store.ts @@ -50,6 +50,7 @@ export default class FakeApiTokenStore async insert(newToken: IApiTokenCreate): Promise { const apiToken = { createdAt: new Date(), + project: newToken.projects?.join(',') || '*', ...newToken, }; this.tokens.push(apiToken); diff --git a/src/test/fixtures/fake-feature-strategies-store.ts b/src/test/fixtures/fake-feature-strategies-store.ts index a8d8060be2..5dad977a64 100644 --- a/src/test/fixtures/fake-feature-strategies-store.ts +++ b/src/test/fixtures/fake-feature-strategies-store.ts @@ -148,13 +148,17 @@ export default class FakeFeatureStrategiesStore if (featureQuery.project) { return ( toggle.name.startsWith(featureQuery.namePrefix) && - featureQuery.project.includes(toggle.project) + featureQuery.project.some((project) => + project.includes(toggle.project), + ) ); } return toggle.name.startsWith(featureQuery.namePrefix); } if (featureQuery.project) { - return featureQuery.project.includes(toggle.project); + return featureQuery.project.some((project) => + project.includes(toggle.project), + ); } return toggle.archived === archived; }); diff --git a/src/test/fixtures/fake-feature-toggle-client-store.ts b/src/test/fixtures/fake-feature-toggle-client-store.ts index 823b754ce3..aa87fcf5f1 100644 --- a/src/test/fixtures/fake-feature-toggle-client-store.ts +++ b/src/test/fixtures/fake-feature-toggle-client-store.ts @@ -19,13 +19,17 @@ export default class FakeFeatureToggleClientStore if (featureQuery.project) { return ( toggle.name.startsWith(featureQuery.namePrefix) && - featureQuery.project.includes(toggle.project) + featureQuery.project.some((project) => + project.includes(toggle.project), + ) ); } return toggle.name.startsWith(featureQuery.namePrefix); } if (featureQuery.project) { - return featureQuery.project.includes(toggle.project); + return featureQuery.project.some((project) => + project.includes(toggle.project), + ); } return toggle.archived === archived; });