opendev/gerrit
Code Issues Proposed changes

Fix login servlets when canonicalWebUrl is not set

Browse Source 
Each login servlet knows enough about the incoming request to be able
to not need the canonical web address for redirection purposes.  In
the case gerrit.canonicalWebUrl is not set, use the incoming request
to build up the URL.

This solution is a work-around for the fact that somewhere before 2.5
Colby broke the HttpServletRequest scope based version of the
@CanonicalWebUrl provider.  Because Guice cannot supply the request in
some contexts we pass the request into the provider as an argument.

Long term all of these authentication methods will be ejected into
their own plugins and it will be possible to revisit how this
configuration is handled.

Change-Id: I0e00b89020860a02b5d6ea77da5c784f5f0bb1b8
This commit is contained in:
Shawn Pearce
2013-05-01 17:11:28 +01:00
parent 8ea0c8b12b
commit ace2c6d9f4
6 changed files with 73 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -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);

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

@@ -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());