A new search API for API keys - core search function

[ywangd]

This PR adds a new API for searching API keys. The API supports searching API keys with a controlled list of field names and a subset of Query DSL. It also provides a translation layer between the field names used in the REST layer and those in the index layer. This is to prevent tight coupling between the user facing request and index mappings so that they can evolve separately.

Compared to the Get API key API, this new search API automatically applies calling user's security context similar to regular searches, e.g. if the user has only manage_own_api_key privilege, only keys owned by the user are returned in the search response.

Relates: #71023
Supercedes: #73705

[elasticmachine]

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

[ywangd]

As discussed, this is the simple way of applying calling user's security context. It works but is probably not a long term solution. I am open to alternatives.

[tvernum]

My preference would be to do this in an interceptor, but I don't think that's very easy to do.

Logically, it would be a case of checking whether the Role has unconditional access to some action, but that's not actually easy to do with an artibrary Authz Engine, and what action would you use?

We could take an approach like we do with the create_doc index privilege and have different logical action names for the unrestricted query vs the restricted query.

That is,

 public final class QueryApiKeyAction extends ActionType<QueryApiKeyResponse> {

    public static final String QUERY_ALL = NAME + "/all"
}

And then have special case code somewhere like:

if( role.grantsAction(QueryApiKeyAction.QUERY_ALL ) ) {
   request.setFilterForCurrentUser(false);
} else {
   request.setFilterForCurrentUser(true);
}

I don't have a great suggestion, but this approach really bothers me and I don't want to set a precedent that gets reused for other actions. This is definitely not how we want to make this work in general.

[ywangd]

I assume your example is about the interceptor approach. We don't have such an interceptor for cluster actions, but I assume it would work similarly to the interceptors for index requests. The interface is

void intercept(
  RequestInfo requestInfo, 
  AuthorizationEngine authorizationEngine, 
  AuthorizationInfo authorizationInfo,
  ActionListener<Void> listener);

Are you concerning about that, inside this method, we need to only rely on the fact that both AuthorizationEngine and AuthorizationInfo are interfaces and we need do it in a way that does not make RBACEngine and our Role special for at least in this method? It's challenging even with the "separate action name" approach. None of the existing methods fit this purpose really well. We can sorta fit it into AuthorizationEngine#authorizeClusterAction. But it would be better if we can add a new method to AuthorizationEngine. I think we'll need high bandwidth conversation on this.

[ywangd]

I came up with a new approach inspired by your comment. The idea is to authorize the action a second time if it fails for the first time and it is the QueryApiKey action. I think this approach is promising because:

• It is simple. A few lines change in AuthorizationService
• In theory works with arbitrary AuthorizationEngine and AuthorizationInfo
• Does not need a separate action name and works kinda smoothly with the manage_own_api_key privilege.

It has the overhead of a second authorization for the QueryApiKey action. But I think it is acceptable because it authorizing on name is pretty fast and it does not affect any other actions.

[ywangd]

For the time being, this uses the existing findApiKeys method which in turns uses scroll. It will be changed to use normal search once the API supports pagination parameters like size, from etc.

[ywangd]

@elasticmachine update branch

[tvernum]

My preference would be to do this in an interceptor, but I don't think that's very easy to do.

Logically, it would be a case of checking whether the Role has unconditional access to some action, but that's not actually easy to do with an artibrary Authz Engine, and what action would you use?

We could take an approach like we do with the create_doc index privilege and have different logical action names for the unrestricted query vs the restricted query.

That is,

 public final class QueryApiKeyAction extends ActionType<QueryApiKeyResponse> {

    public static final String QUERY_ALL = NAME + "/all"
}

And then have special case code somewhere like:

if( role.grantsAction(QueryApiKeyAction.QUERY_ALL ) ) {
   request.setFilterForCurrentUser(false);
} else {
   request.setFilterForCurrentUser(true);
}

I don't have a great suggestion, but this approach really bothers me and I don't want to set a precedent that gets reused for other actions. This is definitely not how we want to make this work in general.

[tvernum]

Is this special case necessary? Isn't it simpler to always wrap it in a new bool so we know everything works the same way?

[ywangd]

The intention is to always have the final query to be an instance of ApiKeyBoolQueryBuilder because it has special handling to disable certain fields on the SearchExecutionContext level. So yes this is necessary because it converts a normal boolQueryBuilder to a ApiKeyBoolQueryBuilder.

[tvernum]

But can't we just have the if case used all the time? Why doesn't that do what we want?

           QueryBuilder processedQuery = doProcess(queryBuilder);
           finalQuery.must(processedQuery);

[ywangd]

Ah ok. Sorry I see what you mean now. You are right! The must clause can simply accept any query builder including a bool query (now it's kinda obvious after you pointed it out ...) It's a better approach and I assume the end results are the same after lucene level query rewrite. Will update. Thanks!

[tvernum]

Do we want to support ids? Isn't that exposing that API key docs have their doc id set to the api-key-id?

Maybe we need to chat about this one. I think getting the right behaviour for the API Key Id search is important.

[ywangd]

I went back and forth between ids and term. I understand the ids approach exposes the internal knowledge of an API Key document use the API Key ID as its document ID. And this is non-ideal. But the term approach has its own problem:

• How do we handle case-sensitivity? Is it not allowed? It is not obvious and could be trappy either way.
• It's not possible to support prefix, wildcard etc for the _id field, but it is again not obivious just from the Query DSL side.

(I remember I had some other reasons as well, but I cannot remember them right now.)

The bottom line is that the ID field will be an anonamly regardless of the approach. So we need to choose something that are less concerning. If "exposing the doc ID" is a big enough concern, I am happy to take the term (terms) approach.

[tvernum]

Thanks for the explanation.
I assume if in the future the id for the API Key document wasn't exactly the same as the API Key id, (but the key id was a field inside the doc) then we could translate an ids query into a term query.

[ywangd]

Yes it's my thinking as well. Translate ids to a term query (in future) is easy, but not so much the other way around.