With the old implementation of getHTTPAuthenticationScheme, .toLowerCase() was being called when initially parsing the scheme from the request's headers. This felt inconsistent when I began to use the HTTPAuthorizationHeader class and .toString() to create the Authorization headers themselves. It has the effect of requiring consumers to call .toLowerCase(), or instead use .localeCompare, to treat this as being case insensitive.

@azasypkin I'm interested in how you feel about this.

It's fine by me as long as we mention this in the property JS docs. I'm curious if there is any reason we cannot do toLowerCase() for both scheme and credentials in HTTPAuthorizationHeader constructor though?

We definitely can do this in the constructor. When we were previously building the Authorization header manually within the auth providers, we were using a capitalized version, for example: Bearer ${accessToken}. Changing this behavior for the sake of making matching the scheme easier felt wrong. However, I don't want to be introducing future bugs by being pedantic... I wish there was CaseInsensitiveString in JavaScript which overrode equals, etc. but AFAIK there is no such thing, or no ability to create one ourselves which would work with things like a Map and a Set.

Changing this behavior for the sake of making matching the scheme easier felt wrong. However, I don't want to be introducing future bugs by being pedantic...

Yeah, I agree. We use and compare schemes just in a couple of places, I think the risk of accidentally breaking something in this area may be negligible.

We chatted a bit about the function naming. Did you decide to keep as is or go with something like grantAsInternalUser?

note: would you mind also updating https://github.com/elastic/kibana/blob/master/x-pack/plugins/security/server/routes/users/change_password.ts#L46 to use HTTPAuthorizationHeader?

You got it!

optional nit: how do you feel about something like this instead?

question: hmm I'm surprised that Elasticsearch doesn't allow client to specify Api Key name like it does in create API key API. Do you know why? Just out of curiosity.

I don't, unfortunately. It also doesn't allow you to specify a role descriptor. It's not blocking Alerting's usage, so I :ostrich_head_in_sand:'ed it.

question: I guess we don't need asScoped here (as well as as GrantAPIKeyResult)?

The conceptual equivalent to an asScoped "grant" is just createAPIKey. It's safe for us to always allow end-users to use the create API Key API in Elasticsearch and ES will enforce all of the necessary authorization for this to be safe. The ability to grant an API Key in this manner is a new "two credentials API" where the Kibana server should only be able to perform the operation when it has the user's credentials, and the result of the operation should be safe-guarded...

Yeah, but I what I meant is that you're using callAsInternalUser and callAsInternalUser on ScopedClusterClient is exactly the same as on non-scoped ClusterClient and doesn't use request headers you scoped client with. So unless I'm missing something clusterClient.asScoped(request).callAsInternalUser is a bit less performant equivalent of just clusterClient.callAsInternalUser.

I'm with you now, apologies for being dense. Fix forthcoming.

note: I'm not super happy that we should do that, but I can't think of a better solution for the time being as well. So +1.

Just for the records: my main concern is that the fact that we have Authorization header within request.headers is just "temporary" backward compatibility escape hatch for the places that still rely on legacy requests. The idea was to store authHeaders returned from auth hook internally in the core so that it can add them automatically whenever authenticated requested is relayed to ES. And nothing else would have access to them.

However we (Security) register and own this hook, provide auth headers and hence should have an exclusive right to access these headers whenever we need them (e.g. via Core "auth headers accessor" returned as part of the auth hook registration). So even if Core decides to no longer populate request.headers with Authorization we'll still have access to them via some API. Migrating to this API will be an implementation detail that won't change public API we expose unless I'm missing something.

What you've said makes sense. I'm also open to alternatives which require some changes to the authentication providers. I began exploring what it'd look like to allow the auth providers to store the parameters we need to grant an API key in a WeakMap<KibanaRequest, GrantAPIKeyParams> that only the security plugin had access to, but then came on the necessity to parse the Authorization header within the http authentication provider and decided to just do the header parsing logic here.

Yeah, I think what you have now is the best option for the time being. Parsing headers because of http is where I ended up as well when we had initial discussion on that requirement.

It feels we're still missing some pieces either in Security plugin or in the Core that would have allowed us to come with a more future-proof approach, but I'm sure we'll get there and reuse the work you've done here anyway.