keycloak-aplcache

Merge pull request #4242 from mposolda/master KEYCLOAK-4438

6/21/2017 6:54:50 AM

Details

diff --git a/common/src/main/java/org/keycloak/common/util/ConcurrentMultivaluedHashMap.java b/common/src/main/java/org/keycloak/common/util/ConcurrentMultivaluedHashMap.java
index c092c6f..258f076 100755
--- a/common/src/main/java/org/keycloak/common/util/ConcurrentMultivaluedHashMap.java
+++ b/common/src/main/java/org/keycloak/common/util/ConcurrentMultivaluedHashMap.java
@@ -31,9 +31,9 @@ public class ConcurrentMultivaluedHashMap<K, V> extends ConcurrentHashMap<K, Lis
 {
    public void putSingle(K key, V value)
    {
-      List<V> list = new CopyOnWriteArrayList<>();
+      List<V> list = createListInstance();
       list.add(value);
-      put(key, list);
+      put(key, list); // Just override with new List instance
    }
 
    public void addAll(K key, V... newValues)
@@ -84,8 +84,15 @@ public class ConcurrentMultivaluedHashMap<K, V> extends ConcurrentHashMap<K, Lis
    public final List<V> getList(K key)
    {
       List<V> list = get(key);
-      if (list == null)
-         put(key, list = new CopyOnWriteArrayList<V>());
+
+      if (list == null) {
+         list = createListInstance();
+         List<V> existing = putIfAbsent(key, list);
+         if (existing != null) {
+            list = existing;
+         }
+      }
+
       return list;
    }
 
@@ -97,4 +104,8 @@ public class ConcurrentMultivaluedHashMap<K, V> extends ConcurrentHashMap<K, Lis
       }
    }
 
+   protected List<V> createListInstance() {
+      return new CopyOnWriteArrayList<>();
+   }
+
 }
diff --git a/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProviderFactory.java b/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProviderFactory.java
index e061f5e..75e9d18 100755
--- a/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProviderFactory.java
+++ b/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProviderFactory.java
@@ -156,4 +156,9 @@ public class KerberosFederationProviderFactory implements UserStorageProviderFac
                 AuthenticationExecutionModel.Requirement.ALTERNATIVE, AuthenticationExecutionModel.Requirement.DISABLED);
     }
 
+    @Override
+    public void preRemove(KeycloakSession session, RealmModel realm, ComponentModel model) {
+        CredentialHelper.setOrReplaceAuthenticationRequirement(session, realm, CredentialRepresentation.KERBEROS,
+                AuthenticationExecutionModel.Requirement.DISABLED, null);
+    }
 }
diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java
index c962af8..0d4c07b 100755
--- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java
+++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java
@@ -384,8 +384,14 @@ public class LDAPStorageProviderFactory implements UserStorageProviderFactory<LD
 
     }
 
