SEC-1590: WebAuthenticatonDetails.doPopulateAdditionalInformation should be removed #1829

Closed
spring-issuemaster opened this Issue Oct 12, 2010 · 3 comments

1 participant

@spring-issuemaster

Marcel Dullaart (Migrated from SEC-1590) said:

I have a class that extends org.springframework.security.web.authentication.WebAuthenticationDetails class and implements org.springframework.security.core.authority.GrantedAuthoritiesContainer.

It overrides the method doPopulateAdditionalInformation to retrieve the additional information from the request headers.
When running the application I receive an illegalArgumentException thrown from Assert.notNull, with the message "Cannot pass a null GrantedAuthority collection".

Investigating this showed that the WebAuthenticationDetails constructor calls doPopulateAdditionalInformation as its last step, this does initialize my members with correct values, but after the constructor returns, my derived class constructor gets called, re-initializing the fields again, re-setting my List back to null.

@spring-issuemaster

Marcel Dullaart said:

Hereunder is the exact exception
java.lang.IllegalArgumentException: Cannot pass a null GrantedAuthority collection org.springframework.util.Assert.notNull(Assert.java:112) org.springframework.security.core.userdetails.User.sortAuthorities(User.java:141)

@spring-issuemaster

Luke Taylor said:

I'm not sure why that method is there at all and I don't really see that it serves any useful purpose. My first instinct would be just to remove it.

Can't you just do whatever work you need to in your object's constructor?

@spring-issuemaster

Marcel Dullaart said:

That's what I ended up doing, first I explicitly called the doPopulateAdditionalInformation method as last step of my constructor, re-initializing the fields to the correct values. That, obviously resulted in my fields being initialized twice, so I renamed the method and only that method now gets called.
So I guess you can remove that method, its empty anyway, any derived class can than just add the functionality needed.

The current implementation suggests that the method can be overridden, while doing so turns out be faulty.

@spring-issuemaster spring-issuemaster added this to the 3.1.0.M2 milestone Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment