Browse Source

Query API Key Information API support for the `typed_keys` request parameter (#106873)

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.

Closes #106817
Albert Zaharovits 1 year ago
parent
commit
b4938e1645

+ 6 - 0
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

+ 4 - 0
docs/reference/rest-api/security/query-api-key.asciidoc

@@ -159,6 +159,10 @@ its <<api-key-role-descriptors,assigned role descriptors>> and the owner user's
 If it exists, the profile uid is returned under the `profile_uid` response field for each API key.
 Defaults to `false`.
 
+`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}
 

+ 2 - 2
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)

+ 5 - 0
rest-api-spec/src/main/resources/rest-api-spec/api/security.query_api_keys.json

@@ -31,6 +31,11 @@
         "type":"boolean",
         "default":false,
         "description": "flag to also retrieve the API Key's owner profile uid, if it exists"
+      },
+      "typed_keys":{
+        "type":"boolean",
+        "default":false,
+        "description": "flag to prefix aggregation names by their respective types in the response"
       }
     },
     "body":{

+ 81 - 55
x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/ApiKeyAggsIT.java

@@ -65,7 +65,8 @@ public class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
             ),
             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 class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
               }
             }
             """, aggs -> {
-            assertThat(((Map<String, Object>) ((Map<String, Object>) aggs.get("hostnames")).get("buckets")).size(), is(2));
+            String aggName = typedAggs ? "filters#hostnames" : "hostnames";
+            assertThat(((Map<String, Object>) ((Map<String, Object>) aggs.get(aggName)).get("buckets")).size(), is(2));
             assertThat(
-                ((Map<String, Object>) ((Map<String, Object>) ((Map<String, Object>) aggs.get("hostnames")).get("buckets")).get(
+                ((Map<String, Object>) ((Map<String, Object>) ((Map<String, Object>) aggs.get(aggName)).get("buckets")).get(
                     "my-org-host-1"
                 )).get("doc_count"),
                 is(2)
             );
             assertThat(
-                ((Map<String, Object>) ((Map<String, Object>) ((Map<String, Object>) aggs.get("hostnames")).get("buckets")).get(
+                ((Map<String, Object>) ((Map<String, Object>) ((Map<String, Object>) 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 class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
               }
             }
             """, aggs -> {
-            assertThat(((Map<String, Object>) ((Map<String, Object>) aggs.get("only_user_keys")).get("buckets")).size(), is(2));
+            String aggName = typedAggs ? "filters#only_user_keys" : "only_user_keys";
+            assertThat(((Map<String, Object>) ((Map<String, Object>) aggs.get(aggName)).get("buckets")).size(), is(2));
             assertThat(
-                ((Map<String, Object>) ((Map<String, Object>) ((Map<String, Object>) aggs.get("only_user_keys")).get("buckets")).get(
+                ((Map<String, Object>) ((Map<String, Object>) ((Map<String, Object>) aggs.get(aggName)).get("buckets")).get(
                     "only_key4_match"
                 )).get("doc_count"),
                 is(1)
             );
             assertThat(
-                ((Map<String, Object>) ((Map<String, Object>) ((Map<String, Object>) aggs.get("only_user_keys")).get("buckets")).get(
+                ((Map<String, Object>) ((Map<String, Object>) ((Map<String, Object>) 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 class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
               }
             }
             """, aggs -> {
-            assertThat(((List<Map<String, Object>>) ((Map<String, Object>) aggs.get("all_user_keys")).get("buckets")).size(), is(4));
+            String aggName = typedAggs ? "filters#all_user_keys" : "all_user_keys";
+            assertThat(((List<Map<String, Object>>) ((Map<String, Object>) aggs.get(aggName)).get("buckets")).size(), is(4));
             assertThat(
-                ((List<Map<String, Object>>) ((Map<String, Object>) aggs.get("all_user_keys")).get("buckets")).get(0).get("doc_count"),
+                ((List<Map<String, Object>>) ((Map<String, Object>) aggs.get(aggName)).get("buckets")).get(0).get("doc_count"),
                 is(2)
             );
             assertThat(
-                ((List<Map<String, Object>>) ((Map<String, Object>) aggs.get("all_user_keys")).get("buckets")).get(1).get("doc_count"),
+                ((List<Map<String, Object>>) ((Map<String, Object>) aggs.get(aggName)).get("buckets")).get(1).get("doc_count"),
                 is(2)
             );
             assertThat(
-                ((List<Map<String, Object>>) ((Map<String, Object>) aggs.get("all_user_keys")).get("buckets")).get(2).get("doc_count"),
+                ((List<Map<String, Object>>) ((Map<String, Object>) aggs.get(aggName)).get("buckets")).get(2).get("doc_count"),
                 is(2)
             );
             // the "other" bucket
             assertThat(
-                ((List<Map<String, Object>>) ((Map<String, Object>) aggs.get("all_user_keys")).get("buckets")).get(3).get("doc_count"),
+                ((List<Map<String, Object>>) ((Map<String, Object>) 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 class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
               }
             }
             """, aggs -> {
-            List<Map<String, Object>> level1Buckets = (List<Map<String, Object>>) ((Map<String, Object>) aggs.get("level1")).get("buckets");
+            String level1AggName = typedAggs ? "filters#level1" : "level1";
+            List<Map<String, Object>> level1Buckets = (List<Map<String, Object>>) ((Map<String, Object>) 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<String, Object>) ((Map<String, Object>) ((Map<String, Object>) level1Buckets.get(0).get("level2")).get("buckets"))
-                    .get("invalidated")).get("doc_count"),
+                ((Map<String, Object>) ((Map<String, Object>) ((Map<String, Object>) level1Buckets.get(0).get(level2AggName)).get(
+                    "buckets"
+                )).get("invalidated")).get("doc_count"),
                 is(0)
             );
             assertThat(
-                ((Map<String, Object>) ((Map<String, Object>) ((Map<String, Object>) level1Buckets.get(0).get("level2")).get("buckets"))
-                    .get("not-invalidated")).get("doc_count"),
+                ((Map<String, Object>) ((Map<String, Object>) ((Map<String, Object>) 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<String, Object>) ((Map<String, Object>) ((Map<String, Object>) level1Buckets.get(1).get("level2")).get("buckets"))
-                    .get("invalidated")).get("doc_count"),
+                ((Map<String, Object>) ((Map<String, Object>) ((Map<String, Object>) level1Buckets.get(1).get(level2AggName)).get(
+                    "buckets"
+                )).get("invalidated")).get("doc_count"),
                 is(0)
             );
             assertThat(
-                ((Map<String, Object>) ((Map<String, Object>) ((Map<String, Object>) level1Buckets.get(1).get("level2")).get("buckets"))
-                    .get("not-invalidated")).get("doc_count"),
+                ((Map<String, Object>) ((Map<String, Object>) ((Map<String, Object>) 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 class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
             );
         }
         {
-            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 class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
         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 class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
               }
             }
             """, aggs -> {
-            List<Map<String, Object>> buckets = (List<Map<String, Object>>) ((Map<String, Object>) aggs.get("all_keys_by_type")).get(
-                "buckets"
-            );
+            String aggName = typedAggs ? "composite#all_keys_by_type" : "all_keys_by_type";
+            List<Map<String, Object>> buckets = (List<Map<String, Object>>) ((Map<String, Object>) aggs.get(aggName)).get("buckets");
             assertThat(buckets.size(), is(3));
             assertThat(((Map<String, Object>) buckets.get(0).get("key")).get("type"), is("cross_cluster"));
             assertThat(((Map<String, Object>) buckets.get(1).get("key")).get("type"), is("other"));
@@ -342,7 +354,7 @@ public class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
             }
         });
 
-        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 class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
             """, aggs -> {
             assertThat(aggs.size(), is(4));
             // 3 types
-            assertThat(((Map<String, Object>) aggs.get("type_cardinality")).get("value"), is(3));
+            assertThat(((Map<String, Object>) aggs.get((typedAggs ? "cardinality#" : "") + "type_cardinality")).get("value"), is(3));
             if (isAdmin) {
                 // 8 keys
-                assertThat(((Map<String, Object>) aggs.get("type_value_count")).get("value"), is(8));
+                assertThat(((Map<String, Object>) aggs.get((typedAggs ? "value_count#" : "") + "type_value_count")).get("value"), is(8));
             } else {
                 // 4 keys
-                assertThat(((Map<String, Object>) aggs.get("type_value_count")).get("value"), is(4));
+                assertThat(((Map<String, Object>) aggs.get((typedAggs ? "value_count#" : "") + "type_value_count")).get("value"), is(4));
             }
-            assertThat(((Map<String, Object>) aggs.get("missing_type_count")).get("doc_count"), is(0));
-            List<Map<String, Object>> typeTermsBuckets = (List<Map<String, Object>>) ((Map<String, Object>) aggs.get("type_terms")).get(
-                "buckets"
-            );
+            assertThat(((Map<String, Object>) aggs.get((typedAggs ? "missing#" : "") + "missing_type_count")).get("doc_count"), is(0));
+            List<Map<String, Object>> typeTermsBuckets = (List<Map<String, Object>>) ((Map<String, Object>) 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 class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
         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 class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
               }
             }
             """, aggs -> {
-            assertThat(((Map<String, Object>) aggs.get("not_invalidated")).get("doc_count"), is(4)); // 6 - 2 (invalidated)
+            // 6 - 2 (invalidated)
+            assertThat(((Map<String, Object>) aggs.get(typedAggs ? "filter#not_invalidated" : "not_invalidated")).get("doc_count"), is(4));
             List<Map<String, Object>> buckets = (List<Map<String, Object>>) ((Map<String, Object>) ((Map<String, Object>) 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<String, Object>) 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 class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
             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 class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
               }
             }
             """, aggs -> {
-            List<Map<String, Object>> buckets = (List<Map<String, Object>>) ((Map<String, Object>) aggs.get("keys_by_username")).get(
-                "buckets"
-            );
+            List<Map<String, Object>> buckets = (List<Map<String, Object>>) ((Map<String, Object>) 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<String, Object>) buckets.get(0).get("key")).get("usernames"), is("test-user-1"));
-            assertThat(((Map<String, Object>) buckets.get(0).get("not_expired")).get("doc_count"), is(0));
+            assertThat(
+                ((Map<String, Object>) 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<String, Object>) buckets.get(1).get("key")).get("usernames"), is("test-user-2"));
-            assertThat(((Map<String, Object>) buckets.get(1).get("not_expired")).get("doc_count"), is(1));
+            assertThat(
+                ((Map<String, Object>) 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<String, Object>) buckets.get(2).get("key")).get("usernames"), is("test-user-3"));
-            assertThat(((Map<String, Object>) buckets.get(2).get("not_expired")).get("doc_count"), is(2));
+            assertThat(
+                ((Map<String, Object>) 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 class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
     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 class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
         }
         // 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 class ApiKeyAggsIT extends SecurityInBasicRestTestCase {
         }
     }
 
-    void assertAggs(String authHeader, String body, Consumer<Map<String, Object>> aggsVerifier) throws IOException {
-        final Request request = new Request("GET", "/_security/_query/api_key");
+    void assertAggs(String authHeader, boolean typedAggs, String body, Consumer<Map<String, Object>> 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);

+ 7 - 0
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyAction.java

@@ -17,6 +17,7 @@ import org.elasticsearch.rest.RestRequest;
 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 org.elasticsearch.xpack.core.security.action.apikey.QueryApiKeyRequest;
 
 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 final class RestQueryApiKeyAction extends ApiKeyBaseRestHandler {
         return "xpack_security_query_api_key";
     }
 
+    @Override
+    protected Set<String> 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);