Search API keys with a subset of Query DSL

[ywangd]

This PR aims to provide a more feature rich search experience for API keys. It allows a subset of Query DSL to be specified for fields that are explicilty allowed. This change enables the followings:

• Arbitrary logic relationships (AND, OR etc)
• Search the user provided metadata
• Range query for date fields (creation_time for now)
• Support bool, term, terms, prefix, wildcard and range query types

[ywangd]

Notes for reviewers

This purpose of this draft PR is to demostrate the overall approach. It will be polished (or even rewritten) if the overall approach is agreed.

I went back and forth with different ideas and settled down with the following two design decisions:

1. The search query is processed and rewritten at elasticsearch's QueryBuilder level instead of Lucene's Query level.
2. Fields like username, realm_name, id, owner are kept outside of the Query DSL.

The main reason for first decision is simplicity. Since we have tight control of the supported query types and fields, it is sufficient to process them at the query DSL level. Processing at Lucene level is not necessary and technically very challenging because it is outside of security domain and is hard to connect them together.

The second decision was made mainly to work with the manage_own_api_key privilege, which needs to know values of these field from the GetApiKeyRequest. It may be technically possible to extract the values from the Query DSL. But I think it is not worth the complexity. For example, the username, realm_name pair can be specified at any nested level of either must or filter clauses. It is also possible to specify them in a should clause and minimum_should_match: 2. They can also be specified in either a term query or a single item terms query or a wildcard query without wildcard. To count for all these different combinations is an overkill in my opinion. In addition, the owner parameter does not have an equivalent in Query DSL anyway. So we need at least keep it separate. Another argument for keeping these fields separate is that they would resemble existing APIs better and offer similarity. We might need to discuss and rethink how manage_own_api_key work in future, but it should be of its own piece of work. For this PR, I prefer to keep it focus on just the search part and maintain other existing semantics.

Another issue is what the API endpoint should be. This PR works by add a request body to GET /_security/api_key. However, it is not recommended to have a GET API endpoint with request body without also provide an equivalent POST version. In fact, YAML test error out if it finds such an API. But POST /_security/api_key is already taken for creating API keys. So it seems we might after all need a new API for the new search function, something like GET /_security/_search/api_key?

PS: This test class is a quick way to see what queries are supported (or not) for API keys.

[tvernum]

This is interesting, and I think it suggests that this is a path we can make work.
Has anyone from the search area looked at it? I wonder whether there's other (better) ways of doing it.

I have lots of ideas about specifics (endpoint naming, existing parameters, etc) but they're details we can work through once we're happy we have a clear direction.

[ywangd]

Has anyone from the search area looked at it? I wonder whether there's other (better) ways of doing it.

Not yet. I can message Jim to ask who can help out in the search area team.

[jimczi]

I left one comment regarding the setter for the allowed field names but the approach looks good to me.

[jimczi]

The context object has some mutable variables that we should preserve in the mutation. So instead of creating a new object we could add a setter for the allowed field names like we do for setAllowUnmappedFields ?

[ywangd]

I wasn't sure between using setter or constructor. Thanks for the tip. I changed it to use setter.

[jimczi]

this can be extracted in a static variable ?

[ywangd]

Yep it was done in a rush. Extracted now.

[jimczi]

Why not building an ApiKeyBoolQueryBuilder directly ?

[ywangd]

Good point. No need to keep both ApiKeySearchQueryBuidler and ApiKeyBoolQueryBuilder. I consolidated everything into ApiKeyBoolQueryBuilder. Thanks!

[ywangd]

Closing this draft PR in favor of #75335.