Update AuthenticationManager to update weakly stored passwords on login

diff --git a/src/main/java/com/gitblit/manager/AuthenticationManager.java b/src/main/java/com/gitblit/manager/AuthenticationManager.java
index 7a1fd9f24fe5f3b78f8efc58452c924aa35725cd..46be2ef235cd272f09f6ac2e7f6b3719e5da99b4 100644 (file)
--- a/src/main/java/com/gitblit/manager/AuthenticationManager.java
+++ b/src/main/java/com/gitblit/manager/AuthenticationManager.java
@@ -519,7 +519,8 @@ public class AuthenticationManager implements IAuthenticationManager {
         */
        protected UserModel authenticateLocal(UserModel user, char [] password) {
                UserModel returnedUser = null;
-               //weak password hash
+               boolean strongHashUsed = false;
+
                if (user.password.startsWith(StringUtils.MD5_TYPE)) {
                        // password digest
                        String md5 = StringUtils.MD5_TYPE + StringUtils.getMD5(new String(password));
@@ -533,19 +534,47 @@ public class AuthenticationManager implements IAuthenticationManager {
                        if (user.password.equalsIgnoreCase(md5)) {
                                returnedUser = user;
                        }
-               } else if (user.password.equals(new String(password))) {
-                       // plain-text password
-                       returnedUser = user;
                } else if (user.password.startsWith(SecurePasswordHashUtils.PBKDF2WITHHMACSHA256_TYPE)){
-                       //strong hash
+                       // strong hash
                        SecurePasswordHashUtils hashUtils = SecurePasswordHashUtils.get();
             boolean isPasswordValid = hashUtils.isPasswordCorrect(password, user.password);
             if(isPasswordValid){
                returnedUser = user;
+               strongHashUsed = true;
             }
+               } else if (user.password.equals(new String(password))) {
+                       // plain-text password
+                       returnedUser = user;
+               } 
+               
+               // validate user
+               returnedUser = validateAuthentication(returnedUser, AuthenticationType.CREDENTIALS);
+               
+               // if no strong hash was used to store the password, try to update it based on the settings
+               if(!strongHashUsed){
+                       updateStoredPassword(returnedUser, password);
                }
                
-               return validateAuthentication(returnedUser, AuthenticationType.CREDENTIALS);
+               return returnedUser;
+       }
+
+       /**
+        * Update stored password to a strong hash if configured.
+        *
+        * @param user the user to be updated
+        * @param password the password
+        */
+       protected void updateStoredPassword(UserModel user, char[] password) {
+               // check if user has successfully authenticated i.e. is not null 
+               if(user != null){
+                       // check if strong hash algorithm is configured
+                       String algorithm = settings.getString(Keys.realm.passwordStorage, SecurePasswordHashUtils.PBKDF2WITHHMACSHA256);
+                       if(algorithm.equals(SecurePasswordHashUtils.PBKDF2WITHHMACSHA256)){
+                               // rehash the provided correct password and 
+                               user.password = SecurePasswordHashUtils.get().createStoredPasswordFromPassword(password);
+                               userManager.updateUserModel(user);
+                       }       
+               }
        }
 
        /**

diff --git a/src/main/java/com/gitblit/utils/SecurePasswordHashUtils.java b/src/main/java/com/gitblit/utils/SecurePasswordHashUtils.java
index de2c00845c0d8c01999abf4510eb18f27180cb67..289084eee6d269830e252e44ef6e3829c9fe89dd 100644 (file)
--- a/src/main/java/com/gitblit/utils/SecurePasswordHashUtils.java
+++ b/src/main/java/com/gitblit/utils/SecurePasswordHashUtils.java
@@ -11,13 +11,16 @@ import java.security.spec.InvalidKeySpecException;
 import java.util.Arrays;
 
 /**
- * The Class SecurePasswordHashUtils provides methods to create and validate secure hashes from user passwords.
+ * The Class SecurePasswordHashUtils provides methods to create and validate
+ * secure hashes from user passwords.
  * 
- * It uses the concept proposed by OWASP - Hashing Java: https://www.owasp.org/index.php/Hashing_Java
+ * It uses the concept proposed by OWASP - Hashing Java:
+ * https://www.owasp.org/index.php/Hashing_Java
  */
 public class SecurePasswordHashUtils {
 
-       public static final String PBKDF2WITHHMACSHA256_TYPE = "PBKDF2WITHHMACSHA256:";
+       public static final String PBKDF2WITHHMACSHA256 = "PBKDF2WithHmacSHA256";
+       public static final String PBKDF2WITHHMACSHA256_TYPE = PBKDF2WITHHMACSHA256.toUpperCase() + ":";
 
        private static final SecureRandom RANDOM = new SecureRandom();
        private static final int ITERATIONS = 10000;
@@ -112,8 +115,21 @@ public class SecurePasswordHashUtils {
         * @return the sting to be stored in a file (users.conf)
         */
        public String createStoredPasswordFromPassword(String password) {
+               return createStoredPasswordFromPassword(password.toCharArray());
+       }
+
+       /**
+        * Creates the new secure hash from a password and formats it properly to be
+        * stored in a file.
+        *
+        * @param password
+        *            the password to be hashed
+        * @return the sting to be stored in a file (users.conf)
+        */
+       public String createStoredPasswordFromPassword(char[] password) {
                byte[] salt = getNextSalt();
-               return String.format("%s%s%s", SecurePasswordHashUtils.PBKDF2WITHHMACSHA256_TYPE, StringUtils.toHex(salt), StringUtils.toHex(hash(password.toCharArray(), salt)));
+               return String.format("%s%s%s", SecurePasswordHashUtils.PBKDF2WITHHMACSHA256_TYPE, StringUtils.toHex(salt),
+                               StringUtils.toHex(hash(password, salt)));
        }
 
        /**

diff --git a/src/test/java/com/gitblit/tests/AuthenticationManagerTest.java b/src/test/java/com/gitblit/tests/AuthenticationManagerTest.java
index f8dc88855fe1fa83bd6bb658bac1643c125f2547..31b7512c772f42989ad3cd40286d467998369d8b 100644 (file)
--- a/src/test/java/com/gitblit/tests/AuthenticationManagerTest.java
+++ b/src/test/java/com/gitblit/tests/AuthenticationManagerTest.java
@@ -55,6 +55,7 @@ import com.gitblit.manager.UserManager;
 import com.gitblit.models.TeamModel;
 import com.gitblit.models.UserModel;
 import com.gitblit.tests.mock.MemorySettings;
+import com.gitblit.utils.SecurePasswordHashUtils;
 import com.gitblit.utils.XssFilter;
 import com.gitblit.utils.XssFilter.AllowXssFilter;
 
@@ -658,12 +659,17 @@ public class AuthenticationManagerTest extends GitblitUnitTest {
                users.updateUserModel(user);
 
                assertNotNull(auth.authenticate(user.username, user.password.toCharArray(), null));
+               
+               // validate that plaintext password was automatically updated to hashed one
+               assertTrue(user.password.startsWith(SecurePasswordHashUtils.PBKDF2WITHHMACSHA256_TYPE));
+               
                user.disabled = true;
 
                users.updateUserModel(user);
                assertNull(auth.authenticate(user.username, user.password.toCharArray(), null));
                users.deleteUserModel(user);
        }
+       
 
        @Test
        public void testContenairAuthenticate() throws Exception {