Report anonymous roles in authenticate response #61355
Conversation
ywangd
added
>bug
:Security/Authorization
|
Pinging @elastic/es-security (:Security/Authorization) |
jkakavas
left a comment
jkakavas
left a comment
There was a problem hiding this comment.
This looks good to me Yang! Nits are just that, feel free to change or merge as is. Pinging @elastic/kibana-security as they have some tests that expect roles based on the current behavior of the authenticate API and CI will break
| final String[] allRoleNames = Stream.concat( | ||
| Stream.of(user.roles()), Stream.of(anonymousUser.roles())).toArray(String[]::new); | ||
| listener.onResponse(new AuthenticateResponse( | ||
| new Authentication( |
There was a problem hiding this comment.
This is rather verbose but I don't have any good suggestions to do otherwise and we don't seem to have a generic need for a cloneWithChanges method !
There was a problem hiding this comment.
I initially had this logic inside the User class, i.e. User#withRoles with the intention to simply things here. But as you said, there is no generic need for this type of method, which made it seem out of place. So I decided to keep everything in this one class. It is verbose but at least self-contained and hopefully easy to understand.
|
|
||
| private void checkAuthentication() throws IOException { | ||
| final Map<String, Object> auth = getAsMap("/_security/_authenticate"); | ||
| // From file realm, configured in build.gradle |
There was a problem hiding this comment.
nit: add a hint that anonymous user is enabled maybe so the test makes more sense when you look at it ?
There was a problem hiding this comment.
Add a comment about anonymous access is configured in build.gradle
| } else { | ||
| when(anonymousUser.enabled()).thenReturn(false); | ||
| } | ||
| final String[] roleNames = randomList(1, 4, () -> randomAlphaOfLengthBetween(4, 12)).toArray(new String[0]); |
There was a problem hiding this comment.
nit: this only makes sense in the if branch right?
Report anonymous roles in response to "GET _security/_authenticate" API call when: * Anonymous role is enabled * User is not the anonymous user * Credentials is not an API Key
4 participants
Report anonymous roles in response to "GET _security/_authenticate" API call when:
Previous attempt (#53453) tried to solve this issue by re-locating anonymous role resolution from authz to authc. It has since been reverted (#57853) because of potential issues on how anonymous access can be used with persisted authentication objects, e.g. Watchers (#57711). Hence the current attempt tries to limit the solution to only the reporting side avoid potential implications. I personally feel this is an acceptable solution and it is also consistent with the original ask.
Resolves: #47195