ProxyingHandlerMethodArgumentResolver conflict with @AuthenticationPrincipal

[spadou]

In a Spring application (with Spring Boot 3.1.3 currently), using data and security the ProxyingHandlerMethodArgumentResolver will conflict with the use of @AuthenticationPrincipal in a controller when a custom interface is used for the principal. The ProxyingHandlerMethodArgumentResolver will be registered before AuthenticationPrincipalArgumentResolver and handle the argument as a projection in the fallback preventing @AuthenticationPrincipal from working.

I see this was improved in #1237, but only the Spring packages are filtered out. In my case I use a custom interface for the principal to inject due to others requirements, so it is not ignored by the ProxyingHandlerMethodArgumentResolver. The list of ignored packages also doesn't seem configurable so it is not an option to handle the issue. I could also exclude the SpringDataWebAutoConfiguration but this exclude everything (like pageable) so it is not ideal.

There is already a @ProjectedPayload in ProxyingHandlerMethodArgumentResolver so wouldn't it make sense to remove the fallback as it could cause issues in multiple situations?

Here a small example that reproduce the issue:

@SpringBootApplication
public class ArgumentResolverConflictApplication {

    public static void main(String[] args) {
        SpringApplication.run(ArgumentResolverConflictApplication.class, args);
    }

    @RestController
    public static class ArgumentResolverConflictRestController {

        @GetMapping("/principal")
        String principal(@AuthenticationPrincipal CustomUserDetails principal) {
            return principal.getUsername();
        }
    }

    @Bean
    public InMemoryUserDetailsManager inMemoryUserDetailsManager() {
        return new InMemoryUserDetailsManager(User.withUsername("user").password("{noop}password").build()) {
            @Override
            public UserDetails loadUserByUsername(String username) {
                var user = super.loadUserByUsername(username);
                return new CustomUser(user.getUsername(), user.getPassword(), user.isEnabled(), user.isAccountNonExpired(), user.isCredentialsNonExpired(),
                        user.isAccountNonLocked(), user.getAuthorities());
            }
        };
    }

    public interface CustomUserDetails extends UserDetails {
    }

    public static class CustomUser extends User implements CustomUserDetails {
        public CustomUser(String username, String password, boolean enabled, boolean accountNonExpired, boolean credentialsNonExpired, boolean accountNonLocked,
                          Collection<? extends GrantedAuthority> authorities) {
            super(username, password, enabled, accountNonExpired, credentialsNonExpired, accountNonLocked, authorities);
        }
    }
}

If this is run with security and data (jdbc for example), the principal wont be injected in the controller but we'll have an empty projection instead. If SpringDataWebAutoConfiguration is excluded, it works as expected.

[mp911de]

Registering ProxyingHandlerMethodArgumentResolver later should allow other HandlerMethodArgumentResolvers to take precedence. Have you tried doing so by deferring the configuration (i.e. using Ordered or @Order)? Deferring RepositoryRestMvcAutoConfiguration to after the Spring Security web bits would be another path forward.

[spadou]

In this example I'm not configuring anything, it is all Spring Boot auto-configuration, and I don't see an easy way to define the order from external configuration. Defining an order for the WebMvcConfigurers would probably help, but currently neither SpringDataWebConfiguration or WebMvcSecurityConfiguration are defining one, so both would need to be updated.

[odrotbohm]

@rwinch – Do you think it would be possible to explicitly order WebMvcSecurityConfiguration to something like Ordered.LOWEST_PRECENDENCE - 100 so that data has a chance to get behind it?

[spadou]

Looking a bit more into this, and how the ProxyingHandlerMethodArgumentResolver is registered.

Maybe another solution would be to use the ProjectingArgumentResolverRegistrar, that is currently used to register the non catch-all instance of ProxyingHandlerMethodArgumentResolver at the start of the list, to also register the catch-all instance. This way custom argument resolvers are not used, so this avoid any issues with ordering, and we can ensure that the instance is registered at the end of the list in RequestMappingHandlerAdapter with the others catch-all.

[odrotbohm]

That's a good point. The origin of this particular arrangement was to be able to support @ModelAttribute-annotated parameters to in turn keep the programming model consistent no matter if you bind to a concrete class or an interface. I wonder if we should move to require explicit annotations on the parameter of either kind: @ModelAttribute, @ProjectedPayload. Spring Data REST could then augment that to also support types annotated with @Projection as well.

As that is a breaking change, I wonder if it makes sense to log a warning in case of a missing explicit declaration on the upcoming bug fix releases. /cc @mp911de

[knalli]

Well, I found this issue today because I've spend the morning finding the root cause of a strange issue. I think, I have run into the same situation. I also have an interface for the authentication principal (extension of UserDetails).

After upgrading to yesterdays's SB v3.3 including SDC v3.3, the handler resolver for a interface-typed @AuthenticationPrincipial (i.e. AuthenticationPrincipalArgumentResolver) stopped working (but an "empty" proxy of "null"). After realizing that ProxyingHandlerMethodArgumentResolver "catches" it, however, it makes some sense now.

The point here is: This only happens after I followed the warning-leveled-hint about configuring @EnableSpringDataWebSupport. Regardless the actual used pageSerializationMode, this seems to trigger another order. Because the number of resolvers within HandlerMethodArgumentResolverComposite is the same.

Although it seems no out-of-the-box issue for now, if someone fellow the recommendations and also uses interfaces on method arguments, it it actually bugged.

Raising attention for this issue here, and for my understanding, this "not-really-catch-all" handlers should be handled with much lower priority in general?

Edit: If still required, I build a small poc about this.