SEC-1550: UserDetails and Authentication should return Collection<? extends GrantedAuthority> from getAuthorities() #1759

Closed
spring-issuemaster opened this Issue Aug 30, 2010 · 5 comments

1 participant

@spring-issuemaster

Johnathon (Migrated from SEC-1550) said:

The UserDetails interface still expects getAuthorities to return a Collection. 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.

@spring-issuemaster

Luke Taylor said:

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

@spring-issuemaster

Johnathon said:

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 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; }

}

@spring-issuemaster

Luke Taylor said:

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.

@spring-issuemaster

Luke Taylor said:

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

@spring-issuemaster

Johnathon said:

Ah yes, that does seem to be another good change

@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