Can API operations that create access tokens include respective full user info in the response?

[azasypkin]

Context: elastic/kibana#68117 (comment)

Currently every Elasticsearch API call that creates an access token made by Kibana is followed by the request to retrieve full user information (GET /_security/_authenticate):

• /_security/oauth2/token (Kerberos and Token)
• /_security/delegate_pki (PKI)
• /_security/saml/authenticate (SAML)
• /_security/oidc/authenticate (OIDC)

Even though it's not technically required in some cases today we do this for the consistency sake. But soon we're going to implement features that would require this additional round-trip in all cases, e.g. to limit number of currently active sessions based on various criteria (username+realm or specific role name).

Currently /_security/saml/authenticate and /_security/oidc/authenticate return username, but based on the example above in the near future we may need to know user roles and whatnot.

The question we have is, would it make sense to attach full user info right to the response of the operations that produce access tokens (or at least for some of them)?

@elastic/kibana-security

[elasticmachine]

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

[ywangd]

It should be relatively straightforward to add response of GET /_security/_authenticate as a field to the responses of "get token" related APIs. The response will be something like the follows:

{
  "access_token": "46ToAxZFNTJubnEwZVRwNmJIc1hUdEFVbGdB",
  "type": "Bearer",
  "expires_in": 1200,
  "refresh_token": "46ToAxZETzA3RElpa1REaXdXYjNqaGNpTkln",
  "authentication": {
    "username": "foo",
    "roles": [
      "user"
    ],
    "full_name": null,
    "email": null,
    "metadata": {},
    "enabled": true,
    "authentication_realm": {
      "name": "native1",
      "type": "native"
    },
    "lookup_realm": {
      "name": "native1",
      "type": "native"
    }
  }
}

NOTE: The authentication object is of the user that the token is created for, not the facilitating user, e.g. kibana.

For SAML and OIDC authenticate calls, there will be some redundancy for username and realm since they will be repeated again in the new authentication field. But I don't think we should remove the fields from either the top level object or the authentication field because:

• It will be a breaking change if they are removed from the top level object.
• It should not be removed from the authentication field since some of the top level objects do not have username and realm. I don't think it is worth to conditionally include and exclude these fields from authentication depending on the top level object. It's more trouble than its worth. Also an authentication field without username is weird at least.
• The redundancy is pretty minimal.

Here are a list of things we need to touch to implement above suggested changes:

• /_security/oauth2/token - Most changes should be in CreateTokenResponse and its caller, i.e. TransportCreateTokenAction and TransportRefreshTokenAction
• /_security/delegate_pki - DelegatePkiAuthenticationResponse and TransportDelegatePkiAuthenticationAction
• /_security/saml/authenticate - SamlAuthenticateResponse and TransportSamlAuthenticateAction
• /_security/oidc/authenticate - OpenIdConnectAuthenticateResponse and TransportOpenIdConnectAuthenticateAction

[azasypkin]

That would work great for us, thanks for picking this up @ywangd !

[azasypkin]

Thanks for improving this @BigPandaToo ! Filed a Kibana issue to leverage this on our side: elastic/kibana#80952.

[ywangd]

I just noticed a minor issue in the new authentication field. The authentication.authentication_type should always be token. But instead it is the type of the original authentication that the token is created for. For example, in the documentation page, the authentication_type is realm, but should be token.

@BigPandaToo The fix will likely be similar to what is needed for the test failure issue. So these two can go in with one PR. I can work on it if you are busy with other things. Just let me know. Thanks!

Scratch that. After team discussion. We agree the current behaviour is more correct.