opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.10' into stable-2.11

Browse Source 
* stable-2.10:
  OAuth: Simplify protocol implementation
  Allow to link user identity to another OAuth provider
  Update JGit to the 4.0.0.201505050340-m2 version
  Hybrid OpenID/OAuth: Support switching identities
  Hybrid OpenID/OAuth: Allow to link identity accross protocols
  OAuth: Check for session validity during logout

Change-Id: I9da0073a72d8c4327313405b11c66cd253ff640b
This commit is contained in:
David Pursehouse 2015-05-07 10:37:48 +09:00
commit 885ad8a7ef
8 changed files with 135 additions and 99 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyIdentitiesScreen.java
View File

@ -59,7 +59,8 @@ public class MyIdentitiesScreen extends SettingsScreen {
});
add(deleteIdentity);
if (Gerrit.getConfig().getAuthType() == AuthType.OPENID) {
if (Gerrit.getConfig().getAuthType() == AuthType.OPENID
|| Gerrit.getConfig().getAuthType() == AuthType.OAUTH) {
Button linkIdentity = new Button(Util.C.buttonLinkIdentity());
linkIdentity.addClickHandler(new ClickHandler() {
@Override

4
gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthLogoutServlet.java
View File

@ -50,7 +50,9 @@ class OAuthLogoutServlet extends HttpLogoutServlet {
protected void doLogout(HttpServletRequest req, HttpServletResponse rsp)
throws IOException {
super.doLogout(req, rsp);
oauthSession.get().logout();
if (req.getSession(false) != null) {
oauthSession.get().logout();
}
}
}

115
gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java
View File

@ -26,11 +26,14 @@ import com.google.gerrit.extensions.restapi.Url;
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;
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.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.servlet.SessionScoped;
import org.apache.commons.codec.binary.Base64;
@ -52,18 +55,22 @@ class OAuthSession {
private static final SecureRandom randomState = newRandomGenerator();
private final String state;
private final DynamicItem<WebSession> webSession;
private final Provider<IdentifiedUser> identifiedUser;
private final AccountManager accountManager;
private final CanonicalWebUrl urlProvider;
private OAuthServiceProvider serviceProvider;
private OAuthToken token;
private OAuthUserInfo user;
private String redirectToken;
private boolean linkMode;
@Inject
OAuthSession(DynamicItem<WebSession> webSession,
Provider<IdentifiedUser> identifiedUser,
AccountManager accountManager,
CanonicalWebUrl urlProvider) {
this.state = generateRandomState();
this.identifiedUser = identifiedUser;
this.webSession = webSession;
this.accountManager = accountManager;
this.urlProvider = urlProvider;
@ -79,10 +86,6 @@ class OAuthSession {
boolean login(HttpServletRequest request, HttpServletResponse response,
OAuthServiceProvider oauth) throws IOException {
if (isLoggedIn()) {
return true;
}
log.debug("Login " + this);
if (isOAuthFinal(request)) {
@ -122,46 +125,19 @@ class OAuthSession {
private void authenticateAndRedirect(HttpServletRequest req,
HttpServletResponse rsp) throws IOException {
com.google.gerrit.server.account.AuthRequest areq =
new com.google.gerrit.server.account.AuthRequest(user.getExternalId());
AuthRequest areq = new AuthRequest(user.getExternalId());
AuthResult arsp;
try {
String claimedIdentifier = user.getClaimedIdentity();
Account.Id actualId = accountManager.lookup(user.getExternalId());
if (!Strings.isNullOrEmpty(claimedIdentifier)) {
Account.Id claimedId = accountManager.lookup(claimedIdentifier);
if (claimedId != null && actualId != null) {
if (claimedId.equals(actualId)) {
// Both link to the same account, that's what we expected.
log.debug("OAuth2: claimed identity equals current id");
} else {
// This is (for now) a fatal error. There are two records
// for what might be the same user.
//
log.error("OAuth accounts disagree over user identity:\n"
+ " Claimed ID: " + claimedId + " is " + claimedIdentifier
+ "\n" + " Delgate ID: " + actualId + " is "
+ user.getExternalId());
rsp.sendError(HttpServletResponse.SC_FORBIDDEN);
return;
}
} else if (claimedId != null && actualId == null) {
// Claimed account already exists: link to it.
//
log.info("OAuth2: linking claimed identity to {}",
claimedId.toString());
try {
accountManager.link(claimedId, areq);
} catch (OrmException e) {
log.error("Cannot link: " + user.getExternalId()
+ " to user identity:\n"
+ " Claimed ID: " + claimedId + " is " + claimedIdentifier);
rsp.sendError(HttpServletResponse.SC_FORBIDDEN);
return;
}
if (!authenticateWithIdentityClaimedDuringHandshake(areq, rsp,
claimedIdentifier)) {
return;
}
} else if (linkMode) {
if (!authenticateWithLinkedIdentity(areq, rsp)) {
return;
}
} else {
log.debug("OAuth2: claimed identity is empty");
}
areq.setUserName(user.getUserName());
areq.setEmailAddress(user.getEmailAddress());
@ -181,6 +157,59 @@ class OAuthSession {
rsp.sendRedirect(rdr.toString());
}
private boolean authenticateWithIdentityClaimedDuringHandshake(
AuthRequest req, HttpServletResponse rsp, String claimedIdentifier)
throws AccountException, IOException {
Account.Id claimedId = accountManager.lookup(claimedIdentifier);
Account.Id actualId = accountManager.lookup(user.getExternalId());
if (claimedId != null && actualId != null) {
if (claimedId.equals(actualId)) {
// Both link to the same account, that's what we expected.
log.debug("OAuth2: claimed identity equals current id");
} else {
// This is (for now) a fatal error. There are two records
// for what might be the same user.
//
log.error("OAuth accounts disagree over user identity:\n"
+ " Claimed ID: " + claimedId + " is " + claimedIdentifier
+ "\n" + " Delgate ID: " + actualId + " is "
+ user.getExternalId());
rsp.sendError(HttpServletResponse.SC_FORBIDDEN);
return false;
}
} else if (claimedId != null && actualId == null) {
// Claimed account already exists: link to it.
//
log.info("OAuth2: linking claimed identity to {}",
claimedId.toString());
try {
accountManager.link(claimedId, req);
} catch (OrmException e) {
log.error("Cannot link: " + user.getExternalId()
+ " to user identity:\n"
+ " Claimed ID: " + claimedId + " is " + claimedIdentifier);
rsp.sendError(HttpServletResponse.SC_FORBIDDEN);
return false;
}
}
return true;
}
private boolean authenticateWithLinkedIdentity(AuthRequest areq,
HttpServletResponse rsp) throws AccountException, IOException {
try {
accountManager.link(identifiedUser.get().getAccountId(), areq);
} catch (OrmException e) {
log.error("Cannot link: " + user.getExternalId()
+ " to user identity: " + identifiedUser.get().getAccountId());
rsp.sendError(HttpServletResponse.SC_FORBIDDEN);
return false;
} finally {
linkMode = false;
}
return true;
}
void logout() {
token = null;
user = null;
@ -224,4 +253,12 @@ class OAuthSession {
public OAuthServiceProvider getServiceProvider() {
return serviceProvider;
}
public void setLinkMode(boolean linkMode) {
this.linkMode = linkMode;
}
public boolean isLinkMode() {
return linkMode;
}
}

29
gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthWebFilter.java
View File

@ -23,7 +23,6 @@ import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.httpd.HtmlDomUtil;
import com.google.gerrit.httpd.LoginUrlToken;
import com.google.gerrit.httpd.template.SiteHeaderFooter;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.inject.Inject;
import com.google.inject.Provider;
@ -48,7 +47,6 @@ import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
@Singleton
/* OAuth web filter uses active OAuth session to perform OAuth requests */
@ -56,7 +54,6 @@ class OAuthWebFilter implements Filter {
static final String GERRIT_LOGIN = "/login";
private final Provider<String> urlProvider;
private final Provider<CurrentUser> currentUserProvider;
private final Provider<OAuthSession> oauthSessionProvider;
private final DynamicMap<OAuthServiceProvider> oauthServiceProviders;
private final SiteHeaderFooter header;
@ -64,12 +61,10 @@ class OAuthWebFilter implements Filter {
@Inject
OAuthWebFilter(@CanonicalWebUrl @Nullable Provider<String> urlProvider,
Provider<CurrentUser> currentUserProvider,
DynamicMap<OAuthServiceProvider> oauthServiceProviders,
Provider<OAuthSession> oauthSessionProvider,
SiteHeaderFooter header) {
this.urlProvider = urlProvider;
this.currentUserProvider = currentUserProvider;
this.oauthServiceProviders = oauthServiceProviders;
this.oauthSessionProvider = oauthSessionProvider;
this.header = header;
@ -88,30 +83,20 @@ class OAuthWebFilter implements Filter {
public void doFilter(ServletRequest request, ServletResponse response,
FilterChain chain) throws IOException, ServletException {
HttpServletRequest httpRequest = (HttpServletRequest) request;
HttpSession httpSession = ((HttpServletRequest) request).getSession(false);
OAuthSession oauthSession = oauthSessionProvider.get();
if (currentUserProvider.get().isIdentifiedUser()) {
if (httpSession != null) {
httpSession.invalidate();
}
chain.doFilter(request, response);
return;
} else {
if (oauthSession.isLoggedIn()) {
oauthSession.logout();
}
}
HttpServletResponse httpResponse = (HttpServletResponse) response;
OAuthSession oauthSession = oauthSessionProvider.get();
if (request.getParameter("link") != null) {
oauthSession.setLinkMode(true);
oauthSession.setServiceProvider(null);
}
String provider = httpRequest.getParameter("provider");
OAuthServiceProvider service = ssoProvider == null
? oauthSession.getServiceProvider()
: ssoProvider;
if ((isGerritLogin(httpRequest)
|| oauthSession.isOAuthFinal(httpRequest))
&& !oauthSession.isLoggedIn()) {
if (isGerritLogin(httpRequest) || oauthSession.isOAuthFinal(httpRequest)) {
if (service == null && Strings.isNullOrEmpty(provider)) {
selectProvider(httpRequest, httpResponse, null);
return;

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

@ -175,9 +175,9 @@ class LoginForm extends HttpServlet {
oauthSession.logout();
}
if ((isGerritLogin(req)
|| oauthSession.isOAuthFinal(req))
&& !oauthSession.isLoggedIn()) {
|| oauthSession.isOAuthFinal(req))) {
oauthSession.setServiceProvider(oauthProvider);
oauthSession.setLinkMode(link);
oauthSession.login(req, res, oauthProvider);
}
}
@ -304,7 +304,7 @@ class LoginForm extends HttpServlet {
oauthServiceProviders.byPlugin(pluginName);
for (Map.Entry<String, Provider<OAuthServiceProvider>> e
: m.entrySet()) {
addProvider(providers, pluginName, e.getKey(),
addProvider(providers, link, pluginName, e.getKey(),
e.getValue().get().getName());
}
}
@ -327,13 +327,18 @@ class LoginForm extends HttpServlet {
}
}
private static void addProvider(Element form, String pluginName,
String id, String serviceName) {
private static void addProvider(Element form, boolean link,
String pluginName, String id, String serviceName) {
Element div = form.getOwnerDocument().createElement("div");
div.setAttribute("id", id);
Element hyperlink = form.getOwnerDocument().createElement("a");
hyperlink.setAttribute("href", String.format("?id=%s_%s",
StringBuilder u = new StringBuilder(String.format("?id=%s_%s",
pluginName, id));
if (link) {
u.append("&link");
}
hyperlink.setAttribute("href", u.toString());
hyperlink.setTextContent(serviceName +
" (" + pluginName + " plugin)");
div.appendChild(hyperlink);

32
gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OAuthSessionOverOpenID.java
View File

@ -27,11 +27,13 @@ import com.google.gerrit.httpd.CanonicalWebUrl;
import com.google.gerrit.httpd.LoginUrlToken;
import com.google.gerrit.httpd.WebSession;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountException;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AuthResult;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.servlet.SessionScoped;
import org.apache.commons.codec.binary.Base64;
@ -55,19 +57,23 @@ class OAuthSessionOverOpenID {
private static final SecureRandom randomState = newRandomGenerator();
private final String state;
private final DynamicItem<WebSession> webSession;
private final Provider<IdentifiedUser> identifiedUser;
private final AccountManager accountManager;
private final CanonicalWebUrl urlProvider;
private OAuthServiceProvider serviceProvider;
private OAuthToken token;
private OAuthUserInfo user;
private String redirectToken;
private boolean linkMode;
@Inject
OAuthSessionOverOpenID(DynamicItem<WebSession> webSession,
Provider<IdentifiedUser> identifiedUser,
AccountManager accountManager,
CanonicalWebUrl urlProvider) {
this.state = generateRandomState();
this.webSession = webSession;
this.identifiedUser = identifiedUser;
this.accountManager = accountManager;
this.urlProvider = urlProvider;
}
@ -82,10 +88,6 @@ class OAuthSessionOverOpenID {
boolean login(HttpServletRequest request, HttpServletResponse response,
OAuthServiceProvider oauth) throws IOException {
if (isLoggedIn()) {
return true;
}
log.debug("Login " + this);
if (isOAuthFinal(request)) {
@ -96,7 +98,6 @@ class OAuthSessionOverOpenID {
log.debug("Login-Retrieve-User " + this);
token = oauth.getAccessToken(new OAuthVerifier(request.getParameter("code")));
user = oauth.getUserInfo(token);
if (isLoggedIn()) {
@ -124,6 +125,7 @@ class OAuthSessionOverOpenID {
try {
String claimedIdentifier = user.getClaimedIdentity();
Account.Id actualId = accountManager.lookup(user.getExternalId());
// Use case 1: claimed identity was provided during handshake phase
if (!Strings.isNullOrEmpty(claimedIdentifier)) {
Account.Id claimedId = accountManager.lookup(claimedIdentifier);
if (claimedId != null && actualId != null) {
@ -153,6 +155,18 @@ class OAuthSessionOverOpenID {
return;
}
}
} else if (linkMode) {
// Use case 2: link mode activated from the UI
try {
accountManager.link(identifiedUser.get().getAccountId(), areq);
} catch (OrmException e) {
log.error("Cannot link: " + user.getExternalId()
+ " to user identity: " + identifiedUser.get().getAccountId());
rsp.sendError(HttpServletResponse.SC_FORBIDDEN);
return;
} finally {
linkMode = false;
}
}
areq.setUserName(user.getUserName());
areq.setEmailAddress(user.getEmailAddress());
@ -213,4 +227,12 @@ class OAuthSessionOverOpenID {
public OAuthServiceProvider getServiceProvider() {
return serviceProvider;
}
public void setLinkMode(boolean linkMode) {
this.linkMode = linkMode;
}
public boolean isLinkMode() {
return linkMode;
}
}

20
gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OAuthWebFilterOverOpenID.java
View File

@ -17,7 +17,6 @@ package com.google.gerrit.httpd.auth.openid;
import com.google.common.collect.Iterables;
import com.google.gerrit.extensions.auth.oauth.OAuthServiceProvider;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.server.CurrentUser;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@ -34,7 +33,6 @@ import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
/** OAuth web filter uses active OAuth session to perform OAuth requests */
@ -42,16 +40,13 @@ import javax.servlet.http.HttpSession;
class OAuthWebFilterOverOpenID implements Filter {
static final String GERRIT_LOGIN = "/login";
private final Provider<CurrentUser> currentUserProvider;
private final Provider<OAuthSessionOverOpenID> oauthSessionProvider;
private final DynamicMap<OAuthServiceProvider> oauthServiceProviders;
private OAuthServiceProvider ssoProvider;
@Inject
OAuthWebFilterOverOpenID(Provider<CurrentUser> currentUserProvider,
DynamicMap<OAuthServiceProvider> oauthServiceProviders,
OAuthWebFilterOverOpenID(DynamicMap<OAuthServiceProvider> oauthServiceProviders,
Provider<OAuthSessionOverOpenID> oauthSessionProvider) {
this.currentUserProvider = currentUserProvider;
this.oauthServiceProviders = oauthServiceProviders;
this.oauthSessionProvider = oauthSessionProvider;
}
@ -69,15 +64,6 @@ class OAuthWebFilterOverOpenID implements Filter {
public void doFilter(ServletRequest request, ServletResponse response,
FilterChain chain) throws IOException, ServletException {
HttpServletRequest httpRequest = (HttpServletRequest) request;
HttpSession httpSession = ((HttpServletRequest) request).getSession(false);
if (currentUserProvider.get().isIdentifiedUser()) {
if (httpSession != null) {
httpSession.invalidate();
}
chain.doFilter(request, response);
return;
}
HttpServletResponse httpResponse = (HttpServletResponse) response;
OAuthSessionOverOpenID oauthSession = oauthSessionProvider.get();
@ -85,9 +71,7 @@ class OAuthWebFilterOverOpenID implements Filter {
? oauthSession.getServiceProvider()
: ssoProvider;
if ((isGerritLogin(httpRequest)
|| oauthSession.isOAuthFinal(httpRequest))
&& !oauthSession.isLoggedIn()) {
if (isGerritLogin(httpRequest) || oauthSession.isOAuthFinal(httpRequest)) {
if (service == null) {
throw new IllegalStateException("service is unknown");
}

14
lib/jgit/BUCK
View File

@ -1,13 +1,13 @@
include_defs('//lib/maven.defs')
REPO = GERRIT # Leave here even if set to MAVEN_CENTRAL.
VERS = '3.7.0.201502260915-r.58-g65c379e'
REPO = MAVEN_CENTRAL # Leave here even if set to MAVEN_CENTRAL.
VERS = '4.0.0.201505050340-m2'
maven_jar(
name = 'jgit',
id = 'org.eclipse.jgit:org.eclipse.jgit:' + VERS,
bin_sha1 = '8fc9620ec499169facad3355f7417eb6a8aff511',
src_sha1 = '40bd9ae8af8e0b03eb4e43f44f5feda8b7325221',
bin_sha1 = '1cc3120d39ed2b55584e631634e65c5d2e6c1cf7',
src_sha1 = '425f578cc9d5ccb8f3b050a5ab1e2d7a0becb25d',
license = 'jgit',
repository = REPO,
unsign = True,
@ -22,7 +22,7 @@ maven_jar(
maven_jar(
name = 'jgit-servlet',
id = 'org.eclipse.jgit:org.eclipse.jgit.http.server:' + VERS,
sha1 = 'cecc2b9c0b94455348c3a0c63eb83f72cc595757',
sha1 = '2a9f55d1d92afef795542b995db6ab261007857f',
license = 'jgit',
repository = REPO,
deps = [':jgit'],
@ -36,7 +36,7 @@ maven_jar(
maven_jar(
name = 'jgit-archive',
id = 'org.eclipse.jgit:org.eclipse.jgit.archive:' + VERS,
sha1 = '7ccc7c78bf47566045ea7a3c08508ba18e4684ca',
sha1 = 'ee3954753067818f8f734981a01c13ac33425f2c',
license = 'jgit',
repository = REPO,
deps = [':jgit',
@ -53,7 +53,7 @@ maven_jar(
maven_jar(
name = 'junit',
id = 'org.eclipse.jgit:org.eclipse.jgit.junit:' + VERS,
sha1 = '87d64d722447dc3971ace30d2a72593c72a4d05f',
sha1 = '6cc19f8f0a1791e26d4225625ecba6a31d9b830e',
license = 'DO_NOT_DISTRIBUTE',
repository = REPO,
unsign = True,