diff --git a/core/src/main/java/org/springframework/security/GrantedAuthorityImpl.java b/core/src/main/java/org/springframework/security/GrantedAuthorityImpl.java index daf6b9f1f0..f9aa32986a 100644 --- a/core/src/main/java/org/springframework/security/GrantedAuthorityImpl.java +++ b/core/src/main/java/org/springframework/security/GrantedAuthorityImpl.java @@ -21,8 +21,13 @@ import org.springframework.util.Assert; /** - * Basic concrete implementation of a {@link GrantedAuthority}.

Stores a String representation of an - * authority granted to the {@link Authentication} object.

+ * Basic concrete implementation of a {@link GrantedAuthority}. + * + *

+ * Stores a String representation of an authority granted to the {@link Authentication} object. + *

+ * If compared to a custom authority which returns null from {@link #getAuthority}, the compareTo + * method will return -1, so the custom authority will take precedence. * * @author Ben Alex * @version $Id$ @@ -36,7 +41,6 @@ public class GrantedAuthorityImpl implements GrantedAuthority, Serializable { //~ Constructors =================================================================================================== public GrantedAuthorityImpl(String role) { - super(); Assert.hasText(role, "A granted authority textual representation is required"); this.role = role; } @@ -71,8 +75,13 @@ public class GrantedAuthorityImpl implements GrantedAuthority, Serializable { public int compareTo(Object o) { if (o != null && o instanceof GrantedAuthority) { - GrantedAuthority rhs = (GrantedAuthority) o; - return this.role.compareTo(rhs.getAuthority()); + String rhsRole = ((GrantedAuthority) o).getAuthority(); + + if (rhsRole == null) { + return -1; + } + + return role.compareTo(rhsRole); } return -1; } diff --git a/core/src/test/java/org/springframework/security/GrantedAuthorityImplTests.java b/core/src/test/java/org/springframework/security/GrantedAuthorityImplTests.java index 1ccd052c3c..b1eb9d293f 100644 --- a/core/src/test/java/org/springframework/security/GrantedAuthorityImplTests.java +++ b/core/src/test/java/org/springframework/security/GrantedAuthorityImplTests.java @@ -15,7 +15,9 @@ package org.springframework.security; -import junit.framework.TestCase; +import static org.junit.Assert.*; + +import org.junit.Test; /** @@ -24,28 +26,10 @@ import junit.framework.TestCase; * @author Ben Alex * @version $Id$ */ -public class GrantedAuthorityImplTests extends TestCase { - //~ Constructors =================================================================================================== - - public GrantedAuthorityImplTests() { - super(); - } - - public GrantedAuthorityImplTests(String arg0) { - super(arg0); - } - - //~ Methods ======================================================================================================== - - public static void main(String[] args) { - junit.textui.TestRunner.run(GrantedAuthorityImplTests.class); - } - - public final void setUp() throws Exception { - super.setUp(); - } - - public void testObjectEquals() throws Exception { +public class GrantedAuthorityImplTests { + + @Test + public void equalsBehavesAsExpected() throws Exception { GrantedAuthorityImpl auth1 = new GrantedAuthorityImpl("TEST"); GrantedAuthorityImpl auth2 = new GrantedAuthorityImpl("TEST"); assertEquals(auth1, auth2); @@ -59,32 +43,52 @@ public class GrantedAuthorityImplTests extends TestCase { GrantedAuthorityImpl auth3 = new GrantedAuthorityImpl("NOT_EQUAL"); assertTrue(!auth1.equals(auth3)); - MockGrantedAuthorityImpl mock1 = new MockGrantedAuthorityImpl("TEST"); + MockGrantedAuthority mock1 = new MockGrantedAuthority("TEST"); assertEquals(auth1, mock1); - MockGrantedAuthorityImpl mock2 = new MockGrantedAuthorityImpl("NOT_EQUAL"); + MockGrantedAuthority mock2 = new MockGrantedAuthority("NOT_EQUAL"); assertTrue(!auth1.equals(mock2)); Integer int1 = new Integer(222); assertTrue(!auth1.equals(int1)); } - public void testToString() { + @Test + public void toStringReturnsAuthorityValue() { GrantedAuthorityImpl auth = new GrantedAuthorityImpl("TEST"); assertEquals("TEST", auth.toString()); } + @Test + public void compareToGrantedAuthorityWithSameValueReturns0() { + assertEquals(0, new GrantedAuthorityImpl("TEST").compareTo(new MockGrantedAuthority("TEST"))); + } + + @Test + public void compareToNullReturnsNegativeOne() { + assertEquals(-1, new GrantedAuthorityImpl("TEST").compareTo(null)); + } + + /* SEC-899 */ + @Test + public void compareToHandlesCustomAuthorityWhichReturnsNullFromGetAuthority() { + assertEquals(-1, new GrantedAuthorityImpl("TEST").compareTo(new MockGrantedAuthority())); + } + //~ Inner Classes ================================================================================================== - private class MockGrantedAuthorityImpl implements GrantedAuthority, Comparable { + private class MockGrantedAuthority implements GrantedAuthority { private String role; - public MockGrantedAuthorityImpl(String role) { + public MockGrantedAuthority() { + } + + public MockGrantedAuthority(String role) { this.role = role; } public int compareTo(Object o) { - return this.role.compareTo(((GrantedAuthority)o).getAuthority()); + throw new UnsupportedOperationException(); } public String getAuthority() {