Introducing new `TypedAuthentication<T>` Interface - 7.0 Proposal

[Crain-32]

*Keeping Original Post for Context
Expected Behavior
The Authentication Interface is core to Spring Security, however there are some genuine pain points with it and the Developer Experience with Spring Security. This suggestion should be seen in line with the efforts of #13266 .
To help improve this I suggest adding a single Generic to Authentication,

public interface Authentication<T> extends Serializable {
    	Collection<? extends GrantedAuthority> getAuthorities();
    	
    	Object getCredentials();
    	
    	Object getDetails();
    	// Alternative would be keep Object getPrincipal() and include a new Function
    	T getPrincipal();
    	
    	boolean isAuthenticated();

        void setAuthenticated(boolean isAuthenticated);
}
// New Interface 
public interface UserAuthentication extends Authentication<UserDetails> {
     @Override
     default Collection<? extends GrantedAuthority> getAuthorities() {
         return this.getPrincipal().getAuthorities();
     }
}
// Example of a Future Interface
public interface JwtAuthentication extends Authentication<ImaginarySpringSecurityJwt> {}

This is similar to what was mentioned here, however taking a more step based approach, and focusing first on the main downstream usage.
To reduce adoption friction I'd also expect the following interface to be created,

public interface LegacyAuthentication extends Authentication<Object> {}

Current Behavior
Weak Typing with
Authentication.getPrinciple() -> Object

Context

The Authentication Interface is very old, and thus it falls into a hard spot of needing improvements, but being so core to the framework that adjusting it will have serious considerations for everyone involved.

Personally at work I've had to deal with the pain I assume many people have, which is Object getDetails() returns a UserDetails Object, but getPrincipal() doesn't, as getDetails sounds like it should return UserDetails. This means something like @AuthenticationPrincipal isn't useful, most of the Testing Support provided by Spring Security requires custom workarounds to use, etc. By putting a Generic around getPrincipal() and/or introducing a UserAuthentication, we're able to hint to Developers the approach Spring Security recommends, which I expect should improve the downstream experience of Spring Security.

As mentioned in the Issue Comment linked above, the ideal world would be Authentication<C, D, P>, but due to the core functionality of Authentication, we should take this safely, and I believe we can get most of the benefits for Developers by limiting the Generic to getPrincipal().

Alternatives
Wrap Authentication for typing, as an example

public TypedAuthentication<T> extends Authentication {
  @Override
  T getPrinciple();
}

This would provide a new Code Path for this "New" style to use, however would still leave the underlying smell. I believe we should only look into it if the potential User Disruption from the Generic Authentication is deemed to high to even consider this.

Downstream Issues
Although not on the Template, I figured if I'm going to suggest a breaking change, I should list the considerations I know about.

1, Contributor Conflicts, sweeping updates across all Modules as they move from Authentication to LegacyAuthentication, or Authentication<?>. This could be a source of merge conflicts, and without additional effort from maintainers down the line negate the Type Support this Issue is looking to provide.

2, Large Scale Documentation Updates, as mentioned Authentication is core to Spring Security, so this could entail reviewing the entire Spring Security Documentation for adjustments.

3, User Adoption Friction, given how we're swapping for a Generic our Users will have to deal with adjusting their implementations or variable references in line with the adjustment, providing LegacyAuthentication should hopefully improve that experience for them.

Post Attempt

The unfortunate truth is that Authentication is too ingrained in Spring to attempt to adjust it with Generics. I attempted it this afternoon to see the amount of effort it would take. Several hundred lines and files later I've conceded to the Java Compiler (And the pain of adding <?> to every reference)

I believe the alternate approach of introducing a new TypedAuthentication<T> extends Authentication is the best approach now, ideally with an upper bound like UserDetails to make the Compiler happy. The main issue I was experiencing was that because we had no upper bound, Java would complain about the capture groups, and because the original value was Object I couldn't introduce a new Upper Bound without creating breaking change that would invalidate the entire effort.

[Crain-32]

