Skip to content

Commit

Permalink
Add new password policy to validate passwords on login
Browse files Browse the repository at this point in the history
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 keycloak#14150

Signed-off-by: Tobias Kantusch <[email protected]>
  • Loading branch information
sirkrypt0 committed Jul 30, 2024
1 parent ae1aaef commit 640404e
Show file tree
Hide file tree
Showing 12 changed files with 207 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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() {

}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,4 @@ org.keycloak.policy.BlacklistPasswordPolicyProviderFactory
org.keycloak.policy.NotEmailPasswordPolicyProviderFactory
org.keycloak.policy.RecoveryCodesWarningThresholdPasswordPolicyProviderFactory
org.keycloak.policy.MaxAuthAgePasswordPolicyProviderFactory
org.keycloak.policy.ValidateOnLoginPasswordPolicyProviderFactory
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Object> policyConfig;
private Builder builder;

Expand Down Expand Up @@ -133,6 +137,10 @@ public int getMaxAuthAge() {
}
}

public boolean shouldValidateOnLogin() {
return policyConfig.containsKey(VALIDATE_ON_LOGIN_ID);
}

@Override
public String toString() {
return builder.asString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -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));
}
Expand All @@ -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
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -209,6 +210,29 @@ private boolean validateUser(AuthenticationFlowContext context, UserModel user,
}

public boolean validatePassword(AuthenticationFlowContext context, UserModel user, MultivaluedMap<String, String> 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);
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down
17 changes: 17 additions & 0 deletions themes/src/main/resources/theme/base/login/login-policy-error.ftl
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<#import "template.ftl" as layout>
<@layout.registrationLayout displayMessage=false; section>
<#if section = "header">
${msg("passwordPolicyErrorTitle")}
<#elseif section = "form">
<div id="kc-terms-text">
${kcSanitize(msg("passwordPolicyErrorMessage"))?no_esc}
</div>
<form class="form-actions" action="${url.loginAction}" method="POST">
<#if !userReadOnly>
<input class="${properties.kcButtonClass!} ${properties.kcButtonPrimaryClass!} ${properties.kcButtonLargeClass!}" name="continueToUpdate" id="kc-accept" type="submit" value="${msg("doContinue")}"/>
</#if>
<input class="${properties.kcButtonClass!} ${properties.kcButtonDefaultClass!} ${properties.kcButtonLargeClass!}" name="cancelUpdate" id="kc-decline" type="submit" value="${msg("doCancel")}"/>
</form>
<div class="clearfix"></div>
</#if>
</@layout.registrationLayout>
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 640404e

Please sign in to comment.