Merge "Remove auth.allowGoogleAccountUpgrade feature"

This commit is contained in:
Shawn Pearce 2014-05-28 05:40:38 +00:00 committed by Gerrit Code Review
commit 08ca4b704c
5 changed files with 1 additions and 112 deletions

View File

@ -391,23 +391,6 @@ To enable the actual usage of contributor agreement the project
specific config option in the `project.config` must be set: specific config option in the `project.config` must be set:
link:config-project-config.html[receive.requireContributorAgreement]. 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:: [[auth.trustContainerAuth]]auth.trustContainerAuth::
+ +
If true then it is the responsibility of the container hosting If true then it is the responsibility of the container hosting

View File

@ -249,9 +249,6 @@ public class MyIdentitiesScreen extends SettingsScreen {
} else if (k.isScheme(OpenIdUrls.URL_YAHOO)) { } else if (k.isScheme(OpenIdUrls.URL_YAHOO)) {
return OpenIdUtil.C.nameYahoo(); return OpenIdUtil.C.nameYahoo();
} else if (k.isScheme(AccountExternalId.LEGACY_GAE)) {
return OpenIdUtil.C.nameGoogle() + " (Imported from Google AppEngine)";
} else { } else {
return k.getExternalId(); return k.getExternalId();
} }

View File

@ -36,9 +36,6 @@ public final class AccountExternalId {
/** Scheme for the username used to authenticate an account, e.g. over SSH. */ /** Scheme for the username used to authenticate an account, e.g. over SSH. */
public static final String SCHEME_USERNAME = "username:"; 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<com.google.gwtorm.client.Key<?>> { public static class Key extends StringKey<com.google.gwtorm.client.Key<?>> {
private static final long serialVersionUID = 1L; private static final long serialVersionUID = 1L;

View File

@ -14,7 +14,6 @@
package com.google.gerrit.server.account; 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.AccessSection;
import com.google.gerrit.common.data.GlobalCapability; import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.data.Permission; 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.client.AccountGroupMemberAudit;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser; 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.project.ProjectCache;
import com.google.gerrit.server.util.TimeUtil; import com.google.gerrit.server.util.TimeUtil;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@ -38,9 +36,7 @@ import com.google.inject.Singleton;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicBoolean;
/** Tracks authentication related details for user accounts. */ /** Tracks authentication related details for user accounts. */
@ -52,7 +48,6 @@ public class AccountManager {
private final SchemaFactory<ReviewDb> schema; private final SchemaFactory<ReviewDb> schema;
private final AccountCache byIdCache; private final AccountCache byIdCache;
private final AccountByEmailCache byEmailCache; private final AccountByEmailCache byEmailCache;
private final AuthConfig authConfig;
private final Realm realm; private final Realm realm;
private final IdentifiedUser.GenericFactory userFactory; private final IdentifiedUser.GenericFactory userFactory;
private final ChangeUserName.Factory changeUserNameFactory; private final ChangeUserName.Factory changeUserNameFactory;
@ -62,14 +57,13 @@ public class AccountManager {
@Inject @Inject
AccountManager(final SchemaFactory<ReviewDb> schema, AccountManager(final SchemaFactory<ReviewDb> schema,
final AccountCache byIdCache, final AccountByEmailCache byEmailCache, final AccountCache byIdCache, final AccountByEmailCache byEmailCache,
final AuthConfig authConfig, final Realm accountMapper, final Realm accountMapper,
final IdentifiedUser.GenericFactory userFactory, final IdentifiedUser.GenericFactory userFactory,
final ChangeUserName.Factory changeUserNameFactory, final ChangeUserName.Factory changeUserNameFactory,
final ProjectCache projectCache) throws OrmException { final ProjectCache projectCache) throws OrmException {
this.schema = schema; this.schema = schema;
this.byIdCache = byIdCache; this.byIdCache = byIdCache;
this.byEmailCache = byEmailCache; this.byEmailCache = byEmailCache;
this.authConfig = authConfig;
this.realm = accountMapper; this.realm = accountMapper;
this.userFactory = userFactory; this.userFactory = userFactory;
this.changeUserNameFactory = changeUserNameFactory; this.changeUserNameFactory = changeUserNameFactory;
@ -197,67 +191,6 @@ public class AccountManager {
private AuthResult create(final ReviewDb db, final AuthRequest who) private AuthResult create(final ReviewDb db, final AuthRequest who)
throws OrmException, AccountException { throws OrmException, AccountException {
if (authConfig.isAllowGoogleAccountUpgrade()
&& who.isScheme(OpenIdUrls.URL_GOOGLE + "?")
&& who.getEmailAddress() != null) {
final List<AccountExternalId> openId = new ArrayList<>();
final List<AccountExternalId> 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.Id newId = new Account.Id(db.nextAccountId());
final Account account = new Account(newId, TimeUtil.nowTs()); final Account account = new Account(newId, TimeUtil.nowTs());
final AccountExternalId extId = createId(newId, who); final AccountExternalId extId = createId(newId, who);

View File

@ -54,8 +54,6 @@ public class AuthConfig {
private final SignedToken emailReg; private final SignedToken emailReg;
private final SignedToken restToken; private final SignedToken restToken;
private final boolean allowGoogleAccountUpgrade;
@Inject @Inject
AuthConfig(@GerritServerConfig final Config cfg) AuthConfig(@GerritServerConfig final Config cfg)
throws XsrfException { throws XsrfException {
@ -97,13 +95,6 @@ public class AuthConfig {
} else { } else {
restToken = null; restToken = null;
} }
if (authType == AuthType.OPENID) {
allowGoogleAccountUpgrade =
cfg.getBoolean("auth", "allowgoogleaccountupgrade", false);
} else {
allowGoogleAccountUpgrade = false;
}
} }
private static List<OpenIdProviderPattern> toPatterns(Config cfg, String name) { private static List<OpenIdProviderPattern> toPatterns(Config cfg, String name) {
@ -172,10 +163,6 @@ public class AuthConfig {
return restToken; return restToken;
} }
public boolean isAllowGoogleAccountUpgrade() {
return allowGoogleAccountUpgrade;
}
/** OpenID identities which the server permits for authentication. */ /** OpenID identities which the server permits for authentication. */
public List<OpenIdProviderPattern> getAllowedOpenIDs() { public List<OpenIdProviderPattern> getAllowedOpenIDs() {
return allowedOpenIDs; return allowedOpenIDs;
@ -237,14 +224,6 @@ public class AuthConfig {
} }
private boolean isTrusted(final AccountExternalId id) { 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)) { if (id.isScheme(AccountExternalId.SCHEME_MAILTO)) {
// mailto identities are created by sending a unique validation // mailto identities are created by sending a unique validation
// token to the address and asking them to come back to the site // token to the address and asking them to come back to the site