Merge "Fix login servlets when canonicalWebUrl is not set" into stable-2.6

gerrit-httpd/src/main/java/com/google/gerrit/httpd/CanonicalWebUrl.java

@@ -0,0 +1,47 @@
+// Copyright (C) 2013 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 javax.annotation.Nullable;
+import javax.servlet.http.HttpServletRequest;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+public class CanonicalWebUrl {
+  private final Provider<String> configured;
+
+  @Inject
+  CanonicalWebUrl(
+      @com.google.gerrit.server.config.CanonicalWebUrl
+      @Nullable
+      Provider<String> provider) {
+    configured = provider;
+  }
+
+  public String get(HttpServletRequest req) {
+    String url = configured.get();
+    return url != null ? url : computeFromRequest(req);
+  }
+
+  static String computeFromRequest(HttpServletRequest req) {
+    StringBuffer url = req.getRequestURL();
+    url.setLength(url.length() - req.getServletPath().length());
+    if (url.charAt(url.length() - 1) != '/') {
+      url.append('/');
+    }
+    return url.toString();
+  }
+}

gerrit-httpd/src/main/java/com/google/gerrit/httpd/HttpCanonicalWebUrlProvider.java

@@ -14,7 +14,6 @@
 
 package com.google.gerrit.httpd;
 
-import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.config.CanonicalWebUrlProvider;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.inject.Inject;
@@ -26,7 +25,7 @@ import org.eclipse.jgit.lib.Config;
 
 import javax.servlet.http.HttpServletRequest;
 
-/** Sets {@link CanonicalWebUrl} to current HTTP request if not configured. */
+/** Sets {@code CanonicalWebUrl} to current HTTP request if not configured. */
 public class HttpCanonicalWebUrlProvider extends CanonicalWebUrlProvider {
   private Provider<HttpServletRequest> requestProvider;
 
@@ -65,13 +64,7 @@ public class HttpCanonicalWebUrlProvider extends CanonicalWebUrlProvider {
           throw noWeb;
         }
       }
-
-      final StringBuffer url = req.getRequestURL();
-      url.setLength(url.length() - req.getServletPath().length());
-      if (url.charAt(url.length() - 1) != '/') {
-        url.append('/');
-      }
-      return url.toString();
+      return CanonicalWebUrl.computeFromRequest(req);
     }
 
     // We have no way of guessing our HTTP url.

gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/container/HttpLoginServlet.java

@@ -15,13 +15,13 @@
 package com.google.gerrit.httpd.auth.container;
 
 import com.google.gerrit.common.PageLinks;
+import com.google.gerrit.httpd.CanonicalWebUrl;
 import com.google.gerrit.httpd.HtmlDomUtil;
 import com.google.gerrit.httpd.WebSession;
 import com.google.gerrit.server.account.AccountException;
 import com.google.gerrit.server.account.AccountManager;
 import com.google.gerrit.server.account.AuthRequest;
 import com.google.gerrit.server.account.AuthResult;
