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

The name is now required when creating or granting API keys.
elastic · Aug 4, 2020 · 6ae04a5 · 6ae04a5
1 parent c9f2124
commit 6ae04a5
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 13 deletions.
diff --git a/.../rest-high-level/src/main/java/org/elasticsearch/client/security/CreateApiKeyRequest.java b/.../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");

diff --git a/.../core/src/main/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequest.java b/.../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) ? List.of() : List.copyOf(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;

diff --git a/.../src/test/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequestTests.java b/.../src/test/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequestTests.java
@@ -28,7 +28,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();
@@ -38,25 +39,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();
@@ -66,7 +67,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 {

diff --git a/...qa/security-trial/src/test/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java b/...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.Tuple;
 import org.elasticsearch.common.settings.SecureString;
@@ -26,6 +27,7 @@
 import java.util.Map;
 import java.util.Set;
 
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.hamcrest.Matchers.instanceOf;
@@ -115,4 +117,22 @@ 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 = Map.ofEntries(
+            Map.entry("grant_type", "password"),
+            Map.entry("username", END_USER),
+            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"));
+    }
 }
diff --git a/...lugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java b/...lugin/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.node.info.NodeInfo;
 import org.elasticsearch.action.admin.indices.refresh.RefreshAction;
@@ -221,6 +222,15 @@ public void testMultipleApiKeysCanHaveSameName() {
         }
     }
 
+    public void testCreateApiKeyWithoutNameWillFail() {
+        Client client = client().filterWithHeader(Collections.singletonMap("Authorization",
+            UsernamePasswordToken.basicAuthHeaderValue(SecuritySettingsSource.TEST_SUPERUSER,
+                SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING)));
+        final ActionRequestValidationException e =
+            expectThrows(ActionRequestValidationException.class, () -> new CreateApiKeyRequestBuilder(client).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);