my code
i defined an in-memory User object.
see the breakpoint result
source code differ
Spring Security Core version 6.1.0 used "addAll();" but Spring Security Core version 6.1.2 is "new ArrayList<>(authorities);"
I want to know what the official team's intention was in making these modifications.
Comment From: andreilisa
@CyrilStar, just have a look to answer from this question and i think it is the reason.
Comment From: codingtim
It appears they first added a clear before the addAll in commit:
https://github.com/spring-projects/spring-security/commit/4def40506759420e9f660c154380284631fca05a
This was done under issues: https://github.com/spring-projects/spring-security/issues/12533
Which is weird cause the initial fix for that issue was committed on January 30th and the commit that added the clear call was added June 13th.
Then the implementation was changed from clear + addAll to new ArrayList as a "polish" in commit: https://github.com/spring-projects/spring-security/commit/1f04baa4a3e76992c9142fe554b7a627fb6ae795
Now the initial change of adding clear was done so calling authorities twice would result in replacing the authorities inside the builder instead of adding.
I can understand that.
BUT the roles method on the builder calls authorities so this results in authorities being replaced.
You aren't the first one to notice this. Someone else opened similar ticket https://github.com/spring-projects/spring-security/issues/12958 which was closed as a duplicate of the ticket that caused the issue :exploding_head:
Bottom line: it seems impossible to use both roles() and authorities() as they both replace all authorities. Was this intentional? I don't think so as the tests only use authorities() and no calls to roles() is made.
Just another nice Spring Security breaking change in a patch release.
Comment From: sjohnr
Apologies for the confusion everyone. Please see release notes for release 6.1.1 which includes gh-13290.
Here's the timeline, hopefully it clears everything up:
- An enhancement was requested via gh-12533 to allow empty authorities by default (meaning you don't need to specify
roles(...)orauthorities(...)when building the user). - This was added via 3229bfa40ff6937c0bd75edad7e4a132533ce266, which erroneously changed the behavior to add authorities to the list. This change partially addressed the enhancement but was not intended to change behavior.
- This comment on the commit noted that this broke backwards compatibility because you could no longer overwrite the authorities on the builder.
- PR gh-13290 was opened to address this bug.
- I added polish commit 1f04baa4a3e76992c9142fe554b7a627fb6ae795 to simplify the code so it resembled the code prior to the original change, but still included the original enhancement.
In conclusion, the UserBuilder should now function as it did before, with the added ability to have empty authorities by default and omit a call to authorities(...). So the intention of the latest fix in the patch release (6.1.1) was to restore the original behavior because it was not intended to be a breaking change.
Having said that, the behavior outlined in the OP of this issue is not intended, and since we can't have it act both ways, we will keep it the way it was prior to 6.1. I'm going to go ahead and close this issue based on that explanation.