Skip to content

Commit 03d8904

Browse files
author
Rob Winch
committed
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
1 parent d77ca17 commit 03d8904

File tree

6 files changed

+36
-46
lines changed

6 files changed

+36
-46
lines changed

cas/src/main/java/org/springframework/security/cas/authentication/CasAuthenticationToken.java

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.springframework.security.core.GrantedAuthority;
2525
import org.springframework.security.core.SpringSecurityCoreVersion;
2626
import org.springframework.security.core.userdetails.UserDetails;
27+
import org.springframework.util.Assert;
2728

2829
/**
2930
* Represents a successful CAS <code>Authentication</code>.
@@ -117,15 +118,8 @@ private CasAuthenticationToken(final Integer keyHash, final Object principal,
117118
// ========================================================================================================
118119

119120
private static Integer extractKeyHash(String key) {
120-
Object value = nullSafeValue(key);
121-
return value.hashCode();
122-
}
123-
124-
private static Object nullSafeValue(Object value) {
125-
if (value == null || "".equals(value)) {
126-
throw new IllegalArgumentException("Cannot pass null or empty values to constructor");
127-
}
128-
return value;
121+
Assert.hasLength(key, "key cannot be null or empty");
122+
return key.hashCode();
129123
}
130124

131125
public boolean equals(final Object obj) {

cas/src/test/java/org/springframework/security/cas/authentication/CasAuthenticationTokenTests.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.assertj.core.api.Assertions.assertThat;
2020
import static org.assertj.core.api.Assertions.fail;
2121

22+
import java.util.Collections;
2223
import java.util.List;
2324

2425
import org.jasig.cas.client.validation.Assertion;
@@ -102,6 +103,12 @@ public void testConstructorRejectsNulls() {
102103
}
103104
}
104105

106+
@Test(expected = IllegalArgumentException.class)
107+
public void constructorWhenEmptyKeyThenThrowsException() {
108+
new CasAuthenticationToken("", "user", "password", Collections.<GrantedAuthority>emptyList(),
109+
new User("user", "password", Collections.<GrantedAuthority>emptyList()), null);
110+
}
111+
105112
@Test
106113
public void testEqualsWhenEqual() {
107114
final Assertion assertion = new AssertionImpl("test");

cas/src/test/java/org/springframework/security/cas/jackson2/CasAuthenticationTokenMixinTests.java

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -76,18 +76,6 @@ ObjectMapper buildObjectMapper() {
7676
return mapper;
7777
}
7878

79-
@Test(expected = IllegalArgumentException.class)
80-
public void nullKeyTest() {
81-
new CasAuthenticationToken(null, "user", PASSWORD, Collections.<GrantedAuthority>emptyList(),
82-
new User("user", PASSWORD, Collections.<GrantedAuthority>emptyList()), null);
83-
}
84-
85-
@Test(expected = IllegalArgumentException.class)
86-
public void blankKeyTest() {
87-
new CasAuthenticationToken("", "user", PASSWORD, Collections.<GrantedAuthority>emptyList(),
88-
new User("user", PASSWORD, Collections.<GrantedAuthority>emptyList()), null);
89-
}
90-
9179
@Test
9280
public void serializeCasAuthenticationTest() throws JsonProcessingException, JSONException {
9381
CasAuthenticationToken token = createCasAuthenticationToken();

core/src/main/java/org/springframework/security/authentication/AnonymousAuthenticationToken.java

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.Collection;
2121

2222
import org.springframework.security.core.GrantedAuthority;
23+
import org.springframework.util.Assert;
2324

2425
/**
2526
* Represents an anonymous <code>Authentication</code>.
@@ -48,7 +49,7 @@ public class AnonymousAuthenticationToken extends AbstractAuthenticationToken im
4849
*/
4950
public AnonymousAuthenticationToken(String key, Object principal,
5051
Collection<? extends GrantedAuthority> authorities) {
51-
this(extractKeyHash(key), nullSafeValue(principal), authorities);
52+
this(extractKeyHash(key), principal, authorities);
5253
}
5354

