Support changing Google Account identity strings
If auth.allowGoogleAccountUpgrade is set to true we now allow the user's OpenID identity token to change during sign-in, such as if the server hostname was recently changed. Change-Id: I5d88ce8d39186ef22fd47b257a3c791c2b2c12bd Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
		| @@ -131,24 +131,18 @@ By default this is false (no agreements are used). | |||||||
|  |  | ||||||
| auth.allowGoogleAccountUpgrade:: | auth.allowGoogleAccountUpgrade:: | ||||||
| + | + | ||||||
| Allow old Gerrit1 users to seamlessly upgrade from Google Accounts | Allows Google Account users to automatically update their Gerrit | ||||||
| on Google App Engine to OpenID authentication.  This should only be | account when/if their Google Account OpenID identity token changes. | ||||||
| set to true on a Gerrit2 database that was imported from a Gerrit1 | Identity tokens can change if the server changes hostnames, or | ||||||
| database run on Google App Engine.  Having it enabled incurs an | for other reasons known only to Google.  The upgrade path works | ||||||
| extra database query when new Google Account users register with | by matching users by email address if the identity is not present, | ||||||
| the Gerrit2 server. | and then changing the identity. | ||||||
| + | + | ||||||
| Its strongly encouraged to unset this once the following query | This setting also permits old Gerrit 1.x users to seamlessly upgrade | ||||||
| drops to 0, or close to 0: | from Google Accounts on Google App Engine to OpenID authentication. | ||||||
| + | + | ||||||
| ==== | Having this enabled incurs an extra database query when Google | ||||||
|   SELECT COUNT(*) FROM account_external_ids e | Account users register with the Gerrit2 server. | ||||||
|   WHERE e.external_id LIKE 'Google Account %' |  | ||||||
|   AND NOT EXISTS (SELECT 1 |  | ||||||
|     FROM account_external_ids o |  | ||||||
|     WHERE o.account_id = e.account_id |  | ||||||
| 	AND o.external_id LIKE '%.google.com%/id?id=%'); |  | ||||||
| ==== |  | ||||||
| + | + | ||||||
| By default, unset/false. | By default, unset/false. | ||||||
|  |  | ||||||
|   | |||||||
| @@ -25,6 +25,7 @@ import com.google.gwtorm.client.Transaction; | |||||||
| import com.google.inject.Inject; | import com.google.inject.Inject; | ||||||
| import com.google.inject.Singleton; | import com.google.inject.Singleton; | ||||||
|  |  | ||||||
|  | import java.util.ArrayList; | ||||||
| import java.util.Collections; | import java.util.Collections; | ||||||
| import java.util.List; | import java.util.List; | ||||||
|  |  | ||||||
| @@ -175,16 +176,57 @@ public class AccountManager { | |||||||
|     if (authConfig.isAllowGoogleAccountUpgrade() |     if (authConfig.isAllowGoogleAccountUpgrade() | ||||||
|         && who.isScheme(OpenIdUtil.URL_GOOGLE + "?") |         && who.isScheme(OpenIdUtil.URL_GOOGLE + "?") | ||||||
|         && who.getEmailAddress() != null) { |         && who.getEmailAddress() != null) { | ||||||
|       final List<AccountExternalId> legacyAppEngine = |       final List<AccountExternalId> openId = new ArrayList<AccountExternalId>(); | ||||||
|           db.accountExternalIds().byExternal( |       final List<AccountExternalId> v1 = new ArrayList<AccountExternalId>(); | ||||||
|               AccountExternalId.LEGACY_GAE + who.getEmailAddress()).toList(); |  | ||||||
|  |  | ||||||
|       if (legacyAppEngine.size() == 1) { |       for (final AccountExternalId extId : db.accountExternalIds() | ||||||
|  |           .byEmailAddress(who.getEmailAddress())) { | ||||||
|  |         if (extId.isScheme(OpenIdUtil.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()); | ||||||
|  |         newId.setLastUsedOn(); | ||||||
|  |  | ||||||
|  |         if (openId.size() == 1) { | ||||||
|  |           final AccountExternalId oldId = openId.get(0); | ||||||
|  |           final Transaction txn = db.beginTransaction(); | ||||||
|  |           db.accountExternalIds().delete(Collections.singleton(oldId), txn); | ||||||
|  |           db.accountExternalIds().insert(Collections.singleton(newId), txn); | ||||||
|  |           txn.commit(); | ||||||
|  |         } else { | ||||||
|  |           db.accountExternalIds().insert(Collections.singleton(newId)); | ||||||
|  |         } | ||||||
|  |         return new AuthResult(accountId, false); | ||||||
|  |  | ||||||
|  |       } else if (v1.size() == 1) { | ||||||
|         // Exactly one user was imported from Gerrit 1.x with this email |         // Exactly one user was imported from Gerrit 1.x with this email | ||||||
|         // address. Upgrade their account by deleting the legacy import |         // address. Upgrade their account by deleting the legacy import | ||||||
|         // identity and creating a new identity matching the token we have. |         // identity and creating a new identity matching the token we have. | ||||||
|         // |         // | ||||||
|         final AccountExternalId oldId = legacyAppEngine.get(0); |         final AccountExternalId oldId = v1.get(0); | ||||||
|         final AccountExternalId newId = createId(oldId.getAccountId(), who); |         final AccountExternalId newId = createId(oldId.getAccountId(), who); | ||||||
|         newId.setEmailAddress(who.getEmailAddress()); |         newId.setEmailAddress(who.getEmailAddress()); | ||||||
|         newId.setLastUsedOn(); |         newId.setLastUsedOn(); | ||||||
| @@ -194,7 +236,7 @@ public class AccountManager { | |||||||
|         txn.commit(); |         txn.commit(); | ||||||
|         return new AuthResult(newId.getAccountId(), false); |         return new AuthResult(newId.getAccountId(), false); | ||||||
|  |  | ||||||
|       } else if (legacyAppEngine.size() > 1) { |       } else if (v1.size() > 1) { | ||||||
|         throw new AccountException("Multiple Gerrit 1.x accounts found"); |         throw new AccountException("Multiple Gerrit 1.x accounts found"); | ||||||
|       } |       } | ||||||
|     } |     } | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Shawn O. Pearce
					Shawn O. Pearce