I am looking at OAuth and just realized that InteractiveAuthenticationSuccessEvent is not a subclass of AuthenticationSuccessEvent. If I am using an @EvenListener to grab the login event, I have to define both classes (as far as I can see, AuthenticationSuccessEvent is not fired with OAuth)
Is that an oversight?
Comment From: vpavic
If InteractiveAuthenticationSuccessEvent was a subclass of a AuthenticationSuccessEvent you'd basically have two separate instances of AuthenticationSuccessEvent published, wouldn't you?
AuthenticationSuccessEvent is published by AuthenticationManager manager implementation and InteractiveAuthenticationSuccessEvent is published subsequently by a filter (username/password, remember me, pre-auth...) that handles an interactive mean of authentication.
Comment From: jgrandja
@snicoll I'm not sure if the class hierarchy is an oversight or intentional? I'm pretty sure @rwinch may explain the reasoning for this.
You may define one handler method as follows:
@EventListener({AuthenticationSuccessEvent.class, InteractiveAuthenticationSuccessEvent.class})
public void processAuthenticationSuccessEvent(AbstractAuthenticationEvent event) {
// Handle
}
However, as @vpavic noted, it will get called twice. But you could just handle the specific event you're interested in.
Comment From: snicoll
Maybe there is a misunderstanding on my end. Looking at the doc and the name of those two events, I expected InteractiveAuthenticationSuccessEvent to be a plain AuthenticationSuccessEventand not a separate thing. This does not seem the case. It would be nice if the documentation explains the difference then.
(The reason why I created this issue is that I am using OAuth and the only event I got on login is InteractiveAuthenticationSuccessEvent and my listener on AuthenticationSuccessEvent wasn't fired).
Long story short: looks like a doc improvement at this point. Thanks.
Comment From: jgrandja
Understood and agreed that InteractiveAuthenticationSuccessEvent does look like a type of AuthenticationSuccessEvent (by naming) and may very well be assumed by the user as was in your case.
We will provide clarity in the docs.
Thanks for pointing this out.
Comment From: jgrandja
So I did some digging related to this issue, and this is what I found:
was migrated from this JIRA issue
which has the following 2 commits:
SEC-70 and SEC-71: Refactor event publishing SEC-70: Refactor event publishing
The changes in SEC-70: Refactor event publishing renamed AuthenticationEvent to AbstractAuthenticationEvent and re-organized the event hierarchy amongst a number of other changes.
But the interesting thing I found is the other commit SEC-70 and SEC-71: Refactor event publishing is that the InteractiveAuthenticationSuccessEvent originally extended ApplicationEvent but was brought into the event hierarchy re-org and at that point extended AbstractAuthenticationEvent. There is no extra information that I can find on why it did not extend AuthenticationSuccessEvent, but as discussed, I would assume that this decision was made to avoid the double notification to registered listeners of AuthenticationSuccessEvent's.
The InteractiveAuthenticationSuccessEvent is published by:
- Implementations of AbstractAuthenticationProcessingFilter
- AbstractPreAuthenticatedProcessingFilter (extends GenericFilterBean)
- RememberMeAuthenticationFilter (extends GenericFilterBean)
I'm not sure how much this is in use though. I do see LoggerListener is referencing it as well but outside of that no where else in our codebase. The reference manual doesn't even document InteractiveAuthenticationSuccessEvent so I'm thinking that not many users are registering for this event.
So what I'm thinking as far as the change goes is that we definitely need to document the javadoc for InteractiveAuthenticationSuccessEvent and maybe even change the name. The name does imply it extends from AuthenticationSuccessEvent, as @snicoll indicated.
Comment From: mauromol
I know it's useless, but (as long as I'm missing something) I would also say that the name InteractiveAuthenticationSuccessEvent sounds unfortunate to me: it suggests it's a subclass of AuthenticationSuccessEvents, thrown when an interactive login process is performed, but:
- it's fired at a different time (after session fixation protection is applied, for instance, while AuthenticationSuccessEvent is sent before that)
- it's fired in contexts that I would not define "interactive": "remember me" and "pre-authenticated" scenarios sounds like to be non-interactive to me and I would have expected exactly the opposite in those cases (i.e.: to receive AuthenticationSuccessEvents but not InteractiveAuthenticationSuccessEvents)