Add support for the `type` parameter to the Query API Key API #103695

albertzaharovits · 2023-12-22T16:41:51Z

This adds support for the type parameter to the Query API key API.
The type for an API Key can currently be either rest or cross_cluster.

Relates: #101691

…c case

albertzaharovits · 2023-12-29T12:42:31Z

.../security/src/main/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilder.java

+        "creator.principal",
+        "creator.realm"


I slipped in this tiny unrelated change.
It explicitly mentions creator.principal and creator.realm as fields allowed to be referred to in queries, rather than the current "anything that starts with creator." approach. I think this slightly simplifies the understanding of it all (only metadata.* is allowed in a wildcard fashion), since only these two fields can be handled by ApiKeyFieldNameTranslators#FIELD_NAME_TRANSLATORS and anything else errors.

Makes sense, thanks for the clean up!

elasticsearchmachine · 2023-12-29T12:45:51Z

Hi @albertzaharovits, I've created a changelog YAML for you.

elasticsearchmachine · 2023-12-29T12:45:51Z

Pinging @elastic/es-security (Team:Security)

n1v0lg

LGTM!

One follow up we should keep track of is to see if we need to update the API spec (e.g., see here: elastic/elasticsearch-specification#2371).

.../security/src/main/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilder.java

n1v0lg · 2024-01-10T09:25:12Z

.../security/src/main/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilder.java

+        "creator.principal",
+        "creator.realm"


Makes sense, thanks for the clean up!

n1v0lg · 2024-01-10T10:19:47Z

...src/main/java/org/elasticsearch/xpack/security/action/apikey/TransportQueryApiKeyAction.java

-        );
+        final AtomicBoolean accessesApiKeyTypeField = new AtomicBoolean(false);
+        final ApiKeyBoolQueryBuilder apiKeyBoolQueryBuilder = ApiKeyBoolQueryBuilder.build(request.getQueryBuilder(), fieldName -> {
+            if (API_KEY_TYPE_RUNTIME_MAPPING_FIELD.equals(fieldName)) {


Wondering if we can skip the boolean ref here and do this directly, in the lambda:

// only add the query-level runtime field to the search request if it's actually referring the "type" field if (API_KEY_TYPE_RUNTIME_MAPPING_FIELD.equals(fieldName)) { searchSourceBuilder.runtimeMappings(API_KEY_TYPE_RUNTIME_MAPPING); }

Ah no that wouldn't work because we need this to happen only once

Well... I don't think it needs to happen once, searchSourceBuilder.runtimeMappings(API_KEY_TYPE_RUNTIME_MAPPING) can be invoked multiple times, but I would still prefer the current implementation.

n1v0lg · 2024-01-10T10:39:49Z

.../qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/QueryApiKeyIT.java

+        apiKeyIdsSubsetDifference.removeAll(apiKeyIdsSubset);
+
+        List<String> apiKeyRestTypeQueries = List.of("""
+            {"query": {"term": {"type": "rest" }}}""", """


Optional, but could also add a NOT cross_cluster query here since that's probably a query end-users might try.

n1v0lg · 2024-01-10T10:51:25Z

.../security/src/main/java/org/elasticsearch/xpack/security/support/ApiKeyBoolQueryBuilder.java

            finalQuery.must(processedQuery);
        }
        finalQuery.filter(QueryBuilders.termQuery("doc_type", "api_key"));
+        fieldNameVisitor.accept("doc_type");


It feels a little off to pass this in like that but I don't have a good alternative suggestion. I had hoped we'd have a generic utility method that allows us to get all field names from a QueryBuilder but we don't.

We could think about turning the consumer into a dedicated class but not worth it for now IMO.

So I'm ++ on this approach as is.

I took inspiration from the field visitor that lucene queries implement:

elasticsearch/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCache.java

Line 67 in 4b21499

weight.getQuery().visit(new QueryVisitor() {

, but alas query builders don't have that facility.

albertzaharovits · 2024-01-10T15:59:22Z

One follow up we should keep track of is to see if we need to update the API spec (e.g., see here: elastic/elasticsearch-specification#2371).

I haven't considered it, but I don't think we need to do anything, the spec mentions the parameters of the API, but this PR extends the language of one of the parameters (the query one).

…ecurity/support/ApiKeyBoolQueryBuilder.java Co-authored-by: Nikolaj Volgushev <[email protected]>

…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.

* Change release version lookup to an instance method (#104902) * Upgrade to Lucene 9.9.2 (#104753) This commit upgrades to Lucene 9.9.2. * Improve `CANNOT_REBALANCE_CAN_ALLOCATE` explanation (#104904) Clarify that in this situation there is a rebalancing move that would improve the cluster balance, but there's some reason why rebalancing is not happening. Also points at the `can_rebalance_cluster_decisions` as well as the node-by-node decisions since the action needed could be described in either place. * Get from translog fails with large dense_vector (#104700) This change fixes the engine to apply the current codec when retrieving documents from the translog. We need to use the same codec than the main index in order to ensure that all the source data is indexable. The internal codec treats some fields differently than the default one, for instance dense_vectors are limited to 1024 dimensions. This PR ensures that these customizations are applied when indexing document for translog retrieval. Closes #104639 Co-authored-by: Elastic Machine <[email protected]> * [Connector Secrets] Add delete API endpoint (#104815) * Add DELETE endpoint for /_connector/_secret/{id} * Add endpoint to write_connector_secrets cluster privilege * Merge Aggregations into InternalAggregations (#104896) This commit merges Aggregations into InternalAggregations in order to remove the unnecessary hierarchy. * [Profiling] Simplify cost calculation (#104816) * [Profiling] Add the number of cores to HostMetadata * Update AWS pricelist (remove cost_factor, add usd_per_hour) * Switch cost calculations from 'cost_factor' to 'usd_per_hour' * Remove superfluous CostEntry.toXContent() * Check for Number type in CostEntry.fromSource() * Add comment * Retry get_from_translog during relocations (#104579) During a promotable relocation, a `get_from_translog` sent by the unpromotable shard to handle a real-time get might encounter `ShardNotFoundException` or `IndexNotFoundException`. In these cases, we should retry. This is just for `GET`. I'll open a second PR for `mGET`. The relevant IT is in the Stateless PR. Relates ES-5727 * indicating fix for 8.12.1 for int8_hnsw (#104912) * Removing the assumption from some tests that the request builder's request() method always returns the same object (#104881) * [DOCS] Adds get setting and update settings asciidoc files to security API index (#104916) * [DOCS] Adds get setting and update settings asciidoc files to security API index. * [DOCS] Fixes references in docs. * Reuse APMMeterService of APMTelemetryProvider (#104906) * Mute more tests that tend to leak searchhits (#104922) * ESQL: Fix SearchStats#count(String) to count values not rows (#104891) SearchStats#count incorrectly counts the number of documents (or rows) in which a document appears instead of the actual number of values. This PR fixes this by looking at the term frequency instead of the doc count. Fix #104795 * Adding request source for cohere (#104926) * Fixing a broken javadoc comment in ReindexDocumentationIT (#104930) This fixes a javadoc comment that was broken by #104881 * Fix enabling / disabling of APM agent "recording" in APMAgentSettings (#104324) * 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. * Apply publish plugin to es-opensaml-security-api project (#104933) * Support `match` for the Query API Key API (#104594) This adds support for the `match` query type to the Query API key Information API. Note that since string values associated to API Keys are mapped as `keywords`, a `match` query with no analyzer parameter is effectively equivalent to a `term` query for such fields (e.g. `name`, `username`, `realm_name`). Relates: #101691 * [Connectors API] Relax strict response parsing for get/list operations (#104909) * Limit concurrent shards per node for ESQL (#104832) Today, we allow ESQL to execute against an unlimited number of shards concurrently on each node. This can lead to cases where we open and hold too many shards, equivalent to opening too many file descriptors or using too much memory for FieldInfos in ValuesSourceReaderOperator. This change limits the number of concurrent shards to 10 per node. This number was chosen based on the _search API, which limits it to 5. Besides the primary reason stated above, this change has other implications: We might execute fewer shards for queries with LIMIT only, leading to scenarios where we execute only some high-priority shards then stop. For now, we don't have a partial reduce at the node level, but if we introduce one in the future, it might not be as efficient as executing all shards at the same time. There are pauses between batches because batches are executed sequentially one by one. However, I believe the performance of queries executing against many shards (after can_match) is less important than resiliency. Closes #103666 * [DOCS] Support for nested functions in ES|QL STATS...BY (#104788) * Document nested expressions for stats * More docs * Apply suggestions from review - count-distinct.asciidoc - Content restructured, moving the section about approximate counts to end of doc. - count.asciidoc - Clarified that omitting the `expression` parameter in `COUNT` is equivalent to `COUNT(*)`, which counts the number of rows. - percentile.asciidoc - Moved the note about `PERCENTILE` being approximate and non-deterministic to end of doc. - stats.asciidoc - Clarified the `STATS` command - Added a note indicating that individual `null` values are skipped during aggregation * Comment out mentioning a buggy behavior * Update sum with inline function example, update test file * Fix typo * Delete line * Simplify wording * Fix conflict fix typo --------- Co-authored-by: Liam Thompson <[email protected]> Co-authored-by: Liam Thompson <[email protected]> * [ML] Passing input type through to cohere request (#104781) * Pushing input type through to cohere request * switching logic to allow request to always override * Fixing failure * Removing getModelId calls * Addressing feedback * Switching to enumset * [Transform] Unmute 2 remaining continuous tests: HistogramGroupByIT and TermsGroupByIT (#104898) * Adding ActionRequestLazyBuilder implementation of RequestBuilder (#104927) This introduces a second implementation of RequestBuilder (#104778). As opposed to ActionRequestBuilder, ActionRequestLazyBuilder does not create its request until the request() method is called, and does not hold onto that request (so each call to request() gets a new request instance). This PR also updates BulkRequestBuilder to inherit from ActionRequestLazyBuilder as an example of its use. * Update versions to skip after backport to 8.12 (#104953) * Update/Cleanup references to old tracing.apm.* legacy settings in favor of the telemetry.* settings (#104917) * Exclude tests that do not work in a mixed cluster scenario (#104935) * ES|QL: Improve type validation in aggs for UNSIGNED_LONG and better support for VERSION (#104911) * [Connector API] Make update configuration action non-additive (#104615) * Save allocating enum values array in two hot spots (#104952) Our readEnum code instantiates/clones enum value arrays on read. Normally, this doesn't matter much but the two spots adjusted here are visibly hot during bulk indexing, causing GBs of allocations during e.g. the http_logs indexing run. * ESQL: Correct out-of-range filter pushdowns (#99961) Fix pushed down filters for binary comparisons that compare a byte/short/int/long with an out of range value, like WHERE some_int_field < 1E300. * [DOCS] Dense vector element type should be float for OpenAI (#104966) * Fix test assertions (#104963) * Move functions that generate lucene geometries under a utility class (#104928) We have functions that generate lucene geometries scattered in different places of the code. This commit moves everything under a utility class. * fixing index versions --------- Co-authored-by: Simon Cooper <[email protected]> Co-authored-by: Chris Hegarty <[email protected]> Co-authored-by: David Turner <[email protected]> Co-authored-by: Jim Ferenczi <[email protected]> Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Navarone Feekery <[email protected]> Co-authored-by: Ignacio Vera <[email protected]> Co-authored-by: Tim Rühsen <[email protected]> Co-authored-by: Pooya Salehi <[email protected]> Co-authored-by: Keith Massey <[email protected]> Co-authored-by: István Zoltán Szabó <[email protected]> Co-authored-by: Moritz Mack <[email protected]> Co-authored-by: Costin Leau <[email protected]> Co-authored-by: Jonathan Buttner <[email protected]> Co-authored-by: Albert Zaharovits <[email protected]> Co-authored-by: Mark Vieira <[email protected]> Co-authored-by: Jedr Blaszyk <[email protected]> Co-authored-by: Nhat Nguyen <[email protected]> Co-authored-by: Abdon Pijpelink <[email protected]> Co-authored-by: Liam Thompson <[email protected]> Co-authored-by: Liam Thompson <[email protected]> Co-authored-by: Przemysław Witek <[email protected]> Co-authored-by: Joe Gallo <[email protected]> Co-authored-by: Lorenzo Dematté <[email protected]> Co-authored-by: Luigi Dell'Aquila <[email protected]> Co-authored-by: Armin Braun <[email protected]> Co-authored-by: Alexander Spies <[email protected]> Co-authored-by: David Kyle <[email protected]>

Runtime field support

23c2053

albertzaharovits self-assigned this Dec 22, 2023

elasticsearchmachine added the v8.13.0 label Dec 22, 2023

albertzaharovits added 20 commits December 27, 2023 14:04

Merge branch 'main' into query-api-key-type-runtime-field

1db1806

WIP meh

c84b841

WIP

ba575e1

Painless

6b3d7b6

Okish test

c465cde

An IT

aace531

Merge branch 'main' into query-api-key-type-runtime-field

a1c7d61

Nits

e4d64b1

WTF

d13e199

IT is polished now

e8dba56

Remove IT nit fixture

8ae29f2

Don't try to deserialize non-rest keys

7d666ad

ApiKeyBackwardsCompatibilityIT

19a1e72

ApiKeyBackwardsCompatibilityIT

2673e61

Merge branch 'main' into query-api-key-type-runtime-field

6dc9189

only API keys created pre-8.9 are relevant for the rest-type query bw…

05653c5

…c case

testQueryCrossClusterApiKeysByType

859fa4b

Merge branch 'main' into query-api-key-type-runtime-field

38458f1

ApiKeyBoolQueryBuilderTests

5ed7840

javadoc and docs

3dfe94a

albertzaharovits commented Dec 29, 2023

View reviewed changes

albertzaharovits added >enhancement :Security/Security Security issues without another label labels Dec 29, 2023

albertzaharovits requested a review from n1v0lg December 29, 2023 12:45

albertzaharovits marked this pull request as ready for review December 29, 2023 12:45

elasticsearchmachine added the Team:Security Meta label for security team label Dec 29, 2023

albertzaharovits changed the title ~~Runtime field support for the Query API Key API~~ Add support for the type parameter to the Query API Key API Dec 29, 2023

albertzaharovits changed the title ~~Add support for the type parameter to the Query API Key API~~ Add support for the type parameter to the Query API Key API Dec 29, 2023

n1v0lg approved these changes Jan 10, 2024

View reviewed changes

Merge branch 'main' into query-api-key-type-runtime-field

20727d1

albertzaharovits force-pushed the query-api-key-type-runtime-field branch from 944541d to 20727d1 Compare January 10, 2024 16:00

albertzaharovits and others added 2 commits January 10, 2024 18:01

Update x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/s…

c64b574

…ecurity/support/ApiKeyBoolQueryBuilder.java Co-authored-by: Nikolaj Volgushev <[email protected]>

Test nit

fb69b9e

albertzaharovits merged commit f4aaa20 into elastic:main Jan 11, 2024
19 checks passed

albertzaharovits deleted the query-api-key-type-runtime-field branch January 11, 2024 08:53

albertzaharovits mentioned this pull request Jan 22, 2024

Add type parameter support, for sorting, to the Query API Key API #104625

Merged

albertzaharovits mentioned this pull request Feb 19, 2024

Enhance Query API key information API #101691

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for the `type` parameter to the Query API Key API #103695

Add support for the `type` parameter to the Query API Key API #103695

albertzaharovits commented Dec 22, 2023 •

edited

Loading

albertzaharovits Dec 29, 2023

n1v0lg Jan 10, 2024

elasticsearchmachine commented Dec 29, 2023

elasticsearchmachine commented Dec 29, 2023

n1v0lg left a comment

n1v0lg Jan 10, 2024

n1v0lg Jan 10, 2024

n1v0lg Jan 10, 2024

albertzaharovits Jan 10, 2024

n1v0lg Jan 10, 2024

n1v0lg Jan 10, 2024

albertzaharovits Jan 10, 2024

albertzaharovits commented Jan 10, 2024

Add support for the type parameter to the Query API Key API #103695

Add support for the type parameter to the Query API Key API #103695

Conversation

albertzaharovits commented Dec 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticsearchmachine commented Dec 29, 2023

elasticsearchmachine commented Dec 29, 2023

n1v0lg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

albertzaharovits commented Jan 10, 2024

Add support for the `type` parameter to the Query API Key API #103695

Add support for the `type` parameter to the Query API Key API #103695

albertzaharovits commented Dec 22, 2023 •

edited

Loading