ContainerAuthFilter: honor username provided by container

When 'trustContainerAuth' is enabled and proxy does authentication
on root (instead of '/login/'), ServletRequest#getRemoteUser is
null. In this case we need to pull the username from 'Authorization'
header. It is done the same way in HttpAuthFilter already.

Move username extraction logic from HttpAuthFilter to
RemoteUserUtil and add some tests.

Update javadoc to reflect current situation:
ContainerAuthFilter is also used for the REST API; see:
GitOverHttpModule#configureServlets: filter("/a/*").through(authFilter)

Change-Id: I0cf21fb7ecd8a958fad270704c11ebfffd9fea93
Bug: Issue 2209

5 files changed, 144 insertions, 47 deletions

diff --git a/gerrit-httpd/BUCK b/gerrit-httpd/BUCK
index a4d127da47..3345018c78 100644
--- a/gerrit-httpd/BUCK
+++ b/gerrit-httpd/BUCK
@@ -61,6 +61,7 @@ java_test(
     '//lib:gwtorm',
     '//lib:guava',
     '//lib:servlet-api-3_1',
+    '//lib:truth',
     '//lib/easymock:easymock',
     '//lib/guice:guice',
     '//lib/jgit:jgit',
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ContainerAuthFilter.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ContainerAuthFilter.java
index 91fb6aff7f..4bd9ef51d8 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ContainerAuthFilter.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ContainerAuthFilter.java
@@ -14,13 +14,18 @@
 
 package com.google.gerrit.httpd;
 
+import static com.google.common.base.MoreObjects.firstNonNull;
+import static com.google.common.base.Strings.emptyToNull;
+import static com.google.common.net.HttpHeaders.AUTHORIZATION;
 import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
 import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;
 
 import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.gerrit.httpd.restapi.RestApiServlet;
 import com.google.gerrit.server.AccessPath;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.config.AuthConfig;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
@@ -46,9 +51,9 @@ import javax.servlet.http.HttpServletResponse;
  * lookup the account and set the account ID in our current session.
  * <p>
  * This filter should only be configured to run, when authentication is
- * configured to trust container authentication. This filter is intended only to
+ * configured to trust container authentication. This filter is intended to
  * protect the {@link GitOverHttpServlet} and its handled URLs, which provide remote
- * repository access over HTTP.
+ * repository access over HTTP. It also protects {@link RestApiServlet}.
  */
 @Singleton
 class ContainerAuthFilter implements Filter {
@@ -57,13 +62,20 @@ class ContainerAuthFilter implements Filter {
   private final DynamicItem<WebSession> session;
   private final AccountCache accountCache;
   private final Config config;
+  private final String loginHttpHeader;
 
   @Inject
-  ContainerAuthFilter(DynamicItem<WebSession> session, AccountCache accountCache,
+  ContainerAuthFilter(DynamicItem<WebSession> session,
+      AccountCache accountCache,
+      AuthConfig authConfig,
       @GerritServerConfig Config config) {
     this.session = session;
     this.accountCache = accountCache;
     this.config = config;
+
+    loginHttpHeader = firstNonNull(
+        emptyToNull(authConfig.getLoginHttpHeader()),
+        AUTHORIZATION);
   }
 
   @Override
@@ -87,7 +99,7 @@ class ContainerAuthFilter implements Filter {
 
   private boolean verify(HttpServletRequest req, HttpServletResponse rsp)
       throws IOException {
-    String username = req.getRemoteUser();
+    String username = RemoteUserUtil.getRemoteUser(req, loginHttpHeader);
     if (username == null) {
       rsp.sendError(SC_FORBIDDEN);
       return false;
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/RemoteUserUtil.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/RemoteUserUtil.java
new file mode 100644
index 0000000000..7116cf0478
--- /dev/null
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/RemoteUserUtil.java
@@ -0,0 +1,93 @@
+// Copyright (C) 2015 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.httpd;
+
+import static com.google.common.base.Strings.emptyToNull;
+import static com.google.common.net.HttpHeaders.AUTHORIZATION;
+
+import org.eclipse.jgit.util.Base64;
+
+import javax.servlet.http.HttpServletRequest;
+
+public class RemoteUserUtil {
+  /**
+   * Tries to get username from a request with following strategies:
+   * <ul>
+   * <li>ServletRequest#getRemoteUser</li>
+   * <li>HTTP 'Authorization' header</li>
+   * <li>Custom HTTP header</li>
+   * </ul>
+   *
+   * @param req request to extract username from.
+   * @param loginHeader name of header which is used for extracting
+   *    username.
+   * @return the extracted username or null.
+   */
+  public static String getRemoteUser(HttpServletRequest req,
+      String loginHeader) {
+    if (AUTHORIZATION.equals(loginHeader)) {
+      String user = emptyToNull(req.getRemoteUser());
+      if (user != null) {
+        // The container performed the authentication, and has the user
+        // identity already decoded for us. Honor that as we have been
+        // 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);
+    } else {
+      // Nonstandard HTTP header. We have been told to trust this
+      // header blindly as-is.
+      return emptyToNull(req.getHeader(loginHeader));
+    }
+  }
+
+  /**
+   * Extracts username from an HTTP Basic or Digest authentication
+   * header.
+   *
+   * @param auth header value which is used for extracting.
+   * @return username if available or null.
+   */
+  public static String extractUsername(String auth) {
+    auth = emptyToNull(auth);
+
+    if (auth == null) {
+      return null;
+
+    } else if (auth.startsWith("Basic ")) {
+      auth = auth.substring("Basic ".length());
+      auth = new String(Base64.decode(auth));
+      final int c = auth.indexOf(':');
+      return c > 0 ? auth.substring(0, c) : null;
+
+    } else if (auth.startsWith("Digest ")) {
+      final int u = auth.indexOf("username=\"");
+      if (u <= 0) {
+        return null;
+      }
+      auth = auth.substring(u + 10);
+      final int e = auth.indexOf('"');
+      return e > 0 ? auth.substring(0, e) : null;
+
+    } else {
+      return null;
+    }
+  }
+}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/container/HttpAuthFilter.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/container/HttpAuthFilter.java
index 19c8342351..949f392e32 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/container/HttpAuthFilter.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/container/HttpAuthFilter.java
@@ -22,6 +22,7 @@ import static com.google.gerrit.reviewdb.client.AccountExternalId.SCHEME_GERRIT;
 import com.google.gerrit.extensions.registration.DynamicItem;
 import com.google.gerrit.httpd.HtmlDomUtil;
 import com.google.gerrit.httpd.WebSession;
+import com.google.gerrit.httpd.RemoteUserUtil;
 import com.google.gerrit.httpd.raw.HostPageServlet;
 import com.google.gerrit.reviewdb.client.AccountExternalId;
 import com.google.gerrit.server.config.AuthConfig;
@@ -30,8 +31,6 @@ import com.google.gwtjsonrpc.server.RPCServletUtils;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 
-import org.eclipse.jgit.util.Base64;
-
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.OutputStream;
@@ -137,47 +136,7 @@ class HttpAuthFilter implements Filter {
   }
 
   String getRemoteUser(HttpServletRequest req) {
-    if (AUTHORIZATION.equals(loginHeader)) {
-      String user = emptyToNull(req.getRemoteUser());
-      if (user != null) {
-        // The container performed the authentication, and has the user
-        // identity already decoded for us. Honor that as we have been
-        // 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 = emptyToNull(req.getHeader(AUTHORIZATION));
-      if (auth == null) {
-        return null;
-
-      } else if (auth.startsWith("Basic ")) {
-        auth = auth.substring("Basic ".length());
-        auth = new String(Base64.decode(auth));
-        final int c = auth.indexOf(':');
-        return c > 0 ? auth.substring(0, c) : null;
-
-      } else if (auth.startsWith("Digest ")) {
-        final int u = auth.indexOf("username=\"");
-        if (u <= 0) {
-          return null;
-        }
-        auth = auth.substring(u + 10);
-        final int e = auth.indexOf('"');
-        return e > 0 ? auth.substring(0, e) : null;
-
-      } else {
-        return null;
-      }
-    } else {
-      // Nonstandard HTTP header. We have been told to trust this
-      // header blindly as-is.
-      //
-      return emptyToNull(req.getHeader(loginHeader));
-    }
+    return RemoteUserUtil.getRemoteUser(req, loginHeader);
   }
 
   String getRemoteDisplayname(HttpServletRequest req) {
diff --git a/gerrit-httpd/src/test/java/com/google/gerrit/httpd/RemoteUserUtilTest.java b/gerrit-httpd/src/test/java/com/google/gerrit/httpd/RemoteUserUtilTest.java
new file mode 100644
index 0000000000..b6d0b0b16a
--- /dev/null
+++ b/gerrit-httpd/src/test/java/com/google/gerrit/httpd/RemoteUserUtilTest.java
@@ -0,0 +1,32 @@
+// Copyright (C) 2015 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.httpd;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.httpd.RemoteUserUtil.extractUsername;
+
+import org.junit.Test;
+
+public class RemoteUserUtilTest {
+  @Test
+  public void testExtractUsername() {
+    assertThat(extractUsername(null)).isNull();
+    assertThat(extractUsername("")).isNull();
+    assertThat(extractUsername("Basic dXNlcjpwYXNzd29yZA=="))
+        .isEqualTo("user");
+    assertThat(extractUsername("Digest username=\"user\", realm=\"test\""))
+        .isEqualTo("user");
+  }
+}