From 85510403edb60ee6146ce0354137f1b8028c8522 Mon Sep 17 00:00:00 2001 From: Thomas Draebing Date: Mon, 15 Mar 2021 08:03:00 +0100 Subject: Respect auth.userNameToLowerCase when creating accounts via REST or SSH The CreateAccount SSH command and REST API was able to create accounts with usernames that contained uppercase letters even if auth.userNameToLowerCase was set to true. However, such a user could never log in. The username should be all lower case in the notedb as it is done for accounts created during login with an IDP like LDAP, if ldap.localUsernameToLowerCase is set. If no display name (input.name) is set, the originally provided username will be used as a display name to conserve capitalization in the UI. Bug: Issue 14246 Change-Id: If0f120f188e9f5bdf8008c4e66a55568180e7351 --- Documentation/config-gerrit.txt | 4 ++- .../server/restapi/account/CreateAccount.java | 19 ++++++++-- .../acceptance/rest/account/CreateAccountIT.java | 41 ++++++++++++++++++++++ 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index e63e0bea39..53993b2673 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -612,7 +612,9 @@ set to `${sAMAccountName.toLowerCase}`). It is important that for all existing accounts this username is already in lower case. It is not possible to convert the usernames of the existing accounts to lower case because this would break the access to existing per-user -branches and Gerrit provides no tool to do such a conversion. +branches and Gerrit provides no tool to do such a conversion. Accounts +created using the REST API or the `create-account` SSH command will +be created with all lowercase characters, when this option is set. + Setting this parameter to `true` will prevent all users from login that have a non-lower-case username. diff --git a/java/com/google/gerrit/server/restapi/account/CreateAccount.java b/java/com/google/gerrit/server/restapi/account/CreateAccount.java index c110194335..6ad6d97743 100644 --- a/java/com/google/gerrit/server/restapi/account/CreateAccount.java +++ b/java/com/google/gerrit/server/restapi/account/CreateAccount.java @@ -44,6 +44,7 @@ import com.google.gerrit.server.account.AccountsUpdate; import com.google.gerrit.server.account.VersionedAuthorizedKeys; import com.google.gerrit.server.account.externalids.DuplicateExternalIdKeyException; import com.google.gerrit.server.account.externalids.ExternalId; +import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.group.GroupResolver; import com.google.gerrit.server.group.db.GroupsUpdate; import com.google.gerrit.server.group.db.InternalGroupUpdate; @@ -59,6 +60,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Set; import org.eclipse.jgit.errors.ConfigInvalidException; @@ -75,6 +77,7 @@ public class CreateAccount private final PluginSetContext externalIdCreators; private final Provider groupsUpdate; private final OutgoingEmailValidator validator; + private final AuthConfig authConfig; @Inject CreateAccount( @@ -86,7 +89,8 @@ public class CreateAccount AccountLoader.Factory infoLoader, PluginSetContext externalIdCreators, @UserInitiated Provider groupsUpdate, - OutgoingEmailValidator validator) { + OutgoingEmailValidator validator, + AuthConfig authConfig) { this.seq = seq; this.groupResolver = groupResolver; this.authorizedKeys = authorizedKeys; @@ -96,6 +100,7 @@ public class CreateAccount this.externalIdCreators = externalIdCreators; this.groupsUpdate = groupsUpdate; this.validator = validator; + this.authConfig = authConfig; } @Override @@ -109,14 +114,18 @@ public class CreateAccount public Response apply(IdString id, AccountInput input) throws BadRequestException, ResourceConflictException, UnprocessableEntityException, IOException, ConfigInvalidException, PermissionBackendException { - String username = id.get(); - if (input.username != null && !username.equals(input.username)) { + String username = applyCaseOfUsername(id.get()); + if (input.username != null && !username.equals(applyCaseOfUsername(input.username))) { throw new BadRequestException("username must match URL"); } if (!ExternalId.isValidUsername(username)) { throw new BadRequestException("Invalid username '" + username + "'"); } + if (input.name == null) { + input.name = input.username; + } + Set groups = parseGroups(input.groups); Account.Id accountId = Account.id(seq.nextAccountId()); @@ -175,6 +184,10 @@ public class CreateAccount return Response.created(info); } + private String applyCaseOfUsername(String username) { + return authConfig.isUserNameToLowerCase() ? username.toLowerCase(Locale.US) : username; + } + private Set parseGroups(List groups) throws UnprocessableEntityException { Set groupUuids = new HashSet<>(); diff --git a/javatests/com/google/gerrit/acceptance/rest/account/CreateAccountIT.java b/javatests/com/google/gerrit/acceptance/rest/account/CreateAccountIT.java index aca6c4cbac..1b83d92828 100644 --- a/javatests/com/google/gerrit/acceptance/rest/account/CreateAccountIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/account/CreateAccountIT.java @@ -17,6 +17,7 @@ package com.google.gerrit.acceptance.rest.account; import static com.google.common.truth.Truth8.assertThat; import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.GerritConfig; import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.extensions.api.accounts.AccountInput; import org.junit.Test; @@ -31,4 +32,44 @@ public class CreateAccountIT extends AbstractDaemonTest { r.assertCreated(); assertThat(accountCache.getByUsername(input.username)).isPresent(); } + + @Test + @GerritConfig(name = "auth.userNameToLowerCase", value = "false") + public void createAccountRestApiUserNameToLowerCaseFalse() throws Exception { + AccountInput input = new AccountInput(); + input.username = "JohnDoe"; + assertThat(accountCache.getByUsername(input.username)).isEmpty(); + RestResponse r = adminRestSession.put("/accounts/" + input.username, input); + r.assertCreated(); + assertThat(accountCache.getByUsername(input.username)).isPresent(); + } + + @Test + @GerritConfig(name = "auth.userNameToLowerCase", value = "true") + public void createAccountRestApiUserNameToLowerCaseTrue() throws Exception { + testUserNameToLowerCase("John1", "John1", "john1"); + assertThat(accountCache.getByUsername("John1")).isEmpty(); + + testUserNameToLowerCase("john2", "John2", "john2"); + assertThat(accountCache.getByUsername("John2")).isEmpty(); + + testUserNameToLowerCase("John3", "john3", "john3"); + assertThat(accountCache.getByUsername("John3")).isEmpty(); + + testUserNameToLowerCase("John4", "johN4", "john4"); + assertThat(accountCache.getByUsername("John4")).isEmpty(); + assertThat(accountCache.getByUsername("johN4")).isEmpty(); + + testUserNameToLowerCase("john5", "john5", "john5"); + } + + private void testUserNameToLowerCase(String usernameUrl, String usernameInput, String usernameDb) + throws Exception { + AccountInput input = new AccountInput(); + input.username = usernameInput; + assertThat(accountCache.getByUsername(usernameDb)).isEmpty(); + RestResponse r = adminRestSession.put("/accounts/" + usernameUrl, input); + r.assertCreated(); + assertThat(accountCache.getByUsername(usernameDb)).isPresent(); + } } -- cgit v1.2.3