auth0 / node-jwks-rsa

A library to retrieve RSA public keys from a JWKS (JSON Web Key Set) endpoint.
MIT License
837 stars 236 forks source link

http proxy error after upgrading from 1.6.0 to 1.8.0 #144

Closed shnplr closed 4 years ago

shnplr commented 4 years ago

This library no longer works behind corporate proxy. Our proxy scheme is 'http://:80' for both http & https.

{
jwksUri: 'https://server.com/.well-known/jwks.json',
proxy: 'http://@proxy.example.com:80'
}

Current workaround is to use "1.7.0"

Note: A minor issue with "@" needed to prevent NPE request.js does not check proxy.auth for null.

lubomir-haralampiev commented 4 years ago

I'm using this great module too and experiencing the same problem, which ended with pinning to 1.7.0.

Likely this is because a https request needs to go through the proxy only working with http. This is a reatively old problem axios users are experiencing: https://github.com/axios/axios/issues/925 https://github.com/axios/axios/issues/2072#issuecomment-609650888

As there are working workarounds suggested, I'm not sure when/if this be solved on axios side.

Maybe it could be an option, to provide the possibility to inject a complete axios instance, when initializing the jwks-rsa Client?

tcvv commented 4 years ago

We had this problem and we workaround it by overwritting the getKeys function of the JwksClient object instance we're using (I suppose you can also overwrite the prototype one). Since the function only performs the network request, you can perform it using a library of your choice. Not sure how to fix it at a library level though, unless we could somehow (optionally) inject our own prepared request object, which fullfills the same api as the existing request.js wrapper, when creating the client.

metamono commented 4 years ago

Seems like we have the same problem #142

It's about time to do something here!!!

jimmyjames commented 4 years ago

This is likely related to migrating from the deprecated request module to axios, introduced in 1.8. As discussed above, the current workaround is to use 1.7 if you are impacted by this, while we work on a fix. Apologies for the unintended bug introduced here!

jimmyjames commented 4 years ago

It appears this is related to https://github.com/axios/axios/issues/925 when using an HTTP proxy to an HTTPS endpoint.

The common recommended fix is to configure an httpsAgent to handle the proxy, either using https-proxy-agent or tunnel. Because we do allow customization of the http(s) agent via requestAgentOptions, we’ll need to be careful how we can still support that agent customization while also supporting proxy (also, it appears that https-proxy-agent doesn't support some common agent features, such as keepAlive). If we can’t get a fix into axios itself, perhaps we could instead allow agent customization by accepting an Agent instance, instead of options that we use to construct the agent. This way those who need to configure this library with a proxy could do something like:

const client = jwksClient({
  jwksUri: 'https://YOUR-DOMAIN/.well-known/jwks.json',
  httpsAgent: new HttpsProxyAgent({
    host: '127.0.0.1', 
    port: '9090', 
    auth: 'username:password'
  })
});

We’d then need to deprecate the current proxy and requestAgentOptions and document how to configure an agent for proxy support.

As mentioned earlier, if you are using this library behind a proxy, it’s recommended to stay on v1.7 until we either (hopefully) get a fix on the Axios side or implement a workaround, perhaps as described above.

ghislaincote commented 4 years ago

Hi, I DID NOT find the current thread in time... and ran into axios/axios#2072 (comment)

So, I tried to fix it myself, over v 1.9, with a similar solution as above (using 'https-proxy-agent'). As a result, the code for the request wrapper would be simplified (the agent takes the proxy string as a whole)... but I ignore the possible consequences on non-https proxy traffic. We would have to test.

FYI, it works in our environnement. We are using a 'http' proxy, to get our jwks objects from a 'https://domain/path' url.

//THIS IS MY WHOLE src/wrappers/request.js REPLACEMENT FILE !

import http from 'http';
import https from 'https';
import httpsProxyAgent from 'https-proxy-agent';
import { request } from 'axios';

export default function(options, cb) {
  const requestOptions = {
    url: options.uri,
    headers: options.headers,
    timeout: options.timeout
  };

  if (options.proxy) {
    //Needed because of limitation of Axios with https traffic tunnelled into http corporate proxies
    //See : https://github.com/axios/axios/issues/2072
    requestOptions.proxy = false; //Oddly, this must be false, since we are using the agent
    requestOptions.httpsAgent = new httpsProxyAgent(options.proxy);
  } 

  if (options.agentOptions || options.strictSSL != undefined) {
    const agentOptions = {
      ...(options.strictSSL != undefined) && { rejectUnauthorized: options.strictSSL },
      ...options.agentOptions
    };
    requestOptions.httpAgent = new http.Agent(agentOptions);
    requestOptions.httpsAgent = new https.Agent(agentOptions);
  }

  request(requestOptions)
    .then(response => cb(null, response))
    .catch(err => cb(err));
};

What do you think ?

I am not a contributor to this project, and have no Idea how to be. If you tell me how, I will submit the code above, after required tests (again, tell me which), of course.

In the meantime, as per @jimmyjames suggestion, we will most probably switch to 1.7, to fix an issue we have in production.

Thanks !

catchin commented 4 years ago

I have a related problem with the new 1.10.0 version. As now it seems 'https-proxy-agent' is pulled in as a dependency, but in the package.json it is only a devDependency. Therefore, I get the following error:

Error: Cannot find module 'https-proxy-agent'
Require stack:
- .../node_modules/jwks-rsa/lib/wrappers/request.js
- .../node_modules/jwks-rsa/lib/JwksClient.js
- .../node_modules/jwks-rsa/lib/index.js
...
lubomir-haralampiev commented 4 years ago

Thanks for working on this issue.
We are still missing one aspect. Until 1.7 it was possible to define the proxy configuration in the environment variables HTTP_PROXY, HTTPS_PROXY and NO_PROXY. The current solution doesn't interpret the environment and the implemented fix doesn't apply.

I implemented a solution in https://github.com/auth0/node-jwks-rsa/pull/188