Merge changes from topics "use-ExternalId-isValidUsername", "move-username-regexs-to-ExternalId" into stable-2.14

* changes:
  Use ExternalId.isValidUsername instead of ExternalId.USER_NAME_PATTERN_REGEX
  Move regular expressions for user name from Account to ExternalId
  AccountIT: Add basic tests for creating user with {in}valid username

11 files changed, 70 insertions, 39 deletions

diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 61a38302d1..f97cb2aa6a 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -852,6 +852,29 @@ public class AccountIT extends AbstractDaemonTest {
     gApi.accounts().id(admin.username).index();
   }
 
+  @Test
+  public void createUserWithValidUsername() throws Exception {
+    ImmutableList<String> names =
+        ImmutableList.of(
+            "user@domain", "user-name", "user_name", "1234", "user1234", "1234@domain");
+    for (String name : names) {
+      gApi.accounts().create(name);
+    }
+  }
+
+  @Test
+  public void createUserWithInvalidUsername() throws Exception {
+    ImmutableList<String> invalidNames = ImmutableList.of("@", "@foo", "-", "-foo", "_", "_foo");
+    for (String name : invalidNames) {
+      try {
+        gApi.accounts().create(name);
+        fail(String.format("Expected BadRequestException for username [%s]", name));
+      } catch (BadRequestException e) {
+        assertThat(e).hasMessageThat().contains("must contain only");
+      }
+    }
+  }
+
   private void assertSequenceNumbers(List<SshKeyInfo> sshKeys) {
     int seq = 1;
     for (SshKeyInfo key : sshKeys) {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/UsernameField.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/UsernameField.java
index 839a3e60cd..a717a9038e 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/UsernameField.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/UsernameField.java
@@ -23,7 +23,6 @@ import com.google.gerrit.client.rpc.NativeString;
 import com.google.gerrit.client.rpc.RestApi;
 import com.google.gerrit.client.ui.OnEditEnabler;
 import com.google.gerrit.extensions.client.AccountFieldName;
-import com.google.gerrit.reviewdb.client.Account;
 import com.google.gwt.event.dom.client.ClickEvent;
 import com.google.gwt.event.dom.client.ClickHandler;
 import com.google.gwt.event.dom.client.KeyCodes;
@@ -38,6 +37,12 @@ import com.google.gwtexpui.globalkey.client.NpTextBox;
 import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
 
 class UsernameField extends Composite {
+  // If these regular expressions are modified the same modifications should be done to the
+  // corresponding regular expressions in the
+  // com.google.gerrit.server.account.ExternalId class.
+  private static final String USER_NAME_PATTERN_FIRST_REGEX = "[a-zA-Z0-9]";
+  private static final String USER_NAME_PATTERN_REST_REGEX = "[a-zA-Z0-9._@-]";
+
   private CopyableLabel userNameLbl;
   private NpTextBox userNameTxt;
   private Button setUserName;
@@ -185,9 +190,9 @@ class UsernameField extends Composite {
           final TextBox box = (TextBox) event.getSource();
           final String re;
           if (box.getCursorPos() == 0) {
-            re = Account.USER_NAME_PATTERN_FIRST;
+            re = USER_NAME_PATTERN_FIRST_REGEX;
           } else {
-            re = Account.USER_NAME_PATTERN_REST;
+            re = USER_NAME_PATTERN_REST_REGEX;
           }
           if (!String.valueOf(code).matches("^" + re + "$")) {
             event.preventDefault();
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Account.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Account.java
index f1c7056d0f..d1fcbe0c45 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Account.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Account.java
@@ -44,31 +44,6 @@ import java.sql.Timestamp;
  * </ul>
  */
 public final class Account {
-  public static final String USER_NAME_PATTERN_FIRST = "[a-zA-Z0-9]";
-  public static final String USER_NAME_PATTERN_REST = "[a-zA-Z0-9._@-]";
-  public static final String USER_NAME_PATTERN_LAST = "[a-zA-Z0-9]";
-
-  /** Regular expression that {@link #userName} must match. */
-  public static final String USER_NAME_PATTERN =
-      "^"
-          + //
-          "("
-          + //
-          USER_NAME_PATTERN_FIRST
-          + //
-          USER_NAME_PATTERN_REST
-          + "*"
-          + //
-          USER_NAME_PATTERN_LAST
-          + //
-          "|"
-          + //
-          USER_NAME_PATTERN_FIRST
-          + //
-          ")"
-          + //
-          "$";
-
   /** Key local to Gerrit to identify a user. */
   public static class Id extends IntKey<com.google.gwtorm.client.Key<?>> {
     private static final long serialVersionUID = 1L;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java
index 9803143247..7e1157ccfa 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java
@@ -105,7 +105,7 @@ public class AccountResolver {
       return Collections.emptySet();
     }
 
-    if (nameOrEmail.matches(Account.USER_NAME_PATTERN)) {
+    if (ExternalId.isValidUsername(nameOrEmail)) {
       AccountState who = byId.getByUsername(nameOrEmail);
       if (who != null) {
         return Collections.singleton(who.getAccount().getId());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/ChangeUserName.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/ChangeUserName.java
index c9a7aabf21..b58edfb4c6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/ChangeUserName.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/ChangeUserName.java
@@ -19,7 +19,6 @@ import static java.util.stream.Collectors.toSet;
 
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.errors.NameAlreadyUsedException;
-import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.ssh.SshKeyCache;
@@ -31,7 +30,6 @@ import com.google.inject.assistedinject.Assisted;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.concurrent.Callable;
-import java.util.regex.Pattern;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -39,7 +37,6 @@ import org.slf4j.LoggerFactory;
 /** Operation to change the username of an account. */
 public class ChangeUserName implements Callable<VoidResult> {
   private static final Logger log = LoggerFactory.getLogger(ChangeUserName.class);
-  private static final Pattern USER_NAME_PATTERN = Pattern.compile(Account.USER_NAME_PATTERN);
 
   public static final String USERNAME_CANNOT_BE_CHANGED = "Username cannot be changed.";
 
@@ -90,7 +87,7 @@ public class ChangeUserName implements Callable<VoidResult> {
 
     ExternalIdsUpdate externalIdsUpdate = externalIdsUpdateFactory.create();
     if (newUsername != null && !newUsername.isEmpty()) {
-      if (!USER_NAME_PATTERN.matcher(newUsername).matches()) {
+      if (!ExternalId.isValidUsername(newUsername)) {
         throw new InvalidUserNameException();
       }
 
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
index 451246ba71..f384a3502d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
@@ -111,7 +111,7 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
       throw new BadRequestException("username must match URL");
     }
 
-    if (!username.matches(Account.USER_NAME_PATTERN)) {
+    if (!ExternalId.isValidUsername(username)) {
       throw new BadRequestException(
           "Username '" + username + "' must contain only letters, numbers, _, - or .");
     }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/ExternalId.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/ExternalId.java
index f830f2c44f..ad1d2fbbf2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/ExternalId.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/ExternalId.java
@@ -31,12 +31,43 @@ import com.google.gerrit.reviewdb.client.AccountExternalId;
 import java.io.Serializable;
 import java.util.Collection;
 import java.util.Set;
+import java.util.regex.Pattern;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ObjectId;
 
 @AutoValue
 public abstract class ExternalId implements Serializable {
+  // If these regular expressions are modified the same modifications should be done to the
+  // corresponding regular expressions in the
+  // com.google.gerrit.client.account.UsernameField class.
+  private static final String USER_NAME_PATTERN_FIRST_REGEX = "[a-zA-Z0-9]";
+  private static final String USER_NAME_PATTERN_REST_REGEX = "[a-zA-Z0-9._@-]";
+  private static final String USER_NAME_PATTERN_LAST_REGEX = "[a-zA-Z0-9]";
+
+  /** Regular expression that a username must match. */
+  private static final String USER_NAME_PATTERN_REGEX =
+      "^("
+          + //
+          USER_NAME_PATTERN_FIRST_REGEX
+          + //
+          USER_NAME_PATTERN_REST_REGEX
+          + "*"
+          + //
+          USER_NAME_PATTERN_LAST_REGEX
+          + //
+          "|"
+          + //
+          USER_NAME_PATTERN_FIRST_REGEX
+          + //
+          ")$";
+
+  private static final Pattern USER_NAME_PATTERN = Pattern.compile(USER_NAME_PATTERN_REGEX);
+
+  public static boolean isValidUsername(String username) {
+    return USER_NAME_PATTERN.matcher(username).matches();
+  }
+
   private static final long serialVersionUID = 1L;
 
   private static final String EXTERNAL_ID_SECTION = "externalId";
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/InvalidUserNameException.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/InvalidUserNameException.java
index d60b7af85f..e8d0df7b2b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/InvalidUserNameException.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/InvalidUserNameException.java
@@ -14,9 +14,7 @@
 
 package com.google.gerrit.server.account;
 
-import com.google.gerrit.reviewdb.client.Account;
-
-/** Error indicating the SSH user name does not match {@link Account#USER_NAME_PATTERN} pattern. */
+/** Error indicating the SSH user name does not match the expected pattern. */
 public class InvalidUserNameException extends Exception {
 
   private static final long serialVersionUID = 1L;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/args4j/AccountIdHandler.java b/gerrit-server/src/main/java/com/google/gerrit/server/args4j/AccountIdHandler.java
index 75628015bb..9ee6901c03 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/args4j/AccountIdHandler.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/args4j/AccountIdHandler.java
@@ -21,6 +21,7 @@ import com.google.gerrit.server.account.AccountException;
 import com.google.gerrit.server.account.AccountManager;
 import com.google.gerrit.server.account.AccountResolver;
 import com.google.gerrit.server.account.AuthRequest;
+import com.google.gerrit.server.account.ExternalId;
 import com.google.gerrit.server.config.AuthConfig;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -90,7 +91,7 @@ public class AccountIdHandler extends OptionHandler<Account.Id> {
   }
 
   private Account.Id createAccountByLdap(String user) throws CmdLineException, IOException {
-    if (!user.matches(Account.USER_NAME_PATTERN)) {
+    if (!ExternalId.isValidUsername(user)) {
       throw new CmdLineException(owner, "user \"" + user + "\" not found");
     }
 
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java
index 5c1a292cde..150ac01a69 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java
@@ -37,6 +37,7 @@ import com.google.gerrit.server.account.AccountManager;
 import com.google.gerrit.server.account.AccountResolver;
 import com.google.gerrit.server.account.AccountsCollection;
 import com.google.gerrit.server.account.AuthRequest;
+import com.google.gerrit.server.account.ExternalId;
 import com.google.gerrit.server.account.GroupControl;
 import com.google.gerrit.server.config.AuthConfig;
 import com.google.gerrit.server.group.AddMembers.Input;
@@ -198,7 +199,7 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
   }
 
   private Account createAccountByLdap(String user) throws IOException {
-    if (!user.matches(Account.USER_NAME_PATTERN)) {
+    if (!ExternalId.isValidUsername(user)) {
       return null;
     }
 
diff --git a/plugins/singleusergroup b/plugins/singleusergroup
-Subproject 570b6e287a74750d37d2a94e2cf66297c004dce
+Subproject 1568d7755c70cdb26ddc865a7181c90f2448067