Skip to content

Commit 2eb9729

Browse files
[8.13] Query API Keys support for both aggs and aggregations keywords (#… (#107108)
The Query API Key Information endpoint supports aggs since #104895. But some lang clients actually use the aggregations term in requests, as the preferred synonym. This PR adds support for the aggregations request term as a synonym for the existing aggs term. Backport of #107054
1 parent b1eb32b commit 2eb9729

File tree

5 files changed

+96
-14
lines changed

5 files changed

+96
-14
lines changed

docs/changelog/107054.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 107054
2+
summary: Query API Keys support for both `aggs` and `aggregations` keywords
3+
area: Security
4+
type: enhancement
5+
issues:
6+
- 106839

docs/reference/rest-api/security/query-api-key.asciidoc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ simply mentioning `metadata` (not followed by any dot and sub-field name).
227227
NOTE: You cannot query the role descriptors of an API key.
228228
====
229229

230-
`aggs`::
230+
`aggs` or `aggregations`::
231231
(Optional, object) Any <<search-aggregations,aggregations>> to run over the corpus of returned API keys.
232232
Aggregations and queries work together. Aggregations are computed only on the API keys that match the query.
233233
This supports only a subset of aggregation types, namely: <<search-aggregations-bucket-terms-aggregation,terms>>,

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public void testFiltersAggs() throws IOException {
9898
// other bucket
9999
assertAggs(API_KEY_USER_AUTH_HEADER, typedAggs, """
100100
{
101-
"aggs": {
101+
"aggregations": {
102102
"only_user_keys": {
103103
"filters": {
104104
"other_bucket_key": "other_user_keys",
@@ -267,7 +267,7 @@ public void testFiltersAggs() throws IOException {
267267
"good-api-key-invalidated": { "term": {"invalidated": false}}
268268
}
269269
},
270-
"aggs": {
270+
"aggregations": {
271271
"wrong-field": {
272272
"filters": {
273273
"filters": {
@@ -487,7 +487,7 @@ public void testFilterAggs() throws IOException {
487487
{ "usernames": { "terms": { "field": "username" } } }
488488
]
489489
},
490-
"aggs": {
490+
"aggregations": {
491491
"not_expired": {
492492
"filter": {
493493
"range": {
@@ -564,7 +564,7 @@ public void testDisallowedAggTypes() {
564564
);
565565
request.setJsonEntity("""
566566
{
567-
"aggs": {
567+
"aggregations": {
568568
"all_.security_docs": {
569569
"global": {},
570570
"aggs": {

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

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636
import static org.elasticsearch.rest.RestRequest.Method.GET;
3737
import static org.elasticsearch.rest.RestRequest.Method.POST;
3838
import static org.elasticsearch.search.aggregations.AggregatorFactories.parseAggregators;
39+
import static org.elasticsearch.search.builder.SearchSourceBuilder.AGGREGATIONS_FIELD;
40+
import static org.elasticsearch.search.builder.SearchSourceBuilder.AGGS_FIELD;
3941
import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg;
4042

4143
/**
@@ -47,19 +49,27 @@ public final class RestQueryApiKeyAction extends ApiKeyBaseRestHandler {
4749
@SuppressWarnings("unchecked")
4850
private static final ConstructingObjectParser<Payload, Void> PARSER = new ConstructingObjectParser<>(
4951
"query_api_key_request_payload",
50-
a -> new Payload(
51-
(QueryBuilder) a[0],
52-
(AggregatorFactories.Builder) a[1],
53-
(Integer) a[2],
54-
(Integer) a[3],
55-
(List<FieldSortBuilder>) a[4],
56-
(SearchAfterBuilder) a[5]
57-
)
52+
a -> {
53+
if (a[1] != null && a[2] != null) {
54+
throw new IllegalArgumentException("Duplicate 'aggs' or 'aggregations' field");
55+
} else {
56+
return new Payload(
57+
(QueryBuilder) a[0],
58+
(AggregatorFactories.Builder) (a[1] != null ? a[1] : a[2]),
59+
(Integer) a[3],
60+
(Integer) a[4],
61+
(List<FieldSortBuilder>) a[5],
62+
(SearchAfterBuilder) a[6]
63+
);
64+
}
65+
}
5866
);
5967

6068
static {
6169
PARSER.declareObject(optionalConstructorArg(), (p, c) -> parseTopLevelQuery(p), new ParseField("query"));
62-
PARSER.declareObject(optionalConstructorArg(), (p, c) -> parseAggregators(p), new ParseField("aggs"));
70+
// only one of aggs or aggregations is allowed
71+
PARSER.declareObject(optionalConstructorArg(), (p, c) -> parseAggregators(p), AGGREGATIONS_FIELD);
72+
PARSER.declareObject(optionalConstructorArg(), (p, c) -> parseAggregators(p), AGGS_FIELD);
6373
PARSER.declareInt(optionalConstructorArg(), new ParseField("from"));
6474
PARSER.declareInt(optionalConstructorArg(), new ParseField("size"));
6575
PARSER.declareObjectArray(optionalConstructorArg(), (p, c) -> {

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyActionTests.java

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.elasticsearch.action.ActionResponse;
1414
import org.elasticsearch.action.ActionType;
1515
import org.elasticsearch.client.internal.node.NodeClient;
16+
import org.elasticsearch.common.Strings;
1617
import org.elasticsearch.common.bytes.BytesArray;
1718
import org.elasticsearch.common.settings.Settings;
1819
import org.elasticsearch.env.Environment;
@@ -34,12 +35,14 @@
3435
import org.elasticsearch.test.rest.FakeRestRequest;
3536
import org.elasticsearch.threadpool.ThreadPool;
3637
import org.elasticsearch.xcontent.NamedXContentRegistry;
38+
import org.elasticsearch.xcontent.XContentParseException;
3739
import org.elasticsearch.xcontent.XContentType;
3840
import org.elasticsearch.xpack.core.security.action.apikey.QueryApiKeyRequest;
3941
import org.elasticsearch.xpack.core.security.action.apikey.QueryApiKeyResponse;
4042

4143
import java.util.List;
4244

45+
import static org.hamcrest.Matchers.containsString;
4346
import static org.hamcrest.Matchers.equalTo;
4447
import static org.hamcrest.Matchers.hasSize;
4548
import static org.hamcrest.Matchers.is;
@@ -135,6 +138,69 @@ public <Request extends ActionRequest, Response extends ActionResponse> void doE
135138
assertNotNull(responseSetOnce.get());
136139
}
137140

141+
public void testAggsAndAggregationsTogether() {
142+
String agg1;
143+
String agg2;
144+
if (randomBoolean()) {
145+
agg1 = "aggs";
146+
agg2 = "aggregations";
147+
} else {
148+
agg1 = "aggregations";
149+
agg2 = "aggs";
150+
}
151+
final String requestBody = Strings.format("""
152+
{
153+
"%s": {
154+
"all_keys_by_type": {
155+
"composite": {
156+
"sources": [
157+
{ "type": { "terms": { "field": "type" } } }
158+
]
159+
}
160+
}
161+
},
162+
"%s": {
163+
"type_cardinality": {
164+
"cardinality": {
165+
"field": "type"
166+
}
167+
}
168+
}
169+
}""", agg1, agg2);
170+
171+
final FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry()).withContent(
172+
new BytesArray(requestBody),
173+
XContentType.JSON
174+
).build();
175+
final SetOnce<RestResponse> responseSetOnce = new SetOnce<>();
176+
final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) {
177+
@Override
178+
public void sendResponse(RestResponse restResponse) {
179+
responseSetOnce.set(restResponse);
180+
}
181+
};
182+
final var client = new NodeClient(Settings.EMPTY, threadPool) {
183+
@SuppressWarnings("unchecked")
184+
@Override
185+
public <Request extends ActionRequest, Response extends ActionResponse> void doExecute(
186+
ActionType<Response> action,
187+
Request request,
188+
ActionListener<Response> listener
189+
) {
190+
fail("TEST failed, request parsing should've failed");
191+
listener.onResponse((Response) QueryApiKeyResponse.EMPTY);
192+
}
193+
};
194+
RestQueryApiKeyAction restQueryApiKeyAction = new RestQueryApiKeyAction(Settings.EMPTY, mockLicenseState);
195+
XContentParseException ex = expectThrows(
196+
XContentParseException.class,
197+
() -> restQueryApiKeyAction.handleRequest(restRequest, restChannel, client)
198+
);
199+
assertThat(ex.getCause().getMessage(), containsString("Duplicate 'aggs' or 'aggregations' field"));
200+
assertThat(ex.getMessage(), containsString("Failed to build [query_api_key_request_payload]"));
201+
assertNull(responseSetOnce.get());
202+
}
203+
138204
public void testParsingSearchParameters() throws Exception {
139205
final String requestBody = """
140206
{

0 commit comments

Comments
 (0)