Default password hashing algorithm should be set to default password hash provider (#28128)

Closes #28120

Signed-off-by: stianst <stianst@gmail.com>

server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProvider.java

@@ -61,7 +61,7 @@ public class Pbkdf2PasswordHashProvider implements PasswordHashProvider {
 
     @Override
     public boolean policyCheck(PasswordPolicy policy, PasswordCredentialModel credential) {
-        int policyHashIterations = policy.getHashIterations();
+        int policyHashIterations = policy != null ? policy.getHashIterations() : -1;
         if (policyHashIterations == -1) {
             policyHashIterations = defaultIterations;
         }

server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProviderFactory.java

@@ -55,4 +55,9 @@ public class Pbkdf2PasswordHashProviderFactory extends AbstractPbkdf2PasswordHas
     public String getId() {
         return ID;
     }
+
+    @Override
+    public int order() {
+        return -100;
+    }
 }

server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2Sha256PasswordHashProviderFactory.java

@@ -27,4 +27,9 @@ public class Pbkdf2Sha256PasswordHashProviderFactory extends AbstractPbkdf2Passw
     public String getId() {
         return ID;
     }
+
+    @Override
+    public int order() {
+        return 100;
+    }
 }

server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2Sha512PasswordHashProviderFactory.java

@@ -27,4 +27,9 @@ public class Pbkdf2Sha512PasswordHashProviderFactory extends AbstractPbkdf2Passw
     public String getId() {
         return ID;
     }
+
+    @Override
+    public int order() {
+        return 200;
+    }
 }

server-spi-private/src/main/java/org/keycloak/policy/HashAlgorithmPasswordPolicyProviderFactory.java

@@ -32,6 +32,8 @@ public class HashAlgorithmPasswordPolicyProviderFactory implements PasswordPolic
 
     private KeycloakSession session;
 
+    private String defaultHashAlgorithm;
+
     @Override
     public PasswordPolicyProvider create(KeycloakSession session) {
         this.session = session;
@@ -44,6 +46,7 @@ public class HashAlgorithmPasswordPolicyProviderFactory implements PasswordPolic
 
     @Override
     public void postInit(KeycloakSessionFactory factory) {
+        defaultHashAlgorithm = factory.getProviderFactory(PasswordHashProvider.class).getId();
     }
 
     @Override
@@ -77,7 +80,7 @@ public class HashAlgorithmPasswordPolicyProviderFactory implements PasswordPolic
 
     @Override
     public String getDefaultConfigValue() {
-        return PasswordPolicy.HASH_ALGORITHM_DEFAULT;
+        return defaultHashAlgorithm;
     }
 
     @Override
@@ -87,12 +90,14 @@ public class HashAlgorithmPasswordPolicyProviderFactory implements PasswordPolic
 
     @Override
     public Object parseConfig(String value) {
-        String providerId = value != null && value.length() > 0 ? value : PasswordPolicy.HASH_ALGORITHM_DEFAULT;
+        if (value == null) {
-        PasswordHashProvider provider = session.getProvider(PasswordHashProvider.class, providerId);
+            throw new PasswordPolicyConfigException("Password hashing provider id must be set");
+        }
+        PasswordHashProvider provider = session.getProvider(PasswordHashProvider.class, value);
         if (provider == null) {
             throw new PasswordPolicyConfigException("Password hashing provider not found");
         }
-        return providerId;
+        return value;
     }
 
 }

server-spi-private/src/main/java/org/keycloak/policy/HashIterationsPasswordPolicyProviderFactory.java

@@ -75,7 +75,7 @@ public class HashIterationsPasswordPolicyProviderFactory implements PasswordPoli
 
     @Override
     public String getDefaultConfigValue() {
-        return String.valueOf(PasswordPolicy.HASH_ITERATIONS_DEFAULT);
+        return "-1";
     }
 
     @Override

server-spi/src/main/java/org/keycloak/models/PasswordPolicy.java

@@ -33,12 +33,8 @@ public class PasswordPolicy implements Serializable {
 
     public static final String HASH_ALGORITHM_ID = "hashAlgorithm";
 
-    public static final String HASH_ALGORITHM_DEFAULT = "pbkdf2-sha512";
-
     public static final String HASH_ITERATIONS_ID = "hashIterations";
 
-    public static final int HASH_ITERATIONS_DEFAULT = 210_000;
-
     public static final String PASSWORD_HISTORY_ID = "passwordHistory";
 
     public static final String FORCE_EXPIRED_ID = "forceExpiredPasswordChange";
@@ -81,7 +77,7 @@ public class PasswordPolicy implements Serializable {
         if (policyConfig.containsKey(HASH_ALGORITHM_ID)) {
             return getPolicyConfig(HASH_ALGORITHM_ID);
         } else {
-            return HASH_ALGORITHM_DEFAULT;
+            return null;
         }
     }
 

services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java

@@ -100,27 +100,16 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
         return challengeResponse;
     }
 
