Authenticate API should return the roles that are inhereted from enabled anonymous access

[jkakavas]

Authenticated users inherit the roles that are specified for anonymous users. There is already an issue tracking making this more obvious in the documentation.

The _authenticate API should also return the anonymous roles in the response along with the roles that the authenticated user is assigned and/or mapped to.

[elasticmachine]

Pinging @elastic/es-security

[tvernum]

@ywangd I've assigned this to you, no rush, but I think we should get it sorted when we can.

I don't know whether we want to add the anonymous role to theroles field, or add a new anonymous_role field, or something like that, but I think you and @jkakavas could come up with an agreement on that.

[ywangd]

Thanks @tvernum

@jkakavas From the surface, I'd prefer a new field for it because of clarity. Otherwise users may have new confusion from another direction, i.e. from "where are my missing roles" to "what are these extra roles". I'd personally name it to something like inherited_roles. But I understand it could be a pre-mature "optimization" unless we could extend this concept to tokens, api keys or something else.

Not sure whether there is any backwards compatibility issue for adding a new field. Hope it's not too much to justify a new field.

[jkakavas]

We generally are fine with adding fields to responses, these are not considered breaking changes and we have the tooling in our serialization logic to take care of version guards. We recently ( in a minor in 6.x ) added the realm information in the _authenticate response too.

I agree that the new field fits the principle of least astonishment better

[ywangd]

We had a fair amount of discussion since last post here. This issue is getting more involved and here is a status update based on the discussion during team meeting:

TLDR: We prefer to have anonymous roles listed under existing roles field. I personally prefer to solve the immediate problem first and create a separate issue for relocating the logic of anonymous role resolving.

First of all, on the contrary to the initial discussion, we now agree that the anonymous roles should be just part of the existing roles field instead of a new one. I think the single strong argument is from @jkakavas as the follows:

The purpose of the API is to tell us which roles the user has, but not why they have them.

The second question is where the anonymous role should be handled. The lifecycle of a role goes through following phases:

1. Get role name (RoleMapper)
2. Retrieve role descriptor (RolesStore, BiConsumer functional interface)
3. Build role object (CompositeRolesStore, which is not a RolesStore)

In general, step 1 is handled in authentication phase, while step 2 and 3 are handled in authorization phase. However, the anonymous role is a special case in that its role names are handled in authorization phase, specifically CompositeRolesStore#getRoles. Consequently, these role names are not available in the authentication object. This has two implications:

1. The anonymous role names are inconvenient to access when constructing the response for /_security/_authenticate. This belongs to the bug we are trying to fix here.
2. The authentication object is no longer the source of truth for user roles, which is a deeper problem and can possibly be handled in a separate issue.

If we just focus on issue 1, we can add new logic in TransportAuthenticateAction to retrieve the full list of user roles from the transient data in threadContext, details can be found here.

For issue 2, we need to move some logic from CompositeRolesStore#getRoles to the authentication phase. Based on my reading, there is no a single choke point in authentication where this logic can be applied. We need at least add it in two places, Authenticator#handleNullToken and Authenticator#finishAuthentication. There will also be a behaviour change (possibly breaking change?). The anonymous role settings are node scope, current handling in the authorization phase means they could be different depends on which node performs authorization. If we move this logic into authentication, the anonymous role will then just depend on the node where authentication is performed, because the roles are transfered across wires as part of authentication and honored by all nodes.

Base on above analysis, I prefer to fix issue 1 first and create a separate issue to deal with issue 2. This way we treat the symptom first for immediate user benefit and buy more time to think through on issue 2 and its implications. Once issue 2 is fixed, it should be simple to leverage it to refactor issue 1 (most likely deleting code).

[jkakavas]

There will also be a behaviour change (possibly breaking change?). The anonymous role settings are node scope, current handling in the authorization phase means they could be different depends on which node performs authorization. If we move this logic into authentication, the anonymous role will then just depend on the node where authentication is performed, because the roles are transfered across wires as part of authentication and honored by all nodes.

Authentication is by design node specific so I don't see any issue here.

Base on above analysis, I prefer to fix issue 1 first and create a separate issue to deal with issue 2.

++

[ywangd]

Authentication is by design node specific so I don't see any issue here.

But that specific node will be different after the code change. Say we have a 2-node cluster, A and B and they have different anonymous role settings, a and b, respectively. The user authenticates against node A and request is also passed to node B from A.

Currently, the logic is as follows:

user  -->   A   -->   B
           (a)       (b)  <- runtime anonymous role

After the change, it will become

user  -->   A   -->   B
           (a)       (a)  <- runtime anonymous role

Note that the runtime anonymous role is change to a instead of b for node B . This could be what we want since it feels more inline with other authentication behaviours, i.e. authenticating node specific. But the behaviour is different nonetheless.

[jkakavas]

Gotcha. I think I see it as current behavior is a bug and we should fix it, rather than we're making a breaking change in expected behavior.

[ywangd]

I have a change of heart again. I'd like to tackle both issues together. Because fixing issue 1 alone feels hacky and the solution does not do exactly what I want in all cases. The role names are sanitized, deduplicated and compacted during authorization. Although they do represent the final runtime privileges, they could read quite different from those in the authentication object.
Given we consider the behaviour change of issue 2 as bug fixing, plus the fact that the combine solution is better than individual ones, I am working towards having both of them fixed.