Spring Security
  1. Spring Security
  2. SEC-1590

WebAuthenticatonDetails.doPopulateAdditionalInformation should be removed

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.3
    • Fix Version/s: 3.1.0.M2
    • Component/s: Web
    • Labels:
      None
    • Environment:
      SLES 10, Sun JDK 6-20, JBoss eap 4.3

      Description

      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<GrantedAuthority> back to null.

        Activity

        Hide
        Marcel Dullaart added a comment -

        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)

        Show
        Marcel Dullaart added a comment - 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)
        Hide
        Luke Taylor added a comment -

        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?

        Show
        Luke Taylor added a comment - 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?
        Hide
        Marcel Dullaart added a comment -

        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.

        Show
        Marcel Dullaart added a comment - 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.

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Marcel Dullaart
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: