From f1e67ef36b95c6b485054961e1c9858b40ced04a Mon Sep 17 00:00:00 2001 From: Shazron Abdullah Date: Tue, 12 Nov 2024 23:52:59 +0800 Subject: [PATCH 1/3] fix: add proper self-signed cert tests (with proxy option rejectUnauthorized) needs new @adobe/aio-lib-test-proxy release --- test/proxy.test.js | 152 +++++++++++++++++++++++++++++++-------------- 1 file changed, 106 insertions(+), 46 deletions(-) diff --git a/test/proxy.test.js b/test/proxy.test.js index 346590d..b654dda 100644 --- a/test/proxy.test.js +++ b/test/proxy.test.js @@ -189,14 +189,15 @@ describe('http proxy', () => { }) }) -describe('https proxy', () => { +describe('https proxy (self-signed)', () => { const protocol = 'https' let proxyServer, apiServer const portNotInUse = 3009 + const selfSigned = true describe('no auth', () => { beforeAll(async () => { - proxyServer = await createHttpsProxy() + proxyServer = await createHttpsProxy({ selfSigned }) apiServer = await createApiServer({ port: 3001, useSsl: true }) }) @@ -212,29 +213,48 @@ describe('https proxy', () => { const testUrl = `${protocol}://localhost:${apiServerAddress.port}/mirror?${queryString.stringify(queryObject)}` const proxyUrl = proxyServer.url - const proxyFetch = new ProxyFetch({ proxyUrl, rejectUnauthorized: false }) - const response = await proxyFetch.fetch(testUrl) + // IGNORE self-signed cert errors + { + const proxyFetch = new ProxyFetch({ proxyUrl, rejectUnauthorized: false }) + const response = await proxyFetch.fetch(testUrl) - const json = await response.json() - expect(json).toStrictEqual(queryObject) + const json = await response.json() + expect(json).toStrictEqual(queryObject) + } + // DO NOT ignore self-signed cert errors + { + const proxyFetch = new ProxyFetch({ proxyUrl, rejectUnauthorized: true }) + await expect(async () => { + await proxyFetch.fetch(testUrl) + }).rejects.toThrow('self-signed certificate in certificate chain') + } }) - test('failure', async () => { + test('failure (non-existent port)', async () => { // connect to non-existent server port const testUrl = `${protocol}://localhost:${portNotInUse}/mirror/?foo=bar` - const proxyUrl = proxyServer.url - const proxyFetch = new ProxyFetch({ proxyUrl, rejectUnauthorized: false }) - const response = await proxyFetch.fetch(testUrl) - expect(response.ok).toEqual(false) - expect(response.status).toEqual(502) + // IGNORE self-signed cert errors + { + const proxyFetch = new ProxyFetch({ proxyUrl, rejectUnauthorized: false }) + const response = await proxyFetch.fetch(testUrl) + expect(response.ok).toEqual(false) + expect(response.status).toEqual(502) + } + // DO NOT ignore self-signed cert errors + { + const proxyFetch = new ProxyFetch({ proxyUrl, rejectUnauthorized: true }) + await expect(async () => { + await proxyFetch.fetch(testUrl) + }).rejects.toThrow('self-signed certificate in certificate chain') + } }) }) describe('basic auth', () => { beforeAll(async () => { - proxyServer = await createHttpsProxy({ useBasicAuth: true }) + proxyServer = await createHttpsProxy({ useBasicAuth: true, selfSigned }) apiServer = await createApiServer({ port: 3001, useSsl: true }) }) @@ -253,17 +273,28 @@ describe('https proxy', () => { 'Proxy-Authorization': 'Basic ' + Buffer.from(`${username}:${password}`).toString('base64') } const proxyUrl = proxyServer.url - const proxyFetch = new ProxyFetch({ proxyUrl, username, password, rejectUnauthorized: false }) - const testUrl = `${protocol}://localhost:${apiServerPort}/mirror?${queryString.stringify(queryObject)}` - const response = await proxyFetch.fetch(testUrl, { headers }) - const spy = jest.spyOn(proxyFetch, 'fetch').mockImplementation(() => testUrl) - const pattern = /\b^https\b/ - expect(proxyFetch.fetch()).toMatch(new RegExp(pattern)) - spy.mockRestore() - expect(response.ok).toEqual(true) - const json = await response.json() - expect(json).toStrictEqual(queryObject) + // IGNORE self-signed cert errors + { + const proxyFetch = new ProxyFetch({ proxyUrl, username, password, rejectUnauthorized: false }) + const response = await proxyFetch.fetch(testUrl, { headers }) + + const spy = jest.spyOn(proxyFetch, 'fetch').mockImplementation(() => testUrl) + const pattern = /\b^https\b/ + expect(proxyFetch.fetch()).toMatch(new RegExp(pattern)) + spy.mockRestore() + + expect(response.ok).toEqual(true) + const json = await response.json() + expect(json).toStrictEqual(queryObject) + } + // DO NOT ignore self-signed cert errors + { + const proxyFetch = new ProxyFetch({ proxyUrl, rejectUnauthorized: true }) + await expect(async () => { + await proxyFetch.fetch(testUrl) + }).rejects.toThrow('self-signed certificate in certificate chain') + } }) test('failure', async () => { @@ -276,22 +307,33 @@ describe('https proxy', () => { 'Proxy-Authorization': 'Basic ' + Buffer.from(`${username}:${password}`).toString('base64') } const proxyUrl = proxyServer.url - const proxyFetch = new ProxyFetch({ proxyUrl, username, password, rejectUnauthorized: false }) - const testUrl = `${protocol}://localhost:${apiServerPort}/mirror?${queryString.stringify(queryObject)}` - const response = await proxyFetch.fetch(testUrl, { headers }) - const spy = jest.spyOn(proxyFetch, 'fetch').mockImplementation(() => testUrl) - const pattern = /\b^http\b/ - expect(proxyFetch.fetch()).not.toMatch(new RegExp(pattern)) - spy.mockRestore() - expect(response.ok).toEqual(false) - expect(response.status).toEqual(403) + // IGNORE self-signed cert errors + { + const proxyFetch = new ProxyFetch({ proxyUrl, username, password, rejectUnauthorized: false }) + const response = await proxyFetch.fetch(testUrl, { headers }) + + const spy = jest.spyOn(proxyFetch, 'fetch').mockImplementation(() => testUrl) + const pattern = /\b^http\b/ + expect(proxyFetch.fetch()).not.toMatch(new RegExp(pattern)) + spy.mockRestore() + + expect(response.ok).toEqual(false) + expect(response.status).toEqual(403) + } + // DO NOT ignore self-signed cert errors + { + const proxyFetch = new ProxyFetch({ proxyUrl, rejectUnauthorized: true }) + await expect(async () => { + await proxyFetch.fetch(testUrl) + }).rejects.toThrow('self-signed certificate in certificate chain') + } }) }) describe('HttpExponentialBackoff', () => { beforeAll(async () => { - proxyServer = await createHttpsProxy() + proxyServer = await createHttpsProxy({ selfSigned }) apiServer = await createApiServer({ port: 3001, useSsl: true }) }) @@ -306,27 +348,45 @@ describe('https proxy', () => { const testUrl = `${protocol}://localhost:${apiServerPort}/mirror?${queryString.stringify(queryObject)}` const proxyUrl = proxyServer.url - const fetchRetry = new HttpExponentialBackoff() - const response = await fetchRetry.exponentialBackoff(testUrl, { method: 'GET' }, { - proxy: { proxyUrl, rejectUnauthorized: false } - }) - const json = await response.json() - expect(json).toStrictEqual(queryObject) + + // IGNORE self-signed cert errors + { + const response = await fetchRetry.exponentialBackoff(testUrl, { method: 'GET' }, { + proxy: { proxyUrl, rejectUnauthorized: false } + }) + const json = await response.json() + expect(json).toStrictEqual(queryObject) + } + // DO NOT ignore self-signed cert errors + await expect(async () => { + return fetchRetry.exponentialBackoff(testUrl, { method: 'GET' }, { + proxy: { proxyUrl, rejectUnauthorized: true } + }) + }).rejects.toThrow('self-signed certificate in certificate chain') }) test('failure', async () => { // connect to non-existent server port const testUrl = `${protocol}://localhost:3009/mirror/?foo=bar` const proxyUrl = proxyServer.url - const fetchRetry = new HttpExponentialBackoff() - const response = await fetchRetry.exponentialBackoff(testUrl, { method: 'GET' }, { - proxy: { proxyUrl, rejectUnauthorized: false }, - maxRetries: 2 - }, [], 0) // retryDelay must be zero for test timings - expect(response.ok).toEqual(false) - expect(response.status).toEqual(502) + + // IGNORE self-signed cert errors + { + const response = await fetchRetry.exponentialBackoff(testUrl, { method: 'GET' }, { + proxy: { proxyUrl, rejectUnauthorized: false }, + maxRetries: 2 + }, [], 0) // retryDelay must be zero for test timings + expect(response.ok).toEqual(false) + expect(response.status).toEqual(502) + } + // DO NOT ignore self-signed cert errors + await expect(async () => { + return fetchRetry.exponentialBackoff(testUrl, { method: 'GET' }, { + proxy: { proxyUrl, rejectUnauthorized: true } + }) + }).rejects.toThrow('self-signed certificate in certificate chain') }) }) }) From 7a180fcb774d9ce0cac625028c318d64f831dfd1 Mon Sep 17 00:00:00 2001 From: Shazron Abdullah Date: Wed, 13 Nov 2024 15:35:04 +0800 Subject: [PATCH 2/3] temp: update @adobe/aio-lib-test-proxy to PR branch version. To be reverted when the final release is out. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 2569d29..58c30d0 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "deprecated": false, "description": "Adobe I/O Lib Core Networking", "devDependencies": { - "@adobe/aio-lib-test-proxy": "^1.0.0", + "@adobe/aio-lib-test-proxy": "github:adobe/aio-lib-test-proxy#story/ACNA-3241", "@adobe/eslint-config-aio-lib-config": "^2.0.1", "babel-runtime": "^6.26.0", "dotenv": "^16.3.1", From 396a7afe450c2cbd23eaa6fb631d1ea9b021b5f9 Mon Sep 17 00:00:00 2001 From: Shazron Abdullah Date: Wed, 13 Nov 2024 16:25:30 +0800 Subject: [PATCH 3/3] fix: update http-proxy-agent and https-proxy-agent to v7.x. Patch HttpsProxyAgent. --- package.json | 4 ++-- src/ProxyFetch.js | 30 ++++++++++++++++++++++-------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index 58c30d0..4938788 100644 --- a/package.json +++ b/package.json @@ -14,8 +14,8 @@ "@adobe/aio-lib-core-errors": "^4.0.0", "@adobe/aio-lib-core-logging": "^3.0.0", "fetch-retry": "^5.0.4", - "http-proxy-agent": "^4.0.1", - "https-proxy-agent": "2.2.4", + "http-proxy-agent": "^7", + "https-proxy-agent": "^7", "node-fetch": "^2.6.4", "proxy-from-env": "^1.1.0" }, diff --git a/src/ProxyFetch.js b/src/ProxyFetch.js index 780816d..192ad72 100644 --- a/src/ProxyFetch.js +++ b/src/ProxyFetch.js @@ -13,12 +13,30 @@ const loggerNamespace = '@adobe/aio-lib-core-networking:ProxyFetch' const logger = require('@adobe/aio-lib-core-logging')(loggerNamespace, { level: process.env.LOG_LEVEL }) const originalFetch = require('node-fetch') const { codes } = require('./SDKErrors') -const HttpProxyAgent = require('http-proxy-agent') -const HttpsProxyAgent = require('https-proxy-agent') +const { HttpProxyAgent } = require('http-proxy-agent') +const { HttpsProxyAgent } = require('https-proxy-agent') const { urlToHttpOptions } = require('./utils') /* global Response, Request */ +/** + * HttpsProxyAgent needs a patch for TLS connections. + * It doesn't pass in the original options during a SSL connect. + * + * See https://github.com/TooTallNate/proxy-agents/issues/89 + * An alternative is to use https://github.com/delvedor/hpagent + */ +class PatchedHttpsProxyAgent extends HttpsProxyAgent { + constructor (proxyUrl, opts) { + super(proxyUrl, opts) + this.savedOpts = opts + } + + async connect (req, opts) { + return super.connect(req, { ...this.savedOpts, ...opts }) + } +} + /** * @private * @@ -35,19 +53,15 @@ function proxyAgent (resourceUrl, authOptions) { proxyOpts.auth = `${username}:${password}` } - // the passing on of this property to the underlying implementation only works on https-proxy-agent@2.2.4 - // this is only used for unit-tests and passed in the constructor proxyOpts.rejectUnauthorized = rejectUnauthorized if (rejectUnauthorized === false) { logger.warn(`proxyAgent - rejectUnauthorized is set to ${rejectUnauthorized}`) } - proxyOpts.ALPNProtocols = ['http/1.1'] - if (resourceUrl.startsWith('https')) { - return new HttpsProxyAgent(proxyOpts) + return new PatchedHttpsProxyAgent(proxyUrl, proxyOpts) } else { - return new HttpProxyAgent(proxyOpts) + return new HttpProxyAgent(proxyUrl, proxyOpts) } }