Publish an AuditEvent on logout #41278
Conversation
|
@Chu3laMan Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
spring-projects-issues
added
the
status: waiting-for-triage
|
@Chu3laMan Thank you for signing the Contributor License Agreement! |
There was a problem hiding this comment.
Thanks for the PR, @Chu3laMan. I've left a few comments for you to consider when you have a minute.
| @@ -93,6 +99,18 @@ private void onAuthenticationSuccessEvent(AuthenticationSuccessEvent event) { | |||
| publish(new AuditEvent(event.getAuthentication().getName(), AUTHENTICATION_SUCCESS, data)); | |||
| } | |||
|
|
|||
| private void onLogoutSuccessEvent(LogoutSuccessEvent event) { | |||
| Map<String, Object> data = new LinkedHashMap<>(); | |||
| if(event.getAuthentication() != null) { | |||
There was a problem hiding this comment.
This check isn't needed as getAuthentication() should never return null.
| if(event.getAuthentication().getDetails() != null) { | ||
| data.put("details", event.getAuthentication().getDetails()); | ||
| } | ||
| data.put("username", event.getAuthentication().getName()); |
There was a problem hiding this comment.
The username's already included in the event (it's its principal) so it doesn't needed to be included as a separate piece of data.
| @@ -51,6 +52,8 @@ public class AuthenticationAuditListener extends AbstractAuthenticationAuditList | |||
| */ | |||
| public static final String AUTHENTICATION_SWITCH = "AUTHENTICATION_SWITCH"; | |||
|
|
|||
| public static final String LOGOUT_SUCCESS = "LOGOUT_SUCCESS"; | |||
There was a problem hiding this comment.
Please add some javadoc here that describes the constant. It should also be marked with @since 3.4.0.
| @@ -73,6 +76,9 @@ else if (this.webListener != null && this.webListener.accepts(event)) { | |||
| else if (event instanceof AuthenticationSuccessEvent successEvent) { | |||
| onAuthenticationSuccessEvent(successEvent); | |||
| } | |||
| else if(event instanceof LogoutSuccessEvent logoutSuccessEvent) { | |||
There was a problem hiding this comment.
Please check the formatting. You can run ./gradlew spring-boot-project:spring-boot-actuator:format to correct it automatically.
Thank you for your remarks, do not we need to handle error during logout phase in the same function? |
mhalbritter
added
type: enhancement
I'm not sure I don't understand - what exactly do you mean? |
|
I merged the PR - thank you very much and congratulations on your first contribution 🎉! |
I mean during the logout session, if an error occurred, don't we need to handle it via a custom event? |
I really appreciate it, Thank you @philwebb , @wilkinsona, and @mhalbritter |
This is all handled by Spring Security. This component in Boot only translates the Spring Security Events into audit events. |
No description provided.