Browse Source

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

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
Yogesh Gaikwad 6 years ago
parent
commit
3b876159e2

+ 4 - 4
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>>.
 

+ 3 - 4
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

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

@@ -43,13 +43,13 @@ public final class CreateApiKeyRequest extends ActionRequest {
      * @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) ? List.of() : List.copyOf(roleDescriptors);
         this.expiration = expiration;
     }
 
@@ -57,7 +57,7 @@ public final class CreateApiKeyRequest extends ActionRequest {
         super(in);
         this.name = in.readString();
         this.expiration = in.readOptionalTimeValue();
-        this.roleDescriptors = Collections.unmodifiableList(in.readList(RoleDescriptor::new));
+        this.roleDescriptors = List.copyOf(in.readList(RoleDescriptor::new));
         this.refreshPolicy = WriteRequest.RefreshPolicy.readFrom(in);
     }
 
@@ -77,7 +77,7 @@ public final class CreateApiKeyRequest extends ActionRequest {
         return expiration;
     }
 
-    public void setExpiration(TimeValue expiration) {
+    public void setExpiration(@Nullable TimeValue expiration) {
         this.expiration = expiration;
     }
 
@@ -85,8 +85,8 @@ public final class CreateApiKeyRequest extends ActionRequest {
         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) ? List.of() : List.copyOf(roleDescriptors);
     }
 
     public WriteRequest.RefreshPolicy getRefreshPolicy() {

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

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

@@ -59,4 +59,22 @@ public class CreateApiKeyRequestBuilderTests extends ESTestCase {
             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")));
+        }
+    }
 }

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

@@ -82,10 +82,16 @@ public class CreateApiKeyRequestTests extends ESTestCase {
         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 : List.of();
+        } 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 +101,7 @@ public class CreateApiKeyRequestTests extends ESTestCase {
         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 +110,11 @@ public class CreateApiKeyRequestTests extends ESTestCase {
                 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());
+                }
             }
         }
     }