From eb70db1deef5a547f48371a2833c7db3ab3982cd Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Mon, 21 Jan 2008 14:45:29 +0000 Subject: [PATCH] SEC-638: Allow property names as well as method names to be used in ReflectionSaltSource. --- .../dao/salt/ReflectionSaltSource.java | 63 ++++++++++----- .../dao/salt/ReflectionSaltSourceTests.java | 76 ++++++------------- 2 files changed, 70 insertions(+), 69 deletions(-) diff --git a/core/src/main/java/org/springframework/security/providers/dao/salt/ReflectionSaltSource.java b/core/src/main/java/org/springframework/security/providers/dao/salt/ReflectionSaltSource.java index 790e2f9081..75dcc887bf 100644 --- a/core/src/main/java/org/springframework/security/providers/dao/salt/ReflectionSaltSource.java +++ b/core/src/main/java/org/springframework/security/providers/dao/salt/ReflectionSaltSource.java @@ -22,15 +22,20 @@ import org.springframework.security.providers.dao.SaltSource; import org.springframework.security.userdetails.UserDetails; import org.springframework.beans.factory.InitializingBean; +import org.springframework.beans.BeanUtils; +import org.springframework.util.ReflectionUtils; +import org.springframework.util.Assert; import java.lang.reflect.Method; +import java.beans.PropertyDescriptor; /** - * Obtains a salt from a specified property of the {@link org.springframework.security.userdetails.User} object.

This allows - * you to subclass User and provide an additional bean getter for a salt. You should use a synthetic - * value that does not change, such as a database primary key. Do not use username if it is likely to - * change.

+ * Obtains a salt from a specified property of the {@link org.springframework.security.userdetails.User} object. + *

+ * This allows you to subclass User and provide an additional bean getter for a salt. You should use a + * synthetic value that does not change, such as a database primary key. Do not use username if it is + * likely to change. * * @author Ben Alex * @version $Id$ @@ -43,39 +48,59 @@ public class ReflectionSaltSource implements SaltSource, InitializingBean { //~ Methods ======================================================================================================== public void afterPropertiesSet() throws Exception { - if ((this.getUserPropertyToUse() == null) || "".equals(this.getUserPropertyToUse())) { - throw new IllegalArgumentException("A userPropertyToUse must be set"); - } + Assert.hasText(userPropertyToUse, "A userPropertyToUse must be set"); } /** - * Performs reflection on the passed User to obtain the salt.

The property identified by - * userPropertyToUse must be available from the passed User object. If it is not - * available, an {@link AuthenticationServiceException} will be thrown.

+ * Performs reflection on the passed User to obtain the salt. + *

+ * The property identified by userPropertyToUse must be available from the passed User + * object. If it is not available, an {@link AuthenticationServiceException} will be thrown. * * @param user which contains the method identified by userPropertyToUse * - * @return the result of invoking user.userPropertyToUse() + * @return the result of invoking user.userPropertyToUse(), or if the method doesn't exist, + * user.getUserPropertyToUse(). * * @throws AuthenticationServiceException if reflection fails */ public Object getSalt(UserDetails user) { - try { - Method reflectionMethod = user.getClass().getMethod(this.userPropertyToUse, new Class[] {}); + Method saltMethod = findSaltMethod(user); - return reflectionMethod.invoke(user, new Object[] {}); + try { + return saltMethod.invoke(user, new Object[] {}); } catch (Exception exception) { throw new AuthenticationServiceException(exception.getMessage(), exception); } } - public String getUserPropertyToUse() { + private Method findSaltMethod(UserDetails user) { + Method saltMethod = ReflectionUtils.findMethod(user.getClass(), userPropertyToUse); + + if (saltMethod == null) { + PropertyDescriptor pd = BeanUtils.getPropertyDescriptor(user.getClass(), userPropertyToUse); + + if (pd != null) { + saltMethod = pd.getReadMethod(); + } + + if (saltMethod == null) { + throw new AuthenticationServiceException("Unable to find salt method on user Object. Does the class '" + + user.getClass().getName() + "' have a method or getter named '" + userPropertyToUse + "' ?"); + } + } + + return saltMethod; + } + + protected String getUserPropertyToUse() { return userPropertyToUse; } /** - * The method name to call to obtain the salt. If your UserDetails contains a - * UserDetails.getSalt() method, you should set this property to getSalt. + * The method name to call to obtain the salt. Can be either a method name or a bean property name. If your + * UserDetails contains a UserDetails.getSalt() method, you should set this property to + * "getSalt" or "salt". * * @param userPropertyToUse the name of the getter to call to obtain the salt from the * UserDetails @@ -83,4 +108,8 @@ public class ReflectionSaltSource implements SaltSource, InitializingBean { public void setUserPropertyToUse(String userPropertyToUse) { this.userPropertyToUse = userPropertyToUse; } + + public String toString() { + return "ReflectionSaltSource[ userPropertyToUse='" + userPropertyToUse + "'; ]"; + } } diff --git a/core/src/test/java/org/springframework/security/providers/dao/salt/ReflectionSaltSourceTests.java b/core/src/test/java/org/springframework/security/providers/dao/salt/ReflectionSaltSourceTests.java index 857c9765c4..69d42d0e10 100644 --- a/core/src/test/java/org/springframework/security/providers/dao/salt/ReflectionSaltSourceTests.java +++ b/core/src/test/java/org/springframework/security/providers/dao/salt/ReflectionSaltSourceTests.java @@ -15,8 +15,6 @@ package org.springframework.security.providers.dao.salt; -import junit.framework.TestCase; - import org.springframework.security.AuthenticationServiceException; import org.springframework.security.GrantedAuthority; import org.springframework.security.GrantedAuthorityImpl; @@ -24,6 +22,8 @@ import org.springframework.security.GrantedAuthorityImpl; import org.springframework.security.userdetails.User; import org.springframework.security.userdetails.UserDetails; +import org.junit.Test; +import static junit.framework.Assert.*; /** * Tests {@link ReflectionSaltSource}. @@ -31,66 +31,38 @@ import org.springframework.security.userdetails.UserDetails; * @author Ben Alex * @version $Id$ */ -public class ReflectionSaltSourceTests extends TestCase { - //~ Constructors =================================================================================================== - - public ReflectionSaltSourceTests() { - super(); - } - - public ReflectionSaltSourceTests(String arg0) { - super(arg0); - } +public class ReflectionSaltSourceTests { + private UserDetails user = new User("scott", "wombat", true, true, true, true, + new GrantedAuthority[] {new GrantedAuthorityImpl("HOLDER")}); //~ Methods ======================================================================================================== - public static void main(String[] args) { - junit.textui.TestRunner.run(ReflectionSaltSourceTests.class); - } - - public final void setUp() throws Exception { - super.setUp(); - } - - public void testDetectsMissingUserPropertyToUse() throws Exception { + @Test(expected=IllegalArgumentException.class) + public void detectsMissingUserPropertyToUse() throws Exception { ReflectionSaltSource saltSource = new ReflectionSaltSource(); - - try { - saltSource.afterPropertiesSet(); - fail("Should have thrown IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - assertEquals("A userPropertyToUse must be set", expected.getMessage()); - } + saltSource.afterPropertiesSet(); } - public void testExceptionWhenInvalidPropertyRequested() { + @Test(expected=AuthenticationServiceException.class) + public void exceptionIsThrownWhenInvalidPropertyRequested() throws Exception { ReflectionSaltSource saltSource = new ReflectionSaltSource(); saltSource.setUserPropertyToUse("getDoesNotExist"); - - UserDetails user = new User("scott", "wombat", true, true, true, true, - new GrantedAuthority[] {new GrantedAuthorityImpl("HOLDER")}); - - try { - saltSource.getSalt(user); - fail("Should have thrown AuthenticationServiceException"); - } catch (AuthenticationServiceException expected) { - assertTrue(true); - } - } - - public void testGettersSetters() { - ReflectionSaltSource saltSource = new ReflectionSaltSource(); - saltSource.setUserPropertyToUse("getUsername"); - assertEquals("getUsername", saltSource.getUserPropertyToUse()); - } - - public void testNormalOperation() throws Exception { - ReflectionSaltSource saltSource = new ReflectionSaltSource(); - saltSource.setUserPropertyToUse("getUsername"); saltSource.afterPropertiesSet(); + saltSource.getSalt(user); + } + + @Test + public void methodNameAsPropertyToUseReturnsCorrectSaltValue() { + ReflectionSaltSource saltSource = new ReflectionSaltSource(); + saltSource.setUserPropertyToUse("getUsername"); - UserDetails user = new User("scott", "wombat", true, true, true, true, - new GrantedAuthority[] {new GrantedAuthorityImpl("HOLDER")}); assertEquals("scott", saltSource.getSalt(user)); } + + @Test + public void propertyNameAsPropertyToUseReturnsCorrectSaltValue() { + ReflectionSaltSource saltSource = new ReflectionSaltSource(); + saltSource.setUserPropertyToUse("password"); + assertEquals("wombat", saltSource.getSalt(user)); + } }