ZOOKEEPER-4860: Disable X-Forwarded-For by default

Reviewers: eolivelli, purushah
Author: anmolnar
Closes #2187 from anmolnar/ZOOKEEPER-4860

zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md

@@ -1572,14 +1572,15 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp
         - 1. Regenerate `superDigest` when migrating to new algorithm.
         - 2. `SetAcl` for a znode which already had a digest auth of old algorithm.
 
-* *IPAuthenticationProvider.skipxforwardedfor* :
-    (Java system property: **zookeeper.IPAuthenticationProvider.skipxforwardedfor**)
+* *IPAuthenticationProvider.usexforwardedfor* :
+    (Java system property: **zookeeper.IPAuthenticationProvider.usexforwardedfor**)
     **New in 3.9.3:**
-    IPAuthenticationProvider needs the client IP address to authenticate the user.
-    By default, it tries to read **X-Forwarded-For** HTTP header first and if it's not
-    found, reads the **Host** header. Some proxy configuration requires this to
-    properly identify the client IP, but we can disable it relying only on the **Host**
-    header by setting this config option to **true**.
+    IPAuthenticationProvider uses the client IP address to authenticate the user. By 
+    default it reads the **Host** HTTP header to detect client IP address. In some 
+    proxy configurations the proxy server adds the **X-Forwarded-For** header to
+    the request in order to provide the IP address of the original client request. 
+    By enabling **usexforwardedfor** ZooKeeper setting, **X-Forwarded-For** will be preferred
+    over the standard **Host** header.
     Default value is **false**.
 
 * *X509AuthenticationProvider.superUser* :

zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/IPAuthenticationProvider.java

@@ -30,7 +30,7 @@ import org.apache.zookeeper.server.ServerCnxn;
 public class IPAuthenticationProvider implements AuthenticationProvider {
     public static final String X_FORWARDED_FOR_HEADER_NAME = "X-Forwarded-For";
 
-    static final String SKIP_X_FORWARDED_FOR_KEY = "zookeeper.IPAuthenticationProvider.skipxforwardedfor";
+    public static final String USE_X_FORWARDED_FOR_KEY = "zookeeper.IPAuthenticationProvider.usexforwardedfor";
 
     public String getScheme() {
         return "ip";
@@ -152,7 +152,7 @@ public class IPAuthenticationProvider implements AuthenticationProvider {
      * @return IP address
      */
     public static String getClientIPAddress(final HttpServletRequest request) {
-        if (Boolean.getBoolean(SKIP_X_FORWARDED_FOR_KEY)) {
+        if (!Boolean.getBoolean(USE_X_FORWARDED_FOR_KEY)) {
             return request.getRemoteAddr();
         }
 

zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/IPAuthenticationProviderTest.java

@@ -17,7 +17,7 @@
  */
 package org.apache.zookeeper.server.auth;
 
-import static org.apache.zookeeper.server.auth.IPAuthenticationProvider.SKIP_X_FORWARDED_FOR_KEY;
+import static org.apache.zookeeper.server.auth.IPAuthenticationProvider.USE_X_FORWARDED_FOR_KEY;
 import static org.apache.zookeeper.server.auth.IPAuthenticationProvider.X_FORWARDED_FOR_HEADER_NAME;
 import static org.junit.Assert.assertEquals;
 import static org.mockito.Mockito.doReturn;
@@ -33,19 +33,33 @@ public class IPAuthenticationProviderTest {
 
   @Before
   public void setUp() throws Exception {
-    System.clearProperty(SKIP_X_FORWARDED_FOR_KEY);
+    System.clearProperty(USE_X_FORWARDED_FOR_KEY);
     request = mock(HttpServletRequest.class);
   }
 
   @After
   public void tearDown() {
-    System.clearProperty(SKIP_X_FORWARDED_FOR_KEY);
+    System.clearProperty(USE_X_FORWARDED_FOR_KEY);
   }
 
   @Test
   public void testGetClientIPAddressSkipXForwardedFor() {
     // Arrange
-    System.setProperty(SKIP_X_FORWARDED_FOR_KEY, "true");
+    System.setProperty(USE_X_FORWARDED_FOR_KEY, "false");
+    doReturn("192.168.1.1").when(request).getRemoteAddr();
+    doReturn("192.168.1.2,192.168.1.3,192.168.1.4").when(request).getHeader(X_FORWARDED_FOR_HEADER_NAME);
+
+    // Act
+    String clientIp = IPAuthenticationProvider.getClientIPAddress(request);
+
+    // Assert
+    assertEquals("192.168.1.1", clientIp);
+  }
+
+  @Test
+  public void testGetClientIPAddressDefaultBehaviour() {
+    // Arrange
+    System.clearProperty(USE_X_FORWARDED_FOR_KEY);
     doReturn("192.168.1.1").when(request).getRemoteAddr();
     doReturn("192.168.1.2,192.168.1.3,192.168.1.4").when(request).getHeader(X_FORWARDED_FOR_HEADER_NAME);
 
@@ -59,7 +73,7 @@ public class IPAuthenticationProviderTest {
   @Test
   public void testGetClientIPAddressWithXForwardedFor() {
     // Arrange
-    System.setProperty(SKIP_X_FORWARDED_FOR_KEY, "false");
+    System.setProperty(USE_X_FORWARDED_FOR_KEY, "true");
     doReturn("192.168.1.1").when(request).getRemoteAddr();
     doReturn("192.168.1.2,192.168.1.3,192.168.1.4").when(request).getHeader(X_FORWARDED_FOR_HEADER_NAME);
 
@@ -73,7 +87,7 @@ public class IPAuthenticationProviderTest {
   @Test
   public void testGetClientIPAddressMissingXForwardedFor() {
     // Arrange
-    System.setProperty(SKIP_X_FORWARDED_FOR_KEY, "false");
+    System.setProperty(USE_X_FORWARDED_FOR_KEY, "false");
     doReturn("192.168.1.1").when(request).getRemoteAddr();
 
     // Act

zookeeper-server/src/test/java/org/apache/zookeeper/test/IPAuthTest.java

@@ -23,13 +23,24 @@ import static org.mockito.Mockito.mock;
 import java.util.Arrays;
 import java.util.List;
 import javax.servlet.http.HttpServletRequest;
-import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.data.Id;
 import org.apache.zookeeper.server.auth.IPAuthenticationProvider;
-import org.junit.jupiter.api.Test;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
 import org.mockito.Mockito;
 
-public class IPAuthTest extends ZKTestCase {
+public class IPAuthTest {
+    @Before
+    public void setUp() {
+        System.setProperty(IPAuthenticationProvider.USE_X_FORWARDED_FOR_KEY, "true");
+    }
+
+    @After
+    public void tearDown() {
+        System.clearProperty(IPAuthenticationProvider.USE_X_FORWARDED_FOR_KEY);
+    }
+
     @Test
     public void testHandleAuthentication_Forwarded() {
         final IPAuthenticationProvider provider = new IPAuthenticationProvider();