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

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) ? 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;

x-pack/plugin/core/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 {

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.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"));
+ }
 }

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.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);