SEC-2983: InMemoryUserDetailsManager#loadUserByUsername should preserve custom UserDetails types

[spring-projects-issues]

William Gorder (Migrated from SEC-2983) said:

Is there a good reason for not returning MutableUser.delegate in the InMemoryUserDetailsManager.loadUserByUsername() method? What I have run into is I want to provide my own user details, so I pass them to the create() method but when I get them back from authentication.getPrincipal() its been replaced with the vanilla spring security User object since apparently rather then just returning the UserDetails that we already have it takes the delegate and creates a new org.springframework.security.core.userdetails.User.

What I am proposing is a something different then the MutableUserDetails interface. The MutableUser object would need to expose its delegate so that it could be returned if it exists. Currently the only way to get what I want is to create my own version of InMemoryUserDetailsManager and InMemoryUserDetailsManagerConfigurer unless I am missing something. It just seems that by exposing a create(UserDetails) method it is implied that we would get the UserDetails provided and not something different. I see this bit of the API as confusing and bit misleading. There is nothing in the Java Doc implying that that underlying implementation will be changed.

[jzheaux]

Is there a good reason for not returning MutableUser.delegate in the InMemoryUserDetailsManager.loadUserByUsername() method?

I believe one benefit is that User implements CredentialsContainer, ensuring that the password will be removed completely after authentication.

Currently the only way to get what I want is to create my own version of InMemoryUserDetailsManager and InMemoryUserDetailsManagerConfigurer

Things have obviously changed a lot since the 3.x days. Today publishing a UserDetailsManager bean or, even simpler, a UserDetailsService is quite simple and is the recommended route for custom UserDetails implementations.

That said, one thing I think we could do is store any MutableUserDetails instead of wrapping it like so:

public void createUser(UserDetails user) {
    if (user instanceof MutableUserDetails mutable) {
        this.users.put(user.getUsername().toLowerCase(), mutable);
    } else {
        this.users.put(user.getUsername().toLowerCase(), new MutableUser(user));
    }
}

// ...

public UserDetails loadUserByUsername(String username) ...
    UserDetails user = this.users.get(username.toLowerCase());
    if (user == null ) ...
    if (user instanceof CredentialsContainer container) {
        return container;
    }
    return new User(...);
}

In this way, I believe the credential-clearing functionality would be preserved, MutableUser would remain untouched, and you would have easy access to your custom type through casting.