Expected Behavior
I think LogoutFilter should check if Authentication variable is null. If auth is null, then logout failed and skipped to next filter.
- as-is https://github.com/spring-projects/spring-security/blob/d2d1f191332239a00cb87562f6d8d7b284eb43ba/web/src/main/java/org/springframework/security/web/authentication/logout/LogoutFilter.java#L96-L108
- to-be
if (requiresLogout(request, response)) {
Authentication auth = this.securityContextHolderStrategy.getContext().getAuthentication();
if(auth != null) { // add null check
if (this.logger.isDebugEnabled()) {
this.logger.debug(LogMessage.format("Logging out [%s]", auth));
}
this.handler.logout(request, response, auth);
this.logoutSuccessHandler.onLogoutSuccess(request, response, auth);
}
return;
}
Current Behavior
Currently, logoutSuccessHandler works even if auth is null.
Context
If there's an other way to avoid this problem, please let me know. Thank you.
Comment From: YunByungil
Can you show me the SecurityFilterChain code?
Comment From: oliviarla
@YunByungil I'm not sure what you exactly want to see, but I wrote this code in SecurityConfig.
http.logout()
.invalidateHttpSession(true)
.logoutSuccessHandler(
(request, response, authentication) -> response.setStatus(HttpServletResponse.SC_OK));
Comment From: YunByungil
@oliviarla Can I write it in Korean here?
Comment From: oliviarla
@YunByungil maybe english is better for everyone :)
Comment From: YunByungil
@oliviarla As far as I know, if you write http.logout logic, the logic in this document will not work normally because the security itself provides logout
I think you can erase http.logout() and just use the filter
Please give me feedback if I'm wrong.
Comment From: oliviarla
@YunByungil
Thank you for response.
The reason why I use http.logout() is to return HTTP 200 status using logout success handler. If I remove this phrase, HTTP 404 status occurred. (I didn't wrote logout controller bc it's unnecessary)
Comment From: YunByungil
@oliviarla Then the success handler should not be implemented as above, but if you implement it separately, you can do the process you want
Note that http.logout() has an authentication value of null unconditionally. Initialize the context before entering the handler.
Comment From: jzheaux
Thanks for the suggestion, @oliviarla. However, it would not be backward compatible to stop calling logout handlers when Authentication is null.
There are handlers like SecurityContextLogoutHandler, RememberMeServices, and CookieClearingLogoutHandler that all expect to be called when logout is requested. Also, given that LogoutFilter has been written this way for many years, there are many LogoutHandler implementations that may also implicitly be expecting that they will always be called when logout is requested.
So, instead of changing the filter, please check for null in your LogoutHandler and LogoutSuccessHandler implementations.