Automatically set Git proxy environment variables from system configuration#9154
Automatically set Git proxy environment variables from system configuration#9154outofambit merged 79 commits intodevelopmentfrom
Conversation
We want to introduce more arguments up in here so lets get rid of the early returns
\t is technically valid in PAC strings
Technically the resetting isn't necessary but it doesn't cost us anything and both setCreateTutorialRepositoryProps and _closePopup are noops if the popup has already disappeared
Let's cross this bridge when we have to This reverts commit c16afcc.
Chromium ensures we get compliant strings
This caused a "Pulling [Object object]" progress text
|
@niik I have set up Fiddler on OSX, the traffic is being captured, the Network settings were auto-updated, the |
You shouldn't have to set that variable at all, please unset it using
Wait, are you saying you're seeing the "The revocation function was unable to check revocation for the certificate" error on macOS? That shouldn't be possible. Could you provide a screenshot of the error? Could you also please provide the output of running |
|
I removed the git config settings and here is the output of This is the error I am seeing: Note: Fiddler renders my wifi useless as it changes my proxy settings. Not sure if this is part of the problem. Steps I took to setup:
|
So this error is not the same as what I was talking about above, i.e. "The revocation function was unable to check revocation for the certificate". The reason you're seeing this error is because you haven't trusted the fiddler root certificate yet, see https://docs.telerik.com/fiddler-everywhere/user-guide/settings/https/https-decryption and https://docs.telerik.com/fiddler-everywhere/knowledge-base/how-to-install-fiddler-root-certificate-on-mac-os.
This will also be solved once you trust the certificate. |
|
Perfect, thanks! The requests are coming through now on OSX upon fetching:
Will test Windows once the revocation function PR is implemented. |
outofambit
left a comment
There was a problem hiding this comment.
one question for @niik but looks good!
See #9126 #9127, and #9139 for the work leading up to this PR.
Description
This is the final piece for supporting automatic Git proxy configuration for (non-authenticating proxies).
When GitHub Desktop is about to perform a network operation (such as push, pull, fetch etc) we're now going to automatically add a
https_proxyorhttp_proxyenvironment variable with the proxy url that we've retrieved and reformatted for cURL use.We first inspect the environment to see if the user has manually configured any of the relevant proxy environment variables. If they haven't we'll set them ourselves.
Why env vars and not Git configuration variables?
We have two options when it comes to providing Git with per-invocation configuration. Either we set an environment variable that Git understands or we pass a Git configuration key and value using the
git -cparameter.Using the
git -c http.proxy=fooapproach would override anyhttp.proxysetting the user might have configured in their global (and/or repo level) git config. More importantly though the-capproach wouldn't be inherited by subcommands executed by Git such as Git LFS.What about proxies requiring authentication
That'd be the next logical step. While the non-Git bits of GitHub Desktop supports non-authenticating proxies today it doesn't support authenticating proxies meaning that users wouldn't be able to sign in to their accounts or access the GitHub API. We need to start at that end and implement the login event, provide Electron with credentials (taken from keychain/credential manager by default and prompted if no such credentials could be found).
The current workaround of manually configuring Git proxy details described in #2789 (comment) would work for the Git bits but would do nothing to fix the API portion.
Testing
I don't have a definitive guide here but this is what I've been doing.
Install Fiddler Everywhere and turn on capturing. Fiddler will automatically configure the system proxy settings to point to its own proxy port.
Optionally enable HTTPS-decryption if you'd like to inspect to the requests.
Do a fetch in GitHub Desktop. You should see a request being made from Git
If you just want to verify that GitHub Desktop is setting the environment variables correctly you may also specify an invalid proxy url in your system configuration and verify that the fetch doesn't go through.
If you want to verify that Electron is receiving the proxy configuration from the operating system you can copy and paste the following command into the DevTools console
HELP I'm getting an error saying "The revocation function was unable to check revocation for the certificate."
Look at that, you're testing on Windows. Good on ya! See https://github.com/desktop/desktop/blob/development/docs/known-issues.md#certificate-revocation-check-fails---3326. Man-in-the-middle intercepting SSL proxy (such as fiddler or corporate proxies that decrypts https shudders) usually don't implement the necessary logic to support certificate revocation checks. You can set
http.schannelCheckRevoketo false in order to test this but this is obviously something we're going to want to have a better solution for. That will be my next step and I'm hoping to have a PR to show in the beginning of next week.