From 00c953346f27c4647ddd2429da9a85fc5a64d34c Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Mon, 21 Dec 2020 10:45:58 +1100 Subject: [PATCH] Deprecate the id field for the InvalidateApiKey API (#66317) This PR deprecates the usage of the id field in the payload for the InvalidateApiKey API. The ids field introduced in #63224 is now the recommended way for performing (bulk) API key invalidation. --- .../security/InvalidateApiKeyRequest.java | 90 +++++++++++++++---- .../SecurityDocumentationIT.java | 18 ++++ .../InvalidateApiKeyRequestTests.java | 26 ++++-- .../security/invalidate-api-key.asciidoc | 6 +- .../security/invalidate-api-keys.asciidoc | 24 ++--- .../action/InvalidateApiKeyRequest.java | 66 +++++++++----- .../action/InvalidateApiKeyRequestTests.java | 35 ++++---- .../ManageOwnApiKeyClusterPrivilegeTests.java | 2 +- .../apikey/RestInvalidateApiKeyAction.java | 3 +- .../audit/logfile/LoggingAuditTrailTests.java | 21 +++-- .../RestInvalidateApiKeyActionTests.java | 8 +- .../rest-api-spec/test/api_key/10_basic.yml | 20 +---- 12 files changed, 203 insertions(+), 116 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/security/InvalidateApiKeyRequest.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/security/InvalidateApiKeyRequest.java index 351294e36d38..f7af3da600c8 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/security/InvalidateApiKeyRequest.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/security/InvalidateApiKeyRequest.java @@ -26,6 +26,9 @@ import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; +import java.util.Arrays; +import java.util.List; +import java.util.stream.IntStream; /** * Request for invalidating API key(s) so that it can no longer be used @@ -34,21 +37,28 @@ public final class InvalidateApiKeyRequest implements Validatable, ToXContentObj private final String realmName; private final String userName; - private final String id; + private final List ids; private final String name; private final boolean ownedByAuthenticatedUser; // pkg scope for testing + @Deprecated InvalidateApiKeyRequest(@Nullable String realmName, @Nullable String userName, @Nullable String apiKeyId, @Nullable String apiKeyName, boolean ownedByAuthenticatedUser) { - if (Strings.hasText(realmName) == false && Strings.hasText(userName) == false && Strings.hasText(apiKeyId) == false - && Strings.hasText(apiKeyName) == false && ownedByAuthenticatedUser == false) { - throwValidationError("One of [api key id, api key name, username, realm name] must be specified if [owner] flag is false"); + this(realmName, userName, apiKeyName, ownedByAuthenticatedUser, apiKeyIdToIds(apiKeyId)); + } + + InvalidateApiKeyRequest(@Nullable String realmName, @Nullable String userName, + @Nullable String apiKeyName, boolean ownedByAuthenticatedUser, @Nullable List apiKeyIds) { + validateApiKeyIds(apiKeyIds); + if (Strings.hasText(realmName) == false && Strings.hasText(userName) == false && apiKeyIds == null + && Strings.hasText(apiKeyName) == false && ownedByAuthenticatedUser == false) { + throwValidationError("One of [api key id(s), api key name, username, realm name] must be specified if [owner] flag is false"); } - if (Strings.hasText(apiKeyId) || Strings.hasText(apiKeyName)) { + if (apiKeyIds != null || Strings.hasText(apiKeyName)) { if (Strings.hasText(realmName) || Strings.hasText(userName)) { throwValidationError( - "username or realm name must not be specified when the api key id or api key name is specified"); + "username or realm name must not be specified when the api key id(s) or api key name is specified"); } } if (ownedByAuthenticatedUser) { @@ -56,16 +66,33 @@ public final class InvalidateApiKeyRequest implements Validatable, ToXContentObj throwValidationError("neither username nor realm-name may be specified when invalidating owned API keys"); } } - if (Strings.hasText(apiKeyId) && Strings.hasText(apiKeyName)) { - throwValidationError("only one of [api key id, api key name] can be specified"); + if (apiKeyIds != null && Strings.hasText(apiKeyName)) { + throwValidationError("only one of [api key id(s), api key name] can be specified"); } this.realmName = realmName; this.userName = userName; - this.id = apiKeyId; + this.ids = apiKeyIds == null ? null : List.copyOf(apiKeyIds); this.name = apiKeyName; this.ownedByAuthenticatedUser = ownedByAuthenticatedUser; } + private void validateApiKeyIds(@Nullable List apiKeyIds) { + if (apiKeyIds != null) { + if (apiKeyIds.size() == 0) { + throwValidationError("Argument [apiKeyIds] cannot be an empty array"); + } else { + final int[] idxOfBlankIds = IntStream.range(0, apiKeyIds.size()) + .filter(i -> Strings.hasText(apiKeyIds.get(i)) == false).toArray(); + if (idxOfBlankIds.length > 0) { + throwValidationError("Argument [apiKeyIds] must not contain blank id, but got blank " + + (idxOfBlankIds.length == 1 ? "id" : "ids") + " at index " + + (idxOfBlankIds.length == 1 ? "position" : "positions") + ": " + + Arrays.toString(idxOfBlankIds)); + } + } + } + } + private void throwValidationError(String message) { throw new IllegalArgumentException(message); } @@ -78,8 +105,20 @@ public final class InvalidateApiKeyRequest implements Validatable, ToXContentObj return userName; } + @Deprecated public String getId() { - return id; + if (ids == null) { + return null; + } else if (ids.size() == 1) { + return ids.get(0); + } else { + throw new IllegalArgumentException("Cannot get a single api key id when multiple ids have been set [" + + Strings.collectionToCommaDelimitedString(ids) + "]"); + } + } + + public List getIds() { + return ids; } public String getName() { @@ -96,7 +135,7 @@ public final class InvalidateApiKeyRequest implements Validatable, ToXContentObj * @return {@link InvalidateApiKeyRequest} */ public static InvalidateApiKeyRequest usingRealmName(String realmName) { - return new InvalidateApiKeyRequest(realmName, null, null, null, false); + return new InvalidateApiKeyRequest(realmName, null, null, false, null); } /** @@ -105,7 +144,7 @@ public final class InvalidateApiKeyRequest implements Validatable, ToXContentObj * @return {@link InvalidateApiKeyRequest} */ public static InvalidateApiKeyRequest usingUserName(String userName) { - return new InvalidateApiKeyRequest(null, userName, null, null, false); + return new InvalidateApiKeyRequest(null, userName, null, false, null); } /** @@ -115,7 +154,7 @@ public final class InvalidateApiKeyRequest implements Validatable, ToXContentObj * @return {@link InvalidateApiKeyRequest} */ public static InvalidateApiKeyRequest usingRealmAndUserName(String realmName, String userName) { - return new InvalidateApiKeyRequest(realmName, userName, null, null, false); + return new InvalidateApiKeyRequest(realmName, userName, null, false, null); } /** @@ -126,7 +165,18 @@ public final class InvalidateApiKeyRequest implements Validatable, ToXContentObj * @return {@link InvalidateApiKeyRequest} */ public static InvalidateApiKeyRequest usingApiKeyId(String apiKeyId, boolean ownedByAuthenticatedUser) { - return new InvalidateApiKeyRequest(null, null, apiKeyId, null, ownedByAuthenticatedUser); + return new InvalidateApiKeyRequest(null, null, null, ownedByAuthenticatedUser, apiKeyIdToIds(apiKeyId)); + } + + /** + * Creates invalidate API key request for given api key ids + * @param apiKeyIds api key ids + * @param ownedByAuthenticatedUser set {@code true} if the request is only for the API keys owned by current authenticated user else + * {@code false} + * @return {@link InvalidateApiKeyRequest} + */ + public static InvalidateApiKeyRequest usingApiKeyIds(List apiKeyIds, boolean ownedByAuthenticatedUser) { + return new InvalidateApiKeyRequest(null, null, null, ownedByAuthenticatedUser, apiKeyIds); } /** @@ -137,14 +187,14 @@ public final class InvalidateApiKeyRequest implements Validatable, ToXContentObj * @return {@link InvalidateApiKeyRequest} */ public static InvalidateApiKeyRequest usingApiKeyName(String apiKeyName, boolean ownedByAuthenticatedUser) { - return new InvalidateApiKeyRequest(null, null, null, apiKeyName, ownedByAuthenticatedUser); + return new InvalidateApiKeyRequest(null, null, apiKeyName, ownedByAuthenticatedUser, null); } /** * Creates invalidate api key request to invalidate api keys owned by the current authenticated user. */ public static InvalidateApiKeyRequest forOwnedApiKeys() { - return new InvalidateApiKeyRequest(null, null, null, null, true); + return new InvalidateApiKeyRequest(null, null, null, true, null); } @Override @@ -156,8 +206,8 @@ public final class InvalidateApiKeyRequest implements Validatable, ToXContentObj if (userName != null) { builder.field("username", userName); } - if (id != null) { - builder.field("id", id); + if (ids != null) { + builder.field("ids", ids); } if (name != null) { builder.field("name", name); @@ -165,4 +215,8 @@ public final class InvalidateApiKeyRequest implements Validatable, ToXContentObj builder.field("owner", ownedByAuthenticatedUser); return builder.endObject(); } + + static List apiKeyIdToIds(@Nullable String apiKeyId) { + return Strings.hasText(apiKeyId) ? List.of(apiKeyId) : null; + } } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SecurityDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SecurityDocumentationIT.java index 187de1c88581..d76dd6188002 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SecurityDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SecurityDocumentationIT.java @@ -2204,6 +2204,24 @@ public class SecurityDocumentationIT extends ESRestHighLevelClientTestCase { assertThat(previouslyInvalidatedApiKeyIds.size(), equalTo(0)); } + { + // tag::invalidate-api-key-ids-request + InvalidateApiKeyRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingApiKeyIds( + Arrays.asList("kI3QZHYBnpSXoDRq1XzR", "ko3SZHYBnpSXoDRqk3zm"), false); + // end::invalidate-api-key-ids-request + + InvalidateApiKeyResponse invalidateApiKeyResponse = client.security().invalidateApiKey(invalidateApiKeyRequest, + RequestOptions.DEFAULT); + + final List errors = invalidateApiKeyResponse.getErrors(); + final List invalidatedApiKeyIds = invalidateApiKeyResponse.getInvalidatedApiKeys(); + final List previouslyInvalidatedApiKeyIds = invalidateApiKeyResponse.getPreviouslyInvalidatedApiKeys(); + + assertTrue(errors.isEmpty()); + assertThat(invalidatedApiKeyIds.size(), equalTo(0)); + assertThat(previouslyInvalidatedApiKeyIds.size(), equalTo(0)); + } + { createApiKeyRequest = new CreateApiKeyRequest("k2", roles, expiration, refreshPolicy); CreateApiKeyResponse createApiKeyResponse2 = client.security().createApiKey(createApiKeyRequest, RequestOptions.DEFAULT); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/security/InvalidateApiKeyRequestTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/security/InvalidateApiKeyRequestTests.java index a29adb9ea382..db19bbc54296 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/security/InvalidateApiKeyRequestTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/security/InvalidateApiKeyRequestTests.java @@ -24,14 +24,24 @@ import org.elasticsearch.test.ESTestCase; import java.io.IOException; import java.util.Optional; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import static org.elasticsearch.client.security.InvalidateApiKeyRequest.apiKeyIdToIds; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; public class InvalidateApiKeyRequestTests extends ESTestCase { public void testRequestValidation() { - InvalidateApiKeyRequest request = InvalidateApiKeyRequest.usingApiKeyId(randomAlphaOfLength(5), randomBoolean()); + InvalidateApiKeyRequest request; + if (randomBoolean()) { + request = InvalidateApiKeyRequest.usingApiKeyId(randomAlphaOfLength(5), randomBoolean()); + } else { + request = InvalidateApiKeyRequest.usingApiKeyIds( + IntStream.range(1, randomIntBetween(1, 5)).mapToObj(ignored -> randomAlphaOfLength(5)).collect(Collectors.toList()), + randomBoolean()); + } Optional ve = request.validate(); assertThat(ve.isPresent(), is(false)); request = InvalidateApiKeyRequest.usingApiKeyName(randomAlphaOfLength(5), randomBoolean()); @@ -61,19 +71,19 @@ public class InvalidateApiKeyRequestTests extends ESTestCase { { "realm", randomNullOrEmptyString(), randomNullOrEmptyString(), randomNullOrEmptyString(), "true" }, { randomNullOrEmptyString(), "user", randomNullOrEmptyString(), randomNullOrEmptyString(), "true" } }; String[] expectedErrorMessages = new String[] { - "One of [api key id, api key name, username, realm name] must be specified if [owner] flag is false", - "username or realm name must not be specified when the api key id or api key name is specified", - "username or realm name must not be specified when the api key id or api key name is specified", - "username or realm name must not be specified when the api key id or api key name is specified", - "only one of [api key id, api key name] can be specified", + "One of [api key id(s), api key name, username, realm name] must be specified if [owner] flag is false", + "username or realm name must not be specified when the api key id(s) or api key name is specified", + "username or realm name must not be specified when the api key id(s) or api key name is specified", + "username or realm name must not be specified when the api key id(s) or api key name is specified", + "only one of [api key id(s), api key name] can be specified", "neither username nor realm-name may be specified when invalidating owned API keys", "neither username nor realm-name may be specified when invalidating owned API keys" }; for (int i = 0; i < inputs.length; i++) { final int caseNo = i; IllegalArgumentException ve = expectThrows(IllegalArgumentException.class, - () -> new InvalidateApiKeyRequest(inputs[caseNo][0], inputs[caseNo][1], inputs[caseNo][2], inputs[caseNo][3], - Boolean.valueOf(inputs[caseNo][4]))); + () -> new InvalidateApiKeyRequest(inputs[caseNo][0], inputs[caseNo][1], inputs[caseNo][3], + Boolean.valueOf(inputs[caseNo][4]), apiKeyIdToIds(inputs[caseNo][2]))); assertNotNull(ve); assertThat(ve.getMessage(), equalTo(expectedErrorMessages[caseNo])); } diff --git a/docs/java-rest/high-level/security/invalidate-api-key.asciidoc b/docs/java-rest/high-level/security/invalidate-api-key.asciidoc index d1f747da8826..61ab471ce86d 100644 --- a/docs/java-rest/high-level/security/invalidate-api-key.asciidoc +++ b/docs/java-rest/high-level/security/invalidate-api-key.asciidoc @@ -23,10 +23,10 @@ The +{request}+ supports invalidating . A specific key or all API keys owned by the current authenticated user -===== Specific API key by API key id +===== Specific API keys by a list of API key ids ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- -include-tagged::{doc-tests-file}[invalidate-api-key-id-request] +include-tagged::{doc-tests-file}[invalidate-api-key-ids-request] -------------------------------------------------- ===== Specific API key by API key name @@ -80,4 +80,4 @@ invalidated. ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- include-tagged::{doc-tests-file}[{api}-response] --------------------------------------------------- \ No newline at end of file +-------------------------------------------------- diff --git a/x-pack/docs/en/rest-api/security/invalidate-api-keys.asciidoc b/x-pack/docs/en/rest-api/security/invalidate-api-keys.asciidoc index 64806da5c7f0..a757225312ae 100644 --- a/x-pack/docs/en/rest-api/security/invalidate-api-keys.asciidoc +++ b/x-pack/docs/en/rest-api/security/invalidate-api-keys.asciidoc @@ -31,6 +31,7 @@ The following parameters can be specified in the body of a DELETE request and pertain to invalidating api keys: `id`:: +deprecated:[7.12.0, "Use ids instead"] (Optional, string) An API key id. This parameter cannot be used when any of `ids`, `name`, `realm_name` or `username` are used. @@ -56,7 +57,7 @@ by the currently authenticated user. Defaults to false. The 'realm_name' or 'username' parameters cannot be specified when this parameter is set to 'true' as they are assumed to be the currently authenticated ones. -NOTE: At least one of "id", "name", "username" and "realm_name" must be specified +NOTE: At least one of "id", "ids", "name", "username" and "realm_name" must be specified if "owner" is "false" (default). [[security-api-invalidate-api-key-response-body]] @@ -94,22 +95,9 @@ API key information. For example: // TESTRESPONSE[s/VuaCfGcBCdbkQm-e5aOx/$body.id/] // TESTRESPONSE[s/ui2lp2axTNmsyakw9tvNnw/$body.api_key/] -The following example invalidates the API key identified by specified `id` +The following example invalidates the API key identified by specified `ids` immediately: -[source,console] --------------------------------------------------- -DELETE /_security/api_key -{ - "id" : "VuaCfGcBCdbkQm-e5aOx" -} --------------------------------------------------- -// TEST[s/VuaCfGcBCdbkQm-e5aOx/$body.id/] -// TEST[continued] - -The above request can also be performed using the `ids` field which takes -a list of API keys for bulk invalidation: - [source,console] -------------------------------------------------- DELETE /_security/api_key @@ -117,6 +105,8 @@ DELETE /_security/api_key "ids" : [ "VuaCfGcBCdbkQm-e5aOx" ] } -------------------------------------------------- +// TEST[s/VuaCfGcBCdbkQm-e5aOx/$body.id/] +// TEST[continued] The following example invalidates the API key identified by specified `name` immediately: @@ -151,14 +141,14 @@ DELETE /_security/api_key } -------------------------------------------------- -The following example invalidates the API key identified by the specified `id` if +The following example invalidates the API key identified by the specified `ids` if it is owned by the currently authenticated user immediately: [source,console] -------------------------------------------------- DELETE /_security/api_key { - "id" : "VuaCfGcBCdbkQm-e5aOx", + "ids" : ["VuaCfGcBCdbkQm-e5aOx"], "owner" : "true" } -------------------------------------------------- diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequest.java index 1c74bc91b377..9927e91eee50 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequest.java @@ -33,7 +33,7 @@ public final class InvalidateApiKeyRequest extends ActionRequest { private final boolean ownedByAuthenticatedUser; public InvalidateApiKeyRequest() { - this(null, null, null, null, false, null); + this(null, null, null, false, null); } public InvalidateApiKeyRequest(StreamInput in) throws IOException { @@ -44,8 +44,9 @@ public final class InvalidateApiKeyRequest extends ActionRequest { ids = in.readOptionalStringArray(); } else { final String id = in.readOptionalString(); - ids = Strings.hasText(id) == false ? null : new String[] { id }; + ids = Strings.hasText(id) ? new String[] { id } : null; } + validateIds(ids); name = textOrNull(in.readOptionalString()); if (in.getVersion().onOrAfter(Version.V_7_4_0)) { ownedByAuthenticatedUser = in.readOptionalBoolean(); @@ -54,17 +55,18 @@ public final class InvalidateApiKeyRequest extends ActionRequest { } } + @Deprecated public InvalidateApiKeyRequest(@Nullable String realmName, @Nullable String userName, @Nullable String id, @Nullable String name, boolean ownedByAuthenticatedUser) { - this(realmName, userName, id, name, ownedByAuthenticatedUser, null); + this(realmName, userName, name, ownedByAuthenticatedUser, Strings.hasText(id) ? new String[] { id } : null); } + @Deprecated public InvalidateApiKeyRequest(@Nullable String realmName, @Nullable String userName, @Nullable String id, @Nullable String name, boolean ownedByAuthenticatedUser, @Nullable String[] ids) { if (id != null && ids != null) { throw new IllegalArgumentException("Must use either [id] or [ids], not both at the same time"); } - this.realmName = textOrNull(realmName); this.userName = textOrNull(userName); if (Strings.hasText(id)) { @@ -72,6 +74,17 @@ public final class InvalidateApiKeyRequest extends ActionRequest { } else { this.ids = ids; } + validateIds(this.ids); + this.name = textOrNull(name); + this.ownedByAuthenticatedUser = ownedByAuthenticatedUser; + } + + public InvalidateApiKeyRequest(@Nullable String realmName, @Nullable String userName, + @Nullable String name, boolean ownedByAuthenticatedUser, @Nullable String[] ids) { + validateIds(ids); + this.realmName = textOrNull(realmName); + this.userName = textOrNull(userName); + this.ids = ids; this.name = textOrNull(name); this.ownedByAuthenticatedUser = ownedByAuthenticatedUser; } @@ -107,7 +120,7 @@ public final class InvalidateApiKeyRequest extends ActionRequest { * @return {@link InvalidateApiKeyRequest} */ public static InvalidateApiKeyRequest usingRealmName(String realmName) { - return new InvalidateApiKeyRequest(realmName, null, null, null, false); + return new InvalidateApiKeyRequest(realmName, null, null, false, null); } /** @@ -117,7 +130,7 @@ public final class InvalidateApiKeyRequest extends ActionRequest { * @return {@link InvalidateApiKeyRequest} */ public static InvalidateApiKeyRequest usingUserName(String userName) { - return new InvalidateApiKeyRequest(null, userName, null, null, false); + return new InvalidateApiKeyRequest(null, userName, null, false, null); } /** @@ -128,7 +141,7 @@ public final class InvalidateApiKeyRequest extends ActionRequest { * @return {@link InvalidateApiKeyRequest} */ public static InvalidateApiKeyRequest usingRealmAndUserName(String realmName, String userName) { - return new InvalidateApiKeyRequest(realmName, userName, null, null, false); + return new InvalidateApiKeyRequest(realmName, userName, null, false, null); } /** @@ -140,7 +153,7 @@ public final class InvalidateApiKeyRequest extends ActionRequest { * @return {@link InvalidateApiKeyRequest} */ public static InvalidateApiKeyRequest usingApiKeyId(String id, boolean ownedByAuthenticatedUser) { - return new InvalidateApiKeyRequest(null, null, id, null, ownedByAuthenticatedUser); + return new InvalidateApiKeyRequest(null, null, null, ownedByAuthenticatedUser, new String[]{ id }); } /** @@ -152,32 +165,19 @@ public final class InvalidateApiKeyRequest extends ActionRequest { * @return {@link InvalidateApiKeyRequest} */ public static InvalidateApiKeyRequest usingApiKeyName(String name, boolean ownedByAuthenticatedUser) { - return new InvalidateApiKeyRequest(null, null, null, name, ownedByAuthenticatedUser); + return new InvalidateApiKeyRequest(null, null, name, ownedByAuthenticatedUser, null); } /** * Creates invalidate api key request to invalidate api keys owned by the current authenticated user. */ public static InvalidateApiKeyRequest forOwnedApiKeys() { - return new InvalidateApiKeyRequest(null, null, null, null, true); + return new InvalidateApiKeyRequest(null, null, null, true, null); } @Override public ActionRequestValidationException validate() { ActionRequestValidationException validationException = null; - if (ids != null) { - if (ids.length == 0) { - validationException = addValidationError("Field [ids] cannot be an empty array", validationException); - } else { - final int[] idxOfBlankIds = IntStream.range(0, ids.length).filter(i -> Strings.hasText(ids[i]) == false).toArray(); - if (idxOfBlankIds.length > 0) { - validationException = addValidationError("Field [ids] must not contain blank id, but got blank " - + (idxOfBlankIds.length == 1 ? "id" : "ids") + " at index " - + (idxOfBlankIds.length == 1 ? "position" : "positions") + ": " - + Arrays.toString(idxOfBlankIds), validationException); - } - } - } if (Strings.hasText(realmName) == false && Strings.hasText(userName) == false && ids == null && Strings.hasText(name) == false && ownedByAuthenticatedUser == false) { validationException = addValidationError("One of [api key id(s), api key name, username, realm name] must be specified if " + @@ -247,4 +247,24 @@ public final class InvalidateApiKeyRequest extends ActionRequest { public int hashCode() { return Objects.hash(realmName, userName, ids, name, ownedByAuthenticatedUser); } + + private void validateIds(@Nullable String[] ids) { + if (ids != null) { + if (ids.length == 0) { + final ActionRequestValidationException validationException = new ActionRequestValidationException(); + validationException.addValidationError("Field [ids] cannot be an empty array"); + throw validationException; + } else { + final int[] idxOfBlankIds = IntStream.range(0, ids.length).filter(i -> Strings.hasText(ids[i]) == false).toArray(); + if (idxOfBlankIds.length > 0) { + final ActionRequestValidationException validationException = new ActionRequestValidationException(); + validationException.addValidationError("Field [ids] must not contain blank id, but got blank " + + (idxOfBlankIds.length == 1 ? "id" : "ids") + " at index " + + (idxOfBlankIds.length == 1 ? "position" : "positions") + ": " + + Arrays.toString(idxOfBlankIds)); + throw validationException; + } + } + } + } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequestTests.java index 50e48c86b52d..9a93c960ad22 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequestTests.java @@ -43,27 +43,23 @@ public class InvalidateApiKeyRequestTests extends ESTestCase { } public void testNonNullIdsCannotBeEmptyNorContainBlankId() { - InvalidateApiKeyRequest invalidateApiKeyRequest = new InvalidateApiKeyRequest( - randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)), - randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)), - null, - randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)), - false, - new String[]{}); - ActionRequestValidationException validationException = invalidateApiKeyRequest.validate(); - assertNotNull(validationException); + + ActionRequestValidationException validationException = + expectThrows(ActionRequestValidationException.class, () -> new InvalidateApiKeyRequest( + randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)), + randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)), + randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)), + false, + new String[] {})); assertThat(validationException.getMessage(), containsString("Field [ids] cannot be an empty array")); - invalidateApiKeyRequest = new InvalidateApiKeyRequest( - randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)), - randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)), - null, - randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)), - false, - new String[]{randomAlphaOfLength(12), null}); - validationException = invalidateApiKeyRequest.validate(); - assertNotNull(validationException); - + validationException = + expectThrows(ActionRequestValidationException.class, () -> new InvalidateApiKeyRequest( + randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)), + randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)), + randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)), + false, + new String[] { randomAlphaOfLength(12), null })); assertThat(validationException.getMessage(), containsString("Field [ids] must not contain blank id, " + "but got blank id at index position: [1]")); } @@ -232,7 +228,6 @@ public class InvalidateApiKeyRequestTests extends ESTestCase { final InvalidateApiKeyRequest invalidateApiKeyRequest = new InvalidateApiKeyRequest( randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)), randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)), - null, randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)), false, new String[]{randomAlphaOfLength(12), randomAlphaOfLength(12)}); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java index 64b040e247b2..8364e5f521fe 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java @@ -94,7 +94,7 @@ public class ManageOwnApiKeyClusterPrivilegeTests extends ESTestCase { final TransportRequest invalidateApiKeyRequest = randomFrom( InvalidateApiKeyRequest.usingRealmAndUserName("realm1", randomAlphaOfLength(7)), InvalidateApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), "joe"), - new InvalidateApiKeyRequest(randomAlphaOfLength(5), randomAlphaOfLength(7), null, null, false)); + new InvalidateApiKeyRequest(randomAlphaOfLength(5), randomAlphaOfLength(7), null, false, null)); assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/get", getApiKeyRequest, authentication)); assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", invalidateApiKeyRequest, authentication)); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestInvalidateApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestInvalidateApiKeyAction.java index ba3a885fa719..8e83a9098c5c 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestInvalidateApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestInvalidateApiKeyAction.java @@ -42,7 +42,7 @@ public final class RestInvalidateApiKeyAction extends SecurityBaseRestHandler { static { PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("realm_name")); PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("username")); - PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("id")); + PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("id").withAllDeprecated("ids")); PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("name")); PARSER.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), new ParseField("owner")); PARSER.declareStringArray(ConstructingObjectParser.optionalConstructorArg(), new ParseField("ids")); @@ -77,5 +77,4 @@ public final class RestInvalidateApiKeyAction extends SecurityBaseRestHandler { public String getName() { return "xpack_security_invalidate_api_key"; } - } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java index a484460df3c7..a328d9094f4d 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java @@ -596,13 +596,22 @@ public class LoggingAuditTrailTests extends ESTestCase { final InvalidateApiKeyRequest invalidateApiKeyRequest; if (randomBoolean()) { - invalidateApiKeyRequest = new InvalidateApiKeyRequest(randomFrom(randomAlphaOfLength(8), null), - randomFrom(randomAlphaOfLength(8), null), null, randomFrom(randomAlphaOfLength(8), null), randomBoolean(), - randomFrom(randomArray(3, String[]::new, () -> randomAlphaOfLength(8)), null)); + invalidateApiKeyRequest = new InvalidateApiKeyRequest( + randomFrom(randomAlphaOfLength(8), null), + randomFrom(randomAlphaOfLength(8), null), + null, + randomFrom(randomAlphaOfLength(8), null), + randomBoolean(), + randomFrom(randomArray(1,3, String[]::new, () -> randomAlphaOfLength(8)), null) + ); } else { - invalidateApiKeyRequest = new InvalidateApiKeyRequest(randomFrom(randomAlphaOfLength(8), null), - randomFrom(randomAlphaOfLength(8), null), randomAlphaOfLength(8), randomFrom(randomAlphaOfLength(8), null), - randomBoolean()); + invalidateApiKeyRequest = new InvalidateApiKeyRequest( + randomFrom(randomAlphaOfLength(8), null), + randomFrom(randomAlphaOfLength(8), null), + randomAlphaOfLength(8), + randomFrom(randomAlphaOfLength(8), null), + randomBoolean() + ); } auditTrail.accessGranted(requestId, authentication, InvalidateApiKeyAction.NAME, invalidateApiKeyRequest, authorizationInfo); List output = CapturingLogger.output(logger.getName(), Level.INFO); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestInvalidateApiKeyActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestInvalidateApiKeyActionTests.java index 8da8f5950111..8df2cdddee10 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestInvalidateApiKeyActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestInvalidateApiKeyActionTests.java @@ -69,7 +69,9 @@ public class RestInvalidateApiKeyActionTests extends ESTestCase { final String json3 = "{ \"username\": \"user-x\" }"; final String json4 = "{ \"id\" : \"api-key-id-1\" }"; final String json5 = "{ \"name\" : \"api-key-name-1\" }"; - final String json = randomFrom(json1, json2, json3, json4, json5); + final String json6 = "{ \"ids\" : [\"api-key-id-1\"] }"; + final String json = randomFrom(json1, json2, json3, json4, json5, json6); + final boolean assertDeprecationWarning = json == json4; // we want object identity comparison here final FakeRestRequest restRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) .withContent(new BytesArray(json), XContentType.JSON).build(); @@ -117,7 +119,9 @@ public class RestInvalidateApiKeyActionTests extends ESTestCase { assertThat(actual.getPreviouslyInvalidatedApiKeys(), equalTo(invalidateApiKeyResponseExpected.getPreviouslyInvalidatedApiKeys())); assertThat(actual.getErrors(), equalTo(invalidateApiKeyResponseExpected.getErrors())); - + if (assertDeprecationWarning) { + assertWarnings("Deprecated field [id] used, replaced by [ids]"); + } } } diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/api_key/10_basic.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/api_key/10_basic.yml index 58cdc2657f5d..6cf2ff3c15d0 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/api_key/10_basic.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/api_key/10_basic.yml @@ -223,19 +223,6 @@ teardown: - set: { id: api_key_id_1 } - transform_and_set: { login_creds: "#base64EncodeCredentials(id,api_key)" } - - do: - headers: - Authorization: "Basic YXBpX2tleV91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" # api_key_user - security.invalidate_api_key: - body: > - { - "id": "${api_key_id_1}" - } - - length: { "invalidated_api_keys" : 1 } - - match: { "invalidated_api_keys.0" : "${api_key_id_1}" } - - length: { "previously_invalidated_api_keys" : 0 } - - match: { "error_count" : 0 } - - do: headers: Authorization: "Basic YXBpX2tleV91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" # api_key_user @@ -278,9 +265,10 @@ teardown: { "ids": [ "${api_key_id_1}", "${api_key_id_2}", "${api_key_id_3}" ] } - - length: { "invalidated_api_keys" : 2 } - - match: { "invalidated_api_keys.0" : "/^(${api_key_id_2}|${api_key_id_3})$/" } - - match: { "invalidated_api_keys.1" : "/^(${api_key_id_2}|${api_key_id_3})$/" } + - length: { "invalidated_api_keys" : 3 } + - match: { "invalidated_api_keys.0" : "/^(${api_key_id_1}|${api_key_id_2}|${api_key_id_3})$/" } + - match: { "invalidated_api_keys.1" : "/^(${api_key_id_1}|${api_key_id_2}|${api_key_id_3})$/" } + - match: { "invalidated_api_keys.2" : "/^(${api_key_id_1}|${api_key_id_2}|${api_key_id_3})$/" } - length: { "previously_invalidated_api_keys" : 0 } - match: { "error_count" : 0 }