Report anonymous roles in authenticate response #61355
Conversation
ywangd
added
>bug
:Security/Authorization
|
Pinging @elastic/es-security (:Security/Authorization) |
| 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.
| @@ -132,7 +132,7 @@ 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
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