From f56dfad7b35817d0a7a5af523a7efb6d741cccaa Mon Sep 17 00:00:00 2001 From: Justin Dalrymple <jdalrymple@users.noreply.github.com> Date: Mon, 26 Jun 2023 13:27:50 -0400 Subject: [PATCH] Support defered token retrieval (#3317) --- .../unit/resources/ApplicationAppearance.ts | 2 +- packages/core/test/unit/resources/General.ts | 4 +- .../core/test/unit/resources/GroupHooks.ts | 1 - packages/core/test/unit/resources/Issues.ts | 2 +- .../unit/resources/MergeRequestApprovals.ts | 2 +- .../core/test/unit/resources/ProjectWikis.ts | 5 +- packages/core/test/unit/resources/Runners.ts | 2 +- .../unit/templates/ResourceDiscussions.ts | 1 - packages/requester-utils/src/BaseResource.ts | 28 +++++-- .../requester-utils/src/RequesterUtils.ts | 12 ++- .../requester-utils/test/unit/BaseResource.ts | 73 ++++++++++++++----- .../test/unit/RequesterUtils.ts | 29 +++++++- .../rest/test/integration/nodejs/General.ts | 36 +++++++++ packages/rest/test/unit/Requester.ts | 3 + scripts/docker-compose.yml | 2 +- 15 files changed, 160 insertions(+), 42 deletions(-) create mode 100644 packages/rest/test/integration/nodejs/General.ts diff --git a/packages/core/test/unit/resources/ApplicationAppearance.ts b/packages/core/test/unit/resources/ApplicationAppearance.ts index 33046188b..62b9ded40 100644 --- a/packages/core/test/unit/resources/ApplicationAppearance.ts +++ b/packages/core/test/unit/resources/ApplicationAppearance.ts @@ -27,7 +27,7 @@ describe('ApplicationAppearance.edit', () => { it('should request PUT /application/appearence without arguments', async () => { await service.edit(); - expect(RequestHelper.put()).toHaveBeenCalledWith(service, 'application/appearence', undefined); + expect(RequestHelper.put()).toHaveBeenCalledWith(service, 'application/appearence', {}); }); it('should request PUT /application/appearence with a logo property', async () => { diff --git a/packages/core/test/unit/resources/General.ts b/packages/core/test/unit/resources/General.ts index d3f04d11d..2817cb226 100644 --- a/packages/core/test/unit/resources/General.ts +++ b/packages/core/test/unit/resources/General.ts @@ -11,7 +11,9 @@ describe('Instantiating services', () => { expect(service.constructor.name).toBe(k); expect(service.url).toBeDefined(); expect(service.rejectUnauthorized).toBeTruthy(); - expect(service.headers).toMatchObject({ 'private-token': 'abcdefg' }); + expect(service.authHeaders).toMatchObject({ + 'private-token': expect.any(Function), + }); expect(service.queryTimeout).toBe(300000); }); }); diff --git a/packages/core/test/unit/resources/GroupHooks.ts b/packages/core/test/unit/resources/GroupHooks.ts index bb87cc30f..fb3160412 100644 --- a/packages/core/test/unit/resources/GroupHooks.ts +++ b/packages/core/test/unit/resources/GroupHooks.ts @@ -20,7 +20,6 @@ describe('Instantiating GroupHooks service', () => { expect(service).toBeInstanceOf(GroupHooks); expect(service.url).toBeDefined(); expect(service.rejectUnauthorized).toBeTruthy(); - expect(service.headers).toMatchObject({ 'private-token': 'abcdefg' }); }); it('should call /groups prefix', () => { diff --git a/packages/core/test/unit/resources/Issues.ts b/packages/core/test/unit/resources/Issues.ts index 33e095335..3e42ab429 100644 --- a/packages/core/test/unit/resources/Issues.ts +++ b/packages/core/test/unit/resources/Issues.ts @@ -136,7 +136,7 @@ describe('Issues.show', () => { it('should request GET /projects/:id/issues/:id', async () => { await service.show(1, { projectId: 2 }); - expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'projects/1/issues/2', undefined); + expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'projects/2/issues/1', {}); }); }); diff --git a/packages/core/test/unit/resources/MergeRequestApprovals.ts b/packages/core/test/unit/resources/MergeRequestApprovals.ts index 2cb180cf9..a2f0c6961 100644 --- a/packages/core/test/unit/resources/MergeRequestApprovals.ts +++ b/packages/core/test/unit/resources/MergeRequestApprovals.ts @@ -45,7 +45,7 @@ describe('MergeRequestApprovals.editConfiguration', () => { it('should request POST /projects/:id/approvals without options', async () => { await service.editConfiguration(3); - expect(RequestHelper.post()).toHaveBeenCalledWith(service, 'projects/3/approvals', {}); + expect(RequestHelper.post()).toHaveBeenCalledWith(service, 'projects/3/approvals', undefined); }); it('should request POST /projects/:id/approvals', async () => { diff --git a/packages/core/test/unit/resources/ProjectWikis.ts b/packages/core/test/unit/resources/ProjectWikis.ts index 4b9485b03..40f571661 100644 --- a/packages/core/test/unit/resources/ProjectWikis.ts +++ b/packages/core/test/unit/resources/ProjectWikis.ts @@ -33,7 +33,10 @@ describe('ProjectWikis.create', () => { it('should request POST /projects/:id/wikis', async () => { await service.create(1, 'content', 'title'); - expect(RequestHelper.post()).toHaveBeenCalledWith(service, '1/wikis', undefined); + expect(RequestHelper.post()).toHaveBeenCalledWith(service, '1/wikis', { + content: 'content', + title: 'title', + }); }); }); diff --git a/packages/core/test/unit/resources/Runners.ts b/packages/core/test/unit/resources/Runners.ts index 07198f44d..b90e74ba7 100644 --- a/packages/core/test/unit/resources/Runners.ts +++ b/packages/core/test/unit/resources/Runners.ts @@ -79,7 +79,7 @@ describe('Runners.remove', () => { it('should request DEL /runners/:id', async () => { await service.remove({ runnerId: 2 }); - expect(RequestHelper.del()).toHaveBeenCalledWith(service, 'runners/2', undefined); + expect(RequestHelper.del()).toHaveBeenCalledWith(service, 'runners/2', {}); }); it('should request DEL /runners with token', async () => { diff --git a/packages/core/test/unit/templates/ResourceDiscussions.ts b/packages/core/test/unit/templates/ResourceDiscussions.ts index 877ef1b0d..92d94a794 100644 --- a/packages/core/test/unit/templates/ResourceDiscussions.ts +++ b/packages/core/test/unit/templates/ResourceDiscussions.ts @@ -20,7 +20,6 @@ describe('Instantiating ResourceDiscussions service', () => { expect(service).toBeInstanceOf(ResourceDiscussions); expect(service.url).toBeDefined(); expect(service.rejectUnauthorized).toBeTruthy(); - expect(service.headers).toMatchObject({ 'private-token': 'abcdefg' }); }); }); diff --git a/packages/requester-utils/src/BaseResource.ts b/packages/requester-utils/src/BaseResource.ts index 9869b0ad6..1b99d5a9a 100644 --- a/packages/requester-utils/src/BaseResource.ts +++ b/packages/requester-utils/src/BaseResource.ts @@ -13,16 +13,18 @@ export interface RootResourceOptions<C> { profileMode?: 'execution' | 'memory'; } +export type GitlabToken = string | (() => Promise<string>); + export interface BaseRequestOptionsWithOAuthToken<C> extends RootResourceOptions<C> { - oauthToken: string; + oauthToken: GitlabToken; } export interface BaseRequestOptionsWithAccessToken<C> extends RootResourceOptions<C> { - token: string; + token: GitlabToken; } export interface BaseRequestOptionsWithJobToken<C> extends RootResourceOptions<C> { - jobToken: string; + jobToken: GitlabToken; } export type BaseResourceOptions<C> = @@ -30,6 +32,10 @@ export type BaseResourceOptions<C> = | BaseRequestOptionsWithAccessToken<C> | BaseRequestOptionsWithJobToken<C>; +function getDynamicToken(tokenArgument: (() => Promise<string>) | string): Promise<string> { + return tokenArgument instanceof Function ? tokenArgument() : Promise.resolve(tokenArgument); +} + export class BaseResource<C extends boolean = false> { public readonly url: string; @@ -39,6 +45,8 @@ export class BaseResource<C extends boolean = false> { public readonly headers: { [header: string]: string }; + public readonly authHeaders: { [authHeader: string]: () => Promise<string> }; + public readonly camelize: C | undefined; public readonly rejectUnauthorized: boolean; @@ -59,14 +67,22 @@ export class BaseResource<C extends boolean = false> { this.url = [host, 'api', 'v4', prefixUrl].join('/'); this.headers = {}; + this.authHeaders = {}; this.rejectUnauthorized = rejectUnauthorized; this.camelize = camelize; this.queryTimeout = queryTimeout; // Handle auth tokens - if ('oauthToken' in tokens) this.headers.authorization = `Bearer ${tokens.oauthToken}`; - else if ('jobToken' in tokens) this.headers['job-token'] = tokens.jobToken; - else if ('token' in tokens) this.headers['private-token'] = tokens.token; + if ('oauthToken' in tokens) + this.authHeaders.authorization = async () => { + const token = await getDynamicToken(tokens.oauthToken); + + return `Bearer ${token}`; + }; + else if ('jobToken' in tokens) + this.authHeaders['job-token'] = async () => getDynamicToken(tokens.jobToken); + else if ('token' in tokens) + this.authHeaders['private-token'] = async () => getDynamicToken(tokens.token); else { throw new ReferenceError('A token, oauthToken or jobToken must be passed'); } diff --git a/packages/requester-utils/src/RequesterUtils.ts b/packages/requester-utils/src/RequesterUtils.ts index 90c23749f..7bf0480bd 100644 --- a/packages/requester-utils/src/RequesterUtils.ts +++ b/packages/requester-utils/src/RequesterUtils.ts @@ -25,6 +25,7 @@ export interface Constructable<T = any> { export type DefaultResourceOptions = { headers: { [header: string]: string }; + authHeaders: { [authHeader: string]: () => Promise<string> }; url: string; rejectUnauthorized: boolean; }; @@ -89,7 +90,7 @@ function isFormData(object: FormData | Record<string, unknown>) { return typeof object === 'object' && object.constructor.name === 'FormData'; } -export function defaultOptionsHandler( +export async function defaultOptionsHandler( resourceOptions: DefaultResourceOptions, { body, @@ -100,7 +101,7 @@ export function defaultOptionsHandler( method = 'get', }: DefaultRequestOptions = {}, ): Promise<RequestOptions> { - const { headers: preconfiguredHeaders, url } = resourceOptions; + const { headers: preconfiguredHeaders, authHeaders, url } = resourceOptions; const headers = { ...preconfiguredHeaders }; const defaultOptions: RequestOptions = { headers, @@ -118,10 +119,15 @@ export function defaultOptionsHandler( defaultOptions.body = body as FormData; } else { defaultOptions.body = JSON.stringify(decamelizeKeys(body)); - headers['content-type'] = 'application/json'; + defaultOptions.headers['content-type'] = 'application/json'; } } + // Append dynamic auth header + const [authHeaderKey, authHeaderFn] = Object.entries(authHeaders)[0]; + + defaultOptions.headers[authHeaderKey] = await authHeaderFn(); + // Format query parameters const q = formatQuery(searchParams); diff --git a/packages/requester-utils/test/unit/BaseResource.ts b/packages/requester-utils/test/unit/BaseResource.ts index 14ba70b53..dc411e7a0 100644 --- a/packages/requester-utils/test/unit/BaseResource.ts +++ b/packages/requester-utils/test/unit/BaseResource.ts @@ -8,17 +8,6 @@ describe('Creation of BaseResource instance', () => { expect(service.url).toBe('https://gitlab.com/api/v4/'); }); - it('should use the Oauth Token when a given both a Private Token and a Oauth Token', () => { - const service = new BaseResource({ - requesterFn: jest.fn(), - token: 'test', - oauthToken: '1234', - }); - - expect(service.headers['private-token']).toBeUndefined(); - expect(service.headers.authorization).toBe('Bearer 1234'); - }); - it('should append api and version number to host when using a custom host url', () => { const service = new BaseResource({ requesterFn: jest.fn(), @@ -35,34 +24,78 @@ describe('Creation of BaseResource instance', () => { expect(service.camelize).toBe(true); }); - it('should add Oauth token to authorization header as a bearer token', () => { + it('should accept a string oauthToken', async () => { const service = new BaseResource({ requesterFn: jest.fn(), - host: 'https://testing.com', oauthToken: '1234', }); - expect(service.headers.authorization).toBe('Bearer 1234'); + expect(service.authHeaders.authorization).toBeFunction(); + + await expect(service.authHeaders.authorization()).resolves.toBe('Bearer 1234'); }); - it('should add Private token to private-token header', () => { + it('should accept a function oauthToken that returns a promise<string>', async () => { + const service = new BaseResource({ + requesterFn: jest.fn(), + oauthToken: () => Promise.resolve('1234'), + }); + + expect(service.authHeaders.authorization).toBeFunction(); + + await expect(service.authHeaders.authorization()).resolves.toBe('Bearer 1234'); + }); + + it('should use the Oauth Token when a given both a Private Token and a Oauth Token', async () => { + const service = new BaseResource({ + requesterFn: jest.fn(), + token: 'test', + oauthToken: () => Promise.resolve('1234'), + }); + + expect(Object.keys(service.authHeaders).length).toBe(1); + expect(service.authHeaders.authorization).toBeFunction(); + await expect(service.authHeaders.authorization()).resolves.toBe('Bearer 1234'); + }); + + it('should accept a string token (private-token)', async () => { const service = new BaseResource({ requesterFn: jest.fn(), - host: 'https://testing.com', token: '1234', }); - expect(service.headers['private-token']).toBe('1234'); + expect(service.authHeaders['private-token']).toBeFunction(); + await expect(service.authHeaders['private-token']()).resolves.toBe('1234'); }); - it('should add Job token to job-token header', () => { + it('should accept a function token (private-token) that returns a promise<string>', async () => { + const service = new BaseResource({ + requesterFn: jest.fn(), + token: () => Promise.resolve('1234'), + }); + + expect(service.authHeaders['private-token']).toBeFunction(); + await expect(service.authHeaders['private-token']()).resolves.toBe('1234'); + }); + + it('should accept a string jobToken (job-token)', async () => { const service = new BaseResource({ requesterFn: jest.fn(), - host: 'https://testing.com', jobToken: '1234', }); - expect(service.headers['job-token']).toBe('1234'); + expect(service.authHeaders['job-token']).toBeFunction(); + await expect(service.authHeaders['job-token']()).resolves.toBe('1234'); + }); + + it('should accept a function jobToken (job-token) that returns a promise<string>', async () => { + const service = new BaseResource({ + requesterFn: jest.fn(), + jobToken: () => Promise.resolve('1234'), + }); + + expect(service.authHeaders['job-token']).toBeFunction(); + await expect(service.authHeaders['job-token']()).resolves.toBe('1234'); }); it('should set the X-Profile-Token header if the profileToken option is given', () => { diff --git a/packages/requester-utils/test/unit/RequesterUtils.ts b/packages/requester-utils/test/unit/RequesterUtils.ts index faca210fd..b9d9cd898 100644 --- a/packages/requester-utils/test/unit/RequesterUtils.ts +++ b/packages/requester-utils/test/unit/RequesterUtils.ts @@ -11,6 +11,9 @@ const methods = ['get', 'put', 'patch', 'delete', 'post']; describe('defaultOptionsHandler', () => { const serviceOptions = { headers: { test: '5' }, + authHeaders: { + token: () => Promise.resolve('1234'), + }, url: 'testurl', rejectUnauthorized: false, }; @@ -43,18 +46,18 @@ describe('defaultOptionsHandler', () => { }); it('should not assign the sudo property if omitted', async () => { - const { headers } = (await defaultOptionsHandler(serviceOptions, { + const { headers } = await defaultOptionsHandler(serviceOptions, { sudo: undefined, method: 'get', - })) as { headers: Record<string, string> }; + }); expect(headers.sudo).toBeUndefined(); }); it('should assign the sudo property if passed', async () => { - const { headers } = (await defaultOptionsHandler(serviceOptions, { + const { headers } = await defaultOptionsHandler(serviceOptions, { sudo: 'testsudo', - })) as { headers: Record<string, string> }; + }); expect(headers.sudo).toBe('testsudo'); }); @@ -88,6 +91,15 @@ describe('defaultOptionsHandler', () => { expect(searchParams).toBe('this_search_term=5'); }); + + it('should append dynamic authentication token headers', async () => { + const { headers } = await defaultOptionsHandler(serviceOptions, { + sudo: undefined, + method: 'get', + }); + + expect(headers.token).toBe('1234'); + }); }); describe('createInstance', () => { @@ -95,6 +107,9 @@ describe('createInstance', () => { const optionsHandler = jest.fn(() => Promise.resolve({} as RequestOptions)); const serviceOptions = { headers: { test: '5' }, + authHeaders: { + token: () => Promise.resolve('1234'), + }, url: 'testurl', rejectUnauthorized: false, }; @@ -130,11 +145,17 @@ describe('createInstance', () => { it('should respect the closure variables', async () => { const serviceOptions1 = { headers: { test: '5' }, + authHeaders: { + token: () => Promise.resolve('1234'), + }, url: 'testurl', rejectUnauthorized: false, }; const serviceOptions2 = { headers: { test: '5' }, + authHeaders: { + token: () => Promise.resolve('1234'), + }, url: 'testurl2', rejectUnauthorized: true, }; diff --git a/packages/rest/test/integration/nodejs/General.ts b/packages/rest/test/integration/nodejs/General.ts new file mode 100644 index 000000000..c338ebdad --- /dev/null +++ b/packages/rest/test/integration/nodejs/General.ts @@ -0,0 +1,36 @@ +import { Projects } from '../../../src'; + +const { GITLAB_PERSONAL_ACCESS_TOKEN = '', GITLAB_URL = '', TEST_ID = Date.now() } = process.env; + +describe('Dynamic Token Resolution', () => { + it('should support a dynamic resolution of a private token', async () => { + const service = new Projects({ + host: GITLAB_URL, + token: () => Promise.resolve(GITLAB_PERSONAL_ACCESS_TOKEN), + }); + + const name = `Project with dynamic private token - NODEJS ${TEST_ID}`; + const p = await service.create({ name }); + + expect(p).toBeObject(); + expect(p.name).toEqual(name); + }); + + it('should support a delayed dynamic resolution of a private token', async () => { + const service = new Projects({ + host: GITLAB_URL, + token: () => + new Promise((res) => { + setTimeout(() => { + res(GITLAB_PERSONAL_ACCESS_TOKEN); + }, 1000); + }), + }); + + const name = `Project with delyed dynamic private token - NODEJS ${TEST_ID}`; + const p = await service.create({ name }); + + expect(p).toBeObject(); + expect(p.name).toEqual(name); + }); +}); diff --git a/packages/rest/test/unit/Requester.ts b/packages/rest/test/unit/Requester.ts index 7d35f3b71..0aed1e7b9 100644 --- a/packages/rest/test/unit/Requester.ts +++ b/packages/rest/test/unit/Requester.ts @@ -285,6 +285,9 @@ describe('defaultRequest', () => { headers: { test: '5' }, url: 'testurl', rejectUnauthorized: true, + authHeaders: { + token: () => Promise.resolve('1234'), + }, }; it('should not assign the agent property if given https url and not rejectUnauthorized', async () => { diff --git a/scripts/docker-compose.yml b/scripts/docker-compose.yml index a6b922339..2e1744cf8 100644 --- a/scripts/docker-compose.yml +++ b/scripts/docker-compose.yml @@ -5,7 +5,7 @@ services: container_name: 'gitlab' environment: GITLAB_URL: 'http://localhost:8080' - PERSONAL_ACCESS_TOKEN: gitbeaker + PERSONAL_ACCESS_TOKEN: superstrongpassword123 GITLAB_OMNIBUS_CONFIG: | gitlab_rails['monitoring_whitelist'] = ['0.0.0.0/0', '172.17.0.1']; GITLAB_ROOT_PASSWORD: gitbeaker