Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Relay the Java networking system properties to sub-processes to support proxy servers #17

Merged
merged 10 commits into from
Mar 13, 2016

Conversation

olivierdagenais
Copy link
Contributor

Summary

Without these changes, a program using this component would open a web browser that would not follow the same proxy server configuration as the program and thus would be unable to complete the OAuth 2.0 Authorization Code Flow when access to the authorization endpoint needed to be performed through a proxy server. This is because oauth2-useragent uses sub-processes to launch web browsers and the network-related configuration of the parent process was not propagated to the child processes.

This pull request propagates the values of networking-related system properties to the sub-processes such that if the main program can reach resources through a proxy server, so will the web browsers used for OAuth 2.0 flows.

Manual testing

  1. Set up a proxy server on the LAN.
    • I installed Polipo on 192.168.0.117. It's listening on all interfaces at port 8123.
  2. Temporarily update the Git Credential Manager for Mac and Linux's pom.xml to point to the SNAPSHOT version of oauth2-useragent containing these changes.
  3. Configure a VSTS account with 2-factor authentication.
  4. For each of Windows 10, Mac OS X 10.10.5 and Fedora 22:
    1. Copy the SNAPSHOT build of GCM4ML containing these oauth2-useragent changes and configure Git to point to that version, enabling debug mode by setting -Ddebug=true.

    2. Configure the networking to prevent being able to reach web servers without going through a proxy server.

      • I did this by temporarily configuring the DHCP reservation to clear the 003 Router option and then renewing the lease.
      • Running wget yields Network is unreachable or No route to host.
    3. Clear or invalidate the saved credentials (if any) for the VSTS account, to force authentication.

      • Windows & Linux: I did this by renaming insecureStore.xml to insecureStore.xml.old.
      • Mac: I did this by deleting the corresponding Keychain entry.
    4. Run a git clone against a Git repo in the VSTS account.

      • Git fails saying it can't reach the host.
    5. Configure Git to use our proxy server by running:

      git config --global http.proxy http://192.168.0.117:8123
      
    6. Try the git clone again.

      • The GCM4ML emits the following:

        BaseVsoAuthentication::detectAuthority
            detected visualstudio.com, checking AAD vs MSA
            failed detection
            authority is basic
        

        ...which means it's unable to reach VSTS and so Git falls back to prompting for credentials.

    7. Update the credential.helper configuration to include the following:

      -Dhttps.proxyHost=192.168.0.117 -Dhttps.proxyPort=8123
      
    8. Try the git clone again.

      • Notice that this time, the GCM4ML "AAD vs MSA" check succeeds with server has reponded and the web browser is popped up to complete the authentication. ✅
    9. Complete the authentication and notice the git clone succeeding.

    10. Windows & Linux (Oracle documents this setting as only being available "[on] recent Windows systems and on Gnome 2.x systems")

      1. Update the credential.helper configuration to replace the two https.proxy* properties with the following:

        -Djava.net.useSystemProxies=true
        
      2. Configure the proxy server for the operating system.

      3. Clear or invalidate the saved credentials for the VSTS account, to force authentication.

      4. Run git fetch inside the local copy of the Git repository.

        • Notice that the GCM4ML "AAD vs MSA" check succeeds again, the web browser again is popped up and, after completing the authentication, Git indeed communicates with the remote. ✅
      5. Clear the proxy server configuration for the operating system.

    11. Undo the Git http.proxy configuration.

    12. Undo the networking configuration, so as to restore direct access to web servers.

Mission accomplished!

Oli Dagenais added 3 commits March 9, 2016 16:26
This will make available to the sub-processes the same properties that
were defined in the parent process.

Assert.assertEquals(1, commands.size());
Assert.assertEquals("-Dhttp.proxyHost=192.0.2.42", commands.get(0));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps it's worth adding a test for all supported properties to catch any changes to the supported set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I understand the suggestion. Do you mean we add a copy of the contents of NETWORKING_PROPERTY_NAMES in a unit test that asserts that NETWORKING_PROPERTY_NAMES is equal to the copy?

I think I'd rather craft an integration test that uses org.littleshoot:littleproxy:1.1.0-beta1 like the test I added in tfs-plugin that sets up a proxy server, connects to a remote host and then asserts that the proxy server was used. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

craft an integration test that uses org.littleshoot:littleproxy:1.1.0-beta1 like the test I added in tfs-plugin that sets up a proxy server, connects to a remote host and then asserts that the proxy server was used

I'll be implementing this; please hold off on merging until I follow up.

Thanks!

  • Oli

Oli Dagenais added 6 commits March 10, 2016 12:59
This avoids the chance of conflict if a workstation already had 8089 or
9443 bound to something else (which I did).
I originally wrote this class for jenkinsci/tfs-plugin and I need it
pretty much as-is for this project, as well.
The browser pop-up should connect through littleproxy to reach WireMock,
which is verified via proxyWasUsed().
@olivierdagenais
Copy link
Contributor Author

I was able to add two positive tests (they assert that the proxy server was indeed used) and I couldn't get a negative test going (configures the proxy, but turns it off, which should cause requests to fail), maybe because the JavaFX browser decides to try connecting directly if the proxy server isn't responding.

olivierdagenais added a commit that referenced this pull request Mar 13, 2016
Add support for unauthenticated proxy servers

This is done by relaying the Java networking system properties to sub-processes created for launching the web browsers.
@olivierdagenais olivierdagenais merged commit 3ef49b9 into master Mar 13, 2016
@olivierdagenais olivierdagenais deleted the relay_proxy_properties branch March 13, 2016 23:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants