diff --git a/core/src/main/java/org/springframework/security/access/intercept/MethodInvocationPrivilegeEvaluator.java b/core/src/main/java/org/springframework/security/access/intercept/MethodInvocationPrivilegeEvaluator.java index 33050df604..acf2a815f5 100644 --- a/core/src/main/java/org/springframework/security/access/intercept/MethodInvocationPrivilegeEvaluator.java +++ b/core/src/main/java/org/springframework/security/access/intercept/MethodInvocationPrivilegeEvaluator.java @@ -68,8 +68,7 @@ public class MethodInvocationPrivilegeEvaluator implements InitializingBean { return true; } - if ((authentication == null) || (authentication.getAuthorities() == null) - || (authentication.getAuthorities().isEmpty())) { + if (authentication == null || authentication.getAuthorities().isEmpty()) { return false; } diff --git a/core/src/main/java/org/springframework/security/authentication/AbstractAuthenticationToken.java b/core/src/main/java/org/springframework/security/authentication/AbstractAuthenticationToken.java index 6dbd9f95cd..54cdfa1dd8 100644 --- a/core/src/main/java/org/springframework/security/authentication/AbstractAuthenticationToken.java +++ b/core/src/main/java/org/springframework/security/authentication/AbstractAuthenticationToken.java @@ -22,6 +22,7 @@ import java.util.Collections; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.core.userdetails.UserDetails; @@ -46,26 +47,23 @@ public abstract class AbstractAuthenticationToken implements Authentication { /** * Creates a token with the supplied array of authorities. * - * @param authorities the list of GrantedAuthoritys for the - * principal represented by this authentication object. A - * null value indicates that no authorities have been - * granted (pursuant to the interface contract specified by {@link - * Authentication#getAuthorities()}null should only be - * presented if the principal has not been authenticated). + * @param authorities the collection of GrantedAuthoritys for the + * principal represented by this authentication object. */ public AbstractAuthenticationToken(Collection authorities) { if (authorities == null) { - this.authorities = null; - } else { - for (GrantedAuthority a: authorities) { - if(a == null) { - throw new IllegalArgumentException("Authorities collection cannot contain any null elements"); - } - } - ArrayList temp = new ArrayList(authorities.size()); - temp.addAll(authorities); - this.authorities = Collections.unmodifiableList(temp); + this.authorities = AuthorityUtils.NO_AUTHORITIES; + return; } + + for (GrantedAuthority a: authorities) { + if (a == null) { + throw new IllegalArgumentException("Authorities collection cannot contain any null elements"); + } + } + ArrayList temp = new ArrayList(authorities.size()); + temp.addAll(authorities); + this.authorities = Collections.unmodifiableList(temp); } //~ Methods ======================================================================================================== @@ -77,14 +75,8 @@ public abstract class AbstractAuthenticationToken implements Authentication { AbstractAuthenticationToken test = (AbstractAuthenticationToken) obj; - if (!(authorities == null && test.authorities == null)) { - // Not both null - if (authorities == null || test.authorities == null) { - return false; - } - if(!authorities.equals(test.authorities)) { - return false; - } + if (!authorities.equals(test.authorities)) { + return false; } if ((this.details == null) && (test.getDetails() != null)) { @@ -141,10 +133,8 @@ public abstract class AbstractAuthenticationToken implements Authentication { public int hashCode() { int code = 31; - if (authorities != null) { - for (GrantedAuthority authority : authorities) { - code ^= authority.hashCode(); - } + for (GrantedAuthority authority : authorities) { + code ^= authority.hashCode(); } if (this.getPrincipal() != null) { @@ -179,14 +169,14 @@ public abstract class AbstractAuthenticationToken implements Authentication { } public String toString() { - StringBuffer sb = new StringBuffer(); + StringBuilder sb = new StringBuilder(); sb.append(super.toString()).append(": "); sb.append("Principal: ").append(this.getPrincipal()).append("; "); sb.append("Password: [PROTECTED]; "); sb.append("Authenticated: ").append(this.isAuthenticated()).append("; "); sb.append("Details: ").append(this.getDetails()).append("; "); - if (authorities != null) { + if (!authorities.isEmpty()) { sb.append("Granted Authorities: "); int i = 0; diff --git a/core/src/main/java/org/springframework/security/authentication/jaas/JaasAuthenticationProvider.java b/core/src/main/java/org/springframework/security/authentication/jaas/JaasAuthenticationProvider.java index c518dec5f0..be51530e9b 100644 --- a/core/src/main/java/org/springframework/security/authentication/jaas/JaasAuthenticationProvider.java +++ b/core/src/main/java/org/springframework/security/authentication/jaas/JaasAuthenticationProvider.java @@ -185,10 +185,7 @@ public class JaasAuthenticationProvider implements AuthenticationProvider, Appli // Create a set to hold the authorities, and add any that have already been applied. authorities = new HashSet(); - - if (request.getAuthorities() != null) { - authorities.addAll(request.getAuthorities()); - } + authorities.addAll(request.getAuthorities()); // Get the subject principals and pass them to each of the AuthorityGranters Set principals = loginContext.getSubject().getPrincipals(); diff --git a/core/src/main/java/org/springframework/security/core/Authentication.java b/core/src/main/java/org/springframework/security/core/Authentication.java index bc7e320d2b..23caff103f 100644 --- a/core/src/main/java/org/springframework/security/core/Authentication.java +++ b/core/src/main/java/org/springframework/security/core/Authentication.java @@ -51,10 +51,14 @@ public interface Authentication extends Principal, Serializable { /** * Set by an AuthenticationManager to indicate the authorities that the principal has been * granted. Note that classes should not rely on this value as being valid unless it has been set by a trusted - * AuthenticationManager.

Implementations should ensure that modifications to the returned - * array do not affect the state of the Authentication object (e.g. by returning an array copy).

+ * AuthenticationManager. + *

+ * Implementations should ensure that modifications to the returned collection + * array do not affect the state of the Authentication object, or use an unmodifiable instance. + *

* - * @return the authorities granted to the principal, or null if authentication has not been completed + * @return the authorities granted to the principal, or an empty collection if the token has not been authenticated. + * Never null. */ Collection getAuthorities(); diff --git a/core/src/main/java/org/springframework/security/core/userdetails/User.java b/core/src/main/java/org/springframework/security/core/userdetails/User.java index e8d4eaf721..4b9d110ddd 100644 --- a/core/src/main/java/org/springframework/security/core/userdetails/User.java +++ b/core/src/main/java/org/springframework/security/core/userdetails/User.java @@ -74,7 +74,7 @@ public class User implements UserDetails { * locked * @param authorities the authorities that should be granted to the caller * if they presented the correct username and password and the user - * is enabled + * is enabled. Not null. * * @throws IllegalArgumentException if a null value was passed * either as a parameter or as an element in the @@ -210,7 +210,7 @@ public class User implements UserDetails { } public String toString() { - StringBuffer sb = new StringBuffer(); + StringBuilder sb = new StringBuilder(); sb.append(super.toString()).append(": "); sb.append("Username: ").append(this.username).append("; "); sb.append("Password: [PROTECTED]; "); @@ -219,7 +219,7 @@ public class User implements UserDetails { sb.append("credentialsNonExpired: ").append(this.credentialsNonExpired).append("; "); sb.append("AccountNonLocked: ").append(this.accountNonLocked).append("; "); - if (this.getAuthorities() != null) { + if (!authorities.isEmpty()) { sb.append("Granted Authorities: "); boolean first = true; diff --git a/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsImpl.java b/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsImpl.java index 13cb273f5a..d4c8047b48 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsImpl.java +++ b/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsImpl.java @@ -17,6 +17,8 @@ package org.springframework.security.ldap.userdetails; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.List; import javax.naming.Name; @@ -140,7 +142,7 @@ public class LdapUserDetailsImpl implements LdapUserDetails, PasswordPolicyData */ public static class Essence { protected LdapUserDetailsImpl instance = createTarget(); - private Collection mutableAuthorities = new ArrayList(); + private List mutableAuthorities = new ArrayList(); public Essence() { } @@ -184,7 +186,7 @@ public class LdapUserDetailsImpl implements LdapUserDetails, PasswordPolicyData Assert.notNull(instance.username, "username must not be null"); Assert.notNull(instance.getDn(), "Distinguished name must not be null"); - instance.authorities = getGrantedAuthorities(); + instance.authorities = Collections.unmodifiableList(mutableAuthorities); LdapUserDetails newInstance = instance; @@ -206,7 +208,8 @@ public class LdapUserDetailsImpl implements LdapUserDetails, PasswordPolicyData } public void setAuthorities(Collection authorities) { - mutableAuthorities = authorities; + mutableAuthorities = new ArrayList(); + mutableAuthorities.addAll(authorities); } public void setCredentialsNonExpired(boolean credentialsNonExpired) { diff --git a/taglibs/src/main/java/org/springframework/security/taglibs/authz/LegacyAuthorizeTag.java b/taglibs/src/main/java/org/springframework/security/taglibs/authz/LegacyAuthorizeTag.java index 725aa259f7..edd6810cb5 100644 --- a/taglibs/src/main/java/org/springframework/security/taglibs/authz/LegacyAuthorizeTag.java +++ b/taglibs/src/main/java/org/springframework/security/taglibs/authz/LegacyAuthorizeTag.java @@ -125,10 +125,6 @@ public class LegacyAuthorizeTag extends TagSupport { return Collections.emptyList(); } - if ((null == currentUser.getAuthorities())) { - return Collections.emptyList(); - } - return currentUser.getAuthorities(); } diff --git a/web/src/main/java/org/springframework/security/web/access/DefaultWebInvocationPrivilegeEvaluator.java b/web/src/main/java/org/springframework/security/web/access/DefaultWebInvocationPrivilegeEvaluator.java index 671a41966b..b2919908b0 100644 --- a/web/src/main/java/org/springframework/security/web/access/DefaultWebInvocationPrivilegeEvaluator.java +++ b/web/src/main/java/org/springframework/security/web/access/DefaultWebInvocationPrivilegeEvaluator.java @@ -130,8 +130,7 @@ public class DefaultWebInvocationPrivilegeEvaluator implements WebInvocationPriv return true; } - if ((authentication == null) || (authentication.getAuthorities() == null) - || authentication.getAuthorities().isEmpty()) { + if (authentication == null || authentication.getAuthorities().isEmpty()) { return false; } diff --git a/web/src/test/java/org/springframework/security/web/authentication/preauth/PreAuthenticatedAuthenticationTokenTests.java b/web/src/test/java/org/springframework/security/web/authentication/preauth/PreAuthenticatedAuthenticationTokenTests.java index 56e7bf43cd..1deebc46e3 100755 --- a/web/src/test/java/org/springframework/security/web/authentication/preauth/PreAuthenticatedAuthenticationTokenTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/preauth/PreAuthenticatedAuthenticationTokenTests.java @@ -25,7 +25,7 @@ public class PreAuthenticatedAuthenticationTokenTests extends TestCase { assertEquals(principal, token.getPrincipal()); assertEquals(credentials, token.getCredentials()); assertEquals(details, token.getDetails()); - assertNull(token.getAuthorities()); + assertTrue(token.getAuthorities().isEmpty()); } public void testPreAuthenticatedAuthenticationTokenRequestWithoutDetails() { @@ -35,7 +35,7 @@ public class PreAuthenticatedAuthenticationTokenTests extends TestCase { assertEquals(principal, token.getPrincipal()); assertEquals(credentials, token.getCredentials()); assertNull(token.getDetails()); - assertNull(token.getAuthorities()); + assertTrue(token.getAuthorities().isEmpty()); } public void testPreAuthenticatedAuthenticationTokenResponse() {