keycloak-aplcache
Changes
federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProviderFactory.java 5(+5 -0)
testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/TestContext.java 9(+7 -2)
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