From 11927e4d589b9cdfcd1017984586868b07028f8a Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Mon, 29 Jul 2019 13:29:59 +1000 Subject: [PATCH 01/10] REST API changes for manage-own-api-key privilege This commit adds a flag that can be set to `true` if the API key request (Get or Invalidate) is for the API keys owned by the current authenticated user only. The Get API behavior would be: - when `my_api_keys_only` is set to `true` `GET /_security/api_key?id=abcd&my_api_keys_only=true` the Rest controller will take care of setting `realm_name` and `username` to the values for authenticated user and only return results if it finds one owned by the currently authenticated user. - when `my_api_keys_only` is set to `false` (default) `GET /_security/api_key?id=abcd` the Rest controller will assume `realm_name` and `username` to be unspecified meaning it will try to search for the API key across users and realms. This will fail if the user has only `manage_own_api_key` privilege. Similarly, for Delete API key behavior: - when `my_api_keys_only` is set to `true` `DELETE /_security/api_key` ``` { "id" : "VuaCfGcBCdbkQm-e5aOx", "my_api_keys_only": "true" } ``` the Rest controller will take care of setting `realm_name` and `username` to the values for authenticated user and only invalidate key if it finds one owned by the currently authenticated user. - when `my_api_keys_only` is set to `false` (default) `DELETE /_security/api_key` ``` { "id" : "VuaCfGcBCdbkQm-e5aOx", "my_api_keys_only": "true" } ``` the Rest controller will assume `realm_name` and `username` to be unspecified meaning it will try to search for the API key across users and realms. This will fail if the user has only `manage_own_api_key` privilege. --- .../security/action/GetApiKeyRequest.java | 35 +++++++++---- .../action/InvalidateApiKeyRequest.java | 50 +++++++++++++------ .../action/GetApiKeyRequestTests.java | 46 +++++++++++------ .../action/InvalidateApiKeyRequestTests.java | 48 +++++++++++------- .../action/apikey/RestGetApiKeyAction.java | 3 +- .../apikey/RestInvalidateApiKeyAction.java | 4 +- .../security/authc/ApiKeyIntegTests.java | 25 ++++++---- 7 files changed, 143 insertions(+), 68 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java index 125602f68c5e2..ee04c593ccc51 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java @@ -26,9 +26,10 @@ public final class GetApiKeyRequest extends ActionRequest { private final String userName; private final String apiKeyId; private final String apiKeyName; + private final boolean myApiKeysOnly; public GetApiKeyRequest() { - this(null, null, null, null); + this(null, null, null, null, false); } public GetApiKeyRequest(StreamInput in) throws IOException { @@ -37,14 +38,16 @@ public GetApiKeyRequest(StreamInput in) throws IOException { userName = in.readOptionalString(); apiKeyId = in.readOptionalString(); apiKeyName = in.readOptionalString(); + myApiKeysOnly = in.readOptionalBoolean(); } public GetApiKeyRequest(@Nullable String realmName, @Nullable String userName, @Nullable String apiKeyId, - @Nullable String apiKeyName) { + @Nullable String apiKeyName, boolean myApiKeysOnly) { this.realmName = realmName; this.userName = userName; this.apiKeyId = apiKeyId; this.apiKeyName = apiKeyName; + this.myApiKeysOnly = myApiKeysOnly; } public String getRealmName() { @@ -63,13 +66,17 @@ public String getApiKeyName() { return apiKeyName; } + public boolean myApiKeysOnly() { + return myApiKeysOnly; + } + /** * Creates get API key request for given realm name * @param realmName realm name * @return {@link GetApiKeyRequest} */ public static GetApiKeyRequest usingRealmName(String realmName) { - return new GetApiKeyRequest(realmName, null, null, null); + return new GetApiKeyRequest(realmName, null, null, null, false); } /** @@ -78,7 +85,7 @@ public static GetApiKeyRequest usingRealmName(String realmName) { * @return {@link GetApiKeyRequest} */ public static GetApiKeyRequest usingUserName(String userName) { - return new GetApiKeyRequest(null, userName, null, null); + return new GetApiKeyRequest(null, userName, null, null, false); } /** @@ -88,25 +95,27 @@ public static GetApiKeyRequest usingUserName(String userName) { * @return {@link GetApiKeyRequest} */ public static GetApiKeyRequest usingRealmAndUserName(String realmName, String userName) { - return new GetApiKeyRequest(realmName, userName, null, null); + return new GetApiKeyRequest(realmName, userName, null, null, false); } /** * Creates get API key request for given api key id * @param apiKeyId api key id + * @param myApiKeysOnly set {@code true} if the request is only for the API keys owned by current authenticated user else {@code false} * @return {@link GetApiKeyRequest} */ - public static GetApiKeyRequest usingApiKeyId(String apiKeyId) { - return new GetApiKeyRequest(null, null, apiKeyId, null); + public static GetApiKeyRequest usingApiKeyId(String apiKeyId, boolean myApiKeysOnly) { + return new GetApiKeyRequest(null, null, apiKeyId, null, myApiKeysOnly); } /** * Creates get api key request for given api key name * @param apiKeyName api key name + * @param myApiKeysOnly set {@code true} if the request is only for the API keys owned by current authenticated user else {@code false} * @return {@link GetApiKeyRequest} */ - public static GetApiKeyRequest usingApiKeyName(String apiKeyName) { - return new GetApiKeyRequest(null, null, null, apiKeyName); + public static GetApiKeyRequest usingApiKeyName(String apiKeyName, boolean myApiKeysOnly) { + return new GetApiKeyRequest(null, null, null, apiKeyName, myApiKeysOnly); } @Override @@ -124,6 +133,13 @@ public ActionRequestValidationException validate() { validationException); } } + if (myApiKeysOnly) { + if (Strings.hasText(realmName) || Strings.hasText(userName)) { + validationException = addValidationError( + "username or realm name must not be specified when retrieving owned API keys", + validationException); + } + } if (Strings.hasText(apiKeyId) && Strings.hasText(apiKeyName)) { validationException = addValidationError("only one of [api key id, api key name] can be specified", validationException); } @@ -137,6 +153,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(userName); out.writeOptionalString(apiKeyId); out.writeOptionalString(apiKeyName); + out.writeOptionalBoolean(myApiKeysOnly); } } 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 15a2c87becd20..74158cb304233 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 @@ -26,9 +26,10 @@ public final class InvalidateApiKeyRequest extends ActionRequest { private final String userName; private final String id; private final String name; + private final boolean myApiKeysOnly; public InvalidateApiKeyRequest() { - this(null, null, null, null); + this(null, null, null, null, false); } public InvalidateApiKeyRequest(StreamInput in) throws IOException { @@ -37,14 +38,16 @@ public InvalidateApiKeyRequest(StreamInput in) throws IOException { userName = in.readOptionalString(); id = in.readOptionalString(); name = in.readOptionalString(); + myApiKeysOnly = in.readOptionalBoolean(); } public InvalidateApiKeyRequest(@Nullable String realmName, @Nullable String userName, @Nullable String id, - @Nullable String name) { + @Nullable String name, boolean myApiKeysOnly) { this.realmName = realmName; this.userName = userName; this.id = id; this.name = name; + this.myApiKeysOnly = myApiKeysOnly; } public String getRealmName() { @@ -63,65 +66,83 @@ public String getName() { return name; } + public boolean myApiKeysOnly() { + return myApiKeysOnly; + } + /** * Creates invalidate api key request for given realm name + * * @param realmName realm name * @return {@link InvalidateApiKeyRequest} */ public static InvalidateApiKeyRequest usingRealmName(String realmName) { - return new InvalidateApiKeyRequest(realmName, null, null, null); + return new InvalidateApiKeyRequest(realmName, null, null, null, false); } /** * Creates invalidate API key request for given user name + * * @param userName user name * @return {@link InvalidateApiKeyRequest} */ public static InvalidateApiKeyRequest usingUserName(String userName) { - return new InvalidateApiKeyRequest(null, userName, null, null); + return new InvalidateApiKeyRequest(null, userName, null, null, false); } /** * Creates invalidate API key request for given realm and user name + * * @param realmName realm name - * @param userName user name + * @param userName user name * @return {@link InvalidateApiKeyRequest} */ public static InvalidateApiKeyRequest usingRealmAndUserName(String realmName, String userName) { - return new InvalidateApiKeyRequest(realmName, userName, null, null); + return new InvalidateApiKeyRequest(realmName, userName, null, null, false); } /** * Creates invalidate API key request for given api key id + * * @param id api key id + * @param myApiKeysOnly 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 usingApiKeyId(String id) { - return new InvalidateApiKeyRequest(null, null, id, null); + public static InvalidateApiKeyRequest usingApiKeyId(String id, boolean myApiKeysOnly) { + return new InvalidateApiKeyRequest(null, null, id, null, myApiKeysOnly); } /** * Creates invalidate api key request for given api key name + * * @param name api key name + * @param myApiKeysOnly 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 usingApiKeyName(String name) { - return new InvalidateApiKeyRequest(null, null, null, name); + public static InvalidateApiKeyRequest usingApiKeyName(String name, boolean myApiKeysOnly) { + return new InvalidateApiKeyRequest(null, null, null, name, myApiKeysOnly); } @Override public ActionRequestValidationException validate() { ActionRequestValidationException validationException = null; if (Strings.hasText(realmName) == false && Strings.hasText(userName) == false && Strings.hasText(id) == false - && Strings.hasText(name) == false) { + && Strings.hasText(name) == false) { validationException = addValidationError("One of [api key id, api key name, username, realm name] must be specified", - validationException); + validationException); } if (Strings.hasText(id) || Strings.hasText(name)) { if (Strings.hasText(realmName) || Strings.hasText(userName)) { validationException = addValidationError( - "username or realm name must not be specified when the api key id or api key name is specified", - validationException); + "username or realm name must not be specified when the api key id or api key name is specified", + validationException); + } + } + if (myApiKeysOnly) { + if (Strings.hasText(realmName) || Strings.hasText(userName)) { + validationException = addValidationError( + "username or realm name must not be specified when invalidating owned API keys", + validationException); } } if (Strings.hasText(id) && Strings.hasText(name)) { @@ -137,5 +158,6 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(userName); out.writeOptionalString(id); out.writeOptionalString(name); + out.writeOptionalBoolean(myApiKeysOnly); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java index 27be0d88eb82c..ca86dbfa1afd2 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java @@ -22,10 +22,10 @@ public class GetApiKeyRequestTests extends ESTestCase { public void testRequestValidation() { - GetApiKeyRequest request = GetApiKeyRequest.usingApiKeyId(randomAlphaOfLength(5)); + GetApiKeyRequest request = GetApiKeyRequest.usingApiKeyId(randomAlphaOfLength(5), randomBoolean()); ActionRequestValidationException ve = request.validate(); assertNull(ve); - request = GetApiKeyRequest.usingApiKeyName(randomAlphaOfLength(5)); + request = GetApiKeyRequest.usingApiKeyName(randomAlphaOfLength(5), randomBoolean()); ve = request.validate(); assertNull(ve); request = GetApiKeyRequest.usingRealmName(randomAlphaOfLength(5)); @@ -45,12 +45,14 @@ class Dummy extends ActionRequest { String user; String apiKeyId; String apiKeyName; + boolean ownedApiKeysOnly; Dummy(String[] a) { realm = a[0]; user = a[1]; apiKeyId = a[2]; apiKeyName = a[3]; + ownedApiKeysOnly = Boolean.parseBoolean(a[4]); } @Override @@ -65,23 +67,31 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(user); out.writeOptionalString(apiKeyId); out.writeOptionalString(apiKeyName); + out.writeOptionalBoolean(ownedApiKeysOnly); } } - String[][] inputs = new String[][] { - { randomFrom(new String[] { null, "" }), randomFrom(new String[] { null, "" }), randomFrom(new String[] { null, "" }), - randomFrom(new String[] { null, "" }) }, - { randomFrom(new String[] { null, "" }), "user", "api-kid", "api-kname" }, - { "realm", randomFrom(new String[] { null, "" }), "api-kid", "api-kname" }, - { "realm", "user", "api-kid", randomFrom(new String[] { null, "" }) }, - { randomFrom(new String[] { null, "" }), randomFrom(new String[] { null, "" }), "api-kid", "api-kname" } }; - String[][] expectedErrorMessages = new String[][] { { "One of [api key id, api key name, username, realm name] must be 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" }, - { "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" }, - { "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" } }; + String[][] inputs = new String[][]{ + {randomNullOrEmptyString(), randomNullOrEmptyString(), randomNullOrEmptyString(), + randomNullOrEmptyString(), "false"}, + {randomNullOrEmptyString(), "user", "api-kid", "api-kname", "false"}, + {"realm", randomNullOrEmptyString(), "api-kid", "api-kname", "false"}, + {"realm", "user", "api-kid", randomNullOrEmptyString(), "false"}, + {randomNullOrEmptyString(), randomNullOrEmptyString(), "api-kid", "api-kname", "false"}, + {"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"}, + {"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"}, + {"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"}, + {"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"}, + {"username or realm name must not be specified when retrieving owned API keys"}, + {"username or realm name must not be specified when retrieving owned API keys"} + }; for (int caseNo = 0; caseNo < inputs.length; caseNo++) { try (ByteArrayOutputStream bos = new ByteArrayOutputStream(); @@ -100,4 +110,8 @@ public void writeTo(StreamOutput out) throws IOException { } } } + + private static String randomNullOrEmptyString() { + return randomFrom(new String[]{"", null}); + } } 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 3d7fd90234286..3eba8dc4fd2a8 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 @@ -22,10 +22,10 @@ public class InvalidateApiKeyRequestTests extends ESTestCase { public void testRequestValidation() { - InvalidateApiKeyRequest request = InvalidateApiKeyRequest.usingApiKeyId(randomAlphaOfLength(5)); + InvalidateApiKeyRequest request = InvalidateApiKeyRequest.usingApiKeyId(randomAlphaOfLength(5), randomBoolean()); ActionRequestValidationException ve = request.validate(); assertNull(ve); - request = InvalidateApiKeyRequest.usingApiKeyName(randomAlphaOfLength(5)); + request = InvalidateApiKeyRequest.usingApiKeyName(randomAlphaOfLength(5), randomBoolean()); ve = request.validate(); assertNull(ve); request = InvalidateApiKeyRequest.usingRealmName(randomAlphaOfLength(5)); @@ -45,12 +45,14 @@ class Dummy extends ActionRequest { String user; String apiKeyId; String apiKeyName; + boolean ownedApiKeysOnly; Dummy(String[] a) { realm = a[0]; user = a[1]; apiKeyId = a[2]; apiKeyName = a[3]; + ownedApiKeysOnly = Boolean.parseBoolean(a[4]); } @Override @@ -65,24 +67,31 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(user); out.writeOptionalString(apiKeyId); out.writeOptionalString(apiKeyName); + out.writeOptionalBoolean(ownedApiKeysOnly); } } - String[][] inputs = new String[][] { - { randomFrom(new String[] { null, "" }), randomFrom(new String[] { null, "" }), randomFrom(new String[] { null, "" }), - randomFrom(new String[] { null, "" }) }, - { randomFrom(new String[] { null, "" }), "user", "api-kid", "api-kname" }, - { "realm", randomFrom(new String[] { null, "" }), "api-kid", "api-kname" }, - { "realm", "user", "api-kid", randomFrom(new String[] { null, "" }) }, - { randomFrom(new String[] { null, "" }), randomFrom(new String[] { null, "" }), "api-kid", "api-kname" } }; - String[][] expectedErrorMessages = new String[][] { { "One of [api key id, api key name, username, realm name] must be 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" }, - { "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" }, - { "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" } }; - + String[][] inputs = new String[][]{ + {randomNullOrEmptyString(), randomNullOrEmptyString(), randomNullOrEmptyString(), + randomNullOrEmptyString(), "false"}, + {randomNullOrEmptyString(), "user", "api-kid", "api-kname", "false"}, + {"realm", randomNullOrEmptyString(), "api-kid", "api-kname", "false"}, + {"realm", "user", "api-kid", randomNullOrEmptyString(), "false"}, + {randomNullOrEmptyString(), randomNullOrEmptyString(), "api-kid", "api-kname", "false"}, + {"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"}, + {"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"}, + {"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"}, + {"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"}, + {"username or realm name must not be specified when invalidating owned API keys"}, + {"username or realm name must not be specified when invalidating owned API keys"} + }; for (int caseNo = 0; caseNo < inputs.length; caseNo++) { try (ByteArrayOutputStream bos = new ByteArrayOutputStream(); @@ -101,4 +110,9 @@ public void writeTo(StreamOutput out) throws IOException { } } } + + private static String randomNullOrEmptyString() { + return randomFrom(new String[]{"", null}); + } + } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyAction.java index 71ed5a06efb65..72ebf884d9ca8 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyAction.java @@ -39,7 +39,8 @@ protected RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClien final String apiKeyName = request.param("name"); final String userName = request.param("username"); final String realmName = request.param("realm_name"); - final GetApiKeyRequest getApiKeyRequest = new GetApiKeyRequest(realmName, userName, apiKeyId, apiKeyName); + final boolean myApiKeysOnly = request.paramAsBoolean("my_api_keys_only", false); + final GetApiKeyRequest getApiKeyRequest = new GetApiKeyRequest(realmName, userName, apiKeyId, apiKeyName, myApiKeysOnly); return channel -> client.execute(GetApiKeyAction.INSTANCE, getApiKeyRequest, new RestBuilderListener(channel) { @Override 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 b11a0edde42f8..7d3ff77dcf6b3 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 @@ -31,7 +31,8 @@ public final class RestInvalidateApiKeyAction extends ApiKeyBaseRestHandler { static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("invalidate_api_key", a -> { - return new InvalidateApiKeyRequest((String) a[0], (String) a[1], (String) a[2], (String) a[3]); + return new InvalidateApiKeyRequest((String) a[0], (String) a[1], (String) a[2], (String) a[3], (a[4] == null) ? false : + (Boolean) a[4]); }); static { @@ -39,6 +40,7 @@ public final class RestInvalidateApiKeyAction extends ApiKeyBaseRestHandler { PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("username")); PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("id")); PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("name")); + PARSER.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), new ParseField("my_api_keys_only")); } public RestInvalidateApiKeyAction(Settings settings, RestController controller, XPackLicenseState licenseState) { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java index 5c6c04b6ad491..7309dfa2225d8 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java @@ -171,7 +171,7 @@ public void testCreateApiKeyFailsWhenApiKeyWithSameNameAlreadyExists() throws In // Now invalidate the API key PlainActionFuture listener = new PlainActionFuture<>(); - client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyName(keyName), listener); + client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyName(keyName, false), listener); InvalidateApiKeyResponse invalidateResponse = listener.get(); verifyInvalidateResponse(1, responses, invalidateResponse); @@ -222,7 +222,7 @@ public void testInvalidateApiKeysForApiKeyId() throws InterruptedException, Exec Client client = client().filterWithHeader(Collections.singletonMap("Authorization", UsernamePasswordToken .basicAuthHeaderValue(SecuritySettingsSource.TEST_SUPERUSER, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING))); PlainActionFuture listener = new PlainActionFuture<>(); - client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(responses.get(0).getId()), listener); + client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(responses.get(0).getId(), false), listener); InvalidateApiKeyResponse invalidateResponse = listener.get(); verifyInvalidateResponse(1, responses, invalidateResponse); } @@ -232,7 +232,8 @@ public void testInvalidateApiKeysForApiKeyName() throws InterruptedException, Ex Client client = client().filterWithHeader(Collections.singletonMap("Authorization", UsernamePasswordToken .basicAuthHeaderValue(SecuritySettingsSource.TEST_SUPERUSER, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING))); PlainActionFuture listener = new PlainActionFuture<>(); - client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyName(responses.get(0).getName()), listener); + client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyName(responses.get(0).getName(), false), + listener); InvalidateApiKeyResponse invalidateResponse = listener.get(); verifyInvalidateResponse(1, responses, invalidateResponse); } @@ -254,7 +255,8 @@ public void testInvalidatedApiKeysDeletedByRemover() throws Exception { List createdApiKeys = createApiKeys(2, null); PlainActionFuture listener = new PlainActionFuture<>(); - client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(createdApiKeys.get(0).getId()), listener); + client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(createdApiKeys.get(0).getId(), false), + listener); InvalidateApiKeyResponse invalidateResponse = listener.get(); assertThat(invalidateResponse.getInvalidatedApiKeys().size(), equalTo(1)); assertThat(invalidateResponse.getPreviouslyInvalidatedApiKeys().size(), equalTo(0)); @@ -270,7 +272,8 @@ public void testInvalidatedApiKeysDeletedByRemover() throws Exception { // invalidate API key to trigger remover listener = new PlainActionFuture<>(); - client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(createdApiKeys.get(1).getId()), listener); + client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(createdApiKeys.get(1).getId(), false), + listener); assertThat(listener.get().getInvalidatedApiKeys().size(), is(1)); awaitApiKeysRemoverCompletion(); @@ -343,7 +346,8 @@ public void testExpiredApiKeysBehaviorWhenKeysExpired1WeekBeforeAnd1DayBefore() // Invalidate to trigger the remover PlainActionFuture listener = new PlainActionFuture<>(); - client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(createdApiKeys.get(2).getId()), listener); + client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(createdApiKeys.get(2).getId(), false), + listener); assertThat(listener.get().getInvalidatedApiKeys().size(), is(1)); awaitApiKeysRemoverCompletion(); @@ -391,7 +395,7 @@ public void testActiveApiKeysWithNoExpirationNeverGetDeletedByRemover() throws E .basicAuthHeaderValue(SecuritySettingsSource.TEST_SUPERUSER, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING))); PlainActionFuture listener = new PlainActionFuture<>(); // trigger expired keys remover - client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(responses.get(1).getId()), listener); + client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(responses.get(1).getId(), false), listener); InvalidateApiKeyResponse invalidateResponse = listener.get(); assertThat(invalidateResponse.getInvalidatedApiKeys().size(), equalTo(1)); assertThat(invalidateResponse.getPreviouslyInvalidatedApiKeys().size(), equalTo(0)); @@ -414,7 +418,8 @@ public void testGetApiKeysForRealm() throws InterruptedException, ExecutionExcep Set expectedValidKeyIds = null; if (invalidate) { PlainActionFuture listener = new PlainActionFuture<>(); - client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(responses.get(0).getId()), listener); + client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(responses.get(0).getId(), false), + listener); InvalidateApiKeyResponse invalidateResponse = listener.get(); invalidatedApiKeyIds = invalidateResponse.getInvalidatedApiKeys(); expectedValidKeyIds = responses.stream().filter(o -> !o.getId().equals(responses.get(0).getId())).map(o -> o.getId()) @@ -459,7 +464,7 @@ public void testGetApiKeysForApiKeyId() throws InterruptedException, ExecutionEx Client client = client().filterWithHeader(Collections.singletonMap("Authorization", UsernamePasswordToken .basicAuthHeaderValue(SecuritySettingsSource.TEST_SUPERUSER, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING))); PlainActionFuture listener = new PlainActionFuture<>(); - client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingApiKeyId(responses.get(0).getId()), listener); + client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingApiKeyId(responses.get(0).getId(), false), listener); GetApiKeyResponse response = listener.get(); verifyGetResponse(1, responses, response, Collections.singleton(responses.get(0).getId()), null); } @@ -469,7 +474,7 @@ public void testGetApiKeysForApiKeyName() throws InterruptedException, Execution Client client = client().filterWithHeader(Collections.singletonMap("Authorization", UsernamePasswordToken .basicAuthHeaderValue(SecuritySettingsSource.TEST_SUPERUSER, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING))); PlainActionFuture listener = new PlainActionFuture<>(); - client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingApiKeyName(responses.get(0).getName()), listener); + client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingApiKeyName(responses.get(0).getName(), false), listener); GetApiKeyResponse response = listener.get(); verifyGetResponse(1, responses, response, Collections.singleton(responses.get(0).getId()), null); } From d8664040f09aad6a5ed79910de37a1db0acbfd08 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Tue, 30 Jul 2019 12:42:36 +1000 Subject: [PATCH 02/10] address review comment, add versioning to newly added flag --- .../security/action/GetApiKeyRequest.java | 32 +++++++++++++++-- .../action/InvalidateApiKeyRequest.java | 33 +++++++++++++++-- .../action/GetApiKeyRequestTests.java | 35 +++++++++++++++++++ .../action/InvalidateApiKeyRequestTests.java | 35 +++++++++++++++++++ 4 files changed, 131 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java index ee04c593ccc51..d547c264ce082 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.core.security.action; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.common.Nullable; @@ -14,6 +15,7 @@ import org.elasticsearch.common.io.stream.StreamOutput; import java.io.IOException; +import java.util.Objects; import static org.elasticsearch.action.ValidateActions.addValidationError; @@ -38,7 +40,11 @@ public GetApiKeyRequest(StreamInput in) throws IOException { userName = in.readOptionalString(); apiKeyId = in.readOptionalString(); apiKeyName = in.readOptionalString(); - myApiKeysOnly = in.readOptionalBoolean(); + if (in.getVersion().onOrAfter(Version.V_7_4_0)) { + myApiKeysOnly = in.readOptionalBoolean(); + } else { + myApiKeysOnly = false; + } } public GetApiKeyRequest(@Nullable String realmName, @Nullable String userName, @Nullable String apiKeyId, @@ -153,7 +159,29 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(userName); out.writeOptionalString(apiKeyId); out.writeOptionalString(apiKeyName); - out.writeOptionalBoolean(myApiKeysOnly); + if (out.getVersion().onOrAfter(Version.V_7_4_0)) { + out.writeOptionalBoolean(myApiKeysOnly); + } + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + GetApiKeyRequest that = (GetApiKeyRequest) o; + return myApiKeysOnly == that.myApiKeysOnly && + Objects.equals(realmName, that.realmName) && + Objects.equals(userName, that.userName) && + Objects.equals(apiKeyId, that.apiKeyId) && + Objects.equals(apiKeyName, that.apiKeyName); } + @Override + public int hashCode() { + return Objects.hash(realmName, userName, apiKeyId, apiKeyName, myApiKeysOnly); } +} 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 74158cb304233..83037db3444f7 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 @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.core.security.action; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.common.Nullable; @@ -14,6 +15,7 @@ import org.elasticsearch.common.io.stream.StreamOutput; import java.io.IOException; +import java.util.Objects; import static org.elasticsearch.action.ValidateActions.addValidationError; @@ -38,7 +40,11 @@ public InvalidateApiKeyRequest(StreamInput in) throws IOException { userName = in.readOptionalString(); id = in.readOptionalString(); name = in.readOptionalString(); - myApiKeysOnly = in.readOptionalBoolean(); + if (in.getVersion().onOrAfter(Version.V_7_4_0)) { + myApiKeysOnly = in.readOptionalBoolean(); + } else { + myApiKeysOnly = false; + } } public InvalidateApiKeyRequest(@Nullable String realmName, @Nullable String userName, @Nullable String id, @@ -158,6 +164,29 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(userName); out.writeOptionalString(id); out.writeOptionalString(name); - out.writeOptionalBoolean(myApiKeysOnly); + if (out.getVersion().onOrAfter(Version.V_7_4_0)) { + out.writeOptionalBoolean(myApiKeysOnly); + } + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + InvalidateApiKeyRequest that = (InvalidateApiKeyRequest) o; + return myApiKeysOnly == that.myApiKeysOnly && + Objects.equals(realmName, that.realmName) && + Objects.equals(userName, that.userName) && + Objects.equals(id, that.id) && + Objects.equals(name, that.name); + } + + @Override + public int hashCode() { + return Objects.hash(realmName, userName, id, name, myApiKeysOnly); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java index ca86dbfa1afd2..931f00a2a6844 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.core.security.action; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.common.io.stream.InputStreamStreamInput; @@ -18,6 +19,8 @@ import java.io.IOException; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; public class GetApiKeyRequestTests extends ESTestCase { @@ -111,6 +114,38 @@ public void writeTo(StreamOutput out) throws IOException { } } + public void testSerialization() throws IOException { + final String apiKeyId = randomAlphaOfLength(5); + final boolean myApiKeysOnly = true; + GetApiKeyRequest getApiKeyRequest = GetApiKeyRequest.usingApiKeyId(apiKeyId, myApiKeysOnly); + { + ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); + OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); + out.setVersion(Version.V_7_2_0); + getApiKeyRequest.writeTo(out); + + InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray())); + inputStreamStreamInput.setVersion(randomFrom(Version.V_7_2_0, Version.V_7_3_0)); + GetApiKeyRequest requestFromInputStream = new GetApiKeyRequest(inputStreamStreamInput); + + assertThat(requestFromInputStream.getApiKeyId(), equalTo(getApiKeyRequest.getApiKeyId())); + // old version so the default for `myApiKeysOnly` is false + assertThat(requestFromInputStream.myApiKeysOnly(), is(false)); + } + { + ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); + OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); + out.setVersion(randomFrom(Version.V_7_4_0, Version.CURRENT)); + getApiKeyRequest.writeTo(out); + + InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray())); + inputStreamStreamInput.setVersion(randomFrom(Version.V_7_4_0, Version.CURRENT)); + GetApiKeyRequest requestFromInputStream = new GetApiKeyRequest(inputStreamStreamInput); + + assertThat(requestFromInputStream, equalTo(getApiKeyRequest)); + } + } + private static String randomNullOrEmptyString() { return randomFrom(new String[]{"", null}); } 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 3eba8dc4fd2a8..1f2586512c3d5 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 @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.core.security.action; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.common.io.stream.InputStreamStreamInput; @@ -18,6 +19,8 @@ import java.io.IOException; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; public class InvalidateApiKeyRequestTests extends ESTestCase { @@ -111,6 +114,38 @@ public void writeTo(StreamOutput out) throws IOException { } } + public void testSerialization() throws IOException { + final String apiKeyId = randomAlphaOfLength(5); + final boolean myApiKeysOnly = true; + InvalidateApiKeyRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, myApiKeysOnly); + { + ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); + OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); + out.setVersion(Version.V_7_2_0); + invalidateApiKeyRequest.writeTo(out); + + InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray())); + inputStreamStreamInput.setVersion(randomFrom(Version.V_7_2_0, Version.V_7_3_0)); + InvalidateApiKeyRequest requestFromInputStream = new InvalidateApiKeyRequest(inputStreamStreamInput); + + assertThat(requestFromInputStream.getId(), equalTo(invalidateApiKeyRequest.getId())); + // old version so the default for `myApiKeysOnly` is false + assertThat(requestFromInputStream.myApiKeysOnly(), is(false)); + } + { + ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); + OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); + out.setVersion(randomFrom(Version.V_7_4_0, Version.CURRENT)); + invalidateApiKeyRequest.writeTo(out); + + InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray())); + inputStreamStreamInput.setVersion(randomFrom(Version.V_7_4_0, Version.CURRENT)); + InvalidateApiKeyRequest requestFromInputStream = new InvalidateApiKeyRequest(inputStreamStreamInput); + + assertThat(requestFromInputStream, equalTo(invalidateApiKeyRequest)); + } + } + private static String randomNullOrEmptyString() { return randomFrom(new String[]{"", null}); } From 88ec2efae2d2bc55c842951e87953e12c9e8cb37 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Thu, 1 Aug 2019 19:07:29 +1000 Subject: [PATCH 03/10] address review comments --- .../security/action/GetApiKeyRequest.java | 42 +++++------ .../action/InvalidateApiKeyRequest.java | 42 +++++------ .../action/GetApiKeyRequestTests.java | 16 ++--- .../action/InvalidateApiKeyRequestTests.java | 16 ++--- .../action/apikey/RestGetApiKeyAction.java | 2 +- .../apikey/RestInvalidateApiKeyAction.java | 2 +- .../apikey/RestGetApiKeyActionTests.java | 71 ++++++++++++++++++- .../RestInvalidateApiKeyActionTests.java | 71 ++++++++++++++++++- 8 files changed, 202 insertions(+), 60 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java index d547c264ce082..73ebdcf460e9d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java @@ -28,7 +28,7 @@ public final class GetApiKeyRequest extends ActionRequest { private final String userName; private final String apiKeyId; private final String apiKeyName; - private final boolean myApiKeysOnly; + private final boolean ownedByAuthenticatedUser; public GetApiKeyRequest() { this(null, null, null, null, false); @@ -41,19 +41,19 @@ public GetApiKeyRequest(StreamInput in) throws IOException { apiKeyId = in.readOptionalString(); apiKeyName = in.readOptionalString(); if (in.getVersion().onOrAfter(Version.V_7_4_0)) { - myApiKeysOnly = in.readOptionalBoolean(); + ownedByAuthenticatedUser = in.readOptionalBoolean(); } else { - myApiKeysOnly = false; + ownedByAuthenticatedUser = false; } } public GetApiKeyRequest(@Nullable String realmName, @Nullable String userName, @Nullable String apiKeyId, - @Nullable String apiKeyName, boolean myApiKeysOnly) { + @Nullable String apiKeyName, boolean ownedByAuthenticatedUser) { this.realmName = realmName; this.userName = userName; this.apiKeyId = apiKeyId; this.apiKeyName = apiKeyName; - this.myApiKeysOnly = myApiKeysOnly; + this.ownedByAuthenticatedUser = ownedByAuthenticatedUser; } public String getRealmName() { @@ -72,8 +72,8 @@ public String getApiKeyName() { return apiKeyName; } - public boolean myApiKeysOnly() { - return myApiKeysOnly; + public boolean ownedByAuthenticatedUser() { + return ownedByAuthenticatedUser; } /** @@ -107,30 +107,32 @@ public static GetApiKeyRequest usingRealmAndUserName(String realmName, String us /** * Creates get API key request for given api key id * @param apiKeyId api key id - * @param myApiKeysOnly set {@code true} if the request is only for the API keys owned by current authenticated user else {@code false} + * @param ownedByAuthenticatedUser set {@code true} if the request is only for the API keys owned by current authenticated user else + * {@code false} * @return {@link GetApiKeyRequest} */ - public static GetApiKeyRequest usingApiKeyId(String apiKeyId, boolean myApiKeysOnly) { - return new GetApiKeyRequest(null, null, apiKeyId, null, myApiKeysOnly); + public static GetApiKeyRequest usingApiKeyId(String apiKeyId, boolean ownedByAuthenticatedUser) { + return new GetApiKeyRequest(null, null, apiKeyId, null, ownedByAuthenticatedUser); } /** * Creates get api key request for given api key name * @param apiKeyName api key name - * @param myApiKeysOnly set {@code true} if the request is only for the API keys owned by current authenticated user else {@code false} + * @param ownedByAuthenticatedUser set {@code true} if the request is only for the API keys owned by current authenticated user else + * {@code false} * @return {@link GetApiKeyRequest} */ - public static GetApiKeyRequest usingApiKeyName(String apiKeyName, boolean myApiKeysOnly) { - return new GetApiKeyRequest(null, null, null, apiKeyName, myApiKeysOnly); + public static GetApiKeyRequest usingApiKeyName(String apiKeyName, boolean ownedByAuthenticatedUser) { + return new GetApiKeyRequest(null, null, null, apiKeyName, ownedByAuthenticatedUser); } @Override public ActionRequestValidationException validate() { ActionRequestValidationException validationException = null; if (Strings.hasText(realmName) == false && Strings.hasText(userName) == false && Strings.hasText(apiKeyId) == false - && Strings.hasText(apiKeyName) == false) { - validationException = addValidationError("One of [api key id, api key name, username, realm name] must be specified", - validationException); + && Strings.hasText(apiKeyName) == false && ownedByAuthenticatedUser == false) { + validationException = addValidationError("One of [api key id, api key name, username, realm name] must be specified if " + + "[owner] flag is false", validationException); } if (Strings.hasText(apiKeyId) || Strings.hasText(apiKeyName)) { if (Strings.hasText(realmName) || Strings.hasText(userName)) { @@ -139,7 +141,7 @@ public ActionRequestValidationException validate() { validationException); } } - if (myApiKeysOnly) { + if (ownedByAuthenticatedUser) { if (Strings.hasText(realmName) || Strings.hasText(userName)) { validationException = addValidationError( "username or realm name must not be specified when retrieving owned API keys", @@ -160,7 +162,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(apiKeyId); out.writeOptionalString(apiKeyName); if (out.getVersion().onOrAfter(Version.V_7_4_0)) { - out.writeOptionalBoolean(myApiKeysOnly); + out.writeOptionalBoolean(ownedByAuthenticatedUser); } } @@ -173,7 +175,7 @@ public boolean equals(Object o) { return false; } GetApiKeyRequest that = (GetApiKeyRequest) o; - return myApiKeysOnly == that.myApiKeysOnly && + return ownedByAuthenticatedUser == that.ownedByAuthenticatedUser && Objects.equals(realmName, that.realmName) && Objects.equals(userName, that.userName) && Objects.equals(apiKeyId, that.apiKeyId) && @@ -182,6 +184,6 @@ public boolean equals(Object o) { @Override public int hashCode() { - return Objects.hash(realmName, userName, apiKeyId, apiKeyName, myApiKeysOnly); + return Objects.hash(realmName, userName, apiKeyId, apiKeyName, ownedByAuthenticatedUser); } } 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 83037db3444f7..0659522e82aa9 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 @@ -28,7 +28,7 @@ public final class InvalidateApiKeyRequest extends ActionRequest { private final String userName; private final String id; private final String name; - private final boolean myApiKeysOnly; + private final boolean ownedByAuthenticatedUser; public InvalidateApiKeyRequest() { this(null, null, null, null, false); @@ -41,19 +41,19 @@ public InvalidateApiKeyRequest(StreamInput in) throws IOException { id = in.readOptionalString(); name = in.readOptionalString(); if (in.getVersion().onOrAfter(Version.V_7_4_0)) { - myApiKeysOnly = in.readOptionalBoolean(); + ownedByAuthenticatedUser = in.readOptionalBoolean(); } else { - myApiKeysOnly = false; + ownedByAuthenticatedUser = false; } } public InvalidateApiKeyRequest(@Nullable String realmName, @Nullable String userName, @Nullable String id, - @Nullable String name, boolean myApiKeysOnly) { + @Nullable String name, boolean ownedByAuthenticatedUser) { this.realmName = realmName; this.userName = userName; this.id = id; this.name = name; - this.myApiKeysOnly = myApiKeysOnly; + this.ownedByAuthenticatedUser = ownedByAuthenticatedUser; } public String getRealmName() { @@ -72,8 +72,8 @@ public String getName() { return name; } - public boolean myApiKeysOnly() { - return myApiKeysOnly; + public boolean ownedByAuthenticatedUser() { + return ownedByAuthenticatedUser; } /** @@ -111,31 +111,33 @@ public static InvalidateApiKeyRequest usingRealmAndUserName(String realmName, St * Creates invalidate API key request for given api key id * * @param id api key id - * @param myApiKeysOnly set {@code true} if the request is only for the API keys owned by current authenticated user else {@code false} + * @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 usingApiKeyId(String id, boolean myApiKeysOnly) { - return new InvalidateApiKeyRequest(null, null, id, null, myApiKeysOnly); + public static InvalidateApiKeyRequest usingApiKeyId(String id, boolean ownedByAuthenticatedUser) { + return new InvalidateApiKeyRequest(null, null, id, null, ownedByAuthenticatedUser); } /** * Creates invalidate api key request for given api key name * * @param name api key name - * @param myApiKeysOnly set {@code true} if the request is only for the API keys owned by current authenticated user else {@code false} + * @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 usingApiKeyName(String name, boolean myApiKeysOnly) { - return new InvalidateApiKeyRequest(null, null, null, name, myApiKeysOnly); + public static InvalidateApiKeyRequest usingApiKeyName(String name, boolean ownedByAuthenticatedUser) { + return new InvalidateApiKeyRequest(null, null, null, name, ownedByAuthenticatedUser); } @Override public ActionRequestValidationException validate() { ActionRequestValidationException validationException = null; if (Strings.hasText(realmName) == false && Strings.hasText(userName) == false && Strings.hasText(id) == false - && Strings.hasText(name) == false) { - validationException = addValidationError("One of [api key id, api key name, username, realm name] must be specified", - validationException); + && Strings.hasText(name) == false && ownedByAuthenticatedUser == false) { + validationException = addValidationError("One of [api key id, api key name, username, realm name] must be specified if " + + "[owner] flag is false", validationException); } if (Strings.hasText(id) || Strings.hasText(name)) { if (Strings.hasText(realmName) || Strings.hasText(userName)) { @@ -144,7 +146,7 @@ public ActionRequestValidationException validate() { validationException); } } - if (myApiKeysOnly) { + if (ownedByAuthenticatedUser) { if (Strings.hasText(realmName) || Strings.hasText(userName)) { validationException = addValidationError( "username or realm name must not be specified when invalidating owned API keys", @@ -165,7 +167,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(id); out.writeOptionalString(name); if (out.getVersion().onOrAfter(Version.V_7_4_0)) { - out.writeOptionalBoolean(myApiKeysOnly); + out.writeOptionalBoolean(ownedByAuthenticatedUser); } } @@ -178,7 +180,7 @@ public boolean equals(Object o) { return false; } InvalidateApiKeyRequest that = (InvalidateApiKeyRequest) o; - return myApiKeysOnly == that.myApiKeysOnly && + return ownedByAuthenticatedUser == that.ownedByAuthenticatedUser && Objects.equals(realmName, that.realmName) && Objects.equals(userName, that.userName) && Objects.equals(id, that.id) && @@ -187,6 +189,6 @@ public boolean equals(Object o) { @Override public int hashCode() { - return Objects.hash(realmName, userName, id, name, myApiKeysOnly); + return Objects.hash(realmName, userName, id, name, ownedByAuthenticatedUser); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java index 931f00a2a6844..985445ff0d0f3 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java @@ -48,14 +48,14 @@ class Dummy extends ActionRequest { String user; String apiKeyId; String apiKeyName; - boolean ownedApiKeysOnly; + boolean ownedByAuthenticatedUser; Dummy(String[] a) { realm = a[0]; user = a[1]; apiKeyId = a[2]; apiKeyName = a[3]; - ownedApiKeysOnly = Boolean.parseBoolean(a[4]); + ownedByAuthenticatedUser = Boolean.parseBoolean(a[4]); } @Override @@ -70,7 +70,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(user); out.writeOptionalString(apiKeyId); out.writeOptionalString(apiKeyName); - out.writeOptionalBoolean(ownedApiKeysOnly); + out.writeOptionalBoolean(ownedByAuthenticatedUser); } } @@ -85,7 +85,7 @@ public void writeTo(StreamOutput out) throws IOException { {randomNullOrEmptyString(), "user", randomNullOrEmptyString(), randomNullOrEmptyString(), "true"} }; String[][] expectedErrorMessages = new String[][]{ - {"One of [api key id, api key name, username, realm name] must be specified"}, + {"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", "only one of [api key id, api key name] can be specified"}, {"username or realm name must not be specified when the api key id or api key name is specified", @@ -116,8 +116,8 @@ public void writeTo(StreamOutput out) throws IOException { public void testSerialization() throws IOException { final String apiKeyId = randomAlphaOfLength(5); - final boolean myApiKeysOnly = true; - GetApiKeyRequest getApiKeyRequest = GetApiKeyRequest.usingApiKeyId(apiKeyId, myApiKeysOnly); + final boolean ownedByAuthenticatedUser = true; + GetApiKeyRequest getApiKeyRequest = GetApiKeyRequest.usingApiKeyId(apiKeyId, ownedByAuthenticatedUser); { ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); @@ -129,8 +129,8 @@ public void testSerialization() throws IOException { GetApiKeyRequest requestFromInputStream = new GetApiKeyRequest(inputStreamStreamInput); assertThat(requestFromInputStream.getApiKeyId(), equalTo(getApiKeyRequest.getApiKeyId())); - // old version so the default for `myApiKeysOnly` is false - assertThat(requestFromInputStream.myApiKeysOnly(), is(false)); + // old version so the default for `ownedByAuthenticatedUser` is false + assertThat(requestFromInputStream.ownedByAuthenticatedUser(), is(false)); } { ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); 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 1f2586512c3d5..cac1ab51918d7 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 @@ -48,14 +48,14 @@ class Dummy extends ActionRequest { String user; String apiKeyId; String apiKeyName; - boolean ownedApiKeysOnly; + boolean ownedByAuthenticatedUser; Dummy(String[] a) { realm = a[0]; user = a[1]; apiKeyId = a[2]; apiKeyName = a[3]; - ownedApiKeysOnly = Boolean.parseBoolean(a[4]); + ownedByAuthenticatedUser = Boolean.parseBoolean(a[4]); } @Override @@ -70,7 +70,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(user); out.writeOptionalString(apiKeyId); out.writeOptionalString(apiKeyName); - out.writeOptionalBoolean(ownedApiKeysOnly); + out.writeOptionalBoolean(ownedByAuthenticatedUser); } } @@ -85,7 +85,7 @@ public void writeTo(StreamOutput out) throws IOException { {randomNullOrEmptyString(), "user", randomNullOrEmptyString(), randomNullOrEmptyString(), "true"}, }; String[][] expectedErrorMessages = new String[][]{ - {"One of [api key id, api key name, username, realm name] must be specified"}, + {"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", "only one of [api key id, api key name] can be specified"}, {"username or realm name must not be specified when the api key id or api key name is specified", @@ -116,8 +116,8 @@ public void writeTo(StreamOutput out) throws IOException { public void testSerialization() throws IOException { final String apiKeyId = randomAlphaOfLength(5); - final boolean myApiKeysOnly = true; - InvalidateApiKeyRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, myApiKeysOnly); + final boolean ownedByAuthenticatedUser = true; + InvalidateApiKeyRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, ownedByAuthenticatedUser); { ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); @@ -129,8 +129,8 @@ public void testSerialization() throws IOException { InvalidateApiKeyRequest requestFromInputStream = new InvalidateApiKeyRequest(inputStreamStreamInput); assertThat(requestFromInputStream.getId(), equalTo(invalidateApiKeyRequest.getId())); - // old version so the default for `myApiKeysOnly` is false - assertThat(requestFromInputStream.myApiKeysOnly(), is(false)); + // old version so the default for `ownedByAuthenticatedUser` is false + assertThat(requestFromInputStream.ownedByAuthenticatedUser(), is(false)); } { ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyAction.java index 72ebf884d9ca8..ca07952478444 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyAction.java @@ -39,7 +39,7 @@ protected RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClien final String apiKeyName = request.param("name"); final String userName = request.param("username"); final String realmName = request.param("realm_name"); - final boolean myApiKeysOnly = request.paramAsBoolean("my_api_keys_only", false); + final boolean myApiKeysOnly = request.paramAsBoolean("owner", false); final GetApiKeyRequest getApiKeyRequest = new GetApiKeyRequest(realmName, userName, apiKeyId, apiKeyName, myApiKeysOnly); return channel -> client.execute(GetApiKeyAction.INSTANCE, getApiKeyRequest, new RestBuilderListener(channel) { 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 7d3ff77dcf6b3..0579932887677 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 @@ -40,7 +40,7 @@ public final class RestInvalidateApiKeyAction extends ApiKeyBaseRestHandler { PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("username")); PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("id")); PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("name")); - PARSER.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), new ParseField("my_api_keys_only")); + PARSER.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), new ParseField("owner")); } public RestInvalidateApiKeyAction(Settings settings, RestController controller, XPackLicenseState licenseState) { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyActionTests.java index c706a251dda35..ea1471b4e35d8 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyActionTests.java @@ -8,11 +8,11 @@ import org.apache.lucene.util.SetOnce; import org.elasticsearch.ElasticsearchSecurityException; -import org.elasticsearch.action.ActionType; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.action.ActionType; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.settings.Settings; @@ -36,6 +36,7 @@ import java.time.temporal.ChronoUnit; import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Map; import static org.hamcrest.Matchers.arrayContaining; @@ -133,6 +134,74 @@ void doExecute(ActionType action, Request request, ActionListener param = mapBuilder().put("owner", Boolean.toString(owner)).map(); + if (owner == false) { + param = mapBuilder().put("owner", Boolean.toString(owner)).put("realm_name", "realm-1").map(); + } + + final FakeRestRequest restRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) + .withParams(param).build(); + + final SetOnce responseSetOnce = new SetOnce<>(); + final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + @Override + public void sendResponse(RestResponse restResponse) { + responseSetOnce.set(restResponse); + } + }; + + final Instant creation = Instant.now(); + final Instant expiration = randomFrom(Arrays.asList(null, Instant.now().plus(10, ChronoUnit.DAYS))); + final ApiKey apiKey1 = new ApiKey("api-key-name-1", "api-key-id-1", creation, expiration, false, + "user-x", "realm-1"); + final ApiKey apiKey2 = new ApiKey("api-key-name-2", "api-key-id-2", creation, expiration, false, + "user-y", "realm-1"); + final GetApiKeyResponse getApiKeyResponseExpectedWhenOwnerFlagIsTrue = new GetApiKeyResponse(Collections.singletonList(apiKey1)); + final GetApiKeyResponse getApiKeyResponseExpectedWhenOwnerFlagIsFalse = new GetApiKeyResponse(List.of(apiKey1, apiKey2)); + + try (NodeClient client = new NodeClient(Settings.EMPTY, threadPool) { + @SuppressWarnings("unchecked") + @Override + public + void doExecute(ActionType action, Request request, ActionListener listener) { + GetApiKeyRequest getApiKeyRequest = (GetApiKeyRequest) request; + ActionRequestValidationException validationException = getApiKeyRequest.validate(); + if (validationException != null) { + listener.onFailure(validationException); + return; + } + + if (getApiKeyRequest.ownedByAuthenticatedUser()) { + listener.onResponse((Response) getApiKeyResponseExpectedWhenOwnerFlagIsTrue); + } else if (getApiKeyRequest.getRealmName() != null && getApiKeyRequest.getRealmName().equals("realm-1")) { + listener.onResponse((Response) getApiKeyResponseExpectedWhenOwnerFlagIsFalse); + } + } + }) { + final RestGetApiKeyAction restGetApiKeyAction = new RestGetApiKeyAction(Settings.EMPTY, mockRestController, mockLicenseState); + + restGetApiKeyAction.handleRequest(restRequest, restChannel, client); + + final RestResponse restResponse = responseSetOnce.get(); + assertNotNull(restResponse); + assertThat(restResponse.status(), is(RestStatus.OK)); + final GetApiKeyResponse actual = GetApiKeyResponse + .fromXContent(createParser(XContentType.JSON.xContent(), restResponse.content())); + if (owner) { + assertThat(actual.getApiKeyInfos().length, is(1)); + assertThat(actual.getApiKeyInfos(), + arrayContaining(apiKey1)); + } else { + assertThat(actual.getApiKeyInfos().length, is(2)); + assertThat(actual.getApiKeyInfos(), + arrayContaining(apiKey1, apiKey2)); + } + } + + } + private static MapBuilder mapBuilder() { return MapBuilder.newMapBuilder(); } 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 21e65c485fb2b..bfeccf227c689 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 @@ -8,11 +8,11 @@ import org.apache.lucene.util.SetOnce; import org.elasticsearch.ElasticsearchSecurityException; -import org.elasticsearch.action.ActionType; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.action.ActionType; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.settings.Settings; @@ -24,6 +24,7 @@ import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestResponse; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.threadpool.ThreadPool; @@ -31,8 +32,11 @@ import org.elasticsearch.xpack.core.security.action.InvalidateApiKeyResponse; import java.util.Collections; +import java.util.List; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -119,4 +123,69 @@ void doExecute(ActionType action, Request request, ActionListener responseSetOnce = new SetOnce<>(); + final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + @Override + public void sendResponse(RestResponse restResponse) { + responseSetOnce.set(restResponse); + } + }; + + final InvalidateApiKeyResponse invalidateApiKeyResponseExpectedWhenOwnerFlagIsTrue = new InvalidateApiKeyResponse( + List.of("api-key-id-1"), Collections.emptyList(), null); + final InvalidateApiKeyResponse invalidateApiKeyResponseExpectedWhenOwnerFlagIsFalse = new InvalidateApiKeyResponse( + List.of("api-key-id-1", "api-key-id-2"), Collections.emptyList(), null); + + try (NodeClient client = new NodeClient(Settings.EMPTY, threadPool) { + @SuppressWarnings("unchecked") + @Override + public + void doExecute(ActionType action, Request request, ActionListener listener) { + InvalidateApiKeyRequest invalidateApiKeyRequest = (InvalidateApiKeyRequest) request; + ActionRequestValidationException validationException = invalidateApiKeyRequest.validate(); + if (validationException != null) { + listener.onFailure(validationException); + return; + } + + if (invalidateApiKeyRequest.ownedByAuthenticatedUser()) { + listener.onResponse((Response) invalidateApiKeyResponseExpectedWhenOwnerFlagIsTrue); + } else if (invalidateApiKeyRequest.getRealmName() != null && invalidateApiKeyRequest.getRealmName().equals("realm-1")) { + listener.onResponse((Response) invalidateApiKeyResponseExpectedWhenOwnerFlagIsFalse); + } + } + }) { + final RestInvalidateApiKeyAction restInvalidateApiKeyAction = new RestInvalidateApiKeyAction(Settings.EMPTY, mockRestController, + mockLicenseState); + + restInvalidateApiKeyAction.handleRequest(restRequest, restChannel, client); + + final RestResponse restResponse = responseSetOnce.get(); + assertNotNull(restResponse); + assertThat(restResponse.status(), is(RestStatus.OK)); + final InvalidateApiKeyResponse actual = InvalidateApiKeyResponse + .fromXContent(createParser(XContentType.JSON.xContent(), restResponse.content())); + if (owner) { + assertThat(actual.getInvalidatedApiKeys().size(), is(1)); + assertThat(actual.getInvalidatedApiKeys(), + containsInAnyOrder("api-key-id-1")); + } else { + assertThat(actual.getInvalidatedApiKeys().size(), is(2)); + assertThat(actual.getInvalidatedApiKeys(), + containsInAnyOrder("api-key-id-1", "api-key-id-2")); + } + } + + } } From f598ab26c5470cd2ecdc291b644c66c8156c2213 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad <902768+bizybot@users.noreply.github.com> Date: Fri, 2 Aug 2019 07:49:02 +1000 Subject: [PATCH 04/10] Update x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java Co-Authored-By: Tim Vernum --- .../xpack/core/security/action/GetApiKeyRequestTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java index 985445ff0d0f3..1e08927b1c40c 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java @@ -147,6 +147,6 @@ public void testSerialization() throws IOException { } private static String randomNullOrEmptyString() { - return randomFrom(new String[]{"", null}); + return randomBoolean() ? "" : null; } } From f04365daf2e29a65731831391a12f0487180bf8a Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad <902768+bizybot@users.noreply.github.com> Date: Fri, 2 Aug 2019 07:49:24 +1000 Subject: [PATCH 05/10] Update x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java Co-Authored-By: Tim Vernum --- .../xpack/core/security/action/GetApiKeyRequestTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java index 1e08927b1c40c..b25f5636bc94f 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java @@ -139,7 +139,7 @@ public void testSerialization() throws IOException { getApiKeyRequest.writeTo(out); InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray())); - inputStreamStreamInput.setVersion(randomFrom(Version.V_7_4_0, Version.CURRENT)); + inputStreamStreamInput.setVersion(VersionUtils.randomVersionBetween(random(), Version.V_7_4_0, null)); GetApiKeyRequest requestFromInputStream = new GetApiKeyRequest(inputStreamStreamInput); assertThat(requestFromInputStream, equalTo(getApiKeyRequest)); From ec151b668b90b533844783bba640ba34c3830e50 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad <902768+bizybot@users.noreply.github.com> Date: Fri, 2 Aug 2019 07:49:43 +1000 Subject: [PATCH 06/10] Update x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java Co-Authored-By: Tim Vernum --- .../xpack/core/security/action/GetApiKeyRequestTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java index b25f5636bc94f..13d4126ad5a05 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java @@ -125,7 +125,7 @@ public void testSerialization() throws IOException { getApiKeyRequest.writeTo(out); InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray())); - inputStreamStreamInput.setVersion(randomFrom(Version.V_7_2_0, Version.V_7_3_0)); + inputStreamStreamInput.setVersion(VersionUtils.randomVersionBetween(random(), Version.V_7_2_0, Version.V_7_3_0)); GetApiKeyRequest requestFromInputStream = new GetApiKeyRequest(inputStreamStreamInput); assertThat(requestFromInputStream.getApiKeyId(), equalTo(getApiKeyRequest.getApiKeyId())); From 4f4a9fceb5548aa42d5c56fd097fcddeda9fc82c Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad <902768+bizybot@users.noreply.github.com> Date: Fri, 2 Aug 2019 07:50:14 +1000 Subject: [PATCH 07/10] Update x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java Co-Authored-By: Tim Vernum --- .../xpack/core/security/action/GetApiKeyRequestTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java index 13d4126ad5a05..e6440e55bc785 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java @@ -135,7 +135,7 @@ public void testSerialization() throws IOException { { ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); - out.setVersion(randomFrom(Version.V_7_4_0, Version.CURRENT)); + out.setVersion(VersionUtils.randomVersionBetween(random(), Version.V_7_4_0, null)); getApiKeyRequest.writeTo(out); InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray())); From 54025140b221636870fb690cd322ecee83d35ea6 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Fri, 2 Aug 2019 09:27:56 +1000 Subject: [PATCH 08/10] import VersionUtils --- .../xpack/core/security/action/GetApiKeyRequestTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java index e6440e55bc785..1fa5576ef2544 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java @@ -13,6 +13,7 @@ import org.elasticsearch.common.io.stream.OutputStreamStreamOutput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.VersionUtils; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; From 6ed0281a2a1872d23503c7bc17200720208c0a85 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Tue, 6 Aug 2019 22:37:51 +1000 Subject: [PATCH 09/10] address review comments --- .../core/security/action/GetApiKeyRequest.java | 2 +- .../security/action/InvalidateApiKeyRequest.java | 2 +- .../core/security/action/GetApiKeyRequestTests.java | 13 +++++++------ .../action/InvalidateApiKeyRequestTests.java | 13 +++++++------ .../action/apikey/RestGetApiKeyActionTests.java | 12 +++++++----- .../apikey/RestInvalidateApiKeyActionTests.java | 12 +++++++----- 6 files changed, 30 insertions(+), 24 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java index 73ebdcf460e9d..a84965127b2d3 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java @@ -144,7 +144,7 @@ public ActionRequestValidationException validate() { if (ownedByAuthenticatedUser) { if (Strings.hasText(realmName) || Strings.hasText(userName)) { validationException = addValidationError( - "username or realm name must not be specified when retrieving owned API keys", + "neither username nor realm-name may be specified when retrieving owned API keys", validationException); } } 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 0659522e82aa9..bca874ef9de39 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 @@ -149,7 +149,7 @@ public ActionRequestValidationException validate() { if (ownedByAuthenticatedUser) { if (Strings.hasText(realmName) || Strings.hasText(userName)) { validationException = addValidationError( - "username or realm name must not be specified when invalidating owned API keys", + "neither username nor realm-name may be specified when invalidating owned API keys", validationException); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java index 1fa5576ef2544..ae4fe4f6c79de 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java @@ -19,6 +19,7 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; +import static org.elasticsearch.test.VersionUtils.randomVersionBetween; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; @@ -93,8 +94,8 @@ public void writeTo(StreamOutput out) throws IOException { "only one of [api key id, api key name] can be 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"}, - {"username or realm name must not be specified when retrieving owned API keys"}, - {"username or realm name must not be specified when retrieving owned API keys"} + {"neither username nor realm-name may be specified when retrieving owned API keys"}, + {"neither username nor realm-name may be specified when retrieving owned API keys"} }; for (int caseNo = 0; caseNo < inputs.length; caseNo++) { @@ -122,11 +123,11 @@ public void testSerialization() throws IOException { { ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); - out.setVersion(Version.V_7_2_0); + out.setVersion(randomVersionBetween(random(), Version.V_7_0_0, Version.V_7_3_0)); getApiKeyRequest.writeTo(out); InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray())); - inputStreamStreamInput.setVersion(VersionUtils.randomVersionBetween(random(), Version.V_7_2_0, Version.V_7_3_0)); + inputStreamStreamInput.setVersion(randomVersionBetween(random(), Version.V_7_0_0, Version.V_7_3_0)); GetApiKeyRequest requestFromInputStream = new GetApiKeyRequest(inputStreamStreamInput); assertThat(requestFromInputStream.getApiKeyId(), equalTo(getApiKeyRequest.getApiKeyId())); @@ -136,11 +137,11 @@ public void testSerialization() throws IOException { { ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); - out.setVersion(VersionUtils.randomVersionBetween(random(), Version.V_7_4_0, null)); + out.setVersion(randomVersionBetween(random(), Version.V_7_4_0, Version.CURRENT)); getApiKeyRequest.writeTo(out); InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray())); - inputStreamStreamInput.setVersion(VersionUtils.randomVersionBetween(random(), Version.V_7_4_0, null)); + inputStreamStreamInput.setVersion(randomVersionBetween(random(), Version.V_7_4_0, Version.CURRENT)); GetApiKeyRequest requestFromInputStream = new GetApiKeyRequest(inputStreamStreamInput); assertThat(requestFromInputStream, equalTo(getApiKeyRequest)); 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 cac1ab51918d7..2f959c4841761 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 @@ -18,6 +18,7 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; +import static org.elasticsearch.test.VersionUtils.randomVersionBetween; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; @@ -92,8 +93,8 @@ public void writeTo(StreamOutput out) throws IOException { "only one of [api key id, api key name] can be 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"}, - {"username or realm name must not be specified when invalidating owned API keys"}, - {"username or realm name must not be specified when invalidating owned API keys"} + {"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 caseNo = 0; caseNo < inputs.length; caseNo++) { @@ -121,11 +122,11 @@ public void testSerialization() throws IOException { { ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); - out.setVersion(Version.V_7_2_0); + out.setVersion(randomVersionBetween(random(), Version.V_7_0_0, Version.V_7_3_0)); invalidateApiKeyRequest.writeTo(out); InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray())); - inputStreamStreamInput.setVersion(randomFrom(Version.V_7_2_0, Version.V_7_3_0)); + inputStreamStreamInput.setVersion(randomVersionBetween(random(), Version.V_7_0_0, Version.V_7_3_0)); InvalidateApiKeyRequest requestFromInputStream = new InvalidateApiKeyRequest(inputStreamStreamInput); assertThat(requestFromInputStream.getId(), equalTo(invalidateApiKeyRequest.getId())); @@ -135,11 +136,11 @@ public void testSerialization() throws IOException { { ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); - out.setVersion(randomFrom(Version.V_7_4_0, Version.CURRENT)); + out.setVersion(randomVersionBetween(random(), Version.V_7_4_0, Version.CURRENT)); invalidateApiKeyRequest.writeTo(out); InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray())); - inputStreamStreamInput.setVersion(randomFrom(Version.V_7_4_0, Version.CURRENT)); + inputStreamStreamInput.setVersion(randomVersionBetween(random(), Version.V_7_4_0, Version.CURRENT)); InvalidateApiKeyRequest requestFromInputStream = new InvalidateApiKeyRequest(inputStreamStreamInput); assertThat(requestFromInputStream, equalTo(invalidateApiKeyRequest)); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyActionTests.java index ea1471b4e35d8..d1046a175670c 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyActionTests.java @@ -135,10 +135,12 @@ void doExecute(ActionType action, Request request, ActionListener param = mapBuilder().put("owner", Boolean.toString(owner)).map(); - if (owner == false) { - param = mapBuilder().put("owner", Boolean.toString(owner)).put("realm_name", "realm-1").map(); + final boolean isGetRequestForOwnedKeysOnly = randomBoolean(); + final Map param; + if (isGetRequestForOwnedKeysOnly) { + param = mapBuilder().put("owner", Boolean.TRUE.toString()).map(); + } else { + param = mapBuilder().put("owner", Boolean.FALSE.toString()).put("realm_name", "realm-1").map(); } final FakeRestRequest restRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) @@ -189,7 +191,7 @@ void doExecute(ActionType action, Request request, ActionListener action, Request request, ActionListener action, Request request, ActionListener Date: Tue, 6 Aug 2019 22:50:10 +1000 Subject: [PATCH 10/10] remove unused import --- .../xpack/core/security/action/GetApiKeyRequestTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java index ae4fe4f6c79de..1c5548af70a81 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java @@ -13,7 +13,6 @@ import org.elasticsearch.common.io.stream.OutputStreamStreamOutput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.VersionUtils; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream;