KEYCLOAK-10307: check password history length in password verification (#6058)

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

@@ -26,6 +26,7 @@ import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserModel;
 
 import java.util.List;
+import java.util.stream.Collectors;
 
 /**
  * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@@ -60,7 +61,8 @@ public class HistoryPasswordPolicyProvider implements PasswordPolicyProvider {
                 }
             }
             List<CredentialModel> passwordHistory = session.userCredentialManager().getStoredCredentialsByType(realm, user, CredentialModel.PASSWORD_HISTORY);
-            for (CredentialModel cred : passwordHistory) {
+            List<CredentialModel> recentPasswordHistory = getRecent(passwordHistory, passwordHistoryPolicyValue - 1);
+            for (CredentialModel cred : recentPasswordHistory) {
                 PasswordHashProvider hash = session.getProvider(PasswordHashProvider.class, cred.getAlgorithm());
                 if (hash.verify(password, cred)) {
                     return new PolicyError(ERROR_MESSAGE, passwordHistoryPolicyValue);
@@ -71,6 +73,13 @@ public class HistoryPasswordPolicyProvider implements PasswordPolicyProvider {
         return null;
     }
 
+    private List<CredentialModel> getRecent(List<CredentialModel> passwordHistory, int limit) {
+        return passwordHistory.stream()
+                .sorted(CredentialModel.comparingByStartDateDesc())
+                .limit(limit)
+                .collect(Collectors.toList());
+    }
+
     @Override
     public Object parseConfig(String value) {
         return parseInteger(value, HistoryPasswordPolicyProviderFactory.DEFAULT_VALUE);

server-spi/src/main/java/org/keycloak/credential/CredentialModel.java

@@ -20,6 +20,7 @@ package org.keycloak.credential;
 import org.keycloak.common.util.MultivaluedHashMap;
 
 import java.io.Serializable;
+import java.util.Comparator;
 
 /**
  * Used just in cases when we want to "directly" update or retrieve the hash or salt of user credential (For example during export/import)
@@ -168,4 +169,12 @@ public class CredentialModel implements Serializable {
     public void setConfig(MultivaluedHashMap<String, String> config) {
         this.config = config;
     }
+
+    public static Comparator<CredentialModel> comparingByStartDateDesc() {
+        return (o1, o2) -> { // sort by date descending
+            Long o1Date = o1.getCreatedDate() == null ? Long.MIN_VALUE : o1.getCreatedDate();
+            Long o2Date = o2.getCreatedDate() == null ? Long.MIN_VALUE : o2.getCreatedDate();
+            return (-o1Date.compareTo(o2Date));
+        };
+    }
 }

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

@@ -118,11 +118,7 @@ public class PasswordCredentialProvider implements CredentialProvider, Credentia
             final int passwordsToLeave = expiredPasswordsPolicyValue - 2;
             if (list.size() > passwordsToLeave) {
                 list.stream()
-                  .sorted((o1, o2) -> { // sort by date descending
+                  .sorted(CredentialModel.comparingByStartDateDesc())
-                      Long o1Date = o1.getCreatedDate() == null ? Long.MIN_VALUE : o1.getCreatedDate();
-                      Long o2Date = o2.getCreatedDate() == null ? Long.MIN_VALUE : o2.getCreatedDate();
-                      return (- o1Date.compareTo(o2Date));
-                  })
                   .skip(passwordsToLeave)
                   .forEach(p -> getCredentialStore().removeStoredCredential(realm, user, p.getId()));
             }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java

@@ -561,6 +561,26 @@ public class AccountFormServiceTest extends AbstractTestRealmKeycloakTest {
         assertNumberOfStoredCredentials(1);
     }
 
+    @Test
+    public void changePasswordToOldOneAfterPasswordHistoryPolicyExpirationChange() {
+        userId = createUser("test", "user-changePasswordToOldOneAfterPasswordHistoryPolicyExpirationChange", "password");
+
+        setPasswordPolicy(PasswordPolicy.PASSWORD_HISTORY_ID + "(3)");
+
+        changePasswordPage.open();
+        loginPage.login("user-changePasswordToOldOneAfterPasswordHistoryPolicyExpirationChange", "password");
+        events.expectLogin().user(userId).client("account").detail(Details.REDIRECT_URI, getAccountRedirectUrl() + "?path=password").assertEvent();
+
+        assertNumberOfStoredCredentials(1);
+        assertChangePasswordSucceeds("password", "password1");
+        assertNumberOfStoredCredentials(2);
+        assertChangePasswordSucceeds("password1", "password2");
+        assertNumberOfStoredCredentials(3);
+
+        setPasswordPolicy(PasswordPolicy.PASSWORD_HISTORY_ID + "(2)");
+        assertChangePasswordSucceeds("password2", "password");
+    }
+
     @Test
     public void changePasswordWithPasswordHistoryPolicyExpiration() {
         userId = createUser("test", "user-changePasswordWithPasswordHistoryPolicyExpiration", "password");