-    protected void runDefaultDummyHash(AuthenticationFlowContext context) {
-        PasswordHashProvider hash = context.getSession().getProvider(PasswordHashProvider.class, PasswordPolicy.HASH_ALGORITHM_DEFAULT);
-        hash.encode("SlightlyLongerDummyPassword", PasswordPolicy.HASH_ITERATIONS_DEFAULT);
-    }
-
     protected void dummyHash(AuthenticationFlowContext context) {
-        PasswordPolicy policy = context.getRealm().getPasswordPolicy();
+        PasswordPolicy passwordPolicy = context.getRealm().getPasswordPolicy();
-        if (policy == null) {
+        PasswordHashProvider provider;
-            runDefaultDummyHash(context);
+        if (passwordPolicy != null && passwordPolicy.getHashAlgorithm() != null) {
-            return;
+            provider = context.getSession().getProvider(PasswordHashProvider.class, passwordPolicy.getHashAlgorithm());
         } else {
-            PasswordHashProvider hash = context.getSession().getProvider(PasswordHashProvider.class, policy.getHashAlgorithm());
+            provider = context.getSession().getProvider(PasswordHashProvider.class);
-            if (hash == null) {
-                runDefaultDummyHash(context);
-                return;
-
-            } else {
-                hash.encode("SlightlyLongerDummyPassword", policy.getHashIterations());
         }
-        }
+        int iterations = passwordPolicy != null ? passwordPolicy.getHashIterations() : -1;
-
+        provider.encode("SlightlyLongerDummyPassword", iterations);
     }
 
     public void testInvalidUser(AuthenticationFlowContext context, UserModel user) {

services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java

@@ -124,12 +124,16 @@ public class PasswordCredentialProvider implements CredentialProvider<PasswordCr
 
 
     protected PasswordHashProvider getHashProvider(PasswordPolicy policy) {
-        PasswordHashProvider hash = session.getProvider(PasswordHashProvider.class, policy.getHashAlgorithm());
+        if (policy != null && policy.getHashAlgorithm() != null) {
-        if (hash == null) {
+            PasswordHashProvider provider = session.getProvider(PasswordHashProvider.class, policy.getHashAlgorithm());
+            if (provider != null) {
+                return provider;
+            } else {
                 logger.warnv("Realm PasswordPolicy PasswordHashProvider {0} not found", policy.getHashAlgorithm());
-            return session.getProvider(PasswordHashProvider.class, PasswordPolicy.HASH_ALGORITHM_DEFAULT);
             }
-        return hash;
+        }
+
+        return session.getProvider(PasswordHashProvider.class);
     }
 
     @Override
@@ -183,23 +187,8 @@ public class PasswordCredentialProvider implements CredentialProvider<PasswordCr
                 logger.debugv("Failed password validation for user {0} ", user.getUsername());
                 return false;
             }
-            PasswordPolicy policy = realm.getPasswordPolicy();
-            if (policy == null) {
-                return true;
-            }
-            hash = getHashProvider(policy);
-            if (hash == null) {
-                return true;
-            }
-            if (hash.policyCheck(policy, password)) {
-                return true;
-            }
 
-            PasswordCredentialModel newPassword = hash.encodedCredential(input.getChallengeResponse(), policy.getHashIterations());
+            rehashPasswordIfRequired(session, realm, user, input, password);
-            newPassword.setId(password.getId());
-            newPassword.setCreatedDate(password.getCreatedDate());
-            newPassword.setUserLabel(password.getUserLabel());
-            user.credentialManager().updateStoredCredential(newPassword);
         } catch (Throwable t) {
             logger.warn("Error when validating user password", t);
             return false;
@@ -208,6 +197,26 @@ public class PasswordCredentialProvider implements CredentialProvider<PasswordCr
         return true;
     }
 
+    private void rehashPasswordIfRequired(KeycloakSession session, RealmModel realm, UserModel user, CredentialInput input, PasswordCredentialModel password) {
+        PasswordPolicy passwordPolicy = realm.getPasswordPolicy();
+        PasswordHashProvider provider;
+        if (passwordPolicy != null && passwordPolicy.getHashAlgorithm() != null) {
+            provider = session.getProvider(PasswordHashProvider.class, passwordPolicy.getHashAlgorithm());
+        } else {
+            provider = session.getProvider(PasswordHashProvider.class);
+        }
+
+        if (!provider.policyCheck(passwordPolicy, password)) {
+            int iterations = passwordPolicy != null ? passwordPolicy.getHashIterations() : -1;
+
+            PasswordCredentialModel newPassword = provider.encodedCredential(input.getChallengeResponse(), iterations);
+            newPassword.setId(password.getId());
+            newPassword.setCreatedDate(password.getCreatedDate());
+            newPassword.setUserLabel(password.getUserLabel());
+            user.credentialManager().updateStoredCredential(newPassword);
+        }
+    }
+
     @Override
     public String getType() {
         return PasswordCredentialModel.TYPE;

testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/BackwardsCompatibilityUserStorage.java

@@ -33,6 +33,7 @@ import org.keycloak.credential.CredentialInputUpdater;
 import org.keycloak.credential.CredentialInputValidator;
 import org.keycloak.credential.CredentialModel;
 import org.keycloak.credential.hash.PasswordHashProvider;
+import org.keycloak.credential.hash.Pbkdf2Sha512PasswordHashProviderFactory;
 import org.keycloak.models.GroupModel;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.OTPPolicy;
@@ -150,7 +151,7 @@ public class BackwardsCompatibilityUserStorage implements UserLookupProvider, Us
             hashProvider.encode(userCredentialModel.getValue(), policy.getHashIterations(), newPassword);
 
             // Test expected values of credentialModel
-            assertEquals(newPassword.getAlgorithm(), policy.getHashAlgorithm());
+            assertEquals(newPassword.getAlgorithm(), Pbkdf2Sha512PasswordHashProviderFactory.ID);
             assertNotNull(newPassword.getValue());
             assertNotNull(newPassword.getSalt());
 
@@ -191,12 +192,11 @@ public class BackwardsCompatibilityUserStorage implements UserLookupProvider, Us
     }
 
     protected PasswordHashProvider getHashProvider(PasswordPolicy policy) {
-        PasswordHashProvider hash = session.getProvider(PasswordHashProvider.class, policy.getHashAlgorithm());
+        if (policy != null && policy.getHashAlgorithm() != null) {
-        if (hash == null) {
+            return session.getProvider(PasswordHashProvider.class, policy.getHashAlgorithm());
-            log.warnv("Realm PasswordPolicy PasswordHashProvider {0} not found", policy.getHashAlgorithm());
+        } else {
-            return session.getProvider(PasswordHashProvider.class, PasswordPolicy.HASH_ALGORITHM_DEFAULT);
+            return session.getProvider(PasswordHashProvider.class);
         }
-        return hash;
     }
 
     @Override

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java

@@ -40,6 +40,7 @@ import org.keycloak.common.util.Base64;
 import org.keycloak.common.util.MultivaluedHashMap;
 import org.keycloak.common.util.ObjectUtil;
 import org.keycloak.credential.CredentialModel;
+import org.keycloak.credential.hash.Pbkdf2Sha512PasswordHashProviderFactory;
 import org.keycloak.events.admin.OperationType;
 import org.keycloak.events.admin.ResourceType;
 import org.keycloak.models.Constants;
@@ -523,8 +524,8 @@ public class UserTest extends AbstractAdminTest {
         CredentialModel credential = fetchCredentials("user_rawpw");
         assertNotNull("Expecting credential", credential);
         PasswordCredentialModel pcm = PasswordCredentialModel.createFromCredentialModel(credential);
-        assertEquals(PasswordPolicy.HASH_ALGORITHM_DEFAULT, pcm.getPasswordCredentialData().getAlgorithm());
+        assertEquals(Pbkdf2Sha512PasswordHashProviderFactory.ID, pcm.getPasswordCredentialData().getAlgorithm());
-        assertEquals(PasswordPolicy.HASH_ITERATIONS_DEFAULT, pcm.getPasswordCredentialData().getHashIterations());
+        assertEquals(Pbkdf2Sha512PasswordHashProviderFactory.DEFAULT_ITERATIONS, pcm.getPasswordCredentialData().getHashIterations());
         assertNotEquals("ABCD", pcm.getPasswordSecretData().getValue());
         assertEquals(CredentialRepresentation.PASSWORD, credential.getType());
     }
@@ -2773,8 +2774,8 @@ public class UserTest extends AbstractAdminTest {
         PasswordCredentialModel credential = PasswordCredentialModel
                 .createFromCredentialModel(fetchCredentials("user_rawpw"));
         assertNotNull("Expecting credential", credential);
-        assertEquals(PasswordPolicy.HASH_ALGORITHM_DEFAULT, credential.getPasswordCredentialData().getAlgorithm());
+        assertEquals(Pbkdf2Sha512PasswordHashProviderFactory.ID, credential.getPasswordCredentialData().getAlgorithm());
-        assertEquals(PasswordPolicy.HASH_ITERATIONS_DEFAULT, credential.getPasswordCredentialData().getHashIterations());
+        assertEquals(Pbkdf2Sha512PasswordHashProviderFactory.DEFAULT_ITERATIONS, credential.getPasswordCredentialData().getHashIterations());
         assertNotEquals("ABCD", credential.getPasswordSecretData().getValue());
         assertEquals(CredentialRepresentation.PASSWORD, credential.getType());
 
@@ -2791,8 +2792,8 @@ public class UserTest extends AbstractAdminTest {
         PasswordCredentialModel updatedCredential = PasswordCredentialModel
                 .createFromCredentialModel(fetchCredentials("user_rawpw"));
         assertNotNull("Expecting credential", updatedCredential);
-        assertEquals(PasswordPolicy.HASH_ALGORITHM_DEFAULT, updatedCredential.getPasswordCredentialData().getAlgorithm());
+        assertEquals(Pbkdf2Sha512PasswordHashProviderFactory.ID, updatedCredential.getPasswordCredentialData().getAlgorithm());
-        assertEquals(PasswordPolicy.HASH_ITERATIONS_DEFAULT, updatedCredential.getPasswordCredentialData().getHashIterations());
+        assertEquals(Pbkdf2Sha512PasswordHashProviderFactory.DEFAULT_ITERATIONS, updatedCredential.getPasswordCredentialData().getHashIterations());
         assertNotEquals("EFGH", updatedCredential.getPasswordSecretData().getValue());
         assertEquals(CredentialRepresentation.PASSWORD, updatedCredential.getType());
     }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/FederatedStorageExportImportTest.java

@@ -84,11 +84,11 @@ public class FederatedStorageExportImportTest extends AbstractAuthTest {
     }
 
     public static PasswordHashProvider getHashProvider(KeycloakSession session, PasswordPolicy policy) {
-        PasswordHashProvider hash = session.getProvider(PasswordHashProvider.class, policy.getHashAlgorithm());
+        if (policy != null && policy.getHashAlgorithm() != null) {
-        if (hash == null) {
+            return session.getProvider(PasswordHashProvider.class, policy.getHashAlgorithm());
-            return session.getProvider(PasswordHashProvider.class, PasswordPolicy.HASH_ALGORITHM_DEFAULT);
+        } else {
+            return session.getProvider(PasswordHashProvider.class);
         }
-        return hash;
     }
 
 

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/PasswordHashingTest.java

@@ -113,6 +113,27 @@ public class PasswordHashingTest extends AbstractTestRealmKeycloakTest {
         assertEncoded(credential, "password", credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA1", 1);
     }
 
+    @Test
+    public void testPasswordRehashedToDefaultProviderIfHashAlgorithmRemoved() {
+        setPasswordPolicy("hashAlgorithm(" + Pbkdf2Sha256PasswordHashProviderFactory.ID + ")");
+
+        String username = "testPasswordRehashedToDefaultProviderIfHashAlgorithmRemoved";
+        createUser(username);
+
+        PasswordCredentialModel credential = PasswordCredentialModel.createFromCredentialModel(fetchCredentials(username));
+
+        assertEquals(Pbkdf2Sha256PasswordHashProviderFactory.ID, credential.getPasswordCredentialData().getAlgorithm());
+
+        setPasswordPolicy("");
+
+        loginPage.open();
+        loginPage.login(username, "password");
+
+        credential = PasswordCredentialModel.createFromCredentialModel(fetchCredentials(username));
+
+        assertEquals(Pbkdf2Sha512PasswordHashProviderFactory.ID, credential.getPasswordCredentialData().getAlgorithm());
+    }
+
     @Test
     public void testPasswordRehashedOnIterationsChanged() throws Exception {
         setPasswordPolicy("hashIterations(10000)");

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/policy/PasswordPolicyTest.java

@@ -19,7 +19,6 @@ package org.keycloak.testsuite.policy;
 
 import org.junit.Assert;
 import org.junit.Test;
-import org.keycloak.credential.hash.Pbkdf2Sha512PasswordHashProviderFactory;
 import org.keycloak.models.ModelException;
 import org.keycloak.models.PasswordPolicy;
 import org.keycloak.models.RealmModel;
@@ -50,15 +49,6 @@ import static org.junit.Assert.fail;
  */
 public class PasswordPolicyTest extends AbstractKeycloakTest {
 
-    @Test
-    public void testDefaultPasswordPolicySettings() {
-        testingClient.server("passwordPolicy").run(session -> {
-            RealmModel realmModel = session.getContext().getRealm();
-            PasswordPolicy passwordPolicy = realmModel.getPasswordPolicy();
-            Assert.assertEquals(Pbkdf2Sha512PasswordHashProviderFactory.ID, passwordPolicy.getHashAlgorithm());
-        });
-    }
-
     @Test
     public void testLength() {
         testingClient.server("passwordPolicy").run(session -> {