Skip to content

Commit 2a5dfde

Browse files
albertzaharovitsalbertzaharovits
authored
Add type parameter support, for sorting, to the Query API Key API (#104625)
This adds support for the `type` parameter, for sorting, to the Query API key API. The type for an API Key can currently be either `rest` or `cross_cluster`. This was overlooked in #103695 when support for the `type` parameter was first introduced only for querying.
1 parent 9ea187d commit 2a5dfde

File tree

4 files changed

+102
-11
lines changed

4 files changed

+102
-11
lines changed

‎docs/changelog/104625.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 104625
2+
summary: "Add support for the `type` parameter, for sorting, to the Query API Key\
3+
\ API"
4+
area: Security
5+
type: enhancement
6+
issues: []

‎x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -778,6 +778,58 @@ public void testQueryCrossClusterApiKeysByType() throws IOException {
778778
assertThat(queryResponse.evaluate("api_keys.0.name"), is("test-cross-key-query-2"));
779779
}
780780

781+
public void testSortApiKeysByType() throws IOException {
782+
List<String> apiKeyIds = new ArrayList<>(2);
783+
// create regular api key
784+
EncodedApiKey encodedApiKey = createApiKey("test-rest-key", Map.of("tag", "rest"));
785+
apiKeyIds.add(encodedApiKey.id());
786+
// create cross-cluster key
787+
Request createRequest = new Request("POST", "/_security/cross_cluster/api_key");
788+
createRequest.setJsonEntity("""
789+
{
790+
"name": "test-cross-key",
791+
"access": {
792+
"search": [
793+
{
794+
"names": [ "whatever" ]
795+
}
796+
]
797+
},
798+
"metadata": { "tag": "cross" }
799+
}""");
800+
setUserForRequest(createRequest, MANAGE_SECURITY_USER, END_USER_PASSWORD);
801+
ObjectPath createResponse = assertOKAndCreateObjectPath(client().performRequest(createRequest));
802+
apiKeyIds.add(createResponse.evaluate("id"));
803+
804+
// desc sort all (2) keys - by type
805+
Request queryRequest = new Request("GET", "/_security/_query/api_key");
806+
queryRequest.addParameter("with_limited_by", String.valueOf(randomBoolean()));
807+
queryRequest.setJsonEntity("""
808+
{"sort":[{"type":{"order":"desc"}}]}""");
809+
setUserForRequest(queryRequest, MANAGE_API_KEY_USER, END_USER_PASSWORD);
810+
ObjectPath queryResponse = assertOKAndCreateObjectPath(client().performRequest(queryRequest));
811+
assertThat(queryResponse.evaluate("total"), is(2));
812+
assertThat(queryResponse.evaluate("count"), is(2));
813+
assertThat(queryResponse.evaluate("api_keys.0.id"), is(apiKeyIds.get(0)));
814+
assertThat(queryResponse.evaluate("api_keys.0.type"), is("rest"));
815+
assertThat(queryResponse.evaluate("api_keys.1.id"), is(apiKeyIds.get(1)));
816+
assertThat(queryResponse.evaluate("api_keys.1.type"), is("cross_cluster"));
817+
818+
// asc sort all (2) keys - by type
819+
queryRequest = new Request("GET", "/_security/_query/api_key");
820+
queryRequest.addParameter("with_limited_by", String.valueOf(randomBoolean()));
821+
queryRequest.setJsonEntity("""
822+
{"sort":[{"type":{"order":"asc"}}]}""");
823+
setUserForRequest(queryRequest, MANAGE_API_KEY_USER, END_USER_PASSWORD);
824+
queryResponse = assertOKAndCreateObjectPath(client().performRequest(queryRequest));
825+
assertThat(queryResponse.evaluate("total"), is(2));
826+
assertThat(queryResponse.evaluate("count"), is(2));
827+
assertThat(queryResponse.evaluate("api_keys.0.id"), is(apiKeyIds.get(1)));
828+
assertThat(queryResponse.evaluate("api_keys.0.type"), is("cross_cluster"));
829+
assertThat(queryResponse.evaluate("api_keys.1.id"), is(apiKeyIds.get(0)));
830+
assertThat(queryResponse.evaluate("api_keys.1.type"), is("rest"));
831+
}
832+
781833
public void testCreateCrossClusterApiKey() throws IOException {
782834
final Request createRequest = new Request("POST", "/_security/cross_cluster/api_key");
783835
createRequest.setJsonEntity("""

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/apikey/TransportQueryApiKeyAction.java

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.List;
2929
import java.util.Map;
3030
import java.util.concurrent.atomic.AtomicBoolean;
31+
import java.util.function.Consumer;
3132

3233
import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_MAIN_ALIAS;
3334

@@ -81,22 +82,25 @@ protected void doExecute(Task task, QueryApiKeyRequest request, ActionListener<Q
8182
}
8283

8384
final AtomicBoolean accessesApiKeyTypeField = new AtomicBoolean(false);
84-
final ApiKeyBoolQueryBuilder apiKeyBoolQueryBuilder = ApiKeyBoolQueryBuilder.build(request.getQueryBuilder(), fieldName -> {
85+
searchSourceBuilder.query(ApiKeyBoolQueryBuilder.build(request.getQueryBuilder(), fieldName -> {
8586
if (API_KEY_TYPE_RUNTIME_MAPPING_FIELD.equals(fieldName)) {
8687
accessesApiKeyTypeField.set(true);
8788
}
88-
}, request.isFilterForCurrentUser() ? authentication : null);
89-
searchSourceBuilder.query(apiKeyBoolQueryBuilder);
89+
}, request.isFilterForCurrentUser() ? authentication : null));
90+
91+
if (request.getFieldSortBuilders() != null) {
92+
translateFieldSortBuilders(request.getFieldSortBuilders(), searchSourceBuilder, fieldName -> {
93+
if (API_KEY_TYPE_RUNTIME_MAPPING_FIELD.equals(fieldName)) {
94+
accessesApiKeyTypeField.set(true);
95+
}
96+
});
97+
}
9098

9199
// only add the query-level runtime field to the search request if it's actually referring the "type" field
92100
if (accessesApiKeyTypeField.get()) {
93101
searchSourceBuilder.runtimeMappings(API_KEY_TYPE_RUNTIME_MAPPING);
94102
}
95103

96-
if (request.getFieldSortBuilders() != null) {
97-
translateFieldSortBuilders(request.getFieldSortBuilders(), searchSourceBuilder);
98-
}
99-
100104
if (request.getSearchAfterBuilder() != null) {
101105
searchSourceBuilder.searchAfter(request.getSearchAfterBuilder().getSortValues());
102106
}
@@ -106,7 +110,11 @@ protected void doExecute(Task task, QueryApiKeyRequest request, ActionListener<Q
106110
}
107111

108112
// package private for testing
109-
static void translateFieldSortBuilders(List<FieldSortBuilder> fieldSortBuilders, SearchSourceBuilder searchSourceBuilder) {
113+
static void translateFieldSortBuilders(
114+
List<FieldSortBuilder> fieldSortBuilders,
115+
SearchSourceBuilder searchSourceBuilder,
116+
Consumer<String> fieldNameVisitor
117+
) {
110118
fieldSortBuilders.forEach(fieldSortBuilder -> {
111119
if (fieldSortBuilder.getNestedSort() != null) {
112120
throw new IllegalArgumentException("nested sorting is not supported for API Key query");
@@ -115,6 +123,7 @@ static void translateFieldSortBuilders(List<FieldSortBuilder> fieldSortBuilders,
115123
searchSourceBuilder.sort(fieldSortBuilder);
116124
} else {
117125
final String translatedFieldName = ApiKeyFieldNameTranslators.translate(fieldSortBuilder.getFieldName());
126+
fieldNameVisitor.accept(translatedFieldName);
118127
if (translatedFieldName.equals(fieldSortBuilder.getFieldName())) {
119128
searchSourceBuilder.sort(fieldSortBuilder);
120129
} else {

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/apikey/TransportQueryApiKeyActionTests.java

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,30 +14,35 @@
1414
import org.elasticsearch.search.sort.SortOrder;
1515
import org.elasticsearch.test.ESTestCase;
1616

17+
import java.util.ArrayList;
1718
import java.util.List;
1819
import java.util.Set;
1920
import java.util.stream.IntStream;
2021

22+
import static org.hamcrest.Matchers.containsInAnyOrder;
2123
import static org.hamcrest.Matchers.equalTo;
2224

2325
public class TransportQueryApiKeyActionTests extends ESTestCase {
2426

2527
public void testTranslateFieldSortBuilders() {
28+
final String metadataField = randomAlphaOfLengthBetween(3, 8);
2629
final List<String> fieldNames = List.of(
2730
"_doc",
2831
"username",
2932
"realm_name",
3033
"name",
3134
"creation",
3235
"expiration",
36+
"type",
3337
"invalidated",
34-
"metadata." + randomAlphaOfLengthBetween(3, 8)
38+
"metadata." + metadataField
3539
);
3640

3741
final List<FieldSortBuilder> originals = fieldNames.stream().map(this::randomFieldSortBuilderWithName).toList();
3842

43+
List<String> sortFields = new ArrayList<>();
3944
final SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.searchSource();
40-
TransportQueryApiKeyAction.translateFieldSortBuilders(originals, searchSourceBuilder);
45+
TransportQueryApiKeyAction.translateFieldSortBuilders(originals, searchSourceBuilder, sortFields::add);
4146

4247
IntStream.range(0, originals.size()).forEach(i -> {
4348
final FieldSortBuilder original = originals.get(i);
@@ -57,6 +62,8 @@ public void testTranslateFieldSortBuilders() {
5762
assertThat(translated.getFieldName(), equalTo("api_key_invalidated"));
5863
} else if (original.getFieldName().startsWith("metadata.")) {
5964
assertThat(translated.getFieldName(), equalTo("metadata_flattened." + original.getFieldName().substring(9)));
65+
} else if ("type".equals(original.getFieldName())) {
66+
assertThat(translated.getFieldName(), equalTo("runtime_key_type"));
6067
} else {
6168
fail("unrecognized field name: [" + original.getFieldName() + "]");
6269
}
@@ -68,14 +75,31 @@ public void testTranslateFieldSortBuilders() {
6875
assertThat(translated.sortMode(), equalTo(original.sortMode()));
6976
}
7077
});
78+
assertThat(
79+
sortFields,
80+
containsInAnyOrder(
81+
"creator.principal",
82+
"creator.realm",
83+
"name",
84+
"creation_time",
85+
"expiration_time",
86+
"runtime_key_type",
87+
"api_key_invalidated",
88+
"metadata_flattened." + metadataField
89+
)
90+
);
7191
}
7292

7393
public void testNestedSortingIsNotAllowed() {
7494
final FieldSortBuilder fieldSortBuilder = new FieldSortBuilder("name");
7595
fieldSortBuilder.setNestedSort(new NestedSortBuilder("name"));
7696
final IllegalArgumentException e = expectThrows(
7797
IllegalArgumentException.class,
78-
() -> TransportQueryApiKeyAction.translateFieldSortBuilders(List.of(fieldSortBuilder), SearchSourceBuilder.searchSource())
98+
() -> TransportQueryApiKeyAction.translateFieldSortBuilders(
99+
List.of(fieldSortBuilder),
100+
SearchSourceBuilder.searchSource(),
101+
ignored -> {}
102+
)
79103
);
80104
assertThat(e.getMessage(), equalTo("nested sorting is not supported for API Key query"));
81105
}

0 commit comments

Comments
 (0)