From ee8991b31935e982f0bb485afb89efdffa6b9232 Mon Sep 17 00:00:00 2001 From: Josh Lewis Date: Tue, 15 Apr 2025 13:37:15 -0600 Subject: [PATCH 1/2] feat: Add disableLogging option to getManagementToken --- src/keys/get-management-token.spec.ts | 64 +++++++++++++++++++++++++++ src/keys/get-management-token.ts | 31 ++++++------- 2 files changed, 80 insertions(+), 15 deletions(-) diff --git a/src/keys/get-management-token.spec.ts b/src/keys/get-management-token.spec.ts index d0fd9b6d..7c25436d 100644 --- a/src/keys/get-management-token.spec.ts +++ b/src/keys/get-management-token.spec.ts @@ -14,6 +14,12 @@ import { Logger } from '../utils' import { sign } from 'jsonwebtoken' import { LRUCache } from 'lru-cache' +// Mock the debug logger to spy on it +const mockDebugInstance = { + extend: sinon.stub().returns(sinon.spy()), +} +sinon.stub(require('debug'), 'default').returns(mockDebugInstance) + const PRIVATE_KEY = fs.readFileSync(path.join(__dirname, '..', '..', 'keys', 'key.pem'), 'utf-8') const APP_ID = 'app_id' const SPACE_ID = 'space_id' @@ -29,6 +35,64 @@ const noop = () => {} const defaultCache: LRUCache = new LRUCache({ max: 10 }) describe('getManagementToken', () => { + let loggerSpy: sinon.SinonSpy + + beforeEach(() => { + // Reset the spy before each test + loggerSpy = mockDebugInstance.extend() as sinon.SinonSpy + loggerSpy.resetHistory() + // Reset cache as well + defaultCache.clear() + }) + + it('fetches a token and logs by default', async () => { + const mockToken = 'token' + // Use the real logger creator, but we spy on the debug instance it creates + const { createLogger } = await import('../utils/logger') + const logger = createLogger({ filename: __filename }) + const post = sinon.stub() + post.resolves({ statusCode: 201, body: JSON.stringify({ token: mockToken }) }) + const httpClient = { post } as unknown as HttpClient + const getManagementTokenFn = createGetManagementToken(logger, httpClient, defaultCache) + + const result = await getManagementTokenFn(PRIVATE_KEY, DEFAULT_OPTIONS) // disableLogging is default false + + assert.deepStrictEqual(result, mockToken) + assert( + post.calledWith( + `spaces/${SPACE_ID}/environments/${ENVIRONMENT_ID}/app_installations/${APP_ID}/access_tokens`, + sinon.match({ headers: { Authorization: sinon.match.string } }), + ), + ) + // Assert that the logger spy was called + assert(loggerSpy.called, 'Logger should have been called by default') + }) + + it('does not log when disableLogging is true', async () => { + const mockToken = 'token-no-log' + const { createLogger } = await import('../utils/logger') + const logger = createLogger({ filename: __filename }) + const post = sinon.stub() + post.resolves({ statusCode: 201, body: JSON.stringify({ token: mockToken }) }) + const httpClient = { post } as unknown as HttpClient + const getManagementTokenFn = createGetManagementToken(logger, httpClient, defaultCache) + + const result = await getManagementTokenFn(PRIVATE_KEY, { + ...DEFAULT_OPTIONS, + disableLogging: true, + }) + + assert.deepStrictEqual(result, mockToken) + assert( + post.calledWith( + `spaces/${SPACE_ID}/environments/${ENVIRONMENT_ID}/app_installations/${APP_ID}/access_tokens`, + sinon.match({ headers: { Authorization: sinon.match.string } }), + ), + ) + // Assert that the logger spy was NOT called + assert(!loggerSpy.called, 'Logger should not have been called when disableLogging is true') + }) + it('fetches a token', async () => { const mockToken = 'token' const logger = noop as unknown as Logger diff --git a/src/keys/get-management-token.ts b/src/keys/get-management-token.ts index 1d62e876..7173079a 100644 --- a/src/keys/get-management-token.ts +++ b/src/keys/get-management-token.ts @@ -18,6 +18,7 @@ export interface GetManagementTokenOptions { keyId?: string reuseToken?: boolean host?: string + disableLogging?: boolean } let defaultCache: LRUCache | undefined @@ -29,15 +30,15 @@ let defaultCache: LRUCache | undefined const generateOneTimeToken = ( privateKey: string, { appId, keyId }: { appId: string; keyId?: string }, - { log }: { log: Logger }, + { log, disableLogging }: { log: Logger; disableLogging: boolean }, ): string => { - log('Signing a JWT token with private key') + if (!disableLogging) log('Signing a JWT token with private key') try { const baseSignOptions: SignOptions = { algorithm: 'RS256', issuer: appId, expiresIn: '10m' } const signOptions: SignOptions = keyId ? { ...baseSignOptions, keyid: keyId } : baseSignOptions const token = sign({}, privateKey, signOptions) - log('Successfully signed token') + if (!disableLogging) log('Successfully signed token') return token } catch (e) { log('Unable to sign token') @@ -52,11 +53,11 @@ const getTokenFromOneTimeToken = async ( spaceId, environmentId, }: { appInstallationId: string; spaceId: string; environmentId: string }, - { log, http }: { log: Logger; http: HttpClient }, + { log, http, disableLogging }: { log: Logger; http: HttpClient; disableLogging: boolean }, ): Promise => { const validateStatusCode = createValidateStatusCode([201]) - log(`Requesting CMA Token with given App Token`) + if (!disableLogging) log(`Requesting CMA Token with given App Token`) const response = await http.post( `spaces/${spaceId}/environments/${environmentId}/app_installations/${appInstallationId}/access_tokens`, @@ -70,9 +71,10 @@ const getTokenFromOneTimeToken = async ( }, ) - log( - `Successfully retrieved CMA Token for app ${appInstallationId} in space ${spaceId} and environment ${environmentId}`, - ) + if (!disableLogging) + log( + `Successfully retrieved CMA Token for app ${appInstallationId} in space ${spaceId} and environment ${environmentId}`, + ) return JSON.parse(response.body).token } @@ -91,13 +93,12 @@ export const createGetManagementToken = ( throw new ReferenceError('Invalid privateKey: expected a string representing a private key') } - if (opts.reuseToken === undefined) { - opts.reuseToken = true - } + const reuseToken = opts.reuseToken ?? true + const disableLogging = opts.disableLogging ?? false const cacheKey = opts.appInstallationId + opts.spaceId + opts.environmentId + privateKey.slice(32, 132) - if (opts.reuseToken) { + if (reuseToken) { const existing = cache.get(cacheKey) as string if (existing) { return existing as string @@ -107,10 +108,10 @@ export const createGetManagementToken = ( const appToken = generateOneTimeToken( privateKey, { appId: opts.appInstallationId, keyId: opts.keyId }, - { log }, + { log, disableLogging }, ) - const ott = await getTokenFromOneTimeToken(appToken, opts, { log, http }) - if (opts.reuseToken) { + const ott = await getTokenFromOneTimeToken(appToken, opts, { log, http, disableLogging }) + if (reuseToken) { const decoded = decode(ott) if (decoded && typeof decoded === 'object' && decoded.exp) { // Internally expire cached tokens a bit earlier to make sure token isn't expired on arrival From 16f9db07de29efce6517dd5bf6d7f8f5163c4a8d Mon Sep 17 00:00:00 2001 From: Josh Lewis Date: Wed, 16 Apr 2025 11:03:31 -0600 Subject: [PATCH 2/2] refactor: Remove global debug logger mock and implement no-op logger for improved logging control --- src/keys/get-management-token.spec.ts | 56 ++++++++++++++++++++------- src/keys/get-management-token.ts | 30 ++++++++------ 2 files changed, 60 insertions(+), 26 deletions(-) diff --git a/src/keys/get-management-token.spec.ts b/src/keys/get-management-token.spec.ts index 7c25436d..f87e7b5c 100644 --- a/src/keys/get-management-token.spec.ts +++ b/src/keys/get-management-token.spec.ts @@ -14,11 +14,8 @@ import { Logger } from '../utils' import { sign } from 'jsonwebtoken' import { LRUCache } from 'lru-cache' -// Mock the debug logger to spy on it -const mockDebugInstance = { - extend: sinon.stub().returns(sinon.spy()), -} -sinon.stub(require('debug'), 'default').returns(mockDebugInstance) +// No longer need to mock debug globally +// sinon.stub(require('debug'), 'default').returns(mockDebugInstance) const PRIVATE_KEY = fs.readFileSync(path.join(__dirname, '..', '..', 'keys', 'key.pem'), 'utf-8') const APP_ID = 'app_id' @@ -36,24 +33,44 @@ const defaultCache: LRUCache = new LRUCache({ max: 10 }) describe('getManagementToken', () => { let loggerSpy: sinon.SinonSpy + let createLoggerStub: sinon.SinonStub beforeEach(() => { // Reset the spy before each test - loggerSpy = mockDebugInstance.extend() as sinon.SinonSpy - loggerSpy.resetHistory() + loggerSpy = sinon.spy() + // Reset stubs if they exist + if (createLoggerStub) { + createLoggerStub.restore() + } // Reset cache as well defaultCache.clear() }) + afterEach(() => { + // Ensure stubs are restored after each test + if (createLoggerStub) { + createLoggerStub.restore() + } + }) + it('fetches a token and logs by default', async () => { const mockToken = 'token' - // Use the real logger creator, but we spy on the debug instance it creates - const { createLogger } = await import('../utils/logger') - const logger = createLogger({ filename: __filename }) + // Stub createLogger to return our spy directly, as debug instances are functions + const loggerUtils = await import('../utils/logger') + // The logger instance itself is the function to call for logging + createLoggerStub = sinon + .stub(loggerUtils, 'createLogger') + .returns(loggerSpy as unknown as Logger) + const post = sinon.stub() post.resolves({ statusCode: 201, body: JSON.stringify({ token: mockToken }) }) const httpClient = { post } as unknown as HttpClient - const getManagementTokenFn = createGetManagementToken(logger, httpClient, defaultCache) + // createGetManagementToken will now receive the loggerSpy via the stub + const getManagementTokenFn = createGetManagementToken( + loggerSpy as unknown as Logger, + httpClient, + defaultCache, + ) const result = await getManagementTokenFn(PRIVATE_KEY, DEFAULT_OPTIONS) // disableLogging is default false @@ -70,12 +87,21 @@ describe('getManagementToken', () => { it('does not log when disableLogging is true', async () => { const mockToken = 'token-no-log' - const { createLogger } = await import('../utils/logger') - const logger = createLogger({ filename: __filename }) + // Stub createLogger like before + const loggerUtils = await import('../utils/logger') + createLoggerStub = sinon + .stub(loggerUtils, 'createLogger') + .returns(loggerSpy as unknown as Logger) + const post = sinon.stub() post.resolves({ statusCode: 201, body: JSON.stringify({ token: mockToken }) }) const httpClient = { post } as unknown as HttpClient - const getManagementTokenFn = createGetManagementToken(logger, httpClient, defaultCache) + // createGetManagementToken receives the spy logger + const getManagementTokenFn = createGetManagementToken( + loggerSpy as unknown as Logger, + httpClient, + defaultCache, + ) const result = await getManagementTokenFn(PRIVATE_KEY, { ...DEFAULT_OPTIONS, @@ -90,6 +116,8 @@ describe('getManagementToken', () => { ), ) // Assert that the logger spy was NOT called + // Because disableLogging: true, the internal logic uses noOpLogger, + // so the log method of the passed-in mockLogger (our spy) is never invoked. assert(!loggerSpy.called, 'Logger should not have been called when disableLogging is true') }) diff --git a/src/keys/get-management-token.ts b/src/keys/get-management-token.ts index 7173079a..5b06df14 100644 --- a/src/keys/get-management-token.ts +++ b/src/keys/get-management-token.ts @@ -23,6 +23,10 @@ export interface GetManagementTokenOptions { let defaultCache: LRUCache | undefined +// Create a logger with a specific namespace that is unlikely to be enabled, +// effectively making it a no-op logger while conforming to the Logger type. +const noOpLogger: Logger = createLogger({ filename: __filename + ':noop' }) + /** * Synchronously sign the given privateKey into a JSON Web Token string * @internal @@ -30,15 +34,15 @@ let defaultCache: LRUCache | undefined const generateOneTimeToken = ( privateKey: string, { appId, keyId }: { appId: string; keyId?: string }, - { log, disableLogging }: { log: Logger; disableLogging: boolean }, + { log }: { log: Logger }, ): string => { - if (!disableLogging) log('Signing a JWT token with private key') + log('Signing a JWT token with private key') try { const baseSignOptions: SignOptions = { algorithm: 'RS256', issuer: appId, expiresIn: '10m' } const signOptions: SignOptions = keyId ? { ...baseSignOptions, keyid: keyId } : baseSignOptions const token = sign({}, privateKey, signOptions) - if (!disableLogging) log('Successfully signed token') + log('Successfully signed token') return token } catch (e) { log('Unable to sign token') @@ -53,11 +57,11 @@ const getTokenFromOneTimeToken = async ( spaceId, environmentId, }: { appInstallationId: string; spaceId: string; environmentId: string }, - { log, http, disableLogging }: { log: Logger; http: HttpClient; disableLogging: boolean }, + { log, http }: { log: Logger; http: HttpClient }, ): Promise => { const validateStatusCode = createValidateStatusCode([201]) - if (!disableLogging) log(`Requesting CMA Token with given App Token`) + log(`Requesting CMA Token with given App Token`) const response = await http.post( `spaces/${spaceId}/environments/${environmentId}/app_installations/${appInstallationId}/access_tokens`, @@ -71,10 +75,9 @@ const getTokenFromOneTimeToken = async ( }, ) - if (!disableLogging) - log( - `Successfully retrieved CMA Token for app ${appInstallationId} in space ${spaceId} and environment ${environmentId}`, - ) + log( + `Successfully retrieved CMA Token for app ${appInstallationId} in space ${spaceId} and environment ${environmentId}`, + ) return JSON.parse(response.body).token } @@ -95,6 +98,7 @@ export const createGetManagementToken = ( const reuseToken = opts.reuseToken ?? true const disableLogging = opts.disableLogging ?? false + const effectiveLog = disableLogging ? noOpLogger : log const cacheKey = opts.appInstallationId + opts.spaceId + opts.environmentId + privateKey.slice(32, 132) @@ -108,9 +112,9 @@ export const createGetManagementToken = ( const appToken = generateOneTimeToken( privateKey, { appId: opts.appInstallationId, keyId: opts.keyId }, - { log, disableLogging }, + { log: effectiveLog }, ) - const ott = await getTokenFromOneTimeToken(appToken, opts, { log, http, disableLogging }) + const ott = await getTokenFromOneTimeToken(appToken, opts, { log: effectiveLog, http }) if (reuseToken) { const decoded = decode(ott) if (decoded && typeof decoded === 'object' && decoded.exp) { @@ -153,9 +157,11 @@ export const getManagementToken = (privateKey: string, opts: GetManagementTokenO defaultCache = new LRUCache({ max: 10 }) } const httpClientOpts = typeof opts.host !== 'undefined' ? { prefixUrl: opts.host } : {} + const disableLogging = opts.disableLogging ?? false + const effectiveLog = disableLogging ? noOpLogger : createLogger({ filename: __filename }) return createGetManagementToken( - createLogger({ filename: __filename }), + effectiveLog, createHttpClient(httpClientOpts), defaultCache!, )(privateKey, opts)