Update encoders so they process salts.

core/src/main/java/org/acegisecurity/providers/encoding/BaseDigestPasswordEncoder.java

@@ -23,7 +23,7 @@ package net.sf.acegisecurity.providers.encoding;
  * @author colin sampaleanu
  * @version $Id$
  */
-public abstract class BaseDigestPasswordEncoder implements PasswordEncoder {
+public abstract class BaseDigestPasswordEncoder extends BasePasswordEncoder {
     //~ Instance fields ========================================================
 
     private boolean encodeHashAsBase64 = false;

core/src/main/java/org/acegisecurity/providers/encoding/BasePasswordEncoder.java

@@ -0,0 +1,115 @@
+/* Copyright 2004 Acegi Technology Pty Limited
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.sf.acegisecurity.providers.encoding;
+
+/**
+ * <p>
+ * Convenience base for all password encoders.
+ * </p>
+ *
+ * @author Ben Alex
+ * @version $Id$
+ */
+public abstract class BasePasswordEncoder implements PasswordEncoder {
+    //~ Methods ================================================================
+
+    /**
+     * Used by subclasses to extract the password and salt from a merged
+     * <code>String</code> created using {@link
+     * #mergePasswordAndSalt(String,Object,boolean)}.
+     * 
+     * <P>
+     * The first element in the returned array is the password. The second
+     * element is the salt. The salt array element will always be present,
+     * even if no salt was found in the <code>mergedPasswordSalt</code>
+     * argument.
+     * </p>
+     *
+     * @param mergedPasswordSalt as generated by
+     *        <code>mergePasswordAndSalt</code>
+     *
+     * @return an array, in which the first element is the password and the
+     *         second the salt
+     *
+     * @throws IllegalArgumentException DOCUMENT ME!
+     */
+    protected String[] demergePasswordAndSalt(String mergedPasswordSalt) {
+        if ((mergedPasswordSalt == null) || "".equals(mergedPasswordSalt)) {
+            throw new IllegalArgumentException(
+                "Cannot pass a null or empty String");
+        }
+
+        String password = mergedPasswordSalt;
+        String salt = "";
+
+        int saltBegins = mergedPasswordSalt.lastIndexOf("{");
+
+        if ((saltBegins != -1)
+            && ((saltBegins + 1) < mergedPasswordSalt.length())) {
+            salt = mergedPasswordSalt.substring(saltBegins + 1,
+                    mergedPasswordSalt.length() - 1);
+            password = mergedPasswordSalt.substring(0, saltBegins);
+        }
+
+        return new String[] {password, salt};
+    }
+
+    /**
+     * Used by subclasses to generate a merged password and salt
+     * <code>String</code>.
+     * 
+     * <P>
+     * The generated password will be in the form of
+     * <code>password{salt}</code>.
+     * </p>
+     * 
+     * <P>
+     * A <code>null</code> can be passed to either method, and will be handled
+     * correctly. If the <code>salt</code> is <code>null</code> or empty, the
+     * resulting generated password will simply be the passed
+     * <code>password</code>. The <code>toString</code> method of the
+     * <code>salt</code> will be used to represent the salt.
+     * </p>
+     *
+     * @param password the password to be used (can be <code>null</code>)
+     * @param salt the salt to be used (can be <code>null</code>)
+     * @param strict ensures salt doesn't contain the delimiters
+     *
+     * @return a merged password and salt <code>String</code>
+     *
+     * @throws IllegalArgumentException DOCUMENT ME!
+     */
+    protected String mergePasswordAndSalt(String password, Object salt,
+        boolean strict) {
+        if (password == null) {
+            password = "";
+        }
+
+        if (strict && (salt != null)) {
+            if ((salt.toString().lastIndexOf("{") != -1)
+                || (salt.toString().lastIndexOf("}") != -1)) {
+                throw new IllegalArgumentException(
+                    "Cannot use { or } in salt.toString()");
+            }
+        }
+
+        if ((salt == null) || "".equals(salt)) {
+            return password;
+        } else {
+            return password + "{" + salt.toString() + "}";
+        }
+    }
+}

core/src/main/java/org/acegisecurity/providers/encoding/Md5PasswordEncoder.java

@@ -29,7 +29,12 @@ import org.apache.commons.codec.digest.DigestUtils;
  * <code>String</code> ("") password.
  * </p>
  * 
+ * <P>
+ * As MD5 is a one-way hash, the salt can contain any characters.
+ * </p>
+ *
  * @author colin sampaleanu
+ * @author Ben Alex
  * @version $Id$
  */
 public class Md5PasswordEncoder extends BaseDigestPasswordEncoder
@@ -38,13 +43,13 @@ public class Md5PasswordEncoder extends BaseDigestPasswordEncoder
 
     public boolean isPasswordValid(String encPass, String rawPass, Object salt) {
         String pass1 = "" + encPass;
-        String pass2 = encodeInternal("" + rawPass);
+        String pass2 = encodeInternal(mergePasswordAndSalt(rawPass, salt, false));
 
         return pass1.equals(pass2);
     }
 
     public String encodePassword(String rawPass, Object salt) {
-        return encodeInternal("" + rawPass);
+        return encodeInternal(mergePasswordAndSalt(rawPass, salt, false));
     }
 
     private String encodeInternal(String input) {

core/src/main/java/org/acegisecurity/providers/encoding/PlaintextPasswordEncoder.java

@@ -20,10 +20,17 @@ package net.sf.acegisecurity.providers.encoding;
  * Plaintext implementation of PasswordEncoder.
  * </p>
  * 
+ * <P>
+ * As callers may wish to extract the password and salts separately from the
+ * encoded password, the salt must not contain reserved characters
+ * (specifically '{' and '}').
+ * </p>
+ *
  * @author colin sampaleanu
+ * @author Ben Alex
  * @version $Id$
  */
-public class PlaintextPasswordEncoder implements PasswordEncoder {
+public class PlaintextPasswordEncoder extends BasePasswordEncoder {
     //~ Instance fields ========================================================
 
     private boolean ignorePasswordCase = false;
@@ -49,8 +56,12 @@ public class PlaintextPasswordEncoder implements PasswordEncoder {
     }
 
     public boolean isPasswordValid(String encPass, String rawPass, Object salt) {
-        String pass1 = "" + encPass;
+        String pass1 = encPass + "";
-        String pass2 = "" + rawPass;
+
+        // Strict delimiters is false because pass2 never persisted anywhere
+        // and we want to avoid unnecessary exceptions as a result (the
+        // authentication will fail as the encodePassword never allows them)
+        String pass2 = mergePasswordAndSalt(rawPass, salt, false);
 
         if (!ignorePasswordCase) {
             return pass1.equals(pass2);
@@ -60,6 +71,28 @@ public class PlaintextPasswordEncoder implements PasswordEncoder {
     }
 
     public String encodePassword(String rawPass, Object salt) {
-        return rawPass;
+        return mergePasswordAndSalt(rawPass, salt, true);
+    }
+
+    /**
+     * Demerges the previously {@link #encodePassword(String,
+     * Object)}<code>String</code>.
+     * 
+     * <P>
+     * The resulting array is guaranteed to always contain two elements. The
+     * first is the password, and the second is the salt.
+     * </p>
+     * 
+     * <P>
+     * Throws an exception if <code>null</code> or an empty <code>String</code>
+     * is passed to the method.
+     * </p>
+     *
+     * @param password from {@link #encodePassword(String, Object)}
+     *
+     * @return an array containing the password and salt
+     */
+    public String[] obtainPasswordAndSalt(String password) {
+        return demergePasswordAndSalt(password);
     }
 }

core/src/main/java/org/acegisecurity/providers/encoding/ShaPasswordEncoder.java

@@ -29,7 +29,12 @@ import org.apache.commons.codec.digest.DigestUtils;
  * <code>String</code> ("") password.
  * </p>
  * 
+ * <P>
+ * As SHA is a one-way hash, the salt can contain any characters.
+ * </p>
+ *
  * @author colin sampaleanu
+ * @author Ben Alex
  * @version $Id$
  */
 public class ShaPasswordEncoder extends BaseDigestPasswordEncoder
@@ -38,13 +43,13 @@ public class ShaPasswordEncoder extends BaseDigestPasswordEncoder
 
     public boolean isPasswordValid(String encPass, String rawPass, Object salt) {
         String pass1 = "" + encPass;
-        String pass2 = encodeInternal("" + rawPass);
+        String pass2 = encodeInternal(mergePasswordAndSalt(rawPass, salt, false));
 
         return pass1.equals(pass2);
     }
 
     public String encodePassword(String rawPass, Object salt) {
-        return encodeInternal("" + rawPass);
+        return encodeInternal(mergePasswordAndSalt(rawPass, salt, false));
     }
 
     private String encodeInternal(String input) {

core/src/main/java/org/acegisecurity/providers/encoding/package.html

@@ -0,0 +1,5 @@
+<html>
+<body>
+Password encoding implementations.
+</body>
+</html>

core/src/test/java/org/acegisecurity/providers/encoding/BasePasswordEncoderTests.java

@@ -0,0 +1,158 @@
+/* Copyright 2004 Acegi Technology Pty Limited
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.sf.acegisecurity.providers.encoding;
+
+import junit.framework.TestCase;
+
+import org.springframework.dao.DataAccessException;
+
+
+/**
+ * <p>
+ * TestCase for BasePasswordEncoder.
+ * </p>
+ *
+ * @author Ben Alex
+ * @version $Id$
+ */
+public class BasePasswordEncoderTests extends TestCase {
+    //~ Methods ================================================================
+
+    public void testDemergeHandlesEmptyAndNullSalts() {
+        MockPasswordEncoder pwd = new MockPasswordEncoder();
+
+        String merged = pwd.nowMergePasswordAndSalt("password", null, true);
+
+        String[] demerged = pwd.nowDemergePasswordAndSalt(merged);
+        assertEquals("password", demerged[0]);
+        assertEquals("", demerged[1]);
+
+        merged = pwd.nowMergePasswordAndSalt("password", "", true);
+
+        demerged = pwd.nowDemergePasswordAndSalt(merged);
+        assertEquals("password", demerged[0]);
+        assertEquals("", demerged[1]);
+    }
+
+    public void testDemergeWithEmptyStringIsRejected() {
+        MockPasswordEncoder pwd = new MockPasswordEncoder();
+
+        try {
+            pwd.nowDemergePasswordAndSalt("");
+            fail("Should have thrown IllegalArgumentException");
+        } catch (IllegalArgumentException expected) {
+            assertEquals("Cannot pass a null or empty String",
+                expected.getMessage());
+        }
+    }
+
+    public void testDemergeWithNullIsRejected() {
+        MockPasswordEncoder pwd = new MockPasswordEncoder();
+
+        try {
+            pwd.nowDemergePasswordAndSalt(null);
+            fail("Should have thrown IllegalArgumentException");
+        } catch (IllegalArgumentException expected) {
+            assertEquals("Cannot pass a null or empty String",
+                expected.getMessage());
+        }
+    }
+
+    public void testMergeDemerge() {
+        MockPasswordEncoder pwd = new MockPasswordEncoder();
+
+        String merged = pwd.nowMergePasswordAndSalt("password", "foo", true);
+        assertEquals("password{foo}", merged);
+
+        String[] demerged = pwd.nowDemergePasswordAndSalt(merged);
+        assertEquals("password", demerged[0]);
+        assertEquals("foo", demerged[1]);
+    }
+
+    public void testMergeDemergeWithDelimitersInPassword() {
+        MockPasswordEncoder pwd = new MockPasswordEncoder();
+
+        String merged = pwd.nowMergePasswordAndSalt("p{ass{w{o}rd", "foo", true);
+        assertEquals("p{ass{w{o}rd{foo}", merged);
+
+        String[] demerged = pwd.nowDemergePasswordAndSalt(merged);
+        System.out.println(demerged[0]);
+        System.out.println(demerged[1]);
+
+        assertEquals("p{ass{w{o}rd", demerged[0]);
+        assertEquals("foo", demerged[1]);
+    }
+
+    public void testMergeDemergeWithNullAsPassword() {
+        MockPasswordEncoder pwd = new MockPasswordEncoder();
+
+        String merged = pwd.nowMergePasswordAndSalt(null, "foo", true);
+        assertEquals("{foo}", merged);
+
+        String[] demerged = pwd.nowDemergePasswordAndSalt(merged);
+        assertEquals("", demerged[0]);
+        assertEquals("foo", demerged[1]);
+    }
+
+    public void testStrictMergeRejectsDelimitersInSalt1() {
+        MockPasswordEncoder pwd = new MockPasswordEncoder();
+
+        try {
+            pwd.nowMergePasswordAndSalt("password", "f{oo", true);
+            fail("Should have thrown IllegalArgumentException");
+        } catch (IllegalArgumentException expected) {
+            assertEquals("Cannot use { or } in salt.toString()",
+                expected.getMessage());
+        }
+    }
+
+    public void testStrictMergeRejectsDelimitersInSalt2() {
+        MockPasswordEncoder pwd = new MockPasswordEncoder();
+
+        try {
+            pwd.nowMergePasswordAndSalt("password", "f}oo", true);
+            fail("Should have thrown IllegalArgumentException");
+        } catch (IllegalArgumentException expected) {
+            assertEquals("Cannot use { or } in salt.toString()",
+                expected.getMessage());
+        }
+    }
+
+    //~ Inner Classes ==========================================================
+
+    private class MockPasswordEncoder extends BasePasswordEncoder {
+        public boolean isPasswordValid(String encPass, String rawPass,
+            Object salt) throws DataAccessException {
+            throw new UnsupportedOperationException(
+                "mock method not implemented");
+        }
+
+        public String encodePassword(String rawPass, Object salt)
+            throws DataAccessException {
+            throw new UnsupportedOperationException(
+                "mock method not implemented");
+        }
+
+        public String[] nowDemergePasswordAndSalt(String password) {
+            return demergePasswordAndSalt(password);
+        }
+
+        public String nowMergePasswordAndSalt(String password, Object salt,
+            boolean strict) {
+            return mergePasswordAndSalt(password, salt, strict);
+        }
+    }
+}

core/src/test/java/org/acegisecurity/providers/encoding/Md5PasswordEncoderTests.java

@@ -0,0 +1,50 @@
+/* Copyright 2004 Acegi Technology Pty Limited
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.sf.acegisecurity.providers.encoding;
+
+import junit.framework.TestCase;
+
+
+/**
+ * <p>
+ * TestCase for PlaintextPasswordEncoder.
+ * </p>
+ *
+ * @author colin sampaleanu
+ * @author Ben Alex
+ * @version $Id$
+ */
+public class Md5PasswordEncoderTests extends TestCase {
+    //~ Methods ================================================================
+
+    public void testBasicFunctionality() {
+        Md5PasswordEncoder pe = new Md5PasswordEncoder();
+        String raw = "abc123";
+        String badRaw = "abc321";
+        String salt = "THIS_IS_A_SALT";
+        String encoded = pe.encodePassword(raw, salt);
+        assertTrue(pe.isPasswordValid(encoded, raw, salt));
+        assertFalse(pe.isPasswordValid(encoded, badRaw, salt));
+        assertTrue(encoded.length() == 32);
+
+        // now try Base64
+        pe.setEncodeHashAsBase64(true);
+        encoded = pe.encodePassword(raw, salt);
+        assertTrue(pe.isPasswordValid(encoded, raw, salt));
+        assertFalse(pe.isPasswordValid(encoded, badRaw, salt));
+        assertTrue(encoded.length() != 32);
+    }
+}

core/src/test/java/org/acegisecurity/providers/encoding/PlaintextPasswordEncoderTests.java

@@ -0,0 +1,69 @@
+/* Copyright 2004 Acegi Technology Pty Limited
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.sf.acegisecurity.providers.encoding;
+
+import junit.framework.TestCase;
+
+
+/**
+ * <p>
+ * TestCase for PlaintextPasswordEncoder.
+ * </p>
+ *
+ * @author colin sampaleanu
+ * @author Ben Alex
+ * @version $Id$
+ */
+public class PlaintextPasswordEncoderTests extends TestCase {
+    //~ Methods ================================================================
+
+    public void testBasicFunctionality() {
+        PlaintextPasswordEncoder pe = new PlaintextPasswordEncoder();
+
+        String raw = "abc123";
+        String rawDiffCase = "AbC123";
+        String badRaw = "abc321";
+        String salt = "THIS_IS_A_SALT";
+
+        String encoded = pe.encodePassword(raw, salt);
+        assertEquals("abc123{THIS_IS_A_SALT}", encoded);
+        assertTrue(pe.isPasswordValid(encoded, raw, salt));
+        assertFalse(pe.isPasswordValid(encoded, badRaw, salt));
+
+        // make sure default is not to ignore password case
+        assertFalse(pe.isIgnorePasswordCase());
+        encoded = pe.encodePassword(rawDiffCase, salt);
+        assertFalse(pe.isPasswordValid(encoded, raw, salt));
+
+        // now check for ignore password case
+        pe = new PlaintextPasswordEncoder();
+        pe.setIgnorePasswordCase(true);
+
+        // should be able to validate even without encoding
+        encoded = pe.encodePassword(rawDiffCase, salt);
+        assertTrue(pe.isPasswordValid(encoded, raw, salt));
+        assertFalse(pe.isPasswordValid(encoded, badRaw, salt));
+    }
+
+    public void testMergeDemerge() {
+        PlaintextPasswordEncoder pwd = new PlaintextPasswordEncoder();
+
+        String merged = pwd.encodePassword("password", "foo");
+        String[] demerged = pwd.obtainPasswordAndSalt(merged);
+        assertEquals("password", demerged[0]);
+        assertEquals("foo", demerged[1]);
+    }
+}

core/src/test/java/org/acegisecurity/providers/encoding/ShaPasswordEncoderTests.java

@@ -0,0 +1,50 @@
+/* Copyright 2004 Acegi Technology Pty Limited
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.sf.acegisecurity.providers.encoding;
+
+import junit.framework.TestCase;
+
+
+/**
+ * <p>
+ * TestCase for ShaPasswordEncoder.
+ * </p>
+ *
+ * @author colin sampaleanu
+ * @author Ben Alex
+ * @version $Id$
+ */
+public class ShaPasswordEncoderTests extends TestCase {
+    //~ Methods ================================================================
+
+    public void testBasicFunctionality() {
+        ShaPasswordEncoder pe = new ShaPasswordEncoder();
+        String raw = "abc123";
+        String badRaw = "abc321";
+        String salt = "THIS_IS_A_SALT";
+        String encoded = pe.encodePassword(raw, salt);
+        assertTrue(pe.isPasswordValid(encoded, raw, salt));
+        assertFalse(pe.isPasswordValid(encoded, badRaw, salt));
+        assertTrue(encoded.length() == 40);
+
+        // now try Base64
+        pe.setEncodeHashAsBase64(true);
+        encoded = pe.encodePassword(raw, salt);
+        assertTrue(pe.isPasswordValid(encoded, raw, salt));
+        assertFalse(pe.isPasswordValid(encoded, badRaw, salt));
+        assertTrue(encoded.length() != 40);
+    }
+}