From 62a1c187a2b6b678480f5924f1ef5a5bc9e3fc96 Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 6 Nov 2017 22:14:05 +0100 Subject: [PATCH] KEYCLOAK-5716 KEYCLOAK-5738 Avoid infinispan deadlock. Ensure code-to-token works correctly in cross-dc --- .../java/org/keycloak/common/util}/Retry.java | 29 +++++++++--- misc/CrossDataCenter.md | 5 ++- .../InfinispanCodeToTokenStoreProvider.java | 25 +++++++++-- .../InfinispanUserSessionProviderFactory.java | 4 ++ .../remotestore/RemoteCacheInvoker.java | 22 +++++++-- .../ConcurrencyJDGRemoteCacheTest.java | 36 +++++++++++---- .../ConcurrencyJDGRemoveSessionTest.java | 4 +- .../ConcurrencyJDGSessionsCacheTest.java | 32 ++++++++++--- .../jboss/common/add-keycloak-caches.xsl | 3 +- .../CacheStatisticsControllerEnricher.java | 2 +- .../testsuite/AbstractKeycloakTest.java | 4 ++ .../AbstractSAMLAdapterClusterTest.java | 2 +- .../concurrency/AbstractConcurrencyTest.java | 18 +++++--- .../concurrency/ConcurrentLoginTest.java | 8 ++-- .../broker/AbstractBaseBrokerTest.java | 2 +- .../crossdc/AbstractAdminCrossDCTest.java | 2 +- .../crossdc/ActionTokenCrossDCTest.java | 2 +- .../crossdc/BruteForceCrossDCTest.java | 45 ++++++++++++++++++- .../crossdc/ConcurrentLoginCrossDCTest.java | 10 +---- .../crossdc/InvalidationCrossDCTest.java | 2 +- .../LastSessionRefreshCrossDCTest.java | 2 +- .../testsuite/crossdc/LoginCrossDCTest.java | 1 - .../crossdc/SessionExpirationCrossDCTest.java | 2 +- .../keycloak/testsuite/forms/LogoutTest.java | 2 +- .../session/LastSessionRefreshUnitTest.java | 2 +- .../java/org/keycloak/testsuite/Retry.java | 45 ------------------- .../keycloaksaml/SamlAdapterTestStrategy.java | 1 - .../testsuite/rule/AbstractKeycloakRule.java | 2 +- 28 files changed, 206 insertions(+), 108 deletions(-) rename {testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite => common/src/main/java/org/keycloak/common/util}/Retry.java (83%) delete mode 100644 testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/Retry.java diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/Retry.java b/common/src/main/java/org/keycloak/common/util/Retry.java similarity index 83% rename from testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/Retry.java rename to common/src/main/java/org/keycloak/common/util/Retry.java index 5b15a3f502..50f916ead1 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/Retry.java +++ b/common/src/main/java/org/keycloak/common/util/Retry.java @@ -1,5 +1,5 @@ /* - * Copyright 2016 Red Hat, Inc. and/or its affiliates + * Copyright 2017 Red Hat, Inc. and/or its affiliates * and other contributors as indicated by the @author tags. * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -15,9 +15,7 @@ * limitations under the License. */ -package org.keycloak.testsuite; - -import java.util.function.Supplier; +package org.keycloak.common.util; /** * @author Stian Thorgersen @@ -44,7 +42,9 @@ public class Retry { executionIndex++; if (attemptsCount > 0) { try { - Thread.sleep(intervalMillis); + if (intervalMillis > 0) { + Thread.sleep(intervalMillis); + } } catch (InterruptedException ie) { ie.addSuppressed(e); throw new RuntimeException(ie); @@ -73,7 +73,9 @@ public class Retry { attemptsCount--; if (attemptsCount > 0) { try { - Thread.sleep(intervalMillis); + if (intervalMillis > 0) { + Thread.sleep(intervalMillis); + } } catch (InterruptedException ie) { ie.addSuppressed(e); throw new RuntimeException(ie); @@ -85,4 +87,19 @@ public class Retry { } } + + /** + * Needed here just because java.util.function.Supplier defined from Java 8 + */ + public interface Supplier { + + /** + * Gets a result. + * + * @return a result + */ + T get(); + } + + } diff --git a/misc/CrossDataCenter.md b/misc/CrossDataCenter.md index fa69f2d42a..a57a7b9b44 100644 --- a/misc/CrossDataCenter.md +++ b/misc/CrossDataCenter.md @@ -95,7 +95,8 @@ Infinispan Server setup ... - + + @@ -103,7 +104,9 @@ Infinispan Server setup + + diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanCodeToTokenStoreProvider.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanCodeToTokenStoreProvider.java index 9aff14aa4a..1928733b1b 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanCodeToTokenStoreProvider.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanCodeToTokenStoreProvider.java @@ -22,6 +22,8 @@ import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import org.infinispan.commons.api.BasicCache; +import org.jboss.logging.Logger; +import org.keycloak.common.util.Retry; import org.keycloak.models.CodeToTokenStoreProvider; import org.keycloak.models.KeycloakSession; import org.keycloak.models.sessions.infinispan.entities.ActionTokenValueEntity; @@ -31,6 +33,8 @@ import org.keycloak.models.sessions.infinispan.entities.ActionTokenValueEntity; */ public class InfinispanCodeToTokenStoreProvider implements CodeToTokenStoreProvider { + public static final Logger logger = Logger.getLogger(InfinispanCodeToTokenStoreProvider.class); + private final Supplier> codeCache; private final KeycloakSession session; @@ -45,9 +49,24 @@ public class InfinispanCodeToTokenStoreProvider implements CodeToTokenStoreProvi int lifespanInSeconds = session.getContext().getRealm().getAccessCodeLifespan(); - BasicCache cache = codeCache.get(); - ActionTokenValueEntity existing = cache.putIfAbsent(codeId, tokenValue, lifespanInSeconds, TimeUnit.SECONDS); - return existing == null; + boolean codeAlreadyExists = Retry.call(() -> { + + try { + BasicCache cache = codeCache.get(); + ActionTokenValueEntity existing = cache.putIfAbsent(codeId, tokenValue, lifespanInSeconds, TimeUnit.SECONDS); + return existing == null; + } catch (RuntimeException re) { + if (logger.isDebugEnabled()) { + logger.debugf(re, "Failed when adding code %s", codeId); + } + + // Rethrow the exception. Retry will take care of handle the exception and eventually retry the operation. + throw re; + } + + }, 3, 0); + + return codeAlreadyExists; } @Override diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProviderFactory.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProviderFactory.java index ae2b3d46cb..76a010eb38 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProviderFactory.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProviderFactory.java @@ -255,6 +255,10 @@ public class InfinispanUserSessionProviderFactory implements UserSessionProvider RemoteCache> remoteCache = (RemoteCache) remoteStores.iterator().next().getRemoteCache(); + if (remoteCache == null) { + throw new IllegalStateException("No remote cache available for the infinispan cache: " + ispnCache.getName()); + } + remoteCacheInvoker.addRemoteCache(ispnCache.getName(), remoteCache, maxIdleLoader); RemoteCacheSessionListener hotrodListener = RemoteCacheSessionListener.createListener(session, ispnCache, remoteCache); diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/remotestore/RemoteCacheInvoker.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/remotestore/RemoteCacheInvoker.java index 3559c82f1b..49024ac686 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/remotestore/RemoteCacheInvoker.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/remotestore/RemoteCacheInvoker.java @@ -17,6 +17,7 @@ package org.keycloak.models.sessions.infinispan.remotestore; +import org.keycloak.common.util.Retry; import org.keycloak.common.util.Time; import java.util.Collections; import java.util.HashMap; @@ -32,6 +33,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.sessions.infinispan.changes.SessionEntityWrapper; import org.keycloak.models.sessions.infinispan.changes.SessionUpdateTask; +import org.keycloak.models.sessions.infinispan.entities.LoginFailureEntity; import org.keycloak.models.sessions.infinispan.entities.SessionEntity; import org.keycloak.models.sessions.infinispan.entities.UserSessionEntity; @@ -71,14 +73,28 @@ public class RemoteCacheInvoker { return; } - long maxIdleTimeMs = context.maxIdleTimeLoader.getMaxIdleTimeMs(realm); + long loadedMaxIdleTimeMs = context.maxIdleTimeLoader.getMaxIdleTimeMs(realm); // Double the timeout to ensure that entry won't expire on remoteCache in case that write of some entities to remoteCache is postponed (eg. userSession.lastSessionRefresh) - maxIdleTimeMs = maxIdleTimeMs * 2; + final long maxIdleTimeMs = loadedMaxIdleTimeMs * 2; logger.debugf("Running task '%s' on remote cache '%s' . Key is '%s'", operation, cacheName, key); - runOnRemoteCache(context.remoteCache, maxIdleTimeMs, key, task, sessionWrapper); + Retry.execute(() -> { + + try { + runOnRemoteCache(context.remoteCache, maxIdleTimeMs, key, task, sessionWrapper); + } catch (RuntimeException re) { + if (logger.isDebugEnabled()) { + logger.debugf(re, "Failed running task '%s' on remote cache '%s' . Key: '%s' . Will try to retry the task", + operation, cacheName, key); + } + + // Rethrow the exception. Retry will take care of handle the exception and eventually retry the operation. + throw re; + } + + }, 10, 0); } diff --git a/model/infinispan/src/test/java/org/keycloak/cluster/infinispan/ConcurrencyJDGRemoteCacheTest.java b/model/infinispan/src/test/java/org/keycloak/cluster/infinispan/ConcurrencyJDGRemoteCacheTest.java index fb0394a133..95133f5a0e 100644 --- a/model/infinispan/src/test/java/org/keycloak/cluster/infinispan/ConcurrencyJDGRemoteCacheTest.java +++ b/model/infinispan/src/test/java/org/keycloak/cluster/infinispan/ConcurrencyJDGRemoteCacheTest.java @@ -30,6 +30,7 @@ import org.infinispan.client.hotrod.annotation.ClientCacheEntryModified; import org.infinispan.client.hotrod.annotation.ClientListener; import org.infinispan.client.hotrod.event.ClientCacheEntryCreatedEvent; import org.infinispan.client.hotrod.event.ClientCacheEntryModifiedEvent; +import org.infinispan.client.hotrod.exceptions.HotRodClientException; import org.infinispan.manager.EmbeddedCacheManager; import org.infinispan.persistence.manager.PersistenceManager; import org.infinispan.persistence.remote.RemoteStore; @@ -47,9 +48,13 @@ public class ConcurrencyJDGRemoteCacheTest { private static Map state = new HashMap<>(); + private RemoteCache remoteCache1; + private RemoteCache remoteCache2; + + public static void main(String[] args) throws Exception { // Init map somehow - for (int i=0 ; i<30 ; i++) { + for (int i=0 ; i<3000 ; i++) { String key = "key-" + i; state.put(key, new EntryInfo()); } @@ -58,6 +63,8 @@ public class ConcurrencyJDGRemoteCacheTest { Worker worker1 = createWorker(1); Worker worker2 = createWorker(2); + long start = System.currentTimeMillis(); + // Start and join workers worker1.start(); worker2.start(); @@ -65,12 +72,16 @@ public class ConcurrencyJDGRemoteCacheTest { worker1.join(); worker2.join(); + long took = System.currentTimeMillis() - start; + // Output for (Map.Entry entry : state.entrySet()) { System.out.println(entry.getKey() + ":::" + entry.getValue()); worker1.cache.remove(entry.getKey()); } + System.out.println("Took: " + took + " ms"); + // Finish JVM worker1.cache.getCacheManager().stop(); worker2.cache.getCacheManager().stop(); @@ -127,7 +138,7 @@ public class ConcurrencyJDGRemoteCacheTest { String cacheKey = entry.getKey(); EntryInfo wrapper = state.get(cacheKey); - int val = getClusterStartupTime(this.cache, cacheKey, wrapper); + int val = getClusterStartupTime(this.cache, cacheKey, wrapper, myThreadId); if (myThreadId == 1) { wrapper.th1.set(val); } else { @@ -141,8 +152,8 @@ public class ConcurrencyJDGRemoteCacheTest { } - public static int getClusterStartupTime(Cache cache, String cacheKey, EntryInfo wrapper) { - Integer startupTime = new Random().nextInt(1024); + public static int getClusterStartupTime(Cache cache, String cacheKey, EntryInfo wrapper, int myThreadId) { + Integer startupTime = myThreadId==1 ? Integer.parseInt(cacheKey.substring(4)) : Integer.parseInt(cacheKey.substring(4)) * 2; // Concurrency doesn't work correctly with this //Integer existingClusterStartTime = (Integer) cache.putIfAbsent(cacheKey, startupTime); @@ -154,21 +165,25 @@ public class ConcurrencyJDGRemoteCacheTest { for (int i=0 ; i<10 ; i++) { try { existingClusterStartTime = (Integer) remoteCache.withFlags(Flag.FORCE_RETURN_VALUE).putIfAbsent(cacheKey, startupTime); - } catch (Exception ce) { + break; + } catch (HotRodClientException ce) { if (i == 9) { throw ce; //break; } else { - System.err.println("EXception: i=" + i); + wrapper.exceptions.incrementAndGet(); + System.err.println("Exception: i=" + i + " for key: " + cacheKey + " and myThreadId: " + myThreadId); } } } - if (existingClusterStartTime == null || startupTime.equals(remoteCache.get(cacheKey))) { + if (existingClusterStartTime == null +// || startupTime.equals(remoteCache.get(cacheKey)) + ) { wrapper.successfulInitializations.incrementAndGet(); return startupTime; } else { - System.err.println("Not equal!!! startupTime=" + startupTime + ", existingClusterStartTime=" + existingClusterStartTime ); + wrapper.failedInitializations.incrementAndGet(); return existingClusterStartTime; } } @@ -178,10 +193,13 @@ public class ConcurrencyJDGRemoteCacheTest { AtomicInteger successfulListenerWrites = new AtomicInteger(0); AtomicInteger th1 = new AtomicInteger(); AtomicInteger th2 = new AtomicInteger(); + AtomicInteger failedInitializations = new AtomicInteger(); + AtomicInteger exceptions = new AtomicInteger(); @Override public String toString() { - return String.format("Inits: %d, listeners: %d, th1: %d, th2: %d", successfulInitializations.get(), successfulListenerWrites.get(), th1.get(), th2.get()); + return String.format("Inits: %d, listeners: %d, failedInits: %d, exceptions: %s, th1: %d, th2: %d", successfulInitializations.get(), successfulListenerWrites.get(), + failedInitializations.get(), exceptions.get(), th1.get(), th2.get()); } } diff --git a/model/infinispan/src/test/java/org/keycloak/cluster/infinispan/ConcurrencyJDGRemoveSessionTest.java b/model/infinispan/src/test/java/org/keycloak/cluster/infinispan/ConcurrencyJDGRemoveSessionTest.java index 536ee33225..7e55783ed2 100644 --- a/model/infinispan/src/test/java/org/keycloak/cluster/infinispan/ConcurrencyJDGRemoveSessionTest.java +++ b/model/infinispan/src/test/java/org/keycloak/cluster/infinispan/ConcurrencyJDGRemoveSessionTest.java @@ -121,6 +121,8 @@ public class ConcurrencyJDGRemoveSessionTest { logger.info("SESSIONS NOT AVAILABLE ON DC2"); + long took = System.currentTimeMillis() - start; + logger.infof("took %d ms", took); // // Start and join workers // worker1.start(); @@ -137,8 +139,6 @@ public class ConcurrencyJDGRemoveSessionTest { cache2.getCacheManager().stop(); } - long took = System.currentTimeMillis() - start; - // // Output // for (Map.Entry entry : state.entrySet()) { // System.out.println(entry.getKey() + ":::" + entry.getValue()); diff --git a/model/infinispan/src/test/java/org/keycloak/cluster/infinispan/ConcurrencyJDGSessionsCacheTest.java b/model/infinispan/src/test/java/org/keycloak/cluster/infinispan/ConcurrencyJDGSessionsCacheTest.java index 2dfb5085d3..0d28458ec8 100644 --- a/model/infinispan/src/test/java/org/keycloak/cluster/infinispan/ConcurrencyJDGSessionsCacheTest.java +++ b/model/infinispan/src/test/java/org/keycloak/cluster/infinispan/ConcurrencyJDGSessionsCacheTest.java @@ -187,6 +187,10 @@ public class ConcurrencyJDGSessionsCacheTest { ", successfulListenerWrites: " + successfulListenerWrites.get() + ", successfulListenerWrites2: " + successfulListenerWrites2.get() + ", failedReplaceCounter: " + failedReplaceCounter.get() + ", failedReplaceCounter2: " + failedReplaceCounter2.get()); + + System.out.println("remoteCache1.notes: " + ((UserSessionEntity) remoteCache1.get("123")).getNotes().size() ); + System.out.println("remoteCache2.notes: " + ((UserSessionEntity) remoteCache2.get("123")).getNotes().size() ); + System.out.println("Histogram: "); //histogram.dumpStats(); @@ -314,14 +318,26 @@ public class ConcurrencyJDGSessionsCacheTest { // In case it's hardcoded (eg. all the replaces are doing same change, so session is defacto not changed), then histogram may contain bigger value than 1 on some places. //String noteKey = "some"; - boolean replaced = false; - while (!replaced) { + ReplaceStatus replaced = ReplaceStatus.NOT_REPLACED; + while (replaced != ReplaceStatus.REPLACED) { VersionedValue versioned = remoteCache.getVersioned("123"); UserSessionEntity oldSession = versioned.getValue(); //UserSessionEntity clone = DistributedCacheConcurrentWritesTest.cloneSession(oldSession); UserSessionEntity clone = oldSession; - clone.getNotes().put(noteKey, "someVal"); + // In case that exception was thrown (ReplaceStatus.ERROR), the remoteCache may have the note. Seems that transactions are not fully rolled-back on the JDG side + // in case that backup fails + if (replaced == ReplaceStatus.NOT_REPLACED) { + clone.getNotes().put(noteKey, "someVal"); + } else if (replaced == ReplaceStatus.ERROR) { + if (clone.getNotes().containsKey(noteKey)) { + System.err.println("I HAVE THE KEY: " + noteKey); + } else { + System.err.println("I DON'T HAVE THE KEY: " + noteKey); + clone.getNotes().put(noteKey, "someVal"); + } + } + //cache.replace("123", clone); replaced = cacheReplace(versioned, clone); } @@ -336,7 +352,7 @@ public class ConcurrencyJDGSessionsCacheTest { } - private boolean cacheReplace(VersionedValue oldSession, UserSessionEntity newSession) { + private ReplaceStatus cacheReplace(VersionedValue oldSession, UserSessionEntity newSession) { try { boolean replaced = remoteCache.replaceWithVersion("123", newSession, oldSession.getVersion()); //boolean replaced = true; @@ -348,15 +364,19 @@ public class ConcurrencyJDGSessionsCacheTest { } else { histogram.increaseSuccessOpsCount(oldSession.getVersion()); } - return replaced; + return replaced ? ReplaceStatus.REPLACED : ReplaceStatus.NOT_REPLACED; } catch (Exception re) { failedReplaceCounter2.incrementAndGet(); - return false; + return ReplaceStatus.ERROR; } //return replaced; } } + + private enum ReplaceStatus { + REPLACED, NOT_REPLACED, ERROR + } /* // Worker, which operates on "classic" cache and rely on operations delegated to the second cache private static class CacheWorker extends Thread { diff --git a/testsuite/integration-arquillian/servers/cache-server/jboss/common/add-keycloak-caches.xsl b/testsuite/integration-arquillian/servers/cache-server/jboss/common/add-keycloak-caches.xsl index 8bd5149ea5..e6c16da2e1 100644 --- a/testsuite/integration-arquillian/servers/cache-server/jboss/common/add-keycloak-caches.xsl +++ b/testsuite/integration-arquillian/servers/cache-server/jboss/common/add-keycloak-caches.xsl @@ -36,7 +36,8 @@ - + + diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/CacheStatisticsControllerEnricher.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/CacheStatisticsControllerEnricher.java index 04c611042b..5ccfb3117c 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/CacheStatisticsControllerEnricher.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/CacheStatisticsControllerEnricher.java @@ -35,7 +35,7 @@ import org.jboss.arquillian.core.spi.Validate; import org.jboss.arquillian.test.spi.TestEnricher; import org.jboss.logging.Logger; import org.keycloak.connections.infinispan.InfinispanConnectionProvider; -import org.keycloak.testsuite.Retry; +import org.keycloak.common.util.Retry; import org.keycloak.testsuite.arquillian.annotation.JmxInfinispanCacheStatistics; import org.keycloak.testsuite.arquillian.annotation.JmxInfinispanChannelStatistics; import org.keycloak.testsuite.arquillian.jmx.JmxConnectorRegistry; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java index dddeba77b6..d545d8bc05 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java @@ -433,6 +433,10 @@ public abstract class AbstractKeycloakTest { } } + public Logger getLogger() { + return log; + } + private static void enableHTTPSForAuthServer() throws IOException, CommandFailedException, TimeoutException, InterruptedException, CliException, OperationException { OnlineManagementClient client = AuthServerTestEnricher.getManagementClient(); Administration administration = new Administration(client); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/cluster/AbstractSAMLAdapterClusterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/cluster/AbstractSAMLAdapterClusterTest.java index f71b757eac..fdd0a0ca88 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/cluster/AbstractSAMLAdapterClusterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/cluster/AbstractSAMLAdapterClusterTest.java @@ -18,7 +18,7 @@ package org.keycloak.testsuite.adapter.servlet.cluster; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.representations.idm.*; -import org.keycloak.testsuite.Retry; +import org.keycloak.common.util.Retry; import org.keycloak.testsuite.adapter.page.EmployeeServletDistributable; import org.keycloak.testsuite.adapter.page.SAMLServlet; import org.keycloak.testsuite.auth.page.AuthRealm; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/AbstractConcurrencyTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/AbstractConcurrencyTest.java index fe20270f63..5cd0110994 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/AbstractConcurrencyTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/AbstractConcurrencyTest.java @@ -17,9 +17,11 @@ package org.keycloak.testsuite.admin.concurrency; +import org.jboss.logging.Logger; import org.keycloak.admin.client.Keycloak; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; import java.util.LinkedList; import java.util.Collection; @@ -54,14 +56,19 @@ public abstract class AbstractConcurrencyTest extends AbstractTestRealmKeycloakT } protected void run(final int numThreads, final int totalNumberOfExecutions, final KeycloakRunnable... runnables) { + run(numThreads, totalNumberOfExecutions, this, runnables); + } + + + public static void run(final int numThreads, final int totalNumberOfExecutions, AbstractKeycloakTest testImpl, final KeycloakRunnable... runnables) { final ExecutorService service = SYNCHRONIZED - ? Executors.newSingleThreadExecutor() - : Executors.newFixedThreadPool(numThreads); + ? Executors.newSingleThreadExecutor() + : Executors.newFixedThreadPool(numThreads); ThreadLocal keycloaks = new ThreadLocal() { @Override protected Keycloak initialValue() { - return Keycloak.getInstance(getAuthServerRoot().toString(), "master", "admin", "admin", org.keycloak.models.Constants.ADMIN_CLI_CLIENT_ID); + return Keycloak.getInstance(testImpl.getAuthServerRoot().toString(), "master", "admin", "admin", org.keycloak.models.Constants.ADMIN_CLI_CLIENT_ID); } }; @@ -95,12 +102,13 @@ public abstract class AbstractConcurrencyTest extends AbstractTestRealmKeycloakT if (! failures.isEmpty()) { RuntimeException ex = new RuntimeException("There were failures in threads. Failures count: " + failures.size()); failures.forEach(ex::addSuppressed); - failures.forEach(e -> log.error(e.getMessage(), e)); + failures.forEach(e -> testImpl.getLogger().error(e.getMessage(), e)); throw ex; } } - protected interface KeycloakRunnable { + + public interface KeycloakRunnable { void run(int threadIndex, Keycloak keycloak, RealmResource realm) throws Throwable; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrentLoginTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrentLoginTest.java index e350584438..ad6efd5d5a 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrentLoginTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrentLoginTest.java @@ -50,7 +50,7 @@ import org.keycloak.admin.client.resource.ClientsResource; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.representations.AccessToken; import org.keycloak.representations.idm.ClientRepresentation; -import org.keycloak.testsuite.Retry; +import org.keycloak.common.util.Retry; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.util.ClientBuilder; import org.keycloak.testsuite.util.OAuthClient; @@ -222,9 +222,9 @@ public class ConcurrentLoginTest extends AbstractConcurrencyTest { oauth1.openLogout(); - // Code should be successfully exchanged for the token just once - Assert.assertEquals(1, codeToTokenSuccessCount.get()); - Assert.assertEquals(DEFAULT_THREADS - 1, codeToTokenErrorsCount.get()); + // Code should be successfully exchanged for the token at max once. In some cases (EG. Cross-DC) it may not be even successfully exchanged + Assert.assertThat(codeToTokenSuccessCount.get(), Matchers.lessThanOrEqualTo(1)); + Assert.assertThat(codeToTokenErrorsCount.get(), Matchers.greaterThanOrEqualTo(DEFAULT_THREADS - 1)); log.infof("Iteration %d passed successfully", i); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java index eabc7d17b0..16430708f5 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java @@ -26,7 +26,7 @@ import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.Assert; -import org.keycloak.testsuite.Retry; +import org.keycloak.common.util.Retry; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.pages.AccountPasswordPage; import org.keycloak.testsuite.pages.AccountUpdateProfilePage; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/AbstractAdminCrossDCTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/AbstractAdminCrossDCTest.java index b318c1457b..5ae521a9e3 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/AbstractAdminCrossDCTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/AbstractAdminCrossDCTest.java @@ -19,7 +19,7 @@ package org.keycloak.testsuite.crossdc; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.events.log.JBossLoggingEventListenerProviderFactory; import org.keycloak.representations.idm.RealmRepresentation; -import org.keycloak.testsuite.Retry; +import org.keycloak.common.util.Retry; import org.keycloak.testsuite.arquillian.InfinispanStatistics; import org.keycloak.testsuite.events.EventsListenerProviderFactory; import org.keycloak.testsuite.util.TestCleanup; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/ActionTokenCrossDCTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/ActionTokenCrossDCTest.java index e16597ac1a..0efa747312 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/ActionTokenCrossDCTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/ActionTokenCrossDCTest.java @@ -20,7 +20,7 @@ import org.keycloak.admin.client.resource.UserResource; import org.keycloak.connections.infinispan.InfinispanConnectionProvider; import org.keycloak.models.UserModel; import org.keycloak.representations.idm.UserRepresentation; -import org.keycloak.testsuite.Retry; +import org.keycloak.common.util.Retry; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.page.LoginPasswordUpdatePage; import org.keycloak.testsuite.pages.ErrorPage; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/BruteForceCrossDCTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/BruteForceCrossDCTest.java index e7d9026ab1..98c47c96bf 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/BruteForceCrossDCTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/BruteForceCrossDCTest.java @@ -21,12 +21,16 @@ import java.io.IOException; import java.net.URISyntaxException; import javax.ws.rs.NotFoundException; + +import org.hamcrest.Matchers; import org.jboss.arquillian.container.test.api.Deployment; import org.jboss.arquillian.container.test.api.TargetsContainer; import org.jboss.shrinkwrap.api.spec.WebArchive; import org.junit.Before; import org.junit.Test; +import org.keycloak.admin.client.Keycloak; +import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.connections.infinispan.InfinispanConnectionProvider; import org.keycloak.models.Constants; import org.keycloak.models.RealmModel; @@ -36,7 +40,8 @@ import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; import org.keycloak.testsuite.Assert; -import org.keycloak.testsuite.Retry; +import org.keycloak.common.util.Retry; +import org.keycloak.testsuite.admin.concurrency.AbstractConcurrencyTest; import org.keycloak.testsuite.client.KeycloakTestingClient; import org.keycloak.testsuite.runonserver.RunOnServerDeployment; import org.keycloak.testsuite.util.ClientBuilder; @@ -210,6 +215,44 @@ public class BruteForceCrossDCTest extends AbstractAdminCrossDCTest { } + @Test + public void testBruteForceConcurrentUpdate() throws Exception { + // Enable 1st node on each DC only + enableDcOnLoadBalancer(DC.FIRST); + enableDcOnLoadBalancer(DC.SECOND); + + // Clear all + adminClient.realms().realm(REALM_NAME).attackDetection().clearAllBruteForce(); + assertStatistics("After brute force cleared", 0, 0, 0); + + // create the entry manually in DC0 + addUserLoginFailure(getTestingClientForStartedNodeInDc(0)); + assertStatistics("After create entry1", 1, 0, 1); + + AbstractConcurrencyTest.KeycloakRunnable runnable = new AbstractConcurrencyTest.KeycloakRunnable() { + + @Override + public void run(int threadIndex, Keycloak keycloak, RealmResource realm) throws Throwable { + createBruteForceFailures(1, "login-test-1"); + } + + }; + + AbstractConcurrencyTest.run(2, 20, this, runnable); + + Retry.execute(() -> { + int dc0user1 = (Integer) getAdminClientForStartedNodeInDc(0).realm(REALM_NAME).attackDetection().bruteForceUserStatus("login-test-1").get("numFailures"); + int dc1user1 = (Integer) getAdminClientForStartedNodeInDc(1).realm(REALM_NAME).attackDetection().bruteForceUserStatus("login-test-1").get("numFailures"); + + log.infof("After concurrent update entry1: dc0User1=%d, dc1user1=%d", dc0user1, dc1user1); + + // The numFailures can be actually bigger than 20. Conflicts can increase the numFailures number to bigger value as they may not be fully reverted (listeners etc) + Assert.assertThat(dc0user1, Matchers.greaterThan(20)); + Assert.assertEquals(dc0user1, dc1user1); + }, 50, 50); + } + + private void assertStatistics(String prefixMessage, int expectedUser1, int expectedUser2, int expectedCacheSize) { Retry.execute(() -> { int dc0user1 = (Integer) getAdminClientForStartedNodeInDc(0).realm(REALM_NAME).attackDetection().bruteForceUserStatus("login-test-1").get("numFailures"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/ConcurrentLoginCrossDCTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/ConcurrentLoginCrossDCTest.java index 2c57be0aad..44605c9141 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/ConcurrentLoginCrossDCTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/ConcurrentLoginCrossDCTest.java @@ -17,6 +17,7 @@ package org.keycloak.testsuite.crossdc; +import org.junit.Assert; import org.keycloak.admin.client.Keycloak; import org.keycloak.admin.client.resource.RealmResource; import java.util.List; @@ -67,15 +68,6 @@ public class ConcurrentLoginCrossDCTest extends ConcurrentLoginTest { } - // TODO: We know that this test won't work in cross-dc setup based on "backup "caches. But we need to add the test that clientSessions - // are invalidated after attempt of reuse the same code multiple times - @Test - @Override - @Ignore - public void concurrentCodeReuseShouldFail() throws Throwable { - - } - @Test public void concurrentLoginWithRandomDcFailures() throws Throwable { log.info("*********************************************"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/InvalidationCrossDCTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/InvalidationCrossDCTest.java index c3819f77a0..49aaa95096 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/InvalidationCrossDCTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/InvalidationCrossDCTest.java @@ -31,7 +31,7 @@ import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.representations.idm.authorization.ResourceRepresentation; import org.keycloak.testsuite.Assert; -import org.keycloak.testsuite.Retry; +import org.keycloak.common.util.Retry; import org.keycloak.testsuite.admin.ApiUtil; /** diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/LastSessionRefreshCrossDCTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/LastSessionRefreshCrossDCTest.java index 8f04936f69..ddbfdb6070 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/LastSessionRefreshCrossDCTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/LastSessionRefreshCrossDCTest.java @@ -24,7 +24,7 @@ import org.junit.Test; import org.keycloak.connections.infinispan.InfinispanConnectionProvider; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.testsuite.Assert; -import org.keycloak.testsuite.Retry; +import org.keycloak.common.util.Retry; import org.keycloak.testsuite.arquillian.ContainerInfo; import org.keycloak.testsuite.rest.representation.RemoteCacheStats; import org.keycloak.testsuite.util.OAuthClient; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/LoginCrossDCTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/LoginCrossDCTest.java index c0eb849048..dacca6fb15 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/LoginCrossDCTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/LoginCrossDCTest.java @@ -22,7 +22,6 @@ import javax.ws.rs.core.Response; import org.apache.http.client.methods.CloseableHttpResponse; import org.junit.Test; import org.keycloak.testsuite.Assert; -import org.keycloak.testsuite.Retry; import org.keycloak.testsuite.util.Matchers; import org.keycloak.testsuite.util.OAuthClient; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/SessionExpirationCrossDCTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/SessionExpirationCrossDCTest.java index 1523b33b64..95c647894a 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/SessionExpirationCrossDCTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/SessionExpirationCrossDCTest.java @@ -36,7 +36,7 @@ import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.representations.idm.UserSessionRepresentation; import org.keycloak.testsuite.Assert; -import org.keycloak.testsuite.Retry; +import org.keycloak.common.util.Retry; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.arquillian.InfinispanStatistics; import org.keycloak.testsuite.arquillian.annotation.JmxInfinispanCacheStatistics; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LogoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LogoutTest.java index 1b1a857e63..790475b9a8 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LogoutTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LogoutTest.java @@ -25,7 +25,7 @@ import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; -import org.keycloak.testsuite.Retry; +import org.keycloak.common.util.Retry; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.pages.AppPage; import org.keycloak.testsuite.pages.LoginPage; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/session/LastSessionRefreshUnitTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/session/LastSessionRefreshUnitTest.java index 569a076e34..f10ca46511 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/session/LastSessionRefreshUnitTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/session/LastSessionRefreshUnitTest.java @@ -37,7 +37,7 @@ import org.keycloak.models.sessions.infinispan.changes.sessions.SessionData; import org.keycloak.models.sessions.infinispan.entities.UserSessionEntity; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.testsuite.AbstractKeycloakTest; -import org.keycloak.testsuite.Retry; +import org.keycloak.common.util.Retry; import org.keycloak.testsuite.runonserver.RunOnServer; import org.keycloak.testsuite.runonserver.RunOnServerDeployment; diff --git a/testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/Retry.java b/testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/Retry.java deleted file mode 100644 index 673d4fbb70..0000000000 --- a/testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/Retry.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright 2016 Red Hat, Inc. and/or its affiliates - * and other contributors as indicated by the @author tags. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.keycloak.testsuite; - -/** - * @author Stian Thorgersen - */ -public class Retry { - - public static void execute(Runnable runnable, int retry, long interval) { - while (true) { - try { - runnable.run(); - return; - } catch (RuntimeException e) { - retry--; - if (retry > 0) { - try { - Thread.sleep(interval); - } catch (InterruptedException ie) { - throw new RuntimeException(ie); - } - } else { - throw e; - } - } - } - } - -} diff --git a/testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/keycloaksaml/SamlAdapterTestStrategy.java b/testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/keycloaksaml/SamlAdapterTestStrategy.java index 1dd5f72d33..31b11f1d1a 100755 --- a/testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/keycloaksaml/SamlAdapterTestStrategy.java +++ b/testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/keycloaksaml/SamlAdapterTestStrategy.java @@ -45,7 +45,6 @@ import org.keycloak.saml.common.constants.*; import org.keycloak.saml.processing.core.saml.v2.constants.X500SAMLProfileConstants; import org.keycloak.services.managers.RealmManager; import org.keycloak.testsuite.KeycloakServer; -import org.keycloak.testsuite.Retry; import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.rule.AbstractKeycloakRule; import org.keycloak.testsuite.rule.ErrorServlet; diff --git a/testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/rule/AbstractKeycloakRule.java b/testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/rule/AbstractKeycloakRule.java index c9dae67d72..4a936fe30c 100755 --- a/testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/rule/AbstractKeycloakRule.java +++ b/testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/rule/AbstractKeycloakRule.java @@ -31,6 +31,7 @@ import org.junit.rules.TemporaryFolder; import org.keycloak.Config; import org.keycloak.adapters.KeycloakConfigResolver; import org.keycloak.adapters.servlet.KeycloakOIDCFilter; +import org.keycloak.common.util.Retry; import org.keycloak.common.util.Time; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakTransaction; @@ -41,7 +42,6 @@ import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.services.managers.RealmManager; import org.keycloak.testsuite.KeycloakServer; -import org.keycloak.testsuite.Retry; import org.keycloak.util.JsonSerialization; import javax.servlet.DispatcherType;