[KEYCLOAK-12254] Fix re-evaluation of conditional flow (#6558)

server-spi/src/main/java/org/keycloak/sessions/CommonClientSessionModel.java

@@ -53,6 +53,8 @@ public interface CommonClientSessionModel {
         SETUP_REQUIRED,
         ATTEMPTED,
         SKIPPED,
-        CHALLENGED
+        CHALLENGED,
+        EVALUATED_TRUE,
+        EVALUATED_FALSE
     }
 }

services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java

@@ -685,6 +685,18 @@ public class AuthenticationProcessor {
         return status == AuthenticationSessionModel.ExecutionStatus.SUCCESS;
     }
 
+    public boolean isEvaluatedTrue(AuthenticationExecutionModel model) {
+        AuthenticationSessionModel.ExecutionStatus status = authenticationSession.getExecutionStatus().get(model.getId());
+        if (status == null) return false;
+        return status == AuthenticationSessionModel.ExecutionStatus.EVALUATED_TRUE;
+    }
+
+    public boolean isEvaluatedFalse(AuthenticationExecutionModel model) {
+        AuthenticationSessionModel.ExecutionStatus status = authenticationSession.getExecutionStatus().get(model.getId());
+        if (status == null) return false;
+        return status == AuthenticationSessionModel.ExecutionStatus.EVALUATED_FALSE;
+    }
+
     public Response handleBrowserExceptionList(AuthenticationFlowException e) {
         LoginFormsProvider forms = session.getProvider(LoginFormsProvider.class).setAuthenticationSession(authenticationSession);
         ServicesLogger.LOGGER.failedAuthentication(e);

services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java

@@ -30,6 +30,7 @@ import org.keycloak.services.ServicesLogger;
 import org.keycloak.services.util.AuthenticationFlowHistoryHelper;
 import org.keycloak.services.util.AuthenticationFlowURLHelper;
 import org.keycloak.sessions.AuthenticationSessionModel;
+import org.keycloak.sessions.CommonClientSessionModel;
 
 import javax.ws.rs.core.MultivaluedHashMap;
 import javax.ws.rs.core.MultivaluedMap;
@@ -374,7 +375,20 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow {
         ConditionalAuthenticator authenticator = (ConditionalAuthenticator) createAuthenticator(factory);
         AuthenticationProcessor.Result context = processor.createAuthenticatorContext(model, authenticator, executionList);
 
-        return !authenticator.matchCondition(context);
+       boolean matchCondition;
+
+        // Retrieve previous evaluation result if any, else evaluate and store result for future re-evaluation
+        if (processor.isEvaluatedTrue(model)) {
+            matchCondition = true;
+        } else if (processor.isEvaluatedFalse(model)) {
+            matchCondition = false;
+        } else {
+            matchCondition = authenticator.matchCondition(context);
+            processor.getAuthenticationSession().setExecutionStatus(model.getId(),
+                    matchCondition ? AuthenticationSessionModel.ExecutionStatus.EVALUATED_TRUE : AuthenticationSessionModel.ExecutionStatus.EVALUATED_FALSE);
+        }
+
+        return !matchCondition;
     }
 
     private boolean isSetupRequired(AuthenticationExecutionModel model) {

testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/authentication/ConditionalUserAttributeValue.java

@@ -0,0 +1,65 @@
+package org.keycloak.testsuite.authentication;
+
+import org.keycloak.authentication.AuthenticationFlowContext;
+import org.keycloak.authentication.AuthenticationFlowError;
+import org.keycloak.authentication.AuthenticationFlowException;
+import org.keycloak.authentication.authenticators.conditional.ConditionalAuthenticator;
+import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.RealmModel;
+import org.keycloak.models.UserModel;
+
+import java.util.List;
+import java.util.Map;
+
+
+public class ConditionalUserAttributeValue implements ConditionalAuthenticator {
+
+    static final ConditionalUserAttributeValue SINGLETON = new ConditionalUserAttributeValue();
+
+    @Override
+    public boolean matchCondition(AuthenticationFlowContext context) {
+        boolean result = false;
+
+        // Retrieve configuration
+        Map<String, String> config = context.getAuthenticatorConfig().getConfig();
+        String attributeName = config.get(ConditionalUserAttributeValueFactory.CONF_ATTRIBUTE_NAME);
+        String attributeValue = config.get(ConditionalUserAttributeValueFactory.CONF_ATTRIBUTE_EXPECTED_VALUE);
+        boolean negateOutput = Boolean.parseBoolean(config.get(ConditionalUserAttributeValueFactory.CONF_NOT));
+
+        UserModel user = context.getUser();
+        if (user == null) {
+            throw new AuthenticationFlowException("authenticator: " + ConditionalUserAttributeValueFactory.PROVIDER_ID, AuthenticationFlowError.UNKNOWN_USER);
+        }
+
+        List<String> lstValues = user.getAttribute(attributeName);
+        if (lstValues != null) {
+            result = lstValues.contains(attributeValue);
+        }
+
+        if (negateOutput) {
+            result = !result;
+        }
+
+        return result;
+    }
+
+    @Override
+    public void action(AuthenticationFlowContext context) {
+        // Not used
+    }
+
+    @Override
+    public boolean requiresUser() {
+        return true;
+    }
+
+    @Override
+    public void setRequiredActions(KeycloakSession session, RealmModel realm, UserModel user) {
+        // Not used
+    }
+
+    @Override
+    public void close() {
+        // Does nothing
+    }
+}

testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/authentication/ConditionalUserAttributeValueFactory.java

@@ -0,0 +1,102 @@
+package org.keycloak.testsuite.authentication;
+
+import org.keycloak.Config;
+import org.keycloak.authentication.authenticators.conditional.ConditionalAuthenticator;
+import org.keycloak.authentication.authenticators.conditional.ConditionalAuthenticatorFactory;
+import org.keycloak.models.AuthenticationExecutionModel;
+import org.keycloak.models.KeycloakSessionFactory;
+import org.keycloak.provider.ProviderConfigProperty;
+
+import java.util.Arrays;
+import java.util.List;
+
+public class ConditionalUserAttributeValueFactory implements ConditionalAuthenticatorFactory {
+
+    public static final String PROVIDER_ID = "conditional-user-attribute";
+
+    public static final String CONF_ATTRIBUTE_NAME = "attribute_name";
+    public static final String CONF_ATTRIBUTE_EXPECTED_VALUE = "attribute_expected_value";
+    public static final String CONF_NOT = "not";
+
+    private static final AuthenticationExecutionModel.Requirement[] REQUIREMENT_CHOICES = {
+            AuthenticationExecutionModel.Requirement.REQUIRED, AuthenticationExecutionModel.Requirement.DISABLED
+    };
+
+    @Override
+    public void init(Config.Scope config) {
+        // no-op
+    }
+
+    @Override
+    public void postInit(KeycloakSessionFactory factory) {
+        // no-op
+    }
+
+    @Override
+    public void close() {
+        // no-op
+    }
+
+    @Override
+    public String getId() {
+        return PROVIDER_ID;
+    }
+
+    @Override
+    public String getDisplayType() {
+        return "Condition - user attribute";
+    }
+
+    @Override
+    public String getReferenceCategory() {
+        return "condition";
+    }
+
+    @Override
+    public boolean isConfigurable() {
+        return true;
+    }
+
+    @Override
+    public AuthenticationExecutionModel.Requirement[] getRequirementChoices() {
+        return REQUIREMENT_CHOICES;
+    }
+
+    @Override
+    public boolean isUserSetupAllowed() {
+        return false;
+    }
+
+    @Override
+    public String getHelpText() {
+        return "Flow is executed only if the user attribute exists and has the expected value";
+    }
+
+    @Override
+    public List<ProviderConfigProperty> getConfigProperties() {
+        ProviderConfigProperty authNoteName = new ProviderConfigProperty();
+        authNoteName.setType(ProviderConfigProperty.STRING_TYPE);
+        authNoteName.setName(CONF_ATTRIBUTE_NAME);
+        authNoteName.setLabel("Attribute name");
+        authNoteName.setHelpText("Name of the attribute to check");
+
+        ProviderConfigProperty authNoteExpectedValue = new ProviderConfigProperty();
+        authNoteExpectedValue.setType(ProviderConfigProperty.STRING_TYPE);
+        authNoteExpectedValue.setName(CONF_ATTRIBUTE_EXPECTED_VALUE);
+        authNoteExpectedValue.setLabel("Expected attribute value");
+        authNoteExpectedValue.setHelpText("Expected value in the attribute");
+
+        ProviderConfigProperty negateOutput = new ProviderConfigProperty();
+        negateOutput.setType(ProviderConfigProperty.BOOLEAN_TYPE);
+        negateOutput.setName(CONF_NOT);
+        negateOutput.setLabel("Negate output");
+        negateOutput.setHelpText("Apply a not to the check result");
+
+        return Arrays.asList(authNoteName, authNoteExpectedValue, negateOutput);
+    }
+
+    @Override
+    public ConditionalAuthenticator getSingleton() {
+        return ConditionalUserAttributeValue.SINGLETON;
+    }
+}

testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/authentication/SetUserAttributeAuthenticator.java

@@ -0,0 +1,61 @@
+package org.keycloak.testsuite.authentication;
+
+import org.keycloak.authentication.AuthenticationFlowContext;
+import org.keycloak.authentication.AuthenticationFlowError;
+import org.keycloak.authentication.Authenticator;
+import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.RealmModel;
+import org.keycloak.models.UserModel;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+public class SetUserAttributeAuthenticator implements Authenticator {
+    @Override
+    public void authenticate(AuthenticationFlowContext context) {
+        // Retrieve configuration
+        Map<String, String> config = context.getAuthenticatorConfig().getConfig();
+        String attrName = config.get(SetUserAttributeAuthenticatorFactory.CONF_ATTR_NAME);
+        String attrValue = config.get(SetUserAttributeAuthenticatorFactory.CONF_ATTR_VALUE);
+
+        UserModel user = context.getUser();
+        if (user.getAttribute(attrName) == null) {
+            user.setSingleAttribute(attrName, attrValue);
+        }
+        else {
+            List<String> attrValues = new ArrayList<>(user.getAttribute(attrName));
+            if (!attrValues.contains(attrValue)) {
+                attrValues.add(attrValue);
+            }
+            user.setAttribute(attrName, attrValues);
+        }
+
+        context.success();
+    }
+
+    @Override
+    public void action(AuthenticationFlowContext context) {
+        context.failure(AuthenticationFlowError.INTERNAL_ERROR);
+    }
+
+    @Override
+    public boolean requiresUser() {
+        return true;
+    }
+
+    @Override
+    public boolean configuredFor(KeycloakSession session, RealmModel realm, UserModel user) {
+        return true;
+    }
+
+    @Override
+    public void setRequiredActions(KeycloakSession session, RealmModel realm, UserModel user) {
+
+    }
+
+    @Override
+    public void close() {
+
+    }
+}

testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/authentication/SetUserAttributeAuthenticatorFactory.java

@@ -0,0 +1,105 @@
+package org.keycloak.testsuite.authentication;
+
+import org.keycloak.Config;
+import org.keycloak.OAuth2Constants;
+import org.keycloak.authentication.Authenticator;
+import org.keycloak.authentication.AuthenticatorFactory;
+import org.keycloak.authentication.DisplayTypeAuthenticatorFactory;
+import org.keycloak.authentication.authenticators.AttemptedAuthenticator;
+import org.keycloak.models.AuthenticationExecutionModel;
+import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.KeycloakSessionFactory;
+import org.keycloak.provider.ProviderConfigProperty;
+
+import java.util.Arrays;
+import java.util.List;
+
+
+public class SetUserAttributeAuthenticatorFactory implements AuthenticatorFactory, DisplayTypeAuthenticatorFactory {
+
+    public static final String PROVIDER_ID = "set-attribute";
+
+    public static final String CONF_ATTR_NAME = "attr_name";
+    public static final String CONF_ATTR_VALUE = "attr_value";
+    protected static final AuthenticationExecutionModel.Requirement[] REQUIREMENT_CHOICES = {
+            AuthenticationExecutionModel.Requirement.REQUIRED,
+            AuthenticationExecutionModel.Requirement.DISABLED};
+
+    @Override
+    public Authenticator createDisplay(KeycloakSession keycloakSession, String displayType) {
+        if (displayType == null) return create(keycloakSession);
+        if (!OAuth2Constants.DISPLAY_CONSOLE.equalsIgnoreCase(displayType)) return null;
+        return AttemptedAuthenticator.SINGLETON;
+    }
+
+    @Override
+    public String getReferenceCategory() {
+        return null;
+    }
+
+    @Override
+    public boolean isConfigurable() {
+        return true;
+    }
+
+    @Override
+    public boolean isUserSetupAllowed() {
+        return false;
+    }
+
+    @Override
+    public AuthenticationExecutionModel.Requirement[] getRequirementChoices() {
+        return REQUIREMENT_CHOICES;
+    }
+
+
+    @Override
+    public String getHelpText() {
+        return "Set a user attribute";
+    }
+
+    @Override
+    public void init(Config.Scope scope) {
+    }
+
+    @Override
+    public void postInit(KeycloakSessionFactory keycloakSessionFactory) {
+
+    }
+
+    @Override
+    public void close() {
+    }
+
+    @Override
+    public String getId() {
+        return PROVIDER_ID;
+    }
+
+    @Override
+    public Authenticator create(KeycloakSession keycloakSession) {
+        return new SetUserAttributeAuthenticator();
+    }
+
+    @Override
+    public String getDisplayType() {
+        return "Set user attribute";
+    }
+
+    @Override
+    public List<ProviderConfigProperty> getConfigProperties() {
+        ProviderConfigProperty attributeName = new ProviderConfigProperty();
+        attributeName.setType(ProviderConfigProperty.STRING_TYPE);
+        attributeName.setName(CONF_ATTR_NAME);
+        attributeName.setLabel("Attribute name");
+        attributeName.setHelpText("Name of the user attribute to set");
+
+        ProviderConfigProperty attributeValue = new ProviderConfigProperty();
+        attributeValue.setType(ProviderConfigProperty.STRING_TYPE);
+        attributeValue.setName(CONF_ATTR_VALUE);
+        attributeValue.setLabel("Attribute value");
+        attributeValue.setHelpText("Value to set in the user attribute");
+
+        return Arrays.asList(attributeName, attributeValue);
+    }
+}

testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/META-INF/services/org.keycloak.authentication.AuthenticatorFactory

@@ -20,4 +20,6 @@ org.keycloak.testsuite.forms.PassThroughRegistration
 org.keycloak.testsuite.forms.ClickThroughAuthenticator
 org.keycloak.testsuite.authentication.ExpectedParamAuthenticatorFactory
 org.keycloak.testsuite.authentication.PushButtonAuthenticatorFactory
-org.keycloak.testsuite.forms.UsernameOnlyAuthenticator
+org.keycloak.testsuite.forms.UsernameOnlyAuthenticator
+org.keycloak.testsuite.authentication.ConditionalUserAttributeValueFactory
+org.keycloak.testsuite.authentication.SetUserAttributeAuthenticatorFactory

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/ProvidersTest.java

@@ -205,6 +205,10 @@ public class ProvidersTest extends AbstractAuthenticationTest {
                 "Flow is executed only if user has the given role.");
         addProviderInfo(result, "conditional-user-configured", "Condition - user configured",
                 "Executes the current flow only if authenticators are configured");
+        addProviderInfo(result, "conditional-user-attribute", "Condition - user attribute",
+                "Flow is executed only if the user attribute exists and has the expected value");
+        addProviderInfo(result, "set-attribute", "Set user attribute",
+                "Set a user attribute");
 
         return result;
     }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BrowserFlowTest.java

@@ -41,6 +41,8 @@ import org.keycloak.testsuite.pages.LoginUsernameOnlyPage;
 import org.keycloak.testsuite.pages.PasswordPage;
 import org.keycloak.testsuite.util.FlowUtil;
 import org.keycloak.testsuite.util.OAuthClient;
+import org.keycloak.testsuite.authentication.ConditionalUserAttributeValueFactory;
+import org.keycloak.testsuite.authentication.SetUserAttributeAuthenticatorFactory;
 import org.keycloak.testsuite.util.URLUtils;
 import org.openqa.selenium.By;
 import org.openqa.selenium.WebDriver;
@@ -444,6 +446,63 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest {
         }
     }
 