-
-
+    @Override
+    public void preRemove(KeycloakSession session, RealmModel realm, ComponentModel model) {
+        String allowKerberosCfg = model.getConfig().getFirst(KerberosConstants.ALLOW_KERBEROS_AUTHENTICATION);
+        if (Boolean.valueOf(allowKerberosCfg)) {
+            CredentialHelper.setOrReplaceAuthenticationRequirement(session, realm, CredentialRepresentation.KERBEROS,
+                    AuthenticationExecutionModel.Requirement.DISABLED, null);
+        }
+    }
 
     @Override
     public SynchronizationResult sync(KeycloakSessionFactory sessionFactory, String realmId, UserStorageProviderModel model) {
diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java
index 33ca943..eba62db 100755
--- a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java
+++ b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java
@@ -1885,6 +1885,7 @@ public class RealmAdapter implements RealmModel, JpaModel<RealmEntity> {
         ComponentEntity c = em.find(ComponentEntity.class, component.getId());
         if (c == null) return;
         session.users().preRemove(this, component);
+        ComponentUtil.notifyPreRemove(session, this, component);
         removeComponents(component.getId());
         getEntity().getComponents().remove(c);
     }
@@ -1896,7 +1897,10 @@ public class RealmAdapter implements RealmModel, JpaModel<RealmEntity> {
         getEntity().getComponents().stream()
                 .filter(sameParent)
                 .map(this::entityToModel)
-                .forEach(c -> session.users().preRemove(this, c));
+                .forEach((ComponentModel c) -> {
+                    session.users().preRemove(this, c);
+                    ComponentUtil.notifyPreRemove(session, this, c);
+                });
 
         getEntity().getComponents().removeIf(sameParent);
     }
diff --git a/server-spi/src/main/java/org/keycloak/component/ComponentFactory.java b/server-spi/src/main/java/org/keycloak/component/ComponentFactory.java
index 695637f..b4c6f44 100644
--- a/server-spi/src/main/java/org/keycloak/component/ComponentFactory.java
+++ b/server-spi/src/main/java/org/keycloak/component/ComponentFactory.java
@@ -80,6 +80,18 @@ public interface ComponentFactory<CreatedType, ProviderType extends Provider> ex
     }
 
     /**
+     * Called before the component is removed.
+     *
+     * @param session
+     * @param realm
+     * @param model model of the component, which is going to be removed
+     */
+    default
+    void preRemove(KeycloakSession session, RealmModel realm, ComponentModel model) {
+
+    }
+
+    /**
      * These are config properties that are common across all implementation of this component type
      *
      * @return
diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ComponentUtil.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ComponentUtil.java
index 1d41f0d..0c603c4 100644
--- a/server-spi-private/src/main/java/org/keycloak/models/utils/ComponentUtil.java
+++ b/server-spi-private/src/main/java/org/keycloak/models/utils/ComponentUtil.java
@@ -17,6 +17,7 @@
 
 package org.keycloak.models.utils;
 
+import org.jboss.logging.Logger;
 import org.keycloak.component.ComponentFactory;
 import org.keycloak.component.ComponentModel;
 import org.keycloak.models.KeycloakSession;
@@ -38,6 +39,8 @@ import java.util.Map;
  */
 public class ComponentUtil {
 
+    private static final Logger logger = Logger.getLogger(ComponentUtil.class);
+
     public static Map<String, ProviderConfigProperty> getComponentConfigProperties(KeycloakSession session, ComponentRepresentation component) {
         return getComponentConfigProperties(session, component.getProviderType(), component.getProviderId());
     }
@@ -102,5 +105,14 @@ public class ComponentUtil {
             ((OnUpdateComponent)session.userStorageManager()).onUpdate(session, realm, oldModel, newModel);
         }
     }
+    public static void notifyPreRemove(KeycloakSession session, RealmModel realm, ComponentModel model) {
+        try {
+            ComponentFactory factory = getComponentFactory(session, model);
+            factory.preRemove(session, realm, model);
+        } catch (IllegalArgumentException iae) {
+            // We allow to remove broken providers without throwing an exception
+            logger.warn(iae.getMessage());
+        }
+    }
 
 }
diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/TestContext.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/TestContext.java
index 30b0405..cfe6d84 100644
--- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/TestContext.java
+++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/TestContext.java
@@ -20,6 +20,7 @@ import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 import javax.ws.rs.NotFoundException;
 
