diff options
author | David Ostrovsky <david@ostrovsky.org> | 2015-03-15 08:46:08 +0100 |
---|---|---|
committer | David Ostrovsky <david@ostrovsky.org> | 2015-03-20 07:31:40 +0100 |
commit | 043c85728abbd67b8d5e7fca1b2f9b8f50178cc7 (patch) | |
tree | 3fef1838399e651b2ff867b578824edaaf753fb1 | |
parent | 735f7fa45831679ff378ce53c84a53633ebf7ac2 (diff) |
OAuth: Allow to link claimed identity to existing accounts
One of use cases OAuth plugin based authentication scheme is aiming
to support is switch from deprecated OpenID provider to OAuth scheme
offered by the same povider. In this specific case the database is
already pre-populated with OpenID accounts. After switching the auth
scheme to OAuth all existing accounts must be linked to the new OAuth
identity.
To support linking new OAuth identity to existing accounts, user info
extension point is extended with claimed identity attribute. When
passed, the account for this identity is looked up and when found new
OAuth identity is linked to it.
Change-Id: Ia6489762dd370bfbbaa16a7418cd3106d2d1112a
3 files changed, 47 insertions, 4 deletions
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthUserInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthUserInfo.java index 23a7bec4b4..388ce361e5 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthUserInfo.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthUserInfo.java @@ -20,15 +20,18 @@ public class OAuthUserInfo { private final String userName; private final String emailAddress; private final String displayName; + private final String claimedIdentity; public OAuthUserInfo(String externalId, String userName, String emailAddress, - String displayName) { + String displayName, + String claimedIdentity) { this.externalId = externalId; this.userName = userName; this.emailAddress = emailAddress; this.displayName = displayName; + this.claimedIdentity = claimedIdentity; } public String getExternalId() { @@ -46,4 +49,8 @@ public class OAuthUserInfo { public String getDisplayName() { return displayName; } + + public String getClaimedIdentity() { + return claimedIdentity; + } } diff --git a/gerrit-oauth/BUCK b/gerrit-oauth/BUCK index 4641e8168e..fa5a8e2cb1 100644 --- a/gerrit-oauth/BUCK +++ b/gerrit-oauth/BUCK @@ -11,9 +11,11 @@ java_library( '//gerrit-common:annotations', '//gerrit-extension-api:api', '//gerrit-httpd:httpd', + '//gerrit-reviewdb:server', '//gerrit-server:server', '//lib:gson', '//lib:guava', + '//lib:gwtorm', '//lib/commons:codec', '//lib/guice:guice', '//lib/guice:guice-servlet', diff --git a/gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java b/gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java index d625e02abd..27a1f57144 100644 --- a/gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java +++ b/gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java @@ -23,9 +23,11 @@ import com.google.gerrit.extensions.auth.oauth.OAuthUserInfo; import com.google.gerrit.extensions.auth.oauth.OAuthVerifier; import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.httpd.WebSession; +import com.google.gerrit.reviewdb.client.Account; 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.servlet.SessionScoped; @@ -113,11 +115,43 @@ class OAuthSession { throws IOException { com.google.gerrit.server.account.AuthRequest areq = new com.google.gerrit.server.account.AuthRequest(user.getExternalId()); - areq.setUserName(user.getUserName()); - areq.setEmailAddress(user.getEmailAddress()); - areq.setDisplayName(user.getDisplayName()); 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. + } 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. + // + 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; + } + } + } + areq.setUserName(user.getUserName()); + areq.setEmailAddress(user.getEmailAddress()); + areq.setDisplayName(user.getDisplayName()); arsp = accountManager.authenticate(areq); } catch (AccountException e) { log.error("Unable to authenticate user \"" + user + "\"", e); |