From 7bc488f1f79cf9923440ddae8d4fc995e48f3974 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Mon, 7 Aug 2023 19:08:24 +0200 Subject: [PATCH] No querying by id or name --- .../action/apikey/GetApiKeyRequest.java | 12 ++++- .../action/apikey/GetApiKeyRequestTests.java | 45 +++++++++++-------- .../xpack/security/apikey/ApiKeyRestIT.java | 15 ------- 3 files changed, 36 insertions(+), 36 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/GetApiKeyRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/GetApiKeyRequest.java index 87c1a49c19fc6..18adaa3f89261 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/GetApiKeyRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/GetApiKeyRequest.java @@ -25,6 +25,8 @@ */ public final class GetApiKeyRequest extends ActionRequest { + public static TransportVersion TRANSPORT_VERSION_ACTIVE_ONLY = TransportVersion.V_8_500_053; + private final String realmName; private final String userName; private final String apiKeyId; @@ -49,7 +51,7 @@ public GetApiKeyRequest(StreamInput in) throws IOException { } else { withLimitedBy = false; } - if (in.getTransportVersion().onOrAfter(TransportVersion.V_8_500_052)) { + if (in.getTransportVersion().onOrAfter(TRANSPORT_VERSION_ACTIVE_ONLY)) { activeOnly = in.readBoolean(); } else { activeOnly = false; @@ -116,6 +118,12 @@ public ActionRequestValidationException validate() { validationException ); } + if (activeOnly) { + validationException = addValidationError( + "active_only must not be [true] when the api key id or api key name is specified", + validationException + ); + } } if (ownedByAuthenticatedUser) { if (Strings.hasText(realmName) || Strings.hasText(userName)) { @@ -144,7 +152,7 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getTransportVersion().onOrAfter(TransportVersion.V_8_5_0)) { out.writeBoolean(withLimitedBy); } - if (out.getTransportVersion().onOrAfter(TransportVersion.V_8_500_052)) { + if (out.getTransportVersion().onOrAfter(TRANSPORT_VERSION_ACTIVE_ONLY)) { out.writeBoolean(activeOnly); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/GetApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/GetApiKeyRequestTests.java index 86613099dc9c0..61e2fa6b53ab8 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/GetApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/GetApiKeyRequestTests.java @@ -21,10 +21,8 @@ import java.util.function.Supplier; import static org.elasticsearch.test.TransportVersionUtils.randomVersionBetween; -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.nullValue; +import static org.elasticsearch.xpack.core.security.action.apikey.GetApiKeyRequest.TRANSPORT_VERSION_ACTIVE_ONLY; +import static org.hamcrest.Matchers.*; public class GetApiKeyRequestTests extends ESTestCase { @@ -38,13 +36,17 @@ public void testRequestValidation() { request = GetApiKeyRequest.builder().apiKeyName(randomAlphaOfLength(5)).ownedByAuthenticatedUser(randomBoolean()).build(); ve = request.validate(); assertNull(ve); - request = GetApiKeyRequest.builder().realmName(randomAlphaOfLength(5)).build(); + request = GetApiKeyRequest.builder().realmName(randomAlphaOfLength(5)).activeOnly(randomBoolean()).build(); ve = request.validate(); assertNull(ve); - request = GetApiKeyRequest.builder().userName(randomAlphaOfLength(5)).build(); + request = GetApiKeyRequest.builder().userName(randomAlphaOfLength(5)).activeOnly(randomBoolean()).build(); ve = request.validate(); assertNull(ve); - request = GetApiKeyRequest.builder().realmName(randomAlphaOfLength(5)).userName(randomAlphaOfLength(7)).build(); + request = GetApiKeyRequest.builder() + .realmName(randomAlphaOfLength(5)) + .userName(randomAlphaOfLength(7)) + .activeOnly(randomBoolean()) + .build(); ve = request.validate(); assertNull(ve); } @@ -56,6 +58,7 @@ class Dummy extends ActionRequest { String apiKeyId; String apiKeyName; boolean ownedByAuthenticatedUser; + boolean activeOnly; Dummy(String[] a) { realm = a[0]; @@ -63,6 +66,7 @@ class Dummy extends ActionRequest { apiKeyId = a[2]; apiKeyName = a[3]; ownedByAuthenticatedUser = Boolean.parseBoolean(a[4]); + activeOnly = Boolean.parseBoolean(a[5]); } @Override @@ -79,24 +83,26 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(apiKeyName); out.writeOptionalBoolean(ownedByAuthenticatedUser); out.writeBoolean(randomBoolean()); - out.writeBoolean(randomBoolean()); + out.writeBoolean(activeOnly); } } String[][] inputs = new String[][] { - { 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" } }; + { randomNullOrEmptyString(), "user", "api-kid", "api-kname", "false", "true" }, + { "realm", randomNullOrEmptyString(), "api-kid", "api-kname", "false", "true" }, + { "realm", "user", "api-kid", randomNullOrEmptyString(), "false", "false" }, + { randomNullOrEmptyString(), randomNullOrEmptyString(), "api-kid", "api-kname", "false", "false" }, + { "realm", randomNullOrEmptyString(), randomNullOrEmptyString(), randomNullOrEmptyString(), "true", "false" }, + { randomNullOrEmptyString(), "user", randomNullOrEmptyString(), randomNullOrEmptyString(), "true", "false" } }; String[][] expectedErrorMessages = new String[][] { { "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" }, + "only one of [api key id, api key name] can be specified", + "active_only must not be [true] 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" }, + "only one of [api key id, api key name] can be specified", + "active_only must not be [true] 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" }, { "neither username nor realm-name may be specified when retrieving owned API keys" }, @@ -193,12 +199,13 @@ public void testSerialization() throws IOException { .build(); ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); - TransportVersion activeOnlyVersion = TransportVersion.V_8_500_053; - out.setTransportVersion(randomVersionBetween(random(), activeOnlyVersion, TransportVersion.current())); + out.setTransportVersion(randomVersionBetween(random(), TRANSPORT_VERSION_ACTIVE_ONLY, TransportVersion.current())); getApiKeyRequest.writeTo(out); InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray())); - inputStreamStreamInput.setTransportVersion(randomVersionBetween(random(), activeOnlyVersion, TransportVersion.current())); + inputStreamStreamInput.setTransportVersion( + randomVersionBetween(random(), TRANSPORT_VERSION_ACTIVE_ONLY, TransportVersion.current()) + ); GetApiKeyRequest requestFromInputStream = new GetApiKeyRequest(inputStreamStreamInput); assertThat(requestFromInputStream, equalTo(getApiKeyRequest)); diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java index 2d29bbbad711a..4bb5a39bb410d 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java @@ -1481,21 +1481,6 @@ public void testGetActiveOnlyApiKeys() throws Exception { assertResponseContainsApiKeyIds(response, apiKey0.id, apiKey1.id, apiKey2.id); } - // Active-only throws if used with id for existing but non-active API keys - { - final ResponseException e = expectThrows( - ResponseException.class, - () -> getApiKeysWithRequestParams(Map.of("id", randomFrom(apiKey0, apiKey2).id, "active_only", "true")) - ); - - assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(404)); - } - { - final GetApiKeyResponse response = getApiKeysWithRequestParams(Map.of("id", apiKey1.id, "active_only", "true")); - - assertResponseContainsApiKeyIds(response, apiKey1.id); - } - getSecurityClient().invalidateApiKeys(apiKey1.id); // Active-only returns empty when no keys found {