From aecfe251e49d511b66a951171ad66ff505f1805e Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Tue, 25 Feb 2020 14:46:03 +0100 Subject: [PATCH] KEYCLOAK-12816 Fix representation to model conversion --- .../models/utils/RepresentationToModel.java | 10 ++--- .../AuthenticationManagementResource.java | 2 +- .../keycloak/testsuite/updaters/Creator.java | 35 ++++++++++++++++ .../testsuite/forms/CustomFlowTest.java | 42 ++++++++----------- 4 files changed, 57 insertions(+), 32 deletions(-) diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java index 58a4cc9eb0..7b455e7da2 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java @@ -133,8 +133,6 @@ import org.keycloak.storage.UserStorageProvider; import org.keycloak.storage.UserStorageProviderModel; import org.keycloak.storage.federated.UserFederatedStorageProvider; import org.keycloak.util.JsonSerialization; -import org.keycloak.validation.ClientValidationContext; -import org.keycloak.validation.ClientValidationProvider; import org.keycloak.validation.ClientValidationUtil; public class RepresentationToModel { @@ -1482,7 +1480,7 @@ public class RepresentationToModel { } } if (rep.getAttributes() != null) { - for (Map.Entry entry : rep.getAttributes().entrySet()) { + for (Map.Entry entry : removeEmptyString(rep.getAttributes()).entrySet()) { resource.setAttribute(entry.getKey(), entry.getValue()); } } @@ -1887,7 +1885,7 @@ public class RepresentationToModel { identityProviderModel.setAuthenticateByDefault(representation.isAuthenticateByDefault()); identityProviderModel.setStoreToken(representation.isStoreToken()); identityProviderModel.setAddReadTokenRoleOnCreate(representation.isAddReadTokenRoleOnCreate()); - identityProviderModel.setConfig(new HashMap<>(representation.getConfig())); + identityProviderModel.setConfig(removeEmptyString(representation.getConfig())); String flowAlias = representation.getFirstBrokerLoginFlowAlias(); if (flowAlias == null) { @@ -2032,7 +2030,7 @@ public class RepresentationToModel { public static RequiredActionProviderModel toModel(RequiredActionProviderRepresentation rep) { RequiredActionProviderModel model = new RequiredActionProviderModel(); - model.setConfig(rep.getConfig()); + model.setConfig(removeEmptyString(rep.getConfig())); model.setPriority(rep.getPriority()); model.setDefaultAction(rep.isDefaultAction()); model.setEnabled(rep.isEnabled()); @@ -2723,7 +2721,7 @@ public class RepresentationToModel { } } - private static Map removeEmptyString(Map map) { + public static Map removeEmptyString(Map map) { if (map == null) { return null; } 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 6ba347954f..464a1e622a 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 @@ -1208,7 +1208,7 @@ public class AuthenticationManagementResource { } exists.setAlias(rep.getAlias()); - exists.setConfig(rep.getConfig()); + exists.setConfig(RepresentationToModel.removeEmptyString(rep.getConfig())); realm.updateAuthenticatorConfig(exists); adminEvent.operation(OperationType.UPDATE).resource(ResourceType.AUTHENTICATOR_CONFIG).resourcePath(session.getContext().getUri()).representation(rep).success(); } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/Creator.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/Creator.java index 7358de9b07..ed672a4c54 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/Creator.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/Creator.java @@ -17,6 +17,7 @@ package org.keycloak.testsuite.updaters; import org.keycloak.admin.client.Keycloak; +import org.keycloak.admin.client.resource.AuthenticationManagementResource; import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.ClientsResource; import org.keycloak.admin.client.resource.ComponentResource; @@ -26,11 +27,16 @@ import org.keycloak.admin.client.resource.GroupsResource; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.admin.client.resource.UserResource; import org.keycloak.admin.client.resource.UsersResource; +import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation; +import org.keycloak.representations.idm.AuthenticationFlowRepresentation; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.ComponentRepresentation; import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; import javax.ws.rs.core.Response; import org.jboss.logging.Logger; @@ -90,6 +96,15 @@ public class Creator implements AutoCloseable { } } + public static Creator.Flow create(RealmResource realmResource, AuthenticationFlowRepresentation rep) { + final AuthenticationManagementResource authMgmgRes = realmResource.flows(); + try (Response response = authMgmgRes.createFlow(rep)) { + String createdId = getCreatedId(response); + LOG.debugf("Created flow ID %s", createdId); + return new Flow(createdId, rep.getAlias(), authMgmgRes, () -> authMgmgRes.deleteFlow(createdId)); + } + } + private final String id; private final T resource; private final Runnable closer; @@ -123,4 +138,24 @@ public class Creator implements AutoCloseable { } } + public static class Flow extends Creator { + + private final String alias; + + public Flow(String id, String alias, AuthenticationManagementResource resource, Runnable closer) { + super(id, resource, closer); + this.alias = alias; + } + + public AuthenticationExecutionInfoRepresentation addExecution(String providerId) { + Map c = new HashMap<>(); + c.put("provider", providerId); + resource().addExecution(alias, c); // addExecution only handles "provider" in data + return resource().getExecutions(alias).stream() + .filter(aer -> Objects.equals(providerId, aer.getProviderId())) + .findFirst() + .orElse(null); + } + + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/CustomFlowTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/CustomFlowTest.java index 98c30700a5..fc78fd3fb1 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/CustomFlowTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/CustomFlowTest.java @@ -48,6 +48,7 @@ import org.keycloak.testsuite.pages.LoginPasswordUpdatePage; import org.keycloak.testsuite.pages.RegisterPage; import org.keycloak.testsuite.pages.TermsAndConditionsPage; import org.keycloak.testsuite.rest.representation.AuthenticatorState; +import org.keycloak.testsuite.updaters.Creator; import org.keycloak.testsuite.util.ClientBuilder; import org.keycloak.testsuite.util.ExecutionBuilder; import org.keycloak.testsuite.util.FlowBuilder; @@ -58,12 +59,9 @@ import org.keycloak.testsuite.util.UserBuilder; import javax.ws.rs.core.Response; import java.util.HashMap; -import java.util.List; import java.util.Map; import javax.ws.rs.core.Response.Status; -import org.hamcrest.Matchers; -import static org.hamcrest.Matchers.hasSize; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; import static org.keycloak.testsuite.util.Matchers.statusCodeIs; @@ -259,8 +257,6 @@ public class CustomFlowTest extends AbstractFlowTest { @Test public void validateX509FlowUpdate() throws Exception { - String flowId = null; - AuthenticationManagementResource authMgmtResource = testRealm().flows(); String flowAlias = "Browser Flow With Extra 2"; AuthenticationFlowRepresentation flow = new AuthenticationFlowRepresentation(); @@ -270,23 +266,12 @@ public class CustomFlowTest extends AbstractFlowTest { flow.setTopLevel(true); flow.setBuiltIn(false); - try { - String executionId; - try (Response response = authMgmtResource.createFlow(flow)) { - Assert.assertThat("Create flow", response, statusCodeIs(Response.Status.CREATED)); - AuthenticationFlowRepresentation newFlow = findFlowByAlias(flowAlias); - flowId = newFlow.getId(); - } + try (Creator.Flow amr = Creator.create(testRealm(), flow)) { + AuthenticationManagementResource authMgmtResource = amr.resource(); - //add execution - username-password form - Map data = new HashMap<>(); - data.put("provider", ValidateX509CertificateUsernameFactory.PROVIDER_ID); - authMgmtResource.addExecution(flowAlias, data); - - List executions = authMgmtResource.getExecutions(flowAlias); - assertThat(executions, hasSize(1)); - final AuthenticationExecutionInfoRepresentation execution = executions.get(0); - executionId = execution.getId(); + //add execution - X509 username + final AuthenticationExecutionInfoRepresentation execution = amr.addExecution(ValidateX509CertificateUsernameFactory.PROVIDER_ID); + String executionId = execution.getId(); Map config = new HashMap<>(); config.put(AbstractX509ClientCertificateAuthenticator.ENABLE_CRL, Boolean.TRUE.toString()); @@ -294,13 +279,20 @@ public class CustomFlowTest extends AbstractFlowTest { authConfig.setAlias("Config alias"); authConfig.setConfig(config); + String acId; try (Response resp = authMgmtResource.newExecutionConfig(executionId, authConfig)) { assertThat(resp, statusCodeIs(Status.CREATED)); + acId = ApiUtil.getCreatedId(resp); } - } finally { - if (flowId != null) { - authMgmtResource.deleteFlow(flowId); - } + + authConfig = authMgmtResource.getAuthenticatorConfig(acId); + authConfig.getConfig().put(AbstractX509ClientCertificateAuthenticator.ENABLE_CRL, Boolean.FALSE.toString()); + authConfig.getConfig().put(AbstractX509ClientCertificateAuthenticator.CRL_RELATIVE_PATH, ""); + + authMgmtResource.updateAuthenticatorConfig(acId, authConfig); + + // Saving the same options for the second time would fail for CRL_RELATIVE_PATH on Oracle due to "" == NULL weirdness + authMgmtResource.updateAuthenticatorConfig(acId, authConfig); } }