From 448488c1b7691f18a1ce7490cfc7678b3526143b Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Sun, 21 Jan 2024 15:32:12 +0200 Subject: [PATCH 1/8] Main --- .../rest-api/security/query-api-key.asciidoc | 8 +++-- .../support/ApiKeyBoolQueryBuilder.java | 33 ++++++++++++++++++- .../support/ApiKeyBoolQueryBuilderTests.java | 9 +++-- 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/docs/reference/rest-api/security/query-api-key.asciidoc b/docs/reference/rest-api/security/query-api-key.asciidoc index a08a8fd1858b6..af30a29ad1398 100644 --- a/docs/reference/rest-api/security/query-api-key.asciidoc +++ b/docs/reference/rest-api/security/query-api-key.asciidoc @@ -52,9 +52,11 @@ You can specify the following parameters in the request body: (Optional, string) A <> to filter which API keys to return. The query supports a subset of query types, including <>, <>, -<>, <>, <>, -<>, <>, <>, -<>, and <> +<>, <>, +<>, <>, +<>, <>, +<>, <>, +and <> + You can query the following public values associated with an API key. + diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilder.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilder.java index 9f7b84e4a2698..80e07fbfddce4 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilder.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilder.java @@ -14,6 +14,7 @@ import org.elasticsearch.index.query.IdsQueryBuilder; import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.MatchNoneQueryBuilder; +import org.elasticsearch.index.query.MatchQueryBuilder; import org.elasticsearch.index.query.PrefixQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; @@ -139,13 +140,43 @@ private static QueryBuilder doProcess(QueryBuilder qb, Consumer fieldNam } else if (qb instanceof final PrefixQueryBuilder query) { final String translatedFieldName = ApiKeyFieldNameTranslators.translate(query.fieldName()); fieldNameVisitor.accept(translatedFieldName); - return QueryBuilders.prefixQuery(translatedFieldName, query.value()).caseInsensitive(query.caseInsensitive()); + return QueryBuilders.prefixQuery(translatedFieldName, query.value()) + .caseInsensitive(query.caseInsensitive()) + .rewrite(query.rewrite()); } else if (qb instanceof final WildcardQueryBuilder query) { final String translatedFieldName = ApiKeyFieldNameTranslators.translate(query.fieldName()); fieldNameVisitor.accept(translatedFieldName); return QueryBuilders.wildcardQuery(translatedFieldName, query.value()) .caseInsensitive(query.caseInsensitive()) .rewrite(query.rewrite()); + } else if (qb instanceof final MatchQueryBuilder query) { + final String translatedFieldName = ApiKeyFieldNameTranslators.translate(query.fieldName()); + fieldNameVisitor.accept(translatedFieldName); + final MatchQueryBuilder matchQueryBuilder = QueryBuilders.matchQuery(translatedFieldName, query.value()); + if (query.operator() != null) { + matchQueryBuilder.operator(query.operator()); + } + if (query.analyzer() != null) { + matchQueryBuilder.analyzer(query.analyzer()); + } + if (query.fuzziness() != null) { + matchQueryBuilder.fuzziness(query.fuzziness()); + } + if (query.minimumShouldMatch() != null) { + matchQueryBuilder.minimumShouldMatch(query.minimumShouldMatch()); + } + if (query.fuzzyRewrite() != null) { + matchQueryBuilder.fuzzyRewrite(query.fuzzyRewrite()); + } + if (query.zeroTermsQuery() != null) { + matchQueryBuilder.zeroTermsQuery(query.zeroTermsQuery()); + } + matchQueryBuilder.prefixLength(query.prefixLength()) + .maxExpansions(query.maxExpansions()) + .fuzzyTranspositions(query.fuzzyTranspositions()) + .lenient(query.lenient()) + .autoGenerateSynonymsPhraseQuery(query.autoGenerateSynonymsPhraseQuery()); + return matchQueryBuilder; } else if (qb instanceof final RangeQueryBuilder query) { if (query.relation() != null) { throw new IllegalArgumentException("range query with relation is not supported for API Key query"); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilderTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilderTests.java index 4064d9f0ce4da..5bda61c0bd499 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilderTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilderTests.java @@ -14,6 +14,7 @@ import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.MatchNoneQueryBuilder; import org.elasticsearch.index.query.MultiTermQueryBuilder; +import org.elasticsearch.index.query.Operator; import org.elasticsearch.index.query.PrefixQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; @@ -405,7 +406,6 @@ public void testDisallowedQueryTypes() { final Authentication authentication = randomBoolean() ? AuthenticationTests.randomAuthentication(null, null) : null; final AbstractQueryBuilder> q1 = randomFrom( - QueryBuilders.matchQuery(randomAlphaOfLength(5), randomAlphaOfLength(5)), QueryBuilders.constantScoreQuery(mock(QueryBuilder.class)), QueryBuilders.boostingQuery(mock(QueryBuilder.class), mock(QueryBuilder.class)), QueryBuilders.queryStringQuery("q=a:42"), @@ -773,7 +773,7 @@ private void assertCommonFilterQueries(ApiKeyBoolQueryBuilder qb, Authentication } private QueryBuilder randomSimpleQuery(String fieldName) { - return switch (randomIntBetween(0, 8)) { + return switch (randomIntBetween(0, 9)) { case 0 -> QueryBuilders.termQuery(fieldName, randomAlphaOfLengthBetween(3, 8)); case 1 -> QueryBuilders.termsQuery(fieldName, randomArray(1, 3, String[]::new, () -> randomAlphaOfLengthBetween(3, 8))); case 2 -> QueryBuilders.idsQuery().addIds(randomArray(1, 3, String[]::new, () -> randomAlphaOfLength(22))); @@ -788,6 +788,11 @@ private QueryBuilder randomSimpleQuery(String fieldName) { .field(fieldName) .lenient(randomBoolean()) .analyzeWildcard(randomBoolean()); + case 9 -> QueryBuilders.matchQuery(fieldName, randomAlphaOfLengthBetween(3, 8)) + .operator(randomFrom(Operator.OR, Operator.AND)) + .lenient(randomBoolean()) + .maxExpansions(randomIntBetween(1, 100)) + .analyzer(randomFrom(randomAlphaOfLength(4), null)); default -> throw new IllegalStateException("illegal switch case"); }; } From ec95701ef84df468197096c9988ff9c95d0fd0b2 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 22 Jan 2024 15:31:26 +0200 Subject: [PATCH 2/8] le test --- .../support/ApiKeyBoolQueryBuilderTests.java | 165 ++++++++++++++++-- 1 file changed, 146 insertions(+), 19 deletions(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilderTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilderTests.java index 5bda61c0bd499..3d68ce9c7ad96 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilderTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilderTests.java @@ -7,12 +7,14 @@ package org.elasticsearch.xpack.security.support; +import org.elasticsearch.common.unit.Fuzziness; import org.elasticsearch.index.query.AbstractQueryBuilder; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.DistanceFeatureQueryBuilder; import org.elasticsearch.index.query.IdsQueryBuilder; import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.MatchNoneQueryBuilder; +import org.elasticsearch.index.query.MatchQueryBuilder; import org.elasticsearch.index.query.MultiTermQueryBuilder; import org.elasticsearch.index.query.Operator; import org.elasticsearch.index.query.PrefixQueryBuilder; @@ -25,10 +27,12 @@ import org.elasticsearch.index.query.TermQueryBuilder; import org.elasticsearch.index.query.TermsQueryBuilder; import org.elasticsearch.index.query.WildcardQueryBuilder; +import org.elasticsearch.index.query.ZeroTermsQueryOption; import org.elasticsearch.indices.TermsLookup; import org.elasticsearch.script.Script; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.security.authc.Authentication; +import org.elasticsearch.xpack.core.security.authc.AuthenticationField; import org.elasticsearch.xpack.core.security.authc.AuthenticationTests; import org.elasticsearch.xpack.core.security.authc.RealmConfig; import org.elasticsearch.xpack.core.security.user.User; @@ -48,11 +52,13 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.emptyIterable; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.iterableWithSize; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; @@ -61,17 +67,120 @@ public class ApiKeyBoolQueryBuilderTests extends ESTestCase { public void testBuildFromSimpleQuery() { - final Authentication authentication = randomBoolean() ? AuthenticationTests.randomAuthentication(null, null) : null; - final QueryBuilder q1 = randomSimpleQuery("name"); - final List queryFields = new ArrayList<>(); - final ApiKeyBoolQueryBuilder apiKeyQb1 = ApiKeyBoolQueryBuilder.build(q1, queryFields::add, authentication); - assertQueryFields(queryFields, q1, authentication); - assertCommonFilterQueries(apiKeyQb1, authentication); - final List mustQueries = apiKeyQb1.must(); + { + QueryBuilder qb = randomSimpleQuery("name"); + List queryFields = new ArrayList<>(); + ApiKeyBoolQueryBuilder apiKeyQb = ApiKeyBoolQueryBuilder.build(qb, queryFields::add, null); + assertQueryFields(queryFields, qb, null); + assertCommonFilterQueries(apiKeyQb, null); + List mustQueries = apiKeyQb.must(); + assertThat(mustQueries, hasSize(1)); + assertThat(mustQueries.get(0), equalTo(qb)); + assertThat(apiKeyQb.should(), emptyIterable()); + assertThat(apiKeyQb.mustNot(), emptyIterable()); + } + { + Authentication authentication = AuthenticationTests.randomAuthentication(null, null); + QueryBuilder qb = randomSimpleQuery("name"); + List queryFields = new ArrayList<>(); + ApiKeyBoolQueryBuilder apiKeyQb = ApiKeyBoolQueryBuilder.build(qb, queryFields::add, authentication); + assertQueryFields(queryFields, qb, authentication); + assertCommonFilterQueries(apiKeyQb, authentication); + List mustQueries = apiKeyQb.must(); + assertThat(mustQueries, hasSize(1)); + assertThat(mustQueries.get(0), equalTo(qb)); + assertThat(apiKeyQb.should(), emptyIterable()); + assertThat(apiKeyQb.mustNot(), emptyIterable()); + } + { + String apiKeyId = randomUUID(); + Authentication authentication = AuthenticationTests.randomApiKeyAuthentication(AuthenticationTests.randomUser(), apiKeyId); + QueryBuilder qb = randomSimpleQuery("name"); + List queryFields = new ArrayList<>(); + ApiKeyBoolQueryBuilder apiKeyQb = ApiKeyBoolQueryBuilder.build(qb, queryFields::add, authentication); + assertQueryFields(queryFields, qb, authentication); + assertCommonFilterQueries(apiKeyQb, authentication); + List mustQueries = apiKeyQb.must(); + assertThat(mustQueries, hasSize(1)); + assertThat(mustQueries.get(0), equalTo(qb)); + assertThat(apiKeyQb.should(), emptyIterable()); + assertThat(apiKeyQb.mustNot(), emptyIterable()); + } + } + + public void testMatchQueryBuilderPropertiesArePreservedFor() { + // the match query has many properties, that all must be preserved after limiting for API Key docs only + Authentication authentication = randomFrom( + AuthenticationTests.randomApiKeyAuthentication(AuthenticationTests.randomUser(), randomUUID()), + AuthenticationTests.randomAuthentication(null, null), + null + ); + String fieldName = randomFrom( + "username", + "realm_name", + "name", + "type", + "creation", + "expiration", + "invalidated", + "invalidation", + "metadata", + "metadata.what.ever" + ); + MatchQueryBuilder matchQueryBuilder = QueryBuilders.matchQuery(fieldName, new Object()); + if (randomBoolean()) { + matchQueryBuilder.operator(randomFrom(Operator.OR, Operator.AND)); + } + if (randomBoolean()) { + matchQueryBuilder.analyzer(randomAlphaOfLength(4)); + } + if (randomBoolean()) { + matchQueryBuilder.fuzziness(randomFrom(Fuzziness.ZERO, Fuzziness.ONE, Fuzziness.TWO, Fuzziness.AUTO)); + } + if (randomBoolean()) { + matchQueryBuilder.minimumShouldMatch(randomAlphaOfLength(4)); + } + if (randomBoolean()) { + matchQueryBuilder.fuzzyRewrite(randomAlphaOfLength(4)); + } + if (randomBoolean()) { + matchQueryBuilder.zeroTermsQuery(randomFrom(ZeroTermsQueryOption.NONE, ZeroTermsQueryOption.ALL, ZeroTermsQueryOption.NULL)); + } + if (randomBoolean()) { + matchQueryBuilder.prefixLength(randomNonNegativeInt()); + } + if (randomBoolean()) { + matchQueryBuilder.maxExpansions(randomIntBetween(1, 100)); + } + if (randomBoolean()) { + matchQueryBuilder.fuzzyTranspositions(randomBoolean()); + } + if (randomBoolean()) { + matchQueryBuilder.lenient(randomBoolean()); + } + if (randomBoolean()) { + matchQueryBuilder.autoGenerateSynonymsPhraseQuery(randomBoolean()); + } + List queryFields = new ArrayList<>(); + ApiKeyBoolQueryBuilder apiKeyMatchQueryBuilder = ApiKeyBoolQueryBuilder.build(matchQueryBuilder, queryFields::add, authentication); + assertThat(queryFields, hasItem(ApiKeyFieldNameTranslators.translate(fieldName))); + List mustQueries = apiKeyMatchQueryBuilder.must(); assertThat(mustQueries, hasSize(1)); - assertThat(mustQueries.get(0), equalTo(q1)); - assertTrue(apiKeyQb1.should().isEmpty()); - assertTrue(apiKeyQb1.mustNot().isEmpty()); + assertThat(mustQueries.get(0), instanceOf(MatchQueryBuilder.class)); + MatchQueryBuilder matchQueryBuilder2 = (MatchQueryBuilder) mustQueries.get(0); + assertThat(matchQueryBuilder2.fieldName(), is(ApiKeyFieldNameTranslators.translate(matchQueryBuilder.fieldName()))); + assertThat(matchQueryBuilder2.value(), is(matchQueryBuilder.value())); + assertThat(matchQueryBuilder2.operator(), is(matchQueryBuilder.operator())); + assertThat(matchQueryBuilder2.analyzer(), is(matchQueryBuilder.analyzer())); + assertThat(matchQueryBuilder2.fuzziness(), is(matchQueryBuilder.fuzziness())); + assertThat(matchQueryBuilder2.minimumShouldMatch(), is(matchQueryBuilder.minimumShouldMatch())); + assertThat(matchQueryBuilder2.fuzzyRewrite(), is(matchQueryBuilder.fuzzyRewrite())); + assertThat(matchQueryBuilder2.zeroTermsQuery(), is(matchQueryBuilder.zeroTermsQuery())); + assertThat(matchQueryBuilder2.prefixLength(), is(matchQueryBuilder.prefixLength())); + assertThat(matchQueryBuilder2.maxExpansions(), is(matchQueryBuilder.maxExpansions())); + assertThat(matchQueryBuilder2.fuzzyTranspositions(), is(matchQueryBuilder.fuzzyTranspositions())); + assertThat(matchQueryBuilder2.lenient(), is(matchQueryBuilder.lenient())); + assertThat(matchQueryBuilder2.autoGenerateSynonymsPhraseQuery(), is(matchQueryBuilder.autoGenerateSynonymsPhraseQuery())); } public void testQueryForDomainAuthentication() { @@ -760,16 +869,34 @@ private void assertCommonFilterQueries(ApiKeyBoolQueryBuilder qb, Authentication if (authentication == null) { return; } - assertTrue( - tqb.stream() - .anyMatch( - q -> q.equals(QueryBuilders.termQuery("creator.principal", authentication.getEffectiveSubject().getUser().principal())) + if (authentication.isApiKey()) { + List idsQueryBuilders = qb.filter() + .stream() + .filter(q -> q.getClass() == IdsQueryBuilder.class) + .map(q -> (IdsQueryBuilder) q) + .toList(); + assertThat(idsQueryBuilders, iterableWithSize(1)); + assertThat( + idsQueryBuilders.get(0), + equalTo( + QueryBuilders.idsQuery() + .addIds((String) authentication.getAuthenticatingSubject().getMetadata().get(AuthenticationField.API_KEY_ID_KEY)) ) - ); - assertTrue( - tqb.stream() - .anyMatch(q -> q.equals(QueryBuilders.termQuery("creator.realm", ApiKeyService.getCreatorRealmName(authentication)))) - ); + ); + } else { + assertTrue( + tqb.stream() + .anyMatch( + q -> q.equals( + QueryBuilders.termQuery("creator.principal", authentication.getEffectiveSubject().getUser().principal()) + ) + ) + ); + assertTrue( + tqb.stream() + .anyMatch(q -> q.equals(QueryBuilders.termQuery("creator.realm", ApiKeyService.getCreatorRealmName(authentication)))) + ); + } } private QueryBuilder randomSimpleQuery(String fieldName) { From d896401038933ec098fdc3b687ddf55e52255350 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 22 Jan 2024 16:02:01 +0200 Subject: [PATCH 3/8] Simplest IT --- .../org/elasticsearch/xpack/security/QueryApiKeyIT.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryApiKeyIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryApiKeyIT.java index e552befc267c8..c93776c4725a0 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryApiKeyIT.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryApiKeyIT.java @@ -63,6 +63,14 @@ public void testQuery() throws IOException { apiKeys.forEach(k -> assertThat(k, not(hasKey("_sort")))); }); + assertQuery(API_KEY_ADMIN_AUTH_HEADER, """ + { "query": { "match": {"name": {"query": "my-ingest-key-1 my-org/alert-key-1", "analyzer": "whitespace"} } } }""", apiKeys -> { + assertThat(apiKeys.size(), equalTo(2)); + assertThat(apiKeys.get(0).get("name"), oneOf("my-ingest-key-1", "my-org/alert-key-1")); + assertThat(apiKeys.get(1).get("name"), oneOf("my-ingest-key-1", "my-org/alert-key-1")); + apiKeys.forEach(k -> assertThat(k, not(hasKey("_sort")))); + }); + // An empty request body means search for all keys assertQuery(API_KEY_ADMIN_AUTH_HEADER, randomBoolean() ? "" : """ {"query":{"match_all":{}}}""", apiKeys -> assertThat(apiKeys.size(), equalTo(6))); From 3aa08bb5d6e73069cee3ce3880f9d97db00bbe4d Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 22 Jan 2024 17:44:17 +0200 Subject: [PATCH 4/8] Javadoc and test nit --- docs/reference/rest-api/security/query-api-key.asciidoc | 6 ++++++ .../org/elasticsearch/xpack/security/QueryApiKeyIT.java | 3 +++ 2 files changed, 9 insertions(+) diff --git a/docs/reference/rest-api/security/query-api-key.asciidoc b/docs/reference/rest-api/security/query-api-key.asciidoc index af30a29ad1398..e3eb5e0fc005e 100644 --- a/docs/reference/rest-api/security/query-api-key.asciidoc +++ b/docs/reference/rest-api/security/query-api-key.asciidoc @@ -60,6 +60,12 @@ and <> + You can query the following public values associated with an API key. + +NOTE: The queryable string values associated with API keys are internally mapped as <>s. +Consequently, if no <> parameter is specified for a +<> query, then the provided match query string is interpreted as +a single keyword value. The <> query is hence equivalent to a +<> one. ++ .Valid values for `query` [%collapsible%open] ==== diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryApiKeyIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryApiKeyIT.java index c93776c4725a0..e9c640236ceb5 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryApiKeyIT.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryApiKeyIT.java @@ -75,6 +75,9 @@ public void testQuery() throws IOException { assertQuery(API_KEY_ADMIN_AUTH_HEADER, randomBoolean() ? "" : """ {"query":{"match_all":{}}}""", apiKeys -> assertThat(apiKeys.size(), equalTo(6))); + assertQuery(API_KEY_ADMIN_AUTH_HEADER, randomBoolean() ? "" : """ + { "query": { "match": {"type": "rest"} } }""", apiKeys -> assertThat(apiKeys.size(), equalTo(6))); + assertQuery( API_KEY_ADMIN_AUTH_HEADER, """ From 8c9fe0247f23d01aed25ce5fe31e114acb8611a9 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 22 Jan 2024 17:46:07 +0200 Subject: [PATCH 5/8] Update docs/changelog/104594.yaml --- docs/changelog/104594.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/104594.yaml diff --git a/docs/changelog/104594.yaml b/docs/changelog/104594.yaml new file mode 100644 index 0000000000000..7729eb028f68e --- /dev/null +++ b/docs/changelog/104594.yaml @@ -0,0 +1,5 @@ +pr: 104594 +summary: Support of `match` for the Query API Key API +area: Authentication +type: enhancement +issues: [] From c3533e5a11b7c48d7ea432cdf35a6043a123ad05 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 22 Jan 2024 18:54:59 +0200 Subject: [PATCH 6/8] Doc nit --- docs/reference/rest-api/security/query-api-key.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/rest-api/security/query-api-key.asciidoc b/docs/reference/rest-api/security/query-api-key.asciidoc index e3eb5e0fc005e..7ff7fe9727f1a 100644 --- a/docs/reference/rest-api/security/query-api-key.asciidoc +++ b/docs/reference/rest-api/security/query-api-key.asciidoc @@ -60,10 +60,10 @@ and <> + You can query the following public values associated with an API key. + -NOTE: The queryable string values associated with API keys are internally mapped as <>s. +NOTE: The queryable string values associated with API keys are internally mapped as <>. Consequently, if no <> parameter is specified for a <> query, then the provided match query string is interpreted as -a single keyword value. The <> query is hence equivalent to a +a single keyword value. Such a <> query is hence equivalent to a <> one. + .Valid values for `query` From 06252a99888bf33cac71f6403b0aea3107b32009 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Tue, 30 Jan 2024 16:00:16 +0200 Subject: [PATCH 7/8] Some boost tests --- .../support/ApiKeyBoolQueryBuilder.java | 20 ++++-- .../support/ApiKeyBoolQueryBuilderTests.java | 65 +++++++++++++++---- 2 files changed, 65 insertions(+), 20 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilder.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilder.java index 80e07fbfddce4..651427d07e651 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilder.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilder.java @@ -112,7 +112,8 @@ private static QueryBuilder doProcess(QueryBuilder qb, Consumer fieldNam if (qb instanceof final BoolQueryBuilder query) { final BoolQueryBuilder newQuery = QueryBuilders.boolQuery() .minimumShouldMatch(query.minimumShouldMatch()) - .adjustPureNegative(query.adjustPureNegative()); + .adjustPureNegative(query.adjustPureNegative()) + .boost(query.boost()); query.must().stream().map(q -> ApiKeyBoolQueryBuilder.doProcess(q, fieldNameVisitor)).forEach(newQuery::must); query.should().stream().map(q -> ApiKeyBoolQueryBuilder.doProcess(q, fieldNameVisitor)).forEach(newQuery::should); query.mustNot().stream().map(q -> ApiKeyBoolQueryBuilder.doProcess(q, fieldNameVisitor)).forEach(newQuery::mustNot); @@ -125,30 +126,34 @@ private static QueryBuilder doProcess(QueryBuilder qb, Consumer fieldNam } else if (qb instanceof final TermQueryBuilder query) { final String translatedFieldName = ApiKeyFieldNameTranslators.translate(query.fieldName()); fieldNameVisitor.accept(translatedFieldName); - return QueryBuilders.termQuery(translatedFieldName, query.value()).caseInsensitive(query.caseInsensitive()); + return QueryBuilders.termQuery(translatedFieldName, query.value()) + .caseInsensitive(query.caseInsensitive()) + .boost(query.boost()); } else if (qb instanceof final ExistsQueryBuilder query) { final String translatedFieldName = ApiKeyFieldNameTranslators.translate(query.fieldName()); fieldNameVisitor.accept(translatedFieldName); - return QueryBuilders.existsQuery(translatedFieldName); + return QueryBuilders.existsQuery(translatedFieldName).boost(query.boost()); } else if (qb instanceof final TermsQueryBuilder query) { if (query.termsLookup() != null) { throw new IllegalArgumentException("terms query with terms lookup is not supported for API Key query"); } final String translatedFieldName = ApiKeyFieldNameTranslators.translate(query.fieldName()); fieldNameVisitor.accept(translatedFieldName); - return QueryBuilders.termsQuery(translatedFieldName, query.getValues()); + return QueryBuilders.termsQuery(translatedFieldName, query.getValues()).boost(query.boost()); } else if (qb instanceof final PrefixQueryBuilder query) { final String translatedFieldName = ApiKeyFieldNameTranslators.translate(query.fieldName()); fieldNameVisitor.accept(translatedFieldName); return QueryBuilders.prefixQuery(translatedFieldName, query.value()) .caseInsensitive(query.caseInsensitive()) - .rewrite(query.rewrite()); + .rewrite(query.rewrite()) + .boost(query.boost()); } else if (qb instanceof final WildcardQueryBuilder query) { final String translatedFieldName = ApiKeyFieldNameTranslators.translate(query.fieldName()); fieldNameVisitor.accept(translatedFieldName); return QueryBuilders.wildcardQuery(translatedFieldName, query.value()) .caseInsensitive(query.caseInsensitive()) - .rewrite(query.rewrite()); + .rewrite(query.rewrite()) + .boost(query.boost()); } else if (qb instanceof final MatchQueryBuilder query) { final String translatedFieldName = ApiKeyFieldNameTranslators.translate(query.fieldName()); fieldNameVisitor.accept(translatedFieldName); @@ -175,7 +180,8 @@ private static QueryBuilder doProcess(QueryBuilder qb, Consumer fieldNam .maxExpansions(query.maxExpansions()) .fuzzyTranspositions(query.fuzzyTranspositions()) .lenient(query.lenient()) - .autoGenerateSynonymsPhraseQuery(query.autoGenerateSynonymsPhraseQuery()); + .autoGenerateSynonymsPhraseQuery(query.autoGenerateSynonymsPhraseQuery()) + .boost(query.boost()); return matchQueryBuilder; } else if (qb instanceof final RangeQueryBuilder query) { if (query.relation() != null) { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilderTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilderTests.java index 3d68ce9c7ad96..44b81b96e2154 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilderTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilderTests.java @@ -108,26 +108,49 @@ public void testBuildFromSimpleQuery() { } } - public void testMatchQueryBuilderPropertiesArePreservedFor() { - // the match query has many properties, that all must be preserved after limiting for API Key docs only + public void testPrefixQueryBuilderPropertiesArePreserved() { Authentication authentication = randomFrom( AuthenticationTests.randomApiKeyAuthentication(AuthenticationTests.randomUser(), randomUUID()), AuthenticationTests.randomAuthentication(null, null), null ); - String fieldName = randomFrom( - "username", - "realm_name", - "name", - "type", - "creation", - "expiration", - "invalidated", - "invalidation", - "metadata", - "metadata.what.ever" + String fieldName = randomValidFieldName(); + PrefixQueryBuilder prefixQueryBuilder = QueryBuilders.prefixQuery(fieldName, randomAlphaOfLengthBetween(0, 4)); + if (randomBoolean()) { + prefixQueryBuilder.boost(Math.abs(randomFloat())); + } + if (randomBoolean()) { + prefixQueryBuilder.caseInsensitive(randomBoolean()); + } + if (randomBoolean()) { + prefixQueryBuilder.rewrite(randomAlphaOfLengthBetween(0, 4)); + } + List queryFields = new ArrayList<>(); + ApiKeyBoolQueryBuilder apiKeyMatchQueryBuilder = ApiKeyBoolQueryBuilder.build(prefixQueryBuilder, queryFields::add, authentication); + assertThat(queryFields, hasItem(ApiKeyFieldNameTranslators.translate(fieldName))); + List mustQueries = apiKeyMatchQueryBuilder.must(); + assertThat(mustQueries, hasSize(1)); + assertThat(mustQueries.get(0), instanceOf(PrefixQueryBuilder.class)); + PrefixQueryBuilder prefixQueryBuilder2 = (PrefixQueryBuilder) mustQueries.get(0); + assertThat(prefixQueryBuilder2.fieldName(), is(ApiKeyFieldNameTranslators.translate(prefixQueryBuilder.fieldName()))); + assertThat(prefixQueryBuilder2.value(), is(prefixQueryBuilder.value())); + assertThat(prefixQueryBuilder2.boost(), is(prefixQueryBuilder.boost())); + assertThat(prefixQueryBuilder2.caseInsensitive(), is(prefixQueryBuilder.caseInsensitive())); + assertThat(prefixQueryBuilder2.rewrite(), is(prefixQueryBuilder.rewrite())); + } + + public void testMatchQueryBuilderPropertiesArePreserved() { + // the match query has many properties, that all must be preserved after limiting for API Key docs only + Authentication authentication = randomFrom( + AuthenticationTests.randomApiKeyAuthentication(AuthenticationTests.randomUser(), randomUUID()), + AuthenticationTests.randomAuthentication(null, null), + null ); + String fieldName = randomValidFieldName(); MatchQueryBuilder matchQueryBuilder = QueryBuilders.matchQuery(fieldName, new Object()); + if (randomBoolean()) { + matchQueryBuilder.boost(Math.abs(randomFloat())); + } if (randomBoolean()) { matchQueryBuilder.operator(randomFrom(Operator.OR, Operator.AND)); } @@ -181,6 +204,7 @@ public void testMatchQueryBuilderPropertiesArePreservedFor() { assertThat(matchQueryBuilder2.fuzzyTranspositions(), is(matchQueryBuilder.fuzzyTranspositions())); assertThat(matchQueryBuilder2.lenient(), is(matchQueryBuilder.lenient())); assertThat(matchQueryBuilder2.autoGenerateSynonymsPhraseQuery(), is(matchQueryBuilder.autoGenerateSynonymsPhraseQuery())); + assertThat(matchQueryBuilder2.boost(), is(matchQueryBuilder.boost())); } public void testQueryForDomainAuthentication() { @@ -934,4 +958,19 @@ private void assertQueryFields(List actualQueryFields, QueryBuilder quer assertThat(actualQueryFields, hasItem("creator.realm")); } } + + private static String randomValidFieldName() { + return randomFrom( + "username", + "realm_name", + "name", + "type", + "creation", + "expiration", + "invalidated", + "invalidation", + "metadata", + "metadata.what.ever" + ); + } } From 9b19647f5843064eb822609179ec749bfd25f715 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Tue, 30 Jan 2024 18:13:05 +0200 Subject: [PATCH 8/8] Nit --- docs/reference/rest-api/security/query-api-key.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/rest-api/security/query-api-key.asciidoc b/docs/reference/rest-api/security/query-api-key.asciidoc index 7ff7fe9727f1a..394464dc21456 100644 --- a/docs/reference/rest-api/security/query-api-key.asciidoc +++ b/docs/reference/rest-api/security/query-api-key.asciidoc @@ -64,7 +64,7 @@ NOTE: The queryable string values associated with API keys are internally mapped Consequently, if no <> parameter is specified for a <> query, then the provided match query string is interpreted as a single keyword value. Such a <> query is hence equivalent to a -<> one. +<> query. + .Valid values for `query` [%collapsible%open]