UPDATED_PASSWORD required-action triggered only when login using password

`UpdatePassword.evaluateTriggers` adds the required-action to the user by evaluating the expiration password policy. Added a check that skips the evaluation if no password used during auth flow. This check uses the value of an auth note set in the `validatePassword` method of the `AbstractUsernameFormAuthenticator`.
Manually adding UPDATED_PASSWORD required-action to the user continues to trigger the action regardless of the authentication method.

Closes #17155

Signed-off-by: graziang <g.graziano94@gmail.com>
This commit is contained in:
graziang 2024-02-15 11:54:47 +01:00 committed by Marek Posolda
parent c94f9f5716
commit 1f57fc141c
6 changed files with 63 additions and 1 deletions

View file

@ -41,6 +41,7 @@ import java.util.stream.Collectors;
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.PASSWORD_VALIDATED;
public class AuthenticatorUtil {
@ -58,6 +59,10 @@ public class AuthenticatorUtil {
return "true".equals(authSession.getAuthNote(FORCED_REAUTHENTICATION));
}
public static boolean isPasswordValidated(AuthenticationSessionModel authSession) {
return "true".equals(authSession.getAuthNote(PASSWORD_VALIDATED));
}
/**
* Set authentication session note for callbacks defined for {@link AuthenticationFlowCallbackFactory) factories
*

View file

@ -228,6 +228,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
if (isDisabledByBruteForce(context, user)) return false;
if (password != null && !password.isEmpty() && user.credentialManager().isValid(UserCredentialModel.password(password))) {
context.getAuthenticationSession().setAuthNote(AuthenticationManager.PASSWORD_VALIDATED, "true");
return true;
} else {
return badPasswordHandler(context, user, clearUser,false);

View file

@ -30,6 +30,8 @@ import org.keycloak.representations.idm.CredentialRepresentation;
import jakarta.ws.rs.core.MultivaluedMap;
import jakarta.ws.rs.core.Response;
import org.keycloak.services.managers.AuthenticationManager;
import java.util.LinkedList;
import java.util.List;
@ -52,7 +54,7 @@ public class ValidatePassword extends AbstractDirectGrantAuthenticator {
context.failure(AuthenticationFlowError.INVALID_USER, challengeResponse);
return;
}
context.getAuthenticationSession().setAuthNote(AuthenticationManager.PASSWORD_VALIDATED, "true");
context.success();
}

View file

@ -71,6 +71,9 @@ public class UpdatePassword implements RequiredActionProvider, RequiredActionFac
@Override
public void evaluateTriggers(RequiredActionContext context) {
if(!AuthenticatorUtil.isPasswordValidated(context.getAuthenticationSession())) {
return;
}
int daysToExpirePassword = context.getRealm().getPasswordPolicy().getDaysToExpirePassword();
if(daysToExpirePassword != -1) {
PasswordCredentialProvider passwordProvider = (PasswordCredentialProvider)context.getSession().getProvider(CredentialProvider.class, PasswordCredentialProviderFactory.PROVIDER_ID);

View file

@ -148,6 +148,9 @@ public class AuthenticationManager {
// 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";
// authSession note with flag that is true if the user's password has been correctly validated
public static final String PASSWORD_VALIDATED = "PASSWORD_VALIDATED";
protected static final Logger logger = Logger.getLogger(AuthenticationManager.class);
public static final String FORM_USERNAME = "username";

View file

@ -23,8 +23,11 @@ import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.keycloak.admin.client.resource.UserResource;
import org.keycloak.authentication.authenticators.browser.UsernameFormFactory;
import org.keycloak.events.EventType;
import org.keycloak.models.AuthenticationExecutionModel;
import org.keycloak.models.UserModel.RequiredAction;
import org.keycloak.models.utils.DefaultAuthenticationFlows;
import org.keycloak.representations.idm.EventRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
@ -35,9 +38,12 @@ import org.keycloak.testsuite.pages.AppPage;
import org.keycloak.testsuite.pages.AppPage.RequestType;
import org.keycloak.testsuite.pages.LoginPage;
import org.keycloak.testsuite.pages.LoginPasswordUpdatePage;
import org.keycloak.testsuite.pages.LoginUsernameOnlyPage;
import org.keycloak.testsuite.util.GreenMailRule;
import org.keycloak.testsuite.util.OAuthClient;
import org.keycloak.testsuite.util.SecondBrowser;
import org.keycloak.testsuite.util.FlowUtil;
import org.keycloak.testsuite.util.RealmManager;
import org.openqa.selenium.WebDriver;
import java.util.LinkedList;
@ -72,6 +78,9 @@ public class RequiredActionResetPasswordTest extends AbstractTestRealmKeycloakTe
@Page
protected LoginPage loginPage;
@Page
protected LoginUsernameOnlyPage loginUsernameOnlyPage;
@Page
protected LoginPasswordUpdatePage changePasswordPage;
@ -134,6 +143,45 @@ public class RequiredActionResetPasswordTest extends AbstractTestRealmKeycloakTe
assertEquals("All sessions are still active", 2, testUser.getUserSessions().size());
}
@Test
public void resetPasswordActionNotTriggered() {
String newFlowAlias = "browser - username only";
try {
RealmManager.realm(testRealm()).passwordPolicy("forceExpiredPasswordChange(1)");
setTimeOffset(60 * 60 * 48);
//create username only flow
testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session).copyBrowserFlow(newFlowAlias));
testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session)
.selectFlow(newFlowAlias)
.clear()
.addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, UsernameFormFactory.PROVIDER_ID)
.defineAsBrowserFlow() // Activate this new flow
);
loginUsernameOnlyPage.open();
loginUsernameOnlyPage.login("test-user@localhost");
events.expectLogin().assertEvent();
}
finally {
//reset browser flow and delete username only flow
RealmRepresentation realm = testRealm().toRepresentation();
realm.setBrowserFlow(DefaultAuthenticationFlows.BROWSER_FLOW);
testRealm().update(realm);
testRealm().flows()
.getFlows()
.stream()
.filter(flowRep -> flowRep.getAlias().equals(newFlowAlias))
.findFirst()
.ifPresent(authenticationFlowRepresentation ->
testRealm().flows().deleteFlow(authenticationFlowRepresentation.getId()));
setTimeOffset(0);
RealmManager.realm(testRealm()).passwordPolicy(null);
}
}
private void requireUpdatePassword() {
UserRepresentation userRep = findUser("test-user@localhost");
if (userRep.getRequiredActions() == null) {