@@ -54,7 +55,7 @@ public final class TestContext {
     private boolean initialized;
 
     // Key is realmName, value are objects to clean after the test method
-    private Map<String, TestCleanup> cleanups = new HashMap<>();
+    private Map<String, TestCleanup> cleanups = new ConcurrentHashMap<>();
 
     public TestContext(SuiteContext suiteContext, Class testClass) {
         this.suiteContext = suiteContext;
@@ -146,7 +147,11 @@ public final class TestContext {
         TestCleanup cleanup = cleanups.get(realmName);
         if (cleanup == null) {
             cleanup = new TestCleanup(adminClient, realmName);
-            cleanups.put(realmName, cleanup);
+            TestCleanup existing = cleanups.putIfAbsent(realmName, cleanup);
+
+            if (existing != null) {
+                cleanup = existing;
+            }
         }
         return cleanup;
     }
diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/TestCleanup.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/TestCleanup.java
index 17ff44a..192712e 100644
--- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/TestCleanup.java
+++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/TestCleanup.java
@@ -17,13 +17,13 @@
 
 package org.keycloak.testsuite.util;
 
-import java.util.LinkedList;
 import java.util.List;
 
 import javax.ws.rs.NotFoundException;
 
 import org.keycloak.admin.client.Keycloak;
 import org.keycloak.admin.client.resource.RealmResource;
+import org.keycloak.common.util.ConcurrentMultivaluedHashMap;
 
 /**
  * Enlist resources to be cleaned after test method
@@ -32,18 +32,21 @@ import org.keycloak.admin.client.resource.RealmResource;
  */
 public class TestCleanup {
 
+    private static final String IDENTITY_PROVIDER_ALIASES = "IDENTITY_PROVIDER_ALIASES";
+    private static final String USER_IDS = "USER_IDS";
+    private static final String COMPONENT_IDS = "COMPONENT_IDS";
+    private static final String CLIENT_UUIDS = "CLIENT_UUIDS";
+    private static final String ROLE_IDS = "ROLE_IDS";
+    private static final String GROUP_IDS = "GROUP_IDS";
+    private static final String AUTH_FLOW_IDS = "AUTH_FLOW_IDS";
+    private static final String AUTH_CONFIG_IDS = "AUTH_CONFIG_IDS";
+
     private final Keycloak adminClient;
     private final String realmName;
 
+    // Key is kind of entity (eg. "client", "role", "user" etc), Values are all kind of entities of given type to cleanup
+    private ConcurrentMultivaluedHashMap<String, String> entities = new ConcurrentMultivaluedHashMap<>();
 
-    private List<String> identityProviderAliases;
-    private List<String> userIds;
-    private List<String> componentIds;
-    private List<String> clientUuids;
-    private List<String> roleIds;
-    private List<String> groupIds;
-    private List<String> authFlowIds;
-    private List<String> authConfigIds;
 
     public TestCleanup(Keycloak adminClient, String realmName) {
         this.adminClient = adminClient;
@@ -52,72 +55,49 @@ public class TestCleanup {
 
 
     public void addUserId(String userId) {
-        if (userIds == null) {
-            userIds = new LinkedList<>();
-        }
-        userIds.add(userId);
+        entities.add(USER_IDS, userId);
     }
 
 
     public void addIdentityProviderAlias(String identityProviderAlias) {
-        if (identityProviderAliases == null) {
-            identityProviderAliases = new LinkedList<>();
-        }
-        identityProviderAliases.add(identityProviderAlias);
+        entities.add(IDENTITY_PROVIDER_ALIASES, identityProviderAlias);
     }
 
 
     public void addComponentId(String componentId) {
-        if (componentIds == null) {
-            componentIds = new LinkedList<>();
-        }
-        componentIds.add(componentId);
+        entities.add(COMPONENT_IDS, componentId);
     }
 
 
     public void addClientUuid(String clientUuid) {
-        if (clientUuids == null) {
-            clientUuids = new LinkedList<>();
-        }
-        clientUuids.add(clientUuid);
+        entities.add(CLIENT_UUIDS, clientUuid);
     }
 
 
     public void addRoleId(String roleId) {
-        if (roleIds == null) {
-            roleIds = new LinkedList<>();
-        }
-        roleIds.add(roleId);
+        entities.add(ROLE_IDS, roleId);
     }
 
 
     public void addGroupId(String groupId) {
-        if (groupIds == null) {
-            groupIds = new LinkedList<>();
-        }
-        groupIds.add(groupId);
+        entities.add(GROUP_IDS, groupId);
     }
 
 
     public void addAuthenticationFlowId(String flowId) {
-        if (authFlowIds == null) {
-            authFlowIds = new LinkedList<>();
-        }
-        authFlowIds.add(flowId);
+        entities.add(AUTH_FLOW_IDS, flowId);
     }
 
 
     public void addAuthenticationConfigId(String executionConfigId) {
-        if (authConfigIds == null) {
-            authConfigIds = new LinkedList<>();
-        }
-        authConfigIds.add(executionConfigId);
+        entities.add(AUTH_CONFIG_IDS, executionConfigId);
     }
 
 
     public void executeCleanup() {
         RealmResource realm = adminClient.realm(realmName);
 
+        List<String> userIds = entities.get(USER_IDS);
         if (userIds != null) {
             for (String userId : userIds) {
                 try {
@@ -128,6 +108,7 @@ public class TestCleanup {
             }
         }
 
+        List<String> identityProviderAliases = entities.get(IDENTITY_PROVIDER_ALIASES);
         if (identityProviderAliases != null) {
             for (String idpAlias : identityProviderAliases) {
                 try {
@@ -138,6 +119,7 @@ public class TestCleanup {
             }
         }
 
+        List<String> componentIds = entities.get(COMPONENT_IDS);
         if (componentIds != null) {
             for (String componentId : componentIds) {
                 try {
@@ -148,6 +130,7 @@ public class TestCleanup {
             }
         }
 
+        List<String> clientUuids = entities.get(CLIENT_UUIDS);
         if (clientUuids != null) {
             for (String clientUuId : clientUuids) {
                 try {
@@ -158,6 +141,7 @@ public class TestCleanup {
             }
         }
 
+        List<String> roleIds = entities.get(ROLE_IDS);
         if (roleIds != null) {
             for (String roleId : roleIds) {
                 try {
@@ -168,6 +152,7 @@ public class TestCleanup {
             }
         }
 
+        List<String> groupIds = entities.get(GROUP_IDS);
         if (groupIds != null) {
             for (String groupId : groupIds) {
                 try {
@@ -178,6 +163,7 @@ public class TestCleanup {
             }
         }
 
+        List<String> authFlowIds = entities.get(AUTH_FLOW_IDS);
         if (authFlowIds != null) {
             for (String flowId : authFlowIds) {
                 try {
@@ -188,6 +174,7 @@ public class TestCleanup {
             }
         }
 
+        List<String> authConfigIds = entities.get(AUTH_CONFIG_IDS);
         if (authConfigIds != null) {
             for (String configId : authConfigIds) {
                 try {
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserStorageRestTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserStorageRestTest.java
index 15f0564..d7f494f 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserStorageRestTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserStorageRestTest.java
@@ -156,6 +156,65 @@ public class UserStorageRestTest extends AbstractAdminTest {
 
     }
 
+
+    // KEYCLOAK-4438
+    @Test
+    public void testKerberosAuthenticatorDisabledWhenProviderRemoved() {
+        // Assert kerberos authenticator DISABLED
+        AuthenticationExecutionInfoRepresentation kerberosExecution = findKerberosExecution();
+        Assert.assertEquals(kerberosExecution.getRequirement(), AuthenticationExecutionModel.Requirement.DISABLED.toString());
+
+        // create LDAP provider with kerberos
+        ComponentRepresentation ldapRep = new ComponentRepresentation();
+        ldapRep.setName("ldap2");
+        ldapRep.setProviderId("ldap");
+        ldapRep.setProviderType(UserStorageProvider.class.getName());
+        ldapRep.setConfig(new MultivaluedHashMap<>());
+        ldapRep.getConfig().putSingle("priority", Integer.toString(2));
+        ldapRep.getConfig().putSingle(KerberosConstants.ALLOW_KERBEROS_AUTHENTICATION, "true");
+
+
+        String id = createComponent(ldapRep);
+
+        // Assert kerberos authenticator ALTERNATIVE
+        kerberosExecution = findKerberosExecution();
+        Assert.assertEquals(kerberosExecution.getRequirement(), AuthenticationExecutionModel.Requirement.ALTERNATIVE.toString());
+
+        // Remove LDAP provider
+        realm.components().component(id).remove();
+
+        // Assert kerberos authenticator DISABLED
+        kerberosExecution = findKerberosExecution();
+        Assert.assertEquals(kerberosExecution.getRequirement(), AuthenticationExecutionModel.Requirement.DISABLED.toString());
+
+        // Add kerberos provider
+        ComponentRepresentation kerberosRep = new ComponentRepresentation();
+        kerberosRep.setName("kerberos");
+        kerberosRep.setProviderId("kerberos");
+        kerberosRep.setProviderType(UserStorageProvider.class.getName());
+        kerberosRep.setConfig(new MultivaluedHashMap<>());
+        kerberosRep.getConfig().putSingle("priority", Integer.toString(2));
+
+        id = createComponent(kerberosRep);
+
+
+        // Assert kerberos authenticator ALTERNATIVE
+        kerberosExecution = findKerberosExecution();
+        Assert.assertEquals(kerberosExecution.getRequirement(), AuthenticationExecutionModel.Requirement.ALTERNATIVE.toString());
+
+        // Switch kerberos authenticator to REQUIRED
+        kerberosExecution.setRequirement(AuthenticationExecutionModel.Requirement.REQUIRED.toString());
+        realm.flows().updateExecutions("browser", kerberosExecution);
+
+        // Remove Kerberos provider
+        realm.components().component(id).remove();
+
+        // Assert kerberos authenticator DISABLED
+        kerberosExecution = findKerberosExecution();
+        Assert.assertEquals(kerberosExecution.getRequirement(), AuthenticationExecutionModel.Requirement.DISABLED.toString());
+    }
+
+
     @Test
     public void testValidateAndCreateLdapProvider() {
         // Invalid filter