diff --git a/app/oauth2/oauth2-route.js b/app/oauth2/oauth2-route.js index 826ee5fc9..1aeddef67 100644 --- a/app/oauth2/oauth2-route.js +++ b/app/oauth2/oauth2-route.js @@ -13,7 +13,8 @@ const oauth2Route = (req, res, next) => { { maxAge: jsonResult.expires_in * 1000, httpOnly: true, - secure: config.get('security.secure_auth_cookie_enabled') + secure: config.get('security.secure_auth_cookie_enabled'), + sameSite: 'Lax' }); res.status(204).send(); } diff --git a/app/security/cors.js b/app/security/cors.js index 679c3676d..197de6afa 100644 --- a/app/security/cors.js +++ b/app/security/cors.js @@ -1,47 +1,71 @@ const config = require('config'); const sanitize = require('../util/sanitize'); -const WILDCARD = '*'; - -const createWhitelistValidator = (val) => { - const whitelist = config.get('security.cors_origin_whitelist').split(','); - for (let w of whitelist) { - if (val === w || WILDCARD === w) { - return true; - } +const escapeRegex = (str) => + str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + +const ALLOWED_HEADERS = ['content-type', 'authorization']; + +const isOriginAllowed = (origin) => { + if (typeof origin !== 'string') return false; + + const whitelist = config + .get('security.cors_origin_whitelist') + .split(',') + .map(w => w.trim()); + + if (whitelist.includes('*')) { + throw new Error('CORS whitelist cannot contain "*"'); + } + + return whitelist.some(w => { + if (w === origin) return true; + + if (w.includes('*')) { + const pattern = + '^' + + escapeRegex(w).replace(/\\\*/g, '[^.]+') + + '$'; + + return new RegExp(pattern).test(origin); } + return false; + }); }; -const corsOptions = { - allowOrigin: createWhitelistValidator, - allowCredentials: true, - allowMethods: config.get('security.cors_origin_methods') +const resolveAllowedHeaders = (req) => { + const requested = req.get('Access-Control-Request-Headers'); + + if (!requested) return 'Content-Type, Authorization'; + + const filtered = requested + .split(',') + .map(h => h.trim()) + .filter(h => ALLOWED_HEADERS.includes(h.toLowerCase())) + .join(', '); + + return filtered || 'Content-Type, Authorization'; }; const handleCors = (req, res, next) => { - if (corsOptions.allowOrigin) { - const origin = req.get('origin'); - if (corsOptions.allowOrigin(origin)) { - res.set('Access-Control-Allow-Origin', sanitize.sanitizeData(origin)); - } - } else { - res.set('Access-Control-Allow-Origin', '*'); - } - if (corsOptions.allowCredentials) { - res.set('Access-Control-Allow-Credentials', corsOptions.allowCredentials); - } - if (corsOptions.allowMethods) { - res.set('Access-Control-Allow-Methods', corsOptions.allowMethods); - } - res.set('Access-Control-Allow-Headers', sanitize.sanitizeData(req.get('Access-Control-Request-Headers'))); - if('OPTIONS' === req.method) { - res - .status(200) - .end(); - } else { - next(); - } + const origin = req.get('origin'); + + if (!origin || !isOriginAllowed(origin)) { + return res.status(403).end(); + } + + res.set('Access-Control-Allow-Origin', sanitize.sanitizeData(origin)); + res.set('Access-Control-Allow-Credentials', true); + res.set('Access-Control-Allow-Methods', config.get('security.cors_origin_methods')); + res.set('Access-Control-Allow-Headers', resolveAllowedHeaders(req)); + res.set('Vary', 'Origin'); + + if (req.method === 'OPTIONS') { + return res.status(200).end(); + } + + next(); }; module.exports = handleCors; diff --git a/charts/ccd-api-gateway-web/values.aat.template.yaml b/charts/ccd-api-gateway-web/values.aat.template.yaml index f22bd538f..491508c06 100644 --- a/charts/ccd-api-gateway-web/values.aat.template.yaml +++ b/charts/ccd-api-gateway-web/values.aat.template.yaml @@ -2,4 +2,4 @@ nodejs: image: ${IMAGE_NAME} ingressHost: ${SERVICE_FQDN} environment: - CORS_ORIGIN_WHITELIST: "*" + CORS_ORIGIN_WHITELIST: "https://*.preview.platform.hmcts.net" diff --git a/charts/ccd-api-gateway-web/values.preview.template.yaml b/charts/ccd-api-gateway-web/values.preview.template.yaml index 96fb639f3..f4c7015b1 100644 --- a/charts/ccd-api-gateway-web/values.preview.template.yaml +++ b/charts/ccd-api-gateway-web/values.preview.template.yaml @@ -5,7 +5,7 @@ nodejs: IDAM_OAUTH2_TOKEN_ENDPOINT: https://idam-api.aat.platform.hmcts.net/oauth2/token IDAM_OAUTH2_LOGOUT_ENDPOINT: https://idam-api.aat.platform.hmcts.net/session/:token IDAM_BASE_URL: https://idam-api.aat.platform.hmcts.net - CORS_ORIGIN_WHITELIST: "*" + CORS_ORIGIN_WHITELIST: "https://*.preview.platform.hmcts.net" TIMING-ALLOW-ORIGIN: "*" PROXY_MV_ANNOTATIONS_API_URL: https://em-anno-aat.service.core-compute-aat.internal diff --git a/test/oauth2/oauth2-route.spec.js b/test/oauth2/oauth2-route.spec.js index 240901a5a..53b385acc 100644 --- a/test/oauth2/oauth2-route.spec.js +++ b/test/oauth2/oauth2-route.spec.js @@ -4,10 +4,14 @@ const proxyquire = require('proxyquire'); const sinon = require('sinon'); const sinonChai = require('sinon-chai'); const sinonExpressMock = require('sinon-express-mock'); -const ACCESS_TOKEN_COOKIE_NAME = require('../../app/oauth2/oauth2-route').COOKIE_ACCESS_TOKEN; + +const ACCESS_TOKEN_COOKIE_NAME = + require('../../app/oauth2/oauth2-route').COOKIE_ACCESS_TOKEN; + chai.use(sinonChai); describe('oauth2Route', () => { + const TOKEN = { access_token: 'ey123.ey456', expires_in: 3600 @@ -22,96 +26,118 @@ describe('oauth2Route', () => { let responseFromPromiseMock; beforeEach(() => { - config = { get: sinon.stub() }; + + request = sinonExpressMock.mockReq(); + response = sinonExpressMock.mockRes(); + next = sinon.stub(); + responseFromPromiseMock = { status: 200, json: sinon.stub() }; - request = sinonExpressMock.mockReq(); - response = sinonExpressMock.mockRes(); - next = sinon.stub(); accessTokenRequest = sinon.stub(); - accessTokenRequest.withArgs(request).returns(Promise.resolve(responseFromPromiseMock)); + accessTokenRequest.withArgs(request) + .returns(Promise.resolve(responseFromPromiseMock)); oauth2Route = proxyquire('../../app/oauth2/oauth2-route', { './access-token-request': accessTokenRequest, - 'config': config + config }).oauth2Route; }); - it('should set an accessToken cookie with the "secure" flag enabled', done => { + it('should set accessToken cookie with secure flag enabled', (done) => { config.get.withArgs('security.secure_auth_cookie_enabled').returns(true); - responseFromPromiseMock.json.withArgs().returns(Promise.resolve(TOKEN)); + responseFromPromiseMock.json.returns(Promise.resolve(TOKEN)); - response.send.callsFake( () => { + response.status.callsFake(() => response); + response.send.callsFake(() => { try { + expect(accessTokenRequest).to.have.been.calledWith(request); + expect(config.get).to.have.been.calledWith('security.secure_auth_cookie_enabled'); + + expect(response.cookie).to.have.been.calledWith( + ACCESS_TOKEN_COOKIE_NAME, + TOKEN.access_token, + { + maxAge: TOKEN.expires_in * 1000, + httpOnly: true, + secure: true, + sameSite: 'Lax' + } + ); + + expect(response.status).to.have.been.calledWith(204); - expect(accessTokenRequest).to.be.calledWith(request); - expect(config.get).to.be.calledWith('security.secure_auth_cookie_enabled'); - expect(response.cookie).to.be.calledWith(ACCESS_TOKEN_COOKIE_NAME, TOKEN.access_token, - { maxAge: TOKEN.expires_in * 1000, httpOnly: true, secure: true }); - expect(response.status).to.be.calledWith(204); done(); - } catch (e) { - done(e); + } catch (err) { + done(err); } }); oauth2Route(request, response, next); }); - it('should set an accessToken cookie with the "secure" flag disabled', done => { + it('should set accessToken cookie with secure flag disabled', (done) => { config.get.withArgs('security.secure_auth_cookie_enabled').returns(false); - responseFromPromiseMock.json.withArgs().returns(Promise.resolve(TOKEN)); + responseFromPromiseMock.json.returns(Promise.resolve(TOKEN)); + response.status.callsFake(() => response); response.send.callsFake(() => { try { - expect(accessTokenRequest).to.be.calledWith(request); - expect(config.get).to.be.calledWith('security.secure_auth_cookie_enabled'); - expect(response.cookie).to.be.calledWith(ACCESS_TOKEN_COOKIE_NAME, TOKEN.access_token, - { maxAge: TOKEN.expires_in * 1000, httpOnly: true, secure: false }); - expect(response.status).to.be.calledWith(204); + expect(response.cookie).to.have.been.calledWith( + ACCESS_TOKEN_COOKIE_NAME, + TOKEN.access_token, + { + maxAge: TOKEN.expires_in * 1000, + httpOnly: true, + secure: false, + sameSite: 'Lax' + } + ); + + expect(response.status).to.have.been.calledWith(204); done(); - } catch (e) { - done(e); + } catch (err) { + done(err); } }); oauth2Route(request, response, next); }); - it('should fail to obation an accessToken dude to unauthorized request.', done => { + it('should call next with error when token request fails', (done) => { - let expectedError = { + const expectedError = { status: 502, message: 'Internal Server Error' }; - let unauthorizedAccessTokenRequest = sinon.stub(); - unauthorizedAccessTokenRequest.withArgs(request).returns(Promise.resolve(expectedError)); + const failingRequest = sinon.stub() + .withArgs(request) + .returns(Promise.resolve(expectedError)); - let unauthorizedOauth2Route = proxyquire('../../app/oauth2/oauth2-route', { - './access-token-request': unauthorizedAccessTokenRequest, - 'config': config + const failingRoute = proxyquire('../../app/oauth2/oauth2-route', { + './access-token-request': failingRequest, + config }).oauth2Route; - next.callsFake((result) => { + next.callsFake((err) => { try { - - expect(unauthorizedAccessTokenRequest).to.be.calledWith(request); - expect(result).to.eql(expectedError); + expect(failingRequest).to.have.been.calledWith(request); + expect(err).to.eql(expectedError); done(); } catch (e) { done(e); } }); - unauthorizedOauth2Route(request, response, next); + failingRoute(request, response, next); }); + }); diff --git a/test/security/cors.spec.js b/test/security/cors.spec.js index 5c9d13860..a76199599 100644 --- a/test/security/cors.spec.js +++ b/test/security/cors.spec.js @@ -8,10 +8,12 @@ chai.use(sinonChai); describe('CORS', () => { - const ORIGIN = 'http://localhost:3451'; - const ORIGIN_2 = 'http://ccd-aat.platform.hmcts.net'; - const METHODS = 'GET,POST,OPTIONS,PUT,DELETE'; - const HEADERS = 'Authorization'; + const ORIGIN_1 = 'https://www-ccd.aat.platform.hmcts.net'; + const ORIGIN_2 = 'https://manage-case.aat.platform.hmcts.net'; + const UNAUTHORISED_ORIGIN = 'https://test.com'; + + const METHODS = 'GET,POST,PUT,PATCH,DELETE,OPTIONS'; + const HEADERS = 'Content-Type, Authorization'; let config, req, res, next, handleCors; @@ -20,53 +22,149 @@ describe('CORS', () => { get: sinon.stub() }; - config.get.withArgs('security.cors_origin_whitelist').returns(ORIGIN); - config.get.withArgs('security.cors_origin_methods').returns(METHODS); + // default whitelist + config.get.withArgs('security.cors_origin_whitelist') + .returns(`${ORIGIN_1},${ORIGIN_2}`); - req = sinonExpressMock.mockReq(); - req.get.withArgs('origin').returns(ORIGIN); - req.get.withArgs('Access-Control-Request-Headers').returns(HEADERS); + config.get.withArgs('security.cors_origin_methods') + .returns(METHODS); + req = sinonExpressMock.mockReq(); res = sinonExpressMock.mockRes({}); next = sinon.stub(); handleCors = proxyquire('../../app/security/cors', { - 'config': config + config }); }); - it('should add CORS headers to response', () => { + it('should allow requests from whitelisted origin', () => { + req.get.withArgs('origin').returns(ORIGIN_1); + req.get.withArgs('Access-Control-Request-Headers').returns(HEADERS); + handleCors(req, res, next); - expect(res.set).to.have.been.calledWith('Access-Control-Allow-Origin', ORIGIN); - expect(res.set).to.have.been.calledWith('Access-Control-Allow-Credentials', true); - expect(res.set).to.have.been.calledWith('Access-Control-Allow-Methods', METHODS); - expect(res.set).to.have.been.calledWith('Access-Control-Allow-Headers', HEADERS); + expect(res.set).to.have.been.calledWith( + 'Access-Control-Allow-Origin', + ORIGIN_1 + ); + + expect(res.set).to.have.been.calledWith( + 'Access-Control-Allow-Credentials', + true + ); + + expect(res.set).to.have.been.calledWith( + 'Access-Control-Allow-Methods', + METHODS + ); + + expect(res.set).to.have.been.calledWith( + 'Access-Control-Allow-Headers', + HEADERS + ); + + expect(next).to.have.been.called; }); - it('should support multiple whitelisted origins', () => { - config.get.withArgs('security.cors_origin_whitelist').returns(`${ORIGIN},${ORIGIN_2}`); + it('should allow multiple whitelisted origins', () => { req.get.withArgs('origin').returns(ORIGIN_2); + req.get.withArgs('Access-Control-Request-Headers').returns(HEADERS); handleCors(req, res, next); - expect(res.set).to.have.been.calledWith('Access-Control-Allow-Origin', ORIGIN_2); + expect(res.set).to.have.been.calledWith( + 'Access-Control-Allow-Origin', + ORIGIN_2 + ); }); - it('should not allow non-whitelisted origins', () => { - req.get.withArgs('origin').returns(ORIGIN_2); + it('should reject non-whitelisted origins with 403', () => { + req.get.withArgs('origin').returns(UNAUTHORISED_ORIGIN); handleCors(req, res, next); - expect(res.set).not.to.have.been.calledWith('Access-Control-Allow-Origin', ORIGIN_2); + expect(res.status).to.have.been.calledWith(403); + expect(res.end).to.have.been.called; + expect(res.set).not.to.have.been.calledWith( + 'Access-Control-Allow-Origin', + UNAUTHORISED_ORIGIN + ); }); - it('should allow any origin when whitelist contains wildcard *', () => { - config.get.withArgs('security.cors_origin_whitelist').returns(`${ORIGIN},*`); - req.get.withArgs('origin').returns(ORIGIN_2); + it('should require origin header', () => { + req.get.withArgs('origin').returns(undefined); handleCors(req, res, next); - expect(res.set).to.have.been.calledWith('Access-Control-Allow-Origin', ORIGIN_2); + expect(res.status).to.have.been.calledWith(403); + expect(res.end).to.have.been.called; }); + + describe('CORS wildcard pattern support', () => { + + const WILDCARD = 'https://*.preview.platform.hmcts.net'; + + beforeEach(() => { + config.get.withArgs('security.cors_origin_whitelist') + .returns(WILDCARD); + }); + + it('should allow valid preview subdomain', () => { + const origin = 'https://ccd-api-gateway-web-pr-712.preview.platform.hmcts.net'; + + req.get.withArgs('origin').returns(origin); + req.get.withArgs('Access-Control-Request-Headers').returns(HEADERS); + + handleCors(req, res, next); + + expect(res.set).to.have.been.calledWith( + 'Access-Control-Allow-Origin', + origin + ); + expect(next).to.have.been.called; + }); + + it('should reject different domain (hmcts2)', () => { + const origin = 'https://ccd-api-gateway-web-pr-712.preview.platform.hmcts2.net'; + + req.get.withArgs('origin').returns(origin); + + handleCors(req, res, next); + + expect(res.status).to.have.been.calledWith(403); + }); + + it('should reject nested subdomains', () => { + const origin = 'https://a.b.preview.platform.hmcts.net'; + + req.get.withArgs('origin').returns(origin); + + handleCors(req, res, next); + + expect(res.status).to.have.been.calledWith(403); + }); + + it('should reject suffix attack domains', () => { + const origin = 'https://preview.platform.hmcts.net.test.com'; + + req.get.withArgs('origin').returns(origin); + + handleCors(req, res, next); + + expect(res.status).to.have.been.calledWith(403); + }); + + it('should reject completely unrelated domains', () => { + const origin = 'https://test.com'; + + req.get.withArgs('origin').returns(origin); + + handleCors(req, res, next); + + expect(res.status).to.have.been.calledWith(403); + }); + + }); + }); diff --git a/yarn-audit-known-issues b/yarn-audit-known-issues index 2d969fcd9..72350baec 100644 --- a/yarn-audit-known-issues +++ b/yarn-audit-known-issues @@ -1,4 +1,4 @@ {"value":"glob","children":{"ID":"glob (deprecation)","Issue":"Old versions of glob are not supported, and contain widely publicized security vulnerabilities, which have been fixed in the current version. Please update. Support for old versions may be purchased (at exorbitant rates) by contacting i@izs.me","Severity":"moderate","Vulnerable Versions":"7.2.3","Tree Versions":["7.2.3"],"Dependents":["nyc@npm:15.1.0"]}} {"value":"inflight","children":{"ID":"inflight (deprecation)","Issue":"This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.","Severity":"moderate","Vulnerable Versions":"1.0.6","Tree Versions":["1.0.6"],"Dependents":["glob@npm:7.2.3"]}} {"value":"rimraf","children":{"ID":"rimraf (deprecation)","Issue":"Rimraf versions prior to v4 are no longer supported","Severity":"moderate","Vulnerable Versions":"3.0.2","Tree Versions":["3.0.2"],"Dependents":["nyc@npm:15.1.0"]}} -{"value":"uuid","children":{"ID":1116970,"Issue":"uuid: Missing buffer bounds check in v3/v5/v6 when buf is provided","URL":"https://github.com/advisories/GHSA-w5hq-g745-h8pq","Severity":"moderate","Vulnerable Versions":"<14.0.0","Tree Versions":["8.3.2"],"Dependents":["@azure/functions@npm:3.5.1"]}} +{"value":"uuid","children":{"ID":"uuid (deprecation)","Issue":"uuid@10 and below is no longer supported. For ESM codebases, update to uuid@latest. For CommonJS codebases, use uuid@11 (but be aware this version will likely be deprecated in 2028).","Severity":"moderate","Vulnerable Versions":"8.3.2","Tree Versions":["8.3.2"],"Dependents":["@azure/functions@npm:3.5.1"]}}