diff --git a/cas/src/main/java/org/springframework/security/cas/authentication/CasAuthenticationToken.java b/cas/src/main/java/org/springframework/security/cas/authentication/CasAuthenticationToken.java index e8250195f8..fe8cbcab6d 100644 --- a/cas/src/main/java/org/springframework/security/cas/authentication/CasAuthenticationToken.java +++ b/cas/src/main/java/org/springframework/security/cas/authentication/CasAuthenticationToken.java @@ -24,6 +24,7 @@ import org.springframework.security.authentication.AbstractAuthenticationToken; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.SpringSecurityCoreVersion; import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.util.Assert; /** * Represents a successful CAS Authentication. @@ -117,15 +118,8 @@ public class CasAuthenticationToken extends AbstractAuthenticationToken implemen // ======================================================================================================== private static Integer extractKeyHash(String key) { - Object value = nullSafeValue(key); - return value.hashCode(); - } - - private static Object nullSafeValue(Object value) { - if (value == null || "".equals(value)) { - throw new IllegalArgumentException("Cannot pass null or empty values to constructor"); - } - return value; + Assert.hasLength(key, "key cannot be null or empty"); + return key.hashCode(); } public boolean equals(final Object obj) { diff --git a/cas/src/test/java/org/springframework/security/cas/authentication/CasAuthenticationTokenTests.java b/cas/src/test/java/org/springframework/security/cas/authentication/CasAuthenticationTokenTests.java index ce1b1ece33..068a2b881b 100644 --- a/cas/src/test/java/org/springframework/security/cas/authentication/CasAuthenticationTokenTests.java +++ b/cas/src/test/java/org/springframework/security/cas/authentication/CasAuthenticationTokenTests.java @@ -19,6 +19,7 @@ package org.springframework.security.cas.authentication; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; +import java.util.Collections; import java.util.List; import org.jasig.cas.client.validation.Assertion; @@ -102,6 +103,12 @@ public class CasAuthenticationTokenTests { } } + @Test(expected = IllegalArgumentException.class) + public void constructorWhenEmptyKeyThenThrowsException() { + new CasAuthenticationToken("", "user", "password", Collections.emptyList(), + new User("user", "password", Collections.emptyList()), null); + } + @Test public void testEqualsWhenEqual() { final Assertion assertion = new AssertionImpl("test"); diff --git a/cas/src/test/java/org/springframework/security/cas/jackson2/CasAuthenticationTokenMixinTests.java b/cas/src/test/java/org/springframework/security/cas/jackson2/CasAuthenticationTokenMixinTests.java index 8605a12a23..9ab7aa6e6a 100644 --- a/cas/src/test/java/org/springframework/security/cas/jackson2/CasAuthenticationTokenMixinTests.java +++ b/cas/src/test/java/org/springframework/security/cas/jackson2/CasAuthenticationTokenMixinTests.java @@ -76,18 +76,6 @@ public class CasAuthenticationTokenMixinTests { return mapper; } - @Test(expected = IllegalArgumentException.class) - public void nullKeyTest() { - new CasAuthenticationToken(null, "user", PASSWORD, Collections.emptyList(), - new User("user", PASSWORD, Collections.emptyList()), null); - } - - @Test(expected = IllegalArgumentException.class) - public void blankKeyTest() { - new CasAuthenticationToken("", "user", PASSWORD, Collections.emptyList(), - new User("user", PASSWORD, Collections.emptyList()), null); - } - @Test public void serializeCasAuthenticationTest() throws JsonProcessingException, JSONException { CasAuthenticationToken token = createCasAuthenticationToken(); diff --git a/core/src/main/java/org/springframework/security/authentication/AnonymousAuthenticationToken.java b/core/src/main/java/org/springframework/security/authentication/AnonymousAuthenticationToken.java index 04c1f156f6..988d728c65 100644 --- a/core/src/main/java/org/springframework/security/authentication/AnonymousAuthenticationToken.java +++ b/core/src/main/java/org/springframework/security/authentication/AnonymousAuthenticationToken.java @@ -20,6 +20,7 @@ import java.io.Serializable; import java.util.Collection; import org.springframework.security.core.GrantedAuthority; +import org.springframework.util.Assert; /** * Represents an anonymous Authentication. @@ -48,7 +49,7 @@ public class AnonymousAuthenticationToken extends AbstractAuthenticationToken im */ public AnonymousAuthenticationToken(String key, Object principal, Collection authorities) { - this(extractKeyHash(key), nullSafeValue(principal), authorities); + this(extractKeyHash(key), principal, authorities); } /** @@ -63,9 +64,10 @@ public class AnonymousAuthenticationToken extends AbstractAuthenticationToken im Collection authorities) { super(authorities); - if (authorities == null || authorities.isEmpty()) { - throw new IllegalArgumentException("Cannot pass null or empty values to constructor"); + if (principal == null || "".equals(principal)) { + throw new IllegalArgumentException("principal cannot be null or empty"); } + Assert.notEmpty(authorities, "authorities cannot be null or empty"); this.keyHash = keyHash; this.principal = principal; @@ -76,15 +78,8 @@ public class AnonymousAuthenticationToken extends AbstractAuthenticationToken im // ======================================================================================================== private static Integer extractKeyHash(String key) { - Object value = nullSafeValue(key); - return value.hashCode(); - } - - private static Object nullSafeValue(Object value) { - if (value == null || "".equals(value)) { - throw new IllegalArgumentException("Cannot pass null or empty values to constructor"); - } - return value; + Assert.hasLength(key, "key cannot be empty or null"); + return key.hashCode(); } public boolean equals(Object obj) { diff --git a/core/src/test/java/org/springframework/security/authentication/anonymous/AnonymousAuthenticationTokenTests.java b/core/src/test/java/org/springframework/security/authentication/anonymous/AnonymousAuthenticationTokenTests.java index 13031de5b4..7547507867 100644 --- a/core/src/test/java/org/springframework/security/authentication/anonymous/AnonymousAuthenticationTokenTests.java +++ b/core/src/test/java/org/springframework/security/authentication/anonymous/AnonymousAuthenticationTokenTests.java @@ -19,6 +19,7 @@ package org.springframework.security.authentication.anonymous; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; +import java.util.Collections; import java.util.List; import org.junit.Test; @@ -146,4 +147,19 @@ public class AnonymousAuthenticationTokenTests { token.setAuthenticated(false); assertThat(!token.isAuthenticated()).isTrue(); } + + @Test(expected = IllegalArgumentException.class) + public void constructorWhenNullAuthoritiesThenThrowIllegalArgumentException() throws Exception { + new AnonymousAuthenticationToken("key", "principal", null); + } + + @Test(expected = IllegalArgumentException.class) + public void constructorWhenEmptyAuthoritiesThenThrowIllegalArgumentException() throws Exception { + new AnonymousAuthenticationToken("key", "principal", Collections.emptyList()); + } + + @Test(expected = IllegalArgumentException.class) + public void constructorWhenPrincipalIsEmptyStringThenThrowIllegalArgumentException() throws Exception { + new AnonymousAuthenticationToken("key", "", ROLES_12); + } } diff --git a/core/src/test/java/org/springframework/security/jackson2/AnonymousAuthenticationTokenMixinTests.java b/core/src/test/java/org/springframework/security/jackson2/AnonymousAuthenticationTokenMixinTests.java index 542148e8f8..80da00c1d5 100644 --- a/core/src/test/java/org/springframework/security/jackson2/AnonymousAuthenticationTokenMixinTests.java +++ b/core/src/test/java/org/springframework/security/jackson2/AnonymousAuthenticationTokenMixinTests.java @@ -16,19 +16,18 @@ package org.springframework.security.jackson2; +import java.io.IOException; + import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonMappingException; import org.json.JSONException; import org.junit.Test; import org.skyscreamer.jsonassert.JSONAssert; + import org.springframework.security.authentication.AnonymousAuthenticationToken; -import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.core.userdetails.User; -import java.io.IOException; -import java.util.Collections; - import static org.assertj.core.api.Assertions.assertThat; /** @@ -45,15 +44,6 @@ public class AnonymousAuthenticationTokenMixinTests extends AbstractMixinTests { "[{\"@class\": \"org.springframework.security.core.authority.SimpleGrantedAuthority\", \"role\": \"ROLE_USER\"}]]}, \"authenticated\": true, \"keyHash\": " + hashKey.hashCode() + "," + "\"authorities\": [\"java.util.ArrayList\", [{\"@class\": \"org.springframework.security.core.authority.SimpleGrantedAuthority\", \"role\": \"ROLE_USER\"}]]}"; - @Test(expected = IllegalArgumentException.class) - public void testWithNullAuthorities() throws JsonProcessingException, JSONException { - new AnonymousAuthenticationToken("key", "principal", null); - } - - @Test(expected = IllegalArgumentException.class) - public void testWithEmptyAuthorities() throws JsonProcessingException, JSONException { - new AnonymousAuthenticationToken("key", "principal", Collections.emptyList()); - } @Test public void serializeAnonymousAuthenticationTokenTest() throws JsonProcessingException, JSONException {