SEC-638: Allow property names as well as method names to be used in ReflectionSaltSource.

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.<P>This allows
- * you to subclass <code>User</code> 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 <code>username</code> if it is likely to
- * change.</p>
+ * Obtains a salt from a specified property of the {@link org.springframework.security.userdetails.User} object.
+ * <p>
+ * This allows you to subclass <code>User</code> 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 <code>username</code> 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 <code>User</code> to obtain the salt.<P>The property identified by
-     * <code>userPropertyToUse</code> must be available from the passed <code>User</code> object. If it is not
-     * available, an {@link AuthenticationServiceException} will be thrown.</p>
+     * Performs reflection on the passed <code>User</code> to obtain the salt.
+     * <p>
+     * The property identified by <code>userPropertyToUse</code> must be available from the passed <code>User</code>
+     * object. If it is not available, an {@link AuthenticationServiceException} will be thrown.
      *
      * @param user which contains the method identified by <code>userPropertyToUse</code>
      *
-     * @return the result of invoking <code>user.userPropertyToUse()</code>
+     * @return the result of invoking <tt>user.userPropertyToUse()</tt>, or if the method doesn't exist,
+     * <tt>user.getUserPropertyToUse()</tt>.
      *
      * @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 <code>UserDetails</code> contains a
-     * <code>UserDetails.getSalt()</code> method, you should set this property to <code>getSalt</code>.
+     * The method name to call to obtain the salt. Can be either a method name or a bean property name. If your
+     * <code>UserDetails</code> contains a <code>UserDetails.getSalt()</code> method, you should set this property to
+     * "getSalt" or "salt".
      *
      * @param userPropertyToUse the name of the <b>getter</b> to call to obtain the salt from the
      *        <code>UserDetails</code>
@@ -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 + "'; ]";
+    }
 }

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));
+    }
 }