Update AuthenticationManager to update weakly stored passwords on login

3 files changed, 61 insertions, 10 deletions

diff --git a/src/main/java/com/gitblit/manager/AuthenticationManager.java b/src/main/java/com/gitblit/manager/AuthenticationManager.java
index 7a1fd9f2..46be2ef2 100644
--- 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 de2c0084..289084ee 100644
--- 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 f8dc8885..31b7512c 100644
--- 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 {