From 129c6898556c2f947a44e0e62cf25d6184e5e550 Mon Sep 17 00:00:00 2001 From: harture <31417989+harture@users.noreply.github.com> Date: Thu, 28 Nov 2019 09:30:31 +0100 Subject: [PATCH] [KEYCLOAK-12253] Fix conditional authenticators are evaluated even if they are disabled (#6553) --- .../DefaultAuthenticationFlow.java | 1 + .../testsuite/forms/BrowserFlowTest.java | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java b/services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java index 52a4274ab5..d34347ff9a 100755 --- a/services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java +++ b/services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java @@ -352,6 +352,7 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow { List modelList = processor.getRealm().getAuthenticationExecutions(model.getFlowId()); List conditionalAuthenticatorList = modelList.stream() .filter(this::isConditionalAuthenticator) + .filter(s -> s.isEnabled()) .collect(Collectors.toList()); return conditionalAuthenticatorList.isEmpty() || conditionalAuthenticatorList.stream().anyMatch(m-> conditionalNotMatched(m, modelList)); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BrowserFlowTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BrowserFlowTest.java index 274e567d5a..22a7872fd1 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BrowserFlowTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BrowserFlowTest.java @@ -363,6 +363,37 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest { ); } + // A conditional flow with disabled conditional authenticator should automatically be disabled + @Test + public void testFlowDisabledWhenConditionalAuthenticatorIsDisabled() { + try { + configureBrowserFlowWithConditionalSubFlowHavingDisabledConditionalAuthenticator("browser - disabled conditional authenticator"); + // Flow is conditional but it is missing a conditional authentication executor + // The whole flow is disabled + Assert.assertFalse(needsPassword("user-with-two-configured-otp")); + } finally { + revertFlows("browser - disabled conditional authenticator"); + } + } + + private void configureBrowserFlowWithConditionalSubFlowHavingDisabledConditionalAuthenticator(String newFlowAlias) { + 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.DISABLED, ConditionalUserConfiguredAuthenticatorFactory.PROVIDER_ID); + + // Update the browser forms only with a UsernameForm + subFlow.addAuthenticatorExecution(Requirement.REQUIRED, PasswordFormFactory.PROVIDER_ID); + })) + .defineAsBrowserFlow() + ); + } + // Configure a conditional authenticator in a non-conditional sub-flow // In such case, the flow is evaluated and the conditional authenticator is considered as disabled @Test