Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query API Keys support for both aggs and aggregations keywords #107054

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog/107054.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 107054
summary: Query API Keys support for both `aggs` and `aggregations` keywords
area: Security
type: enhancement
issues:
- 106839
2 changes: 1 addition & 1 deletion docs/reference/rest-api/security/query-api-key.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ simply mentioning `metadata` (not followed by any dot and sub-field name).
NOTE: You cannot query the role descriptors of an API key.
====

`aggs`::
`aggs` or `aggregations`::
(Optional, object) Any <<search-aggregations,aggregations>> to run over the corpus of returned API keys.
Aggregations and queries work together. Aggregations are computed only on the API keys that match the query.
This supports only a subset of aggregation types, namely: <<search-aggregations-bucket-terms-aggregation,terms>>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public void testFiltersAggs() throws IOException {
// other bucket
assertAggs(API_KEY_USER_AUTH_HEADER, typedAggs, """
{
"aggs": {
"aggregations": {
"only_user_keys": {
"filters": {
"other_bucket_key": "other_user_keys",
Expand Down Expand Up @@ -267,7 +267,7 @@ public void testFiltersAggs() throws IOException {
"good-api-key-invalidated": { "term": {"invalidated": false}}
}
},
"aggs": {
"aggregations": {
"wrong-field": {
"filters": {
"filters": {
Expand Down Expand Up @@ -487,7 +487,7 @@ public void testFilterAggs() throws IOException {
{ "usernames": { "terms": { "field": "username" } } }
]
},
"aggs": {
"aggregations": {
"not_expired": {
"filter": {
"range": {
Expand Down Expand Up @@ -564,7 +564,7 @@ public void testDisallowedAggTypes() {
);
request.setJsonEntity("""
{
"aggs": {
"aggregations": {
"all_.security_docs": {
"global": {},
"aggs": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
import static org.elasticsearch.rest.RestRequest.Method.GET;
import static org.elasticsearch.rest.RestRequest.Method.POST;
import static org.elasticsearch.search.aggregations.AggregatorFactories.parseAggregators;
import static org.elasticsearch.search.builder.SearchSourceBuilder.AGGREGATIONS_FIELD;
import static org.elasticsearch.search.builder.SearchSourceBuilder.AGGS_FIELD;
import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg;

/**
Expand All @@ -47,19 +49,27 @@ public final class RestQueryApiKeyAction extends ApiKeyBaseRestHandler {
@SuppressWarnings("unchecked")
private static final ConstructingObjectParser<Payload, Void> PARSER = new ConstructingObjectParser<>(
"query_api_key_request_payload",
a -> new Payload(
(QueryBuilder) a[0],
(AggregatorFactories.Builder) a[1],
(Integer) a[2],
(Integer) a[3],
(List<FieldSortBuilder>) a[4],
(SearchAfterBuilder) a[5]
)
a -> {
if (a[1] != null && a[2] != null) {
throw new IllegalArgumentException("Duplicate 'aggs' or 'aggregations' field");
} else {
return new Payload(
(QueryBuilder) a[0],
(AggregatorFactories.Builder) (a[1] != null ? a[1] : a[2]),
(Integer) a[3],
(Integer) a[4],
(List<FieldSortBuilder>) a[5],
(SearchAfterBuilder) a[6]
);
}
}
);

static {
PARSER.declareObject(optionalConstructorArg(), (p, c) -> parseTopLevelQuery(p), new ParseField("query"));
PARSER.declareObject(optionalConstructorArg(), (p, c) -> parseAggregators(p), new ParseField("aggs"));
// only one of aggs or aggregations is allowed
PARSER.declareObject(optionalConstructorArg(), (p, c) -> parseAggregators(p), AGGREGATIONS_FIELD);
PARSER.declareObject(optionalConstructorArg(), (p, c) -> parseAggregators(p), AGGS_FIELD);
PARSER.declareInt(optionalConstructorArg(), new ParseField("from"));
PARSER.declareInt(optionalConstructorArg(), new ParseField("size"));
PARSER.declareObjectArray(optionalConstructorArg(), (p, c) -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.action.ActionType;
import org.elasticsearch.client.internal.node.NodeClient;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
Expand All @@ -35,6 +36,7 @@
import org.elasticsearch.test.rest.FakeRestRequest;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.xcontent.NamedXContentRegistry;
import org.elasticsearch.xcontent.XContentParseException;
import org.elasticsearch.xcontent.XContentParser;
import org.elasticsearch.xcontent.XContentType;
import org.elasticsearch.xpack.core.security.action.apikey.ApiKey;
Expand All @@ -48,6 +50,7 @@
import java.util.Map;

import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
Expand Down Expand Up @@ -145,6 +148,69 @@ public <Request extends ActionRequest, Response extends ActionResponse> void doE
assertNotNull(responseSetOnce.get());
}

public void testAggsAndAggregationsTogether() {
String agg1;
String agg2;
if (randomBoolean()) {
agg1 = "aggs";
agg2 = "aggregations";
} else {
agg1 = "aggregations";
agg2 = "aggs";
}
final String requestBody = Strings.format("""
{
"%s": {
"all_keys_by_type": {
"composite": {
"sources": [
{ "type": { "terms": { "field": "type" } } }
]
}
}
},
"%s": {
"type_cardinality": {
"cardinality": {
"field": "type"
}
}
}
}""", agg1, agg2);

final FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry()).withContent(
new BytesArray(requestBody),
XContentType.JSON
).build();
final SetOnce<RestResponse> responseSetOnce = new SetOnce<>();
final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) {
@Override
public void sendResponse(RestResponse restResponse) {
responseSetOnce.set(restResponse);
}
};
final var client = new NodeClient(Settings.EMPTY, threadPool) {
@SuppressWarnings("unchecked")
@Override
public <Request extends ActionRequest, Response extends ActionResponse> void doExecute(
ActionType<Response> action,
Request request,
ActionListener<Response> listener
) {
fail("TEST failed, request parsing should've failed");
listener.onResponse((Response) QueryApiKeyResponse.EMPTY);
}
};
RestQueryApiKeyAction restQueryApiKeyAction = new RestQueryApiKeyAction(Settings.EMPTY, mockLicenseState);
XContentParseException ex = expectThrows(
XContentParseException.class,
() -> restQueryApiKeyAction.handleRequest(restRequest, restChannel, client)
);
assertThat(ex.getCause().getMessage(), containsString("Duplicate 'aggs' or 'aggregations' field"));
assertThat(ex.getMessage(), containsString("Failed to build [query_api_key_request_payload]"));
assertNull(responseSetOnce.get());
}

public void testParsingSearchParameters() throws Exception {
final String requestBody = """
{
Expand Down