From f69ff5d2700bd20c6dd62c5d4a5f5c34f4f0a3b6 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Wed, 31 May 2023 22:23:57 -0300 Subject: [PATCH] Execution config not duplicated when duplicating flows Closes #12012 --- .../AuthenticationManagementResource.java | 20 ++++++ .../AbstractAuthenticationTest.java | 2 +- .../admin/authentication/FlowTest.java | 72 +++++++++++++++++++ 3 files changed, 93 insertions(+), 1 deletion(-) diff --git a/services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java b/services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java index ec2f95698c..2c88603170 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java @@ -392,6 +392,26 @@ public class AuthenticationManagementResource { execution.setFlowId(copy.getId()); copy(realm, newName, subFlow, copy); } + + if (execution.getAuthenticatorConfig() != null) { + AuthenticatorConfigModel config = realm.getAuthenticatorConfigById(execution.getAuthenticatorConfig()); + + if (config == null) { + logger.debugf("Authentication execution with id [%s] not found", config.getId()); + throw new IllegalStateException("Authentication execution configuration not found"); + } + + config.setId(null); + + if (config.getAlias() != null) { + config.setAlias(newName + " " + config.getAlias()); + } + + AuthenticatorConfigModel newConfig = realm.addAuthenticatorConfig(config); + + execution.setAuthenticatorConfig(newConfig.getId()); + } + execution.setId(null); execution.setParentFlow(to.getId()); realm.addAuthenticatorExecution(execution); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/AbstractAuthenticationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/AbstractAuthenticationTest.java index f587ffa97e..15124b0066 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/AbstractAuthenticationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/AbstractAuthenticationTest.java @@ -110,7 +110,7 @@ public abstract class AbstractAuthenticationTest extends AbstractKeycloakTest { Assert.assertEquals("Execution authenticator - " + actual.getAuthenticator(), expected.getAuthenticator(), actual.getAuthenticator()); Assert.assertEquals("Execution userSetupAllowed - " + actual.getAuthenticator(), expected.isUserSetupAllowed(), actual.isUserSetupAllowed()); Assert.assertEquals("Execution authenticatorFlow - " + actual.getAuthenticator(), expected.isAuthenticatorFlow(), actual.isAuthenticatorFlow()); - Assert.assertEquals("Execution authenticatorConfig - " + actual.getAuthenticator(), expected.getAuthenticatorConfig(), actual.getAuthenticatorConfig()); + Assert.assertEquals("Execution authenticatorConfig - " + actual.getAuthenticatorConfig(), expected.getAuthenticatorConfig(), actual.getAuthenticatorConfig()); Assert.assertEquals("Execution priority - " + actual.getAuthenticator(), expected.getPriority(), actual.getPriority()); Assert.assertEquals("Execution requirement - " + actual.getAuthenticator(), expected.getRequirement(), actual.getRequirement()); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/FlowTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/FlowTest.java index e524b6ea7a..7cf3c367f8 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/FlowTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/FlowTest.java @@ -19,15 +19,19 @@ package org.keycloak.testsuite.admin.authentication; import org.junit.Assert; import org.junit.Test; +import org.keycloak.authentication.authenticators.browser.IdentityProviderAuthenticatorFactory; import org.keycloak.common.Profile; import org.keycloak.common.util.StreamUtil; import org.keycloak.events.admin.OperationType; import org.keycloak.events.admin.ResourceType; +import org.keycloak.models.utils.DefaultAuthenticationFlows; import org.keycloak.representations.idm.AuthenticationExecutionExportRepresentation; import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation; import org.keycloak.representations.idm.AuthenticationFlowRepresentation; +import org.keycloak.representations.idm.AuthenticatorConfigRepresentation; import org.keycloak.representations.idm.OAuth2ErrorRepresentation; import org.keycloak.testsuite.ProfileAssume; +import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.util.AdminEventPaths; import org.keycloak.testsuite.util.ContainerAssume; @@ -43,7 +47,10 @@ import java.nio.charset.Charset; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Set; import java.util.function.Predicate; +import java.util.stream.Collectors; import static org.hamcrest.Matchers.containsString; import static org.junit.Assert.assertEquals; @@ -504,4 +511,69 @@ public class FlowTest extends AbstractAuthenticationTest { error = e.getResponse().readEntity(OAuth2ErrorRepresentation.class); Assert.assertEquals("It is illegal to remove execution from a built in flow", error.getError()); } + + @Test + public void testExecutionConfigDuplicated() { + AuthenticationFlowRepresentation existingFlow = null; + + for (AuthenticationFlowRepresentation flow : authMgmtResource.getFlows()) { + if (flow.getAlias().equals(DefaultAuthenticationFlows.BROWSER_FLOW)) { + existingFlow = flow; + } + } + + Assert.assertNotNull(existingFlow); + + List executions = authMgmtResource.getExecutions(existingFlow.getAlias()); + AuthenticationExecutionInfoRepresentation executionWithConfig = null; + + for (AuthenticationExecutionInfoRepresentation execution : executions) { + if (IdentityProviderAuthenticatorFactory.PROVIDER_ID.equals(execution.getProviderId())) { + executionWithConfig = execution; + } + } + + Assert.assertNotNull(executionWithConfig); + + AuthenticatorConfigRepresentation executionConfig = new AuthenticatorConfigRepresentation(); + + executionConfig.setAlias("test-execution-config"); + executionConfig.setConfig(Map.of("key", "value")); + + try (Response response = authMgmtResource.newExecutionConfig(executionWithConfig.getId(), executionConfig)) { + getCleanup().addAuthenticationConfigId(ApiUtil.getCreatedId(response)); + assertAdminEvents.assertEvent(testRealmId, OperationType.CREATE, AdminEventPaths.authAddExecutionConfigPath(executionWithConfig.getId()), executionConfig, ResourceType.AUTH_EXECUTION); + } + + String newFlowName = "Duplicated of " + DefaultAuthenticationFlows.BROWSER_FLOW; + Map copyFlowParams = Map.of("newName", newFlowName); + authMgmtResource.copy(existingFlow.getAlias(), copyFlowParams).close(); + assertAdminEvents.assertEvent(testRealmId, OperationType.CREATE, AdminEventPaths.authCopyFlowPath("browser"), copyFlowParams, ResourceType.AUTH_FLOW); + + AuthenticationFlowRepresentation newFlow = null; + + for (AuthenticationFlowRepresentation flow : authMgmtResource.getFlows()) { + if (flow.getAlias().equals(newFlowName)) { + newFlow = flow; + } + } + + Set existingExecutionConfigIds = authMgmtResource.getExecutions(existingFlow.getAlias()) + .stream().map(AuthenticationExecutionInfoRepresentation::getAuthenticationConfig) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); + + assertFalse(existingExecutionConfigIds.isEmpty()); + + Set newExecutionConfigIds = authMgmtResource.getExecutions(newFlow.getAlias()) + .stream().map(AuthenticationExecutionInfoRepresentation::getAuthenticationConfig) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); + + assertFalse(newExecutionConfigIds.isEmpty()); + + for (String executionConfigId : newExecutionConfigIds) { + Assert.assertFalse("Execution config not duplicated", existingExecutionConfigIds.contains(executionConfigId)); + } + } }