+    private void configureBrowserFlowWithConditionalSubFlowWithChangingConditionWhileFlowEvaluation() {
+        final String newFlowAlias = "browser - changing condition";
+        testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session).copyBrowserFlow(newFlowAlias));
+        testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session)
+                .selectFlow(newFlowAlias)
+                .inForms(forms -> forms
+                        .clear()
+                        .addAuthenticatorExecution(Requirement.REQUIRED, UsernameFormFactory.PROVIDER_ID)
+                        .addSubFlowExecution(Requirement.CONDITIONAL, subFlow -> {
+                            // Add authenticators to this flow: 1 conditional authenticator and a basic authenticator executions
+                            subFlow.addAuthenticatorExecution(Requirement.REQUIRED, ConditionalUserAttributeValueFactory.PROVIDER_ID,
+                                    config -> {
+                                        config.getConfig().put(ConditionalUserAttributeValueFactory.CONF_ATTRIBUTE_NAME, "attribute");
+                                        config.getConfig().put(ConditionalUserAttributeValueFactory.CONF_ATTRIBUTE_EXPECTED_VALUE, "value");
+                                        config.getConfig().put(ConditionalUserAttributeValueFactory.CONF_NOT, Boolean.toString(true));
+                                    });
+
+                            // Set the attribute value
+                            subFlow.addAuthenticatorExecution(Requirement.REQUIRED, SetUserAttributeAuthenticatorFactory.PROVIDER_ID,
+                                    config -> {
+                                        config.getConfig().put(SetUserAttributeAuthenticatorFactory.CONF_ATTR_NAME, "attribute");
+                                        config.getConfig().put(SetUserAttributeAuthenticatorFactory.CONF_ATTR_VALUE, "value");
+                                    });
+
+
+                            // Requires Password
+                            subFlow.addAuthenticatorExecution(Requirement.REQUIRED, PasswordFormFactory.PROVIDER_ID);
+
+                            // Requires TOTP
+                            subFlow.addAuthenticatorExecution(Requirement.REQUIRED, OTPFormAuthenticatorFactory.PROVIDER_ID);
+                        }))
+                .defineAsBrowserFlow()
+        );
+    }
+
+    // Configure a conditional authenticator with a condition which change while the flow evaluation
+    // In such case, all the required authenticator inside the subflow should be evaluated even if the condition has changed
+    @Test
+    public void testConditionalAuthenticatorWithConditionalSubFlowWithChangingConditionWhileFlowEvaluation() {
+        try {
+            configureBrowserFlowWithConditionalSubFlowWithChangingConditionWhileFlowEvaluation();
+
+            // provides username
+            loginUsernameOnlyPage.open();
+            loginUsernameOnlyPage.login("user-with-two-configured-otp");
+
+            // The conditional sub flow is executed only if a specific user attribute is not set.
+            // This sub flow will set the user attribute and displays password form.
+            passwordPage.assertCurrent();
+            passwordPage.login("password");
+
+            Assert.assertTrue(oneTimeCodePage.isOtpLabelPresent());
+        } finally {
+            revertFlows("browser - changing condition");
+        }
+    }
+
     @Test
     public void testAlternativeNonInteractiveExecutorInSubflow() {
         final String newFlowAlias = "browser - alternative non-interactive executor";
@@ -965,7 +1024,7 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest {
             testRealm().flows().removeRequiredAction(WebAuthnRegisterFactory.PROVIDER_ID);
             UserRepresentation user = testRealm().users().search("test-user@localhost").get(0);
             user.setRequiredActions(Collections.emptyList());
-            testRealm().users().get(user.getId()).update(user);;
+            testRealm().users().get(user.getId()).update(user);
         }
     }