Enforce username validation on PUT security/user (elastic/elasticsearch#4118)

Validation was being applied within UsersTool but not via the REST API
This also causes some validation messages in PutUser to change.

Closes elastic/elasticsearch#4018

Original commit: elastic/x-pack-elasticsearch@8e02146dee

elasticsearch/src/main/java/org/elasticsearch/xpack/security/action/user/PutUserRequest.java

@@ -13,8 +13,10 @@ import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.xpack.security.authc.support.CharArrays;
 import org.elasticsearch.xpack.security.support.MetadataUtils;
+import org.elasticsearch.xpack.security.support.Validation;
 
 import java.io.IOException;
 import java.util.Map;
@@ -43,6 +45,11 @@ public class PutUserRequest extends ActionRequest implements UserRequest, WriteR
         ActionRequestValidationException validationException = null;
         if (username == null) {
             validationException = addValidationError("user is missing", validationException);
+        } else {
+            Validation.Error error = Validation.Users.validateUsername(username, false, Settings.EMPTY);
+            if (error != null) {
+                validationException = addValidationError(error.toString(), validationException);
+            }
         }
         if (roles == null) {
             validationException = addValidationError("roles are missing", validationException);

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

@@ -0,0 +1,61 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.xpack.security.action.user;
+
+import org.elasticsearch.action.ActionRequestValidationException;
+import org.elasticsearch.test.ESTestCase;
+
+import java.util.Collections;
+import java.util.Date;
+
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
+
+public class PutUserRequestTests extends ESTestCase {
+
+    public void testValidateReturnsNullForCorrectData() throws Exception {
+        final PutUserRequest request = new PutUserRequest();
+        request.username("foo");
+        request.roles("bar");
+        request.metadata(Collections.singletonMap("created", new Date()));
+        final ActionRequestValidationException validation = request.validate();
+        assertThat(validation, is(nullValue()));
+    }
+
+    public void testValidateRejectsNullUserName() throws Exception {
+        final PutUserRequest request = new PutUserRequest();
+        request.username(null);
+        request.roles("bar");
+        final ActionRequestValidationException validation = request.validate();
+        assertThat(validation, is(notNullValue()));
+        assertThat(validation.validationErrors(), contains(is("user is missing")));
+        assertThat(validation.validationErrors().size(), is(1));
+    }
+
+    public void testValidateRejectsUserNameThatStartsWithAnInvalidCharacter() throws Exception {
+        final PutUserRequest request = new PutUserRequest();
+        request.username("$foo");
+        request.roles("bar");
+        final ActionRequestValidationException validation = request.validate();
+        assertThat(validation, is(notNullValue()));
+        assertThat(validation.validationErrors(), contains(containsString("must begin with")));
+        assertThat(validation.validationErrors().size(), is(1));
+    }
+
+    public void testValidateRejectsMetaDataWithLeadingUnderscore() throws Exception {
+        final PutUserRequest request = new PutUserRequest();
+        request.username("foo");
+        request.roles("bar");
+        request.metadata(Collections.singletonMap("_created", new Date()));
+        final ActionRequestValidationException validation = request.validate();
+        assertThat(validation, is(notNullValue()));
+        assertThat(validation.validationErrors(), contains(containsString("metadata keys")));
+        assertThat(validation.validationErrors().size(), is(1));
+    }
+}

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

@@ -479,6 +479,15 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase {
         }
     }
 
+    public void testCannotCreateUserWithLeadingDigitInUserName() throws Exception {
+        SecurityClient client = securityClient();
+        ValidationException v = expectThrows(ValidationException.class,
+                () -> client.preparePutUser("99bottles", "on-th3-w@ll".toCharArray(), "admin_role").get()
+        );
+        assertThat(v.getMessage(), containsString("valid username"));
+        assertThat(v.getMessage(), containsString("must begin with"));
+    }
+
     public void testUsersAndRolesDoNotInterfereWithIndicesStats() throws Exception {
         client().prepareIndex("foo", "bar").setSource("ignore", "me").get();
 
@@ -504,7 +513,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase {
         final String username = randomFrom(ElasticUser.NAME, KibanaUser.NAME);
         IllegalArgumentException exception = expectThrows(IllegalArgumentException.class,
                 () -> securityClient().preparePutUser(username, randomBoolean() ? "changeme".toCharArray() : null, "admin").get());
-        assertThat(exception.getMessage(), containsString("user [" + username + "] is reserved"));
+        assertThat(exception.getMessage(), containsString("Username [" + username + "] is reserved"));
 
         exception = expectThrows(IllegalArgumentException.class,
                 () -> securityClient().prepareDeleteUser(username).get());
@@ -520,7 +529,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase {
 
         exception = expectThrows(IllegalArgumentException.class,
                 () -> securityClient().preparePutUser(AnonymousUser.DEFAULT_ANONYMOUS_USERNAME, "foobar".toCharArray()).get());
-        assertThat(exception.getMessage(), containsString("user [" + AnonymousUser.DEFAULT_ANONYMOUS_USERNAME + "] is anonymous"));
+        assertThat(exception.getMessage(), containsString("Username [" + AnonymousUser.DEFAULT_ANONYMOUS_USERNAME + "] is reserved"));
 
         exception = expectThrows(IllegalArgumentException.class,
                 () -> securityClient().preparePutUser(SystemUser.NAME, "foobar".toCharArray()).get());