KEYCLOAK-12816 Fix representation to model conversion

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<String, String> entry : rep.getAttributes().entrySet()) {
+            for (Map.Entry<String, String> 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<String, String> removeEmptyString(Map<String, String> map) {
+    public static Map<String, String> removeEmptyString(Map<String, String> map) {
         if (map == null) {
             return null;
         }

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();
     }

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<T> 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<T> implements AutoCloseable {
         }
     }
 
+    public static class Flow extends Creator<AuthenticationManagementResource> {
+
+        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<String, String> 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);
+        }
+
+    }
 }

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 {
+        try (Creator.Flow amr = Creator.create(testRealm(), flow)) {
-            String executionId;
+            AuthenticationManagementResource authMgmtResource = amr.resource();
-            try (Response response = authMgmtResource.createFlow(flow)) {
-                Assert.assertThat("Create flow", response, statusCodeIs(Response.Status.CREATED));
-                AuthenticationFlowRepresentation newFlow = findFlowByAlias(flowAlias);
-                flowId = newFlow.getId();
-            }
 
-            //add execution - username-password form
+            //add execution - X509 username
-            Map<String, String> data = new HashMap<>();
+            final AuthenticationExecutionInfoRepresentation execution = amr.addExecution(ValidateX509CertificateUsernameFactory.PROVIDER_ID);
-            data.put("provider", ValidateX509CertificateUsernameFactory.PROVIDER_ID);
+            String executionId = execution.getId();
-            authMgmtResource.addExecution(flowAlias, data);
-
-            List<AuthenticationExecutionInfoRepresentation> executions = authMgmtResource.getExecutions(flowAlias);
-            assertThat(executions, hasSize(1));
-            final AuthenticationExecutionInfoRepresentation execution = executions.get(0);
-            executionId = execution.getId();
 
             Map<String, String> 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) {
+            authConfig = authMgmtResource.getAuthenticatorConfig(acId);
-                authMgmtResource.deleteFlow(flowId);
+            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);
         }
     }