From 55679971f011dac9a91b964675e7a8342cc2c1e0 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Fri, 18 Dec 2009 13:49:02 +0000 Subject: [PATCH] SEC-1337: Make User serializable by moving anonymous comparator class --- .../security/core/userdetails/User.java | 33 ++++++++++--------- .../security/core/userdetails/UserTests.java | 28 ++++++++++++---- 2 files changed, 40 insertions(+), 21 deletions(-) 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 4b9d110ddd..69587393b1 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 @@ -185,21 +185,7 @@ public class User implements UserDetails { Assert.notNull(authorities, "Cannot pass a null GrantedAuthority collection"); // Ensure array iteration order is predictable (as per UserDetails.getAuthorities() contract and SEC-717) SortedSet sortedAuthorities = - new TreeSet(new Comparator() { - public int compare(GrantedAuthority g1, GrantedAuthority g2) { - // Neither should ever be null as each entry is checked before adding it to the set. - // If the authority is null, it is a custom authority and should precede others. - if (g2.getAuthority() == null) { - return -1; - } - - if (g1.getAuthority() == null) { - return 1; - } - - return g1.getAuthority().compareTo(g2.getAuthority()); - } - }); + new TreeSet(new AuthorityComparator()); for (GrantedAuthority grantedAuthority : authorities) { Assert.notNull(grantedAuthority, "GrantedAuthority list cannot contain any null elements"); @@ -209,6 +195,23 @@ public class User implements UserDetails { return sortedAuthorities; } + private static class AuthorityComparator implements Comparator { + public int compare(GrantedAuthority g1, GrantedAuthority g2) { + // Neither should ever be null as each entry is checked before adding it to the set. + // If the authority is null, it is a custom authority and should precede others. + if (g2.getAuthority() == null) { + return -1; + } + + if (g1.getAuthority() == null) { + return 1; + } + + return g1.getAuthority().compareTo(g2.getAuthority()); + } + } + + public String toString() { StringBuilder sb = new StringBuilder(); sb.append(super.toString()).append(": "); diff --git a/core/src/test/java/org/springframework/security/core/userdetails/UserTests.java b/core/src/test/java/org/springframework/security/core/userdetails/UserTests.java index 7af509dfe6..013faa372d 100644 --- a/core/src/test/java/org/springframework/security/core/userdetails/UserTests.java +++ b/core/src/test/java/org/springframework/security/core/userdetails/UserTests.java @@ -15,16 +15,16 @@ package org.springframework.security.core.userdetails; +import static org.junit.Assert.*; + +import java.io.ByteArrayOutputStream; +import java.io.ObjectOutputStream; import java.util.List; -import junit.framework.TestCase; - - +import org.junit.Test; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.core.authority.GrantedAuthorityImpl; -import org.springframework.security.core.userdetails.User; -import org.springframework.security.core.userdetails.UserDetails; /** @@ -33,10 +33,11 @@ import org.springframework.security.core.userdetails.UserDetails; * @author Ben Alex * @version $Id$ */ -public class UserTests extends TestCase { +public class UserTests { private static final List ROLE_12 = AuthorityUtils.createAuthorityList("ROLE_ONE","ROLE_TWO"); //~ Methods ======================================================================================================== + @Test public void testEquals() { User user1 = new User("rod", "koala", true, true, true, true,ROLE_12); @@ -57,6 +58,7 @@ public class UserTests extends TestCase { AuthorityUtils.createAuthorityList("ROLE_ONE")))); } + @Test public void testNoArgConstructorDoesntExist() { Class clazz = User.class; @@ -67,6 +69,7 @@ public class UserTests extends TestCase { } } + @Test public void testNullValuesRejected() throws Exception { try { new User(null, "koala", true, true, true, true,ROLE_12); @@ -89,6 +92,7 @@ public class UserTests extends TestCase { } } + @Test public void testNullWithinGrantedAuthorityElementIsRejected() throws Exception { try { List auths = AuthorityUtils.createAuthorityList("ROLE_ONE"); @@ -100,6 +104,7 @@ public class UserTests extends TestCase { } } + @Test public void testUserGettersSetter() throws Exception { UserDetails user = new User("rod", "koala", true, true, true, true, AuthorityUtils.createAuthorityList("ROLE_TWO","ROLE_ONE")); @@ -111,8 +116,19 @@ public class UserTests extends TestCase { assertTrue(user.toString().indexOf("rod") != -1); } + @Test public void testUserIsEnabled() throws Exception { UserDetails user = new User("rod", "koala", false, true, true, true, ROLE_12); assertTrue(!user.isEnabled()); } + + @Test + public void useIsSerializable() throws Exception { + UserDetails user = new User("rod", "koala", false, true, true, true, ROLE_12); + // Serialize to a byte array + ByteArrayOutputStream bos = new ByteArrayOutputStream() ; + ObjectOutputStream out = new ObjectOutputStream(bos) ; + out.writeObject(user); + out.close(); + } }