From c816d5e030bb30a51e28ce6957f86362a1f36882 Mon Sep 17 00:00:00 2001 From: Douglas Palmer Date: Wed, 23 Oct 2024 20:51:30 -0700 Subject: [PATCH] Flaky test: org.keycloak.testsuite.broker.KcOidcBrokerTest#testPostBrokerLoginFlowWithOTP_bruteForceEnabled Closes #34075 Signed-off-by: Douglas Palmer --- .../testsuite/pages/LoginConfigTotpPage.java | 16 ++++---- .../testsuite/pages/LoginTotpPage.java | 2 +- .../keycloak/testsuite/util/WaitUtils.java | 29 +++++++++++++++ .../broker/AbstractAdvancedBrokerTest.java | 6 +-- .../testsuite/forms/BruteForceTest.java | 37 +++---------------- 5 files changed, 46 insertions(+), 44 deletions(-) diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginConfigTotpPage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginConfigTotpPage.java index 89f683092c..c292111a7c 100755 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginConfigTotpPage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginConfigTotpPage.java @@ -38,7 +38,7 @@ public class LoginConfigTotpPage extends LogoutSessionsPage { @FindBy(css = "input[type=\"submit\"]") private WebElement submitButton; - + @FindBy(name = "cancel-aia") private WebElement cancelAIAButton; @@ -59,21 +59,21 @@ public class LoginConfigTotpPage extends LogoutSessionsPage { public void configure(String totp) { totpInput.sendKeys(totp); - submitButton.click(); + submit(); } public void configure(String totp, String userLabel) { totpInput.sendKeys(totp); totpLabelInput.sendKeys(userLabel); - submitButton.click(); + submit(); } public void submit() { - submitButton.click(); + UIUtils.clickLink(submitButton); } - + public void cancel() { - cancelAIAButton.click(); + UIUtils.clickLink(cancelAIAButton); } public String getTotpSecret() { @@ -94,11 +94,11 @@ public class LoginConfigTotpPage extends LogoutSessionsPage { } public void clickManual() { - manualLink.click(); + UIUtils.clickLink(manualLink); } public void clickBarcode() { - barcodeLink.click(); + UIUtils.clickLink(barcodeLink); } public String getInputCodeError() { diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginTotpPage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginTotpPage.java index c30d0794fc..32b1b1ca5a 100755 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginTotpPage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginTotpPage.java @@ -52,7 +52,7 @@ public class LoginTotpPage extends LanguageComboboxAwarePage { otpInput.clear(); if (totp != null) otpInput.sendKeys(totp); - submitButton.click(); + UIUtils.clickLink(submitButton); } public String getAlertError() { diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/WaitUtils.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/WaitUtils.java index a703ad821c..dd9fa5af33 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/WaitUtils.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/WaitUtils.java @@ -17,6 +17,8 @@ package org.keycloak.testsuite.util; import org.jboss.arquillian.graphene.wait.ElementBuilder; +import org.keycloak.executors.ExecutorsProvider; +import org.keycloak.testsuite.client.KeycloakTestingClient; import org.openqa.selenium.By; import org.openqa.selenium.TimeoutException; import org.openqa.selenium.WebDriver; @@ -27,10 +29,13 @@ import org.openqa.selenium.support.ui.FluentWait; import org.openqa.selenium.support.ui.WebDriverWait; import java.time.Duration; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.ThreadPoolExecutor; import java.util.logging.Level; import java.util.logging.Logger; import static org.jboss.arquillian.graphene.Graphene.waitGui; +import static org.junit.Assert.assertEquals; import static org.keycloak.testsuite.util.DroneUtils.getCurrentDriver; import static org.openqa.selenium.support.ui.ExpectedConditions.javaScriptThrowsNoExceptions; import static org.openqa.selenium.support.ui.ExpectedConditions.not; @@ -157,4 +162,28 @@ public final class WaitUtils { waitUntilElementIsNotPresent(By.className("modal-backdrop")); } + public static long getNumExecutors(KeycloakTestingClient testingClient) { + String numExecutors = testingClient.server().fetchString(session -> { + ExecutorsProvider provider = session.getProvider(ExecutorsProvider.class); + ExecutorService executor = provider.getExecutor("bruteforce"); + ThreadPoolExecutor threadPoolExecutor = (ThreadPoolExecutor) executor; + return threadPoolExecutor.getCompletedTaskCount(); + }); + return Long.valueOf(numExecutors); + } + + public static void waitForExecutors(KeycloakTestingClient testingClient, long numExecutors) { + testingClient.server().run(session -> { + ExecutorsProvider provider = session.getProvider(ExecutorsProvider.class); + ExecutorService executor = provider.getExecutor("bruteforce"); + ThreadPoolExecutor threadPoolExecutor = (ThreadPoolExecutor) executor; + do { + try { + Thread.sleep(1000); + } catch (Exception e) { + } + } while (!threadPoolExecutor.getQueue().isEmpty()); + assertEquals(numExecutors, threadPoolExecutor.getCompletedTaskCount()); + }); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedBrokerTest.java index 09d5bbe319..8fb8a9b484 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedBrokerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedBrokerTest.java @@ -532,7 +532,7 @@ public abstract class AbstractAdvancedBrokerTest extends AbstractBrokerTest { updateExecutions(AbstractBrokerTest::disableUpdateProfileOnFirstLogin); testingClient.server(bc.consumerRealmName()).run(configurePostBrokerLoginWithOTP(bc.getIDPAlias())); - // Enable brute force protector in cosumer realm + // Enable brute force protector in consumer realm RealmResource realm = adminClient.realm(bc.consumerRealmName()); RealmRepresentation consumerRealmRep = realm.toRepresentation(); consumerRealmRep.setBruteForceProtected(true); @@ -567,14 +567,14 @@ public abstract class AbstractAdvancedBrokerTest extends AbstractBrokerTest { Map bruteForceStatus = realm.attackDetection().bruteForceUserStatus(user.getId()); assertFalse("User should not be disabled by brute force.", (boolean) bruteForceStatus.get("disabled")); + long numExecutors = WaitUtils.getNumExecutors(testingClient); // Login for 2 times with incorrect TOTP. This should temporarily disable the user loginTotpPage.login("bad-totp"); Assert.assertEquals("Invalid authenticator code.", loginTotpPage.getInputError()); - WaitUtils.waitForPageToLoad(); loginTotpPage.login("bad-totp"); Assert.assertEquals("Invalid authenticator code.", loginTotpPage.getInputError()); - WaitUtils.waitForPageToLoad(); + WaitUtils.waitForExecutors(testingClient, numExecutors+2); bruteForceStatus = realm.attackDetection().bruteForceUserStatus(user.getId()); assertTrue("User should be disabled by brute force.", (boolean) bruteForceStatus.get("disabled")); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java index 68dcfa75cb..ff18fdce05 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java @@ -28,7 +28,6 @@ import org.keycloak.events.Details; import org.keycloak.events.Errors; import org.keycloak.events.EventType; import org.keycloak.events.email.EmailEventListenerProviderFactory; -import org.keycloak.executors.ExecutorsProvider; import org.keycloak.models.Constants; import org.keycloak.models.UserModel; import org.keycloak.models.credential.PasswordCredentialModel; @@ -56,11 +55,10 @@ import org.keycloak.testsuite.util.UserBuilder; import jakarta.mail.MessagingException; import jakarta.mail.internet.MimeMessage; +import org.keycloak.testsuite.util.WaitUtils; import java.net.MalformedURLException; import java.util.*; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.ThreadPoolExecutor; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; @@ -418,11 +416,11 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { try { realm.setMaxDeltaTimeSeconds(5); testRealm().update(realm); - long numExecutors = getNumExecutors(); + long numExecutors = WaitUtils.getNumExecutors(testingClient); loginInvalidPassword(); //Wait for brute force executor to process the login and then wait for delta time - waitForExecutors(numExecutors + 1); + WaitUtils.waitForExecutors(testingClient, numExecutors + 1); testingClient.testing().setTimeOffset(Collections.singletonMap("offset", String.valueOf(5))); loginInvalidPassword(); @@ -440,11 +438,11 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { realm.setMaxDeltaTimeSeconds(5); realm.setPermanentLockout(true); testRealm().update(realm); - long numExecutors = getNumExecutors(); + long numExecutors = WaitUtils.getNumExecutors(testingClient); loginInvalidPassword(); //Wait for brute force executor to process the login and then wait for delta time - waitForExecutors(numExecutors + 1); + WaitUtils.waitForExecutors(testingClient, numExecutors + 1); testingClient.testing().setTimeOffset(Collections.singletonMap("offset", String.valueOf(5))); loginInvalidPassword(); @@ -459,31 +457,6 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { } } - private long getNumExecutors() { - String numExecutors = testingClient.server().fetchString(session -> { - ExecutorsProvider provider = session.getProvider(ExecutorsProvider.class); - ExecutorService executor = provider.getExecutor("bruteforce"); - ThreadPoolExecutor threadPoolExecutor = (ThreadPoolExecutor) executor; - return threadPoolExecutor.getTaskCount(); - }); - return Long.valueOf(numExecutors); - } - - private void waitForExecutors(long numExecutors) { - testingClient.server().run(session -> { - ExecutorsProvider provider = session.getProvider(ExecutorsProvider.class); - ExecutorService executor = provider.getExecutor("bruteforce"); - ThreadPoolExecutor threadPoolExecutor = (ThreadPoolExecutor) executor; - while (!threadPoolExecutor.getQueue().isEmpty()) { - try { - Thread.sleep(1000); - } catch (Exception e) { - } - } - assertEquals(numExecutors, threadPoolExecutor.getCompletedTaskCount()); - }); - } - @Test public void testWait() throws Exception { loginSuccess();