From 1177f017c7764bdd9a2bc5b935169dbbe20ebc02 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Mon, 7 Aug 2023 15:06:19 +0200 Subject: [PATCH 01/23] Support getting active-only API keys --- .../org/elasticsearch/TransportVersion.java | 3 ++- .../action/apikey/GetApiKeyRequest.java | 22 +++++++++++++++++-- .../apikey/TransportGetApiKeyAction.java | 2 +- .../xpack/security/authc/ApiKeyService.java | 5 +++-- .../action/apikey/RestGetApiKeyAction.java | 2 ++ .../security/authc/ApiKeyServiceTests.java | 6 +++-- 6 files changed, 32 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersion.java b/server/src/main/java/org/elasticsearch/TransportVersion.java index af3e9907b8664..5fed065896f5f 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersion.java +++ b/server/src/main/java/org/elasticsearch/TransportVersion.java @@ -174,9 +174,10 @@ private static TransportVersion registerTransportVersion(int id, String uniqueId public static final TransportVersion V_8_500_049 = registerTransportVersion(8_500_049, "828bb6ce-2fbb-11ee-be56-0242ac120002"); public static final TransportVersion V_8_500_050 = registerTransportVersion(8_500_050, "69722fa2-7c0a-4227-86fb-6d6a9a0a0321"); public static final TransportVersion V_8_500_051 = registerTransportVersion(8_500_051, "a28b43bc-bb5f-4406-afcf-26900aa98a71"); + public static final TransportVersion V_8_500_052 = registerTransportVersion(8_500_051, "b76ef950-af03-4dda-85c2-6400ec442e7e"); private static class CurrentHolder { - private static final TransportVersion CURRENT = findCurrent(V_8_500_051); + private static final TransportVersion CURRENT = findCurrent(V_8_500_052); // finds the pluggable current version, or uses the given fallback private static TransportVersion findCurrent(TransportVersion fallback) { 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 51ca2060388c6..146bb50e2bf0e 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 @@ -31,6 +31,7 @@ public final class GetApiKeyRequest extends ActionRequest { private final String apiKeyName; private final boolean ownedByAuthenticatedUser; private final boolean withLimitedBy; + private final boolean activeOnly; public GetApiKeyRequest(StreamInput in) throws IOException { super(in); @@ -48,6 +49,11 @@ public GetApiKeyRequest(StreamInput in) throws IOException { } else { withLimitedBy = false; } + if (in.getTransportVersion().onOrAfter(TransportVersion.V_8_500_052)) { + activeOnly = in.readBoolean(); + } else { + activeOnly = false; + } } private GetApiKeyRequest( @@ -56,7 +62,8 @@ private GetApiKeyRequest( @Nullable String apiKeyId, @Nullable String apiKeyName, boolean ownedByAuthenticatedUser, - boolean withLimitedBy + boolean withLimitedBy, + boolean activeOnly ) { this.realmName = textOrNull(realmName); this.userName = textOrNull(userName); @@ -64,6 +71,7 @@ private GetApiKeyRequest( this.apiKeyName = textOrNull(apiKeyName); this.ownedByAuthenticatedUser = ownedByAuthenticatedUser; this.withLimitedBy = withLimitedBy; + this.activeOnly = activeOnly; } private static String textOrNull(@Nullable String arg) { @@ -94,6 +102,10 @@ public boolean withLimitedBy() { return withLimitedBy; } + public boolean activeOnly() { + return activeOnly; + } + @Override public ActionRequestValidationException validate() { ActionRequestValidationException validationException = null; @@ -167,6 +179,7 @@ public static class Builder { private String apiKeyName = null; private boolean ownedByAuthenticatedUser = false; private boolean withLimitedBy = false; + private boolean activeOnly = false; public Builder realmName(String realmName) { this.realmName = realmName; @@ -206,8 +219,13 @@ public Builder withLimitedBy(boolean withLimitedBy) { return this; } + public Builder activeOnly(boolean activeOnly) { + this.activeOnly = activeOnly; + return this; + } + public GetApiKeyRequest build() { - return new GetApiKeyRequest(realmName, userName, apiKeyId, apiKeyName, ownedByAuthenticatedUser, withLimitedBy); + return new GetApiKeyRequest(realmName, userName, apiKeyId, apiKeyName, ownedByAuthenticatedUser, withLimitedBy, activeOnly); } } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/apikey/TransportGetApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/apikey/TransportGetApiKeyAction.java index 1bd0562593cc1..b627693ebda3d 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/apikey/TransportGetApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/apikey/TransportGetApiKeyAction.java @@ -57,7 +57,7 @@ protected void doExecute(Task task, GetApiKeyRequest request, ActionListener listener ) { ensureEnabled(); @@ -1901,8 +1902,8 @@ public void getApiKeys( username, apiKeyName, apiKeyIds, - false, - false, + activeOnly, + activeOnly, hit -> convertSearchHitToApiKeyInfo(hit, withLimitedBy), ActionListener.wrap(apiKeyInfos -> { if (apiKeyInfos.isEmpty()) { 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 2e2b4fb950391..cd751740dd0fb 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 @@ -50,6 +50,7 @@ protected RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClien final String realmName = request.param("realm_name"); final boolean myApiKeysOnly = request.paramAsBoolean("owner", false); final boolean withLimitedBy = request.paramAsBoolean("with_limited_by", false); + final boolean activeOnly = request.paramAsBoolean("active_only", false); final GetApiKeyRequest getApiKeyRequest = GetApiKeyRequest.builder() .realmName(realmName) .userName(userName) @@ -57,6 +58,7 @@ protected RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClien .apiKeyName(apiKeyName) .ownedByAuthenticatedUser(myApiKeysOnly) .withLimitedBy(withLimitedBy) + .activeOnly(activeOnly) .build(); return channel -> client.execute(GetApiKeyAction.INSTANCE, getApiKeyRequest, new RestBuilderListener<>(channel) { @Override diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java index 81829b0c94dee..e28b40ccde890 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java @@ -283,7 +283,8 @@ public void testGetApiKeys() throws Exception { String apiKeyName = randomFrom(randomAlphaOfLengthBetween(3, 8), null); String[] apiKeyIds = generateRandomStringArray(4, 4, true, true); PlainActionFuture getApiKeyResponsePlainActionFuture = new PlainActionFuture<>(); - service.getApiKeys(realmNames, username, apiKeyName, apiKeyIds, randomBoolean(), getApiKeyResponsePlainActionFuture); + final boolean activeOnly = false; + service.getApiKeys(realmNames, username, apiKeyName, apiKeyIds, randomBoolean(), activeOnly, getApiKeyResponsePlainActionFuture); final BoolQueryBuilder boolQuery = QueryBuilders.boolQuery().filter(QueryBuilders.termQuery("doc_type", "api_key")); if (realmNames != null && realmNames.length > 0) { if (realmNames.length == 1) { @@ -1157,7 +1158,7 @@ public void testParseRoleDescriptors() { assertThat(roleWithoutRestriction.getRestriction().getWorkflows(), nullValue()); } - public void testApiKeyServiceDisabled() throws Exception { + public void testApiKeyServiceDisabled() { final Settings settings = Settings.builder().put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), false).build(); final ApiKeyService service = createApiKeyService(settings); @@ -1169,6 +1170,7 @@ public void testApiKeyServiceDisabled() throws Exception { null, null, randomBoolean(), + randomBoolean(), new PlainActionFuture<>() ) ); From 6b3df0971734d5c74dd518432f336e4ee0c7389e Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Mon, 7 Aug 2023 15:10:16 +0200 Subject: [PATCH 02/23] Tweaks --- .../core/security/action/apikey/GetApiKeyRequest.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 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 146bb50e2bf0e..87c1a49c19fc6 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 @@ -144,6 +144,9 @@ 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)) { + out.writeBoolean(activeOnly); + } } @Override @@ -160,12 +163,13 @@ public boolean equals(Object o) { && Objects.equals(userName, that.userName) && Objects.equals(apiKeyId, that.apiKeyId) && Objects.equals(apiKeyName, that.apiKeyName) - && withLimitedBy == that.withLimitedBy; + && withLimitedBy == that.withLimitedBy + && activeOnly == that.activeOnly; } @Override public int hashCode() { - return Objects.hash(realmName, userName, apiKeyId, apiKeyName, ownedByAuthenticatedUser, withLimitedBy); + return Objects.hash(realmName, userName, apiKeyId, apiKeyName, ownedByAuthenticatedUser, withLimitedBy, activeOnly); } public static Builder builder() { From c18ed69910aa9bfedda239ae84e694d34d5ede7a Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Mon, 7 Aug 2023 15:11:07 +0200 Subject: [PATCH 03/23] Update docs/changelog/98259.yaml --- docs/changelog/98259.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/98259.yaml diff --git a/docs/changelog/98259.yaml b/docs/changelog/98259.yaml new file mode 100644 index 0000000000000..d99c748e48a48 --- /dev/null +++ b/docs/changelog/98259.yaml @@ -0,0 +1,5 @@ +pr: 98259 +summary: Support getting active-only API keys +area: Security +type: enhancement +issues: [] From c66e265b147ac0d7de01959f1ce0a07489cdfaeb Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Mon, 7 Aug 2023 15:15:33 +0200 Subject: [PATCH 04/23] Changelog --- docs/changelog/98259.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/98259.yaml b/docs/changelog/98259.yaml index d99c748e48a48..662b1707757f0 100644 --- a/docs/changelog/98259.yaml +++ b/docs/changelog/98259.yaml @@ -1,5 +1,5 @@ pr: 98259 -summary: Support getting active-only API keys +summary: Support getting active-only API keys via Get API keys API area: Security type: enhancement issues: [] From e6a896c2f6035301507b3d7522aad040e9083b7f Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Mon, 7 Aug 2023 15:24:16 +0200 Subject: [PATCH 05/23] Fix version --- server/src/main/java/org/elasticsearch/TransportVersion.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersion.java b/server/src/main/java/org/elasticsearch/TransportVersion.java index 5fed065896f5f..1277a88fcda9c 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersion.java +++ b/server/src/main/java/org/elasticsearch/TransportVersion.java @@ -174,7 +174,7 @@ private static TransportVersion registerTransportVersion(int id, String uniqueId public static final TransportVersion V_8_500_049 = registerTransportVersion(8_500_049, "828bb6ce-2fbb-11ee-be56-0242ac120002"); public static final TransportVersion V_8_500_050 = registerTransportVersion(8_500_050, "69722fa2-7c0a-4227-86fb-6d6a9a0a0321"); public static final TransportVersion V_8_500_051 = registerTransportVersion(8_500_051, "a28b43bc-bb5f-4406-afcf-26900aa98a71"); - public static final TransportVersion V_8_500_052 = registerTransportVersion(8_500_051, "b76ef950-af03-4dda-85c2-6400ec442e7e"); + public static final TransportVersion V_8_500_052 = registerTransportVersion(8_500_052, "b76ef950-af03-4dda-85c2-6400ec442e7e"); private static class CurrentHolder { private static final TransportVersion CURRENT = findCurrent(V_8_500_052); From bda405afc147a7c1dcba04a6e4e1525d34ba7ac9 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Mon, 7 Aug 2023 16:15:32 +0200 Subject: [PATCH 06/23] Serialization tests --- .../action/apikey/GetApiKeyRequestTests.java | 37 +++++++++++++++++-- 1 file changed, 34 insertions(+), 3 deletions(-) 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 e88bdf2751778..3cb9d284ae3b7 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 @@ -79,6 +79,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(apiKeyName); out.writeOptionalBoolean(ownedByAuthenticatedUser); out.writeBoolean(randomBoolean()); + out.writeBoolean(randomBoolean()); } } @@ -143,6 +144,7 @@ public void testSerialization() throws IOException { .apiKeyId(apiKeyId) .ownedByAuthenticatedUser(true) .withLimitedBy(randomBoolean()) + .activeOnly(randomBoolean()) .build(); ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); @@ -157,17 +159,46 @@ public void testSerialization() throws IOException { assertThat(requestFromInputStream.ownedByAuthenticatedUser(), is(true)); // old version so the default for `withLimitedBy` is false assertThat(requestFromInputStream.withLimitedBy(), is(false)); + // old version so the default for `activeOnly` is false + assertThat(requestFromInputStream.activeOnly(), is(false)); } { - final GetApiKeyRequest getApiKeyRequest = GetApiKeyRequest.builder().apiKeyId(apiKeyId).withLimitedBy(randomBoolean()).build(); + final GetApiKeyRequest getApiKeyRequest = GetApiKeyRequest.builder() + .apiKeyId(apiKeyId) + .ownedByAuthenticatedUser(randomBoolean()) + .withLimitedBy(randomBoolean()) + .activeOnly(randomBoolean()) + .build(); + ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); + OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); + TransportVersion beforeActiveOnly = TransportVersion.V_8_500_051; + out.setTransportVersion(randomVersionBetween(random(), TransportVersion.V_8_5_0, beforeActiveOnly)); + getApiKeyRequest.writeTo(out); + + InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray())); + inputStreamStreamInput.setTransportVersion(randomVersionBetween(random(), TransportVersion.V_8_5_0, beforeActiveOnly)); + GetApiKeyRequest requestFromInputStream = new GetApiKeyRequest(inputStreamStreamInput); + + assertThat(requestFromInputStream.getApiKeyId(), equalTo(getApiKeyRequest.getApiKeyId())); + assertThat(requestFromInputStream.ownedByAuthenticatedUser(), is(getApiKeyRequest.ownedByAuthenticatedUser())); + assertThat(requestFromInputStream.withLimitedBy(), is(getApiKeyRequest.withLimitedBy())); + // old version so the default for `activeOnly` is false + assertThat(requestFromInputStream.activeOnly(), is(false)); + } + { + final GetApiKeyRequest getApiKeyRequest = GetApiKeyRequest.builder() + .apiKeyId(apiKeyId) + .withLimitedBy(randomBoolean()) + .activeOnly(randomBoolean()) + .build(); ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); - out.setTransportVersion(randomVersionBetween(random(), TransportVersion.V_8_5_0, TransportVersion.current())); + out.setTransportVersion(randomVersionBetween(random(), TransportVersion.V_8_500_052, TransportVersion.current())); getApiKeyRequest.writeTo(out); InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray())); inputStreamStreamInput.setTransportVersion( - randomVersionBetween(random(), TransportVersion.V_8_5_0, TransportVersion.current()) + randomVersionBetween(random(), TransportVersion.V_8_500_052, TransportVersion.current()) ); GetApiKeyRequest requestFromInputStream = new GetApiKeyRequest(inputStreamStreamInput); From 99bfcaacad711c27bf6ff46aa68220729cdfb026 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Mon, 7 Aug 2023 17:05:15 +0200 Subject: [PATCH 07/23] Rest test --- .../xpack/security/apikey/ApiKeyRestIT.java | 73 ++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) 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 379a447e86ce4..5d68908ade5a1 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 @@ -8,6 +8,7 @@ package org.elasticsearch.xpack.security.apikey; import org.apache.http.client.methods.HttpGet; +import org.apache.http.util.EntityUtils; import org.elasticsearch.client.Request; import org.elasticsearch.client.RequestOptions; import org.elasticsearch.client.Response; @@ -15,11 +16,13 @@ import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.core.Strings; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.core.Tuple; import org.elasticsearch.test.XContentTestUtils; import org.elasticsearch.test.rest.ObjectPath; import org.elasticsearch.transport.TcpTransport; import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xcontent.json.JsonXContent; import org.elasticsearch.xpack.core.security.action.apikey.ApiKey; @@ -34,10 +37,15 @@ import java.io.IOException; import java.time.Instant; import java.time.temporal.ChronoUnit; +import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; + +import javax.annotation.Nullable; import static org.elasticsearch.test.SecuritySettingsSourceField.ES_TEST_ROOT_ROLE; import static org.elasticsearch.test.SecuritySettingsSourceField.ES_TEST_ROOT_ROLE_DESCRIPTOR; @@ -1414,6 +1422,61 @@ public void testWorkflowsRestrictionValidation() throws IOException { } } + public void testGetActiveOnlyApiKeys() throws Exception { + final EncodedApiKey apiKey0 = createApiKey("key-0", Collections.emptyMap()); + final EncodedApiKey apiKey1 = createApiKey("key-1", Collections.emptyMap()); + // Set short enough expiration for the API key to be expired by the time we query for it + final EncodedApiKey apiKey2 = createApiKey("key-2", Collections.emptyMap(), TimeValue.timeValueNanos(1)); + + { + final var request = new Request(HttpGet.METHOD_NAME, "/_security/api_key/"); + request.addParameter("active_only", "true"); + + final GetApiKeyResponse response = GetApiKeyResponse.fromXContent(getParser(adminClient().performRequest(request))); + + assertResponseContainsApiKeyIds(response, apiKey0.id, apiKey1.id); + } + { + final var request = new Request(HttpGet.METHOD_NAME, "/_security/api_key/"); + if (randomBoolean()) { + request.addParameter("active_only", "false"); + } + + final GetApiKeyResponse response = GetApiKeyResponse.fromXContent(getParser(adminClient().performRequest(request))); + + assertResponseContainsApiKeyIds(response, apiKey0.id, apiKey1.id, apiKey2.id); + } + + getSecurityClient().invalidateApiKeys(apiKey0.id); + { + final var request = new Request(HttpGet.METHOD_NAME, "/_security/api_key/"); + request.addParameter("active_only", "true"); + + final GetApiKeyResponse response = GetApiKeyResponse.fromXContent(getParser(adminClient().performRequest(request))); + + assertResponseContainsApiKeyIds(response, apiKey1.id); + } + { + final var request = new Request(HttpGet.METHOD_NAME, "/_security/api_key/"); + if (randomBoolean()) { + request.addParameter("active_only", "false"); + } + + final GetApiKeyResponse response = GetApiKeyResponse.fromXContent(getParser(adminClient().performRequest(request))); + + assertResponseContainsApiKeyIds(response, apiKey0.id, apiKey1.id, apiKey2.id); + } + } + + private static void assertResponseContainsApiKeyIds(GetApiKeyResponse response, String... ids) { + assertThat(Arrays.stream(response.getApiKeyInfos()).map(ApiKey::getId).collect(Collectors.toList()), containsInAnyOrder(ids)); + } + + private XContentParser getParser(Response response) throws IOException { + final byte[] responseBody = EntityUtils.toByteArray(response.getEntity()); + return XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, responseBody); + } + private Response performRequestWithManageOwnApiKeyUser(Request request) throws IOException { request.setOptions( RequestOptions.DEFAULT.toBuilder() @@ -1555,7 +1618,15 @@ private Response performRequestUsingRandomAuthMethod(final Request request) thro } private EncodedApiKey createApiKey(final String apiKeyName, final Map metadata) throws IOException { - final Map createApiKeyRequestBody = Map.of("name", apiKeyName, "metadata", metadata); + return createApiKey(apiKeyName, metadata, null); + } + + private EncodedApiKey createApiKey(final String apiKeyName, final Map metadata, @Nullable TimeValue expiration) + throws IOException { + + final Map createApiKeyRequestBody = expiration == null + ? Map.of("name", apiKeyName, "metadata", metadata) + : Map.of("name", apiKeyName, "metadata", metadata, "expiration", expiration); final Request createApiKeyRequest = new Request("POST", "_security/api_key"); createApiKeyRequest.setJsonEntity(XContentTestUtils.convertToXContent(createApiKeyRequestBody, XContentType.JSON).utf8ToString()); From 5226afa0e48f1f8aa149a3acf9e6931bdecaebdb Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Mon, 7 Aug 2023 17:36:48 +0200 Subject: [PATCH 08/23] Moar tests --- .../xpack/security/apikey/ApiKeyRestIT.java | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) 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 5d68908ade5a1..5bc72316ab968 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 @@ -56,6 +56,7 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.emptyArray; import static org.hamcrest.Matchers.emptyString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; @@ -1466,6 +1467,37 @@ 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 var request = new Request(HttpGet.METHOD_NAME, "/_security/api_key/"); + request.addParameter("id", randomFrom(apiKey0, apiKey2).id); + request.addParameter("active_only", "true"); + + final ResponseException e = expectThrows(ResponseException.class, () -> adminClient().performRequest(request)); + + assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(404)); + } + { + final var request = new Request(HttpGet.METHOD_NAME, "/_security/api_key/"); + request.addParameter("id", apiKey1.id); + request.addParameter("active_only", "true"); + + final GetApiKeyResponse response = GetApiKeyResponse.fromXContent(getParser(adminClient().performRequest(request))); + + assertResponseContainsApiKeyIds(response, apiKey1.id); + } + + getSecurityClient().invalidateApiKeys(apiKey1.id); + // Active-only returns empty when no keys found + { + final var request = new Request(HttpGet.METHOD_NAME, "/_security/api_key/"); + request.addParameter("active_only", "true"); + + final GetApiKeyResponse response = GetApiKeyResponse.fromXContent(getParser(adminClient().performRequest(request))); + + assertThat(response.getApiKeyInfos(), emptyArray()); + } } private static void assertResponseContainsApiKeyIds(GetApiKeyResponse response, String... ids) { @@ -1653,6 +1685,10 @@ private ObjectPath fetchCrossClusterApiKeyById(String apiKeyId) throws IOExcepti fetchRequest = new Request("GET", "/_security/api_key"); fetchRequest.addParameter("id", apiKeyId); fetchRequest.addParameter("with_limited_by", String.valueOf(randomBoolean())); + if (randomBoolean()) { + fetchRequest.addParameter("active_only", "false"); + } + } else { fetchRequest = new Request("GET", "/_security/_query/api_key"); fetchRequest.addParameter("with_limited_by", String.valueOf(randomBoolean())); From aa65d03fb5770a294a5e5ea97ababd79dcce0cf2 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Mon, 7 Aug 2023 18:18:11 +0200 Subject: [PATCH 09/23] Yet more tests --- .../xpack/security/apikey/ApiKeyRestIT.java | 65 +++++++++++-------- .../xpack/security/authc/ApiKeyService.java | 2 +- 2 files changed, 39 insertions(+), 28 deletions(-) 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 48e7da56a0c97..2d29bbbad711a 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 @@ -40,6 +40,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -1430,60 +1431,67 @@ public void testGetActiveOnlyApiKeys() throws Exception { final EncodedApiKey apiKey2 = createApiKey("key-2", Collections.emptyMap(), TimeValue.timeValueNanos(1)); { - final var request = new Request(HttpGet.METHOD_NAME, "/_security/api_key/"); - request.addParameter("active_only", "true"); + final Map parameters = new HashMap<>(); + parameters.put("active_only", "true"); + if (randomBoolean()) { + parameters.put("username", MANAGE_OWN_API_KEY_USER); + } - final GetApiKeyResponse response = GetApiKeyResponse.fromXContent(getParser(adminClient().performRequest(request))); + final GetApiKeyResponse response = getApiKeysWithRequestParams(parameters); assertResponseContainsApiKeyIds(response, apiKey0.id, apiKey1.id); } { - final var request = new Request(HttpGet.METHOD_NAME, "/_security/api_key/"); + final Map parameters = new HashMap<>(); + if (randomBoolean()) { + parameters.put("active_only", "false"); + } if (randomBoolean()) { - request.addParameter("active_only", "false"); + parameters.put("username", MANAGE_OWN_API_KEY_USER); } - final GetApiKeyResponse response = GetApiKeyResponse.fromXContent(getParser(adminClient().performRequest(request))); + final GetApiKeyResponse response = getApiKeysWithRequestParams(parameters); assertResponseContainsApiKeyIds(response, apiKey0.id, apiKey1.id, apiKey2.id); } getSecurityClient().invalidateApiKeys(apiKey0.id); { - final var request = new Request(HttpGet.METHOD_NAME, "/_security/api_key/"); - request.addParameter("active_only", "true"); + final Map parameters = new HashMap<>(); + parameters.put("active_only", "true"); + if (randomBoolean()) { + parameters.put("username", MANAGE_OWN_API_KEY_USER); + } - final GetApiKeyResponse response = GetApiKeyResponse.fromXContent(getParser(adminClient().performRequest(request))); + final GetApiKeyResponse response = getApiKeysWithRequestParams(parameters); assertResponseContainsApiKeyIds(response, apiKey1.id); } { - final var request = new Request(HttpGet.METHOD_NAME, "/_security/api_key/"); + final Map parameters = new HashMap<>(); + if (randomBoolean()) { + parameters.put("active_only", "false"); + } if (randomBoolean()) { - request.addParameter("active_only", "false"); + parameters.put("username", MANAGE_OWN_API_KEY_USER); } - final GetApiKeyResponse response = GetApiKeyResponse.fromXContent(getParser(adminClient().performRequest(request))); + final GetApiKeyResponse response = getApiKeysWithRequestParams(parameters); assertResponseContainsApiKeyIds(response, apiKey0.id, apiKey1.id, apiKey2.id); } // Active-only throws if used with id for existing but non-active API keys { - final var request = new Request(HttpGet.METHOD_NAME, "/_security/api_key/"); - request.addParameter("id", randomFrom(apiKey0, apiKey2).id); - request.addParameter("active_only", "true"); - - final ResponseException e = expectThrows(ResponseException.class, () -> adminClient().performRequest(request)); + 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 var request = new Request(HttpGet.METHOD_NAME, "/_security/api_key/"); - request.addParameter("id", apiKey1.id); - request.addParameter("active_only", "true"); - - final GetApiKeyResponse response = GetApiKeyResponse.fromXContent(getParser(adminClient().performRequest(request))); + final GetApiKeyResponse response = getApiKeysWithRequestParams(Map.of("id", apiKey1.id, "active_only", "true")); assertResponseContainsApiKeyIds(response, apiKey1.id); } @@ -1491,20 +1499,23 @@ public void testGetActiveOnlyApiKeys() throws Exception { getSecurityClient().invalidateApiKeys(apiKey1.id); // Active-only returns empty when no keys found { - final var request = new Request(HttpGet.METHOD_NAME, "/_security/api_key/"); - request.addParameter("active_only", "true"); - - final GetApiKeyResponse response = GetApiKeyResponse.fromXContent(getParser(adminClient().performRequest(request))); + final GetApiKeyResponse response = getApiKeysWithRequestParams(Map.of("active_only", "true")); assertThat(response.getApiKeyInfos(), emptyArray()); } } + private static GetApiKeyResponse getApiKeysWithRequestParams(Map requestParams) throws IOException { + final var request = new Request(HttpGet.METHOD_NAME, "/_security/api_key/"); + request.addParameters(requestParams); + return GetApiKeyResponse.fromXContent(getParser(adminClient().performRequest(request))); + } + private static void assertResponseContainsApiKeyIds(GetApiKeyResponse response, String... ids) { assertThat(Arrays.stream(response.getApiKeyInfos()).map(ApiKey::getId).collect(Collectors.toList()), containsInAnyOrder(ids)); } - private XContentParser getParser(Response response) throws IOException { + private static XContentParser getParser(Response response) throws IOException { final byte[] responseBody = EntityUtils.toByteArray(response.getEntity()); return XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, responseBody); } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java index 9ec6714f94609..79bbfefe518a6 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java @@ -1908,7 +1908,7 @@ public void getApiKeys( ActionListener.wrap(apiKeyInfos -> { if (apiKeyInfos.isEmpty()) { logger.debug( - "No active api keys found for realms {}, user [{}], api key name [{}] and api key ids {}", + "No api keys found for realms {}, user [{}], api key name [{}] and api key ids {}", Arrays.toString(realmNames), username, apiKeyName, From 7bc488f1f79cf9923440ddae8d4fc995e48f3974 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Mon, 7 Aug 2023 19:08:24 +0200 Subject: [PATCH 10/23] 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 { From d081b7c17962c50d205439a1a317602360bbf0c6 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Mon, 7 Aug 2023 19:56:19 +0200 Subject: [PATCH 11/23] Fix imports --- .../core/security/action/apikey/GetApiKeyRequestTests.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 61e2fa6b53ab8..87b64a419a25f 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 @@ -22,7 +22,10 @@ import static org.elasticsearch.test.TransportVersionUtils.randomVersionBetween; import static org.elasticsearch.xpack.core.security.action.apikey.GetApiKeyRequest.TRANSPORT_VERSION_ACTIVE_ONLY; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; public class GetApiKeyRequestTests extends ESTestCase { From 6b29031b90cb18ba07bcc03eae552318e7a3d206 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Mon, 7 Aug 2023 21:30:29 +0200 Subject: [PATCH 12/23] Test with multiple users --- .../action/apikey/GetApiKeyRequestTests.java | 4 + .../xpack/security/apikey/ApiKeyRestIT.java | 85 ++++++++++++------- 2 files changed, 59 insertions(+), 30 deletions(-) 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 87b64a419a25f..9e99ac6f1d4ff 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 @@ -93,6 +93,8 @@ public void writeTo(StreamOutput out) throws IOException { String[][] inputs = new String[][] { { randomNullOrEmptyString(), "user", "api-kid", "api-kname", "false", "true" }, { "realm", randomNullOrEmptyString(), "api-kid", "api-kname", "false", "true" }, + { randomNullOrEmptyString(), randomNullOrEmptyString(), randomNullOrEmptyString(), "api-kname", "false", "true" }, + { randomNullOrEmptyString(), randomNullOrEmptyString(), "api-kid", randomNullOrEmptyString(), "false", "true" }, { "realm", "user", "api-kid", randomNullOrEmptyString(), "false", "false" }, { randomNullOrEmptyString(), randomNullOrEmptyString(), "api-kid", "api-kname", "false", "false" }, { "realm", randomNullOrEmptyString(), randomNullOrEmptyString(), randomNullOrEmptyString(), "true", "false" }, @@ -106,6 +108,8 @@ public void writeTo(StreamOutput out) throws IOException { "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", "active_only must not be [true] when the api key id or api key name is specified" }, + { "active_only must not be [true] when the api key id or api key name is 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" }, 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 4bb5a39bb410d..7fe4611eb8b66 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 @@ -1431,14 +1431,7 @@ public void testGetActiveOnlyApiKeys() throws Exception { final EncodedApiKey apiKey2 = createApiKey("key-2", Collections.emptyMap(), TimeValue.timeValueNanos(1)); { - final Map parameters = new HashMap<>(); - parameters.put("active_only", "true"); - if (randomBoolean()) { - parameters.put("username", MANAGE_OWN_API_KEY_USER); - } - - final GetApiKeyResponse response = getApiKeysWithRequestParams(parameters); - + final GetApiKeyResponse response = getApiKeysWithRequestParams(Map.of("active_only", "true")); assertResponseContainsApiKeyIds(response, apiKey0.id, apiKey1.id); } { @@ -1446,25 +1439,14 @@ public void testGetActiveOnlyApiKeys() throws Exception { if (randomBoolean()) { parameters.put("active_only", "false"); } - if (randomBoolean()) { - parameters.put("username", MANAGE_OWN_API_KEY_USER); - } - final GetApiKeyResponse response = getApiKeysWithRequestParams(parameters); - assertResponseContainsApiKeyIds(response, apiKey0.id, apiKey1.id, apiKey2.id); } getSecurityClient().invalidateApiKeys(apiKey0.id); - { - final Map parameters = new HashMap<>(); - parameters.put("active_only", "true"); - if (randomBoolean()) { - parameters.put("username", MANAGE_OWN_API_KEY_USER); - } - - final GetApiKeyResponse response = getApiKeysWithRequestParams(parameters); + { + final GetApiKeyResponse response = getApiKeysWithRequestParams(Map.of("active_only", "true")); assertResponseContainsApiKeyIds(response, apiKey1.id); } { @@ -1472,24 +1454,69 @@ public void testGetActiveOnlyApiKeys() throws Exception { if (randomBoolean()) { parameters.put("active_only", "false"); } - if (randomBoolean()) { - parameters.put("username", MANAGE_OWN_API_KEY_USER); - } - final GetApiKeyResponse response = getApiKeysWithRequestParams(parameters); - assertResponseContainsApiKeyIds(response, apiKey0.id, apiKey1.id, apiKey2.id); } getSecurityClient().invalidateApiKeys(apiKey1.id); - // Active-only returns empty when no keys found + { final GetApiKeyResponse response = getApiKeysWithRequestParams(Map.of("active_only", "true")); - assertThat(response.getApiKeyInfos(), emptyArray()); } } + public void testGetActiveOnlyApiKeysWithMultipleUsers() throws Exception { + final String manageOwnApiKeyUserApiKeyId = createApiKey("key-0", Collections.emptyMap()).id; + + final var createApiKeyRequest = new Request("POST", "_security/api_key"); + createApiKeyRequest.setJsonEntity(XContentTestUtils.convertToXContent(Map.of("name", "key-1"), XContentType.JSON).utf8ToString()); + setUserForRequest(createApiKeyRequest, MANAGE_API_KEY_USER, END_USER_PASSWORD); + final String manageApiKeyUserApiKeyId = assertOKAndCreateObjectPath(client().performRequest(createApiKeyRequest)).evaluate("id"); + + // Two active API keys + assertResponseContainsApiKeyIds( + getApiKeysWithRequestParams(Map.of("active_only", Boolean.toString(randomBoolean()))), + manageOwnApiKeyUserApiKeyId, + manageApiKeyUserApiKeyId + ); + assertResponseContainsApiKeyIds( + getApiKeysWithRequestParams(Map.of("active_only", Boolean.toString(randomBoolean()), "username", MANAGE_API_KEY_USER)), + manageApiKeyUserApiKeyId + ); + assertResponseContainsApiKeyIds( + getApiKeysWithRequestParams(Map.of("active_only", Boolean.toString(randomBoolean()), "username", MANAGE_OWN_API_KEY_USER)), + manageOwnApiKeyUserApiKeyId + ); + + // One active API key + invalidateApiKeysForUser(MANAGE_OWN_API_KEY_USER); + + assertResponseContainsApiKeyIds(getApiKeysWithRequestParams(Map.of("active_only", "true")), manageApiKeyUserApiKeyId); + assertResponseContainsApiKeyIds( + getApiKeysWithRequestParams(Map.of("active_only", "true", "username", MANAGE_API_KEY_USER)), + manageApiKeyUserApiKeyId + ); + assertThat( + getApiKeysWithRequestParams(Map.of("active_only", "true", "username", MANAGE_OWN_API_KEY_USER)).getApiKeyInfos(), + emptyArray() + ); + + // No more active API keys + invalidateApiKeysForUser(MANAGE_API_KEY_USER); + + assertThat( + getApiKeysWithRequestParams(Map.of("active_only", "true", "username", randomFrom(MANAGE_API_KEY_USER, MANAGE_OWN_API_KEY_USER))) + .getApiKeyInfos(), + emptyArray() + ); + assertResponseContainsApiKeyIds( + getApiKeysWithRequestParams(Map.of("active_only", "false")), + manageOwnApiKeyUserApiKeyId, + manageApiKeyUserApiKeyId + ); + } + private static GetApiKeyResponse getApiKeysWithRequestParams(Map requestParams) throws IOException { final var request = new Request(HttpGet.METHOD_NAME, "/_security/api_key/"); request.addParameters(requestParams); @@ -1513,7 +1540,6 @@ private Response performRequestWithManageOwnApiKeyUser(Request request) throws I return client().performRequest(request); } - @SuppressWarnings("unchecked") private void fetchAndAssertApiKeyContainsWorkflows(String apiKeyId, String roleName, String... expectedWorkflows) throws IOException { Response getApiKeyResponse = fetchApiKey(apiKeyId); List actualWorkflows = assertOKAndCreateObjectPath(getApiKeyResponse).evaluate( @@ -1522,7 +1548,6 @@ private void fetchAndAssertApiKeyContainsWorkflows(String apiKeyId, String roleN assertThat(actualWorkflows, containsInAnyOrder(expectedWorkflows)); } - @SuppressWarnings("unchecked") private void fetchAndAssertApiKeyDoesNotContainWorkflows(String apiKeyId, String roleName) throws IOException { Response getApiKeyResponse = fetchApiKey(apiKeyId); Map restriction = assertOKAndCreateObjectPath(getApiKeyResponse).evaluate( From ed37b3c39b3f91c750c98acf69e0827f3494c1ae Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Tue, 8 Aug 2023 11:42:56 +0200 Subject: [PATCH 13/23] Cross cluster keys --- .../security/apikey/GetApiKeysRestIT.java | 138 +++++++++--------- 1 file changed, 73 insertions(+), 65 deletions(-) diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java index 79556b06a22a4..22f35578ff99a 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java @@ -14,6 +14,7 @@ import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.core.TimeValue; import org.elasticsearch.test.XContentTestUtils; +import org.elasticsearch.transport.TcpTransport; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; @@ -21,7 +22,6 @@ import org.elasticsearch.xpack.core.security.action.apikey.GetApiKeyResponse; import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken; import org.elasticsearch.xpack.security.SecurityOnTrialLicenseRestTestCase; -import org.junit.After; import org.junit.Before; import java.io.IOException; @@ -40,77 +40,50 @@ public class GetApiKeysRestIT extends SecurityOnTrialLicenseRestTestCase { private static final SecureString END_USER_PASSWORD = new SecureString("end-user-password".toCharArray()); private static final String MANAGE_OWN_API_KEY_USER = "manage_own_api_key_user"; - private static final String MANAGE_API_KEY_USER = "manage_api_key_user"; private static final String MANAGE_SECURITY_USER = "manage_security_user"; @Before public void createUsers() throws IOException { createUser(MANAGE_OWN_API_KEY_USER, END_USER_PASSWORD, List.of("manage_own_api_key_role")); createRole("manage_own_api_key_role", Set.of("manage_own_api_key")); - createUser(MANAGE_API_KEY_USER, END_USER_PASSWORD, List.of("manage_api_key_role")); - createRole("manage_api_key_role", Set.of("manage_api_key")); createUser(MANAGE_SECURITY_USER, END_USER_PASSWORD, List.of("manage_security_role")); createRole("manage_security_role", Set.of("manage_security")); } - @After - public void cleanUp() throws IOException { - deleteUser(MANAGE_OWN_API_KEY_USER); - deleteUser(MANAGE_API_KEY_USER); - deleteUser(MANAGE_SECURITY_USER); - deleteRole("manage_own_api_key_role"); - deleteRole("manage_api_key_role"); - deleteRole("manage_security_role"); - invalidateApiKeysForUser(MANAGE_OWN_API_KEY_USER); - invalidateApiKeysForUser(MANAGE_API_KEY_USER); - invalidateApiKeysForUser(MANAGE_SECURITY_USER); - } - public void testGetApiKeysWithActiveOnlyFlag() throws Exception { - final String apiKeyId0 = createApiKey("key-0", MANAGE_OWN_API_KEY_USER); - final String apiKeyId1 = createApiKey("key-1", MANAGE_OWN_API_KEY_USER); + final String apiKeyId0 = createApiKey("key-0", MANAGE_SECURITY_USER); + final String apiKeyId1 = createApiKey("key-1", MANAGE_SECURITY_USER); // Set short enough expiration for the API key to be expired by the time we query for it - final String apiKeyId2 = createApiKey("key-2", MANAGE_OWN_API_KEY_USER, TimeValue.timeValueNanos(1)); + final String apiKeyId2 = createApiKey("key-2", MANAGE_SECURITY_USER, TimeValue.timeValueNanos(1)); - { - final GetApiKeyResponse response = getApiKeysWithRequestParams(Map.of("active_only", "true")); - assertResponseContainsApiKeyIds(response, apiKeyId0, apiKeyId1); - } + assertResponseContainsApiKeyIds(getApiKeysWithRequestParams(Map.of("active_only", "true")), apiKeyId0, apiKeyId1); { final Map parameters = new HashMap<>(); if (randomBoolean()) { parameters.put("active_only", "false"); } - final GetApiKeyResponse response = getApiKeysWithRequestParams(parameters); - assertResponseContainsApiKeyIds(response, apiKeyId0, apiKeyId1, apiKeyId2); + assertResponseContainsApiKeyIds(getApiKeysWithRequestParams(parameters), apiKeyId0, apiKeyId1, apiKeyId2); } getSecurityClient().invalidateApiKeys(apiKeyId0); - { - final GetApiKeyResponse response = getApiKeysWithRequestParams(Map.of("active_only", "true")); - assertResponseContainsApiKeyIds(response, apiKeyId1); - } + assertResponseContainsApiKeyIds(getApiKeysWithRequestParams(Map.of("active_only", "true")), apiKeyId1); { final Map parameters = new HashMap<>(); if (randomBoolean()) { parameters.put("active_only", "false"); } - final GetApiKeyResponse response = getApiKeysWithRequestParams(parameters); - assertResponseContainsApiKeyIds(response, apiKeyId0, apiKeyId1, apiKeyId2); + assertResponseContainsApiKeyIds(getApiKeysWithRequestParams(parameters), apiKeyId0, apiKeyId1, apiKeyId2); } getSecurityClient().invalidateApiKeys(apiKeyId1); - { - final GetApiKeyResponse response = getApiKeysWithRequestParams(Map.of("active_only", "true")); - assertThat(response.getApiKeyInfos(), emptyArray()); - } + assertThat(getApiKeysWithRequestParams(Map.of("active_only", "true")).getApiKeyInfos(), emptyArray()); } public void testGetApiKeysWithActiveOnlyFlagAndMultipleUsers() throws Exception { final String manageOwnApiKeyUserApiKeyId = createApiKey("key-0", MANAGE_OWN_API_KEY_USER); - final String manageApiKeyUserApiKeyId = createApiKey("key-1", MANAGE_API_KEY_USER); + final String manageApiKeyUserApiKeyId = createApiKey("key-1", MANAGE_SECURITY_USER); // Two active API keys assertResponseContainsApiKeyIds( @@ -119,43 +92,57 @@ public void testGetApiKeysWithActiveOnlyFlagAndMultipleUsers() throws Exception manageApiKeyUserApiKeyId ); assertResponseContainsApiKeyIds( - getApiKeysWithRequestParams(Map.of("active_only", Boolean.toString(randomBoolean()), "username", MANAGE_API_KEY_USER)), + getApiKeysWithRequestParams(Map.of("active_only", Boolean.toString(randomBoolean()), "username", MANAGE_SECURITY_USER)), manageApiKeyUserApiKeyId ); assertResponseContainsApiKeyIds( getApiKeysWithRequestParams(Map.of("active_only", Boolean.toString(randomBoolean()), "username", MANAGE_OWN_API_KEY_USER)), manageOwnApiKeyUserApiKeyId ); + assertResponseContainsApiKeyIds( + getApiKeysWithRequestParams(MANAGE_SECURITY_USER, Map.of("active_only", Boolean.toString(randomBoolean()), "owner", "true")), + manageApiKeyUserApiKeyId + ); + assertResponseContainsApiKeyIds( + getApiKeysWithRequestParams(MANAGE_OWN_API_KEY_USER, Map.of("active_only", Boolean.toString(randomBoolean()), "owner", "true")), + manageOwnApiKeyUserApiKeyId + ); // One active API key invalidateApiKeysForUser(MANAGE_OWN_API_KEY_USER); assertResponseContainsApiKeyIds(getApiKeysWithRequestParams(Map.of("active_only", "true")), manageApiKeyUserApiKeyId); assertResponseContainsApiKeyIds( - getApiKeysWithRequestParams(Map.of("active_only", "true", "username", MANAGE_API_KEY_USER)), + getApiKeysWithRequestParams(Map.of("active_only", "true", "username", MANAGE_SECURITY_USER)), + manageApiKeyUserApiKeyId + ); + assertResponseContainsApiKeyIds( + getApiKeysWithRequestParams(MANAGE_SECURITY_USER, Map.of("active_only", "true", "owner", "true")), manageApiKeyUserApiKeyId ); assertThat( getApiKeysWithRequestParams(Map.of("active_only", "true", "username", MANAGE_OWN_API_KEY_USER)).getApiKeyInfos(), emptyArray() ); - - // Test with owner=true flag - assertResponseContainsApiKeyIds( - getApiKeysWithRequestParams(Map.of("active_only", "true", "owner", "true")), - manageApiKeyUserApiKeyId + assertThat( + getApiKeysWithRequestParams(MANAGE_OWN_API_KEY_USER, Map.of("active_only", "true", "owner", "true")).getApiKeyInfos(), + emptyArray() ); // No more active API keys - invalidateApiKeysForUser(MANAGE_API_KEY_USER); + invalidateApiKeysForUser(MANAGE_SECURITY_USER); assertThat( - getApiKeysWithRequestParams(Map.of("active_only", "true", "username", randomFrom(MANAGE_API_KEY_USER, MANAGE_OWN_API_KEY_USER))) - .getApiKeyInfos(), + getApiKeysWithRequestParams( + Map.of("active_only", "true", "username", randomFrom(MANAGE_SECURITY_USER, MANAGE_OWN_API_KEY_USER)) + ).getApiKeyInfos(), emptyArray() ); assertThat( - getApiKeysWithRequestParams(Map.of("active_only", "true", "owner", Boolean.toString(randomBoolean()))).getApiKeyInfos(), + getApiKeysWithRequestParams( + randomFrom(MANAGE_SECURITY_USER, MANAGE_OWN_API_KEY_USER), + Map.of("active_only", "true", "owner", "true") + ).getApiKeyInfos(), emptyArray() ); assertResponseContainsApiKeyIds( @@ -166,7 +153,7 @@ public void testGetApiKeysWithActiveOnlyFlagAndMultipleUsers() throws Exception } private GetApiKeyResponse getApiKeysWithRequestParams(Map requestParams) throws IOException { - return getApiKeysWithRequestParams(MANAGE_API_KEY_USER, requestParams); + return getApiKeysWithRequestParams(MANAGE_SECURITY_USER, requestParams); } private GetApiKeyResponse getApiKeysWithRequestParams(String userOnRequest, Map requestParams) throws IOException { @@ -191,22 +178,43 @@ private String createApiKey(String apiKeyName, String creatorUser) throws IOExce private String createApiKey(String apiKeyName, String creatorUser, @Nullable TimeValue expiration) throws IOException { // Sanity check to ensure API key name and creator name aren't flipped - assert creatorUser.equals(MANAGE_OWN_API_KEY_USER) - || creatorUser.equals(MANAGE_API_KEY_USER) - || creatorUser.equals(MANAGE_SECURITY_USER); - - final Map createApiKeyRequestBody = expiration == null - ? Map.of("name", apiKeyName) - : Map.of("name", apiKeyName, "expiration", expiration); - final var createApiKeyRequest = new Request("POST", "_security/api_key"); - createApiKeyRequest.setJsonEntity(XContentTestUtils.convertToXContent(createApiKeyRequestBody, XContentType.JSON).utf8ToString()); - setUserForRequest(createApiKeyRequest, creatorUser); - - final Response createApiKeyResponse = client().performRequest(createApiKeyRequest); - - assertOK(createApiKeyResponse); - final Map createApiKeyResponseMap = responseAsMap(createApiKeyResponse); - return (String) createApiKeyResponseMap.get("id"); + assert creatorUser.equals(MANAGE_OWN_API_KEY_USER) || creatorUser.equals(MANAGE_SECURITY_USER); + + // Exercise cross cluster keys, if viable (i.e., creator has enough privileges and feature flag is enabled) + final boolean createCrossClusterKey = creatorUser.equals(MANAGE_SECURITY_USER) + && TcpTransport.isUntrustedRemoteClusterEnabled() + && randomBoolean(); + if (createCrossClusterKey) { + final Map createApiKeyRequestBody = expiration == null + ? Map.of("name", apiKeyName, "access", Map.of("search", List.of(Map.of("names", List.of("*"))))) + : Map.of("name", apiKeyName, "expiration", expiration, "access", Map.of("search", List.of(Map.of("names", List.of("*"))))); + final var createApiKeyRequest = new Request("POST", "/_security/cross_cluster/api_key"); + createApiKeyRequest.setJsonEntity( + XContentTestUtils.convertToXContent(createApiKeyRequestBody, XContentType.JSON).utf8ToString() + ); + setUserForRequest(createApiKeyRequest, creatorUser); + + final Response createApiKeyResponse = client().performRequest(createApiKeyRequest); + + assertOK(createApiKeyResponse); + final Map createApiKeyResponseMap = responseAsMap(createApiKeyResponse); + return (String) createApiKeyResponseMap.get("id"); + } else { + final Map createApiKeyRequestBody = expiration == null + ? Map.of("name", apiKeyName) + : Map.of("name", apiKeyName, "expiration", expiration); + final var createApiKeyRequest = new Request("POST", "/_security/api_key"); + createApiKeyRequest.setJsonEntity( + XContentTestUtils.convertToXContent(createApiKeyRequestBody, XContentType.JSON).utf8ToString() + ); + setUserForRequest(createApiKeyRequest, creatorUser); + + final Response createApiKeyResponse = client().performRequest(createApiKeyRequest); + + assertOK(createApiKeyResponse); + final Map createApiKeyResponseMap = responseAsMap(createApiKeyResponse); + return (String) createApiKeyResponseMap.get("id"); + } } private void setUserForRequest(Request request, String username) { From 03a5b6139345f941febaf42336e1869427827f6a Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Tue, 8 Aug 2023 11:50:13 +0200 Subject: [PATCH 14/23] Test 403 --- .../xpack/security/apikey/GetApiKeysRestIT.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java index 22f35578ff99a..a27458914d6d3 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java @@ -11,6 +11,7 @@ import org.apache.http.util.EntityUtils; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; +import org.elasticsearch.client.ResponseException; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.core.TimeValue; import org.elasticsearch.test.XContentTestUtils; @@ -36,6 +37,7 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.emptyArray; +import static org.hamcrest.Matchers.equalTo; public class GetApiKeysRestIT extends SecurityOnTrialLicenseRestTestCase { private static final SecureString END_USER_PASSWORD = new SecureString("end-user-password".toCharArray()); @@ -145,6 +147,11 @@ public void testGetApiKeysWithActiveOnlyFlagAndMultipleUsers() throws Exception ).getApiKeyInfos(), emptyArray() ); + var ex = expectThrows( + ResponseException.class, + () -> getApiKeysWithRequestParams(MANAGE_OWN_API_KEY_USER, Map.of("active_only", "true", "owner", "false")) + ); + assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(403)); assertResponseContainsApiKeyIds( getApiKeysWithRequestParams(randomBoolean() ? Map.of() : Map.of("active_only", "false")), manageOwnApiKeyUserApiKeyId, From 8b3188130664e05183ff2ec6e8cd53f6904fdd90 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Tue, 8 Aug 2023 11:58:24 +0200 Subject: [PATCH 15/23] Nit --- .../xpack/security/apikey/GetApiKeysRestIT.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java index a27458914d6d3..a5c22f83df92e 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java @@ -53,10 +53,10 @@ public void createUsers() throws IOException { } public void testGetApiKeysWithActiveOnlyFlag() throws Exception { - final String apiKeyId0 = createApiKey("key-0", MANAGE_SECURITY_USER); - final String apiKeyId1 = createApiKey("key-1", MANAGE_SECURITY_USER); + final String apiKeyId0 = createApiKey(MANAGE_SECURITY_USER, "key-0"); + final String apiKeyId1 = createApiKey(MANAGE_SECURITY_USER, "key-1"); // Set short enough expiration for the API key to be expired by the time we query for it - final String apiKeyId2 = createApiKey("key-2", MANAGE_SECURITY_USER, TimeValue.timeValueNanos(1)); + final String apiKeyId2 = createApiKey(MANAGE_SECURITY_USER, "key-2", TimeValue.timeValueNanos(1)); assertResponseContainsApiKeyIds(getApiKeysWithRequestParams(Map.of("active_only", "true")), apiKeyId0, apiKeyId1); { @@ -84,8 +84,8 @@ public void testGetApiKeysWithActiveOnlyFlag() throws Exception { } public void testGetApiKeysWithActiveOnlyFlagAndMultipleUsers() throws Exception { - final String manageOwnApiKeyUserApiKeyId = createApiKey("key-0", MANAGE_OWN_API_KEY_USER); - final String manageApiKeyUserApiKeyId = createApiKey("key-1", MANAGE_SECURITY_USER); + final String manageOwnApiKeyUserApiKeyId = createApiKey(MANAGE_OWN_API_KEY_USER, "key-0"); + final String manageApiKeyUserApiKeyId = createApiKey(MANAGE_SECURITY_USER, "key-1"); // Two active API keys assertResponseContainsApiKeyIds( @@ -179,11 +179,11 @@ private static XContentParser getParser(Response response) throws IOException { return XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, responseBody); } - private String createApiKey(String apiKeyName, String creatorUser) throws IOException { - return createApiKey(apiKeyName, creatorUser, null); + private String createApiKey(String creatorUser, String apiKeyName) throws IOException { + return createApiKey(creatorUser, apiKeyName, null); } - private String createApiKey(String apiKeyName, String creatorUser, @Nullable TimeValue expiration) throws IOException { + private String createApiKey(String creatorUser, String apiKeyName, @Nullable TimeValue expiration) throws IOException { // Sanity check to ensure API key name and creator name aren't flipped assert creatorUser.equals(MANAGE_OWN_API_KEY_USER) || creatorUser.equals(MANAGE_SECURITY_USER); From 0bba4b5b5da91a2c2f488bee99dfdb4f5f4d0b5d Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Tue, 8 Aug 2023 12:23:56 +0200 Subject: [PATCH 16/23] Update docs/changelog/98259.yaml --- docs/changelog/98259.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/changelog/98259.yaml b/docs/changelog/98259.yaml index 662b1707757f0..359ec0c6c390c 100644 --- a/docs/changelog/98259.yaml +++ b/docs/changelog/98259.yaml @@ -2,4 +2,5 @@ pr: 98259 summary: Support getting active-only API keys via Get API keys API area: Security type: enhancement -issues: [] +issues: + - 97995 From 4b990090eb38007d812b3dbf448c84efb4625bc3 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Tue, 8 Aug 2023 12:27:12 +0200 Subject: [PATCH 17/23] Clearer constant --- .../core/security/action/apikey/GetApiKeyRequest.java | 6 +++--- .../security/action/apikey/GetApiKeyRequestTests.java | 10 ++++++---- 2 files changed, 9 insertions(+), 7 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 18c300d429086..86588a23f71d1 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,7 +25,7 @@ */ public final class GetApiKeyRequest extends ActionRequest { - static TransportVersion TRANSPORT_VERSION_ACTIVE_ONLY = TransportVersion.V_8_500_054; + static TransportVersion API_KEY_ACTIVE_ONLY_PARAM_TRANSPORT_VERSION = TransportVersion.V_8_500_054; private final String realmName; private final String userName; @@ -51,7 +51,7 @@ public GetApiKeyRequest(StreamInput in) throws IOException { } else { withLimitedBy = false; } - if (in.getTransportVersion().onOrAfter(TRANSPORT_VERSION_ACTIVE_ONLY)) { + if (in.getTransportVersion().onOrAfter(API_KEY_ACTIVE_ONLY_PARAM_TRANSPORT_VERSION)) { activeOnly = in.readBoolean(); } else { activeOnly = false; @@ -152,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(TRANSPORT_VERSION_ACTIVE_ONLY)) { + if (out.getTransportVersion().onOrAfter(API_KEY_ACTIVE_ONLY_PARAM_TRANSPORT_VERSION)) { 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 4a5c0a3d46bba..0d46b904a0820 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 @@ -22,7 +22,7 @@ import java.util.function.Supplier; import static org.elasticsearch.test.TransportVersionUtils.randomVersionBetween; -import static org.elasticsearch.xpack.core.security.action.apikey.GetApiKeyRequest.TRANSPORT_VERSION_ACTIVE_ONLY; +import static org.elasticsearch.xpack.core.security.action.apikey.GetApiKeyRequest.API_KEY_ACTIVE_ONLY_PARAM_TRANSPORT_VERSION; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; @@ -185,7 +185,7 @@ public void testSerialization() throws IOException { .build(); ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); - TransportVersion beforeActiveOnly = TransportVersionUtils.getPreviousVersion(TRANSPORT_VERSION_ACTIVE_ONLY); + TransportVersion beforeActiveOnly = TransportVersionUtils.getPreviousVersion(API_KEY_ACTIVE_ONLY_PARAM_TRANSPORT_VERSION); out.setTransportVersion(randomVersionBetween(random(), TransportVersion.V_8_5_0, beforeActiveOnly)); getApiKeyRequest.writeTo(out); @@ -207,12 +207,14 @@ public void testSerialization() throws IOException { .build(); ByteArrayOutputStream outBuffer = new ByteArrayOutputStream(); OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer); - out.setTransportVersion(randomVersionBetween(random(), TRANSPORT_VERSION_ACTIVE_ONLY, TransportVersion.current())); + out.setTransportVersion( + randomVersionBetween(random(), API_KEY_ACTIVE_ONLY_PARAM_TRANSPORT_VERSION, TransportVersion.current()) + ); getApiKeyRequest.writeTo(out); InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray())); inputStreamStreamInput.setTransportVersion( - randomVersionBetween(random(), TRANSPORT_VERSION_ACTIVE_ONLY, TransportVersion.current()) + randomVersionBetween(random(), API_KEY_ACTIVE_ONLY_PARAM_TRANSPORT_VERSION, TransportVersion.current()) ); GetApiKeyRequest requestFromInputStream = new GetApiKeyRequest(inputStreamStreamInput); From 3ffedab68b2da4e439d21b38354940e7cdbe2105 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Tue, 8 Aug 2023 12:41:20 +0200 Subject: [PATCH 18/23] Unit test --- .../xpack/security/authc/ApiKeyServiceTests.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java index e28b40ccde890..a7bc36efbc92e 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java @@ -266,6 +266,8 @@ public void testCreateApiKeyUsesBulkIndexAction() throws Exception { @SuppressWarnings("unchecked") public void testGetApiKeys() throws Exception { + final long now = randomMillisUpToYear9999(); + when(clock.instant()).thenReturn(Instant.ofEpochMilli(now)); final Settings settings = Settings.builder().put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true).build(); when(client.threadPool()).thenReturn(threadPool); SearchRequestBuilder searchRequestBuilder = Mockito.spy(new SearchRequestBuilder(client, SearchAction.INSTANCE)); @@ -283,7 +285,7 @@ public void testGetApiKeys() throws Exception { String apiKeyName = randomFrom(randomAlphaOfLengthBetween(3, 8), null); String[] apiKeyIds = generateRandomStringArray(4, 4, true, true); PlainActionFuture getApiKeyResponsePlainActionFuture = new PlainActionFuture<>(); - final boolean activeOnly = false; + final boolean activeOnly = randomBoolean(); service.getApiKeys(realmNames, username, apiKeyName, apiKeyIds, randomBoolean(), activeOnly, getApiKeyResponsePlainActionFuture); final BoolQueryBuilder boolQuery = QueryBuilders.boolQuery().filter(QueryBuilders.termQuery("doc_type", "api_key")); if (realmNames != null && realmNames.length > 0) { @@ -311,6 +313,13 @@ public void testGetApiKeys() throws Exception { if (apiKeyIds != null && apiKeyIds.length > 0) { boolQuery.filter(QueryBuilders.idsQuery().addIds(apiKeyIds)); } + if (activeOnly) { + boolQuery.filter(QueryBuilders.termQuery("api_key_invalidated", false)); + final BoolQueryBuilder expiredQuery = QueryBuilders.boolQuery(); + expiredQuery.should(QueryBuilders.rangeQuery("expiration_time").gt(now)); + expiredQuery.should(QueryBuilders.boolQuery().mustNot(QueryBuilders.existsQuery("expiration_time"))); + boolQuery.filter(expiredQuery); + } verify(searchRequestBuilder).setQuery(eq(boolQuery)); verify(searchRequestBuilder).setFetchSource(eq(true)); assertThat(searchRequest.get().source().query(), is(boolQuery)); From 75a2e87ea2f2e63f57362b98b724b6e7fb536cc9 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Tue, 8 Aug 2023 12:54:43 +0200 Subject: [PATCH 19/23] Log message --- .../elasticsearch/xpack/security/authc/ApiKeyService.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java index 79bbfefe518a6..31172146e94f2 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java @@ -1908,11 +1908,12 @@ public void getApiKeys( ActionListener.wrap(apiKeyInfos -> { if (apiKeyInfos.isEmpty()) { logger.debug( - "No api keys found for realms {}, user [{}], api key name [{}] and api key ids {}", + "No api keys found for realms {}, user [{}], api key name [{}], api key ids {}, and active_only flag [{}]", Arrays.toString(realmNames), username, apiKeyName, - Arrays.toString(apiKeyIds) + Arrays.toString(apiKeyIds), + activeOnly ); listener.onResponse(GetApiKeyResponse.emptyResponse()); } else { From 22acfcbc186ab2929f778b428139fec5fdffc103 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Tue, 8 Aug 2023 13:45:23 +0200 Subject: [PATCH 20/23] WIP --- .../action/apikey/GetApiKeyRequest.java | 6 ---- .../action/apikey/GetApiKeyRequestTests.java | 26 +++++---------- .../security/apikey/GetApiKeysRestIT.java | 33 ++++++++++++++++--- 3 files changed, 37 insertions(+), 28 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 86588a23f71d1..ebf36bdcdc421 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 @@ -118,12 +118,6 @@ 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)) { 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 0d46b904a0820..1001cd4863f5d 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 @@ -62,7 +62,6 @@ class Dummy extends ActionRequest { String apiKeyId; String apiKeyName; boolean ownedByAuthenticatedUser; - boolean activeOnly; Dummy(String[] a) { realm = a[0]; @@ -70,7 +69,6 @@ class Dummy extends ActionRequest { apiKeyId = a[2]; apiKeyName = a[3]; ownedByAuthenticatedUser = Boolean.parseBoolean(a[4]); - activeOnly = Boolean.parseBoolean(a[5]); } @Override @@ -87,30 +85,24 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(apiKeyName); out.writeOptionalBoolean(ownedByAuthenticatedUser); out.writeBoolean(randomBoolean()); - out.writeBoolean(activeOnly); + out.writeBoolean(randomBoolean()); } } String[][] inputs = new String[][] { - { randomNullOrEmptyString(), "user", "api-kid", "api-kname", "false", "true" }, - { "realm", randomNullOrEmptyString(), "api-kid", "api-kname", "false", "true" }, - { randomNullOrEmptyString(), randomNullOrEmptyString(), randomNullOrEmptyString(), "api-kname", "false", "true" }, - { randomNullOrEmptyString(), randomNullOrEmptyString(), "api-kid", randomNullOrEmptyString(), "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" } }; + { 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[][] { { "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", - "active_only must not be [true] 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", - "active_only must not be [true] when the api key id or api key name is specified" }, - { "active_only must not be [true] when the api key id or api key name is specified" }, - { "active_only must not be [true] 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" }, { "neither username nor realm-name may be specified when retrieving owned API keys" }, diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java index a5c22f83df92e..4fb56b6cc6e01 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java @@ -32,7 +32,6 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; - import javax.annotation.Nullable; import static org.hamcrest.Matchers.containsInAnyOrder; @@ -58,7 +57,7 @@ public void testGetApiKeysWithActiveOnlyFlag() throws Exception { // Set short enough expiration for the API key to be expired by the time we query for it final String apiKeyId2 = createApiKey(MANAGE_SECURITY_USER, "key-2", TimeValue.timeValueNanos(1)); - assertResponseContainsApiKeyIds(getApiKeysWithRequestParams(Map.of("active_only", "true")), apiKeyId0, apiKeyId1); + // All API keys returned when flag false (implicitly or explicitly) { final Map parameters = new HashMap<>(); if (randomBoolean()) { @@ -67,9 +66,17 @@ public void testGetApiKeysWithActiveOnlyFlag() throws Exception { assertResponseContainsApiKeyIds(getApiKeysWithRequestParams(parameters), apiKeyId0, apiKeyId1, apiKeyId2); } - getSecurityClient().invalidateApiKeys(apiKeyId0); + // Only active keys returned when flag true + assertResponseContainsApiKeyIds(getApiKeysWithRequestParams(Map.of("active_only", "true")), apiKeyId0, apiKeyId1); + // Also works with `name` filter + assertResponseContainsApiKeyIds( + getApiKeysWithRequestParams(Map.of("active_only", "true", "name", randomFrom("*", "key-*"))), + apiKeyId0, + apiKeyId1 + ); - assertResponseContainsApiKeyIds(getApiKeysWithRequestParams(Map.of("active_only", "true")), apiKeyId1); + // Same applies to invalidated key + getSecurityClient().invalidateApiKeys(apiKeyId0); { final Map parameters = new HashMap<>(); if (randomBoolean()) { @@ -77,10 +84,23 @@ public void testGetApiKeysWithActiveOnlyFlag() throws Exception { } assertResponseContainsApiKeyIds(getApiKeysWithRequestParams(parameters), apiKeyId0, apiKeyId1, apiKeyId2); } + assertResponseContainsApiKeyIds(getApiKeysWithRequestParams(Map.of("active_only", "true")), apiKeyId1); + // also works with name filter + assertResponseContainsApiKeyIds( + getApiKeysWithRequestParams(Map.of("active_only", "true", "name", randomFrom("*", "key-*", "key-1"))), + apiKeyId1 + ); + // We get an empty result when no API keys active getSecurityClient().invalidateApiKeys(apiKeyId1); - assertThat(getApiKeysWithRequestParams(Map.of("active_only", "true")).getApiKeyInfos(), emptyArray()); + + // Using together with id parameter, returns 404 for inactive key + var ex = expectThrows( + ResponseException.class, + () -> getApiKeysWithRequestParams(Map.of("active_only", "true", "id", randomFrom(apiKeyId0, apiKeyId1, apiKeyId2))) + ); + assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(404)); } public void testGetApiKeysWithActiveOnlyFlagAndMultipleUsers() throws Exception { @@ -183,6 +203,9 @@ private String createApiKey(String creatorUser, String apiKeyName) throws IOExce return createApiKey(creatorUser, apiKeyName, null); } + /** + * Returns id of created API key. + */ private String createApiKey(String creatorUser, String apiKeyName, @Nullable TimeValue expiration) throws IOException { // Sanity check to ensure API key name and creator name aren't flipped assert creatorUser.equals(MANAGE_OWN_API_KEY_USER) || creatorUser.equals(MANAGE_SECURITY_USER); From f2522d63f9e20b881edd0b2b495c1d79cb74d97a Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Tue, 8 Aug 2023 14:02:10 +0200 Subject: [PATCH 21/23] Spotless --- .../elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java index 4fb56b6cc6e01..d6d30830568da 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java @@ -32,6 +32,7 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; + import javax.annotation.Nullable; import static org.hamcrest.Matchers.containsInAnyOrder; From a325cd7d85ecaca61e74f071751210808f2ac7ab Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Tue, 8 Aug 2023 17:44:41 +0200 Subject: [PATCH 22/23] Realm filter --- .../security/apikey/GetApiKeysRestIT.java | 41 +++++++++++++------ 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java index d6d30830568da..a71c4e342a9ce 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/GetApiKeysRestIT.java @@ -75,6 +75,12 @@ public void testGetApiKeysWithActiveOnlyFlag() throws Exception { apiKeyId0, apiKeyId1 ); + // Also works with `realm_name` filter + assertResponseContainsApiKeyIds( + getApiKeysWithRequestParams(Map.of("active_only", "true", "realm_name", "default_native")), + apiKeyId0, + apiKeyId1 + ); // Same applies to invalidated key getSecurityClient().invalidateApiKeys(apiKeyId0); @@ -96,24 +102,36 @@ public void testGetApiKeysWithActiveOnlyFlag() throws Exception { getSecurityClient().invalidateApiKeys(apiKeyId1); assertThat(getApiKeysWithRequestParams(Map.of("active_only", "true")).getApiKeyInfos(), emptyArray()); - // Using together with id parameter, returns 404 for inactive key - var ex = expectThrows( - ResponseException.class, - () -> getApiKeysWithRequestParams(Map.of("active_only", "true", "id", randomFrom(apiKeyId0, apiKeyId1, apiKeyId2))) - ); - assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(404)); + { + // Using together with id parameter, returns 404 for inactive key + var ex = expectThrows( + ResponseException.class, + () -> getApiKeysWithRequestParams(Map.of("active_only", "true", "id", randomFrom(apiKeyId0, apiKeyId1, apiKeyId2))) + ); + assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(404)); + } + + { + // manage_own_api_key prohibits owner=false, even if active_only is set + var ex = expectThrows( + ResponseException.class, + () -> getApiKeysWithRequestParams(MANAGE_OWN_API_KEY_USER, Map.of("active_only", "true", "owner", "false")) + ); + assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(403)); + } } public void testGetApiKeysWithActiveOnlyFlagAndMultipleUsers() throws Exception { final String manageOwnApiKeyUserApiKeyId = createApiKey(MANAGE_OWN_API_KEY_USER, "key-0"); final String manageApiKeyUserApiKeyId = createApiKey(MANAGE_SECURITY_USER, "key-1"); - // Two active API keys + // Both users' API keys are returned assertResponseContainsApiKeyIds( getApiKeysWithRequestParams(Map.of("active_only", Boolean.toString(randomBoolean()))), manageOwnApiKeyUserApiKeyId, manageApiKeyUserApiKeyId ); + // Filtering by username works (also via owner flag) assertResponseContainsApiKeyIds( getApiKeysWithRequestParams(Map.of("active_only", Boolean.toString(randomBoolean()), "username", MANAGE_SECURITY_USER)), manageApiKeyUserApiKeyId @@ -131,9 +149,10 @@ public void testGetApiKeysWithActiveOnlyFlagAndMultipleUsers() throws Exception manageOwnApiKeyUserApiKeyId ); - // One active API key + // One user's API key is active invalidateApiKeysForUser(MANAGE_OWN_API_KEY_USER); + // Filtering by username still works (also via owner flag) assertResponseContainsApiKeyIds(getApiKeysWithRequestParams(Map.of("active_only", "true")), manageApiKeyUserApiKeyId); assertResponseContainsApiKeyIds( getApiKeysWithRequestParams(Map.of("active_only", "true", "username", MANAGE_SECURITY_USER)), @@ -168,11 +187,7 @@ public void testGetApiKeysWithActiveOnlyFlagAndMultipleUsers() throws Exception ).getApiKeyInfos(), emptyArray() ); - var ex = expectThrows( - ResponseException.class, - () -> getApiKeysWithRequestParams(MANAGE_OWN_API_KEY_USER, Map.of("active_only", "true", "owner", "false")) - ); - assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(403)); + // With flag set to false, we get both inactive keys assertResponseContainsApiKeyIds( getApiKeysWithRequestParams(randomBoolean() ? Map.of() : Map.of("active_only", "false")), manageOwnApiKeyUserApiKeyId, From 21aae8a79d550617ba1b4c5fe691995abda69a2f Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Wed, 9 Aug 2023 10:38:34 +0200 Subject: [PATCH 23/23] Upper case --- .../org/elasticsearch/xpack/security/authc/ApiKeyService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java index 31172146e94f2..3753e8d81ef69 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java @@ -1908,7 +1908,7 @@ public void getApiKeys( ActionListener.wrap(apiKeyInfos -> { if (apiKeyInfos.isEmpty()) { logger.debug( - "No api keys found for realms {}, user [{}], api key name [{}], api key ids {}, and active_only flag [{}]", + "No API keys found for realms {}, user [{}], API key name [{}], API key IDs {}, and active_only flag [{}]", Arrays.toString(realmNames), username, apiKeyName,