diff --git a/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java b/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java
index 2be3471..98fd928 100644
--- a/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java
+++ b/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java
@@ -111,8 +111,8 @@ public class PasswordCredentialProvider implements CredentialProvider, Credentia
CredentialModel oldPassword = getPassword(realm, user);
if (oldPassword == null) return;
int expiredPasswordsPolicyValue = policy.getExpiredPasswords();
+ List<CredentialModel> list = getCredentialStore().getStoredCredentialsByType(realm, user, CredentialModel.PASSWORD_HISTORY);
if (expiredPasswordsPolicyValue > 1) {
- List<CredentialModel> list = getCredentialStore().getStoredCredentialsByType(realm, user, CredentialModel.PASSWORD_HISTORY);
// 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;
@@ -129,7 +129,8 @@ public class PasswordCredentialProvider implements CredentialProvider, Credentia
oldPassword.setType(CredentialModel.PASSWORD_HISTORY);
getCredentialStore().updateCredential(realm, user, oldPassword);
} else {
- session.userCredentialManager().removeStoredCredential(realm, user, oldPassword.getId());
+ list.stream().forEach(p -> getCredentialStore().removeStoredCredential(realm, user, p.getId()));
+ getCredentialStore().removeStoredCredential(realm, user, oldPassword.getId());
}
}
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java
index fdda242..cfae103 100755
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java
@@ -23,6 +23,7 @@ import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.keycloak.admin.client.resource.RealmResource;
+import org.keycloak.credential.CredentialModel;
import org.keycloak.events.Details;
import org.keycloak.events.Errors;
import org.keycloak.events.EventType;
@@ -30,6 +31,8 @@ import org.keycloak.models.AccountRoles;
import org.keycloak.models.AdminRoles;
import org.keycloak.models.Constants;
import org.keycloak.models.PasswordPolicy;
+import org.keycloak.models.RealmModel;
+import org.keycloak.models.UserModel;
import org.keycloak.models.utils.TimeBasedOTP;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.representations.idm.EventRepresentation;
@@ -40,6 +43,7 @@ import org.keycloak.services.resources.account.AccountFormService;
import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
import org.keycloak.testsuite.AssertEvents;
import org.keycloak.testsuite.admin.ApiUtil;
+import org.keycloak.testsuite.arquillian.AuthServerTestEnricher;
import org.keycloak.testsuite.drone.Different;
import org.keycloak.testsuite.pages.AccountApplicationsPage;
import org.keycloak.testsuite.pages.AccountFederatedIdentityPage;
@@ -66,11 +70,14 @@ import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import org.hamcrest.Matchers;
+import org.junit.Assume;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasItems;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
/**
@@ -410,87 +417,168 @@ public class AccountFormServiceTest extends AbstractTestRealmKeycloakTest {
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();
+ events.expectAccount(EventType.UPDATE_PASSWORD).user(userId).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();
+ events.expectAccount(EventType.UPDATE_PASSWORD_ERROR).user(userId).error(Errors.PASSWORD_REJECTED).assertEvent();
}
- @Test
+ private void assertNumberOfStoredCredentials(int expectedNumberOfStoredCredentials) {
+ Assume.assumeTrue("Works only on auth-server-undertow",
+ AuthServerTestEnricher.AUTH_SERVER_CONTAINER.equals(AuthServerTestEnricher.AUTH_SERVER_CONTAINER_DEFAULT));
+
+ final String uId = userId; // Needed for run-on-server
+ testingClient.server("test").run(session -> {
+ RealmModel realm = session.getContext().getRealm();
+ UserModel user = session.users().getUserById(uId, realm);
+ assertThat(user, Matchers.notNullValue());
+ List<CredentialModel> storedCredentials = session.userCredentialManager().getStoredCredentials(realm, user);
+ assertThat(storedCredentials, Matchers.hasSize(expectedNumberOfStoredCredentials));
+ });
+ }
+
+ @Test
public void changePasswordWithPasswordHistoryPolicyThreePasswords() {
+ userId = createUser("test", "user-changePasswordWithPasswordHistoryPolicyThreePasswords", "password");
+
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();
+ loginPage.login("user-changePasswordWithPasswordHistoryPolicyThreePasswords", "password");
+ events.expectLogin().user(userId).client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=password").assertEvent();
assertChangePasswordFails ("password", "password"); // current: password
+ assertNumberOfStoredCredentials(1);
assertChangePasswordSucceeds("password", "password3"); // current: password
+ assertNumberOfStoredCredentials(2);
assertChangePasswordFails ("password3", "password"); // current: password1, history: password
+ assertNumberOfStoredCredentials(2);
assertChangePasswordFails ("password3", "password3"); // current: password1, history: password
+ assertNumberOfStoredCredentials(2);
assertChangePasswordSucceeds("password3", "password4"); // current: password1, history: password
+ assertNumberOfStoredCredentials(3);
assertChangePasswordFails ("password4", "password"); // current: password2, history: password, password1
+ assertNumberOfStoredCredentials(3);
assertChangePasswordFails ("password4", "password3"); // current: password2, history: password, password1
+ assertNumberOfStoredCredentials(3);
assertChangePasswordFails ("password4", "password4"); // current: password2, history: password, password1
+ assertNumberOfStoredCredentials(3);
assertChangePasswordSucceeds("password4", "password5"); // current: password2, history: password, password1
+ assertNumberOfStoredCredentials(3);
assertChangePasswordSucceeds("password5", "password"); // current: password3, history: password1, password2
+ assertNumberOfStoredCredentials(3);
}
@Test
public void changePasswordWithPasswordHistoryPolicyTwoPasswords() {
+ userId = createUser("test", "user-changePasswordWithPasswordHistoryPolicyTwoPasswords", "password");
+
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();
+ loginPage.login("user-changePasswordWithPasswordHistoryPolicyTwoPasswords", "password");
+ events.expectLogin().user(userId).client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=password").assertEvent();
assertChangePasswordFails ("password", "password"); // current: password
+ assertNumberOfStoredCredentials(1);
assertChangePasswordSucceeds("password", "password1"); // current: password
+ assertNumberOfStoredCredentials(2);
assertChangePasswordFails ("password1", "password"); // current: password1, history: password
+ assertNumberOfStoredCredentials(2);
assertChangePasswordFails ("password1", "password1"); // current: password1, history: password
+ assertNumberOfStoredCredentials(2);
assertChangePasswordSucceeds("password1", "password2"); // current: password1, history: password
+ assertNumberOfStoredCredentials(2);
assertChangePasswordFails ("password2", "password1"); // current: password2, history: password1
+ assertNumberOfStoredCredentials(2);
assertChangePasswordFails ("password2", "password2"); // current: password2, history: password1
+ assertNumberOfStoredCredentials(2);
assertChangePasswordSucceeds("password2", "password"); // current: password2, history: password1
+ assertNumberOfStoredCredentials(2);
}
@Test
public void changePasswordWithPasswordHistoryPolicyOnePwds() {
+ userId = createUser("test", "user-changePasswordWithPasswordHistoryPolicyOnePwds", "password");
+
// One password means only the active password is checked
setPasswordPolicy(PasswordPolicy.PASSWORD_HISTORY_ID + "(1)");
changePasswordPage.open();
- loginPage.login("test-user@localhost", "password");
- events.expectLogin().client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=password").assertEvent();
+ loginPage.login("user-changePasswordWithPasswordHistoryPolicyOnePwds", "password");
+ events.expectLogin().user(userId).client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=password").assertEvent();
assertChangePasswordFails ("password", "password"); // current: password
+ assertNumberOfStoredCredentials(1);
assertChangePasswordSucceeds("password", "password6"); // current: password
+ assertNumberOfStoredCredentials(1);
assertChangePasswordFails ("password6", "password6"); // current: password1
+ assertNumberOfStoredCredentials(1);
assertChangePasswordSucceeds("password6", "password"); // current: password1
+ assertNumberOfStoredCredentials(1);
}
@Test
public void changePasswordWithPasswordHistoryPolicyZeroPwdsInHistory() {
+ userId = createUser("test", "user-changePasswordWithPasswordHistoryPolicyZeroPwdsInHistory", "password");
+
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();
+ loginPage.login("user-changePasswordWithPasswordHistoryPolicyZeroPwdsInHistory", "password");
+ events.expectLogin().user(userId).client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=password").assertEvent();
assertChangePasswordFails ("password", "password"); // current: password
+ assertNumberOfStoredCredentials(1);
assertChangePasswordSucceeds("password", "password1"); // current: password
+ assertNumberOfStoredCredentials(1);
assertChangePasswordFails ("password1", "password1"); // current: password1
+ assertNumberOfStoredCredentials(1);
assertChangePasswordSucceeds("password1", "password"); // current: password1
+ assertNumberOfStoredCredentials(1);
+ }
+
+ @Test
+ public void changePasswordWithPasswordHistoryPolicyExpiration() {
+ userId = createUser("test", "user-changePasswordWithPasswordHistoryPolicyExpiration", "password");
+
+ setPasswordPolicy(PasswordPolicy.PASSWORD_HISTORY_ID + "(3)");
+
+ changePasswordPage.open();
+ loginPage.login("user-changePasswordWithPasswordHistoryPolicyExpiration", "password");
+ events.expectLogin().user(userId).client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=password").assertEvent();
+
+ assertNumberOfStoredCredentials(1);
+ assertChangePasswordSucceeds("password", "password2"); // current: password
+ assertNumberOfStoredCredentials(2);
+ assertChangePasswordSucceeds("password2", "password4"); // current: password2, history: password
+ assertNumberOfStoredCredentials(3);
+
+ setPasswordPolicy(PasswordPolicy.PASSWORD_HISTORY_ID + "(2)");
+ assertChangePasswordSucceeds("password4", "password5"); // current: password4, history: password2
+ assertNumberOfStoredCredentials(2);
+
+ setPasswordPolicy(PasswordPolicy.PASSWORD_HISTORY_ID + "(1)");
+ assertChangePasswordSucceeds("password5", "password6"); // current: password5, history: -
+ assertNumberOfStoredCredentials(1);
+
+ setPasswordPolicy(PasswordPolicy.PASSWORD_HISTORY_ID + "(2)");
+ assertChangePasswordSucceeds("password6", "password7"); // current: password6, history: password5
+ assertNumberOfStoredCredentials(2);
+
+ setPasswordPolicy(PasswordPolicy.PASSWORD_HISTORY_ID + "(0)");
+ assertChangePasswordSucceeds("password7", "password8"); // current: password5, history: -
+ assertNumberOfStoredCredentials(1);
}
@Test
