Expected Behavior

I expected the builder method of AnonymousConfigurer.authorities(authorities) to take a List<? extends GrantedAuthority> authorities as parameter

Current Behavior

AnonymousConfigurer.authorities(authorities) takes a List<GrantedAuthority> as parameter

Context As of now, I cannot think of a reason to not allow subtypes of GrantedAuthorities. E.g., when creating a new User, one can specify a Collection<? extends GrantedAuthority>

How has this issue affected you? We converted a list of Strings to GrantedAuthorities but since this is an interface type, we created objects of type SimpleGrantedAuthority. Using Stream.toList() to convert this stream of objects into an unmodifiable list results in a List<SimpleGrantedAuthority>.

What are you trying to accomplish? Directly using a List<SimpleGrantedAuthority> to avoid any casting.

What other alternatives have you considered? Cast the list elements, so the list elements have the correct type.

Are you aware of any workarounds? See above

I'm happy to contribute a PR if this is accepted

Comment From: sjohnr

@tobias-lippert thanks for reaching out!

It looks like this has been an issue for some time. However, the use of Stream#toList may be the real culprit here because it is only aware of the types in the Stream, and not the intended target of the expression (whether an assignment or passing to another method, in this case authorities(...)). In fact, the default method implementation actually does an unchecked cast.

It appears that Stream#toList method has this limitation in general, such that the following will not compile:

List<GrantedAuthority> authorities = List.of("a", "b", "c").stream()
    .map(SimpleGrantedAuthority::new)
    .toList();

I'd argue that this should compile, but it does not. However, this does compile:

List<GrantedAuthority> authorities = List.of("a", "b", "c").stream()
    .map(SimpleGrantedAuthority::new)
    .collect(Collectors.toList());

As a workaround, I recommend the use of Stream#collect as in the following example:

List<String> authorities = List.of("a", "b", "c");
http.anonymous((anonymous) -> anonymous
    .authorities(authorities.stream()
        .map(SimpleGrantedAuthority::new)
        .collect(Collectors.toList())
    )
);

Alternatively, you can cast the entire stream like this:

List<String> authorities = List.of("a", "b", "c");
http.anonymous((anonymous) -> anonymous
    .authorities(authorities.stream()
        .map(SimpleGrantedAuthority::new)
        .map(GrantedAuthority.class::cast)
        .toList()
    )
);

In terms of a fix, we will need to check that changing the method signature does not break backwards compability. My initial instict right now is that such a change would be a breaking change, and as such would need to wait for the next major release. If you have any thoughts, please let me know.

Comment From: tobias-lippert

Thanks, @sjohnr, for your quick reply. I agree with you. I had done the same analysis as you before I opened the issue, and we currently use your second alternative in our code. Your first proposed alternative is also viable, but be aware that Stream#collect(Collectors.toList()) returns a modifiable list in contrast to Stream#toList, which might not be desirable in this context. However, I thought it might make sense to change the behavior of Spring Security to accommodate this (in my opinion, questionable) implementation of the JDK. I saw it's implemented like the change I've proposed in other places of Spring Security.

I didn't know that breaking changes require a major release. I thought a minor release was sufficient, but since it's not critical but rather a nice-to-have improvement, I'm fine with waiting till a major release.

Once you conclude if it's breaking or not and which release should be targeted, I'm happy to contribute a PR, so please keep me updated.

Comment From: sjohnr

Thanks @tobias-lippert.

Your first proposed alternative is also viable, but be aware that Stream#collect(Collectors.toList()) returns a modifiable list in contrast to Stream#toList, which might not be desirable in this context.

That is a good point. However, the authorities list is made unmodifiable when passed to the AnonymousAuthenticationToken (extends AbstractAuthenticationToken), so in terms of ensuring that the Authentication is immutable, it is fine. It is a bit strange that the AnonymousAuthenticationFilter does not make it immutable though, as it should really be the job of the recipient of the List to make it immutable.

Anyway, I digress. :smiley: Thanks for the reply!