Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: hide authorization header from debug logs #181

Merged
merged 1 commit into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 30 additions & 18 deletions lib/AdobeState.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -263,8 +267,6 @@ class AdobeState {
* @memberof AdobeState
*/
async get (key) {
logger.debug(`get '${key}'`)

const schema = {
type: 'object',
properties: {
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -429,8 +436,6 @@ class AdobeState {
}
}

logger.debug('deleteAll', requestOptions)

const schema = {
type: 'object',
properties: {
Expand All @@ -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) {
Expand All @@ -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)
}
Expand All @@ -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 }
Expand Down Expand Up @@ -531,7 +541,6 @@ class AdobeState {
...this.getAuthorizationHeaders()
}
}
logger.debug('list', requestOptions)

const queryParams = {}
if (options.match) {
Expand Down Expand Up @@ -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 })
Expand Down
38 changes: 38 additions & 0 deletions test/AdobeState.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})

// //////////////////////////////////////////////////////////
Expand Down Expand Up @@ -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))
})