Merge pull request #18 from adobe/bugfix/ACNA-1682

fix(breaking): ACNA-1682 - HTTP/HTTPS proxy agent logic

Author: amulyakashyap09

.gitignore

@@ -3,6 +3,7 @@
 coverage
 launch.json
 package-lock.json
+yarn.lock
 /node_modules/
 junit.xml
 test-report.html

src/ProxyFetch.js

@@ -16,10 +16,39 @@ const { codes } = require('./SDKErrors')
 const HttpProxyAgent = require('http-proxy-agent')
 const HttpsProxyAgent = require('https-proxy-agent')
 const { urlToHttpOptions } = require('./utils')
-const http = require('http')
 
 /* global Response, Request */
 
+/**
+ * @private
+ *
+ * @param {string} resourceUrl an endpoint url for proxyAgent selection
+ * @param {object} authOptions an object which contains auth information
+ * @returns {http.Agent} a http.Agent for basic auth proxy
+ */
+function proxyAgent (resourceUrl, authOptions) {
+  const { proxyUrl, username, password, rejectUnauthorized = true } = authOptions
+  const proxyOpts = urlToHttpOptions(proxyUrl)
+
+  if (!proxyOpts.auth && username && password) {
+    logger.debug('username and password not set in proxy url, using credentials passed in the constructor.')
+    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}`)
+  }
+
+  if (resourceUrl.startsWith('https')) {
+    return new HttpsProxyAgent(proxyOpts)
+  } else {
+    return new HttpProxyAgent(proxyOpts)
+  }
+}
+
 /**
  * Proxy Auth Options
  *
@@ -55,34 +84,6 @@ class ProxyFetch {
     return this
   }
 
-  /**
-   * Returns the http.Agent used for this proxy
-   *
-   * @returns {http.Agent} a http.Agent for basic auth proxy
-   */
-  proxyAgent () {
-    const { proxyUrl, username, password, rejectUnauthorized = true } = this.authOptions
-    const proxyOpts = urlToHttpOptions(proxyUrl)
-
-    if (!proxyOpts.auth && username && password) {
-      logger.debug('username and password not set in proxy url, using credentials passed in the constructor.')
-      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}`)
-    }
-
-    if (proxyOpts.protocol.startsWith('https')) {
-      return new HttpsProxyAgent(proxyOpts)
-    } else {
-      return new HttpProxyAgent(proxyOpts)
-    }
-  }
-
   /**
    * Fetch function, using the configured NTLM Auth options.
    *
@@ -93,7 +94,7 @@ class ProxyFetch {
   async fetch (resource, options = {}) {
     return originalFetch(resource, {
       ...options,
-      agent: this.proxyAgent()
+      agent: proxyAgent(resource, this.authOptions)
     })
   }
 }

test/proxy.test.js

@@ -110,6 +110,10 @@ describe('http proxy', () => {
 
       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()).toMatch(new RegExp(pattern))
+      spy.mockRestore()
       expect(response.ok).toEqual(true)
       const json = await response.json()
       expect(json).toStrictEqual(queryObject)
@@ -134,6 +138,10 @@ describe('http proxy', () => {
 
       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()).not.toMatch(new RegExp(pattern))
+      spy.mockRestore()
       expect(response.ok).toEqual(false)
       expect(response.status).toEqual(403)
     })
@@ -249,6 +257,10 @@ describe('https proxy', () => {
 
       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)
@@ -268,6 +280,10 @@ describe('https proxy', () => {
 
       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)
     })