diff --git a/package-lock.json b/package-lock.json index 865cd8b0e..54f7258af 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6491,9 +6491,9 @@ } }, "ipaddr.js": { - "version": "1.9.1", - "resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-1.9.1.tgz", - "integrity": "sha512-0KI/607xoxSToH7GjN1FfSbLoU0+btTicjsQSWQlh/hZykN8KpmMf7uYwPW3R+akZ6R/w18ZlXSHBYXiYUPO3g==" + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-2.0.1.tgz", + "integrity": "sha512-1qTgH9NG+IIJ4yfKs2e6Pp1bZg8wbDbKHT21HrLIeYBTRLgMYKnMTPAuI3Lcs61nfx5h1xlXnbJtH1kX5/d/ng==" }, "is-accessor-descriptor": { "version": "0.1.6", @@ -10459,6 +10459,13 @@ "requires": { "forwarded": "0.2.0", "ipaddr.js": "1.9.1" + }, + "dependencies": { + "ipaddr.js": { + "version": "1.9.1", + "resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-1.9.1.tgz", + "integrity": "sha512-0KI/607xoxSToH7GjN1FfSbLoU0+btTicjsQSWQlh/hZykN8KpmMf7uYwPW3R+akZ6R/w18ZlXSHBYXiYUPO3g==" + } } }, "pseudomap": { diff --git a/package.json b/package.json index fd113615c..431694700 100644 --- a/package.json +++ b/package.json @@ -41,6 +41,7 @@ "express-openapi-validator": "^4.10.8", "github-download-directory": "^2.0.0", "ioredis": "^4.17.3", + "ipaddr.js": "^2.0.1", "js-yaml": "^3.13.1", "json-merger": "^1.1.7", "jsonwebtoken": "^8.5.1", diff --git a/src/utils/authMiddlewares.js b/src/utils/authMiddlewares.js index 21c3814aa..0e3665c33 100644 --- a/src/utils/authMiddlewares.js +++ b/src/utils/authMiddlewares.js @@ -8,7 +8,6 @@ const jwt = require('jsonwebtoken'); const jwtExpress = require('express-jwt'); const jwkToPem = require('jwk-to-pem'); const util = require('util'); -const dns = require('dns').promises; const config = require('../config'); @@ -18,6 +17,7 @@ const { CacheMissError } = require('../cache/cache-utils'); const { UnauthorizedError, UnauthenticatedError } = require('./responses'); const ProjectsService = require('../api/route-services/projects'); const AccessService = require('../api/route-services/access'); +const { isReqFromLocalhost, isReqFromCluster } = require('./isReqFrom'); const accessService = new AccessService(); const projectService = new ProjectsService(); @@ -82,32 +82,7 @@ const authenticationMiddlewareExpress = async (app) => { }); }; - -// eslint-disable-next-line no-useless-escape -const INTERNAL_DOMAINS_REGEX = new RegExp('((\.compute\.internal)|(\.svc\.local))$'); - const checkAuthExpiredMiddleware = (req, res, next) => { - const isReqFromLocalhost = async () => { - const ip = req.connection.remoteAddress; - const host = req.get('host'); - - if (ip === '127.0.0.1' || ip === '::ffff:127.0.0.1' || ip === '::1' || host.indexOf('localhost') !== -1) { - return true; - } - - throw new Error('ip address is not localhost'); - }; - - const isReqFromCluster = async () => { - const domains = await dns.reverse(req.ip); - - if (!domains.some((domain) => INTERNAL_DOMAINS_REGEX.test(domain))) { - throw new Error('ip address does not come from internal sources'); - } - - return true; - }; - if (!req.user) { return next(); } @@ -125,8 +100,9 @@ const checkAuthExpiredMiddleware = (req, res, next) => { return next(new UnauthenticatedError('token has expired')); } + // check if we should ignore expired jwt token for this path and request type - const longTimeoutEndpoints = [{ urlMatcher: /^\/v1\/experiments\/.{32}\/cellSets$/, method: 'PATCH' }]; + const longTimeoutEndpoints = [{ urlMatcher: /experiments\/.{32}\/cellSets$/, method: 'PATCH' }]; const isEndpointIgnored = longTimeoutEndpoints.some( ({ urlMatcher, method }) => ( req.method.toLowerCase() === method.toLowerCase() && urlMatcher.test(req.url) @@ -135,15 +111,13 @@ const checkAuthExpiredMiddleware = (req, res, next) => { // if endpoint is not in ignore list, the JWT is too old, send an error accordingly if (!isEndpointIgnored) { - return next(new UnauthenticatedError('token has expired')); + return next(new UnauthenticatedError('token has expired for non-ignored endpoint')); } - promiseAny([isReqFromCluster(), isReqFromLocalhost()]) - .then(() => { - next(); - }) - .catch(() => { - next(new UnauthenticatedError('token has expired')); + promiseAny([isReqFromCluster(req), isReqFromLocalhost(req)]) + .then(() => next()) + .catch((e) => { + next(new UnauthenticatedError(`invalid request origin ${e}`)); }); return null; diff --git a/src/utils/isReqFrom.js b/src/utils/isReqFrom.js new file mode 100644 index 000000000..035c5560e --- /dev/null +++ b/src/utils/isReqFrom.js @@ -0,0 +1,41 @@ +const dns = require('dns').promises; +const ipaddr = require('ipaddr.js'); + +// eslint-disable-next-line no-useless-escape +const INTERNAL_DOMAINS_REGEX = new RegExp('((\.compute\.internal)|(\.svc\.local))$'); + +const isReqFromLocalhost = async (req) => { + const ip = req.connection.remoteAddress; + const host = req.get('host'); + + if (ip === '127.0.0.1' || ip === '::ffff:127.0.0.1' || ip === '::1' || host.indexOf('localhost') !== -1) { + return true; + } + + throw new Error('ip address is not localhost'); +}; + +const isReqFromCluster = async (req) => { + let remoteAddress = req.ip; + const addr = ipaddr.parse(req.ip); + // req.ip returns IPv4 addresses mapped to IPv6, e.g.: + // 127.0.0.1 (IPv4) -> ::ffff:127.0.0.1 (IPv6) + // dns.reverse is not capable of dealing with them, + // it either uses IPv4 or IPv6, so we need to map those + // IPs back to IPv4 before. + if (addr.kind() === 'ipv6' && addr.isIPv4MappedAddress()) { + remoteAddress = addr.toIPv4Address().toString(); + } + + const domains = await dns.reverse(remoteAddress); + if (domains.some((domain) => INTERNAL_DOMAINS_REGEX.test(domain))) { + return true; + } + + throw new Error('ip address does not come from internal sources'); +}; + +module.exports = { + isReqFromCluster, + isReqFromLocalhost, +}; diff --git a/tests/utils/authMiddlewares.test.js b/tests/utils/authMiddlewares.test.js index 0f0889764..e0fdfa1d7 100644 --- a/tests/utils/authMiddlewares.test.js +++ b/tests/utils/authMiddlewares.test.js @@ -1,5 +1,7 @@ const AWSMock = require('aws-sdk-mock'); + const { + checkAuthExpiredMiddleware, expressAuthorizationMiddleware, authorize, } = require('../../src/utils/authMiddlewares'); @@ -52,6 +54,22 @@ describe('Tests for authorization/authentication middlewares', () => { expect(next).toBeCalledWith(); }); + it('checkAuth accepts expired tokens for patch cellsets', async () => { + mockDynamoGetItem(data); + + const req = { + params: { experimentId: fake.EXPERIMENT_ID }, + user: fake.USER, + url: `/v1/experiments/${fake.EXPERIMENT_ID}/cellSets`, + method: 'PATCH', + ip: '::ffff:127.0.0.1', + }; + const next = jest.fn(); + + const ret = checkAuthExpiredMiddleware(req, {}, next); + expect(ret).toBe(null); + }); + it('Express middleware can reject incorrect users', async () => { mockDynamoGetItem({}); diff --git a/tests/utils/isReqFrom.test.js b/tests/utils/isReqFrom.test.js new file mode 100644 index 000000000..e0ae722ce --- /dev/null +++ b/tests/utils/isReqFrom.test.js @@ -0,0 +1,77 @@ + + +jest.mock('dns', () => ({ + promises: { + reverse: jest.fn((ip) => { + if (ip === '127.0.0.1') return []; + return ['ip-192-168-905-123.region.compute.internal']; + }), + }, +})); + +const { + isReqFromCluster, + isReqFromLocalhost, +} = require('../../src/utils/isReqFrom'); +const fake = require('../test-utils/constants'); + +describe('Tests for isRequestFromCluster and isRequestFromLocalhost', () => { + it('isReqFromLocalhost returns true for localhost', async () => { + const req = { + params: { experimentId: fake.EXPERIMENT_ID }, + user: fake.USER, + url: `/v1/experiments/${fake.EXPERIMENT_ID}/cellSets`, + method: 'PATCH', + connection: { + address: '::ffff:127.0.0.1', + }, + get: () => 'localhost', + }; + + expect(await isReqFromLocalhost(req)).toEqual(true); + }); + + it('isRequestFromLocalhost throws with cluster addr', async () => { + const req = { + params: { experimentId: fake.EXPERIMENT_ID }, + user: fake.USER, + url: `/v1/experiments/${fake.EXPERIMENT_ID}/cellSets`, + method: 'PATCH', + ip: '::ffff:192.0.0.1', + connection: { + address: '::ffff:192.0.0.1', + }, + get: () => 'whatever.internal.compute', + }; + + await expect(isReqFromLocalhost(req)) + .rejects + .toThrow(); + }); + + it('isReqFromCluster returns true for cluster addr', async () => { + const req = { + params: { experimentId: fake.EXPERIMENT_ID }, + user: fake.USER, + url: `/v1/experiments/${fake.EXPERIMENT_ID}/cellSets`, + method: 'PATCH', + ip: '::ffff:192.0.0.1', + }; + + expect(await isReqFromCluster(req)).toEqual(true); + }); + + it('isRequestFromCluster throws with localhost', async () => { + const req = { + params: { experimentId: fake.EXPERIMENT_ID }, + user: fake.USER, + url: `/v1/experiments/${fake.EXPERIMENT_ID}/cellSets`, + method: 'PATCH', + ip: '::ffff:127.0.0.1', + }; + + await expect(isReqFromCluster(req)) + .rejects + .toThrow(); + }); +});