From ee1acffa053dfbd4ab96e9806766f429b225d228 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Fri, 5 Apr 2024 09:50:30 +0300 Subject: [PATCH] Query API Key Information API support for the `typed_keys` request parameter (#106873) (#107110) The typed_keys request parameter is the canonical parameter, that's also used in the regular index _search enpoint, in order to return the types of aggregations in the response. This is required by typed language clients of the _security/_query/api_key endpoint that are using aggregations. Backport of #106873 --- docs/changelog/106873.yaml | 6 + .../rest-api/security/query-api-key.asciidoc | 4 + docs/reference/search/search.asciidoc | 4 +- .../api/security.query_api_keys.json | 5 + .../xpack/security/ApiKeyAggsIT.java | 136 +++++++++++------- .../action/apikey/RestQueryApiKeyAction.java | 7 + 6 files changed, 105 insertions(+), 57 deletions(-) create mode 100644 docs/changelog/106873.yaml diff --git a/docs/changelog/106873.yaml b/docs/changelog/106873.yaml new file mode 100644 index 0000000000000..f823caff7aefe --- /dev/null +++ b/docs/changelog/106873.yaml @@ -0,0 +1,6 @@ +pr: 106873 +summary: Query API Key Information API support for the `typed_keys` request parameter +area: Security +type: enhancement +issues: + - 106817 diff --git a/docs/reference/rest-api/security/query-api-key.asciidoc b/docs/reference/rest-api/security/query-api-key.asciidoc index 88fef9a21ff88..eb3353259d8b9 100644 --- a/docs/reference/rest-api/security/query-api-key.asciidoc +++ b/docs/reference/rest-api/security/query-api-key.asciidoc @@ -154,6 +154,10 @@ its <> and the owner user's (effectively limited by it). An API key cannot retrieve any API key's limited-by role descriptors (including itself) unless it has `manage_api_key` or higher privileges. +`typed_keys`:: +(Optional, Boolean) If `true`, aggregation names are prefixed by their respective types in the response. +Defaults to `false`. + [[security-api-query-api-key-request-body]] ==== {api-request-body-title} diff --git a/docs/reference/search/search.asciidoc b/docs/reference/search/search.asciidoc index cd7d496d89d7b..543114c74ff56 100644 --- a/docs/reference/search/search.asciidoc +++ b/docs/reference/search/search.asciidoc @@ -341,8 +341,8 @@ If `true`, the exact number of hits is returned at the cost of some performance. If `false`, the response does not include the total number of hits matching the query. `typed_keys`:: -(Optional, Boolean) If `true`, aggregation and suggester names are be prefixed -by their respective types in the response. Defaults to `true`. +(Optional, Boolean) If `true`, aggregation and suggester names are prefixed +by their respective types in the response. Defaults to `false`. `version`:: (Optional, Boolean) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/security.query_api_keys.json b/rest-api-spec/src/main/resources/rest-api-spec/api/security.query_api_keys.json index 8079059ea164b..d69998d5a5ade 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/security.query_api_keys.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/security.query_api_keys.json @@ -26,6 +26,11 @@ "type":"boolean", "default":false, "description": "flag to show the limited-by role descriptors of API Keys" + }, + "typed_keys":{ + "type":"boolean", + "default":false, + "description": "flag to prefix aggregation names by their respective types in the response" } }, "body":{ diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/ApiKeyAggsIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/ApiKeyAggsIT.java index f4fa304f9c1e2..427d918fd64d5 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/ApiKeyAggsIT.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/ApiKeyAggsIT.java @@ -65,7 +65,8 @@ public void testFiltersAggs() throws IOException { ), API_KEY_USER_AUTH_HEADER ); - assertAggs(API_KEY_ADMIN_AUTH_HEADER, """ + final boolean typedAggs = randomBoolean(); + assertAggs(API_KEY_ADMIN_AUTH_HEADER, typedAggs, """ { "aggs": { "hostnames": { @@ -79,22 +80,23 @@ public void testFiltersAggs() throws IOException { } } """, aggs -> { - assertThat(((Map) ((Map) aggs.get("hostnames")).get("buckets")).size(), is(2)); + String aggName = typedAggs ? "filters#hostnames" : "hostnames"; + assertThat(((Map) ((Map) aggs.get(aggName)).get("buckets")).size(), is(2)); assertThat( - ((Map) ((Map) ((Map) aggs.get("hostnames")).get("buckets")).get( + ((Map) ((Map) ((Map) aggs.get(aggName)).get("buckets")).get( "my-org-host-1" )).get("doc_count"), is(2) ); assertThat( - ((Map) ((Map) ((Map) aggs.get("hostnames")).get("buckets")).get( + ((Map) ((Map) ((Map) aggs.get(aggName)).get("buckets")).get( "my-org-host-2" )).get("doc_count"), is(2) ); }); // other bucket - assertAggs(API_KEY_USER_AUTH_HEADER, """ + assertAggs(API_KEY_USER_AUTH_HEADER, typedAggs, """ { "aggs": { "only_user_keys": { @@ -108,22 +110,23 @@ public void testFiltersAggs() throws IOException { } } """, aggs -> { - assertThat(((Map) ((Map) aggs.get("only_user_keys")).get("buckets")).size(), is(2)); + String aggName = typedAggs ? "filters#only_user_keys" : "only_user_keys"; + assertThat(((Map) ((Map) aggs.get(aggName)).get("buckets")).size(), is(2)); assertThat( - ((Map) ((Map) ((Map) aggs.get("only_user_keys")).get("buckets")).get( + ((Map) ((Map) ((Map) aggs.get(aggName)).get("buckets")).get( "only_key4_match" )).get("doc_count"), is(1) ); assertThat( - ((Map) ((Map) ((Map) aggs.get("only_user_keys")).get("buckets")).get( + ((Map) ((Map) ((Map) aggs.get(aggName)).get("buckets")).get( "other_user_keys" )).get("doc_count"), is(1) ); }); // anonymous filters - assertAggs(API_KEY_USER_AUTH_HEADER, """ + assertAggs(API_KEY_USER_AUTH_HEADER, typedAggs, """ { "aggs": { "all_user_keys": { @@ -139,27 +142,28 @@ public void testFiltersAggs() throws IOException { } } """, aggs -> { - assertThat(((List>) ((Map) aggs.get("all_user_keys")).get("buckets")).size(), is(4)); + String aggName = typedAggs ? "filters#all_user_keys" : "all_user_keys"; + assertThat(((List>) ((Map) aggs.get(aggName)).get("buckets")).size(), is(4)); assertThat( - ((List>) ((Map) aggs.get("all_user_keys")).get("buckets")).get(0).get("doc_count"), + ((List>) ((Map) aggs.get(aggName)).get("buckets")).get(0).get("doc_count"), is(2) ); assertThat( - ((List>) ((Map) aggs.get("all_user_keys")).get("buckets")).get(1).get("doc_count"), + ((List>) ((Map) aggs.get(aggName)).get("buckets")).get(1).get("doc_count"), is(2) ); assertThat( - ((List>) ((Map) aggs.get("all_user_keys")).get("buckets")).get(2).get("doc_count"), + ((List>) ((Map) aggs.get(aggName)).get("buckets")).get(2).get("doc_count"), is(2) ); // the "other" bucket assertThat( - ((List>) ((Map) aggs.get("all_user_keys")).get("buckets")).get(3).get("doc_count"), + ((List>) ((Map) aggs.get(aggName)).get("buckets")).get(3).get("doc_count"), is(0) ); }); // nested filters - assertAggs(API_KEY_USER_AUTH_HEADER, """ + assertAggs(API_KEY_USER_AUTH_HEADER, typedAggs, """ { "aggs": { "level1": { @@ -184,36 +188,44 @@ public void testFiltersAggs() throws IOException { } } """, aggs -> { - List> level1Buckets = (List>) ((Map) aggs.get("level1")).get("buckets"); + String level1AggName = typedAggs ? "filters#level1" : "level1"; + List> level1Buckets = (List>) ((Map) aggs.get(level1AggName)).get( + "buckets" + ); assertThat(level1Buckets.size(), is(2)); assertThat(level1Buckets.get(0).get("doc_count"), is(2)); assertThat(level1Buckets.get(0).get("key"), is("rest-filter")); + String level2AggName = typedAggs ? "filters#level2" : "level2"; assertThat( - ((Map) ((Map) ((Map) level1Buckets.get(0).get("level2")).get("buckets")) - .get("invalidated")).get("doc_count"), + ((Map) ((Map) ((Map) level1Buckets.get(0).get(level2AggName)).get( + "buckets" + )).get("invalidated")).get("doc_count"), is(0) ); assertThat( - ((Map) ((Map) ((Map) level1Buckets.get(0).get("level2")).get("buckets")) - .get("not-invalidated")).get("doc_count"), + ((Map) ((Map) ((Map) level1Buckets.get(0).get(level2AggName)).get( + "buckets" + )).get("not-invalidated")).get("doc_count"), is(2) ); assertThat(level1Buckets.get(1).get("doc_count"), is(2)); assertThat(level1Buckets.get(1).get("key"), is("user-filter")); assertThat( - ((Map) ((Map) ((Map) level1Buckets.get(1).get("level2")).get("buckets")) - .get("invalidated")).get("doc_count"), + ((Map) ((Map) ((Map) level1Buckets.get(1).get(level2AggName)).get( + "buckets" + )).get("invalidated")).get("doc_count"), is(0) ); assertThat( - ((Map) ((Map) ((Map) level1Buckets.get(1).get("level2")).get("buckets")) - .get("not-invalidated")).get("doc_count"), + ((Map) ((Map) ((Map) level1Buckets.get(1).get(level2AggName)).get( + "buckets" + )).get("not-invalidated")).get("doc_count"), is(2) ); }); // filter on disallowed fields { - Request request = new Request("GET", "/_security/_query/api_key"); + Request request = new Request("GET", "/_security/_query/api_key" + (randomBoolean() ? "?typed_keys" : "")); request.setOptions( request.getOptions() .toBuilder() @@ -240,7 +252,7 @@ public void testFiltersAggs() throws IOException { ); } { - Request request = new Request("GET", "/_security/_query/api_key"); + Request request = new Request("GET", "/_security/_query/api_key" + (randomBoolean() ? "?typed_keys" : "")); request.setOptions( request.getOptions() .toBuilder() @@ -310,7 +322,8 @@ public void testAggsForType() throws IOException { updateApiKeys(systemWriteCreds, "ctx._source['type']='cross_cluster';", crossApiKeyIds); boolean isAdmin = randomBoolean(); - assertAggs(isAdmin ? API_KEY_ADMIN_AUTH_HEADER : API_KEY_USER_AUTH_HEADER, """ + final boolean typedAggs = randomBoolean(); + assertAggs(isAdmin ? API_KEY_ADMIN_AUTH_HEADER : API_KEY_USER_AUTH_HEADER, typedAggs, """ { "size": 0, "aggs": { @@ -324,9 +337,8 @@ public void testAggsForType() throws IOException { } } """, aggs -> { - List> buckets = (List>) ((Map) aggs.get("all_keys_by_type")).get( - "buckets" - ); + String aggName = typedAggs ? "composite#all_keys_by_type" : "all_keys_by_type"; + List> buckets = (List>) ((Map) aggs.get(aggName)).get("buckets"); assertThat(buckets.size(), is(3)); assertThat(((Map) buckets.get(0).get("key")).get("type"), is("cross_cluster")); assertThat(((Map) buckets.get(1).get("key")).get("type"), is("other")); @@ -342,7 +354,7 @@ public void testAggsForType() throws IOException { } }); - assertAggs(isAdmin ? API_KEY_ADMIN_AUTH_HEADER : API_KEY_USER_AUTH_HEADER, """ + assertAggs(isAdmin ? API_KEY_ADMIN_AUTH_HEADER : API_KEY_USER_AUTH_HEADER, typedAggs, """ { "size": 0, "aggs": { @@ -371,23 +383,23 @@ public void testAggsForType() throws IOException { """, aggs -> { assertThat(aggs.size(), is(4)); // 3 types - assertThat(((Map) aggs.get("type_cardinality")).get("value"), is(3)); + assertThat(((Map) aggs.get((typedAggs ? "cardinality#" : "") + "type_cardinality")).get("value"), is(3)); if (isAdmin) { // 8 keys - assertThat(((Map) aggs.get("type_value_count")).get("value"), is(8)); + assertThat(((Map) aggs.get((typedAggs ? "value_count#" : "") + "type_value_count")).get("value"), is(8)); } else { // 4 keys - assertThat(((Map) aggs.get("type_value_count")).get("value"), is(4)); + assertThat(((Map) aggs.get((typedAggs ? "value_count#" : "") + "type_value_count")).get("value"), is(4)); } - assertThat(((Map) aggs.get("missing_type_count")).get("doc_count"), is(0)); - List> typeTermsBuckets = (List>) ((Map) aggs.get("type_terms")).get( - "buckets" - ); + assertThat(((Map) aggs.get((typedAggs ? "missing#" : "") + "missing_type_count")).get("doc_count"), is(0)); + List> typeTermsBuckets = (List>) ((Map) aggs.get( + (typedAggs ? "sterms#" : "") + "type_terms" + )).get("buckets"); assertThat(typeTermsBuckets.size(), is(3)); }); // runtime type field is disallowed { - Request request = new Request("GET", "/_security/_query/api_key"); + Request request = new Request("GET", "/_security/_query/api_key" + (typedAggs ? "?typed_keys" : "")); request.setOptions( request.getOptions() .toBuilder() @@ -432,7 +444,8 @@ public void testFilterAggs() throws IOException { invalidateApiKey(key2User1KeyId, false, API_KEY_ADMIN_AUTH_HEADER); invalidateApiKey(key1User3KeyId, false, API_KEY_ADMIN_AUTH_HEADER); - assertAggs(API_KEY_ADMIN_AUTH_HEADER, """ + final boolean typedAggs = randomBoolean(); + assertAggs(API_KEY_ADMIN_AUTH_HEADER, typedAggs, """ { "size": 0, "aggs": { @@ -451,10 +464,11 @@ public void testFilterAggs() throws IOException { } } """, aggs -> { - assertThat(((Map) aggs.get("not_invalidated")).get("doc_count"), is(4)); // 6 - 2 (invalidated) + // 6 - 2 (invalidated) + assertThat(((Map) aggs.get(typedAggs ? "filter#not_invalidated" : "not_invalidated")).get("doc_count"), is(4)); List> buckets = (List>) ((Map) ((Map) aggs.get( - "not_invalidated" - )).get("keys_by_username")).get("buckets"); + typedAggs ? "filter#not_invalidated" : "not_invalidated" + )).get(typedAggs ? "composite#keys_by_username" : "keys_by_username")).get("buckets"); assertThat(buckets.size(), is(3)); assertThat(((Map) buckets.get(0).get("key")).get("usernames"), is("test-user-1")); assertThat(buckets.get(0).get("doc_count"), is(1)); @@ -464,7 +478,7 @@ public void testFilterAggs() throws IOException { assertThat(buckets.get(2).get("doc_count"), is(1)); }); - assertAggs(API_KEY_ADMIN_AUTH_HEADER, """ + assertAggs(API_KEY_ADMIN_AUTH_HEADER, typedAggs, """ { "aggs": { "keys_by_username": { @@ -488,23 +502,32 @@ public void testFilterAggs() throws IOException { } } """, aggs -> { - List> buckets = (List>) ((Map) aggs.get("keys_by_username")).get( - "buckets" - ); + List> buckets = (List>) ((Map) aggs.get( + typedAggs ? "composite#keys_by_username" : "keys_by_username" + )).get("buckets"); assertThat(buckets.size(), is(3)); assertThat(buckets.get(0).get("doc_count"), is(2)); assertThat(((Map) buckets.get(0).get("key")).get("usernames"), is("test-user-1")); - assertThat(((Map) buckets.get(0).get("not_expired")).get("doc_count"), is(0)); + assertThat( + ((Map) buckets.get(0).get(typedAggs ? "filter#not_expired" : "not_expired")).get("doc_count"), + is(0) + ); assertThat(buckets.get(1).get("doc_count"), is(2)); assertThat(((Map) buckets.get(1).get("key")).get("usernames"), is("test-user-2")); - assertThat(((Map) buckets.get(1).get("not_expired")).get("doc_count"), is(1)); + assertThat( + ((Map) buckets.get(1).get(typedAggs ? "filter#not_expired" : "not_expired")).get("doc_count"), + is(1) + ); assertThat(buckets.get(2).get("doc_count"), is(2)); assertThat(((Map) buckets.get(2).get("key")).get("usernames"), is("test-user-3")); - assertThat(((Map) buckets.get(2).get("not_expired")).get("doc_count"), is(2)); + assertThat( + ((Map) buckets.get(2).get(typedAggs ? "filter#not_expired" : "not_expired")).get("doc_count"), + is(2) + ); }); // "creator" field is disallowed { - Request request = new Request("GET", "/_security/_query/api_key"); + Request request = new Request("GET", "/_security/_query/api_key" + (typedAggs ? "?typed_keys" : "?typed_keys=false")); request.setOptions( request.getOptions() .toBuilder() @@ -533,7 +556,7 @@ public void testFilterAggs() throws IOException { public void testDisallowedAggTypes() { // global aggregation type MUST never be allowed in order to not expose non-owned non-API key docs { - Request request = new Request("GET", "/_security/_query/api_key"); + Request request = new Request("GET", "/_security/_query/api_key" + (randomBoolean() ? "?typed_keys=true" : "")); request.setOptions( request.getOptions() .toBuilder() @@ -559,7 +582,7 @@ public void testDisallowedAggTypes() { } // pipeline aggs are not allowed but could be if there's an identified use-case { - Request request = new Request("GET", "/_security/_query/api_key"); + Request request = new Request("GET", "/_security/_query/api_key" + (randomBoolean() ? "?typed_keys=true" : "")); request.setOptions( request.getOptions() .toBuilder() @@ -587,8 +610,11 @@ public void testDisallowedAggTypes() { } } - void assertAggs(String authHeader, String body, Consumer> aggsVerifier) throws IOException { - final Request request = new Request("GET", "/_security/_query/api_key"); + void assertAggs(String authHeader, boolean typedAggs, String body, Consumer> aggsVerifier) throws IOException { + final Request request = new Request( + "GET", + "/_security/_query/api_key" + (typedAggs ? randomFrom("?typed_keys", "?typed_keys=true") : randomFrom("", "?typed_keys=false")) + ); request.setJsonEntity(body); request.setOptions(request.getOptions().toBuilder().addHeader(HttpHeaders.AUTHORIZATION, authHeader)); final Response response = client().performRequest(request); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyAction.java index c0b85f7beaf50..3d0d1787002a0 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyAction.java @@ -17,6 +17,7 @@ import org.elasticsearch.rest.Scope; import org.elasticsearch.rest.ServerlessScope; import org.elasticsearch.rest.action.RestToXContentListener; +import org.elasticsearch.rest.action.search.RestSearchAction; import org.elasticsearch.search.aggregations.AggregatorFactories; import org.elasticsearch.search.searchafter.SearchAfterBuilder; import org.elasticsearch.search.sort.FieldSortBuilder; @@ -29,6 +30,7 @@ import java.io.IOException; import java.util.List; +import java.util.Set; import static org.elasticsearch.index.query.AbstractQueryBuilder.parseTopLevelQuery; import static org.elasticsearch.rest.RestRequest.Method.GET; @@ -99,6 +101,11 @@ public String getName() { return "xpack_security_query_api_key"; } + @Override + protected Set responseParams() { + return Set.of(RestSearchAction.TYPED_KEYS_PARAM); + } + @Override protected RestChannelConsumer innerPrepareRequest(final RestRequest request, final NodeClient client) throws IOException { final boolean withLimitedBy = request.paramAsBoolean("with_limited_by", false);