Spring Security
  1. Spring Security
  2. SEC-1550

UserDetails and Authentication should return Collection<? extends GrantedAuthority> from getAuthorities()

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Complete
    • Affects Version/s: 3.0.3
    • Fix Version/s: 3.1.0.M2
    • Component/s: None
    • Labels:
      None

      Description

      The UserDetails interface still expects getAuthorities to return a Collection<GrantedAuthority>. This is inconsistent with how the UsernamePasswordAuthenticationToken constructor was changed in the latest release to allow for Collection<? extends GrantedAuthority>. If an implementation class of GrantedAuthority is used and linked to the implementation of UserDetails, it has to be created and handled with a different naming scheme (getRoles for example) in parallel to getAuthorities, in order to convert to the super type. This causes confusion for non-familiar users of the api, and also requires un-necessary code.

        Activity

        Hide
        Luke Taylor added a comment -

        Could you supply some code to illustrate what you mean by the second part of your description (beginning "If an implementation class of ...") ?

        Show
        Luke Taylor added a comment - Could you supply some code to illustrate what you mean by the second part of your description (beginning "If an implementation class of ...") ?
        Hide
        Johnathon added a comment -

        public class Authority implements GrantedAuthority {
        private Role role;

        /* ommitted non-related code */

        public Role getRole()

        { return role; }

        public void setRole(Role role)

        { this.role = role; }

        }

        public class User implements UserDetails {
        // can't name this field 'authorities' because of generic
        // type restriction by UserDetails for overriding method 'getAuthorities'
        private List<Authority> roles;

        /* ommitted non-related code */

        public List<Authority> getRoles()

        { return roles; }

        public void setRoles(List<Authority> roles)

        { this.roles = roles; }

        @Override
        public List<GrantedAuthority> getAuthorities()

        { return (List) roles; }

        }

        Show
        Johnathon added a comment - public class Authority implements GrantedAuthority { private Role role; /* ommitted non-related code */ public Role getRole() { return role; } public void setRole(Role role) { this.role = role; } } public class User implements UserDetails { // can't name this field 'authorities' because of generic // type restriction by UserDetails for overriding method 'getAuthorities' private List<Authority> roles; /* ommitted non-related code */ public List<Authority> getRoles() { return roles; } public void setRoles(List<Authority> roles) { this.roles = roles; } @Override public List<GrantedAuthority> getAuthorities() { return (List) roles; } }
        Hide
        Luke Taylor added a comment -

        OK, but I don't immediately see the benefit of wildcarding the interface method. You can still only read the upper bound of the wildcard from the collection - which is GrantedAuthority in this case.

        Show
        Luke Taylor added a comment - OK, but I don't immediately see the benefit of wildcarding the interface method. You can still only read the upper bound of the wildcard from the collection - which is GrantedAuthority in this case.
        Hide
        Luke Taylor added a comment -

        Hmm, ok. I think this makes sense now. I guess it would also probably make sense in the Authentication interface too.

        Show
        Luke Taylor added a comment - Hmm, ok. I think this makes sense now. I guess it would also probably make sense in the Authentication interface too.
        Hide
        Johnathon added a comment -

        Ah yes, that does seem to be another good change

        Show
        Johnathon added a comment - Ah yes, that does seem to be another good change

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Johnathon
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: