SEC-717: Resolve UserDetails.getAuthorities() sort logic issue.

core/src/main/java/org/springframework/security/GrantedAuthority.java

@@ -17,6 +17,8 @@ package org.springframework.security;
 
 import java.io.Serializable;
 
+import org.springframework.security.userdetails.UserDetails;
+
 /**
  * Represents an authority granted to an {@link Authentication} object.
  *
@@ -26,10 +28,16 @@ import java.io.Serializable;
  * AccessDecisionManager}.
  * </p>
  * 
+ * <p>
+ * Implementations must implement {@link Comparable} in order to ensure that
+ * array sorting logic guaranteed by {@link UserDetails#getAuthorities()} can
+ * be reliably implemented.
+ * </p>
+ *
  * @author Ben Alex
  * @version $Id$
  */
-public interface GrantedAuthority extends Serializable {
+public interface GrantedAuthority extends Serializable, Comparable {
     //~ Methods ========================================================================================================
 
     /**

core/src/main/java/org/springframework/security/GrantedAuthorityImpl.java

@@ -17,6 +17,8 @@ package org.springframework.security;
 
 import java.io.Serializable;
 
+import org.springframework.util.Assert;
+
 
 /**
  * Basic concrete implementation of a {@link GrantedAuthority}.<p>Stores a <code>String</code> representation of an
@@ -35,6 +37,7 @@ public class GrantedAuthorityImpl implements GrantedAuthority, Serializable {
 
     public GrantedAuthorityImpl(String role) {
         super();
+        Assert.hasText(role, "A granted authority textual representation is required");
         this.role = role;
     }
 
@@ -65,4 +68,12 @@ public class GrantedAuthorityImpl implements GrantedAuthority, Serializable {
     public String toString() {
         return this.role;
     }
+
+	public int compareTo(Object o) {
+		if (o != null && o instanceof GrantedAuthority) {
+			GrantedAuthority rhs = (GrantedAuthority) o;
+			return this.role.compareTo(rhs.getAuthority());
+		}
+		return -1;
+	}
 }

core/src/main/java/org/springframework/security/userdetails/User.java

@@ -15,8 +15,11 @@
 
 package org.springframework.security.userdetails;
 
-import org.springframework.security.GrantedAuthority;
+import java.util.Iterator;
+import java.util.SortedSet;
+import java.util.TreeSet;
 
+import org.springframework.security.GrantedAuthority;
 import org.springframework.util.Assert;
 
 
@@ -231,13 +234,15 @@ public class User implements UserDetails {
 
     protected void setAuthorities(GrantedAuthority[] authorities) {
         Assert.notNull(authorities, "Cannot pass a null GrantedAuthority array");
-
+        // Ensure array iteration order is predictable (as per UserDetails.getAuthorities() contract and SEC-xxx)
+        SortedSet sorter = new TreeSet();
         for (int i = 0; i < authorities.length; i++) {
             Assert.notNull(authorities[i],
                 "Granted authority element " + i + " is null - GrantedAuthority[] cannot contain any null elements");
+            sorter.add(authorities[i]);
         }
         
-        this.authorities = authorities;
+        this.authorities = (GrantedAuthority[]) sorter.toArray(new GrantedAuthority[sorter.size()]);
     }
 
     public String toString() {

core/src/main/java/org/springframework/security/userdetails/UserDetails.java

@@ -54,7 +54,7 @@ public interface UserDetails extends Serializable {
     /**
      * Returns the authorities granted to the user. Cannot return <code>null</code>.
      *
-     * @return the authorities (never <code>null</code>)
+     * @return the authorities, sorted by natural key (never <code>null</code>)
      */
     GrantedAuthority[] getAuthorities();
 

core/src/test/java/org/springframework/security/GrantedAuthorityImplTests.java

@@ -76,14 +76,18 @@ public class GrantedAuthorityImplTests extends TestCase {
 
     //~ Inner Classes ==================================================================================================
 
-    private class MockGrantedAuthorityImpl implements GrantedAuthority {
+    private class MockGrantedAuthorityImpl implements GrantedAuthority, Comparable {
         private String role;
 
         public MockGrantedAuthorityImpl(String role) {
             this.role = role;
         }
 
-        private MockGrantedAuthorityImpl() {
+        public int compareTo(Object o) {
+			return this.role.compareTo(((GrantedAuthority)o).getAuthority());
+		}
+
+		private MockGrantedAuthorityImpl() {
             super();
         }
 

core/src/test/java/org/springframework/security/userdetails/UserTests.java

@@ -64,6 +64,11 @@ public class UserTests extends TestCase {
                 new User("rod", "koala", true, true, true, true,
                     new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl("ROLE_TWO")})));
 
+        // Equal as the new User will internally sort the GrantedAuthorities in the correct order, before running equals()
+        assertTrue(user1.equals(
+                new User("rod", "koala", true, true, true, true,
+                    new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_TWO"), new GrantedAuthorityImpl("ROLE_ONE")})));
+
         assertFalse(user1.equals(
                 new User("DIFFERENT_USERNAME", "koala", true, true, true, true,
                     new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl("ROLE_TWO")})));
@@ -153,7 +158,7 @@ public class UserTests extends TestCase {
 
     public void testUserGettersSetter() throws Exception {
         UserDetails user = new User("rod", "koala", true, true, true, true,
-                new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl("ROLE_TWO")});
+                new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_TWO"), new GrantedAuthorityImpl("ROLE_ONE")});
         assertEquals("rod", user.getUsername());
         assertEquals("koala", user.getPassword());
         assertTrue(user.isEnabled());