Merge branch 'stable-3.13' into stable-3.14 * stable-3.13: Introduce auth.httpTrustedProxyNetworks for securing HTTP auth Make RemoteUserUtil a singleton rather than static Release-Notes: skip Change-Id: I833aea2b8c89c2b5b550ba5904df18505eaf335d
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index b1703e5..f45da29 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt
@@ -430,6 +430,26 @@ HTTP header to trust the username from, or unset to select HTTP basic authentication. Only used if `auth.type` is set to `HTTP`. +[[auth.httpTrustedProxyNetworks]]auth.httpTrustedProxyNetworks:: ++ +List of IPv4 networks CIDRs (e.g., `192.168.0.0/16`) that are trusted proxies for receiving +HTTP header authentication. Specify multiple networks by adding multiple entries. +It applies only when `auth.type` is set to `HTTP` or `HTTP_LDAP`. The trusted-proxy +remote address must be IPv4; if Gerrit is reached over IPv6, header-based +authentication is rejected. ++ +[NOTE] +==== +Accepting a trusted authentication HTTP header involves a leap of faith in the security +of the incoming HTTP traffic. By default, the only secure way to ensure that all the +traffic is coming from a trusted authentication proxy is via firewall rules or using +a local proxy on the loopback interface. When using a remote proxy, the security of +the connection between the proxy and Gerrit is in doubt, and this can be mitigated by +the use of `auth.httpTrustedProxyNetworks`. +==== ++ +Default: accept any incoming IPs. + [[auth.httpDisplaynameHeader]]auth.httpDisplaynameHeader:: + HTTP header to retrieve the user's display name from. Only used if `auth.type`
diff --git a/java/com/google/gerrit/httpd/BUILD b/java/com/google/gerrit/httpd/BUILD index 0142031..18334aa 100644 --- a/java/com/google/gerrit/httpd/BUILD +++ b/java/com/google/gerrit/httpd/BUILD
@@ -38,6 +38,7 @@ "//lib/auto:auto-value", "//lib/auto:auto-value-annotations", "//lib/commons:lang3", + "//lib/commons:net", "//lib/errorprone:annotations", "//lib/flogger:api", "//lib/guice",
diff --git a/java/com/google/gerrit/httpd/ContainerAuthFilter.java b/java/com/google/gerrit/httpd/ContainerAuthFilter.java index 517d5db..3abe786 100644 --- a/java/com/google/gerrit/httpd/ContainerAuthFilter.java +++ b/java/com/google/gerrit/httpd/ContainerAuthFilter.java
@@ -66,18 +66,21 @@ private final AccountCache accountCache; private final Config config; private final String loginHttpHeader; + private final RemoteUserUtil remoteUserUtil; @Inject ContainerAuthFilter( DynamicItem<WebSession> session, AccountCache accountCache, AuthConfig authConfig, - @GerritServerConfig Config config) { + @GerritServerConfig Config config, + RemoteUserUtil remoteUserUtil) { this.session = session; this.accountCache = accountCache; this.config = config; loginHttpHeader = firstNonNull(emptyToNull(authConfig.getLoginHttpHeader()), AUTHORIZATION); + this.remoteUserUtil = remoteUserUtil; } @Override @@ -98,7 +101,7 @@ } private boolean verify(HttpServletRequest req, HttpServletResponse rsp) throws IOException { - String username = RemoteUserUtil.getRemoteUser(req, loginHttpHeader); + String username = remoteUserUtil.getRemoteUser(req, loginHttpHeader); if (username == null) { if (isLfsOverSshRequest(req)) { // LFS-over-SSH auth request cannot be authorized by container
diff --git a/java/com/google/gerrit/httpd/RemoteUserUtil.java b/java/com/google/gerrit/httpd/RemoteUserUtil.java index 9ec10e2..8856f91 100644 --- a/java/com/google/gerrit/httpd/RemoteUserUtil.java +++ b/java/com/google/gerrit/httpd/RemoteUserUtil.java
@@ -18,11 +18,71 @@ import static com.google.common.net.HttpHeaders.AUTHORIZATION; import static java.nio.charset.StandardCharsets.UTF_8; +import com.google.common.base.MoreObjects; +import com.google.common.flogger.FluentLogger; import com.google.common.io.BaseEncoding; import com.google.gerrit.common.Nullable; +import com.google.gerrit.server.config.AuthConfig; +import com.google.inject.ProvisionException; +import java.util.Set; +import java.util.concurrent.TimeUnit; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import javax.inject.Inject; +import javax.inject.Singleton; import javax.servlet.http.HttpServletRequest; +import org.apache.commons.net.util.SubnetUtils; +@Singleton public class RemoteUserUtil { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + + /** + * Request attribute carrying the TCP-peer address before any X-Forwarded-For rewrite. + * + * <p>HTTP layers that want their requests evaluated against {@code auth.httpTrustedProxyNetworks} + * must set this attribute on each request. If the attribute is unset, {@link + * HttpServletRequest#getRemoteAddr()} is used as the peer address. + * + * <p>Gerrit's default Jetty container wires this automatically (see {@code + * JettyServer.ForwardedRequestCustomizer}). Other servlet containers (Tomcat, embedded netty, + * etc.) can opt in by installing a {@code Filter} or equivalent that runs <em>before</em> any + * X-Forwarded-For rewrite (e.g., Tomcat's {@code RemoteIpValve}) and sets this attribute to + * {@code request.getRemoteAddr()}. + */ + public static final String PROXY_REMOTE_ADDRESS_ATTR = + "com.google.gerrit.httpd.proxyRemoteAddress"; + + private final Set<SubnetUtils.SubnetInfo> trustedProxySubnets; + private final Set<String> trustedProxyNetworks; + + @Inject + RemoteUserUtil(AuthConfig authConfig) { + // The full list of `trustedProxyNetworks` is also kept as Set<String> + // for allowing the single-IP matching (networks ending with '/32') fast + // lookup whilst the full network matching evaluation is performed + // through the trustedProxySubnets loop. + trustedProxyNetworks = authConfig.getTrustedProxyNetworks(); + + try { + trustedProxySubnets = + trustedProxyNetworks.stream() + // Filter out single IPs because they are not matched by + // subnetwork matching but rather direct containment in + // trustedProxyNetworks + .filter(Predicate.not(RemoteUserUtil::isSingleIp)) + .map(SubnetUtils::new) + .map(SubnetUtils::getInfo) + .collect(Collectors.toSet()); + } catch (IllegalArgumentException e) { + throw new ProvisionException("Invalid auth trusted proxy definition: " + e.getMessage(), e); + } + } + + private static boolean isSingleIp(String network) { + return network.endsWith("/32"); + } + /** * Tries to get username from a request with following strategies: * @@ -37,8 +97,10 @@ * @return the extracted username or null. */ @Nullable - public static String getRemoteUser(HttpServletRequest req, String loginHeader) { - if (AUTHORIZATION.equals(loginHeader)) { + public String getRemoteUser(HttpServletRequest req, String loginHeader) { + boolean isAuthorizationHeader = AUTHORIZATION.equals(loginHeader); + + if (isAuthorizationHeader) { String user = emptyToNull(req.getRemoteUser()); if (user != null) { // The container performed the authentication, and has the user @@ -46,16 +108,55 @@ // configured to honor HTTP authentication. return user; } - - // If the container didn't do the authentication we might - // have done it in the front-end web server. Try to split - // the identity out of the Authorization header and honor it. - String auth = req.getHeader(AUTHORIZATION); - return extractUsername(auth); } - // Nonstandard HTTP header. We have been told to trust this - // header blindly as-is. - return emptyToNull(req.getHeader(loginHeader)); + + if (!isRequestFromTrustedProxyNetworks(req)) { + return null; + } + + String auth = req.getHeader(loginHeader); + return isAuthorizationHeader + ? + // If the container didn't do the authentication we might + // have done it in the front-end web server. Try to split + // the identity out of the Authorization header and honor it. + extractUsername(auth) + : + // Nonstandard HTTP header. We have been told to trust this + // header blindly as-is. + emptyToNull(auth); + } + + private boolean isRequestFromTrustedProxyNetworks(HttpServletRequest req) { + if (trustedProxyNetworks.isEmpty()) { + return true; + } + + String remoteAddress = getRemoteAddress(req); + if (isIpv6Address(remoteAddress)) { + logger.atWarning().atMostEvery(1, TimeUnit.MINUTES).log( + "IPv6 remote address: %s - trusted proxy enforcement supports only IPv4, HTTP header" + + " rejected", + remoteAddress); + return false; + } + + if (trustedProxyNetworks.contains(remoteAddress + "/32")) { + return true; + } + + if (trustedProxySubnets.stream().anyMatch(subnet -> subnet.isInRange(remoteAddress))) { + return true; + } + + logger.atWarning().atMostEvery(1, TimeUnit.MINUTES).log( + "Untrusted remote address: %s - authentication via HTTP header rejected", remoteAddress); + return false; + } + + private static String getRemoteAddress(HttpServletRequest req) { + return MoreObjects.firstNonNull( + (String) req.getAttribute(PROXY_REMOTE_ADDRESS_ATTR), req.getRemoteAddr()); } /** @@ -90,4 +191,8 @@ return null; } } + + private static boolean isIpv6Address(String ipAddress) { + return ipAddress.contains(":"); + } }
diff --git a/java/com/google/gerrit/httpd/auth/container/HttpAuthFilter.java b/java/com/google/gerrit/httpd/auth/container/HttpAuthFilter.java index f0a8b89..710adc4 100644 --- a/java/com/google/gerrit/httpd/auth/container/HttpAuthFilter.java +++ b/java/com/google/gerrit/httpd/auth/container/HttpAuthFilter.java
@@ -67,12 +67,14 @@ private final String externalIdHeader; private final boolean userNameToLowerCase; private final ExternalIdKeyFactory externalIdKeyFactory; + private final RemoteUserUtil remoteUserUtil; @Inject HttpAuthFilter( DynamicItem<WebSession> webSession, AuthConfig authConfig, - ExternalIdKeyFactory externalIdKeyFactory) + ExternalIdKeyFactory externalIdKeyFactory, + RemoteUserUtil remoteUserUtil) throws IOException { this.sessionProvider = webSession; this.externalIdKeyFactory = externalIdKeyFactory; @@ -90,6 +92,7 @@ emailHeader = emptyToNull(authConfig.getHttpEmailHeader()); externalIdHeader = emptyToNull(authConfig.getHttpExternalIdHeader()); userNameToLowerCase = authConfig.isUserNameToLowerCase(); + this.remoteUserUtil = remoteUserUtil; } @Override @@ -138,7 +141,7 @@ } String getRemoteUser(HttpServletRequest req) { - String remoteUser = RemoteUserUtil.getRemoteUser(req, loginHeader); + String remoteUser = remoteUserUtil.getRemoteUser(req, loginHeader); return (userNameToLowerCase && remoteUser != null) ? remoteUser.toLowerCase(Locale.US) : remoteUser;
diff --git a/java/com/google/gerrit/pgm/http/jetty/JettyServer.java b/java/com/google/gerrit/pgm/http/jetty/JettyServer.java index 7cde777..20f5bb3 100644 --- a/java/com/google/gerrit/pgm/http/jetty/JettyServer.java +++ b/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
@@ -23,6 +23,7 @@ import com.google.common.base.Strings; import com.google.gerrit.extensions.client.AuthType; import com.google.gerrit.extensions.events.LifecycleListener; +import com.google.gerrit.httpd.RemoteUserUtil; import com.google.gerrit.pgm.http.jetty.HttpLog.HttpLogFactory; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; @@ -33,6 +34,7 @@ import com.google.inject.servlet.GuiceFilter; import com.google.inject.servlet.GuiceServletContextListener; import java.lang.management.ManagementFactory; +import java.net.InetSocketAddress; import java.net.URI; import java.net.URISyntaxException; import java.nio.file.Files; @@ -57,6 +59,7 @@ import org.eclipse.jetty.ee8.servlet.FilterHolder; import org.eclipse.jetty.ee8.servlet.ServletContextHandler; import org.eclipse.jetty.ee8.servlet.ServletHolder; +import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.UriCompliance; @@ -81,6 +84,38 @@ @Singleton public class JettyServer { + + private static final ForwardedRequestCustomizer FORWARDED_REQUEST_CUSTOMIZER = + new ForwardedRequestCustomizer() { + @Override + public Request customize(Request request, HttpFields.Mutable responseHeaders) { + /* + * The default behavior of ForwardedRequestCustomizer is to overwrite the remote address + * with the value of the X-Forwarded-For header, if present. + * However, it does not "remember" the original remote address and therefore would + * prevent any validation against it. + * + * ForwardedRequestCustomizer's original code fragment: + * <code> + * if (forwarded.hasFor()) + * { + * int forPort = forwarded._for._port > 0 ? forwarded._for._port : request.getRemotePort(); + * request.setRemoteAddr(InetSocketAddress.createUnresolved(forwarded._for._host, forPort)); + * } + * </code> + * + * What we want to achieve here is to remember what it was the original proxy address before + * calling super.customize() and give the possibility to fetch it later down the chain. + */ + request.setAttribute( + RemoteUserUtil.PROXY_REMOTE_ADDRESS_ATTR, + ((InetSocketAddress) request.getConnectionMetaData().getRemoteSocketAddress()) + .getAddress() + .getHostAddress()); + return super.customize(request, responseHeaders); + } + }; + static class Lifecycle implements LifecycleListener { private final JettyServer server; private final Config cfg; @@ -381,12 +416,12 @@ } else if ("proxy-http".equals(u.getScheme())) { defaultPort = 8080; - config.addCustomizer(new ForwardedRequestCustomizer()); + config.addCustomizer(FORWARDED_REQUEST_CUSTOMIZER); c = newServerConnector(server, acceptors, config); } else if ("proxy-https".equals(u.getScheme())) { defaultPort = 8080; - config.addCustomizer(new ForwardedRequestCustomizer()); + config.addCustomizer(FORWARDED_REQUEST_CUSTOMIZER); // For a proxy that terminates TLS, mark every request as HTTPS // unconditionally. ForwardedRequestCustomizer alone only sets // isSecure() when the proxy sends X-Forwarded-Proto=https or
diff --git a/java/com/google/gerrit/server/config/AuthConfig.java b/java/com/google/gerrit/server/config/AuthConfig.java index 7886cb5..f43a8b8 100644 --- a/java/com/google/gerrit/server/config/AuthConfig.java +++ b/java/com/google/gerrit/server/config/AuthConfig.java
@@ -18,6 +18,7 @@ import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME; import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_UUID; +import com.google.common.collect.ImmutableSet; import com.google.gerrit.extensions.client.AuthType; import com.google.gerrit.extensions.client.GitBasicAuthPolicy; import com.google.gerrit.server.account.externalids.ExternalId; @@ -33,6 +34,7 @@ import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.concurrent.TimeUnit; import org.eclipse.jgit.lib.Config; @@ -41,6 +43,7 @@ public class AuthConfig { private final AuthType authType; private final String httpHeader; + private final ImmutableSet<String> trustedProxyNetworks; private final String httpDisplaynameHeader; private final String httpEmailHeader; private final String httpExternalIdHeader; @@ -78,6 +81,8 @@ AuthConfig(@GerritServerConfig Config cfg) throws XsrfException { authType = toType(cfg); httpHeader = cfg.getString("auth", null, "httpheader"); + trustedProxyNetworks = + ImmutableSet.copyOf(cfg.getStringList("auth", null, "httpTrustedProxyNetworks")); httpDisplaynameHeader = cfg.getString("auth", null, "httpdisplaynameheader"); httpEmailHeader = cfg.getString("auth", null, "httpemailheader"); httpExternalIdHeader = cfg.getString("auth", null, "httpexternalidheader"); @@ -384,4 +389,8 @@ public boolean isHttpPasswordFallbackEnabled() { return httpPasswordFallbackEnabled; } + + public Set<String> getTrustedProxyNetworks() { + return trustedProxyNetworks; + } }
diff --git a/javatests/com/google/gerrit/httpd/RemoteUserUtilTest.java b/javatests/com/google/gerrit/httpd/RemoteUserUtilTest.java index f012ee3..ab464e8 100644 --- a/javatests/com/google/gerrit/httpd/RemoteUserUtilTest.java +++ b/javatests/com/google/gerrit/httpd/RemoteUserUtilTest.java
@@ -15,11 +15,45 @@ package com.google.gerrit.httpd; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.httpd.RemoteUserUtil.PROXY_REMOTE_ADDRESS_ATTR; import static com.google.gerrit.httpd.RemoteUserUtil.extractUsername; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; +import static org.mockito.Mockito.when; +import com.google.common.base.Suppliers; +import com.google.common.net.HttpHeaders; +import com.google.gerrit.server.config.AuthConfig; +import com.google.gerrit.util.http.testutil.FakeHttpServletRequest; +import com.google.inject.ProvisionException; +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import java.util.Set; +import java.util.function.Supplier; +import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +@RunWith(MockitoJUnitRunner.class) public class RemoteUserUtilTest { + private static final String CUSTOM_LOGIN_HEADER = "MY_HEADER"; + private static final String EXPECTED_USER = "user"; + private static final String BASIC_AUTHENTICATION_USER_HEADER = + "Basic " + + Base64.getEncoder() + .encodeToString((EXPECTED_USER + ":pass").getBytes(StandardCharsets.UTF_8)); + + private Supplier<RemoteUserUtil> remoteUserUtil; + + @Mock AuthConfig authConfigMock; + + @Before + public void setup() { + when(authConfigMock.getTrustedProxyNetworks()).thenReturn(Set.of()); + remoteUserUtil = Suppliers.memoize(() -> new RemoteUserUtil(authConfigMock)); + } + @Test public void testExtractUsername() { assertThat(extractUsername(null)).isNull(); @@ -27,4 +61,165 @@ assertThat(extractUsername("Basic dXNlcjpwYXNzd29yZA==")).isEqualTo("user"); assertThat(extractUsername("Digest username=\"user\", realm=\"test\"")).isEqualTo("user"); } + + @Test + public void testExtractUserFromRequestWithCustomHeaderAllowedByDefault() throws Exception { + FakeHttpServletRequest fakeRequest = new FakeHttpServletRequest(); + fakeRequest.addHeader(CUSTOM_LOGIN_HEADER, EXPECTED_USER); + assertThat(remoteUserUtil.get().getRemoteUser(fakeRequest, CUSTOM_LOGIN_HEADER)) + .isEqualTo(EXPECTED_USER); + } + + @Test + public void testExtractUserFromRequestWithAuthenticationHeaderAllowedByDefault() + throws Exception { + FakeHttpServletRequest fakeRequest = new FakeHttpServletRequest(); + fakeRequest.addHeader(HttpHeaders.AUTHORIZATION, BASIC_AUTHENTICATION_USER_HEADER); + assertThat(remoteUserUtil.get().getRemoteUser(fakeRequest, HttpHeaders.AUTHORIZATION)) + .isEqualTo(EXPECTED_USER); + } + + @Test + public void testExtractUserFromRequestWithCustomHeaderAllowedUsingProxyExactIPv4Matching() + throws Exception { + String clientIP = "192.168.1.2"; + String proxyId = "80.78.1.3"; + when(authConfigMock.getTrustedProxyNetworks()).thenReturn(Set.of(proxyId + "/32")); + FakeHttpServletRequest fakeRequest = newFakeHttpRequest(clientIP, EXPECTED_USER); + fakeRequest.setAttribute(PROXY_REMOTE_ADDRESS_ATTR, proxyId); + assertThat(remoteUserUtil.get().getRemoteUser(fakeRequest, CUSTOM_LOGIN_HEADER)) + .isEqualTo(EXPECTED_USER); + } + + @Test + public void testExtractUserFromRequestWithCustomHeaderAllowedWithExactIPv4Matching() + throws Exception { + String remoteIp = "192.168.1.2"; + when(authConfigMock.getTrustedProxyNetworks()).thenReturn(Set.of(remoteIp + "/32")); + assertThat( + remoteUserUtil + .get() + .getRemoteUser(newFakeHttpRequest(remoteIp, EXPECTED_USER), CUSTOM_LOGIN_HEADER)) + .isEqualTo(EXPECTED_USER); + } + + @Test + public void testExtractUserFromRequestWithAuthenticationHeaderAllowedWithExactIPv4Matching() + throws Exception { + String remoteIp = "192.168.1.2"; + when(authConfigMock.getTrustedProxyNetworks()).thenReturn(Set.of(remoteIp + "/32")); + assertThat( + remoteUserUtil + .get() + .getRemoteUser( + newFakeAuthHttpRequest(remoteIp, BASIC_AUTHENTICATION_USER_HEADER), + HttpHeaders.AUTHORIZATION)) + .isEqualTo(EXPECTED_USER); + } + + @Test + public void testExtractUserFromRequestWithCustomHeaderAllowedWithExactIPv4InAcceptedRange() + throws Exception { + when(authConfigMock.getTrustedProxyNetworks()) + .thenReturn(Set.of("10.16.0.0/16", "192.168.1.0/24", "8.8.8.8/32")); + assertThat( + remoteUserUtil + .get() + .getRemoteUser(newFakeHttpRequest("10.16.5.1", EXPECTED_USER), CUSTOM_LOGIN_HEADER)) + .isEqualTo(EXPECTED_USER); + } + + @Test + public void + testExtractUserFromRequestWithAuthenticationHeaderAllowedWithExactIPv4InAcceptedRange() + throws Exception { + when(authConfigMock.getTrustedProxyNetworks()) + .thenReturn(Set.of("10.16.0.0/16", "192.168.1.0/24", "8.8.8.8/32")); + assertThat( + remoteUserUtil + .get() + .getRemoteUser( + newFakeAuthHttpRequest("10.16.5.1", BASIC_AUTHENTICATION_USER_HEADER), + HttpHeaders.AUTHORIZATION)) + .isEqualTo(EXPECTED_USER); + } + + @Test + public void testExtractUserFromRequestWithCustomHeaderRejectedWithNonMatchingExactIPv4() + throws Exception { + when(authConfigMock.getTrustedProxyNetworks()).thenReturn(Set.of("2.2.2.2/32")); + assertThat( + remoteUserUtil + .get() + .getRemoteUser(newFakeHttpRequest("1.1.1.1", EXPECTED_USER), CUSTOM_LOGIN_HEADER)) + .isNull(); + } + + @Test + public void testExtractUserFromRequestWithAuthenticationHeaderRejectedWithNonMatchingExactIPv4() + throws Exception { + when(authConfigMock.getTrustedProxyNetworks()).thenReturn(Set.of("2.2.2.2/32")); + assertThat( + remoteUserUtil + .get() + .getRemoteUser( + newFakeAuthHttpRequest("1.1.1.1", BASIC_AUTHENTICATION_USER_HEADER), + HttpHeaders.AUTHORIZATION)) + .isNull(); + } + + @Test + public void testExtractUserFromRequestRejectedWithIPv6() throws Exception { + when(authConfigMock.getTrustedProxyNetworks()).thenReturn(Set.of("255.255.255.255/32")); + assertThat( + remoteUserUtil + .get() + .getRemoteUser( + newFakeHttpRequest("2001:0db8:85a3:0000:0000:8a2e:0370:7334", "user"), + CUSTOM_LOGIN_HEADER)) + .isNull(); + } + + @Test + public void testFailWhenUsingAnInvalidProxyNetworkCIDR() throws Exception { + when(authConfigMock.getTrustedProxyNetworks()).thenReturn(Set.of("invalid-network")); + assertThrows(ProvisionException.class, () -> remoteUserUtil.get()); + } + + @Test + public void testFailWhenUsingSingleIPAsProxyNetworkCIDR() throws Exception { + when(authConfigMock.getTrustedProxyNetworks()).thenReturn(Set.of("192.168.0.1")); + assertThrows(ProvisionException.class, () -> remoteUserUtil.get()); + } + + @Test + public void testFailWhenUsingIPv6AsProxyNetworkCIDR() throws Exception { + when(authConfigMock.getTrustedProxyNetworks()).thenReturn(Set.of("2000::/3")); + assertThrows(ProvisionException.class, () -> remoteUserUtil.get()); + } + + private static FakeHttpServletRequest newFakeHttpRequest(String remoteIp, String expectedUser) { + FakeHttpServletRequest fakeRequest = + new FakeHttpServletRequest() { + @Override + public String getRemoteAddr() { + return remoteIp; + } + }; + fakeRequest.addHeader(CUSTOM_LOGIN_HEADER, expectedUser); + return fakeRequest; + } + + private static FakeHttpServletRequest newFakeAuthHttpRequest( + String remoteIp, String basicAuthHeader) { + FakeHttpServletRequest fakeRequest = + new FakeHttpServletRequest() { + @Override + public String getRemoteAddr() { + return remoteIp; + } + }; + fakeRequest.addHeader(HttpHeaders.AUTHORIZATION, basicAuthHeader); + return fakeRequest; + } }
diff --git a/javatests/com/google/gerrit/httpd/auth/container/HttpAuthFilterTest.java b/javatests/com/google/gerrit/httpd/auth/container/HttpAuthFilterTest.java index a5f8349..32ae2ca 100644 --- a/javatests/com/google/gerrit/httpd/auth/container/HttpAuthFilterTest.java +++ b/javatests/com/google/gerrit/httpd/auth/container/HttpAuthFilterTest.java
@@ -18,6 +18,7 @@ import static org.mockito.Mockito.doReturn; import com.google.gerrit.extensions.registration.DynamicItem; +import com.google.gerrit.httpd.RemoteUserUtil; import com.google.gerrit.httpd.WebSession; import com.google.gerrit.server.account.externalids.ExternalIdKeyFactory; import com.google.gerrit.server.config.AuthConfig; @@ -37,13 +38,14 @@ @Mock private DynamicItem<WebSession> webSession; @Mock private ExternalIdKeyFactory externalIdKeyFactory; @Mock private AuthConfig authConfig; + @Mock private RemoteUserUtil remoteUserUtil; @Test public void getRemoteDisplaynameShouldReturnDisplaynameHeaderWhenHeaderIsConfiguredAndSet() throws IOException { doReturn(DISPLAYNAME_HEADER).when(authConfig).getHttpDisplaynameHeader(); HttpAuthFilter httpAuthFilter = - new HttpAuthFilter(webSession, authConfig, externalIdKeyFactory); + new HttpAuthFilter(webSession, authConfig, externalIdKeyFactory, remoteUserUtil); FakeHttpServletRequest req = new FakeHttpServletRequest(); req.addHeader(DISPLAYNAME_HEADER, DISPLAYNAME); @@ -56,7 +58,7 @@ throws IOException { doReturn(DISPLAYNAME_HEADER).when(authConfig).getHttpDisplaynameHeader(); HttpAuthFilter httpAuthFilter = - new HttpAuthFilter(webSession, authConfig, externalIdKeyFactory); + new HttpAuthFilter(webSession, authConfig, externalIdKeyFactory, remoteUserUtil); FakeHttpServletRequest req = new FakeHttpServletRequest(); @@ -68,7 +70,7 @@ throws IOException { doReturn(DISPLAYNAME_HEADER).when(authConfig).getHttpDisplaynameHeader(); HttpAuthFilter httpAuthFilter = - new HttpAuthFilter(webSession, authConfig, externalIdKeyFactory); + new HttpAuthFilter(webSession, authConfig, externalIdKeyFactory, remoteUserUtil); FakeHttpServletRequest req = new FakeHttpServletRequest(); req.addHeader(DISPLAYNAME_HEADER, "");