diff --git a/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java b/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java
index 0a6fe8c..b5bc57a 100644
--- a/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java
+++ b/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java
@@ -31,9 +31,7 @@ import org.keycloak.policy.PasswordPolicyManagerProvider;
import org.keycloak.policy.PolicyError;
import java.util.Collections;
-import java.util.Comparator;
import java.util.HashSet;
-import java.util.LinkedList;
import java.util.List;
import java.util.Set;
@@ -57,7 +55,7 @@ public class PasswordCredentialProvider implements CredentialProvider, Credentia
}
public CredentialModel getPassword(RealmModel realm, UserModel user) {
- List<CredentialModel> passwords = null;
+ List<CredentialModel> passwords;
if (user instanceof CachedUserModel && !((CachedUserModel)user).isMarkedForEviction()) {
CachedUserModel cached = (CachedUserModel)user;
passwords = (List<CredentialModel>)cached.getCachedWith().get(PASSWORD_CACHE_KEY);
@@ -107,25 +105,20 @@ public class PasswordCredentialProvider implements CredentialProvider, Credentia
CredentialModel oldPassword = getPassword(realm, user);
if (oldPassword == null) return;
int expiredPasswordsPolicyValue = policy.getExpiredPasswords();
- if (expiredPasswordsPolicyValue > -1) {
+ if (expiredPasswordsPolicyValue > 1) {
List<CredentialModel> list = getCredentialStore().getStoredCredentialsByType(realm, user, CredentialModel.PASSWORD_HISTORY);
- List<CredentialModel> history = new LinkedList<>();
- history.addAll(list);
- if (history.size() + 1 >= expiredPasswordsPolicyValue) {
- Collections.sort(history, new Comparator<CredentialModel>() {
- @Override
- public int compare(CredentialModel o1, CredentialModel o2) {
- long o1Date = o1.getCreatedDate() == null ? 0 : o1.getCreatedDate().longValue();
- long o2Date = o2.getCreatedDate() == null ? 0 : o2.getCreatedDate().longValue();
- if (o1Date > o2Date) return 1;
- else if (o1Date < o2Date) return -1;
- else return 0;
- }
- });
- for (int i = 0; i < history.size() + 2 - expiredPasswordsPolicyValue; i++) {
- getCredentialStore().removeStoredCredential(realm, user, history.get(i).getId());
- }
-
+ // oldPassword will expire few lines below, and there is one active password,
+ // hence (expiredPasswordsPolicyValue - 2) passwords should be left in history
+ final int passwordsToLeave = expiredPasswordsPolicyValue - 2;
+ if (list.size() > passwordsToLeave) {
+ list.stream()
+ .sorted((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));
+ })
+ .skip(passwordsToLeave)
+ .forEach(p -> getCredentialStore().removeStoredCredential(realm, user, p.getId()));
}
oldPassword.setType(CredentialModel.PASSWORD_HISTORY);
getCredentialStore().updateCredential(realm, user, oldPassword);
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountTest.java
index b9afd9b..b2487b8 100755
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountTest.java
@@ -22,9 +22,11 @@ import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
+
import org.keycloak.events.Details;
import org.keycloak.events.Errors;
import org.keycloak.events.EventType;
+import org.keycloak.models.PasswordPolicy;
import org.keycloak.models.utils.TimeBasedOTP;
import org.keycloak.representations.idm.EventRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
@@ -50,6 +52,7 @@ import org.keycloak.testsuite.util.IdentityProviderBuilder;
import org.keycloak.testsuite.util.OAuthClient;
import org.keycloak.testsuite.util.RealmBuilder;
import org.keycloak.testsuite.util.UserBuilder;
+
import org.openqa.selenium.By;
import org.openqa.selenium.WebDriver;
@@ -363,33 +366,90 @@ public class AccountTest extends TestRealmKeycloakTest {
events.expectAccount(EventType.UPDATE_PASSWORD).assertEvent();
}
+ private void assertChangePasswordSucceeds(String currentPassword, String newPassword) {
+ changePasswordPage.changePassword(currentPassword, newPassword, newPassword);
+ Assert.assertEquals("Your password has been updated.", profilePage.getSuccess());
+ events.expectAccount(EventType.UPDATE_PASSWORD).assertEvent();
+ }
+
+ private void assertChangePasswordFails(String currentPassword, String newPassword) {
+ changePasswordPage.changePassword(currentPassword, newPassword, newPassword);
+ Assert.assertThat(profilePage.getError(), containsString("Invalid password: must not be equal to any of last"));
+ events.expectAccount(EventType.UPDATE_PASSWORD_ERROR).error(Errors.PASSWORD_REJECTED).assertEvent();
+ }
+
+ @Test
+ public void changePasswordWithPasswordHistoryPolicyThreePasswords() {
+ setPasswordPolicy(PasswordPolicy.PASSWORD_HISTORY_ID + "(3)");
+
+ changePasswordPage.open();
+ loginPage.login("test-user@localhost", "password");
+ events.expectLogin().client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=password").assertEvent();
+
+ assertChangePasswordFails ("password", "password"); // current: password
+ assertChangePasswordSucceeds("password", "password1"); // current: password
+
+ assertChangePasswordFails ("password1", "password"); // current: password1, history: password
+ assertChangePasswordFails ("password1", "password1"); // current: password1, history: password
+ assertChangePasswordSucceeds("password1", "password2"); // current: password1, history: password
+
+ assertChangePasswordFails ("password2", "password"); // current: password2, history: password, password1
+ assertChangePasswordFails ("password2", "password1"); // current: password2, history: password, password1
+ assertChangePasswordFails ("password2", "password2"); // current: password2, history: password, password1
+ assertChangePasswordSucceeds("password2", "password3"); // current: password2, history: password, password1
+
+ assertChangePasswordSucceeds("password3", "password"); // current: password3, history: password1, password2
+ }
+
@Test
- public void changePasswordWithPasswordHistoryPolicy() {
- setPasswordPolicy("passwordHistory(2)");
+ public void changePasswordWithPasswordHistoryPolicyTwoPasswords() {
+ setPasswordPolicy(PasswordPolicy.PASSWORD_HISTORY_ID + "(2)");
changePasswordPage.open();
loginPage.login("test-user@localhost", "password");
events.expectLogin().client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=password").assertEvent();
- changePasswordPage.changePassword("password", "password", "password");
- Assert.assertEquals("Invalid password: must not be equal to any of last 2 passwords.", profilePage.getError());
- events.expectAccount(EventType.UPDATE_PASSWORD_ERROR).error(Errors.PASSWORD_REJECTED).assertEvent();
+ assertChangePasswordFails ("password", "password"); // current: password
+ assertChangePasswordSucceeds("password", "password1"); // current: password
- changePasswordPage.changePassword("password", "password1", "password1");
- Assert.assertEquals("Your password has been updated.", profilePage.getSuccess());
- events.expectAccount(EventType.UPDATE_PASSWORD).assertEvent();
+ assertChangePasswordFails ("password1", "password"); // current: password1, history: password
+ assertChangePasswordFails ("password1", "password1"); // current: password1, history: password
+ assertChangePasswordSucceeds("password1", "password2"); // current: password1, history: password
- changePasswordPage.changePassword("password1", "password", "password");
- Assert.assertEquals("Invalid password: must not be equal to any of last 2 passwords.", profilePage.getError());
- events.expectAccount(EventType.UPDATE_PASSWORD_ERROR).error(Errors.PASSWORD_REJECTED).assertEvent();
+ assertChangePasswordFails ("password2", "password1"); // current: password2, history: password1
+ assertChangePasswordFails ("password2", "password2"); // current: password2, history: password1
+ assertChangePasswordSucceeds("password2", "password"); // current: password2, history: password1
+ }
- changePasswordPage.changePassword("password1", "password1", "password1");
- Assert.assertEquals("Invalid password: must not be equal to any of last 2 passwords.", profilePage.getError());
- events.expectAccount(EventType.UPDATE_PASSWORD_ERROR).error(Errors.PASSWORD_REJECTED).assertEvent();
+ @Test
+ public void changePasswordWithPasswordHistoryPolicyOnePwds() {
+ // One password means only the active password is checked
+ setPasswordPolicy(PasswordPolicy.PASSWORD_HISTORY_ID + "(1)");
- changePasswordPage.changePassword("password1", "password2", "password2");
- Assert.assertEquals("Your password has been updated.", profilePage.getSuccess());
- events.expectAccount(EventType.UPDATE_PASSWORD).assertEvent();
+ changePasswordPage.open();
+ loginPage.login("test-user@localhost", "password");
+ events.expectLogin().client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=password").assertEvent();
+
+ assertChangePasswordFails ("password", "password"); // current: password
+ assertChangePasswordSucceeds("password", "password1"); // current: password
+
+ assertChangePasswordFails ("password1", "password1"); // current: password1
+ assertChangePasswordSucceeds("password1", "password"); // current: password1
+ }
+
+ @Test
+ public void changePasswordWithPasswordHistoryPolicyZeroPwdsInHistory() {
+ setPasswordPolicy(PasswordPolicy.PASSWORD_HISTORY_ID + "(0)");
+
+ changePasswordPage.open();
+ loginPage.login("test-user@localhost", "password");
+ events.expectLogin().client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=password").assertEvent();
+
+ assertChangePasswordFails ("password", "password"); // current: password
+ assertChangePasswordSucceeds("password", "password1"); // current: password
+
+ assertChangePasswordFails ("password1", "password1"); // current: password1
+ assertChangePasswordSucceeds("password1", "password"); // current: password1
}
@Test
