Incorrect implementation and test for function signatureUtils::fetchPublicKeyFromCloudFront

[shazron]

Function signatureUtils::fetchPublicKeyFromCloudFront:

aio-lib-events/src/signatureUtils.js

Lines 147 to 167 in bfb5e17

	/**
	* Fetches public key using the cloud front public key url
	*
	* @param {*} publicKeyUrl - cloud front public key url of format
	* https://static.adobeioevents.com/prod/keys/pub-key-<random-uuid>.pem
	* @returns {string} public key
	*/
	async function fetchPublicKeyFromCloudFront (publicKeyUrl) {
	let pubKey
	await fetch(publicKeyUrl)
	.then(response => response.text())
	.then(text => {
	logger.info('successfully fetched the public key %s from cloud front url %s', text, publicKeyUrl)
	pubKey = text
	})
	.catch(error => {
	logger.error('error fetching the public key from cloud front url %s due to => %s', publicKeyUrl, error.message)
	return helpers.exportFunctions.genErrorResponse(500, error.message)
	})
	return pubKey
	}

This is an odd use of both async/await and Promise.then, it works - but can be confusing for devs not used to it.

The then() clause assigns to a variable pubKey, which works in this case because of the await (if not pubKey would always be undefined), but it could just simply be the return value for the then(), and should therefore end up to be the return value of the await.

But the problematic item is the catch() clause. It returns helpers.exportFunctions.genErrorResponse(500, error.message), which has two problems:

1. Since the return value of the await is not used, this return value will never be used.
2. helpers.exportFunctions.genErrorResponse returns an object, and this function explicitly says it should return a string in the jsdoc. This is a mismatch.

This therefore renders the test "Test Fetch Key from CloudFront with Invalid Pub Key Url -> verify for invalid pub key" in signatureUtils.test.js as an invalid test since it will never throw an error. In fact the test is bogus (it passes but it is made to pass) - it doesn't really test the (mocked) return value - it tests an error that it sets up itself.

The test is here:

aio-lib-events/test/signatureUtils.test.js

Lines 73 to 90 in bfb5e17

	describe('Test Fetch Key from CloudFront with Invalid Pub Key Url', () => {
	it('verify for invalid pub key', async () => {
	const invalidCloudFrontUrl = undefined
	fetch.mockImplementation(() => {
	return Promise.resolve(Error)
	})
	await signatureUtils.fetchPublicKeyFromCloudFront(invalidCloudFrontUrl)
	.then(res => {
	throw new Error('invalid url')
	})
	.catch(e => {
	// eslint-disable-next-line jest/no-conditional-expect
	expect(e instanceof Error).toBe(true)
	// eslint-disable-next-line jest/no-conditional-expect
	expect(e.message).toEqual('invalid url')
	})
	})
	})

[aiojbot]

JIRA issue created: https://jira.corp.adobe.com/browse/ACNA-2497