Repository: Unleash/unleash-client-java
Pull request #91 (2019-11-06 10:32:34)

feat: Add support for http proxies with basic authentication
I am not sure about all the reflection stuff in the test.

dennis-menge: Also one question regarding code style/formatting, do you have anything defined there?
coveralls: 
[![Coverage Status](https://coveralls.io/builds/26798343/badge)](https://coveralls.io/builds/26798343)

Coverage increased (+0.02%) to 93.637% when pulling **6667d0ca8254f0e52b177e9ac5b41ee0d6b03392 on dennis-menge:master** into **29ced6a7e75d52eac8005c95aaa322d79251340d on Unleash:master**.

ivarconr: > Also one question regarding code style/formatting, do you have anything defined there?

It seems like you changed the file from "4 spaces indentation" in to "2 space indentation" making it a bit hard to see what you actually changed. 

Could you mind fix that?
dennis-menge: Should be better now I hope :)
dennis-menge: Hi @ivarconr I removed those blanks in UnleashConfig

Patch:
diff --git a/README.md b/README.md
index 43ee2e0f..733eac51 100644
--- a/README.md
+++ b/README.md
@@ -173,6 +173,19 @@ UnleashConfig unleashConfig = UnleashConfig.builder()
 - **appName** - Required. Should be a unique name identifying the client application using Unleash. 
 - **synchronousFetchOnInitialisation** - Allows the user to specify that the unleash-client should do one synchronous fetch to the `unleash-api` at initialisation. This will slow down the initialisation (the client must wait for a http response). If the `unleash-api` is unavailable the client will silently move on and assume the api will be available later. 
 
+### HTTP Proxy with Authentication
+The Unleash Java client uses `HttpURLConnection` as HTTP Client which already recognizes the common JVM proxy settings such as `http.proxyHost` and
+`http.proxyPort`. So if you are using a Proxy without authentication, everything works out of the box. However, if you have to use Basic Auth
+authentication with your proxy, the related settings such as `http.proxyUser` and `http.proxyPassword` do not get recognized by default. In order
+to enable support for basic auth against a http proxy, you can simply enable the following option on the configuration builder:
+
+```java
+UnleashConfig config = UnleashConfig.builder()
+    .appName("my-app")
+    .unleashAPI("http://unleash.org")
+    .enableProxyAuthenticationByJvmProperties()
+    .build();
+```
 
 ## Local backup
 By default unleash-client fetches the feature toggles from unleash-server every 10s, and stores the 
diff --git a/src/main/java/no/finn/unleash/util/UnleashConfig.java b/src/main/java/no/finn/unleash/util/UnleashConfig.java
index 2330f6f6..79d4aa53 100644
--- a/src/main/java/no/finn/unleash/util/UnleashConfig.java
+++ b/src/main/java/no/finn/unleash/util/UnleashConfig.java
@@ -1,19 +1,17 @@
 package no.finn.unleash.util;
 
+import no.finn.unleash.UnleashContextProvider;
+import no.finn.unleash.event.NoOpSubscriber;
+import no.finn.unleash.event.UnleashSubscriber;
+
 import java.io.File;
-import java.net.HttpURLConnection;
-import java.net.InetAddress;
-import java.net.URI;
-import java.net.UnknownHostException;
+import java.net.*;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
 
-import no.finn.unleash.UnleashContextProvider;
-import no.finn.unleash.event.NoOpSubscriber;
-import no.finn.unleash.event.UnleashSubscriber;
-
 public class UnleashConfig {
+
     static final String UNLEASH_APP_NAME_HEADER = "UNLEASH-APPNAME";
     static final String UNLEASH_INSTANCE_ID_HEADER = "UNLEASH-INSTANCEID";
 
@@ -28,6 +26,7 @@ public class UnleashConfig {
     private final long fetchTogglesInterval;
     private final long sendMetricsInterval;
     private final boolean disableMetrics;
+    private final boolean isProxyAuthenticationByJvmProperties;
     private final UnleashContextProvider contextProvider;
     private final boolean synchronousFetchOnInitialisation;
     private final UnleashScheduledExecutor unleashScheduledExecutor;
@@ -45,12 +44,12 @@ public class UnleashConfig {
             long sendMetricsInterval,
             boolean disableMetrics,
             UnleashContextProvider contextProvider,
+            boolean isProxyAuthenticationByJvmProperties,
             boolean synchronousFetchOnInitialisation,
             UnleashScheduledExecutor unleashScheduledExecutor,
             UnleashSubscriber unleashSubscriber
     ) {
 
-
         if(appName == null) {
             throw new IllegalStateException("You are required to specify the unleash appName");
         }
@@ -71,6 +70,10 @@ public class UnleashConfig {
             throw new IllegalStateException("You are required to specify a subscriber");
         }
 
+        if(isProxyAuthenticationByJvmProperties) {
+            enableProxyAuthentication();
+        }
+
         this.unleashAPI = unleashAPI;
         this.customHttpHeaders = customHttpHeaders;
         this.unleashURLs = new UnleashURLs(unleashAPI);
@@ -83,11 +86,28 @@ public class UnleashConfig {
         this.sendMetricsInterval = sendMetricsInterval;
         this.disableMetrics = disableMetrics;
         this.contextProvider = contextProvider;
+        this.isProxyAuthenticationByJvmProperties = isProxyAuthenticationByJvmProperties;
         this.synchronousFetchOnInitialisation = synchronousFetchOnInitialisation;
         this.unleashScheduledExecutor = unleashScheduledExecutor;
         this.unleashSubscriber = unleashSubscriber;
     }
 
+    public static Builder builder() {
+        return new Builder();
+    }
+
+    public static void setRequestProperties(HttpURLConnection connection, UnleashConfig config) {
+        connection.setRequestProperty(UNLEASH_APP_NAME_HEADER, config.getAppName());
+        connection.setRequestProperty(UNLEASH_INSTANCE_ID_HEADER, config.getInstanceId());
+        connection.setRequestProperty("User-Agent", config.getAppName());
+        config.getCustomHttpHeaders().forEach(connection::setRequestProperty);
+    }
+
+    private void enableProxyAuthentication() {
+        // http.proxyUser http.proxyPassword is only consumed by Apache HTTP Client, for HttpUrlConnection we have to define an Authenticator
+        Authenticator.setDefault(new ProxyAuthenticator());
+    }
+
     public URI getUnleashAPI() {
         return unleashAPI;
     }
@@ -124,10 +144,6 @@ public class UnleashConfig {
         return unleashURLs;
     }
 
-    public static Builder builder() {
-        return new Builder();
-    }
-
     public boolean isDisableMetrics() {
         return disableMetrics;
     }
@@ -152,14 +168,32 @@ public class UnleashConfig {
         return unleashSubscriber;
     }
 
-    public static void setRequestProperties(HttpURLConnection connection, UnleashConfig config) {
-        connection.setRequestProperty(UNLEASH_APP_NAME_HEADER, config.getAppName());
-        connection.setRequestProperty(UNLEASH_INSTANCE_ID_HEADER, config.getInstanceId());
-        connection.setRequestProperty("User-Agent", config.getAppName());
-        config.getCustomHttpHeaders().forEach(connection::setRequestProperty);
+    public boolean isProxyAuthenticationByJvmProperties() {
+        return isProxyAuthenticationByJvmProperties;
+    }
+
+    static class ProxyAuthenticator extends Authenticator {
+
+        @Override
+        protected PasswordAuthentication getPasswordAuthentication() {
+            if(getRequestorType() == RequestorType.PROXY) {
+                final String proto = getRequestingProtocol().toLowerCase();
+                final String proxyHost = System.getProperty(proto + ".proxyHost", "");
+                final String proxyPort = System.getProperty(proto + ".proxyPort", "");
+                final String proxyUser = System.getProperty(proto + ".proxyUser", "");
+                final String proxyPassword = System.getProperty(proto + ".proxyPassword", "");
+
+                // Only apply PasswordAuthentication to requests to the proxy itself - if not set just ignore
+                if(getRequestingHost().equalsIgnoreCase(proxyHost) && Integer.parseInt(proxyPort) == getRequestingPort()) {
+                    return new PasswordAuthentication(proxyUser, proxyPassword.toCharArray());
+                }
+            }
+            return null;
+        }
     }
 
     public static class Builder {
+
         private URI unleashAPI;
         private Map<String, String> customHttpHeaders = new HashMap<>();
         private String appName;
@@ -174,6 +208,7 @@ public class UnleashConfig {
         private boolean synchronousFetchOnInitialisation = false;
         private UnleashScheduledExecutor scheduledExecutor;
         private UnleashSubscriber unleashSubscriber;
+        private boolean isProxyAuthenticationByJvmProperties;
 
         static String getDefaultInstanceId() {
             String hostName = "";
@@ -235,6 +270,11 @@ public class UnleashConfig {
             return this;
         }
 
+        public Builder enableProxyAuthenticationByJvmProperties() {
+            this.isProxyAuthenticationByJvmProperties = true;
+            return this;
+        }
+
         public Builder unleashContextProvider(UnleashContextProvider contextProvider) {
             this.contextProvider = contextProvider;
             return this;
@@ -277,6 +317,7 @@ public class UnleashConfig {
                     sendMetricsInterval,
                     disableMetrics,
                     contextProvider,
+                    isProxyAuthenticationByJvmProperties,
                     synchronousFetchOnInitialisation,
                     Optional.ofNullable(scheduledExecutor).orElseGet(UnleashScheduledExecutorImpl::getInstance),
                     Optional.ofNullable(unleashSubscriber).orElseGet(NoOpSubscriber::new)


Test patch:
diff --git a/src/test/java/no/finn/unleash/util/UnleashConfigTest.java b/src/test/java/no/finn/unleash/util/UnleashConfigTest.java
index c6c8deb3..ec0a3360 100644
--- a/src/test/java/no/finn/unleash/util/UnleashConfigTest.java
+++ b/src/test/java/no/finn/unleash/util/UnleashConfigTest.java
@@ -1,17 +1,19 @@
 package no.finn.unleash.util;
 
-import java.io.File;
-import java.io.IOException;
-import java.net.HttpURLConnection;
-import java.net.URI;
-import java.net.URL;
+import no.finn.unleash.util.UnleashConfig.ProxyAuthenticator;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.function.Executable;
 
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Field;
+import java.net.*;
+import java.net.Authenticator.RequestorType;
+
 import static no.finn.unleash.util.UnleashConfig.UNLEASH_APP_NAME_HEADER;
 import static no.finn.unleash.util.UnleashConfig.UNLEASH_INSTANCE_ID_HEADER;
-import static org.hamcrest.core.Is.is;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.Is.is;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class UnleashConfigTest {
@@ -145,7 +147,7 @@ public class UnleashConfigTest {
 
     @Test
     public void should_require_instanceId() {
-        Executable ex = () ->  UnleashConfig.builder()
+        Executable ex = () -> UnleashConfig.builder()
                 .appName("my-app")
                 .instanceId(null)
                 .unleashAPI("http://unleash.org")
@@ -153,4 +155,51 @@ public class UnleashConfigTest {
 
         assertThrows(IllegalStateException.class, ex);
     }
+
+    @Test
+    public void should_enable_proxy_based_on_jvm_settings()
+            throws IllegalAccessException, NoSuchFieldException {
+
+        String proxyHost = "proxy-host";
+        String proxyPort = "8080";
+        String proxyUser = "my-proxy-user";
+        String proxyPassword = "my-secret";
+
+        System.setProperty("http.proxyHost", proxyHost);
+        System.setProperty("http.proxyPort", proxyPort);
+        System.setProperty("http.proxyUser", proxyUser);
+        System.setProperty("http.proxyPassword", proxyPassword);
+
+        UnleashConfig config = UnleashConfig.builder()
+                .appName("my-app")
+                .unleashAPI("http://unleash.org")
+                .enableProxyAuthenticationByJvmProperties()
+                .build();
+
+        Field authenticator = Authenticator.class.getDeclaredField("theAuthenticator");
+        authenticator.setAccessible(true);
+        ProxyAuthenticator proxyAuthenticator = (ProxyAuthenticator) authenticator.get(null);
+
+        Field requestingAuthType = proxyAuthenticator.getClass().getSuperclass().getDeclaredField("requestingAuthType");
+        requestingAuthType.setAccessible(true);
+        requestingAuthType.set(proxyAuthenticator, RequestorType.PROXY);
+
+        Field requestingProtocol = proxyAuthenticator.getClass().getSuperclass().getDeclaredField("requestingProtocol");
+        requestingProtocol.setAccessible(true);
+        requestingProtocol.set(proxyAuthenticator, "http");
+
+        Field requestingHost = proxyAuthenticator.getClass().getSuperclass().getDeclaredField("requestingHost");
+        requestingHost.setAccessible(true);
+        requestingHost.set(proxyAuthenticator, proxyHost);
+
+        Field requestingPort = proxyAuthenticator.getClass().getSuperclass().getDeclaredField("requestingPort");
+        requestingPort.setAccessible(true);
+        requestingPort.set(proxyAuthenticator, 8080);
+
+        PasswordAuthentication passwordAuthentication = proxyAuthenticator.getPasswordAuthentication();
+
+        assertThat(passwordAuthentication.getUserName(), is(proxyUser));
+        assertThat(new String(passwordAuthentication.getPassword()), is(proxyPassword));
+        assertThat(config.isProxyAuthenticationByJvmProperties(), is(true));
+    }
 }