User/role names can be longer with more characters (elastic/x-pack-elasticsearch#1745)

This changes the validation criteria we use for user and role
names in the file realm, native realm, and the
realm-agnostic code in x-pack security. The new criteria is:

A valid username's length must be at least 1 and no more than 1024
characters. It may not contain leading or trailing whitespace. All
characters in the name must be be alphanumeric (`a-z`, `A-Z`, `0-9`),
printable punctuation or symbols in the https://en.wikipedia.org/wiki/Basic_Latin_(Unicode_block)[Basic Latin (ASCII) block],
or the space character.

Original commit: elastic/x-pack-elasticsearch@f77640f269

docs/en/rest-api/security/users.asciidoc

@@ -10,10 +10,10 @@ To add a user, submit a PUT or POST request to the `/_xpack/security/user/<usern
 endpoint.
 
 [[username-validation]]
-NOTE: A username must be at least 1 character and no longer than 30 characters.
-      The first character must be a letter (`a-z` or `A-Z`) or an underscore (`_`).
-      Subsequent characters can be letters, underscores (`_`), digits (`0-9`),
-      or any of the following symbols `@`, `-`, `.` or `$`
+NOTE: Usernames must be at least 1 and no more than 1024 characters. They can 
+      contain alphanumeric characters (`a-z`, `A-Z`, `0-9`), spaces, 
+      punctuation, and printable symbols in the https://en.wikipedia.org/wiki/Basic_Latin_(Unicode_block)[Basic Latin (ASCII) block]. 
+      Leading or trailing whitespace is not allowed.
 
 [source,js]
 --------------------------------------------------

docs/en/security/authentication/file-realm.asciidoc

@@ -112,10 +112,10 @@ NOTE: To ensure that Elasticsearch can read the user and role information at
 bin/x-pack/users useradd <username>
 ----------------------------------------
 
-A username must be at least 1 character and no longer than 30 characters. The
-first character must be a letter (`a-z` or `A-Z`) or an underscore (`_`).
-Subsequent characters can be letters, underscores (`_`), digits (`0-9`), or any
-of the following symbols `@`, `-`, `.` or `$`.
+Usernames must be at least 1 and no more than 1024 characters. They can 
+contain alphanumeric characters (`a-z`, `A-Z`, `0-9`), spaces, punctuation, and 
+printable symbols in the https://en.wikipedia.org/wiki/Basic_Latin_(Unicode_block)[Basic Latin (ASCII) block]. 
+Leading or trailing whitespace is not allowed.
 
 You can specify the user's password at the command-line with the `-p` option.
 When this option is absent, the command prompts you for the password. Omit the

docs/en/security/authentication/native-realm.asciidoc

@@ -98,10 +98,12 @@ NOTE: To migrate file-based users to the `native` realm, use the
 ===== Adding Users
 
 To add a user, submit a PUT or POST request to the `/_xpack/security/user/<username>`
-endpoint. A username must be at least 1 character long and no longer than 30
-characters. The first character must be a letter (`a-z` or `A-Z`) or an
-underscore (`_`). Subsequent characters can be letters, underscores (`_`),
-digits (`0-9`), or any of the following symbols `@`, `-`, `.` or `$`.
+endpoint.
+
+Usernames must be at least 1 and no more than 1024 characters. They can 
+contain alphanumeric characters (`a-z`, `A-Z`, `0-9`), spaces, punctuation, and 
+printable symbols in the https://en.wikipedia.org/wiki/Basic_Latin_(Unicode_block)[Basic Latin (ASCII) block]. 
+Leading or trailing whitespace is not allowed.
 
 [source,js]
 --------------------------------------------------

docs/en/security/authorization.asciidoc

@@ -164,10 +164,10 @@ A role is defined by the following JSON structure:
     privileges effectively mean no index level permissions).
 
 [[valid-role-name]]
-NOTE: A valid role name must be at least 1 character and no longer than 30
-      characters. It must begin with a letter (`a-z`) or an underscore (`_`).
-      Subsequent characters can be letters, underscores (`_`), digits (`0-9`) or
-      any of the following symbols `@`, `-`, `.` or `$`
+NOTE: Role names must be at least 1 and no more than 1024 characters. They can 
+      contain alphanumeric characters (`a-z`, `A-Z`, `0-9`), spaces, 
+      punctuation, and printable symbols in the https://en.wikipedia.org/wiki/Basic_Latin_(Unicode_block)[Basic Latin (ASCII) block]. 
+      Leading or trailing whitespace is not allowed.
 
 The following describes the structure of an indices permissions entry:
 

plugin/src/main/java/org/elasticsearch/xpack/security/support/Validation.java

@@ -6,14 +6,60 @@
 package org.elasticsearch.xpack.security.support;
 
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm;
 import org.elasticsearch.xpack.security.authz.store.ReservedRolesStore;
 
-import java.util.regex.Pattern;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Set;
+
+import static java.util.Collections.unmodifiableSet;
 
 public final class Validation {
 
-    private static final Pattern COMMON_NAME_PATTERN = Pattern.compile("[a-zA-Z_][a-zA-Z0-9_@\\-\\$\\.]{0,29}");
+    static final int MIN_NAME_LENGTH = 1;
+    static final int MAX_NAME_LENGTH = 1024;
+
+    static final Set<Character> VALID_NAME_CHARS = unmodifiableSet(Sets.newHashSet(
+        ' ', '!', '"', '#', '$', '%', '&', '\'', '(', ')', '*', '+', ',', '-', '.', '/',
+        '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', ':', ';', '<', '=', '>', '?',
+        '@', 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O',
+        'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', '[', '\\', ']', '^', '_',
+        '`', 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o',
+        'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '{', '|', '}', '~'
+    ));
+
+    private static final String INVALID_NAME_MESSAGE =
+        "%1s names must be at least " + MIN_NAME_LENGTH + " and no more than " + MAX_NAME_LENGTH + " characters. " +
+        "They can contain alphanumeric characters (a-z, A-Z, 0-9), spaces, punctuation, and printable symbols in the " +
+        "Basic Latin (ASCII) block. Leading or trailing whitespace is not allowed.";
+
+    private static boolean isValidUserOrRoleName(String name) {
+        if (name.length() < MIN_NAME_LENGTH || name.length() > MAX_NAME_LENGTH) {
+            return false;
+        }
+
+        for (char character : name.toCharArray()) {
+            if (!VALID_NAME_CHARS.contains(character)) {
+                return false;
+            }
+        }
+
+        // We only check against the space character here (U+0020) since it's the only whitespace character in the
+        // set that we allow.
+        //
+        // Note for the future if we allow the full unicode range: the String and Character methods that deal with
+        // whitespace don't work for the whole range. They match characters that are considered whitespace to the Java
+        // language, which doesn't include characters like IDEOGRAPHIC SPACE (U+3000). The best approach here may be
+        // to match against java.util.regex.Pattern's \p{Space} class (which is by default broader than \s) or make a
+        // list from the codepoints listed in this page https://en.wikipedia.org/wiki/Whitespace_character
+        if (name.startsWith(" ") || name.endsWith(" ")) {
+            return false;
+        }
+
+        return true;
+    }
 
     public static final class Users {
 
@@ -27,11 +73,8 @@ public final class Validation {
          * @return {@code null} if valid
          */
         public static Error validateUsername(String username, boolean allowReserved, Settings settings) {
-            if (COMMON_NAME_PATTERN.matcher(username).matches() == false) {
-                return new Error("A valid username must be at least 1 character and no longer than 30 characters. " +
-                        "It must begin with a letter (`a-z` or `A-Z`) or an underscore (`_`). Subsequent " +
-                        "characters can be letters, underscores (`_`), digits (`0-9`) or any of the following " +
-                        "symbols `@`, `-`, `.` or `$`");
+            if (!isValidUserOrRoleName(username)) {
+                return new Error(String.format(Locale.ROOT, INVALID_NAME_MESSAGE, "User"));
             }
             if (allowReserved == false && ReservedRealm.isReserved(username, settings)) {
                 return new Error("Username [" + username + "] is reserved and may not be used.");
@@ -54,11 +97,8 @@ public final class Validation {
         }
 
         public static Error validateRoleName(String roleName, boolean allowReserved) {
-            if (COMMON_NAME_PATTERN.matcher(roleName).matches() == false) {
-                return new Error("A valid role name must be at least 1 character and no longer than 30 characters. " +
-                        "It must begin with a letter (`a-z` or `A-Z`) or an underscore (`_`). Subsequent " +
-                        "characters can be letters, underscores (`_`), digits (`0-9`) or any of the following " +
-                        "symbols `@`, `-`, `.` or `$`");
+            if (!isValidUserOrRoleName(roleName)) {
+                return new Error(String.format(Locale.ROOT, INVALID_NAME_MESSAGE, "Role"));
             }
             if (allowReserved == false && ReservedRolesStore.isReserved(roleName)) {
                 return new Error("Role [" + roleName + "] is reserved and may not be used.");

plugin/src/test/java/org/elasticsearch/xpack/security/action/user/PutUserRequestTests.java

@@ -38,13 +38,13 @@ public class PutUserRequestTests extends ESTestCase {
         assertThat(validation.validationErrors().size(), is(1));
     }
 
-    public void testValidateRejectsUserNameThatStartsWithAnInvalidCharacter() throws Exception {
+    public void testValidateRejectsUserNameThatHasInvalidCharacters() throws Exception {
         final PutUserRequest request = new PutUserRequest();
-        request.username("$foo");
+        request.username("fóóbár");
         request.roles("bar");
         final ActionRequestValidationException validation = request.validate();
         assertThat(validation, is(notNullValue()));
-        assertThat(validation.validationErrors(), contains(containsString("must begin with")));
+        assertThat(validation.validationErrors(), contains(containsString("must be")));
         assertThat(validation.validationErrors().size(), is(1));
     }
 

plugin/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmIntegTests.java

@@ -479,13 +479,12 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase {
         }
     }
 
-    public void testCannotCreateUserWithLeadingDigitInUserName() throws Exception {
+    public void testCannotCreateUserWithInvalidCharactersInName() throws Exception {
         SecurityClient client = securityClient();
         ValidationException v = expectThrows(ValidationException.class,
-                () -> client.preparePutUser("99bottles", "on-th3-w@ll".toCharArray(), "admin_role").get()
+                () -> client.preparePutUser("fóóbár", "my-am@zing-password".toCharArray(), "admin_role").get()
         );
-        assertThat(v.getMessage(), containsString("valid username"));
-        assertThat(v.getMessage(), containsString("must begin with"));
+        assertThat(v.getMessage(), containsString("names must be"));
     }
 
     public void testUsersAndRolesDoNotInterfereWithIndicesStats() throws Exception {

plugin/src/test/java/org/elasticsearch/xpack/security/authc/file/tool/UsersToolTests.java

@@ -177,7 +177,7 @@ public class UsersToolTests extends CommandTestCase {
 
     public void testParseInvalidUsername() throws Exception {
         UserException e = expectThrows(UserException.class, () -> {
-            UsersTool.parseUsername(Collections.singletonList("$34dkl"), Settings.EMPTY);
+            UsersTool.parseUsername(Collections.singletonList("áccented"), Settings.EMPTY);
         });
         assertEquals(ExitCodes.DATA_ERROR, e.exitCode);
         assertTrue(e.getMessage(), e.getMessage().contains("Invalid username"));
@@ -254,10 +254,10 @@ public class UsersToolTests extends CommandTestCase {
 
     public void testParseInvalidRole() throws Exception {
         UserException e = expectThrows(UserException.class, () -> {
-            UsersTool.parseRoles(terminal, new Environment(settings), "$345");
+            UsersTool.parseRoles(terminal, new Environment(settings), "fóóbár");
         });
         assertEquals(ExitCodes.DATA_ERROR, e.exitCode);
-        assertTrue(e.getMessage(), e.getMessage().contains("Invalid role [$345]"));
+        assertTrue(e.getMessage(), e.getMessage().contains("Invalid role [fóóbár]"));
     }
 
     public void testParseMultipleRoles() throws Exception {

plugin/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java

@@ -380,7 +380,7 @@ public class FileRolesStoreTests extends ESTestCase {
         assertThat(entries, hasSize(6));
         assertThat(
                 entries.get(0),
-                startsWith("invalid role definition [$dlk39] in roles file [" + path.toAbsolutePath() + "]. invalid role name"));
+                startsWith("invalid role definition [fóóbár] in roles file [" + path.toAbsolutePath() + "]. invalid role name"));
         assertThat(
                 entries.get(1),
                 startsWith("invalid role definition [role1] in roles file [" + path.toAbsolutePath() + "]"));
@@ -401,7 +401,7 @@ public class FileRolesStoreTests extends ESTestCase {
         assertThat(events, hasSize(1));
         assertThat(
                 events.get(0),
-                startsWith("invalid role definition [$dlk39] in roles file [" + path.toAbsolutePath() + "]. invalid role name"));
+                startsWith("invalid role definition [fóóbár] in roles file [" + path.toAbsolutePath() + "]. invalid role name"));
     }
 
     public void testReservedRoles() throws Exception {

plugin/src/test/java/org/elasticsearch/xpack/security/support/ValidationTests.java

@@ -9,62 +9,36 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.xpack.security.authz.store.ReservedRolesStore;
 import org.elasticsearch.xpack.security.support.Validation.Error;
 import org.elasticsearch.xpack.security.support.Validation.Users;
+import org.elasticsearch.xpack.security.support.Validation.Roles;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.security.user.ElasticUser;
 import org.elasticsearch.xpack.security.user.KibanaUser;
 
-import java.util.Arrays;
-
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 
 public class ValidationTests extends ESTestCase {
-    private static final char[] alphabet = {
-        'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z',
-        'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z'
-    };
 
-    private static final char[] numbers = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9' };
+    private static final Character[] ALLOWED_CHARS = Validation.VALID_NAME_CHARS.toArray(
+        new Character[Validation.VALID_NAME_CHARS.size()]
+    );
 
-    private static final char[] allowedFirstChars = concat(alphabet, new char[]{'_'});
-
-    private static final char[] allowedSubsequent = concat(alphabet, numbers, new char[]{'_', '@', '-', '$', '.'});
-
-    static {
-        Arrays.sort(allowedFirstChars);
-        Arrays.sort(allowedSubsequent);
-    }
-
-    static char[] concat(char[]... arrays) {
-        int length = 0;
-        for (char[] array : arrays) {
-            length += array.length;
-        }
-        char[] newArray = new char[length];
-        int i = 0;
-        for (char[] array : arrays) {
-            System.arraycopy(array, 0, newArray, i, array.length);
-            i += array.length;
-        }
-        return newArray;
-    }
-
-    public void testUsersValidateUsername() throws Exception {
-        int length = randomIntBetween(1, 30);
+    public void testUsernameValid() throws Exception {
+        int length = randomIntBetween(Validation.MIN_NAME_LENGTH, Validation.MAX_NAME_LENGTH);
         String name = new String(generateValidName(length));
         assertThat(Users.validateUsername(name, false, Settings.EMPTY), nullValue());
     }
 
-    public void testReservedUsernames() {
+    public void testUsernameReserved() {
         final String username = randomFrom(ElasticUser.NAME, KibanaUser.NAME);
         final Error error = Users.validateUsername(username, false, Settings.EMPTY);
         assertNotNull(error);
         assertThat(error.toString(), containsString("is reserved"));
     }
 
-    public void testUsersValidateUsernameInvalidLength() throws Exception {
-        int length = frequently() ? randomIntBetween(31, 200) : 0; // invalid length
+    public void testUsernameInvalidLength() throws Exception {
+        int length = frequently() ? randomIntBetween(Validation.MAX_NAME_LENGTH + 1, 2048) : 0;
         char[] name = new char[length];
         if (length > 0) {
             name = generateValidName(length);
@@ -72,9 +46,15 @@ public class ValidationTests extends ESTestCase {
         assertThat(Users.validateUsername(new String(name), false, Settings.EMPTY), notNullValue());
     }
 
-    public void testUsersValidateUsernameInvalidCharacters() throws Exception {
-        int length = randomIntBetween(1, 30); // valid length
-        String name = new String(generateInvalidName(length));
+    public void testUsernameInvalidCharacters() throws Exception {
+        int length = randomIntBetween(Validation.MIN_NAME_LENGTH, Validation.MAX_NAME_LENGTH);
+        String name = new String(generateNameInvalidCharacters(length));
+        assertThat(Users.validateUsername(name, false, Settings.EMPTY), notNullValue());
+    }
+
+    public void testUsernameInvalidWhitespace() throws Exception {
+        int length = randomIntBetween(Validation.MIN_NAME_LENGTH, Validation.MAX_NAME_LENGTH);
+        String name = new String(generateNameInvalidWhitespace(length));
         assertThat(Users.validateUsername(name, false, Settings.EMPTY), notNullValue());
     }
 
@@ -88,82 +68,92 @@ public class ValidationTests extends ESTestCase {
         }
     }
 
-    public void testRolesValidateRoleName() throws Exception {
-        int length = randomIntBetween(1, 30);
+    public void testRoleNameValid() throws Exception {
+        int length = randomIntBetween(Validation.MIN_NAME_LENGTH, Validation.MAX_NAME_LENGTH);
         String name = new String(generateValidName(length));
-        assertThat(Validation.Roles.validateRoleName(name), nullValue());
+        assertThat(Roles.validateRoleName(name), nullValue());
     }
 
-    public void testReservedRoleName() {
+    public void testRoleNameReserved() {
         final String rolename = randomFrom(ReservedRolesStore.names());
-        final Error error = Validation.Roles.validateRoleName(rolename);
+        final Error error = Roles.validateRoleName(rolename);
         assertNotNull(error);
         assertThat(error.toString(), containsString("is reserved"));
 
-        final Error allowed = Validation.Roles.validateRoleName(rolename, true);
+        final Error allowed = Roles.validateRoleName(rolename, true);
         assertNull(allowed);
     }
 
-    public void testRolesValidateRoleNameInvalidLength() throws Exception {
-        int length = frequently() ? randomIntBetween(31, 200) : 0; // invalid length
+    public void testRoleNameInvalidLength() throws Exception {
+        int length = frequently() ? randomIntBetween(Validation.MAX_NAME_LENGTH + 1, 2048) : 0;
         char[] name = new char[length];
         if (length > 0) {
             name = generateValidName(length);
         }
-        assertThat(Users.validateUsername(new String(name), false, Settings.EMPTY), notNullValue());
+        assertThat(Roles.validateRoleName(new String(name), false), notNullValue());
     }
 
-    public void testRolesValidateRoleNameInvalidCharacters() throws Exception {
-        int length = randomIntBetween(1, 30); // valid length
-        String name = new String(generateInvalidName(length));
-        assertThat(Users.validateUsername(name, false, Settings.EMPTY), notNullValue());
+    public void testRoleNameInvalidCharacters() throws Exception {
+        int length = randomIntBetween(Validation.MIN_NAME_LENGTH, Validation.MAX_NAME_LENGTH);
+        String name = new String(generateNameInvalidCharacters(length));
+        assertThat(Roles.validateRoleName(name, false), notNullValue());
+    }
+
+    public void testRoleNameInvalidWhitespace() throws Exception {
+        int length = randomIntBetween(Validation.MIN_NAME_LENGTH, Validation.MAX_NAME_LENGTH);
+        String name = new String(generateNameInvalidWhitespace(length));
+        assertThat(Roles.validateRoleName(name, false), notNullValue());
     }
 
     private static char[] generateValidName(int length) {
-        char first = allowedFirstChars[randomIntBetween(0, allowedFirstChars.length - 1)];
-        char[] subsequent = new char[length - 1];
-        for (int i = 0; i < subsequent.length; i++) {
-            subsequent[i] = allowedSubsequent[randomIntBetween(0, allowedSubsequent.length - 1)];
+        char[] name = new char[length];
+        name[0] = chooseValidNonWhitespaceCharacter();
+        if (length > 1) {
+            for (int i = 1; i < length - 1; i++) {
+                name[i] = chooseValidCharacter();
+            }
         }
-        return concat(new char[]{first}, subsequent);
+        name[length - 1] = chooseValidNonWhitespaceCharacter();
+        return name;
     }
 
-    private static char[] generateInvalidName(int length) {
-        if (length == 1 || randomBoolean()) {
-            // invalid name due to characters not allowed in the beginning of the name
-            char first;
-            while (true) {
-                first = randomUnicodeOfLength(1).charAt(0);
-                final char finalChar = first;
-                if (new String(allowedFirstChars).chars()
-                        .mapToObj(c -> (char) c)
-                        .anyMatch(c -> c.equals(finalChar)) == false) {
-                    break;
-                }
-            }
-            char[] subsequent = new char[length - 1];
-            for (int i = 0; i < subsequent.length; i++) {
-                subsequent[i] = allowedSubsequent[randomIntBetween(0, allowedSubsequent.length - 1)];
-            }
-            return concat(new char[]{first}, subsequent);
-        }
+    private static char chooseValidCharacter() {
+        return randomFrom(ALLOWED_CHARS);
+    }
 
-        // invalid name due to charaters not allowed within the name itself
-        char first = allowedFirstChars[randomIntBetween(0, allowedFirstChars.length - 1)];
-        char[] subsequent = new char[length - 1];
-        for (int i = 0; i < subsequent.length; i++) {
+    private static char chooseValidNonWhitespaceCharacter() {
+        char c = chooseValidCharacter();
+        while (c == ' ') {
+            c = chooseValidCharacter();
+        }
+        return c;
+    }
+
+    private static char[] generateNameInvalidCharacters(int length) {
+        char[] name = new char[length];
+        for (int i = 0; i < length; i++) {
             char c;
             while (true) {
                 c = randomUnicodeOfLength(1).charAt(0);
                 final char finalChar = c;
-                if (new String(allowedSubsequent).chars()
-                        .mapToObj(c1 -> (char) c1)
-                        .anyMatch(c1 -> c1.equals(finalChar)) == false) {
+                if (!Validation.VALID_NAME_CHARS.contains(finalChar)) {
                     break;
                 }
             }
-            subsequent[i] = c;
+            name[i] = c;
         }
-        return concat(new char[]{first}, subsequent);
+
+        return name;
     }
+
+    private static char[] generateNameInvalidWhitespace(int length) {
+        char[] name = generateValidName(length);
+        if (randomBoolean()) {
+            name[0] = ' ';
+        } else {
+            name[name.length - 1] = ' ';
+        }
+        return name;
+    }
+
 }

plugin/src/test/resources/org/elasticsearch/xpack/security/authz/store/invalid_roles.yml

@@ -6,7 +6,7 @@ valid_role:
       privileges:
         - ALL
 
-"$dlk39":
+"fóóbár":
   cluster: all
 
 # invalid role deifnition