From fde5a5ae7e73510a8904e1c1d02ee5b41f729223 Mon Sep 17 00:00:00 2001 From: Maciej Mierzwa Date: Wed, 5 Jun 2024 21:26:07 +0200 Subject: [PATCH] feature: password age in days policy Closes #30210 Signed-off-by: Maciej Mierzwa --- .../policy/AgePasswordPolicyProvider.java | 70 +++++ .../AgePasswordPolicyProviderFactory.java | 55 ++++ ...cloak.policy.PasswordPolicyProviderFactory | 1 + .../org/keycloak/models/PasswordPolicy.java | 10 + .../PasswordCredentialProvider.java | 20 +- .../policy/PasswordAgePolicyTest.java | 245 ++++++++++++++++++ 6 files changed, 397 insertions(+), 4 deletions(-) create mode 100644 server-spi-private/src/main/java/org/keycloak/policy/AgePasswordPolicyProvider.java create mode 100644 server-spi-private/src/main/java/org/keycloak/policy/AgePasswordPolicyProviderFactory.java create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/policy/PasswordAgePolicyTest.java diff --git a/server-spi-private/src/main/java/org/keycloak/policy/AgePasswordPolicyProvider.java b/server-spi-private/src/main/java/org/keycloak/policy/AgePasswordPolicyProvider.java new file mode 100644 index 000000000000..fc24d4b93208 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/policy/AgePasswordPolicyProvider.java @@ -0,0 +1,70 @@ +package org.keycloak.policy; + +import org.keycloak.common.util.Time; +import org.keycloak.credential.hash.PasswordHashProvider; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.PasswordPolicy; +import org.keycloak.models.RealmModel; +import org.keycloak.models.UserModel; +import org.keycloak.models.credential.PasswordCredentialModel; +import org.jboss.logging.Logger; + +import java.time.Duration; + +public class AgePasswordPolicyProvider implements PasswordPolicyProvider { + private static final String ERROR_MESSAGE = "invalidPasswordGenericMessage"; + public static final Logger logger = Logger.getLogger(AgePasswordPolicyProvider.class); + private final KeycloakSession session; + + public AgePasswordPolicyProvider(KeycloakSession session) { + this.session = session; + } + + @Override + public PolicyError validate(String user, String password) { + return null; + } + + @Override + public PolicyError validate(RealmModel realm, UserModel user, String password) { + PasswordPolicy policy = session.getContext().getRealm().getPasswordPolicy(); + int passwordAgePolicyValue = policy.getPolicyConfig(PasswordPolicy.PASSWORD_AGE); + + if (passwordAgePolicyValue != -1) { + //current password check + if (user.credentialManager().getStoredCredentialsByTypeStream(PasswordCredentialModel.TYPE) + .map(PasswordCredentialModel::createFromCredentialModel) + .anyMatch(passwordCredential -> { + PasswordHashProvider hash = session.getProvider(PasswordHashProvider.class, + passwordCredential.getPasswordCredentialData().getAlgorithm()); + return hash != null && hash.verify(password, passwordCredential); + })) { + return new PolicyError(ERROR_MESSAGE, passwordAgePolicyValue); + } + + final long passwordMaxAgeMillis = Time.currentTimeMillis() - Duration.ofDays(passwordAgePolicyValue).toMillis(); + if (passwordAgePolicyValue > 0) { + if (user.credentialManager().getStoredCredentialsByTypeStream(PasswordCredentialModel.PASSWORD_HISTORY) + .filter(credentialModel -> credentialModel.getCreatedDate() > passwordMaxAgeMillis) + .map(PasswordCredentialModel::createFromCredentialModel) + .anyMatch(passwordCredential -> { + PasswordHashProvider hash = session.getProvider(PasswordHashProvider.class, + passwordCredential.getPasswordCredentialData().getAlgorithm()); + return hash.verify(password, passwordCredential); + })) { + return new PolicyError(ERROR_MESSAGE, passwordAgePolicyValue); + } + } + } + return null; + } + + @Override + public Object parseConfig(String value) { + return parseInteger(value, AgePasswordPolicyProviderFactory.DEFAULT_AGE_DAYS); + } + + @Override + public void close() { + } +} diff --git a/server-spi-private/src/main/java/org/keycloak/policy/AgePasswordPolicyProviderFactory.java b/server-spi-private/src/main/java/org/keycloak/policy/AgePasswordPolicyProviderFactory.java new file mode 100644 index 000000000000..2e3619342ae0 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/policy/AgePasswordPolicyProviderFactory.java @@ -0,0 +1,55 @@ +package org.keycloak.policy; + +import org.keycloak.Config; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.KeycloakSessionFactory; +import org.keycloak.models.PasswordPolicy; + +public class AgePasswordPolicyProviderFactory implements PasswordPolicyProviderFactory { + public static final Integer DEFAULT_AGE_DAYS = 30; + + @Override + public String getId() { + return PasswordPolicy.PASSWORD_AGE; + } + + @Override + public String getDisplayName() { + return "Not Recently Used (In Days)"; + } + + @Override + public String getConfigType() { + return PasswordPolicyProvider.INT_CONFIG_TYPE; + } + + @Override + public String getDefaultConfigValue() { + return String.valueOf(DEFAULT_AGE_DAYS); + } + + @Override + public boolean isMultiplSupported() { + return false; + } + + @Override + public PasswordPolicyProvider create(KeycloakSession session) { + return new AgePasswordPolicyProvider(session); + } + + @Override + public void init(Config.Scope config) { + + } + + @Override + public void postInit(KeycloakSessionFactory factory) { + + } + + @Override + public void close() { + + } +} diff --git a/server-spi-private/src/main/resources/META-INF/services/org.keycloak.policy.PasswordPolicyProviderFactory b/server-spi-private/src/main/resources/META-INF/services/org.keycloak.policy.PasswordPolicyProviderFactory index fbcdb937715f..84bd94060317 100644 --- a/server-spi-private/src/main/resources/META-INF/services/org.keycloak.policy.PasswordPolicyProviderFactory +++ b/server-spi-private/src/main/resources/META-INF/services/org.keycloak.policy.PasswordPolicyProviderFactory @@ -32,3 +32,4 @@ org.keycloak.policy.BlacklistPasswordPolicyProviderFactory org.keycloak.policy.NotEmailPasswordPolicyProviderFactory org.keycloak.policy.RecoveryCodesWarningThresholdPasswordPolicyProviderFactory org.keycloak.policy.MaxAuthAgePasswordPolicyProviderFactory +org.keycloak.policy.AgePasswordPolicyProviderFactory \ No newline at end of file diff --git a/server-spi/src/main/java/org/keycloak/models/PasswordPolicy.java b/server-spi/src/main/java/org/keycloak/models/PasswordPolicy.java index 7bcc2bdc446d..b60011af64ba 100755 --- a/server-spi/src/main/java/org/keycloak/models/PasswordPolicy.java +++ b/server-spi/src/main/java/org/keycloak/models/PasswordPolicy.java @@ -45,6 +45,8 @@ public class PasswordPolicy implements Serializable { public static final String MAX_AUTH_AGE_ID = "maxAuthAge"; + public static final String PASSWORD_AGE = "passwordAge"; + private Map policyConfig; private Builder builder; @@ -97,6 +99,14 @@ public int getExpiredPasswords() { } } + public int getPasswordAgeInDays() { + if (policyConfig.containsKey(PASSWORD_AGE)) { + return getPolicyConfig(PASSWORD_AGE); + } else { + return -1; + } + } + public int getDaysToExpirePassword() { if (policyConfig.containsKey(FORCE_EXPIRED_ID)) { return getPolicyConfig(FORCE_EXPIRED_ID); diff --git a/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java b/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java index 95277d88a762..a183cdb9b926 100644 --- a/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java +++ b/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java @@ -29,6 +29,7 @@ import org.keycloak.policy.PasswordPolicyManagerProvider; import org.keycloak.policy.PolicyError; +import java.time.Duration; import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -79,6 +80,7 @@ public CredentialModel createCredential(RealmModel realm, UserModel user, Passwo PasswordPolicy policy = realm.getPasswordPolicy(); int expiredPasswordsPolicyValue = policy.getExpiredPasswords(); + int passwordAgeInDaysPolicy = Math.max(0, policy.getPasswordAgeInDays()); // 1) create new or reset existing password CredentialModel createdCredential; @@ -94,24 +96,34 @@ public CredentialModel createCredential(RealmModel realm, UserModel user, Passwo createdCredential = credentialModel; // 2) add a password history item based on the old password - if (expiredPasswordsPolicyValue > 1) { + if (expiredPasswordsPolicyValue > 1 || passwordAgeInDaysPolicy > 0) { oldPassword.setId(null); oldPassword.setType(PasswordCredentialModel.PASSWORD_HISTORY); - user.credentialManager().createStoredCredential(oldPassword); + oldPassword = user.credentialManager().createStoredCredential(oldPassword); } } - - // 3) remove old password history items + + // 3) remove old password history items, if both history policies are set, more restrictive policy wins final int passwordHistoryListMaxSize = Math.max(0, expiredPasswordsPolicyValue - 1); + + final long passwordMaxAgeMillis = Time.currentTimeMillis() - Duration.ofDays(passwordAgeInDaysPolicy).toMillis(); + + CredentialModel finalOldPassword = oldPassword; user.credentialManager().getStoredCredentialsByTypeStream(PasswordCredentialModel.PASSWORD_HISTORY) .sorted(CredentialModel.comparingByStartDateDesc()) .skip(passwordHistoryListMaxSize) + .filter(credentialModel1 -> !(credentialModel1.getId().equals(finalOldPassword.getId()))) + .filter(credential -> passwordAgePredicate(credential, passwordMaxAgeMillis)) .collect(Collectors.toList()) .forEach(p -> user.credentialManager().removeStoredCredentialById(p.getId())); return createdCredential; } + private boolean passwordAgePredicate(CredentialModel credential, long passwordMaxAgeMillis) { + return credential.getCreatedDate() < passwordMaxAgeMillis; + } + @Override public boolean deleteCredential(RealmModel realm, UserModel user, String credentialId) { return user.credentialManager().removeStoredCredentialById(credentialId); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/policy/PasswordAgePolicyTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/policy/PasswordAgePolicyTest.java new file mode 100644 index 000000000000..e0e91a8c8f4a --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/policy/PasswordAgePolicyTest.java @@ -0,0 +1,245 @@ +package org.keycloak.testsuite.policy; + +import jakarta.ws.rs.BadRequestException; +import jakarta.ws.rs.core.Response; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.keycloak.admin.client.resource.UserResource; +import org.keycloak.common.util.Time; +import org.keycloak.representations.idm.CredentialRepresentation; +import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.testsuite.AbstractAuthTest; + +import java.util.function.Consumer; + +import static org.keycloak.representations.idm.CredentialRepresentation.PASSWORD; +import static org.keycloak.testsuite.admin.ApiUtil.getCreatedId; + + +public class PasswordAgePolicyTest extends AbstractAuthTest { + + UserResource user; + + private void setPasswordAgePolicy(String passwordAge) { + log.info(String.format("Setting %s", passwordAge)); + RealmRepresentation testRealmRepresentation = testRealmResource().toRepresentation(); + testRealmRepresentation.setPasswordPolicy(passwordAge); + testRealmResource().update(testRealmRepresentation); + } + + private void setPasswordHistory(String passwordHistory) { + log.info(String.format("Setting %s", passwordHistory)); + RealmRepresentation testRealmRepresentation = testRealmResource().toRepresentation(); + testRealmRepresentation.setPasswordPolicy(passwordHistory); + testRealmResource().update(testRealmRepresentation); + } + + private void setPasswordAgePolicyValue(String value) { + setPasswordAgePolicy(String.format("passwordAge(%s)", value)); + } + + private void setPasswordAgePolicyValue(int value) { + setPasswordAgePolicyValue(String.valueOf(value)); + } + + private void setPasswordHistoryValue(String value) { + setPasswordHistory(String.format("passwordHistory(%s)", value)); + } + + private void setPasswordHistoryValue(int value) { + setPasswordHistoryValue(String.valueOf(value)); + } + + public UserRepresentation createUserRepresentation(String username) { + UserRepresentation userRepresentation = new UserRepresentation(); + userRepresentation.setUsername(username); + userRepresentation.setEmail(String.format("%s@email.test", userRepresentation.getUsername())); + userRepresentation.setEmailVerified(true); + return userRepresentation; + } + + public UserResource createUser(UserRepresentation user) { + String createdUserId; + try (Response response = testRealmResource().users().create(user)) { + createdUserId = getCreatedId(response); + } + return testRealmResource().users().get(createdUserId); + } + + public void resetUserPassword(UserResource userResource, String newPassword) { + CredentialRepresentation newCredential = new CredentialRepresentation(); + newCredential.setType(PASSWORD); + newCredential.setValue(newPassword); + newCredential.setTemporary(false); + userResource.resetPassword(newCredential); + } + + private void expectBadRequestException(Consumer f) { + try { + f.accept(null); + throw new AssertionError("An expected BadRequestException was not thrown."); + } catch (BadRequestException bre) { + log.info("An expected BadRequestException was caught."); + } + } + + static private int daysToSeconds(int days) { + return days * 24 * 60 * 60; + } + + @Before + public void before() { + user = createUser(createUserRepresentation("test_user")); + } + + @After + public void after() { + user.remove(); + } + + @Test + public void testPasswordHistoryRetrySamePassword() { + setPasswordAgePolicyValue(1); + //set offset to 12h ago + Time.setOffset(-12 * 60 * 60); + resetUserPassword(user, "secret"); + //try to set again same password + Time.setOffset(0); + expectBadRequestException(f -> resetUserPassword(user, "secret")); + } + + @Test + public void testPasswordHistoryWithTwoPasswordsErrorThrown() { + setPasswordAgePolicyValue(1); + //set offset to 12h ago + Time.setOffset(-12 * 60 * 60); + resetUserPassword(user, "secret"); + Time.setOffset(-10 * 60 * 60); + resetUserPassword(user, "secret1"); + + //try to set again same password after 12h + Time.setOffset(0); + expectBadRequestException(f -> resetUserPassword(user, "secret")); + } + + @Test + public void testPasswordHistoryWithTwoPasswords() { + setPasswordAgePolicyValue(1); + //set offset to more than day ago + Time.setOffset(-26 * 60 * 60); + resetUserPassword(user, "secret"); + Time.setOffset(-10 * 60 * 60); + resetUserPassword(user, "secret1"); + + //try to set again same password after 26h + Time.setOffset(0); + resetUserPassword(user, "secret"); + } + + @Test + public void testPasswordHistoryWithMultiplePasswordsErrorThrown() { + setPasswordAgePolicyValue(30); + //set offset to 29 days, 23:55:00h + Time.setOffset(-30 * 24 * 60 * 60 + 60 * 5); + resetUserPassword(user, "secret"); + Time.setOffset(-25 * 24 * 60 * 60); + resetUserPassword(user, "secret1"); + Time.setOffset(-20 * 24 * 60 * 60); + resetUserPassword(user, "secret2"); + Time.setOffset(-10 * 24 * 60 * 60); + resetUserPassword(user, "secret3"); + + //try to set again same password after 30 days, should throw error, 5 minutes too early + Time.setOffset(0); + expectBadRequestException(f -> resetUserPassword(user, "secret")); + } + + @Test + public void testPasswordHistoryWithMultiplePasswords() { + setPasswordAgePolicyValue(30); + //set offset to 30 days, +00:05:00 h ago + Time.setOffset(-30 * 24 * 60 * 60 - 60 * 5); + resetUserPassword(user, "secret"); + Time.setOffset(-25 * 24 * 60 * 60); + resetUserPassword(user, "secret1"); + Time.setOffset(-20 * 24 * 60 * 60); + resetUserPassword(user, "secret2"); + Time.setOffset(-10 * 24 * 60 * 60); + resetUserPassword(user, "secret3"); + + //try to set again same password after 30 days and 5 minutes + Time.setOffset(0); + resetUserPassword(user, "secret"); + } + + + @Test + public void testPasswordAge0Days() { + setPasswordAgePolicyValue(0); + + resetUserPassword(user, "secret"); + //can't set the same password + expectBadRequestException(f -> resetUserPassword(user, "secret")); + resetUserPassword(user, "secret1"); + resetUserPassword(user, "secret"); + } + + @Test + public void testPasswordAgeSetToNegative() { + setPasswordAgePolicyValue(-1); + + resetUserPassword(user, "secret"); + //no check is performed + setPasswordAgePolicyValue(10); + resetUserPassword(user, "secret1"); + resetUserPassword(user, "secret2"); + resetUserPassword(user, "secret3"); + setPasswordAgePolicyValue(-2); + //no check is performed + resetUserPassword(user, "secret"); + resetUserPassword(user, "secret1"); + setPasswordAgePolicyValue(-3); + } + + @Test + public void testPasswordAgeSetToInvalid() { + expectBadRequestException(f -> setPasswordAgePolicyValue("abc")); + expectBadRequestException(f -> setPasswordAgePolicyValue("2a")); + expectBadRequestException(f -> setPasswordAgePolicyValue("asda2")); + expectBadRequestException(f -> setPasswordAgePolicyValue("-/!")); + } + + @Test + public void testBothPasswordHistoryPoliciesPasswordHistoryPolicyTakesOver() { + //1 day + setPasswordAgePolicyValue(1); + //last 3 passwords + setPasswordHistoryValue(3); + Time.setOffset(daysToSeconds(-2)); + resetUserPassword(user, "secret"); + resetUserPassword(user, "secret1"); + resetUserPassword(user, "secret2"); + + Time.setOffset(daysToSeconds(0)); + //password history takes precedence + expectBadRequestException(f -> setPasswordAgePolicyValue("secret")); + } + + @Test + public void testBothPasswordHistoryPoliciesPasswordAgePolicyTakesOver() { + //2 days + setPasswordAgePolicyValue(2); + //last 10 passwords + setPasswordHistoryValue(10); + Time.setOffset(daysToSeconds(-1)); + resetUserPassword(user, "secret"); + resetUserPassword(user, "secret1"); + resetUserPassword(user, "secret2"); + + Time.setOffset(daysToSeconds(0)); + //password age takes precedence + expectBadRequestException(f -> setPasswordAgePolicyValue("secret")); + } +}