From 9e668aa79062657e17fb561bf97d6498021ab54f Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Fri, 23 May 2014 11:48:59 -0700 Subject: [PATCH] Remove auth.allowGoogleAccountUpgrade feature Its been more than 5 years since Gerrit ran on Google AppEngine. Assume everyone has upgraded their installations to a modern 2.x based server, and will not need to this upgrade path enabled. Change-Id: I9c3d7792189cd60cdf51db09eceeeb7961dd73b0 --- Documentation/config-gerrit.txt | 17 ----- .../client/account/MyIdentitiesScreen.java | 3 - .../reviewdb/client/AccountExternalId.java | 3 - .../gerrit/server/account/AccountManager.java | 69 +------------------ .../gerrit/server/config/AuthConfig.java | 21 ------ 5 files changed, 1 insertion(+), 112 deletions(-) diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 49fe77f926..2f551f2125 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -391,23 +391,6 @@ To enable the actual usage of contributor agreement the project specific config option in the `project.config` must be set: link:config-project-config.html[receive.requireContributorAgreement]. -auth.allowGoogleAccountUpgrade:: -+ -Allows Google Account users to automatically update their Gerrit -account when/if their Google Account OpenID identity token changes. -Identity tokens can change if the server changes hostnames, or -for other reasons known only to Google. The upgrade path works -by matching users by email address if the identity is not present, -and then changing the identity. -+ -This setting also permits old Gerrit 1.x users to seamlessly upgrade -from Google Accounts on Google App Engine to OpenID authentication. -+ -Having this enabled incurs an extra database query when Google -Account users register with the Gerrit server. -+ -By default, unset/false. - [[auth.trustContainerAuth]]auth.trustContainerAuth:: + If true then it is the responsibility of the container hosting diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyIdentitiesScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyIdentitiesScreen.java index 4703ef7e24..a84ca99323 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyIdentitiesScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyIdentitiesScreen.java @@ -249,9 +249,6 @@ public class MyIdentitiesScreen extends SettingsScreen { } else if (k.isScheme(OpenIdUrls.URL_YAHOO)) { return OpenIdUtil.C.nameYahoo(); - } else if (k.isScheme(AccountExternalId.LEGACY_GAE)) { - return OpenIdUtil.C.nameGoogle() + " (Imported from Google AppEngine)"; - } else { return k.getExternalId(); } 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 4be5113e77..8181d502af 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,9 +36,6 @@ public final class AccountExternalId { /** Scheme for the username used to authenticate an account, e.g. over SSH. */ public static final String SCHEME_USERNAME = "username:"; - /** Very old scheme from Gerrit Code Review 1.x imports. */ - public static final String LEGACY_GAE = "Google Account "; - public static class Key extends StringKey> { private static final long serialVersionUID = 1L; 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 3ba307607d..77ebe0f352 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 @@ -14,7 +14,6 @@ package com.google.gerrit.server.account; -import com.google.gerrit.common.auth.openid.OpenIdUrls; import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.GlobalCapability; import com.google.gerrit.common.data.Permission; @@ -27,7 +26,6 @@ import com.google.gerrit.reviewdb.client.AccountGroupMember; import com.google.gerrit.reviewdb.client.AccountGroupMemberAudit; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.util.TimeUtil; import com.google.gwtorm.server.OrmException; @@ -38,9 +36,7 @@ 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. */ @@ -52,7 +48,6 @@ public class AccountManager { private final SchemaFactory schema; private final AccountCache byIdCache; private final AccountByEmailCache byEmailCache; - private final AuthConfig authConfig; private final Realm realm; private final IdentifiedUser.GenericFactory userFactory; private final ChangeUserName.Factory changeUserNameFactory; @@ -62,14 +57,13 @@ public class AccountManager { @Inject AccountManager(final SchemaFactory schema, final AccountCache byIdCache, final AccountByEmailCache byEmailCache, - final AuthConfig authConfig, final Realm accountMapper, + final Realm accountMapper, final IdentifiedUser.GenericFactory userFactory, final ChangeUserName.Factory changeUserNameFactory, final ProjectCache projectCache) throws OrmException { this.schema = schema; this.byIdCache = byIdCache; this.byEmailCache = byEmailCache; - this.authConfig = authConfig; this.realm = accountMapper; this.userFactory = userFactory; this.changeUserNameFactory = changeUserNameFactory; @@ -197,67 +191,6 @@ public class AccountManager { private AuthResult create(final ReviewDb db, final AuthRequest who) throws OrmException, AccountException { - if (authConfig.isAllowGoogleAccountUpgrade() - && who.isScheme(OpenIdUrls.URL_GOOGLE + "?") - && who.getEmailAddress() != null) { - final List openId = new ArrayList<>(); - final List v1 = new ArrayList<>(); - - for (final AccountExternalId extId : db.accountExternalIds() - .byEmailAddress(who.getEmailAddress())) { - if (extId.isScheme(OpenIdUrls.URL_GOOGLE + "?")) { - openId.add(extId); - } else if (extId.isScheme(AccountExternalId.LEGACY_GAE)) { - v1.add(extId); - } - } - - if (!openId.isEmpty()) { - // The user has already registered with an OpenID from Google, but - // Google may have changed the user's OpenID identity if this server - // name has changed. Insert a new identity for the user. - // - final Account.Id accountId = openId.get(0).getAccountId(); - - if (openId.size() > 1) { - // Validate all matching identities are actually the same user. - // - for (final AccountExternalId extId : openId) { - if (!accountId.equals(extId.getAccountId())) { - throw new AccountException("Multiple user accounts for " - + who.getEmailAddress() + " using Google Accounts provider"); - } - } - } - - final AccountExternalId newId = createId(accountId, who); - newId.setEmailAddress(who.getEmailAddress()); - - db.accountExternalIds().upsert(Collections.singleton(newId)); - if (openId.size() == 1) { - final AccountExternalId oldId = openId.get(0); - db.accountExternalIds().delete(Collections.singleton(oldId)); - } - return new AuthResult(accountId, newId.getKey(), false); - - } else if (v1.size() == 1) { - // Exactly one user was imported from Gerrit 1.x with this email - // address. Upgrade their account by deleting the legacy import - // identity and creating a new identity matching the token we have. - // - final AccountExternalId oldId = v1.get(0); - final AccountExternalId newId = createId(oldId.getAccountId(), who); - newId.setEmailAddress(who.getEmailAddress()); - - db.accountExternalIds().upsert(Collections.singleton(newId)); - db.accountExternalIds().delete(Collections.singleton(oldId)); - return new AuthResult(newId.getAccountId(), newId.getKey(), false); - - } else if (v1.size() > 1) { - throw new AccountException("Multiple Gerrit 1.x accounts found"); - } - } - final Account.Id newId = new Account.Id(db.nextAccountId()); final Account account = new Account(newId, TimeUtil.nowTs()); final AccountExternalId extId = createId(newId, who); 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 1d640c2d1a..b1ec6e4941 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 @@ -54,8 +54,6 @@ public class AuthConfig { private final SignedToken emailReg; private final SignedToken restToken; - private final boolean allowGoogleAccountUpgrade; - @Inject AuthConfig(@GerritServerConfig final Config cfg) throws XsrfException { @@ -97,13 +95,6 @@ public class AuthConfig { } else { restToken = null; } - - if (authType == AuthType.OPENID) { - allowGoogleAccountUpgrade = - cfg.getBoolean("auth", "allowgoogleaccountupgrade", false); - } else { - allowGoogleAccountUpgrade = false; - } } private static List toPatterns(Config cfg, String name) { @@ -172,10 +163,6 @@ public class AuthConfig { return restToken; } - public boolean isAllowGoogleAccountUpgrade() { - return allowGoogleAccountUpgrade; - } - /** OpenID identities which the server permits for authentication. */ public List getAllowedOpenIDs() { return allowedOpenIDs; @@ -237,14 +224,6 @@ public class AuthConfig { } private boolean isTrusted(final AccountExternalId id) { - if (id.isScheme(AccountExternalId.LEGACY_GAE)) { - // Assume this is a trusted token, its a legacy import from - // a fairly well respected provider and only takes effect if - // the administrator has the import still enabled - // - return isAllowGoogleAccountUpgrade(); - } - if (id.isScheme(AccountExternalId.SCHEME_MAILTO)) { // mailto identities are created by sending a unique validation // token to the address and asking them to come back to the site