diff options
7 files changed, 103 insertions, 7 deletions
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdCaseSensitivityMigrator.java b/java/com/google/gerrit/server/account/externalids/ExternalIdCaseSensitivityMigrator.java index 204ec2d525..a59e9353f7 100644 --- a/java/com/google/gerrit/server/account/externalids/ExternalIdCaseSensitivityMigrator.java +++ b/java/com/google/gerrit/server/account/externalids/ExternalIdCaseSensitivityMigrator.java @@ -93,7 +93,8 @@ public class ExternalIdCaseSensitivityMigrator { ExternalId.Key updatedKey = keyFactory.create(extId.key().scheme(), extId.key().id()); ExternalId.Key oldKey = keyFactory.create(extId.key().scheme(), extId.key().id(), !isUserNameCaseInsensitive); - if (!oldKey.sha1().getName().equals(updatedKey.sha1().getName())) { + if (!oldKey.sha1().getName().equals(updatedKey.sha1().getName()) + && !extId.key().sha1().getName().equals(updatedKey.sha1().getName())) { logger.atInfo().log("Converting note name of external ID: %s", oldKey); ExternalId updatedExtId = externalIdFactory.create( diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdFactory.java b/java/com/google/gerrit/server/account/externalids/ExternalIdFactory.java index fa68cad1cf..bd9c7dfc69 100644 --- a/java/com/google/gerrit/server/account/externalids/ExternalIdFactory.java +++ b/java/com/google/gerrit/server/account/externalids/ExternalIdFactory.java @@ -267,6 +267,8 @@ public class ExternalIdFactory { "Neither case sensitive nor case insensitive SHA1 of external ID '%s' match note ID '%s'", externalIdKeyStr, noteId)); } + externalIdKey = + externalIdKeyFactory.create(externalIdKey.scheme(), externalIdKey.id(), false); } String email = diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIds.java b/java/com/google/gerrit/server/account/externalids/ExternalIds.java index 4e1e524187..9450ff5cbd 100644 --- a/java/com/google/gerrit/server/account/externalids/ExternalIds.java +++ b/java/com/google/gerrit/server/account/externalids/ExternalIds.java @@ -19,6 +19,7 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSetMultimap; import com.google.gerrit.entities.Account; +import com.google.gerrit.server.config.AuthConfig; import com.google.inject.Inject; import com.google.inject.Singleton; import java.io.IOException; @@ -35,11 +36,19 @@ import org.eclipse.jgit.lib.ObjectId; public class ExternalIds { private final ExternalIdReader externalIdReader; private final ExternalIdCache externalIdCache; + private final AuthConfig authConfig; + private final ExternalIdKeyFactory externalIdKeyFactory; @Inject - public ExternalIds(ExternalIdReader externalIdReader, ExternalIdCache externalIdCache) { + public ExternalIds( + ExternalIdReader externalIdReader, + ExternalIdCache externalIdCache, + ExternalIdKeyFactory externalIdKeyFactory, + AuthConfig authConfig) { this.externalIdReader = externalIdReader; this.externalIdCache = externalIdCache; + this.externalIdKeyFactory = externalIdKeyFactory; + this.authConfig = authConfig; } /** Returns all external IDs. */ @@ -54,7 +63,15 @@ public class ExternalIds { /** Returns the specified external ID. */ public Optional<ExternalId> get(ExternalId.Key key) throws IOException { - return externalIdCache.byKey(key); + Optional<ExternalId> externalId = Optional.empty(); + if (authConfig.isUserNameCaseInsensitiveMigrationMode()) { + externalId = + externalIdCache.byKey(externalIdKeyFactory.create(key.scheme(), key.id(), false)); + } + if (!externalId.isPresent()) { + externalId = externalIdCache.byKey(key); + } + return externalId; } /** Returns the specified external ID from the given revision. */ diff --git a/java/com/google/gerrit/server/account/externalids/PasswordVerifier.java b/java/com/google/gerrit/server/account/externalids/PasswordVerifier.java index 33443c1024..eb2bea9c4f 100644 --- a/java/com/google/gerrit/server/account/externalids/PasswordVerifier.java +++ b/java/com/google/gerrit/server/account/externalids/PasswordVerifier.java @@ -20,6 +20,7 @@ import com.google.common.base.Strings; import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.Nullable; import com.google.gerrit.server.account.HashedPassword; +import com.google.gerrit.server.config.AuthConfig; import com.google.inject.Inject; import java.util.Collection; @@ -29,9 +30,12 @@ public class PasswordVerifier { private final ExternalIdKeyFactory externalIdKeyFactory; + private AuthConfig authConfig; + @Inject - public PasswordVerifier(ExternalIdKeyFactory externalIdKeyFactory) { + public PasswordVerifier(ExternalIdKeyFactory externalIdKeyFactory, AuthConfig authConfig) { this.externalIdKeyFactory = externalIdKeyFactory; + this.authConfig = authConfig; } /** Returns {@code true} if there is an external ID matching both the username and password. */ @@ -40,13 +44,23 @@ public class PasswordVerifier { if (password == null) { return false; } + for (ExternalId id : externalIds) { // Only process the "username:$USER" entry, which is unique. - if (!id.isScheme(SCHEME_USERNAME) - || !id.key().equals(externalIdKeyFactory.create(SCHEME_USERNAME, username))) { + if (!id.isScheme(SCHEME_USERNAME)) { continue; } + if (!id.key().equals(externalIdKeyFactory.create(SCHEME_USERNAME, username))) { + if (!authConfig.isUserNameCaseInsensitiveMigrationMode()) { + continue; + } + + if (!id.key().equals(externalIdKeyFactory.create(SCHEME_USERNAME, username, false))) { + continue; + } + } + String hashedStr = id.password(); if (!Strings.isNullOrEmpty(hashedStr)) { try { diff --git a/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java b/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java index 7a76e09e63..1e89a8505b 100644 --- a/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java @@ -94,6 +94,8 @@ import org.junit.Test; public class ExternalIdIT extends AbstractDaemonTest { private static final boolean IS_USER_NAME_CASE_INSENSITIVE_MIGRATION_MODE = false; + private static final boolean CASE_SENSITIVE_USERNAME = false; + private static final boolean CASE_INSENSITIVE_USERNAME = true; @Inject @ServerInitiated private Provider<AccountsUpdate> accountsUpdateProvider; @Inject private ExternalIds externalIds; @@ -904,6 +906,34 @@ public class ExternalIdIT extends AbstractDaemonTest { @Test @GerritConfig(name = "auth.userNameCaseInsensitive", value = "true") @GerritConfig(name = "auth.userNameCaseInsensitiveMigrationMode", value = "true") + public void shouldTolerateDuplicateExternalIdsWhenInMigrationMode() throws Exception { + Account.Id firstAccountId = Account.id(1); + Account.Id secondAccountId = Account.id(2); + + try (Repository allUsersRepo = repoManager.openRepository(allUsers); + MetaDataUpdate md = metaDataUpdateFactory.create(allUsers)) { + ExternalIdNotes extIdNotes = externalIdNotesFactory.load(allUsersRepo); + + createExternalId( + md, extIdNotes, SCHEME_GERRIT, "janedoe", firstAccountId, CASE_SENSITIVE_USERNAME); + createExternalId( + md, extIdNotes, SCHEME_GERRIT, "JaneDoe", secondAccountId, CASE_SENSITIVE_USERNAME); + + ExternalId.Key firstAccountExternalId = + externalIdKeyFactory.create(SCHEME_GERRIT, "janedoe", CASE_INSENSITIVE_USERNAME); + assertThat(externalIds.get(firstAccountExternalId).get().accountId()) + .isEqualTo(firstAccountId); + + ExternalId.Key secondAccountExternalId = + externalIdKeyFactory.create(SCHEME_GERRIT, "JaneDoe", CASE_INSENSITIVE_USERNAME); + assertThat(externalIds.get(secondAccountExternalId).get().accountId()) + .isEqualTo(secondAccountId); + } + } + + @Test + @GerritConfig(name = "auth.userNameCaseInsensitive", value = "true") + @GerritConfig(name = "auth.userNameCaseInsensitiveMigrationMode", value = "true") public void createCaseInsensitiveMigrationModeExternalIdAccountDuringTheMigration() throws Exception { Account.Id accountId = Account.id(66); @@ -982,6 +1012,12 @@ public class ExternalIdIT extends AbstractDaemonTest { assertThat(getAccountId(extIdNotes, scheme, id.toLowerCase())).isEqualTo(accountId.get()); } + /** + * Create external id object + * + * <p>This method skips gerrit.config auth.userNameCaseInsensitiveMigrationMode and allow to + * create case sensitive/insensitive external id + */ protected void createExternalId( MetaDataUpdate md, ExternalIdNotes extIdNotes, diff --git a/javatests/com/google/gerrit/acceptance/server/account/externalids/OnlineExternalIdCaseSensivityMigratorIT.java b/javatests/com/google/gerrit/acceptance/server/account/externalids/OnlineExternalIdCaseSensivityMigratorIT.java index 998f57928f..4eac16fa5f 100644 --- a/javatests/com/google/gerrit/acceptance/server/account/externalids/OnlineExternalIdCaseSensivityMigratorIT.java +++ b/javatests/com/google/gerrit/acceptance/server/account/externalids/OnlineExternalIdCaseSensivityMigratorIT.java @@ -99,6 +99,32 @@ public class OnlineExternalIdCaseSensivityMigratorIT extends AbstractDaemonTest @Test @GerritConfig(name = "auth.userNameCaseInsensitive", value = "true") @GerritConfig(name = "auth.userNameCaseInsensitiveMigrationMode", value = "true") + public void shouldNotThrowExceptionDuringTheMigrationForExternalIdsWithCaseInsensitiveSha1() + throws IOException, ConfigInvalidException { + + final boolean caseInsensitiveUserName = true; + + try (Repository allUsersRepo = repoManager.openRepository(allUsers); + MetaDataUpdate md = metaDataUpdateFactory.create(allUsers)) { + ExternalIdNotes extIdNotes = externalIdNotesFactory.load(allUsersRepo); + + createExternalId(md, extIdNotes, SCHEME_GERRIT, "JonDoe", accountId, caseInsensitiveUserName); + createExternalId( + md, extIdNotes, SCHEME_USERNAME, "JonDoe", accountId, caseInsensitiveUserName); + + assertThat(getExactExternalId(extIdNotes, SCHEME_GERRIT, "JonDoe").isPresent()).isFalse(); + assertThat(getExactExternalId(extIdNotes, SCHEME_GERRIT, "jondoe").isPresent()).isTrue(); + + assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "JonDoe").isPresent()).isFalse(); + assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "jondoe").isPresent()).isTrue(); + + objectUnderTest.migrate(); + } + } + + @Test + @GerritConfig(name = "auth.userNameCaseInsensitive", value = "true") + @GerritConfig(name = "auth.userNameCaseInsensitiveMigrationMode", value = "true") public void shouldNotCreateDuplicateExternaIdNotesWhenUpdatingAccount() throws IOException, ConfigInvalidException { try (Repository allUsersRepo = repoManager.openRepository(allUsers); diff --git a/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java b/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java index 28b714625d..25334fd415 100644 --- a/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java +++ b/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java @@ -104,7 +104,7 @@ public class ProjectBasicAuthFilterTest { extIdKeyFactory = new ExternalIdKeyFactory(new ExternalIdKeyFactory.ConfigImpl(authConfig)); extIdFactory = new ExternalIdFactory(extIdKeyFactory, authConfig); authRequestFactory = new AuthRequest.Factory(extIdKeyFactory); - pwdVerifier = new PasswordVerifier(extIdKeyFactory); + pwdVerifier = new PasswordVerifier(extIdKeyFactory, authConfig); authSuccessful = new AuthResult(AUTH_ACCOUNT_ID, extIdKeyFactory.create("username", AUTH_USER), false); |