Sfoglia il codice sorgente

Polish constructor assertions

Previously the JSON modules didn't use Spring's Assert.

This commit changes the assertions to use Spring's Assert and does
some minor restructuring.

Issue gh-3736
Rob Winch 9 anni fa
parent
commit
03d8904a03

+ 3 - 9
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 <code>Authentication</code>.
@@ -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) {

+ 7 - 0
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.<GrantedAuthority>emptyList(),
+				new User("user", "password", Collections.<GrantedAuthority>emptyList()), null);
+	}
+
 	@Test
 	public void testEqualsWhenEqual() {
 		final Assertion assertion = new AssertionImpl("test");

+ 0 - 12
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.<GrantedAuthority>emptyList(),
-				new User("user", PASSWORD, Collections.<GrantedAuthority>emptyList()), null);
-	}
-
-	@Test(expected = IllegalArgumentException.class)
-	public void blankKeyTest() {
-		new CasAuthenticationToken("", "user", PASSWORD, Collections.<GrantedAuthority>emptyList(),
-				new User("user", PASSWORD, Collections.<GrantedAuthority>emptyList()), null);
-	}
-
 	@Test
 	public void serializeCasAuthenticationTest() throws JsonProcessingException, JSONException {
 		CasAuthenticationToken token = createCasAuthenticationToken();

+ 7 - 12
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 <code>Authentication</code>.
@@ -48,7 +49,7 @@ public class AnonymousAuthenticationToken extends AbstractAuthenticationToken im
 	 */
 	public AnonymousAuthenticationToken(String key, Object principal,
 										Collection<? extends GrantedAuthority> authorities) {
-		this(extractKeyHash(key), nullSafeValue(principal), authorities);
+		this(extractKeyHash(key), principal, authorities);
 	}
 
 	/**
@@ -63,9 +64,10 @@ public class AnonymousAuthenticationToken extends AbstractAuthenticationToken im
 										Collection<? extends GrantedAuthority> 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) {

+ 16 - 0
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.<GrantedAuthority>emptyList());
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void constructorWhenPrincipalIsEmptyStringThenThrowIllegalArgumentException() throws Exception {
+		new AnonymousAuthenticationToken("key", "", ROLES_12);
+	}
 }

+ 3 - 13
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.<GrantedAuthority>emptyList());
-	}
 
 	@Test
 	public void serializeAnonymousAuthenticationTokenTest() throws JsonProcessingException, JSONException {