-
Notifications
You must be signed in to change notification settings - Fork 911
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
Adding HTTP(S) over HTTP(S) proxy support to add accounts. #26137
Conversation
This'll be huge if this fixes the proxy authentication issues! Can you update the description with information about how you've validated that this fixes the proxy issues? |
@lewis-sanchez it looks like the CI build is failing. Do you mind taking a look and getting that clean? I doesn't look related to your change through. |
@lewis-sanchez how are you testing this? Specifically, are we confident this won't introduce side effect in any other authentication use-cases? |
@kburtram, I tested 3 scenarios. Scenario 1 - Using a local proxyThe first scenario I tested was by setting up a local proxy with a tool named ProxyMan: I configured Azure Data Studio to use the address provided by ProxyMan: And that allowed me to see traffic going through the local proxy and to add an account: Scenario 2 - Invalid proxy endpointThe second scenario I tested, was setting the proxy address used by Azure Data Studio to an invalid one. Using an invalid proxy address resulted in the login page hanging and Azure Data Studio to show the following error message: Scenario 3 - Not using a proxyThe third scenario I tested was to not provide Azure Data Studio with a proxy endpoint: Going through the process of adding an account to make sure that there aren't any regressions being introduced and successfully doing that: |
* Adjust axios proxy configuration * Add logging around axios get request logic * Minor clean up * Fix conditional check * Load environment proxy value * Fix compilation error * Review changes * Add additional logging to tunnel logic with axios * Remove unused import * Set proxy false when proxy endpoint found * Log when proxy endpoint is not found * Improve log wording * Prioritize workspace HTTP config before environment variables
…26162) * Adjust axios proxy configuration * Add logging around axios get request logic * Minor clean up * Fix conditional check * Load environment proxy value * Fix compilation error * Review changes * Add additional logging to tunnel logic with axios * Remove unused import * Set proxy false when proxy endpoint found * Log when proxy endpoint is not found * Improve log wording * Prioritize workspace HTTP config before environment variables
This PR fixes #25990,
The PR configures Axios, an HTTP client that Azure Data Studio has a dependency on, to successfully make requests from a proxied environment. In cases where an HTTP proxy is used, the request to get tenants is unable to go through when a proxy only supports HTTP traffic. This can result in the request to hang and prevent completion. To avoid hanging an HTTPS request from a HTTP proxy, an HTTPS over HTTP tunnel is needed, which is what this PR adds.
I validated my changes, by having Azure Data Studio connect to the proxy endpoint set up by ProxyMan, and reviewed the following resources to identify that when Axios makes requests in proxy environments, it can result in errors that stop the request from completing.
https://janmolak.com/node-js-axios-behind-corporate-proxies-8b17a6f31f9d
axios/axios#925
axios/axios#3384
https://stackoverflow.com/questions/43433380/socket-hang-up-when-using-axios-get-but-not-when-using-https-get