-import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gwtexpui.server.CacheHeaders;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -57,13 +57,13 @@ class HttpLoginServlet extends HttpServlet {
       LoggerFactory.getLogger(HttpLoginServlet.class);
 
   private final Provider<WebSession> webSession;
-  private final Provider<String> urlProvider;
+  private final CanonicalWebUrl urlProvider;
   private final AccountManager accountManager;
   private final HttpAuthFilter authFilter;
 
   @Inject
   HttpLoginServlet(final Provider<WebSession> webSession,
-      @CanonicalWebUrl @Nullable final Provider<String> urlProvider,
+      final CanonicalWebUrl urlProvider,
       final AccountManager accountManager,
       final HttpAuthFilter authFilter) {
     this.webSession = webSession;
@@ -121,7 +121,7 @@ class HttpLoginServlet extends HttpServlet {
     }
 
     final StringBuilder rdr = new StringBuilder();
-    rdr.append(urlProvider.get());
+    rdr.append(urlProvider.get(req));
     rdr.append('#');
     if (arsp.isNew() && !token.startsWith(PageLinks.REGISTER + "/")) {
       rdr.append(PageLinks.REGISTER);

gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/ldap/LdapLoginServlet.java

@@ -17,6 +17,7 @@ package com.google.gerrit.httpd.auth.ldap;
 import com.google.common.base.Objects;
 import com.google.common.base.Strings;
 import com.google.gerrit.common.PageLinks;
+import com.google.gerrit.httpd.CanonicalWebUrl;
 import com.google.gerrit.httpd.HtmlDomUtil;
 import com.google.gerrit.httpd.WebSession;
 import com.google.gerrit.httpd.template.SiteHeaderFooter;
@@ -26,7 +27,7 @@ import com.google.gerrit.server.account.AccountUserNameException;
 import com.google.gerrit.server.account.AuthRequest;
 import com.google.gerrit.server.account.AuthResult;
 import com.google.gerrit.server.auth.AuthenticationUnavailableException;
-import com.google.gerrit.server.config.CanonicalWebUrl;
+import com.google.gerrit.server.config.SitePaths;
 import com.google.gwtexpui.server.CacheHeaders;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -55,28 +56,24 @@ class LdapLoginServlet extends HttpServlet {
 
   private final AccountManager accountManager;
   private final Provider<WebSession> webSession;
-  private final Provider<String> urlProvider;
+  private final CanonicalWebUrl urlProvider;
   private final SiteHeaderFooter headers;
 
   @Inject
   LdapLoginServlet(AccountManager accountManager,
       Provider<WebSession> webSession,
-      @CanonicalWebUrl @Nullable Provider<String> urlProvider,
+      CanonicalWebUrl urlProvider,
       SiteHeaderFooter headers) {
     this.accountManager = accountManager;
     this.webSession = webSession;
     this.urlProvider = urlProvider;
     this.headers = headers;
-
-    if (Strings.isNullOrEmpty(urlProvider.get())) {
-      log.error("gerrit.canonicalWebUrl must be set in gerrit.config");
-    }
   }
 
   private void sendForm(HttpServletRequest req, HttpServletResponse res,
       @Nullable String errorMessage) throws IOException {
     String self = req.getRequestURI();
-    String cancel = Objects.firstNonNull(urlProvider.get(), "/");
+    String cancel = Objects.firstNonNull(urlProvider.get(req), "/");
     String token = getToken(req);
     if (!token.equals("/")) {
       cancel += "#" + token;
@@ -146,11 +143,10 @@ class LdapLoginServlet extends HttpServlet {
       return;
     }
 
-    String token = getToken(req);
     StringBuilder dest = new StringBuilder();
-    dest.append(urlProvider.get());
+    dest.append(urlProvider.get(req));
     dest.append('#');
-    dest.append(token);
+    dest.append(getToken(req));
 
     CacheHeaders.setNotCacheable(res);
     webSession.get().login(ares, "1".equals(remember));

gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/LoginForm.java

@@ -161,7 +161,7 @@ class LoginForm extends HttpServlet {
       remember = false;
     }
 
-    DiscoveryResult r = impl.discover(id, mode, remember, token);
+    DiscoveryResult r = impl.discover(req, id, mode, remember, token);
     switch (r.status) {
       case VALID:
         redirect(r, res);

gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java

@@ -16,6 +16,7 @@ package com.google.gerrit.httpd.auth.openid;
 
 import com.google.gerrit.common.PageLinks;
 import com.google.gerrit.common.auth.openid.OpenIdUrls;
+import com.google.gerrit.httpd.CanonicalWebUrl;
 import com.google.gerrit.httpd.WebSession;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.IdentifiedUser;
@@ -24,7 +25,6 @@ import com.google.gerrit.server.account.AccountException;
 import com.google.gerrit.server.account.AccountManager;
 import com.google.gerrit.server.auth.openid.OpenIdProviderPattern;
 import com.google.gerrit.server.config.AuthConfig;
-import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.config.ConfigUtil;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gwtorm.client.KeyUtil;
@@ -63,7 +63,6 @@ import java.net.URL;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
-import javax.annotation.Nullable;
 import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -93,7 +92,7 @@ class OpenIdServiceImpl {
 
   private final Provider<WebSession> webSession;
   private final Provider<IdentifiedUser> identifiedUser;
-  private final Provider<String> urlProvider;
+  private final CanonicalWebUrl urlProvider;
   private final AccountManager accountManager;
   private final ConsumerManager manager;
   private final List<OpenIdProviderPattern> allowedOpenIDs;
@@ -105,7 +104,7 @@ class OpenIdServiceImpl {
   @Inject
   OpenIdServiceImpl(final Provider<WebSession> cf,
       final Provider<IdentifiedUser> iu,
-      @CanonicalWebUrl @Nullable final Provider<String> up,
+      CanonicalWebUrl up,
       @GerritServerConfig final Config config, final AuthConfig ac,
       final AccountManager am) throws ConsumerException, MalformedURLException {
 
@@ -145,10 +144,10 @@ class OpenIdServiceImpl {
   }
 
   @SuppressWarnings("unchecked")
-  DiscoveryResult discover(final String openidIdentifier, final SignInMode mode,
-      final boolean remember, final String returnToken) {
+  DiscoveryResult discover(HttpServletRequest req, String openidIdentifier,
+      final SignInMode mode, final boolean remember, final String returnToken) {
     final State state;
-    state = init(openidIdentifier, mode, remember, returnToken);
+    state = init(req, openidIdentifier, mode, remember, returnToken);
     if (state == null) {
       return new DiscoveryResult(DiscoveryResult.Status.NO_PROVIDER);
     }
@@ -235,7 +234,7 @@ class OpenIdServiceImpl {
       return;
     }
 
-    state = init(rediscoverIdentifier, mode, remember, returnToken);
+    state = init(req, rediscoverIdentifier, mode, remember, returnToken);
     if (state == null) {
       // Re-discovery must have failed, we can't run a login.
       //
@@ -482,7 +481,7 @@ class OpenIdServiceImpl {
     }
 
     final StringBuilder rdr = new StringBuilder();
-    rdr.append(urlProvider.get());
+    rdr.append(urlProvider.get(req));
     rdr.append('#');
     if (isNew && !token.startsWith(PageLinks.REGISTER + "/")) {
       rdr.append(PageLinks.REGISTER);
@@ -507,7 +506,7 @@ class OpenIdServiceImpl {
       webSession.get().logout();
     }
     final StringBuilder rdr = new StringBuilder();
-    rdr.append(urlProvider.get());
+    rdr.append(urlProvider.get(req));
     rdr.append('#');
     rdr.append("SignInFailure");
     rdr.append(',');
@@ -517,8 +516,8 @@ class OpenIdServiceImpl {
     rsp.sendRedirect(rdr.toString());
   }
 
-  private State init(final String openidIdentifier, final SignInMode mode,
-      final boolean remember, final String returnToken) {
+  private State init(HttpServletRequest req, final String openidIdentifier,
+      final SignInMode mode, final boolean remember, final String returnToken) {
     final List<?> list;
     try {
       list = manager.discover(openidIdentifier);
@@ -530,7 +529,7 @@ class OpenIdServiceImpl {
       return null;
     }
 
-    final String contextUrl = urlProvider.get();
+    final String contextUrl = urlProvider.get(req);
     final DiscoveryInformation discovered = manager.associate(list);
     final UrlEncoded retTo = new UrlEncoded(contextUrl + RETURN_URL);
     retTo.put(P_MODE, mode.name());