Service Accounts - token name in response to Authenticate API

[ywangd]

This PR adds the service account token name in a new top level field, token, to the authenticate response. For now, this new field is only shown when the authentication is for a service account.

[elasticmachine]

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

[tvernum]

The implication of this is that the User object is dependent on the credential used, and it's no longer strictly true that the credentials are purely an authentication feature.
For example, it would mean that it would be possible to have a DLS template that tried to use the token name and has a different effective role per credential.

Are we sure that's what we want?

[ywangd]

The implication of this is that the User object is dependent on the credential used, and it's no longer strictly true that the credentials are purely an authentication feature.
For example, it would mean that it would be possible to have a DLS template that tried to use the token name and has a different effective role per credential.

Are we sure that's what we want?

It's a good point. Given we explicitly said different tokens will not be different in terms of privileges, it is definitely non-ideal to have the potential DLS/FLS discrepency. Though I think it is unlikely for service accounts to have DLS/FLS, the theorectical possibility still makes the decision questionable.

The other option that I have considered is to handle it in the place where the authenticate response is assembled (Authentication#toXContentFragment). Basically this means: if the authentication object is for a service account (realmName == service_account), add the token name field into the metadata section of the response object. I didn't choose this approach because it feels that the logic is leaked out from the service account domain. But given the issue you raised, I think we'll just have to go with this approach. What do you think?

[ywangd]

I updated the PR to move the "token name in response" logic into the place where the authentication response is assembled, i.e. Authentication#toXContent. It is ready for another round.

[tvernum]

I think we actually want to put this as a first class concept in the response, not just bury it in the metadata.
We're going to need to do the same thing for #70306

[ywangd]

I think we actually want to put this as a first class concept in the response, not just bury it in the metadata.
We're going to need to do the same thing for #70306

We can add a new top level field of authentication_metadata. I didn't do it because the spec we worked on says "user metadata" (maybe I was reading into it too literally). I agree it is an useful concept and is clearly different from "user metadata".

Do you think it should be made more general, i.e. the new field will contain any metadata that the Authentication object has to offer. Or should it be more specific to _token_name for the time being since we may need sometime to scrunitize what currently available is available in the authentication metadata? I am not sure whether the former would impact too many files. So maybe we could start with the more specific option? We can also mandate a new request parameter for the authentication_metadata, but I don't think I like it.

[tvernum]

I didn't do it because the spec we worked on says "user metadata" (maybe I was reading into it too literally)

I think you're reading it correctly - but it's just not as good an idea as it seemed when I wrote it.

I'm not arguing for authentication_metadata, I'm arguing for something clear and explicit.

For #70306, I think we want to change the API response to be:

{
  "username": "user",
  "api_key" : {
    "name": "sample api key",
    "id": "8IKUhXgBarhMELLcgNuB"
  }
  "roles": [],
  "full_name": null,
  "email": null,
  "metadata": {
    "_reserved": true
  },
  "enabled": true,
  "authentication_realm": {
    "name": "_es_api_key",
    "type": "_es_api_key"
  },
  "lookup_realm": {
    "name": "_es_api_key",
    "type": "_es_api_key"
  },
  "authentication_type": "api_key"
}

(And maybe do something about the realm as well).
Do you agree? My argument here is primarily based on my view of where we're heading in the future - so getting some sort of consensus here matters.

If we assume that we're going to go in that direction for API keys, then what it the appropriate change to make here?
I don't think it's to add the token name to metadata nor is it to create authentication_metadata. It's to add something that is designed for this scenario. Maybe that's "service_account" : { ... } or "token": { ... } or "credential" : { ... }, I'm not sure, but I think we need to think about it at a higher level than just metadata.

[ywangd]

@tvernum I did some more thinking about this. I was initially in favor of a more generic authentication_metadata field to avoid proliferation of top level fields because each top level field comes with some overhead:

1. SetSecurityUser processor may need more flags, one for each top level field.
2. It needs a parse field and upate to the constructor signature for HLRC

But a generic authentication_metadata field also has its own issues. The main one is confusion because:

1. This field name is naturally confusing because of the existing metadata field. We can clarify with documentation, but the confusion will always be there (it could be helpful if we could rename metadata to user_metadata, but that's a breaking change ...)
2. The Authentication#metadata field is used for all sorts of things. It is like a dumping ground for all internal information including API Key role descriptors. Most of them are implementation details and are not supposed to be exposed to end-users. So we will need to filter for it which causes the 2nd confusion on why authentication_metadata and Authentication#metadata are not the same.

I think overall I prefer your proposal because the confusion issue is not only confusing but also feels unprofessional. In terms of the actual field name, I slightly prefer token. The current PR has it as a _token_name field of metadata. So if we lift it up, the name token seems natural. The information really is more about the token and less about the service account. Other information about the service account is either part of user already or should not be shown here (role_descriptor). Also if we allow basic auth for service account in the future, the token field can be dropped from the auth response naturally. But if the field is service_account, it feels weird to either drop it or show it as an empty map.

[tvernum]

I slightly prefer token.

Do you think we would populate that field if the request was authenticated with an OAuth access token? (or perhaps, in the future, a JWT?)
I'm inclined to have a rough idea of the direction we think this should go in before we add it, if that's possible.

[ywangd]

Do you think we would populate that field if the request was authenticated with an OAuth access token? (or perhaps, in the future, a JWT?)

Yes I consider this new field as an expansion to the authentication_type field. This is another reason why I prefer the name token. The field name api_key follows the same principle. So assume JWT also has authentication_type of token, we can populate this field for extra info about the JWT itself.

[ywangd]

The PR is updated to have the token name in a new top level token field, e.g.:

{
  "username": "elastic/fleet-server",
  "roles": [],
  "full_name": "Service account - elastic/fleet-server",
  "email": null,
  "token": {
    "name": "token2"
  },
  "metadata": {
    "_elastic_service_account": true
  },
  "enabled": true,
  "authentication_realm": {
    "name": "service_account",
    "type": "service_account"
  },
  "lookup_realm": {
    "name": "service_account",
    "type": "service_account"
  },
  "authentication_type": "token"
}

[ywangd]

@elasticmachine update branch