diff --git a/.eslintrc.json b/.eslintrc.json index 5b4562377..796f58433 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -9,7 +9,7 @@ "extends": "eslint:recommended", "parserOptions": { "sourceType": "module", - "ecmaVersion": 2017 + "ecmaVersion": 2018 }, "plugins": ["mocha"], "rules": { diff --git a/app/oauth2/access-token-request.js b/app/oauth2/access-token-request.js index ba15d71f0..9ef626100 100644 --- a/app/oauth2/access-token-request.js +++ b/app/oauth2/access-token-request.js @@ -2,6 +2,7 @@ const config = require('config'); const fetch = require('node-fetch'); const { URL } = require('url'); const { Logger } = require('@hmcts/nodejs-logging'); +const { getBasicAuthHeader, redactAuthorizationHeader } = require('./client-auth'); const logger = Logger.getLogger('accessTokenRequest'); @@ -25,9 +26,7 @@ function accessTokenRequest(request) { const options = { method: 'POST', headers: { - 'Authorization': 'Basic ' - + Buffer.from(config.get('idam.oauth2.client_id') + ':' + config.get('secrets.ccd.ccd-api-gateway-oauth2-client-secret')) - .toString('base64'), + 'Authorization': getBasicAuthHeader(), 'Content-Type': 'application/x-www-form-urlencoded' } }; @@ -47,6 +46,7 @@ function accessTokenRequest(request) { }) .catch(error => { logger.error('Failed to obtain access token due to an error:', error); + logger.error('Request headers:', redactAuthorizationHeader(options.headers)); throw error; }); } diff --git a/app/oauth2/client-auth.js b/app/oauth2/client-auth.js new file mode 100644 index 000000000..dab81857d --- /dev/null +++ b/app/oauth2/client-auth.js @@ -0,0 +1,46 @@ +const config = require('config'); + +const AUTHORIZATION_HEADER = 'Authorization'; +const CLIENT_ID_CONFIG_KEY = 'idam.oauth2.client_id'; +const CLIENT_SECRET_CONFIG_KEY = 'secrets.ccd.ccd-api-gateway-oauth2-client-secret'; + +const missingConfigError = (configKey) => { + const error = new Error(`Missing required config: ${configKey}`); + error.status = 500; + error.code = 'OAUTH2_CLIENT_CONFIG_MISSING'; + return error; +}; + +const getRequiredConfig = (configKey) => { + const value = config.get(configKey); + + if (!value) { + throw missingConfigError(configKey); + } + + return value; +}; + +const getBasicAuthHeader = () => { + const clientId = getRequiredConfig(CLIENT_ID_CONFIG_KEY); + const clientSecret = getRequiredConfig(CLIENT_SECRET_CONFIG_KEY); + + const credentials = Buffer.from(`${clientId}:${clientSecret}`).toString('base64'); + return `Basic ${credentials}`; +}; + +const redactAuthorizationHeader = (headers) => { + const result = { ...headers }; + if (result[AUTHORIZATION_HEADER]) { + result[AUTHORIZATION_HEADER] = 'Basic [REDACTED]'; + } + if (result[AUTHORIZATION_HEADER.toLowerCase()]) { + result[AUTHORIZATION_HEADER.toLowerCase()] = 'Basic [REDACTED]'; + } + return result; +}; + +module.exports = { + getBasicAuthHeader, + redactAuthorizationHeader +}; \ No newline at end of file diff --git a/app/oauth2/logout-route.js b/app/oauth2/logout-route.js index aed3edb80..9cef90acd 100644 --- a/app/oauth2/logout-route.js +++ b/app/oauth2/logout-route.js @@ -1,8 +1,12 @@ const config = require('config'); const fetch = require('node-fetch'); +const { Logger } = require('@hmcts/nodejs-logging'); const COOKIE_ACCESS_TOKEN = require('./oauth2-route').COOKIE_ACCESS_TOKEN; const TOKEN_PLACEHOLDER = ':token'; const { userInfoCache } = require('../cache/cache-config'); +const { getBasicAuthHeader, redactAuthorizationHeader } = require('./client-auth'); + +const logger = Logger.getLogger('logoutRoute'); const logoutRoute = (req, res, next) => { const accessToken = req.cookies && req.cookies[COOKIE_ACCESS_TOKEN]; @@ -11,9 +15,7 @@ const logoutRoute = (req, res, next) => { const options = { method: 'DELETE', headers: { - 'Authorization': 'Basic ' - + Buffer.from(config.get('idam.oauth2.client_id') + ':' + config.get('secrets.ccd.ccd-api-gateway-oauth2-client-secret')) - .toString('base64'), + 'Authorization': getBasicAuthHeader(), 'Content-Type': 'application/x-www-form-urlencoded' } }; @@ -23,7 +25,11 @@ const logoutRoute = (req, res, next) => { userInfoCache.del(accessToken); res.status(204).send(); }) - .catch(err => next(err)); + .catch(err => { + logger.error('Failed to logout due to an error:', err); + logger.error('Request headers:', redactAuthorizationHeader(options.headers)); + next(err); + }); } else { next({ error: 'No auth token', diff --git a/eslint.config.mjs b/eslint.config.mjs index 53758ec06..7d30a1430 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -38,7 +38,7 @@ export default defineConfig([{ codecept_helper: true, }, - ecmaVersion: 2017, + ecmaVersion: 2018, sourceType: "module", }, diff --git a/sonar-project.properties b/sonar-project.properties index d8aa87de7..8169eef8a 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -1,4 +1,5 @@ sonar.projectKey=ccd-api-gateway-web +sonar.sourceEncoding=UTF-8 sonar.sources=app/ sonar.tests=test/ sonar.exclusions=node_modules/** diff --git a/test/oauth2/access-token-request.spec.js b/test/oauth2/access-token-request.spec.js index 84c805390..45fb6dd85 100644 --- a/test/oauth2/access-token-request.spec.js +++ b/test/oauth2/access-token-request.spec.js @@ -16,6 +16,7 @@ describe('Access Token Request', () => { const REDIRECT_URL = 'https://localhost/redirect/to'; const UNDEFINED_URI = 'undefined:///oauth2redirect'; const AUTH_CODE = 'xyz789'; + const BASIC_AUTH_HEADER = 'Basic ' + Buffer.from(CLIENT_ID + ':' + CLIENT_SECRET).toString('base64'); const REQUEST = sinonExpressMock.mockReq({ query: { @@ -55,34 +56,39 @@ describe('Access Token Request', () => { let unsuccessfulFetch; let accessTokenRequest; let unsuccessfulAccessTokenRequest; + let clientAuth; beforeEach(() => { config = { get: sinon.stub() }; + clientAuth = { + getBasicAuthHeader: sinon.stub().returns(BASIC_AUTH_HEADER) + }; fetch = fetchMock.sandbox().post(`begin:${TOKEN_ENDPOINT}`, SUCCESSFUL_RESPONSE); accessTokenRequest = proxyquire('../../app/oauth2/access-token-request', { 'config': config, - 'node-fetch': fetch + 'node-fetch': fetch, + './client-auth': clientAuth }); unsuccessfulFetch = fetchMock.sandbox().post(`begin:${TOKEN_ENDPOINT}`, UNSUCCESSFUL_RESPONSE); unsuccessfulAccessTokenRequest = proxyquire('../../app/oauth2/access-token-request', { 'config': config, - 'node-fetch': unsuccessfulFetch + 'node-fetch': unsuccessfulFetch, + './client-auth': clientAuth }); }); it('should call the IdAM OAuth 2 token endpoint with the correct headers and query string parameters', done => { - config.get.withArgs('idam.oauth2.client_id').returns(CLIENT_ID); - config.get.withArgs('secrets.ccd.ccd-api-gateway-oauth2-client-secret').returns(CLIENT_SECRET); config.get.withArgs('idam.oauth2.token_endpoint').returns(TOKEN_ENDPOINT); accessTokenRequest(REQUEST_WITH_HTTPS) .then(() => { expect(fetch.called()).to.be.true; - expect(fetch.lastOptions().headers['Authorization']).to.equal('Basic ' + Buffer.from(CLIENT_ID + ':' + CLIENT_SECRET).toString('base64')); + expect(fetch.lastOptions().headers['Authorization']).to.equal(BASIC_AUTH_HEADER); + expect(clientAuth.getBasicAuthHeader).to.have.been.calledOnce; let requestedUrl = url.parse(fetch.lastUrl(), true); expect(requestedUrl.query.code).to.equal(AUTH_CODE); expect(requestedUrl.query.redirect_uri).to.equal(REDIRECT_URL); @@ -92,14 +98,13 @@ describe('Access Token Request', () => { }); it('should add `https://` prefix', done => { - config.get.withArgs('idam.oauth2.client_id').returns(CLIENT_ID); - config.get.withArgs('secrets.ccd.ccd-api-gateway-oauth2-client-secret').returns(CLIENT_SECRET); config.get.withArgs('idam.oauth2.token_endpoint').returns(TOKEN_ENDPOINT); accessTokenRequest(REQUEST) .then(() => { expect(fetch.called()).to.be.true; - expect(fetch.lastOptions().headers['Authorization']).to.equal('Basic ' + Buffer.from(CLIENT_ID + ':' + CLIENT_SECRET).toString('base64')); + expect(fetch.lastOptions().headers['Authorization']).to.equal(BASIC_AUTH_HEADER); + expect(clientAuth.getBasicAuthHeader).to.have.been.calledOnce; let requestedUrl = url.parse(fetch.lastUrl(), true); expect(requestedUrl.query.code).to.equal(AUTH_CODE); expect(requestedUrl.query.redirect_uri).to.equal(REDIRECT_URL); @@ -111,14 +116,13 @@ describe('Access Token Request', () => { it('should handle unsuccessful responses.', done => { - config.get.withArgs('idam.oauth2.client_id').returns(CLIENT_ID); - config.get.withArgs('secrets.ccd.ccd-api-gateway-oauth2-client-secret').returns(CLIENT_SECRET); config.get.withArgs('idam.oauth2.token_endpoint').returns(TOKEN_ENDPOINT); unsuccessfulAccessTokenRequest(REQUEST) .then((response) => { expect(unsuccessfulFetch.called()).to.be.true; - expect(unsuccessfulFetch.lastOptions().headers['Authorization']).to.equal('Basic ' + Buffer.from(CLIENT_ID + ':' + CLIENT_SECRET).toString('base64')); + expect(unsuccessfulFetch.lastOptions().headers['Authorization']).to.equal(BASIC_AUTH_HEADER); + expect(clientAuth.getBasicAuthHeader).to.have.been.calledOnce; let requestedUrl = url.parse(unsuccessfulFetch.lastUrl(), true); expect(requestedUrl.query.code).to.equal(AUTH_CODE); expect(requestedUrl.query.redirect_uri).to.equal(REDIRECT_URL); @@ -129,8 +133,6 @@ describe('Access Token Request', () => { }); it('should reject undefined uri requests.', async () => { - config.get.withArgs('idam.oauth2.client_id').returns(CLIENT_ID); - config.get.withArgs('secrets.ccd.ccd-api-gateway-oauth2-client-secret').returns(CLIENT_SECRET); config.get.withArgs('idam.oauth2.token_endpoint').returns(TOKEN_ENDPOINT); try { await accessTokenRequest(REQUEST_UNDEFINED_URI); diff --git a/test/oauth2/client-auth.spec.js b/test/oauth2/client-auth.spec.js new file mode 100644 index 000000000..d74b8a382 --- /dev/null +++ b/test/oauth2/client-auth.spec.js @@ -0,0 +1,63 @@ +const chai = require('chai'); +const expect = chai.expect; +const proxyquire = require('proxyquire'); +const sinon = require('sinon'); + +describe('OAuth2 client auth helper', () => { + let config; + let clientAuth; + + beforeEach(() => { + config = { + get: sinon.stub() + }; + + clientAuth = proxyquire('../../app/oauth2/client-auth', { + 'config': config + }); + }); + + it('should build the Basic authorization header from configured credentials', () => { + config.get.withArgs('idam.oauth2.client_id').returns('ccd_gateway'); + config.get.withArgs('secrets.ccd.ccd-api-gateway-oauth2-client-secret').returns('abc123def456'); + + expect(clientAuth.getBasicAuthHeader()) + .to.equal(`Basic ${Buffer.from('ccd_gateway:abc123def456').toString('base64')}`); + }); + + it('should throw when the OAuth2 client secret is missing', () => { + config.get.withArgs('idam.oauth2.client_id').returns('ccd_gateway'); + config.get.withArgs('secrets.ccd.ccd-api-gateway-oauth2-client-secret').returns(''); + + expect(() => clientAuth.getBasicAuthHeader()) + .to.throw('Missing required config: secrets.ccd.ccd-api-gateway-oauth2-client-secret'); + }); + + it('should throw when the OAuth2 client ID is missing', () => { + config.get.withArgs('idam.oauth2.client_id').returns(''); + config.get.withArgs('secrets.ccd.ccd-api-gateway-oauth2-client-secret').returns('abc123def456'); + + expect(() => clientAuth.getBasicAuthHeader()) + .to.throw('Missing required config: idam.oauth2.client_id'); + }); + + it('should redact Authorization headers before logging', () => { + expect(clientAuth.redactAuthorizationHeader({ + Authorization: 'Basic Y2NkX2dhdGV3YXk6YWJjMTIzZGVmNDU2', + 'Content-Type': 'application/x-www-form-urlencoded' + })).to.deep.equal({ + Authorization: 'Basic [REDACTED]', + 'Content-Type': 'application/x-www-form-urlencoded' + }); + }); + + it('should redact lowercase authorization headers before logging', () => { + expect(clientAuth.redactAuthorizationHeader({ + authorization: 'Basic Y2NkX2dhdGV3YXk6YWJjMTIzZGVmNDU2', + 'content-type': 'application/x-www-form-urlencoded' + })).to.deep.equal({ + authorization: 'Basic [REDACTED]', + 'content-type': 'application/x-www-form-urlencoded' + }); + }); +}); \ No newline at end of file diff --git a/test/oauth2/logout-route.spec.js b/test/oauth2/logout-route.spec.js index f25d396c9..0a92f3404 100644 --- a/test/oauth2/logout-route.spec.js +++ b/test/oauth2/logout-route.spec.js @@ -31,6 +31,7 @@ describe('logoutRoute', () => { let userInfoCacheSpy; let sandbox; let clock; + let clientAuth; let cachedUserResolver; let userInfoCache; @@ -46,9 +47,10 @@ describe('logoutRoute', () => { cachedUserResolver = proxyquire('../../app/user/cached-user-resolver', { '../cache/cache-config': { userInfoCache } }); + clientAuth = { + getBasicAuthHeader: sinon.stub().returns('Basic ' + Buffer.from(CLIENT_ID + ':' + CLIENT_SECRET).toString('base64')) + }; - config.get.withArgs('idam.oauth2.client_id').returns(CLIENT_ID); - config.get.withArgs('secrets.ccd.ccd-api-gateway-oauth2-client-secret').returns(CLIENT_SECRET); config.get.withArgs('idam.oauth2.logout_endpoint').returns(LOGOUT_END_POINT); request = sinonExpressMock.mockReq({ @@ -63,7 +65,8 @@ describe('logoutRoute', () => { logoutRoute = proxyquire('../../app/oauth2/logout-route', { 'config': config, - 'node-fetch': fetch + 'node-fetch': fetch, + './client-auth': clientAuth }).logoutRoute; }); @@ -80,6 +83,7 @@ describe('logoutRoute', () => { try { expect(fetch.called(LOGOUT_END_POINT.replace(':token', ACCESS_TOKEN))).to.be.true; expect(fetch.lastOptions().headers['Authorization']).to.equal('Basic ' + Buffer.from(CLIENT_ID + ':' + CLIENT_SECRET).toString('base64')); + expect(clientAuth.getBasicAuthHeader).to.have.been.calledOnce; expect(next).not.to.be.called; expect(response.clearCookie).to.be.calledWith(ACCESS_TOKEN_COOKIE_NAME); @@ -102,8 +106,6 @@ describe('logoutRoute', () => { logoutRoute(request, response, next); - expect(config.get).to.be.calledWith('idam.oauth2.client_id'); - expect(config.get).to.be.calledWith('secrets.ccd.ccd-api-gateway-oauth2-client-secret'); expect(config.get).to.be.calledWith('idam.oauth2.logout_endpoint'); });