Spring Security CsrfConfigurer accessDeniedHandler logic

Summary

Is it by design that: CsrfConfigurer does not reuse accessDeniedHandler logic from ExceptionHandlingConfigurer?

Actual Behavior

CsrfConfigurer is using getAccessDeniedHandler to get accessDeniedHandler and creates its own if no handler specified. In case we've specified only defaultDeniedHandlerMappings in ExceptionHandlingConfigurer there is RequestMatcherDelegatingAccessDeniedHandler being created which is not set as accessDeniedHandler but is only forwarded to exceptionTranslationFilter. In this case CsrfConfigurer creates it's own accessDeniedHandler which does not include defaultDeniedHandlerMappings logic.

Expected Behavior

CsrfConfigurer reuses accessDeniedHandler and defaultDeniedHandlerMappings created in ExceptionHandlingConfigurer

Version

5.1.3

Comment From: rwinch

It appears like it should be using the default access denied handler. If you are having a problem, please provide a complete and minimal sample to reproduce the issue.

Comment From: pbialonc

Here is minimal project reproducing the issue: https://github.com/pbialonc/csrf

CsrfApplicationTests fails on shouldReturnTeapot as only accessDeniedHandler logic is applied for csrf configurer and defaultAccessDeniedHandlerFor does not take any effect.

Note that not defining accessDeniedHandler would not cause defaultAccessDeniedHandlerFor to take effect, it would be still omitted.

This is not an issue for me as I could use global accessDeniedHandler or build custom handler, but I think this is an issue in general.

Comment From: clevertension

i think it is resonable that if we setAccessDeniedHandler explicitly, the defaultAccessDeniedHandler will be invalid, that is the mean of default or exactly be called default for temporarily

Comment From: spring-projects-issues

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

Comment From: jzheaux

Pardon our dust here as we do some issue cleanup. Feedback was already provided earlier, and I don't think the ticket has been fully addressed, yet, so let's keep the issue open.

Comment From: christophejan

I agree that when accessDeniedHandler is set explicitly, everything work fine.

But as @pbialonc said:

Note that not defining accessDeniedHandler would not cause defaultAccessDeniedHandlerFor to take effect, it would be still omitted.

I think this is an issue. When accessDeniedHandler is not defined, CsrfConfigurer use new AccessDeniedHandlerImpl() whatever the defaultAccessDeniedHandlerFor are on the ExceptionHandlingConfigurer.

I think it would be more logic for CsrfConfigurer to retrieve the AccessDeniedHandler from the ExceptionHandlingConfigurer by using getAccessDeniedHandler(H http) instead of getAccessDeniedHandler(). That way, it will take in account accessDeniedHandler as well as defaultAccessDeniedHandlerFor.

Comment From: rwinch

Thanks for the nudge @christophejan Would someone be interested in providing a PR?

Comment From: christophejan

Hello @rwinch, I've open a pull request for this issue but it raise unfortunately much more questions than I was expecting (cf #10154).

Comment From: rwinch

Closing in favor of gh-10154