diff --git a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java index 6f00c23c1f..817b15d483 100755 --- a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java +++ b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java @@ -639,15 +639,9 @@ public class AuthenticationProcessor { public void logFailure() { if (realm.isBruteForceProtected()) { - String username = authenticationSession.getAuthNote(AbstractUsernameFormAuthenticator.ATTEMPTED_USERNAME); - // todo need to handle non form failures - if (username == null) { - - } else { - UserModel user = KeycloakModelUtils.findUserByNameOrEmail(session, realm, username); - if (user != null) { - getBruteForceProtector().failedLogin(realm, user, connection); - } + UserModel user = AuthenticationManager.lookupUserForBruteForceLog(session, realm, authenticationSession); + if (user != null) { + getBruteForceProtector().failedLogin(realm, user, connection); } } } diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java index a19b2efc05..1c604888f2 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -1316,16 +1316,8 @@ public class AuthenticationManager { protected static void logSuccess(KeycloakSession session, AuthenticationSessionModel authSession) { RealmModel realm = session.getContext().getRealm(); - if (realm.isBruteForceProtected()) { - String username = authSession.getAuthNote(AbstractUsernameFormAuthenticator.ATTEMPTED_USERNAME); - // TODO: as above, need to handle non form success - - if(username == null) { - return; - } - - UserModel user = KeycloakModelUtils.findUserByNameOrEmail(session, realm, username); + UserModel user = lookupUserForBruteForceLog(session, realm, authSession); if (user != null) { BruteForceProtector bruteForceProtector = session.getProvider(BruteForceProtector.class); bruteForceProtector.successfulLogin(realm, user, session.getContext().getConnection()); @@ -1333,4 +1325,16 @@ public class AuthenticationManager { } } + public static UserModel lookupUserForBruteForceLog(KeycloakSession session, RealmModel realm, AuthenticationSessionModel authenticationSession) { + UserModel user = authenticationSession.getAuthenticatedUser(); + if (user != null) return user; + + String username = authenticationSession.getAuthNote(AbstractUsernameFormAuthenticator.ATTEMPTED_USERNAME); + if (username != null) { + return KeycloakModelUtils.findUserByNameOrEmail(session, realm, username); + } + + return null; + } + } 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 9b0da15957..35dd083bcb 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 @@ -28,6 +28,7 @@ import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.services.Urls; import org.keycloak.storage.UserStorageProvider; import org.keycloak.testsuite.Assert; +import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.federation.DummyUserFederationProviderFactory; import org.keycloak.testsuite.util.ClientBuilder; import org.keycloak.testsuite.util.OAuthClient; @@ -386,13 +387,7 @@ public abstract class AbstractAdvancedBrokerTest extends AbstractBrokerTest { driver.navigate().to(getAccountUrl(bc.consumerRealmName())); - log.debug("Clicking social " + bc.getIDPAlias()); - loginPage.clickSocial(bc.getIDPAlias()); - waitForPage(driver, "log in to", true); - Assert.assertTrue("Driver should be on the provider realm page right now", - driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/")); - log.debug("Logging in"); - loginPage.login(bc.getUserLogin(), bc.getUserPassword()); + logInWithBroker(bc); totpPage.assertCurrent(); String totpSecret = totpPage.getTotpSecret(); @@ -401,30 +396,77 @@ public abstract class AbstractAdvancedBrokerTest extends AbstractBrokerTest { assertNumFederatedIdentities(realm.users().search(bc.getUserLogin()).get(0).getId(), 1); logoutFromRealm(bc.consumerRealmName()); - log.debug("Clicking social " + bc.getIDPAlias()); - loginPage.clickSocial(bc.getIDPAlias()); - waitForPage(driver, "log in to", true); - Assert.assertTrue("Driver should be on the provider realm page right now", - driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/")); - log.debug("Logging in"); - loginPage.login(bc.getUserLogin(), bc.getUserPassword()); + logInWithBroker(bc); loginTotpPage.assertCurrent(); loginTotpPage.login(totp.generateTOTP(totpSecret)); logoutFromRealm(bc.consumerRealmName()); testingClient.server(bc.consumerRealmName()).run(disablePostBrokerLoginFlow(bc.getIDPAlias())); - log.debug("Clicking social " + bc.getIDPAlias()); - loginPage.clickSocial(bc.getIDPAlias()); - waitForPage(driver, "log in to", true); - Assert.assertTrue("Driver should be on the provider realm page right now", - driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/")); - log.debug("Logging in"); - loginPage.login(bc.getUserLogin(), bc.getUserPassword()); + logInWithBroker(bc); waitForAccountManagementTitle(); accountUpdateProfilePage.assertCurrent(); } + // KEYCLOAK-12986 + @Test + public void testPostBrokerLoginFlowWithOTP_bruteForceEnabled() { + updateExecutions(AbstractBrokerTest::disableUpdateProfileOnFirstLogin); + testingClient.server(bc.consumerRealmName()).run(configurePostBrokerLoginWithOTP(bc.getIDPAlias())); + + // Enable brute force protector in cosumer realm + RealmResource realm = adminClient.realm(bc.consumerRealmName()); + RealmRepresentation consumerRealmRep = realm.toRepresentation(); + consumerRealmRep.setBruteForceProtected(true); + consumerRealmRep.setFailureFactor(2); + consumerRealmRep.setMaxDeltaTimeSeconds(20); + consumerRealmRep.setMaxFailureWaitSeconds(100); + consumerRealmRep.setWaitIncrementSeconds(5); + realm.update(consumerRealmRep); + + try { + driver.navigate().to(getAccountUrl(bc.consumerRealmName())); + + logInWithBroker(bc); + + totpPage.assertCurrent(); + String totpSecret = totpPage.getTotpSecret(); + totpPage.configure(totp.generateTOTP(totpSecret)); + assertNumFederatedIdentities(realm.users().search(bc.getUserLogin()).get(0).getId(), 1); + logoutFromRealm(bc.consumerRealmName()); + + logInWithBroker(bc); + + loginTotpPage.assertCurrent(); + + // Login for 2 times with incorrect TOTP. This should temporarily disable the user + loginTotpPage.login("bad-totp"); + Assert.assertEquals("Invalid authenticator code.", loginTotpPage.getError()); + + loginTotpPage.login("bad-totp"); + Assert.assertEquals("Invalid authenticator code.", loginTotpPage.getError()); + + // Login with valid TOTP. I should not be able to login + loginTotpPage.login(totp.generateTOTP(totpSecret)); + Assert.assertEquals("Invalid authenticator code.", loginTotpPage.getError()); + + // Clear login failures + String userId = ApiUtil.findUserByUsername(realm, bc.getUserLogin()).getId(); + realm.attackDetection().clearBruteForceForUser(userId); + + loginTotpPage.login(totp.generateTOTP(totpSecret)); + waitForAccountManagementTitle(); + logoutFromRealm(bc.consumerRealmName()); + } finally { + testingClient.server(bc.consumerRealmName()).run(disablePostBrokerLoginFlow(bc.getIDPAlias())); + + // Disable brute force protector + consumerRealmRep = realm.toRepresentation(); + consumerRealmRep.setBruteForceProtected(false); + realm.update(consumerRealmRep); + } + } + /** * Refers to in old testsuite: org.keycloak.testsuite.broker.OIDCKeyCloakServerBrokerBasicTest#testLogoutWorksWithTokenTimeout() */