API key name should always be required for creation (#59836) (#60636)

The name is now required when creating or granting API keys.

Author: ywangd

client/rest-high-level/src/main/java/org/elasticsearch/client/security/CreateApiKeyRequest.java

@@ -46,7 +46,7 @@ public final class CreateApiKeyRequest implements Validatable, ToXContentObject
      * @param roles list of {@link Role}s
      * @param expiration to specify expiration for the API key
      */
-    public CreateApiKeyRequest(@Nullable String name, List<Role> roles, @Nullable TimeValue expiration,
+    public CreateApiKeyRequest(String name, List<Role> roles, @Nullable TimeValue expiration,
                                @Nullable final RefreshPolicy refreshPolicy) {
         this.name = name;
         this.roles = Objects.requireNonNull(roles, "roles may not be null");

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequest.java

@@ -11,6 +11,7 @@
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.support.WriteRequest;
 import org.elasticsearch.common.Nullable;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.unit.TimeValue;
@@ -43,7 +44,7 @@ public CreateApiKeyRequest() {}
      * @param roleDescriptors list of {@link RoleDescriptor}s
      * @param expiration to specify expiration for the API key
      */
-    public CreateApiKeyRequest(@Nullable String name, @Nullable List<RoleDescriptor> roleDescriptors, @Nullable TimeValue expiration) {
+    public CreateApiKeyRequest(String name, @Nullable List<RoleDescriptor> roleDescriptors, @Nullable TimeValue expiration) {
         this.name = name;
         this.roleDescriptors = (roleDescriptors == null) ? Collections.emptyList() : Collections.unmodifiableList(roleDescriptors);
         this.expiration = expiration;
@@ -65,7 +66,7 @@ public String getName() {
         return name;
     }
 
-    public void setName(@Nullable String name) {
+    public void setName(String name) {
         this.name = name;
     }
 
@@ -96,15 +97,17 @@ public void setRefreshPolicy(WriteRequest.RefreshPolicy refreshPolicy) {
     @Override
     public ActionRequestValidationException validate() {
         ActionRequestValidationException validationException = null;
-        if (name != null) {
+        if (Strings.isNullOrEmpty(name)) {
+            validationException = addValidationError("api key name is required", validationException);
+        } else {
             if (name.length() > 256) {
-                validationException = addValidationError("name may not be more than 256 characters long", validationException);
+                validationException = addValidationError("api key name may not be more than 256 characters long", validationException);
             }
             if (name.equals(name.trim()) == false) {
-                validationException = addValidationError("name may not begin or end with whitespace", validationException);
+                validationException = addValidationError("api key name may not begin or end with whitespace", validationException);
             }
             if (name.startsWith("_")) {
-                validationException = addValidationError("name may not begin with an underscore", validationException);
+                validationException = addValidationError("api key name may not begin with an underscore", validationException);
             }
         }
         return validationException;

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequestTests.java

@@ -29,7 +29,8 @@ public void testNameValidation() {
         CreateApiKeyRequest request = new CreateApiKeyRequest();
 
         ActionRequestValidationException ve = request.validate();
-        assertNull(ve);
+        assertThat(ve.validationErrors().size(), is(1));
+        assertThat(ve.validationErrors().get(0), containsString("api key name is required"));
 
         request.setName(name);
         ve = request.validate();
@@ -39,25 +40,25 @@ public void testNameValidation() {
         ve = request.validate();
         assertNotNull(ve);
         assertThat(ve.validationErrors().size(), is(1));
-        assertThat(ve.validationErrors().get(0), containsString("name may not be more than 256 characters long"));
+        assertThat(ve.validationErrors().get(0), containsString("api key name may not be more than 256 characters long"));
 
         request.setName(" leading space");
         ve = request.validate();
         assertNotNull(ve);
         assertThat(ve.validationErrors().size(), is(1));
-        assertThat(ve.validationErrors().get(0), containsString("name may not begin or end with whitespace"));
+        assertThat(ve.validationErrors().get(0), containsString("api key name may not begin or end with whitespace"));
 
         request.setName("trailing space ");
         ve = request.validate();
         assertNotNull(ve);
         assertThat(ve.validationErrors().size(), is(1));
-        assertThat(ve.validationErrors().get(0), containsString("name may not begin or end with whitespace"));
+        assertThat(ve.validationErrors().get(0), containsString("api key name may not begin or end with whitespace"));
 
         request.setName(" leading and trailing space ");
         ve = request.validate();
         assertNotNull(ve);
         assertThat(ve.validationErrors().size(), is(1));
-        assertThat(ve.validationErrors().get(0), containsString("name may not begin or end with whitespace"));
+        assertThat(ve.validationErrors().get(0), containsString("api key name may not begin or end with whitespace"));
 
         request.setName("inner space");
         ve = request.validate();
@@ -67,7 +68,7 @@ public void testNameValidation() {
         ve = request.validate();
         assertNotNull(ve);
         assertThat(ve.validationErrors().size(), is(1));
-        assertThat(ve.validationErrors().get(0), containsString("name may not begin with an underscore"));
+        assertThat(ve.validationErrors().get(0), containsString("api key name may not begin with an underscore"));
     }
 
     public void testSerialization() throws IOException {

x-pack/plugin/security/qa/security-trial/src/test/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java

@@ -9,6 +9,7 @@
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.RequestOptions;
 import org.elasticsearch.client.Response;
+import org.elasticsearch.client.ResponseException;
 import org.elasticsearch.client.security.support.ApiKey;
 import org.elasticsearch.common.collect.MapBuilder;
 import org.elasticsearch.common.collect.Tuple;
@@ -27,6 +28,7 @@
 import java.util.HashMap;
 import java.util.Map;
 
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.hamcrest.Matchers.instanceOf;
@@ -114,5 +116,23 @@ public void testGrantApiKeyForOtherUserWithAccessToken() throws IOException {
         assertThat(apiKey.getExpiration(), greaterThanOrEqualTo(minExpiry));
         assertThat(apiKey.getExpiration(), lessThanOrEqualTo(maxExpiry));
     }
+
+    public void testGrantApiKeyWithoutApiKeyNameWillFail() throws IOException {
+        Request request = new Request("POST", "_security/api_key/grant");
+        request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization",
+            UsernamePasswordToken.basicAuthHeaderValue(SYSTEM_USER, SYSTEM_USER_PASSWORD)));
+        final Map<String, Object> requestBody = org.elasticsearch.common.collect.Map.ofEntries(
+            org.elasticsearch.common.collect.Map.entry("grant_type", "password"),
+            org.elasticsearch.common.collect.Map.entry("username", END_USER),
+            org.elasticsearch.common.collect.Map.entry("password", END_USER_PASSWORD.toString())
+        );
+        request.setJsonEntity(XContentTestUtils.convertToXContent(requestBody, XContentType.JSON).utf8ToString());
+
+        final ResponseException e =
+            expectThrows(ResponseException.class, () -> client().performRequest(request));
+
+        assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
+        assertThat(e.getMessage(), containsString("api key name is required"));
+    }
 }
 

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java

@@ -7,6 +7,7 @@
 package org.elasticsearch.xpack.security.authc;
 
 import org.elasticsearch.ElasticsearchSecurityException;
+import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.DocWriteResponse;
 import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
 import org.elasticsearch.action.admin.cluster.node.info.NodeInfo;
@@ -223,6 +224,16 @@ public void testMultipleApiKeysCanHaveSameName() {
         }
     }
 
+    public void testCreateApiKeyWithoutNameWillFail() {
+        Client client = client().filterWithHeader(Collections.singletonMap("Authorization",
+            UsernamePasswordToken.basicAuthHeaderValue(SecuritySettingsSource.TEST_SUPERUSER,
+                SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING)));
+        SecurityClient securityClient = new SecurityClient(client);
+        final ActionRequestValidationException e =
+            expectThrows(ActionRequestValidationException.class, () -> securityClient.prepareCreateApiKey().get());
+        assertThat(e.getMessage(), containsString("api key name is required"));
+    }
+
     public void testInvalidateApiKeysForRealm() throws InterruptedException, ExecutionException {
         int noOfApiKeys = randomIntBetween(3, 5);
         List<CreateApiKeyResponse> responses = createApiKeys(noOfApiKeys, null);