SEC-1495: Convert User class equals and hashcode methods to only use the "username" property.

This prevents situations where other data may have changed when a User object is reloaded (during a subsequent authentication attempt, in which case and Set.contains()/Map.containsKey() will return false even though the collection in question contains a principal representing the same user.
This commit is contained in:
Luke Taylor 2010-06-10 22:27:50 +01:00
parent 1dd4787194
commit d56adb8ffb
2 changed files with 38 additions and 64 deletions

View File

@ -32,8 +32,14 @@ import org.springframework.util.Assert;
* Implemented with value object semantics (immutable after construction, like a <code>String</code>).
* Developers may use this class directly, subclass it, or write their own {@link UserDetails} implementation from
* scratch.
* <p>
* {@code equals} and {@code hashcode} implementations are based on the {@code username} property only, as the
* intention is that lookups of the same user principal object (in a user registry, for example) will match
* where the objects represent the same user, not just when all the properties (authorities, password for
* example) are the same.
*
* @author Ben Alex
* @author Luke Taylor
*/
public class User implements UserDetails {
//~ Instance fields ================================================================================================
@ -153,61 +159,27 @@ public class User implements UserDetails {
}
}
/**
* Returns {@code true} if the supplied object is a {@code User} instance with the
* same {@code username} value.
* <p>
* In other words, the objects are equal if they have the same username, representing the
* same principal.
*/
@Override
public boolean equals(Object rhs) {
if (!(rhs instanceof User) || (rhs == null)) {
return false;
if (rhs instanceof User) {
return username.equals(((User) rhs).username);
}
User user = (User) rhs;
// We rely on constructor to guarantee any User has non-null
// authorities
if (!authorities.equals(user.authorities)) {
return false;
}
// We rely on constructor to guarantee non-null username and password
return (this.getPassword().equals(user.getPassword()) && this.getUsername().equals(user.getUsername())
&& (this.isAccountNonExpired() == user.isAccountNonExpired())
&& (this.isAccountNonLocked() == user.isAccountNonLocked())
&& (this.isCredentialsNonExpired() == user.isCredentialsNonExpired())
&& (this.isEnabled() == user.isEnabled()));
return false;
}
/**
* Returns the hashcode of the {@code username}.
*/
@Override
public int hashCode() {
int code = 9792;
for (GrantedAuthority authority : getAuthorities()) {
code = code * (authority.hashCode() % 7);
}
if (this.getPassword() != null) {
code = code * (this.getPassword().hashCode() % 7);
}
if (this.getUsername() != null) {
code = code * (this.getUsername().hashCode() % 7);
}
if (this.isAccountNonExpired()) {
code = code * -2;
}
if (this.isAccountNonLocked()) {
code = code * -3;
}
if (this.isCredentialsNonExpired()) {
code = code * -5;
}
if (this.isEnabled()) {
code = code * -7;
}
return code;
return username.hashCode();
}
@Override

View File

@ -19,7 +19,9 @@ import static org.junit.Assert.*;
import java.io.ByteArrayOutputStream;
import java.io.ObjectOutputStream;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import org.junit.Test;
import org.springframework.security.core.GrantedAuthority;
@ -37,24 +39,24 @@ public class UserTests {
//~ Methods ========================================================================================================
@Test
public void testEquals() {
User user1 = new User("rod", "koala", true, true, true, true,ROLE_12);
public void equalsReturnsTrueIfUsernamesAreTheSame() {
User user1 = new User("rod", "koala", true, true, true, true, ROLE_12);
assertFalse(user1.equals(null));
assertFalse(user1.equals("A STRING"));
assertTrue(user1.equals(user1));
assertTrue(user1.equals(new User("rod", "koala", true, true, true, true,ROLE_12)));
// Equal as the new User will internally sort the GrantedAuthorities in the correct order, before running equals()
assertTrue(user1.equals(new User("rod", "koala", true, true, true, true,
AuthorityUtils.createAuthorityList("ROLE_TWO","ROLE_ONE"))));
assertFalse(user1.equals(new User("DIFFERENT_USERNAME", "koala", true, true, true, true, ROLE_12)));
assertFalse(user1.equals(new User("rod", "DIFFERENT_PASSWORD", true, true, true, true, ROLE_12)));
assertFalse(user1.equals(new User("rod", "koala", false, true, true, true, ROLE_12)));
assertFalse(user1.equals(new User("rod", "koala", true, false, true, true, ROLE_12)));
assertFalse(user1.equals(new User("rod", "koala", true, true, false, true, ROLE_12)));
assertFalse(user1.equals(new User("rod", "koala", true, true, true, false, ROLE_12)));
assertFalse(user1.equals(new User("rod", "koala", true, true, true, true,
AuthorityUtils.createAuthorityList("ROLE_ONE"))));
assertTrue(user1.equals(new User("rod", "notthesame", true, true, true, true, ROLE_12)));
}
@Test
public void hashLookupOnlyDependsOnUsername() throws Exception {
User user1 = new User("rod", "koala", true, true, true, true, ROLE_12);
Set<UserDetails> users = new HashSet<UserDetails>();
users.add(user1);
assertTrue(users.contains(new User("rod", "koala", true, true, true, true, ROLE_12)));
assertTrue(users.contains(new User("rod", "anotherpass", false, false, false, false, AuthorityUtils.createAuthorityList("ROLE_X"))));
assertFalse(users.contains(new User("bod", "koala", true, true, true, true, ROLE_12)));
}
@Test
@ -116,9 +118,9 @@ public class UserTests {
}
@Test
public void testUserIsEnabled() throws Exception {
public void enabledFlagIsFalseForDisabledAccount() throws Exception {
UserDetails user = new User("rod", "koala", false, true, true, true, ROLE_12);
assertTrue(!user.isEnabled());
assertFalse(user.isEnabled());
}
@Test