fix: hide authorization header from debug logs (#181)

adobe · Sep 4, 2024 · 1f400b3 · 1f400b3
1 parent a0c5d17
commit 1f400b3
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 18 deletions.
diff --git a/lib/AdobeState.js b/lib/AdobeState.js
@@ -127,6 +127,11 @@ async function handleResponse (response, params) {
   }
 }
 
+/** @private */
+function logDebug (op, url, reqOptions) {
+  logger.debug(`${op} ${JSON.stringify({ url, ...utils.withHiddenFields(reqOptions, ['headers.Authorization']) })}`)
+}
+
 /**
  * @abstract
  * @class AdobeState
@@ -185,7 +190,6 @@ class AdobeState {
   createRequestUrl (containerURLPath = '', queryObject = {}) {
     const urlString = `${this.endpoint}/containers/${this.namespace}${containerURLPath}`
 
-    logger.debug('requestUrl string', urlString)
     const requestUrl = new url.URL(urlString)
     // add the query params
     requestUrl.search = (new url.URLSearchParams(queryObject)).toString()
@@ -263,8 +267,6 @@ class AdobeState {
    * @memberof AdobeState
    */
   async get (key) {
-    logger.debug(`get '${key}'`)
-
     const schema = {
       type: 'object',
       properties: {
@@ -289,8 +291,11 @@ class AdobeState {
         ...this.getAuthorizationHeaders()
       }
     }
-    logger.debug('get', requestOptions)
-    const promise = this.fetchRetry.exponentialBackoff(this.createRequestUrl(`/data/${key}`), requestOptions)
+
+    const url = this.createRequestUrl(`/data/${key}`)
+    logDebug('get', url, requestOptions)
+
+    const promise = this.fetchRetry.exponentialBackoff(url, requestOptions)
     const response = await _wrap(promise, { key })
     if (response.ok) {
       // we only expect string values
@@ -356,10 +361,11 @@ class AdobeState {
       body: value
     }
 
-    logger.debug('put', requestOptions)
+    const url = this.createRequestUrl(`/data/${key}`, queryParams)
 
+    logDebug('put', url, requestOptions)
     const promise = this.fetchRetry.exponentialBackoff(
-      this.createRequestUrl(`/data/${key}`, queryParams),
+      url,
       requestOptions
     )
     await _wrap(promise, { key, value, ...options }, true)
@@ -400,9 +406,10 @@ class AdobeState {
       }))
     }
 
-    logger.debug('delete', requestOptions)
+    const url = this.createRequestUrl(`/data/${key}`)
 
-    const promise = this.fetchRetry.exponentialBackoff(this.createRequestUrl(`/data/${key}`), requestOptions)
+    logDebug('delete', url, requestOptions)
+    const promise = this.fetchRetry.exponentialBackoff(url, requestOptions)
     const response = await _wrap(promise, { key })
     if (response.status === 404) {
       return null
@@ -429,8 +436,6 @@ class AdobeState {
       }
     }
 
-    logger.debug('deleteAll', requestOptions)
-
     const schema = {
       type: 'object',
       properties: {
@@ -447,9 +452,12 @@ class AdobeState {
     }
 
     const queryParams = { matchData: options.match }
+    const url = this.createRequestUrl('', queryParams)
+
+    logDebug('deleteAll', url, requestOptions)
 
     // ! be extra cautious, if the `matchData` param is not specified the whole container will be deleted
-    const promise = this.fetchRetry.exponentialBackoff(this.createRequestUrl('', queryParams), requestOptions)
+    const promise = this.fetchRetry.exponentialBackoff(url, requestOptions)
     const response = await _wrap(promise, {})
 
     if (response.status === 404) {
@@ -474,9 +482,10 @@ class AdobeState {
       }
     }
 
-    logger.debug('any', requestOptions)
+    const url = this.createRequestUrl()
+    logDebug('any', url, requestOptions)
 
-    const promise = this.fetchRetry.exponentialBackoff(this.createRequestUrl(), requestOptions)
+    const promise = this.fetchRetry.exponentialBackoff(url, requestOptions)
     const response = await _wrap(promise, {})
     return (response.status !== 404)
   }
@@ -495,9 +504,10 @@ class AdobeState {
       }
     }
 
-    logger.debug('stats', requestOptions)
+    const url = this.createRequestUrl()
+    logDebug('stats', url, requestOptions)
 
-    const promise = this.fetchRetry.exponentialBackoff(this.createRequestUrl(), requestOptions)
+    const promise = this.fetchRetry.exponentialBackoff(url, requestOptions)
     const response = await _wrap(promise, {})
     if (response.status === 404) {
       return { keys: 0, bytesKeys: 0, bytesValues: 0 }
@@ -531,7 +541,6 @@ class AdobeState {
         ...this.getAuthorizationHeaders()
       }
     }
-    logger.debug('list', requestOptions)
 
     const queryParams = {}
     if (options.match) {
@@ -568,8 +577,11 @@ class AdobeState {
       let cursor = 0
 
       do {
+        const url = stateInstance.createRequestUrl('/data', { ...queryParams, cursor })
+        logDebug('list', url, requestOptions)
+
         const promise = stateInstance.fetchRetry.exponentialBackoff(
-          stateInstance.createRequestUrl('/data', { ...queryParams, cursor }),
+          url,
           requestOptions
         )
         const response = await _wrap(promise, { ...queryParams, cursor })

diff --git a/test/AdobeState.test.js b/test/AdobeState.test.js
@@ -73,12 +73,23 @@ jest.mock('@adobe/aio-lib-env', () => {
   }
 })
 
+const mockLogDebug = jest.fn()
+const mockLogError = jest.fn()
+jest.mock('@adobe/aio-lib-core-logging', () => () => {
+  return {
+    debug: (...args) => mockLogDebug(...args),
+    error: (...args) => mockLogError(...args)
+  }
+})
+
 // jest globals //////////////////////////////////////////////////////////
 
 beforeEach(() => {
   delete process.env.AIO_STATE_ENDPOINT
   mockCLIEnv.mockReturnValue(DEFAULT_ENV)
   mockExponentialBackoff.mockReset()
+  mockLogDebug.mockReset()
+  mockLogError.mockReset()
 })
 
 // //////////////////////////////////////////////////////////
@@ -748,3 +759,30 @@ describe('private methods', () => {
     expect(url).toEqual(`https://custom.abc.com/containers/${fakeCredentials.namespace}`)
   })
 })
+
+test('log debug hides authorization header', async () => {
+  const store = await AdobeState.init(fakeCredentials)
+
+  // get
+  const expiryHeaderValue = '1707445350000'
+  const options = {
+    headersGet: (header) => {
+      if (header === HEADER_KEY_EXPIRES) {
+        return expiryHeaderValue
+      }
+    }
+  }
+  mockExponentialBackoff.mockResolvedValue(wrapInFetchResponse('value', options))
+
+  await store.get('a')
+  await store.put('a', '1')
+  await store.delete('a')
+  await store.any()
+
+  mockExponentialBackoff.mockResolvedValue(wrapInFetchResponse(JSON.stringify({ keys: 1 })))
+  await store.list().next
+  await store.deleteAll({ match: 'a' })
+  await store.stats()
+
+  expect(mockLogDebug).not.toHaveBeenCalledWith(expect.stringContaining(fakeCredentials.apikey))
+})