`ExceptionHandlingConfigurer#createDefaultAccessDeniedHandler` should always fallback to `AccessDeniedHandlerImpl`

[liias]

org.springframework.security.config.annotation.web.configurers.ExceptionHandlingConfigurer#createDefaultAccessDeniedHandler has this implementation:

LinkedHashMap<RequestMatcher, AccessDeniedHandler> defaultDeniedHandlerMappings

private AccessDeniedHandler createDefaultAccessDeniedHandler(H http) {
    if (this.defaultDeniedHandlerMappings.isEmpty()) {
        return new AccessDeniedHandlerImpl();
    }
    if (this.defaultDeniedHandlerMappings.size() == 1) {
        return this.defaultDeniedHandlerMappings.values().iterator().next();
    }
    return new RequestMatcherDelegatingAccessDeniedHandler(this.defaultDeniedHandlerMappings,
				new AccessDeniedHandlerImpl());
}

I'd propose that special case for this.defaultDeniedHandlerMappings.size() == 1 is not intuitive, because defaultDeniedHandlerMappings is always added for a specific RequestMatcher.

• If I don't add a handler for a request matcher, then AccessDeniedHandlerImpl is always used.
• If I add two handlers for request matchers, then AccessDeniedHandlerImpl is used when those matchers don't match
• If I add exactly one handler for a request matcher, then that handler is also used when its request matcher does not match

I assume the reason is backwards compatibility rather than design choice?
Sorry if it has been reported before.

[answndud]

I would like to work on this.

My plan is to remove the single-mapping special case so that the fallback AccessDeniedHandlerImpl is used consistently whenever no configured RequestMatcher matches, and to add a regression test for the single-matcher case.

[answndud]

Opened PR #18889 with the fix and regression test for the single-matcher fallback case.

Tests run locally:

• export JAVA_HOME=/opt/homebrew/opt/openjdk/libexec/openjdk.jdk/Contents/Home; export PATH="$JAVA_HOME/bin:$PATH"; export NPM_CONFIG_WORKSPACES=false; ./gradlew :spring-security-config:test --tests org.springframework.security.config.annotation.web.configurers.ExceptionHandlingConfigurerAccessDeniedHandlerTests

[liias]

That's cool, but it would be a breaking change, so spring people need to decide how to make it not breaking, or if to do no changes at all.