Merge "ContainerAuthFilter: honor username provided by container"

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',

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,22 +51,29 @@ 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 {
   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
@@ -85,7 +97,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;

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;
+    }
+  }
+}

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)) {
+    return RemoteUserUtil.getRemoteUser(req, 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));
-    }
   }
 
   String getRemoteDisplayname(HttpServletRequest req) {

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");
+  }
+}