From 97d073567c58e5895a8a1d38a5de41a81b84da98 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Thu, 17 Jul 2014 08:31:06 +0100 Subject: [PATCH] Allow storing user external federated auth token for auth (e.g. GitHub) When using Gerrit with GitHub authentication, we need to "remember" the user's federated auth token in order to allow Gerrit to impersonate the user when communicating with GitHub. The user is always in control as the external identity is listed in the user's external ids and can be removed / revoked at any time. This change is *mandatory* for being able to integrate the GitHub group backend system properly, as GitHub does not allow anonymous browsing of organisations and groups (very hard 60 calls/hour cap) and Gerrit needs to impersonate the user to understand to which organisation / group he belongs to. NOTE: each user can have at most 1 external identity as at the moment is not possibly to authenticate against multiple external systems at once. Change-Id: I8df2540a5643c95d51b26e9d3e32f6cd1cac9f10 --- Documentation/config-gerrit.txt | 14 ++++++ .../httpd/auth/container/HttpAuthFilter.java | 10 ++++ .../auth/container/HttpLoginServlet.java | 27 +++++++++++ .../reviewdb/client/AccountExternalId.java | 15 ++++-- .../gerrit/server/account/AccountManager.java | 47 +++++++++++++++++++ .../gerrit/server/config/AuthConfig.java | 6 +++ 6 files changed, 116 insertions(+), 3 deletions(-) diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index bd82a92d69..adba1aad1a 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -273,6 +273,20 @@ If set, Gerrit trusts and enforces the user's e-mail using the HTTP header and disables the ability to manually modify or register other e-mails from the contact information page. +[[auth.httpExternalIdHeader]]auth.httpExternalIdHeader:: ++ +HTTP header to retrieve the user's external identification token. +Only used if `auth.type` is set to `HTTP`. ++ +If set, Gerrit adds the value contained in the HTTP header to the +user's identity. Typical use is with a federated identity token from +an external system (e.g. GitHub OAuth 2.0 authentication) where +the user's auth token exchanged during authentication handshake +needs to be used for authenticated communication to the external +system later on. ++ +Example: `auth.httpExternalIdHeader: X-GitHub-OTP` + [[auth.loginUrl]]auth.loginUrl:: + URL to redirect a browser to after the end-user has clicked on the 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 5f93a56cee..19c8342351 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 @@ -63,6 +63,7 @@ class HttpAuthFilter implements Filter { private final String loginHeader; private final String displaynameHeader; private final String emailHeader; + private final String externalIdHeader; @Inject HttpAuthFilter(final DynamicItem webSession, @@ -82,6 +83,7 @@ class HttpAuthFilter implements Filter { AUTHORIZATION); displaynameHeader = emptyToNull(authConfig.getHttpDisplaynameHeader()); emailHeader = emptyToNull(authConfig.getHttpEmailHeader()); + externalIdHeader = emptyToNull(authConfig.getHttpExternalIdHeader()); } @Override @@ -194,6 +196,14 @@ class HttpAuthFilter implements Filter { } } + String getRemoteExternalIdToken(HttpServletRequest req) { + if(externalIdHeader != null) { + return emptyToNull(req.getHeader(externalIdHeader)); + } else { + return null; + } + } + String getLoginHeader() { return loginHeader; } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/container/HttpLoginServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/container/HttpLoginServlet.java index 734addb09e..6d8a0cda07 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/container/HttpLoginServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/container/HttpLoginServlet.java @@ -14,18 +14,22 @@ package com.google.gerrit.httpd.auth.container; +import static com.google.gerrit.reviewdb.client.AccountExternalId.SCHEME_EXTERNAL; + import com.google.gerrit.common.PageLinks; import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.httpd.CanonicalWebUrl; import com.google.gerrit.httpd.HtmlDomUtil; import com.google.gerrit.httpd.LoginUrlToken; import com.google.gerrit.httpd.WebSession; +import com.google.gerrit.reviewdb.client.AccountExternalId; 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.AuthConfig; import com.google.gwtexpui.server.CacheHeaders; +import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -122,6 +126,20 @@ class HttpLoginServlet extends HttpServlet { return; } + String remoteExternalId = authFilter.getRemoteExternalIdToken(req); + if (remoteExternalId != null) { + try { + log.debug("Associating external identity \"{}\" to user \"{}\"", + remoteExternalId, user); + updateRemoteExternalId(arsp, remoteExternalId); + } catch (AccountException | OrmException e) { + log.error("Unable to associate external identity \"" + remoteExternalId + + "\" to user \"" + user + "\"", e); + rsp.sendError(HttpServletResponse.SC_FORBIDDEN); + return; + } + } + final StringBuilder rdr = new StringBuilder(); if (arsp.isNew() && authConfig.getRegisterPageUrl() != null) { rdr.append(authConfig.getRegisterPageUrl()); @@ -137,6 +155,15 @@ class HttpLoginServlet extends HttpServlet { rsp.sendRedirect(rdr.toString()); } + private void updateRemoteExternalId(AuthResult arsp, String remoteAuthToken) + throws AccountException, OrmException { + AccountExternalId remoteAuthExtId = + new AccountExternalId(arsp.getAccountId(), new AccountExternalId.Key( + SCHEME_EXTERNAL, remoteAuthToken)); + accountManager.updateLink(arsp.getAccountId(), + new AuthRequest(remoteAuthExtId.getExternalId())); + } + private void replace(Document doc, String name, String value) { Element e = HtmlDomUtil.find(doc, name); if (e != null) { diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountExternalId.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountExternalId.java index 8181d502af..8f9c726a9e 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountExternalId.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountExternalId.java @@ -36,6 +36,9 @@ public final class AccountExternalId { /** Scheme for the username used to authenticate an account, e.g. over SSH. */ public static final String SCHEME_USERNAME = "username:"; + /** Scheme for external auth used during authentication, e.g. OAuth Token */ + public static final String SCHEME_EXTERNAL = "external:"; + public static class Key extends StringKey> { private static final long serialVersionUID = 1L; @@ -65,6 +68,11 @@ public final class AccountExternalId { protected void set(String newValue) { externalId = newValue; } + + public String getScheme() { + int c = externalId.indexOf(':'); + return 0 < c ? externalId.substring(0, c) : null; + } } @Column(id = 1, name = Column.NONE) @@ -126,9 +134,10 @@ public final class AccountExternalId { } public String getSchemeRest() { - String id = getExternalId(); - int c = id.indexOf(':'); - return 0 < c ? id.substring(c + 1) : null; + String scheme = key.getScheme(); + return null != scheme + ? getExternalId().substring(scheme.length() + 1) + : null; } public String getPassword() { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java index 4d13276f65..62615c6a2c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java @@ -29,6 +29,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.project.ProjectCache; import com.google.gwtorm.server.OrmException; +import com.google.gwtorm.server.ResultSet; import com.google.gwtorm.server.SchemaFactory; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -36,7 +37,9 @@ import com.google.inject.Singleton; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; /** Tracks authentication related details for user accounts. */ @@ -359,6 +362,50 @@ public class AccountManager { } } + /** + * Update the link to another unique authentication identity to an existing account. + * + * Existing external identities with the same scheme will be removed and replaced + * with the new one. + * + * @param to account to link the identity onto. + * @param who the additional identity. + * @return the result of linking the identity to the user. + * @throws OrmException + * @throws AccountException the identity belongs to a different account, or it + * cannot be linked at this time. + */ + public AuthResult updateLink(Account.Id to, AuthRequest who) throws OrmException, + AccountException { + ReviewDb db = schema.open(); + try { + AccountExternalId.Key key = id(who); + List filteredKeysByScheme = + filterKeysByScheme(key.getScheme(), db.accountExternalIds() + .byAccount(to)); + if (!filteredKeysByScheme.isEmpty() + && (filteredKeysByScheme.size() > 1 || !filteredKeysByScheme + .contains(key))) { + db.accountExternalIds().deleteKeys(filteredKeysByScheme); + } + byIdCache.evict(to); + return link(to, who); + } finally { + db.close(); + } + } + + private List filterKeysByScheme( + String keyScheme, ResultSet externalIds) { + List filteredExternalIds = new ArrayList<>(); + for (AccountExternalId accountExternalId : externalIds) { + if (accountExternalId.isScheme(keyScheme)) { + filteredExternalIds.add(accountExternalId.getKey()); + } + } + return filteredExternalIds; + } + /** * Unlink an authentication identity from an existing account. * diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java index cf4ab5b216..72f55d41c6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java @@ -38,6 +38,7 @@ public class AuthConfig { private final String httpHeader; private final String httpDisplaynameHeader; private final String httpEmailHeader; + private final String httpExternalIdHeader; private final String registerPageUrl; private final boolean trustContainerAuth; private final boolean enableRunAs; @@ -61,6 +62,7 @@ public class AuthConfig { httpHeader = cfg.getString("auth", null, "httpheader"); httpDisplaynameHeader = cfg.getString("auth", null, "httpdisplaynameheader"); httpEmailHeader = cfg.getString("auth", null, "httpemailheader"); + httpExternalIdHeader = cfg.getString("auth", null, "httpexternalidheader"); loginUrl = cfg.getString("auth", null, "loginurl"); logoutUrl = cfg.getString("auth", null, "logouturl"); registerPageUrl = cfg.getString("auth", null, "registerPageUrl"); @@ -131,6 +133,10 @@ public class AuthConfig { return httpEmailHeader; } + public String getHttpExternalIdHeader() { + return httpExternalIdHeader; + } + public String getLoginUrl() { return loginUrl; }