Spring Security Make return type of UserDetails related services be covariant

Expected Behavior

It would be better if the return type became a covariant:

Mono<? extends UserDetails>

For example, the findByUsername method in ReactiveUserDetailsService interface should look like:

Mono<? extends UserDetails> findByUsername(String username);

and also, the updatePassword method in ReactiveUserDetailsPasswordService interface should be:

Mono<? extends UserDetails> updatePassword(UserDetails user, String newPassword);

Current Behavior

All those methods returns an invariant type:

Mono<UserDetails>

Context

UserDetails is an interface, and it is generally used its implementation in projects (even the org.springframework.security.core.userdetails.User class provided by the library is its implementation). Let's say a class named CustomerUser implemented the UserDetails interface, and a repository could find a mono CustomerUser via:

Mono<CustomerUser> findByUsername(String username);

And then, when implementing the ReactiveUserDetailsService interface, the findByUsername method should be overridden, so it may like like:

@Override
public Mono<UserDetails> findByUsername(String username) {
    // a compile error rises
    return repo.findByUsername(username);
}

However, it causes a compile error because the return type of repository method -- Mono<CustomerUser> -- is neither the Mono<UserDetails> nor any subclass of it even if CustomerUser class is an implementation of UserDetails inerterface. The compile error is an expect behavior because that is exactly what the invariance is about. To make it work, a class cast method is nessary to be added.

By contrast, the getAuthorities method in UserDetails interface returns a covariant type:

Collection<? extends GrantedAuthority>

Therefore, I suggest that these invariant types should be refactored to covariant types.

Comment From: rwinch

Hello @tonny1983 Thank you for your report. Upon additional review, I'm going to decline this change:

• Using a wildcard as a return type should be avoided. Yes, authorities is that way, but it is a little different because it is intentionally meant to make it (partially) immutable. Additionally, I feel that making it a wildcard was a mistake that we do not want to repeat.
• Changing the return type is an unnecessary breaking change that will impact our users. Even in major releases we are very conservative on making breaking changes.
• There are better ways to handle this. I'd suggest taking a look at my presentation Spring Security the Reactive Bits. It demonstrates how to have a data model that does not implement UserDetails and map that to a UserDetails. This prevents your user model from being coupled to Spring Security.

For your situation you can use the following if you like, but I suggest to look at the Spring Security the Reactive Bits as described above.

@Override
public Mono<UserDetails> findByUsername(String username) {
    // a compile error rises
    return repo.findByUsername(username).map(u -> u);
}