Support authentication without anonymous user #52094
Conversation
This change adds a new parameter to the authenticate methods in the AuthenticationService to optionally exclude support for the anonymous user (if an anonymous user exists).
tvernum
added
>non-issue
:Security/Authentication
|
Pinging @elastic/es-security (:Security/Authentication) |
|
This is needed for secondary authc support, since the secondary authenticator should never fall back to anonymous. |
...gin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java
Show resolved
Hide resolved
...gin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java
Outdated
Show resolved
Hide resolved
...urity/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java
Outdated
Show resolved
Hide resolved
| Authenticator(String action, TransportMessage message, User fallbackUser, ActionListener<Authentication> listener) { | ||
| this(new AuditableTransportRequest(auditTrail, failureHandler, threadContext, action, message | ||
| ), fallbackUser, listener); | ||
| Authenticator(String action, TransportMessage message, User fallbackUser, boolean fallbackToAnonymous, |
There was a problem hiding this comment.
Since fallbackToAnonymous makes no sense when fallbackUser is passed in and not null, I'm wondering if this is clearer to have a ctor and a #createAuthenticator() without the fallbackToAnonymous parameter and Objects.notNull for the fallbackUser. It feels like it would be simpler to argue about or understand while reading the code.
There was a problem hiding this comment.
The other option is to merge the two options (fallbackUser and fallbacktoAnonymous) into a single field in some way. Right now those two cases in handleNullToken are almost identical.
There's just enough minor differences to make it a bit tricky though.
I could make it a Supplier<Authentication> fallback, but I don't think I'd get anything cleaner than that.
There was a problem hiding this comment.
@jkakavas Let me know if you'd like to see that change. I've implemented it as you suggested for now, but could do more.
There was a problem hiding this comment.
I think this is clean enough. Maybe we can reconsider additional changes in the refactoring effort that will follow at some point
This change adds a new parameter to the authenticate methods in the AuthenticationService to optionally exclude support for the anonymous user (if an anonymous user exists). Backport of: elastic#52094
This change adds a new parameter to the authenticate methods in the AuthenticationService to optionally exclude support for the anonymous user (if an anonymous user exists). Backport of: #52094
This change adds a new parameter to the authenticate methods in the
AuthenticationService to optionally exclude support for the anonymous
user (if an anonymous user exists).