CME-983: Refactor OAuth2 authorization handling by introducing a clie…

.eslintrc.json

@@ -9,7 +9,7 @@
     "extends": "eslint:recommended",
     "parserOptions": {
         "sourceType": "module",
-        "ecmaVersion": 2017
+        "ecmaVersion": 2018
     },
     "plugins": ["mocha"],
     "rules": {

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;
     });
 }

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
+};

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',

eslint.config.mjs

@@ -38,7 +38,7 @@ export default defineConfig([{
             codecept_helper: true,
         },
 
-        ecmaVersion: 2017,
+        ecmaVersion: 2018,
         sourceType: "module",
     },
 

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/**

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);

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'
+    });
+  });
+});

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');
   });