Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REVIEW ONLY] PP-438 Add node.js proxy module #57

Closed
wants to merge 1 commit into from

Conversation

ijmad
Copy link
Contributor

@ijmad ijmad commented Feb 10, 2016

Passport makes outgoing calls to Auth0 which will need to go via an egress proxy

…uth0 which will need to go via an egress proxy
@@ -1,4 +1,6 @@
if(process.env.ENABLE_NEWRELIC == 'yes') require('newrelic');
if (process.env.ENABLE_NEWRELIC == 'yes') require('newrelic');
if (process.env.HTTP_PROXY) require('global-tunnel').initialize();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need to pass in the proxy details to initialise() here?

@bazbremner
Copy link
Contributor

You may find https://github.com/alphagov/pay-ansible/pull/137 useful, specifically https://github.com/alphagov/pay-ansible/commit/7488942de7aa121e3570a67fb996d127ea2dcce3 which is written to be as friendly to https://www.npmjs.com/package/global-tunnel as I think is sensible to be.

Note: proto is http in the environment, but it looks like global-tunnel wants http:

@joshmyers
Copy link
Contributor

global-tunnel doesn't work with Node 0.12.X
See: salesforce/global-tunnel#7

I think https://github.com/TooTallNate/node-https-proxy-agent should do what we want?

@bazbremner
Copy link
Contributor

We only need to be using proxies to be getting to the Interwebs - if this affects all connections made by selfservice when doing HTTP/HTTPS requests, then this isn't going to work.

The egress proxies only pass traffic from the VPC to the internet - the security groups deliberately do not permit the proxies to pass traffic between all the different zones, so any HTTP/HTTPS clients must be either separately configurable with regards to proxies, or must be capable of honouring NO_PROXY or an equivalent list of IPs and domains that should not use the proxies.

This isn't (at least as far as I've observed) an issue for the connector, as it's at the bottom of the stack and so its only HTTP client interactions should be out to the gateways. For selfservice it's making connections back to publicauth and connector (which should be unproxied) and out to auth0 (which should be proxied).
Still, I'm not done proving that the connector does the right thing either, so watch this space.

@ijmad
Copy link
Contributor Author

ijmad commented Feb 12, 2016

Closing for now, we need to consider our approach here.

@ijmad ijmad closed this Feb 12, 2016
@ijmad ijmad deleted the PP-438-node-proxy branch July 15, 2016 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants