From bca4c357080ad10259a5b87c15133d938c51c2a7 Mon Sep 17 00:00:00 2001 From: mposolda Date: Tue, 3 Oct 2017 16:19:47 +0200 Subject: [PATCH] KEYCLOAK-5371 Fix ActionTokenCrossDCTest and BruteForceCrossDCTest --- .../InfinispanActionTokenStoreProvider.java | 15 +++++- .../entities/ActionTokenValueEntity.java | 5 ++ .../integration-arquillian/HOW-TO-RUN.md | 10 ++-- .../lb/SimpleUndertowLoadBalancer.java | 51 +++++++++++++++++-- .../arquillian/DeploymentTargetModifier.java | 18 +++++-- .../crossdc/AbstractAdminCrossDCTest.java | 6 ++- .../crossdc/AbstractCrossDCTest.java | 12 +++-- .../crossdc/ActionTokenCrossDCTest.java | 23 ++++++--- .../crossdc/BruteForceCrossDCTest.java | 5 +- 9 files changed, 118 insertions(+), 27 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanActionTokenStoreProvider.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanActionTokenStoreProvider.java index 192c9647c7..2a31453a94 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanActionTokenStoreProvider.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanActionTokenStoreProvider.java @@ -16,6 +16,7 @@ */ package org.keycloak.models.sessions.infinispan; +import org.jboss.logging.Logger; import org.keycloak.cluster.ClusterProvider; import org.keycloak.common.util.Time; import org.keycloak.models.*; @@ -33,6 +34,8 @@ import org.infinispan.Cache; */ public class InfinispanActionTokenStoreProvider implements ActionTokenStoreProvider { + private static final Logger LOG = Logger.getLogger(InfinispanActionTokenStoreProvider.class); + private final Cache actionKeyCache; private final InfinispanKeycloakTransaction tx; private final KeycloakSession session; @@ -58,6 +61,8 @@ public class InfinispanActionTokenStoreProvider implements ActionTokenStoreProvi ActionTokenReducedKey tokenKey = new ActionTokenReducedKey(key.getUserId(), key.getActionId(), key.getActionVerificationNonce()); ActionTokenValueEntity tokenValue = new ActionTokenValueEntity(notes); + LOG.debugf("Adding used action token to actionTokens cache: %s", tokenKey.toString()); + this.tx.put(actionKeyCache, tokenKey, tokenValue, key.getExpiration() - Time.currentTime(), TimeUnit.SECONDS); } @@ -68,7 +73,15 @@ public class InfinispanActionTokenStoreProvider implements ActionTokenStoreProvi } ActionTokenReducedKey key = new ActionTokenReducedKey(actionTokenKey.getUserId(), actionTokenKey.getActionId(), actionTokenKey.getActionVerificationNonce()); - return this.actionKeyCache.getAdvancedCache().get(key); + + ActionTokenValueModel value = this.actionKeyCache.getAdvancedCache().get(key); + if (value == null) { + LOG.debugf("Not found any value in actionTokens cache for key: %s", key.toString()); + } else { + LOG.debugf("Found value in actionTokens cache for key: %s", key.toString()); + } + + return value; } @Override diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/entities/ActionTokenValueEntity.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/entities/ActionTokenValueEntity.java index 7e3c76ed5e..647a9df584 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/entities/ActionTokenValueEntity.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/entities/ActionTokenValueEntity.java @@ -45,6 +45,11 @@ public class ActionTokenValueEntity implements ActionTokenValueModel { return notes.get(name); } + @Override + public String toString() { + return String.format("ActionTokenValueEntity [ notes=%s ]", notes.toString()); + } + public static class ExternalizerImpl implements Externalizer { private static final int VERSION_1 = 1; diff --git a/testsuite/integration-arquillian/HOW-TO-RUN.md b/testsuite/integration-arquillian/HOW-TO-RUN.md index 4ed8eaf1a4..81815619ee 100644 --- a/testsuite/integration-arquillian/HOW-TO-RUN.md +++ b/testsuite/integration-arquillian/HOW-TO-RUN.md @@ -460,13 +460,13 @@ The cross DC requires setting a profile specifying used cache server by specifyi a) First compile the Infinispan/JDG test server via the following command: - `mvn -Pcache-server-infinispan -f testsuite/integration-arquillian -DskipTests clean install` + `mvn -Pcache-server-infinispan,auth-servers-crossdc-undertow -f testsuite/integration-arquillian -DskipTests clean install` or - `mvn -Pcache-server-jdg -f testsuite/integration-arquillian -DskipTests clean install` + `mvn -Pcache-server-jdg,auth-servers-crossdc-undertow -f testsuite/integration-arquillian -DskipTests clean install` -b) Then in case you want to use **JBoss-based** containers instead of containers on Embedded Undertow run following command: +b) Then in case you want to use **JBoss-based** Keycloak backend containers instead of containers on Embedded Undertow run following command: `mvn -Pauth-servers-crossdc-jboss,auth-server-wildfly -f testsuite/integration-arquillian -DskipTests clean install` @@ -476,7 +476,7 @@ By default JBoss-based containers use in-memory h2 database. It can be configure `mvn -Pauth-servers-crossdc-jboss,auth-server-wildfly,jpa -f testsuite/integration-arquillian -DskipTests clean install -Djdbc.mvn.groupId=org.mariadb.jdbc -Djdbc.mvn.artifactId=mariadb-java-client -Djdbc.mvn.version=2.0.3 -Dkeycloak.connectionsJpa.url=jdbc:mariadb://localhost:3306/keycloak -Dkeycloak.connectionsJpa.password=keycloak -Dkeycloak.connectionsJpa.user=keycloak` -c1) Then you can run the tests using the following command (adjust the test specification according to your needs) for containers on **Undertow**: +c1) Then you can run the tests using the following command (adjust the test specification according to your needs) for Keycloak backend containers on **Undertow**: `mvn -Pcache-server-infinispan,auth-servers-crossdc-undertow -Dtest=*.crossdc.* -pl testsuite/integration-arquillian/tests/base clean install` @@ -484,7 +484,7 @@ or `mvn -Pcache-server-jdg,auth-servers-crossdc-undertow -Dtest=*.crossdc.* -pl testsuite/integration-arquillian/tests/base clean install` -c2) For **JBoss-based** containers: +c2) For **JBoss-based** Keycloak backend containers: `mvn -Pcache-server-infinispan,auth-servers-crossdc-jboss,auth-server-wildfly -Dtest=*.crossdc.* -pl testsuite/integration-arquillian/tests/base clean install` diff --git a/testsuite/integration-arquillian/servers/auth-server/undertow/src/main/java/org/keycloak/testsuite/arquillian/undertow/lb/SimpleUndertowLoadBalancer.java b/testsuite/integration-arquillian/servers/auth-server/undertow/src/main/java/org/keycloak/testsuite/arquillian/undertow/lb/SimpleUndertowLoadBalancer.java index c102df2830..26be8f79ae 100644 --- a/testsuite/integration-arquillian/servers/auth-server/undertow/src/main/java/org/keycloak/testsuite/arquillian/undertow/lb/SimpleUndertowLoadBalancer.java +++ b/testsuite/integration-arquillian/servers/auth-server/undertow/src/main/java/org/keycloak/testsuite/arquillian/undertow/lb/SimpleUndertowLoadBalancer.java @@ -17,7 +17,12 @@ package org.keycloak.testsuite.arquillian.undertow.lb; +import java.lang.reflect.Field; import java.net.URI; +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedList; +import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -34,6 +39,7 @@ import io.undertow.server.handlers.proxy.ProxyHandler; import io.undertow.util.AttachmentKey; import io.undertow.util.Headers; import org.jboss.logging.Logger; +import org.keycloak.common.util.reflections.Reflections; import org.keycloak.services.managers.AuthenticationSessionManager; import java.util.LinkedHashMap; import java.util.StringTokenizer; @@ -110,11 +116,12 @@ public class SimpleUndertowLoadBalancer { lb.removeHost(uri); lb.addHost(uri, route); }); + log.infof("Load balancer: enable all nodes. All enabled nodes: %s", lb.toString()); } public void disableAllBackendNodes() { - log.debugf("Load balancer: disabling all nodes"); backendNodes.values().forEach(lb::removeHost); + log.infof("Load balancer: disabling all nodes"); } public void enableBackendNodeByName(String nodeName) { @@ -122,8 +129,8 @@ public class SimpleUndertowLoadBalancer { if (uri == null) { throw new IllegalArgumentException("Invalid node: " + nodeName); } - log.debugf("Load balancer: enabling node %s", nodeName); lb.addHost(uri, nodeName); + log.infof("Load balancer: enabled node '%s', All enabled nodes: %s", nodeName, lb.toString()); } public void disableBackendNodeByName(String nodeName) { @@ -131,8 +138,8 @@ public class SimpleUndertowLoadBalancer { if (uri == null) { throw new IllegalArgumentException("Invalid node: " + nodeName); } - log.debugf("Load balancer: disabling node %s", nodeName); lb.removeHost(uri); + log.infof("Load balancer: disabled node '%s', All enabled nodes: %s", nodeName, lb.toString()); } static Map parseNodes(String nodes) { @@ -225,6 +232,19 @@ public class SimpleUndertowLoadBalancer { } + // For now, overriden just this "addHost" method to avoid adding duplicates + @Override + public synchronized LoadBalancingProxyClient addHost(URI host, String jvmRoute) { + List current = getCurrentHostRoutes(); + if (current.contains(jvmRoute)) { + log.infof("Route '%s' already present. Skip adding", jvmRoute); + return this; + } else { + return super.addHost(host, jvmRoute); + } + } + + @Override public void getConnection(ProxyTarget target, HttpServerExchange exchange, ProxyCallback callback, long timeout, TimeUnit timeUnit) { long timeoutMs = timeUnit.toMillis(timeout); @@ -233,6 +253,31 @@ public class SimpleUndertowLoadBalancer { super.getConnection(target, exchange, callbackDelegate, timeout, timeUnit); } + + @Override + public String toString() { + return getCurrentHostRoutes().toString(); + } + + + private List getCurrentHostRoutes() { + Field hostsField = Reflections.findDeclaredField(LoadBalancingProxyClient.class, "hosts"); + hostsField.setAccessible(true); + Host[] hosts = (Host[]) Reflections.getFieldValue(hostsField, this); + + if (hosts == null) { + return Collections.emptyList(); + } + + List hostRoutes = new LinkedList<>(); + for (Host host : hosts) { + Field hostField = Reflections.findDeclaredField(Host.class, "jvmRoute"); + hostField.setAccessible(true); + String route = Reflections.getFieldValue(hostField, host).toString(); + hostRoutes.add(route); + } + return hostRoutes; + } } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/DeploymentTargetModifier.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/DeploymentTargetModifier.java index f7fc8cfc06..339ded7512 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/DeploymentTargetModifier.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/DeploymentTargetModifier.java @@ -22,6 +22,7 @@ import org.jboss.arquillian.container.spi.client.deployment.TargetDescription; import org.jboss.arquillian.container.test.impl.client.deployment.AnnotationDeploymentScenarioGenerator; import org.jboss.arquillian.test.spi.TestClass; import org.jboss.logging.Logger; +import org.keycloak.common.util.StringPropertyReplacer; import java.util.List; @@ -73,12 +74,23 @@ public class DeploymentTargetModifier extends AnnotationDeploymentScenarioGenera if (deployment.getTarget() != null) { String containerQualifier = deployment.getTarget().getName(); if (AUTH_SERVER_CURRENT.equals(containerQualifier)) { - String authServerQualifier = AuthServerTestEnricher.AUTH_SERVER_CONTAINER; - log.infof("Setting target container for deployment %s.%s: %s", testClass.getName(), deployment.getName(), authServerQualifier); - deployment.setTarget(new TargetDescription(authServerQualifier)); + String newAuthServerQualifier = AuthServerTestEnricher.AUTH_SERVER_CONTAINER; + updateAuthServerQualifier(deployment, testClass, newAuthServerQualifier); + } else { + String newAuthServerQualifier = StringPropertyReplacer.replaceProperties(containerQualifier); + if (!newAuthServerQualifier.equals(containerQualifier)) { + updateAuthServerQualifier(deployment, testClass, newAuthServerQualifier); + } } + + } } } + private void updateAuthServerQualifier(DeploymentDescription deployment, TestClass testClass, String newAuthServerQualifier) { + log.infof("Setting target container for deployment %s.%s: %s", testClass.getName(), deployment.getName(), newAuthServerQualifier); + deployment.setTarget(new TargetDescription(newAuthServerQualifier)); + } + } 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 27fed71496..b318c1457b 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 @@ -95,8 +95,10 @@ public abstract class AbstractAdminCrossDCTest extends AbstractCrossDCTest { T newStat = (T) stats.getSingleStatistics(key); Matcher matcherInstance = matcherOnOldStat.apply(oldStat); + + log.infof("assertSingleStatistics '%s' : oldStat: %s, newStat: %s", key, oldStat.toString(), newStat.toString()); assertThat(newStat, matcherInstance); - }, 20, 200); + }, 50, 200); } protected void assertStatistics(InfinispanStatistics stats, Runnable testedCode, BiConsumer, Map> assertionOnStats) { @@ -108,7 +110,7 @@ public abstract class AbstractAdminCrossDCTest extends AbstractCrossDCTest { Retry.execute(() -> { Map newStat = stats.getStatistics(); assertionOnStats.accept(oldStat, newStat); - }, 5, 200); + }, 50, 200); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/AbstractCrossDCTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/AbstractCrossDCTest.java index 0de088fe79..76f6a7e4cc 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/AbstractCrossDCTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/crossdc/AbstractCrossDCTest.java @@ -50,8 +50,8 @@ public abstract class AbstractCrossDCTest extends AbstractTestRealmKeycloakTest // Keep the following constants in sync with arquillian public static final String QUALIFIER_NODE_BALANCER = "auth-server-balancer-cross-dc"; - public static final String QUALIFIER_JBOSS_DC_0_NODE_1 = "auth-server-jboss-cross-dc-0_1"; - public static final String QUALIFIER_JBOSS_DC_1_NODE_1 = "auth-server-jboss-cross-dc-1_1"; + public static final String QUALIFIER_AUTH_SERVER_DC_0_NODE_1 = "auth-server-${node.name}-cross-dc-0_1"; + public static final String QUALIFIER_AUTH_SERVER_DC_1_NODE_1 = "auth-server-${node.name}-cross-dc-1_1"; @ArquillianResource @LoadBalancer(value = QUALIFIER_NODE_BALANCER) @@ -215,7 +215,9 @@ public abstract class AbstractCrossDCTest extends AbstractTestRealmKeycloakTest public void disableDcOnLoadBalancer(DC dc) { int dcIndex = dc.ordinal(); log.infof("Disabling load balancer for dc=%d", dcIndex); - this.suiteContext.getDcAuthServerBackendsInfo().get(dcIndex).forEach(c -> loadBalancerCtrl.disableBackendNodeByName(c.getQualifier())); + this.suiteContext.getDcAuthServerBackendsInfo().get(dcIndex).forEach(containerInfo -> { + loadBalancerCtrl.disableBackendNodeByName(containerInfo.getQualifier()); + }); } /** @@ -231,7 +233,9 @@ public abstract class AbstractCrossDCTest extends AbstractTestRealmKeycloakTest } else { dcNodes.stream() .filter(ContainerInfo::isStarted) - .forEach(c -> loadBalancerCtrl.enableBackendNodeByName(c.getQualifier())); + .forEach(containerInfo -> { + loadBalancerCtrl.enableBackendNodeByName(containerInfo.getQualifier()); + }); } } 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 1a4e079fba..e16597ac1a 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 @@ -42,6 +42,8 @@ import org.keycloak.testsuite.arquillian.annotation.JmxInfinispanChannelStatisti import org.keycloak.testsuite.arquillian.InfinispanStatistics; import org.keycloak.testsuite.arquillian.InfinispanStatistics.Constants; import org.keycloak.testsuite.pages.ProceedPage; + +import java.util.Map; import java.util.concurrent.TimeUnit; import org.hamcrest.Matchers; import static org.hamcrest.Matchers.greaterThan; @@ -104,8 +106,6 @@ public class ActionTokenCrossDCTest extends AbstractAdminCrossDCTest { String link = MailUtils.getPasswordResetEmailLink(message); - Retry.execute(() -> channelStatisticsCrossDc.reset(), 3, 100); - assertSingleStatistics(cacheDc0Node0Statistics, Constants.STAT_CACHE_NUMBER_OF_ENTRIES, () -> driver.navigate().to(link), Matchers::is @@ -115,10 +115,21 @@ public class ActionTokenCrossDCTest extends AbstractAdminCrossDCTest { proceedPage.clickProceedLink(); passwordUpdatePage.assertCurrent(); - // Verify that there was at least one message sent via the channel - assertSingleStatistics(channelStatisticsCrossDc, Constants.STAT_CHANNEL_SENT_MESSAGES, - () -> passwordUpdatePage.changePassword("new-pass", "new-pass"), - old -> greaterThan((Comparable) 0l) + // Verify that there was at least one message sent via the channel - Even if we did the change on DC0, the message may be sent either from DC0 or DC1. Seems it depends on the actionTokens key ownership. + // In case that it was sent from DC1, we will receive it in DC0. + assertStatistics(channelStatisticsCrossDc, + () -> { + passwordUpdatePage.changePassword("new-pass", "new-pass"); + }, + (Map oldStats, Map newStats) -> { + int oldSent = ((Number) oldStats.get(Constants.STAT_CHANNEL_SENT_MESSAGES)).intValue(); + int newSent = ((Number) newStats.get(Constants.STAT_CHANNEL_SENT_MESSAGES)).intValue(); + int oldReceived = ((Number) oldStats.get(Constants.STAT_CHANNEL_RECEIVED_MESSAGES)).intValue(); + int newReceived = ((Number) newStats.get(Constants.STAT_CHANNEL_RECEIVED_MESSAGES)).intValue(); + + log.infof("oldSent: %d, newSent: %d, oldReceived: %d, newReceived: %d", oldSent, newSent, oldReceived, newReceived); + Assert.assertTrue(newSent - oldSent > 0 || newReceived - oldReceived > 0); + } ); assertEquals("Your account has been updated.", driver.getTitle()); 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 735e29134c..3007cab443 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 @@ -52,7 +52,7 @@ public class BruteForceCrossDCTest extends AbstractAdminCrossDCTest { private static final String REALM_NAME = "brute-force-test"; @Deployment(name = "dc0") - @TargetsContainer(QUALIFIER_JBOSS_DC_0_NODE_1) + @TargetsContainer(QUALIFIER_AUTH_SERVER_DC_0_NODE_1) public static WebArchive deployDC0() { return RunOnServerDeployment.create( BruteForceCrossDCTest.class, @@ -64,7 +64,7 @@ public class BruteForceCrossDCTest extends AbstractAdminCrossDCTest { } @Deployment(name = "dc1") - @TargetsContainer(QUALIFIER_JBOSS_DC_1_NODE_1) + @TargetsContainer(QUALIFIER_AUTH_SERVER_DC_1_NODE_1) public static WebArchive deployDC1() { return RunOnServerDeployment.create( BruteForceCrossDCTest.class, @@ -248,7 +248,6 @@ public class BruteForceCrossDCTest extends AbstractAdminCrossDCTest { } - // TODO Having this working on Wildfly might be a challenge. Maybe require @Deployment with @TargetsContainer descriptor generated at runtime as we don't know the container qualifier at compile time... Maybe workaround by add endpoint to TestingResourceProvider if needed.. // resolution on Wildfly: make deployment available on both dc0_1 and dc1_1, see @Deployment methods private void addUserLoginFailure(KeycloakTestingClient testingClient) throws URISyntaxException, IOException { testingClient.server().run(session -> {