Make role descriptors optional when creating API keys (#43481) (#43614)

This commit changes the `role_descriptors` field from required
to optional when creating API key. The default behavior in .NET ES
client is to omit properties with `null` value requiring additional
workarounds. The behavior for the API does not change.
Field names (`id`, `name`) in the invalidate api keys API documentation have been
corrected where they were wrong.

Closes #42053

Author: bizybot

x-pack/docs/en/rest-api/security/create-api-keys.asciidoc

@@ -37,11 +37,11 @@ The following parameters can be specified in the body of a POST or PUT request:
 `name`::
 (string) Specifies the name for this API key.
 
-`role_descriptors` (required)::
+`role_descriptors` (optional)::
 (array-of-role-descriptor) An array of role descriptors for this API key. This
-parameter is required but can be an empty array, which applies the permissions
-of the authenticated user. If you supply role descriptors, they must be a subset
-of the authenticated user's permissions. The structure of role descriptor is the
+parameter is optional. When it is not specified or is an empty array, then the API key will have
+the permissions of the authenticated user. If you supply role descriptors, they must
+be a subset of the authenticated user's permissions. The structure of role descriptor is the
 same as the request for create role API. For more details, see
 <<security-api-roles,role management APIs>>.
 

x-pack/docs/en/rest-api/security/invalidate-api-keys.asciidoc

@@ -31,11 +31,11 @@ pertain to invalidating api keys:
 
 `realm_name` (optional)::
 (string) The name of an authentication realm. This parameter cannot be used with
-either `api_key_id` or `api_key_name`.
+either `id` or `name`.
 
 `username` (optional)::
 (string) The username of a user. This parameter cannot be used with either
-`api_key_id` or `api_key_name`.
+`id` or `name`.
 
 NOTE: While all parameters are optional, at least one of them is required.
 
@@ -47,8 +47,7 @@ If you create an API key as follows:
 ------------------------------------------------------------
 POST /_security/api_key
 {
-  "name": "my-api-key",
-  "role_descriptors": {}
+  "name": "my-api-key"
 }
 ------------------------------------------------------------
 // CONSOLE

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

@@ -43,13 +43,13 @@ public CreateApiKeyRequest() {}
      * @param roleDescriptors list of {@link RoleDescriptor}s
      * @param expiration to specify expiration for the API key
      */
-    public CreateApiKeyRequest(String name, List<RoleDescriptor> roleDescriptors, @Nullable TimeValue expiration) {
+    public CreateApiKeyRequest(String name, @Nullable List<RoleDescriptor> roleDescriptors, @Nullable TimeValue expiration) {
         if (Strings.hasText(name)) {
             this.name = name;
         } else {
             throw new IllegalArgumentException("name must not be null or empty");
         }
-        this.roleDescriptors = Objects.requireNonNull(roleDescriptors, "role descriptors may not be null");
+        this.roleDescriptors = (roleDescriptors == null) ? Collections.emptyList() : Collections.unmodifiableList(roleDescriptors);
         this.expiration = expiration;
     }
 
@@ -77,16 +77,16 @@ public TimeValue getExpiration() {
         return expiration;
     }
 
-    public void setExpiration(TimeValue expiration) {
+    public void setExpiration(@Nullable TimeValue expiration) {
         this.expiration = expiration;
     }
 
     public List<RoleDescriptor> getRoleDescriptors() {
         return roleDescriptors;
     }
 
-    public void setRoleDescriptors(List<RoleDescriptor> roleDescriptors) {
-        this.roleDescriptors = Collections.unmodifiableList(Objects.requireNonNull(roleDescriptors, "role descriptors may not be null"));
+    public void setRoleDescriptors(@Nullable List<RoleDescriptor> roleDescriptors) {
+        this.roleDescriptors = (roleDescriptors == null) ? Collections.emptyList() : Collections.unmodifiableList(roleDescriptors);
     }
 
     public WriteRequest.RefreshPolicy getRefreshPolicy() {

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

@@ -39,7 +39,7 @@ public final class CreateApiKeyRequestBuilder extends ActionRequestBuilder<Creat
 
     static {
         PARSER.declareString(constructorArg(), new ParseField("name"));
-        PARSER.declareNamedObjects(constructorArg(), (p, c, n) -> {
+        PARSER.declareNamedObjects(optionalConstructorArg(), (p, c, n) -> {
             p.nextToken();
             return RoleDescriptor.parse(n, p, false);
         }, new ParseField("role_descriptors"));

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

@@ -59,4 +59,22 @@ public void testParserAndCreateApiRequestBuilder() throws IOException {
             assertThat(request.getExpiration(), equalTo(TimeValue.parseTimeValue("1d", "expiration")));
         }
     }
+
+    public void testParserAndCreateApiRequestBuilderWithNullOrEmptyRoleDescriptors() throws IOException {
+        boolean withExpiration = randomBoolean();
+        boolean noRoleDescriptorsField = randomBoolean();
+        final String json = "{ \"name\" : \"my-api-key\""
+                + ((withExpiration) ? ", \"expiration\": \"1d\"" : "")
+                + ((noRoleDescriptorsField) ? "" : ", \"role_descriptors\": {}")
+                + "}";
+        final BytesArray source = new BytesArray(json);
+        final NodeClient mockClient = mock(NodeClient.class);
+        final CreateApiKeyRequest request = new CreateApiKeyRequestBuilder(mockClient).source(source, XContentType.JSON).request();
+        final List<RoleDescriptor> actualRoleDescriptors = request.getRoleDescriptors();
+        assertThat(request.getName(), equalTo("my-api-key"));
+        assertThat(actualRoleDescriptors.size(), is(0));
+        if (withExpiration) {
+            assertThat(request.getExpiration(), equalTo(TimeValue.parseTimeValue("1d", "expiration")));
+        }
+    }
 }

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

@@ -16,6 +16,7 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 import static org.hamcrest.Matchers.containsString;
@@ -82,10 +83,16 @@ public void testSerialization() throws IOException {
         final TimeValue expiration = randomBoolean() ? null :
             TimeValue.parseTimeValue(randomTimeValue(), "test serialization of create api key");
         final WriteRequest.RefreshPolicy refreshPolicy = randomFrom(WriteRequest.RefreshPolicy.values());
-        final int numDescriptors = randomIntBetween(0, 4);
-        final List<RoleDescriptor> descriptorList = new ArrayList<>();
-        for (int i = 0; i < numDescriptors; i++) {
-            descriptorList.add(new RoleDescriptor("role_" + i, new String[] { "all" }, null, null));
+        boolean nullOrEmptyRoleDescriptors = randomBoolean();
+        final List<RoleDescriptor> descriptorList;
+        if (nullOrEmptyRoleDescriptors) {
+            descriptorList = randomBoolean() ? null : Collections.emptyList();
+        } else {
+            final int numDescriptors = randomIntBetween(1, 4);
+            descriptorList = new ArrayList<>();
+            for (int i = 0; i < numDescriptors; i++) {
+                descriptorList.add(new RoleDescriptor("role_" + i, new String[] { "all" }, null, null));
+            }
         }
 
         final CreateApiKeyRequest request = new CreateApiKeyRequest();
@@ -95,9 +102,7 @@ public void testSerialization() throws IOException {
         if (refreshPolicy != request.getRefreshPolicy() || randomBoolean()) {
             request.setRefreshPolicy(refreshPolicy);
         }
-        if (descriptorList.isEmpty() == false || randomBoolean()) {
-            request.setRoleDescriptors(descriptorList);
-        }
+        request.setRoleDescriptors(descriptorList);
 
         try (BytesStreamOutput out = new BytesStreamOutput()) {
             request.writeTo(out);
@@ -106,7 +111,11 @@ public void testSerialization() throws IOException {
                 assertEquals(name, serialized.getName());
                 assertEquals(expiration, serialized.getExpiration());
                 assertEquals(refreshPolicy, serialized.getRefreshPolicy());
-                assertEquals(descriptorList, serialized.getRoleDescriptors());
+                if (nullOrEmptyRoleDescriptors) {
+                    assertThat(serialized.getRoleDescriptors().isEmpty(), is(true));
+                } else {
+                    assertEquals(descriptorList, serialized.getRoleDescriptors());
+                }
             }
         }
     }