From 640404e5f597d3e6bfc95822b997caf126ae5dd9 Mon Sep 17 00:00:00 2001 From: Tobias Kantusch Date: Tue, 14 May 2024 16:04:12 +0200 Subject: [PATCH] Add new password policy to validate passwords on login Previously, Keycloak would only validate the password policy for new users and password changes. However, it may be desired to force all existing users to update their passwords when the password policy has changed. To accomplish this, this adds a new ValidateOnLogin password policy that can be configured per realm much like the existing password policies. When this policy is present, the password of the user will be validated against the current password policy on each login. This can be done for both, local users and users in the LDAP. When the LDAP is in read-only mode and the password no longer matches the policy, an error is shown, but the user is not given the option to update their password, as that doesn't work with read-only LDAP. Administrators with a read-only LDAP are free to disable the policy on login to avoid this. Currently, users are only shown a generic error message that their password no longer matches the policy, but not the exact error. This is because I didn't find a way to properly pass the PolicyError up to the authenticator which handles the password validation, as the policy errors contain parameters (like minimum lower case chars) and their error messages are localized based on the users locale. Closes #14150 Signed-off-by: Tobias Kantusch --- .../storage/ldap/LDAPStorageProvider.java | 24 ++++++++- ...ValidateOnLoginPasswordPolicyProvider.java | 26 +++++++++ ...eOnLoginPasswordPolicyProviderFactory.java | 54 +++++++++++++++++++ ...cloak.policy.PasswordPolicyProviderFactory | 1 + .../org/keycloak/models/PasswordPolicy.java | 8 +++ .../authentication/AuthenticatorUtil.java | 10 ++++ .../AbstractUsernameFormAuthenticator.java | 40 ++++++++++++++ .../directgrant/ValidatePassword.java | 12 +++++ .../PasswordCredentialProvider.java | 9 ++++ .../managers/AuthenticationManager.java | 3 ++ .../theme/base/login/login-policy-error.ftl | 17 ++++++ .../login/messages/messages_en.properties | 4 ++ 12 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 server-spi-private/src/main/java/org/keycloak/policy/ValidateOnLoginPasswordPolicyProvider.java create mode 100644 server-spi-private/src/main/java/org/keycloak/policy/ValidateOnLoginPasswordPolicyProviderFactory.java create mode 100755 themes/src/main/resources/theme/base/login/login-policy-error.ftl diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java index f975da54b78a..e169a5b92907 100755 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java @@ -55,6 +55,7 @@ import org.keycloak.models.LDAPConstants; import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.ModelException; +import org.keycloak.models.PasswordPolicy; import org.keycloak.models.RealmModel; import org.keycloak.models.RequiredActionProviderModel; import org.keycloak.models.RoleModel; @@ -68,6 +69,7 @@ import org.keycloak.policy.PasswordPolicyManagerProvider; import org.keycloak.policy.PolicyError; import org.keycloak.models.cache.UserCache; +import org.keycloak.services.managers.AuthenticationManager; import org.keycloak.storage.DatastoreProvider; import org.keycloak.storage.StoreManagers; import org.keycloak.storage.ReadOnlyException; @@ -886,7 +888,27 @@ public boolean isConfiguredFor(RealmModel realm, UserModel user, String credenti public boolean isValid(RealmModel realm, UserModel user, CredentialInput input) { if (!(input instanceof UserCredentialModel)) return false; if (input.getType().equals(PasswordCredentialModel.TYPE) && !((UserCredentialManager) user.credentialManager()).isConfiguredLocally(PasswordCredentialModel.TYPE)) { - return validPassword(realm, user, input.getChallengeResponse()); + if(getEditMode() == EditMode.READ_ONLY) { + session.getContext().getAuthenticationSession().setAuthNote(AuthenticationManager.USER_READ_ONLY, "true"); + } + + if(!validPassword(realm, user, input.getChallengeResponse())){ + return false; + } + + // After the password has been validated successfully, check that it still matches the realm's password policy. + // Only do that, if we can write the LDAP store, since only then the password can be updated in Keycloak. + // TODO: Do we want to support read-only LDAP? How to handle this then? Leave the decision to whoever evaluates the + // password policy result, so e.g. show a warning in the browser authenticators / deny authentication? + if (ldapIdentityStore.getConfig().isValidatePasswordPolicy() && realm.getPasswordPolicy().shouldValidateOnLogin()) { + PolicyError error = session.getProvider(PasswordPolicyManagerProvider.class).validate(realm, user, input.getChallengeResponse()); + if (error != null) { + logger.debug("User password no longer matches password policy"); + session.getContext().getAuthenticationSession().setAuthNote(PasswordPolicy.POLICY_ERROR_AUTH_NOTE, "true"); + return false; + } + } + return true; } else { return false; // invalid cred type } diff --git a/server-spi-private/src/main/java/org/keycloak/policy/ValidateOnLoginPasswordPolicyProvider.java b/server-spi-private/src/main/java/org/keycloak/policy/ValidateOnLoginPasswordPolicyProvider.java new file mode 100644 index 000000000000..de820819584d --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/policy/ValidateOnLoginPasswordPolicyProvider.java @@ -0,0 +1,26 @@ +package org.keycloak.policy; + +import org.keycloak.models.RealmModel; +import org.keycloak.models.UserModel; + +public class ValidateOnLoginPasswordPolicyProvider implements PasswordPolicyProvider { + @Override + public PolicyError validate(RealmModel realm, UserModel user, String password) { + return null; + } + + @Override + public PolicyError validate(String user, String password) { + return null; + } + + @Override + public Object parseConfig(String value) { + return null; + } + + @Override + public void close() { + + } +} diff --git a/server-spi-private/src/main/java/org/keycloak/policy/ValidateOnLoginPasswordPolicyProviderFactory.java b/server-spi-private/src/main/java/org/keycloak/policy/ValidateOnLoginPasswordPolicyProviderFactory.java new file mode 100644 index 000000000000..d031fb1e9557 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/policy/ValidateOnLoginPasswordPolicyProviderFactory.java @@ -0,0 +1,54 @@ +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 ValidateOnLoginPasswordPolicyProviderFactory implements PasswordPolicyProviderFactory { + + @Override + public String getDisplayName() { + return "Validate Policy on Login"; + } + + @Override + public String getConfigType() { + return null; + } + + @Override + public String getDefaultConfigValue() { + return null; + } + + @Override + public boolean isMultiplSupported() { + return false; + } + + @Override + public PasswordPolicyProvider create(KeycloakSession session) { + return new ValidateOnLoginPasswordPolicyProvider(); + } + + @Override + public void init(Config.Scope config) { + + } + + @Override + public void postInit(KeycloakSessionFactory factory) { + + } + + @Override + public void close() { + + } + + @Override + public String getId() { + return PasswordPolicy.VALIDATE_ON_LOGIN_ID; + } +} 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..a0617c7fa705 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.ValidateOnLoginPasswordPolicyProviderFactory 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..e83baeaba28f 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,10 @@ public class PasswordPolicy implements Serializable { public static final String MAX_AUTH_AGE_ID = "maxAuthAge"; + public static final String VALIDATE_ON_LOGIN_ID = "validateOnLogin"; + + public static final String POLICY_ERROR_AUTH_NOTE = "POLICY_ERROR"; + private Map policyConfig; private Builder builder; @@ -133,6 +137,10 @@ public int getMaxAuthAge() { } } + public boolean shouldValidateOnLogin() { + return policyConfig.containsKey(VALIDATE_ON_LOGIN_ID); + } + @Override public String toString() { return builder.asString(); diff --git a/services/src/main/java/org/keycloak/authentication/AuthenticatorUtil.java b/services/src/main/java/org/keycloak/authentication/AuthenticatorUtil.java index 7eba2e2ef1dc..58da274a1f05 100755 --- a/services/src/main/java/org/keycloak/authentication/AuthenticatorUtil.java +++ b/services/src/main/java/org/keycloak/authentication/AuthenticatorUtil.java @@ -32,6 +32,7 @@ import org.keycloak.models.AuthenticationFlowModel; import org.keycloak.models.Constants; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.PasswordPolicy; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.services.managers.AuthenticationManager; @@ -49,6 +50,7 @@ import static org.keycloak.services.managers.AuthenticationManager.FORCED_REAUTHENTICATION; import static org.keycloak.services.managers.AuthenticationManager.SSO_AUTH; +import static org.keycloak.services.managers.AuthenticationManager.USER_READ_ONLY; import static org.keycloak.services.managers.AuthenticationManager.PASSWORD_VALIDATED; public class AuthenticatorUtil { @@ -63,6 +65,10 @@ public static boolean isSSOAuthentication(AuthenticationSessionModel authSession return "true".equals(authSession.getAuthNote(SSO_AUTH)); } + public static boolean isUserReadOnlyAuthentication(AuthenticationSessionModel authSession) { + return "true".equals(authSession.getAuthNote(USER_READ_ONLY)); + } + public static boolean isForcedReauthentication(AuthenticationSessionModel authSession) { return "true".equals(authSession.getAuthNote(FORCED_REAUTHENTICATION)); } @@ -71,6 +77,10 @@ public static boolean isPasswordValidated(AuthenticationSessionModel authSession return "true".equals(authSession.getAuthNote(PASSWORD_VALIDATED)); } + public static boolean hasPasswordPolicyError(AuthenticationSessionModel authSession) { + return "true".equals(authSession.getAuthNote(PasswordPolicy.POLICY_ERROR_AUTH_NOTE)); + } + /** * Set authentication session note for callbacks defined for {@link AuthenticationFlowCallbackFactory) factories * diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java index feb7bc473865..ec1c45c6a6d4 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java @@ -21,6 +21,7 @@ import org.keycloak.authentication.AbstractFormAuthenticator; import org.keycloak.authentication.AuthenticationFlowContext; import org.keycloak.authentication.AuthenticationFlowError; +import org.keycloak.authentication.AuthenticatorUtil; import org.keycloak.credential.hash.PasswordHashProvider; import org.keycloak.events.Details; import org.keycloak.events.Errors; @@ -209,6 +210,29 @@ private boolean validateUser(AuthenticationFlowContext context, UserModel user, } public boolean validatePassword(AuthenticationFlowContext context, UserModel user, MultivaluedMap inputData, boolean clearUser) { + // If we have validated the password before, but it didn't match the current password policy, allow to continue to + // update the password or cancel. If we don't check whether the password was validated, users could skip the authenticator simply + // by providing continueUpdate in their form data. + if (AuthenticatorUtil.isPasswordValidated(context.getAuthenticationSession())){ + // Offer the option to update the password if user hit continue, and they are not read-only. + if(inputData.containsKey("continueToUpdate") && !AuthenticatorUtil.isUserReadOnlyAuthentication(context.getAuthenticationSession())) { + // Set a required action on the authentication session instead of on the user, as the password policy could be + // changed again such that the password would be compliant again. Setting an update action on the user could + // make it redundant, if their password is compliant again after a change in the policy. + context.getAuthenticationSession().addRequiredAction(UserModel.RequiredAction.UPDATE_PASSWORD); + return true; + } + + // Otherwise, go back to the login field. + Response challengeResponse = challenge(context, getDefaultChallengeMessage(context), FIELD_PASSWORD); + context.forceChallenge(challengeResponse); + + if (clearUser) { + context.clearUser(); + } + return false; + } + String password = inputData.getFirst(CredentialRepresentation.PASSWORD); if (password == null || password.isEmpty()) { return badPasswordHandler(context, user, clearUser,true); @@ -218,6 +242,22 @@ public boolean validatePassword(AuthenticationFlowContext context, UserModel use if (password != null && !password.isEmpty() && user.credentialManager().isValid(UserCredentialModel.password(password))) { context.getAuthenticationSession().setAuthNote(AuthenticationManager.PASSWORD_VALIDATED, "true"); + + // The credentialManager doesn't return policy errors for us. Instead, the individual credential providers that + // validate the password set this flag to true if there is an error. + if (AuthenticatorUtil.hasPasswordPolicyError(context.getAuthenticationSession())) { + // TODO: Pass read-only state to form. + + Response challenge = context. + form(). + setAttribute("userReadOnly", AuthenticatorUtil.isUserReadOnlyAuthentication(context.getAuthenticationSession())). + createForm("login-policy-error.ftl"); + + context.forceChallenge(challenge); + + return false; + } + return true; } else { return badPasswordHandler(context, user, clearUser,false); diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/directgrant/ValidatePassword.java b/services/src/main/java/org/keycloak/authentication/authenticators/directgrant/ValidatePassword.java index 9e1e1463c251..1c532db503d3 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/directgrant/ValidatePassword.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/directgrant/ValidatePassword.java @@ -19,6 +19,7 @@ import org.keycloak.authentication.AuthenticationFlowContext; import org.keycloak.authentication.AuthenticationFlowError; +import org.keycloak.authentication.AuthenticatorUtil; import org.keycloak.events.Errors; import org.keycloak.models.AuthenticationExecutionModel; import org.keycloak.models.KeycloakSession; @@ -54,6 +55,17 @@ public void authenticate(AuthenticationFlowContext context) { context.failure(AuthenticationFlowError.INVALID_USER, challengeResponse); return; } + + if (AuthenticatorUtil.hasPasswordPolicyError(context.getAuthenticationSession())) { + Response challengeResponse = errorResponse( + Response.Status.UNAUTHORIZED.getStatusCode(), + "invalid_grant", + "User password no longer matches the password policy and must be changed" + ); + context.failure(AuthenticationFlowError.INVALID_USER, challengeResponse); + return; + } + context.getAuthenticationSession().setAuthNote(AuthenticationManager.PASSWORD_VALIDATED, "true"); context.success(); } diff --git a/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java b/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java index 95277d88a762..aa42815b2e73 100644 --- a/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java +++ b/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java @@ -194,6 +194,15 @@ public boolean isValid(RealmModel realm, UserModel user, CredentialInput input) return false; } + if (realm.getPasswordPolicy().shouldValidateOnLogin()){ + // After the password has been validated successfully, check that it still matches the realm's password policy. + PolicyError error = session.getProvider(PasswordPolicyManagerProvider.class).validate(realm, user, input.getChallengeResponse()); + if (error != null) { + logger.debug("User password no longer matches password policy"); + session.getContext().getAuthenticationSession().setAuthNote(PasswordPolicy.POLICY_ERROR_AUTH_NOTE, "true"); + } + } + return true; } diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java index ee555c59882e..f7e69e92d6e7 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -145,6 +145,9 @@ public class AuthenticationManager { // clientSession note with flag that clientSession was authenticated through SSO cookie public static final String SSO_AUTH = "SSO_AUTH"; + // authSession note with flag that is true if user is authenticated through read-only user backend (e.g. read-only LDAP) + public static final String USER_READ_ONLY = "USER_READ_ONLY"; + // authSession note with flag that is true if user is forced to re-authenticate by client (EG. in case of OIDC client by sending "prompt=login") public static final String FORCED_REAUTHENTICATION = "FORCED_REAUTHENTICATION"; diff --git a/themes/src/main/resources/theme/base/login/login-policy-error.ftl b/themes/src/main/resources/theme/base/login/login-policy-error.ftl new file mode 100755 index 000000000000..92e32a4468f3 --- /dev/null +++ b/themes/src/main/resources/theme/base/login/login-policy-error.ftl @@ -0,0 +1,17 @@ +<#import "template.ftl" as layout> +<@layout.registrationLayout displayMessage=false; section> + <#if section = "header"> + ${msg("passwordPolicyErrorTitle")} + <#elseif section = "form"> +
+ ${kcSanitize(msg("passwordPolicyErrorMessage"))?no_esc} +
+
+ <#if !userReadOnly> + + + +
+
+ + diff --git a/themes/src/main/resources/theme/base/login/messages/messages_en.properties b/themes/src/main/resources/theme/base/login/messages/messages_en.properties index 37b69fc0a390..cbb80b4d0b83 100755 --- a/themes/src/main/resources/theme/base/login/messages/messages_en.properties +++ b/themes/src/main/resources/theme/base/login/messages/messages_en.properties @@ -307,6 +307,10 @@ delegationFailedMessage=You may close this browser window and go back to your co noAccessMessage=No access +passwordPolicyErrorTitle=New password policy +passwordPolicyErrorMessage=Your password no longer matches the password policy set by the administrator and therefore must be updated.\ + Click Continue to update your password or click Cancel to go back to the login screen. + invalidPasswordMinLengthMessage=Invalid password: minimum length {0}. invalidPasswordMaxLengthMessage=Invalid password: maximum length {0}. invalidPasswordMinDigitsMessage=Invalid password: must contain at least {0} numerical digits.