Hey @marcusdacoregio, I know you've marked this for team attention, hoping I can just toss this idea into here as well as I was thinking another consideration could be having this wrap the Authentication completely as TypedAuthentication<C, D, P>, instead of just wrapping the getPrincipal. So you'd get the following class.

                               // Unsure if NewDetailsObject is actually useful, just tossing stuff out
                               // Principal should extend UserDetails to help integrate with the Spring Security Testing Support
public TypedAuthentication<C, D extends NewDetailsObject, P extends UserDetails> extends Authentication {

    @Override
    public C getCredentials() { ... }
    
    @Override
    public D getDetails() { ... }
    
    @Override
    public P getPrincipal() { ... }
}

The idea being of slowly replacing Authentication where possible with this fully wrapping TypedAuthentication, instead of trying to rip out Authentication completely. This could be done over several major versions, giving users time to integrate it into their Authentication Chains, and for the Spring Team to work on any additional support interfaces (Akin to the UserDetailsService, but maybe for the getDetails() instead) they deem useful.

That being said I don't expect to see much action on this till the New Year, so I hope you enjoy any holidays you happen to Celebrate, and let me know if there is anything else I can do to support this, as I genuinely believe it can help improve Spring Security's User Experience.

[sjohnr]

Hi @Crain-32, thanks for the suggestion and the details! I've discussed this issue with @marcusdacoregio and would like to offer some feedback and context for you.

First, I know you have mentioned that the PR was submitted because you had some free time and felt like trying your hand at a solution. But as you know, we generally don't want to open PRs for big issues like this without discussion first, as the PR creates a lot of overhead and potential wasted time, since discussion can take many different directions that would influence what a potential PR could look like. So for now, I'd prefer to stick with discussing this issue and not focusing much on the PR. For that reason, I'm going to close the PR for now. Feel free to hang on to the branch if it feels useful to you, and the PR can continue to exist in a closed form to document the work you did up to now.

Regarding this issue itself:

I was thinking another consideration could be having this wrap the Authentication completely as TypedAuthentication<C, D, P>, instead of just wrapping the getPrincipal.

Speaking for myself only, I am not currently favoring the idea of introducing generics as you have suggested in your follow up comment above. While there is background and perspective on every decision that has been made regarding the framework, I find areas where similar generics are present in the framework to be very complicated and I'm not sure they make the framework easier to use. This is my opinion, of course. Sadly, I don't have a recommendation for another way to address complications with Authentication. However, I feel strongly that adding generics is not something we should pursue at this time. I have other reasons for feeling this way as well.

One reason is that #13266 is being used to collect a high level set of possible improvements to simplify and improve the usability of Spring Security. This is a high priority focus for the team right now, and will be where most of our attention is focused. However, we are currently going through a process of acquiring feedback to build the initial list of possible improvements, and community feedback on which improvements we should focus on will be a big part of this initiative. Until that process has taken its course, I don't think we will be looking to put much focus on individual efforts that are not part of the feedback we've gathered from that process.

I think that's enough feedback for the moment. For the above reasons, I'd like to suggest a pause to this issue for now. If you still feel strongly about adding generics as a solution to the problems you outlined, then I would suggest we close this issue (since it is very specific) in favor of a "Consider adding generics to Authentication to support typed return values" or something similar, and provide initial considerations in the current framework for why such a move would be needed and what the potential impacts would be. See issues linked from #13266 for examples. Perhaps some of what you've written here (without the specific solution suggestion) could be adapted into this issue.

What do you think?

[Crain-32]

Thanks for the feedback @sjohnr.

Closing the PR is perfectly fine, it mostly served as a reference to both myself and the team on what the shift in pattern could look like, as I've personally found that to help even in the early stages. As it sounds like the team prefers targeted discussion I'll make sure to respect that for larger changes moving forward.

I'd still like to pursue the topic of making Authentication better, generics or not. I'll make another Issue today that is closer to the format you've mentioned other Issues in #13266 follow, and close this one.