5455
/**
@@ -63,9 +64,10 @@ private AnonymousAuthenticationToken(Integer keyHash, Object principal,
6364
Collection<? extends GrantedAuthority> authorities) {
6465
super(authorities);
6566

66-
if (authorities == null || authorities.isEmpty()) {
67-
throw new IllegalArgumentException("Cannot pass null or empty values to constructor");
67+
if (principal == null || "".equals(principal)) {
68+
throw new IllegalArgumentException("principal cannot be null or empty");
6869
}
70+
Assert.notEmpty(authorities, "authorities cannot be null or empty");
6971

7072
this.keyHash = keyHash;
7173
this.principal = principal;
@@ -76,15 +78,8 @@ private AnonymousAuthenticationToken(Integer keyHash, Object principal,
7678
// ========================================================================================================
7779

7880
private static Integer extractKeyHash(String key) {
79-
Object value = nullSafeValue(key);
80-
return value.hashCode();
81-
}
82-
83-
private static Object nullSafeValue(Object value) {
84-
if (value == null || "".equals(value)) {
85-
throw new IllegalArgumentException("Cannot pass null or empty values to constructor");
86-
}
87-
return value;
81+
Assert.hasLength(key, "key cannot be empty or null");
82+
return key.hashCode();
8883
}
8984

9085
public boolean equals(Object obj) {

core/src/test/java/org/springframework/security/authentication/anonymous/AnonymousAuthenticationTokenTests.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.assertj.core.api.Assertions.assertThat;
2020
import static org.assertj.core.api.Assertions.fail;
2121

22+
import java.util.Collections;
2223
import java.util.List;
2324

2425
import org.junit.Test;
@@ -146,4 +147,19 @@ public void testSetAuthenticatedIgnored() {
146147
token.setAuthenticated(false);
147148
assertThat(!token.isAuthenticated()).isTrue();
148149
}
150+
151+
@Test(expected = IllegalArgumentException.class)
152+
public void constructorWhenNullAuthoritiesThenThrowIllegalArgumentException() throws Exception {
153+
new AnonymousAuthenticationToken("key", "principal", null);
154+
}
155+
156+
@Test(expected = IllegalArgumentException.class)
157+
public void constructorWhenEmptyAuthoritiesThenThrowIllegalArgumentException() throws Exception {
158+
new AnonymousAuthenticationToken("key", "principal", Collections.<GrantedAuthority>emptyList());
159+
}
160+
161+
@Test(expected = IllegalArgumentException.class)
162+
public void constructorWhenPrincipalIsEmptyStringThenThrowIllegalArgumentException() throws Exception {
163+
new AnonymousAuthenticationToken("key", "", ROLES_12);
164+
}
149165
}

core/src/test/java/org/springframework/security/jackson2/AnonymousAuthenticationTokenMixinTests.java

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,18 @@
1616

1717
package org.springframework.security.jackson2;
1818

19+
import java.io.IOException;
20+
1921
import com.fasterxml.jackson.core.JsonProcessingException;
2022
import com.fasterxml.jackson.databind.JsonMappingException;
2123
import org.json.JSONException;
2224
import org.junit.Test;
2325
import org.skyscreamer.jsonassert.JSONAssert;
26+
2427
import org.springframework.security.authentication.AnonymousAuthenticationToken;
25-
import org.springframework.security.core.GrantedAuthority;
2628
import org.springframework.security.core.authority.SimpleGrantedAuthority;
2729
import org.springframework.security.core.userdetails.User;
2830

29-
import java.io.IOException;
30-
import java.util.Collections;
31-
3231
import static org.assertj.core.api.Assertions.assertThat;
3332

3433
/**
@@ -45,15 +44,6 @@ public class AnonymousAuthenticationTokenMixinTests extends AbstractMixinTests {
4544
"[{\"@class\": \"org.springframework.security.core.authority.SimpleGrantedAuthority\", \"role\": \"ROLE_USER\"}]]}, \"authenticated\": true, \"keyHash\": " + hashKey.hashCode() + "," +
4645
"\"authorities\": [\"java.util.ArrayList\", [{\"@class\": \"org.springframework.security.core.authority.SimpleGrantedAuthority\", \"role\": \"ROLE_USER\"}]]}";
4746

48-
@Test(expected = IllegalArgumentException.class)
49-
public void testWithNullAuthorities() throws JsonProcessingException, JSONException {
50-
new AnonymousAuthenticationToken("key", "principal", null);
51-
}
52-
53-
@Test(expected = IllegalArgumentException.class)
54-
public void testWithEmptyAuthorities() throws JsonProcessingException, JSONException {
55-
new AnonymousAuthenticationToken("key", "principal", Collections.<GrantedAuthority>emptyList());
56-
}
5747

5848
@Test
5949
public void serializeAnonymousAuthenticationTokenTest() throws JsonProcessingException, JSONException {

0 commit comments

Comments
 (0)