From 5b0980172d505c89d7b81c68a3bcd09930702522 Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Tue, 12 Jul 2016 15:21:00 +0200 Subject: [PATCH] KEYCLOAK-3267 Fix identity broker login with brute force enabled --- .../resources/IdentityBrokerService.java | 7 +- .../testsuite/broker/AbstractBrokerTest.java | 71 +++++++++++++++++++ .../keycloak/testsuite/util/RealmBuilder.java | 10 +++ 3 files changed, 86 insertions(+), 2 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java index 1c6f23d5fe..c7cbc62709 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java @@ -55,6 +55,7 @@ import org.keycloak.provider.ProviderFactory; import org.keycloak.representations.AccessToken; import org.keycloak.services.managers.AppAuthManager; import org.keycloak.services.managers.AuthenticationManager.AuthResult; +import org.keycloak.services.managers.BruteForceProtector; import org.keycloak.services.managers.ClientSessionCode; import org.keycloak.services.messages.Messages; import org.keycloak.services.ErrorResponse; @@ -338,8 +339,10 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal return ErrorPage.error(session, Messages.ACCOUNT_DISABLED); } if (realm.isBruteForceProtected()) { - event.error(Errors.USER_TEMPORARILY_DISABLED); - return ErrorPage.error(session, Messages.ACCOUNT_DISABLED); + if (session.getProvider(BruteForceProtector.class).isTemporarilyDisabled(session, realm, user)) { + event.error(Errors.USER_TEMPORARILY_DISABLED); + return ErrorPage.error(session, Messages.ACCOUNT_DISABLED); + } } return null; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java index aa44d85de2..a2a16b4bc8 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java @@ -9,15 +9,21 @@ import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.services.messages.Messages; import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.Assert; +import org.keycloak.testsuite.pages.AccountPasswordPage; +import org.keycloak.testsuite.pages.ErrorPage; import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.UpdateAccountInformationPage; +import org.keycloak.testsuite.util.RealmBuilder; import java.io.UnsupportedEncodingException; import java.net.URLEncoder; import java.util.List; +import static org.jgroups.util.Util.assertTrue; +import static org.junit.Assert.assertEquals; import static org.keycloak.testsuite.admin.ApiUtil.createUserWithAdminClient; import static org.keycloak.testsuite.admin.ApiUtil.resetUserPassword; @@ -42,9 +48,16 @@ public abstract class AbstractBrokerTest extends AbstractKeycloakTest { @Page protected LoginPage accountLoginPage; + @Page protected UpdateAccountInformationPage updateAccountInformationPage; + @Page + protected AccountPasswordPage accountPasswordPage; + + @Page + protected ErrorPage errorPage; + @Override public void addTestRealms(List testRealms) { RealmRepresentation providerRealm = createProviderRealm(); @@ -171,6 +184,60 @@ public abstract class AbstractBrokerTest extends AbstractKeycloakTest { testSingleLogout(); } + @Test + public void loginWithExistingUser() { + logInAsUserInIDP(); + + Integer userCount = adminClient.realm(consumerRealmName()).users().count(); + + driver.navigate().to(getAccountUrl(consumerRealmName())); + + log.debug("Clicking social " + getIDPAlias()); + accountLoginPage.clickSocial(getIDPAlias()); + + Assert.assertTrue("Driver should be on the provider realm page right now", driver.getCurrentUrl().contains("/auth/realms/" + providerRealmName() + "/")); + + accountLoginPage.login(getUserLogin(), getUserPassword()); + + System.out.println(driver.getPageSource()); + assertEquals(accountPage.buildUri().toASCIIString().replace("master", "consumer") + "/", driver.getCurrentUrl()); + + assertEquals(userCount, adminClient.realm(consumerRealmName()).users().count()); + } + + // KEYCLOAK-3267 + @Test + public void loginWithExistingUserWithBruteForceEnabled() { + adminClient.realm(consumerRealmName()).update(RealmBuilder.create().bruteForceProtected(true).failureFactor(2).build()); + + loginWithExistingUser(); + + driver.navigate().to(getAccountPasswordUrl(consumerRealmName())); + + accountPasswordPage.changePassword("password", "password"); + + driver.navigate().to(getAuthRoot() + + "/auth/realms/" + providerRealmName() + + "/protocol/" + "openid-connect" + + "/logout"); + + driver.navigate().to(getAccountUrl(consumerRealmName())); + + accountLoginPage.login(getUserLogin(), "invalid"); + accountLoginPage.login(getUserLogin(), "invalid"); + accountLoginPage.login(getUserLogin(), "invalid"); + + assertEquals("Invalid username or password.", accountLoginPage.getError()); + + accountLoginPage.clickSocial(getIDPAlias()); + + Assert.assertTrue("Driver should be on the provider realm page right now", driver.getCurrentUrl().contains("/auth/realms/" + providerRealmName() + "/")); + + accountLoginPage.login(getUserLogin(), getUserPassword()); + + assertEquals("Account is disabled, contact admin.", errorPage.getError()); + } + protected void testSingleLogout() { log.debug("Testing single log out"); @@ -203,4 +270,8 @@ public abstract class AbstractBrokerTest extends AbstractKeycloakTest { private String getAccountUrl(String realmName) { return getAuthRoot() + "/auth/realms/" + realmName + "/account"; } + + private String getAccountPasswordUrl(String realmName) { + return getAuthRoot() + "/auth/realms/" + realmName + "/account/password"; + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/RealmBuilder.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/RealmBuilder.java index fe46b589a9..23b100ede3 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/RealmBuilder.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/RealmBuilder.java @@ -137,6 +137,16 @@ public class RealmBuilder { return this; } + public RealmBuilder bruteForceProtected(boolean bruteForceProtected) { + rep.setBruteForceProtected(bruteForceProtected); + return this; + } + + public RealmBuilder failureFactor(int failureFactor) { + rep.setFailureFactor(failureFactor); + return this; + } + public RealmBuilder otpDigits(int i) { rep.setOtpPolicyDigits(i); return this;