add: implement password migration to scrypt on login.

diff --git a/src/org/cacert/gigi/dbObjects/User.java b/src/org/cacert/gigi/dbObjects/User.java
index cf9c40b91fe3fd60cd16041338cfc563838d2f6c..0b77b97d6e53d76be0cdc2759eff6ae73ae45fe7 100644 (file)
--- a/src/org/cacert/gigi/dbObjects/User.java
+++ b/src/org/cacert/gigi/dbObjects/User.java
@@ -132,7 +132,7 @@ public class User extends CertificateOwner {
         if ( !rs.next()) {
             throw new GigiApiException("User not found... very bad.");
         }
-        if ( !PasswordHash.verifyHash(oldPass, rs.getString(1))) {
+        if (PasswordHash.verifyHash(oldPass, rs.getString(1)) == null) {
             throw new GigiApiException("Old password does not match.");
         }
         rs.close();

diff --git a/src/org/cacert/gigi/pages/LoginPage.java b/src/org/cacert/gigi/pages/LoginPage.java
index b19e8f5b5625d3e1bf64d8256769f9163b684998..128855fcf0dced4e2ed690cfda5c92fdc887ed88 100644 (file)
--- a/src/org/cacert/gigi/pages/LoginPage.java
+++ b/src/org/cacert/gigi/pages/LoginPage.java
@@ -97,7 +97,15 @@ public class LoginPage extends Page {
         ps.setString(1, un);
         GigiResultSet rs = ps.executeQuery();
         if (rs.next()) {
-            if (PasswordHash.verifyHash(pw, rs.getString(1))) {
+            String dbHash = rs.getString(1);
+            String hash = PasswordHash.verifyHash(pw, dbHash);
+            if (hash != null) {
+                if ( !hash.equals(dbHash)) {
+                    GigiPreparedStatement gps = DatabaseConnection.getInstance().prepare("UPDATE `users` SET `password`=? WHERE `email`=?");
+                    gps.setString(1, hash);
+                    gps.setString(2, un);
+                    gps.executeUpdate();
+                }
                 loginSession(req, User.getById(rs.getInt(2)));
             }
         }

diff --git a/src/org/cacert/gigi/util/PasswordHash.java b/src/org/cacert/gigi/util/PasswordHash.java
index d6b0b9066b7ab11ec72177e883f1156227fc3ce7..11e4d34419c78cbc46a6591d4262cfa35ebdf1a8 100644 (file)
--- a/src/org/cacert/gigi/util/PasswordHash.java
+++ b/src/org/cacert/gigi/util/PasswordHash.java
@@ -7,9 +7,28 @@ import com.lambdaworks.crypto.SCryptUtil;
 
 public class PasswordHash {
 
-    public static boolean verifyHash(String password, String hash) {
+    /**
+     * Verifies a password hash.
+     * 
+     * @param password
+     *            The password that should result in the given hash.
+     * @param hash
+     *            The hash to verify the password against.
+     * @return <ul>
+     *         <li><code>null</code>, if the password was valid</li>
+     *         <li><code>hash</code>, if the password is valid and the hash
+     *         doesn't need to be updated</li>
+     *         <li>a new hash, if the password is valid but the hash in the
+     *         database needs to be updated.</li>
+     *         </ul>
+     */
+    public static String verifyHash(String password, String hash) {
         if (hash.contains("$")) {
-            return SCryptUtil.check(password, hash);
+            if (SCryptUtil.check(password, hash)) {
+                return hash;
+            } else {
+                return null;
+            }
         }
         String newhash = sha1(password);
         boolean match = true;
@@ -19,7 +38,11 @@ public class PasswordHash {
         for (int i = 0; i < newhash.length(); i++) {
             match &= newhash.charAt(i) == hash.charAt(i);
         }
-        return match;
+        if (match) {
+            return hash(password);
+        } else {
+            return null;
+        }
     }
 
     private static String sha1(String password) {

diff --git a/tests/org/cacert/gigi/testUtils/RegisteredUser.java b/tests/org/cacert/gigi/testUtils/RegisteredUser.java
new file mode 100644 (file)
index 0000000..a5b5172
--- /dev/null
+++ b/tests/org/cacert/gigi/testUtils/RegisteredUser.java
@@ -0,0 +1,32 @@
+package org.cacert.gigi.testUtils;
+
+import org.cacert.gigi.dbObjects.User;
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runners.model.Statement;
+
+public class RegisteredUser implements TestRule {
+
+    User u;
+
+    @Override
+    public Statement apply(final Statement base, Description description) {
+        return new Statement() {
+
+            @Override
+            public void evaluate() throws Throwable {
+                u = User.getById(ManagedTest.createVerifiedUser("fn", "ln", ManagedTest.createUniqueName() + "@example.org", ManagedTest.TEST_PASSWORD));
+                try {
+                    base.evaluate();
+                } finally {
+
+                }
+            }
+        };
+    }
+
+    public User getUser() {
+        return u;
+    }
+
+}

diff --git a/tests/org/cacert/gigi/util/TestPasswordHash.java b/tests/org/cacert/gigi/util/TestPasswordHash.java
index 1c2d9d3a4e3cd7e7be2ac204da9495c844893db8..6e38a2bd82e78ed870235b7d9a9dd7b3b8990058 100644 (file)
--- a/tests/org/cacert/gigi/util/TestPasswordHash.java
+++ b/tests/org/cacert/gigi/util/TestPasswordHash.java
@@ -8,15 +8,14 @@ public class TestPasswordHash {
 
     @Test
     public void testVerify() {
-        assertTrue(PasswordHash.verifyHash("a", PasswordHash.hash("a")));
-        assertTrue(PasswordHash.verifyHash("", PasswordHash.hash("")));
-        assertTrue(PasswordHash.verifyHash("a1234", PasswordHash.hash("a1234")));
-        assertTrue(PasswordHash.verifyHash("auhlcb4 9x,IUQẞ&lvrvä", PasswordHash.hash("auhlcb4 9x,IUQẞ&lvrvä")));
+        assertTrue(PasswordHash.verifyHash("a", PasswordHash.hash("a")) != null);
+        assertTrue(PasswordHash.verifyHash("a1234", PasswordHash.hash("a1234")) != null);
+        assertTrue(PasswordHash.verifyHash("auhlcb4 9x,IUQẞ&lvrvä", PasswordHash.hash("auhlcb4 9x,IUQẞ&lvrvä")) != null);
     }
 
     @Test
     public void testVerifyNegative() {
-        assertFalse(PasswordHash.verifyHash("b", PasswordHash.hash("a")));
-        assertFalse(PasswordHash.verifyHash("ae", PasswordHash.hash("auhlcb4 9x,IUQẞ&lvrvä")));
+        assertFalse(PasswordHash.verifyHash("b", PasswordHash.hash("a")) != null);
+        assertFalse(PasswordHash.verifyHash("ae", PasswordHash.hash("auhlcb4 9x,IUQẞ&lvrvä")) != null);
     }
 }

diff --git a/tests/org/cacert/gigi/util/TestPasswordMigration.java b/tests/org/cacert/gigi/util/TestPasswordMigration.java
new file mode 100644 (file)
index 0000000..030794c
--- /dev/null
+++ b/tests/org/cacert/gigi/util/TestPasswordMigration.java
@@ -0,0 +1,37 @@
+package org.cacert.gigi.util;
+
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
+
+import java.io.IOException;
+
+import org.cacert.gigi.database.DatabaseConnection;
+import org.cacert.gigi.database.GigiPreparedStatement;
+import org.cacert.gigi.database.GigiResultSet;
+import org.cacert.gigi.testUtils.ManagedTest;
+import org.cacert.gigi.testUtils.RegisteredUser;
+import org.junit.Rule;
+import org.junit.Test;
+
+public class TestPasswordMigration extends ManagedTest {
+
+    @Rule
+    public RegisteredUser ru = new RegisteredUser();
+
+    @Test
+    public void testPasswordMigration() throws IOException {
+        GigiPreparedStatement stmt = DatabaseConnection.getInstance().prepare("UPDATE users SET `password`=SHA1(?) WHERE id=?");
+        stmt.setString(1, "a");
+        stmt.setInt(2, ru.getUser().getId());
+        stmt.execute();
+        String cookie = login(ru.getUser().getEmail(), "a");
+        assertTrue(isLoggedin(cookie));
+
+        stmt = DatabaseConnection.getInstance().prepare("SELECT `password` FROM users WHERE id=?");
+        stmt.setInt(1, ru.getUser().getId());
+        GigiResultSet res = stmt.executeQuery();
+        assertTrue(res.next());
+        String newHash = res.getString(1);
+        assertThat(newHash, containsString("$"));
+    }
+}