-
Notifications
You must be signed in to change notification settings - Fork 196
Fetch: added forward proxy support with HTTPS tunneling. #976
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
base: master
Are you sure you want to change the base?
Conversation
438861e
to
fb71984
Compare
3009f25
to
8ee857b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically looks good, except for few comments.
May be it would be good discuss solution with Ivan, because it is bit different from #956
Supports Basic authentication via Proxy-Authorization header. - js_fetch_proxy - configures forward proxy URL example.conf: ... http { js_import main.js; server { listen 8080; resolver 127.0.0.1:5353; location /api { js_fetch_proxy http://user:[email protected]:3128; js_content main.fetch_handler; } } } This implements nginx#956 feature request on Github.
} | ||
location /http_via_proxy_encoded { | ||
js_fetch_proxy http://user%40domain:p%40ss%[email protected]:%%PORT_8081%%; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be better avoid using url format with username and password because it is obsolete and looks confusing for configuration (there is not very obvious set of chars to be url encoded by admin), can we replace it by 2 clear separated strings for username and password?
Something like:
js_fetch_proxy http://127.0.0.1:%%PORT_8081%% "user_name" "password"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@route443 Hi Ivan,
Do you have any opinion/preference/experience here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @xeioex , I can’t say I prefer the http://ip:port user pass
format over http://user:pass@ip:port
. The latter is still widely used, and encoding special chars isn’t much of a problem. It became obsolete mainly because creds could end up in logs and were considered insecure, however, that’s not really relevant in our case. In my opinion, the lack of support for nginx complex values is actually a bigger limitation - it would eliminate this concern altogether and add more flexibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, the lack of support for nginx complex values is actually a bigger limitation - it would eliminate this concern altogether and add more flexibility.
you would like to see a complex value for js_fetch_proxy
argument?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Undoubtedly, that would provide additional flexibility both in terms of creds management and proxy selection.
For example, if you look at the current OIDC approach, endpoint selection is handled through a map
. It'd be convenient to manage proxy configs the same way.
Unfortunately, with the current approach, we’d still need to explicitly enable proxy support (by uncommenting the corresponding line for openid_connect.server_conf), but it would allow having different proxies/creds for different, say, hosts.
As I suggested in the #956, the greatest flexibility would come from implementing it via ngx.fetch(), though as I understand it, that would be more complex to implement and less transparent for the end admin.
It would also be nice to be able to dynamically enable or disable proxies altogether…
That said, even the current implementation will cover 99% of use cases, since I doubt that:
- anyone uses more than one proxy for the same purpose, or
- anyone intends to split token set retrieval requests between proxy and direct modes.
And even in that case, workarounds could be found.
This patch adds forward proxy support to the ngx.fetch() method in njs, enabling HTTP and HTTPS requests through a proxy server with optional
authentication.
Key Features:
New Directives:
Example nginx.conf:
JavaScript usage: