KEYCLOAK-6011

This commit is contained in:
Bruno Oliveira 2017-12-19 18:02:39 -02:00 committed by Stian Thorgersen
parent ae573f4814
commit 811cd3a04a
3 changed files with 35 additions and 15 deletions

View file

@ -22,7 +22,6 @@ import org.keycloak.authentication.AbstractFormAuthenticator;
import org.keycloak.authentication.AuthenticationFlowContext; import org.keycloak.authentication.AuthenticationFlowContext;
import org.keycloak.authentication.AuthenticationFlowError; import org.keycloak.authentication.AuthenticationFlowError;
import org.keycloak.credential.CredentialInput; import org.keycloak.credential.CredentialInput;
import org.keycloak.credential.CredentialModel;
import org.keycloak.credential.hash.PasswordHashProvider; import org.keycloak.credential.hash.PasswordHashProvider;
import org.keycloak.events.Details; import org.keycloak.events.Details;
import org.keycloak.events.Errors; import org.keycloak.events.Errors;
@ -130,17 +129,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
context.forceChallenge(challengeResponse); context.forceChallenge(challengeResponse);
return false; return false;
} }
if (context.getRealm().isBruteForceProtected()) { if (isTemporarilyDisabledByBruteForce(context, user)) return false;
if (context.getProtector().isTemporarilyDisabled(context.getSession(), context.getRealm(), user)) {
context.getEvent().user(user);
context.getEvent().error(Errors.USER_TEMPORARILY_DISABLED);
Response challengeResponse = temporarilyDisabledUser(context);
// this is not a failure so don't call failureChallenge.
//context.failureChallenge(AuthenticationFlowError.USER_TEMPORARILY_DISABLED, challengeResponse);
context.forceChallenge(challengeResponse);
return false;
}
}
return true; return true;
} }
@ -203,6 +192,9 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
List<CredentialInput> credentials = new LinkedList<>(); List<CredentialInput> credentials = new LinkedList<>();
String password = inputData.getFirst(CredentialRepresentation.PASSWORD); String password = inputData.getFirst(CredentialRepresentation.PASSWORD);
credentials.add(UserCredentialModel.password(password)); credentials.add(UserCredentialModel.password(password));
if (isTemporarilyDisabledByBruteForce(context, user)) return false;
if (password != null && !password.isEmpty() && context.getSession().userCredentialManager().isValid(context.getRealm(), user, credentials)) { if (password != null && !password.isEmpty() && context.getSession().userCredentialManager().isValid(context.getRealm(), user, credentials)) {
return true; return true;
} else { } else {
@ -215,4 +207,19 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
} }
} }
private boolean isTemporarilyDisabledByBruteForce(AuthenticationFlowContext context, UserModel user) {
if (context.getRealm().isBruteForceProtected()) {
if (context.getProtector().isTemporarilyDisabled(context.getSession(), context.getRealm(), user)) {
context.getEvent().user(user);
context.getEvent().error(Errors.USER_TEMPORARILY_DISABLED);
Response challengeResponse = temporarilyDisabledUser(context);
// this is not a failure so don't call failureChallenge.
//context.failureChallenge(AuthenticationFlowError.USER_TEMPORARILY_DISABLED, challengeResponse);
context.forceChallenge(challengeResponse);
return true;
}
}
return false;
}
} }

View file

@ -63,7 +63,7 @@ public class AttackDetectionResourceTest extends AbstractAdminTest {
oauthClient.doLogin("test-user2", "invalid"); oauthClient.doLogin("test-user2", "invalid");
oauthClient.doLogin("nosuchuser", "invalid"); oauthClient.doLogin("nosuchuser", "invalid");
assertBruteForce(detection.bruteForceUserStatus(findUser("test-user@localhost").getId()), 3, true, true); assertBruteForce(detection.bruteForceUserStatus(findUser("test-user@localhost").getId()), 2, true, true);
assertBruteForce(detection.bruteForceUserStatus(findUser("test-user2").getId()), 2, true, true); assertBruteForce(detection.bruteForceUserStatus(findUser("test-user2").getId()), 2, true, true);
assertBruteForce(detection.bruteForceUserStatus("nosuchuser"), 0, false, false); assertBruteForce(detection.bruteForceUserStatus("nosuchuser"), 0, false, false);

View file

@ -311,11 +311,13 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
loginInvalidPassword(); loginInvalidPassword();
loginInvalidPassword(); loginInvalidPassword();
expectTemporarilyDisabled(); expectTemporarilyDisabled();
expectTemporarilyDisabled("test-user@localhost", null, "invalid");
clearUserFailures(); clearUserFailures();
loginSuccess(); loginSuccess();
loginInvalidPassword(); loginInvalidPassword();
loginInvalidPassword(); loginInvalidPassword();
expectTemporarilyDisabled(); expectTemporarilyDisabled();
expectTemporarilyDisabled("test-user@localhost", null, "invalid");
clearAllUserFailures(); clearAllUserFailures();
loginSuccess(); loginSuccess();
} }
@ -326,6 +328,8 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
loginInvalidPassword(); loginInvalidPassword();
loginInvalidPassword(); loginInvalidPassword();
expectTemporarilyDisabled(); expectTemporarilyDisabled();
expectTemporarilyDisabled("test-user@localhost", null, "invalid");
// KEYCLOAK-5420 // KEYCLOAK-5420
// Test to make sure that temporarily disabled doesn't increment failure count // Test to make sure that temporarily disabled doesn't increment failure count
testingClient.testing().setTimeOffset(Collections.singletonMap("offset", String.valueOf(6))); testingClient.testing().setTimeOffset(Collections.singletonMap("offset", String.valueOf(6)));
@ -343,6 +347,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
loginInvalidPassword("test-User@localhost"); loginInvalidPassword("test-User@localhost");
loginInvalidPassword("Test-user@localhost"); loginInvalidPassword("Test-user@localhost");
expectTemporarilyDisabled(); expectTemporarilyDisabled();
expectTemporarilyDisabled("test-user@localhost", null, "invalid");
clearAllUserFailures(); clearAllUserFailures();
} }
@ -363,6 +368,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
loginMissingPassword(); loginMissingPassword();
loginMissingPassword(); loginMissingPassword();
expectTemporarilyDisabled(); expectTemporarilyDisabled();
expectTemporarilyDisabled("test-user@localhost", null, "invalid");
clearUserFailures(); clearUserFailures();
loginSuccess(); loginSuccess();
} }
@ -373,6 +379,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
loginWithTotpFailure(); loginWithTotpFailure();
loginWithTotpFailure(); loginWithTotpFailure();
expectTemporarilyDisabled(); expectTemporarilyDisabled();
expectTemporarilyDisabled("test-user@localhost", null, "invalid");
clearUserFailures(); clearUserFailures();
loginSuccess(); loginSuccess();
} }
@ -383,6 +390,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
loginWithMissingTotp(); loginWithMissingTotp();
loginWithMissingTotp(); loginWithMissingTotp();
expectTemporarilyDisabled(); expectTemporarilyDisabled();
expectTemporarilyDisabled("test-user@localhost", null, "invalid");
clearUserFailures(); clearUserFailures();
loginSuccess(); loginSuccess();
} }
@ -454,6 +462,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
loginInvalidPassword(); loginInvalidPassword();
loginInvalidPassword(); loginInvalidPassword();
expectTemporarilyDisabled(); expectTemporarilyDisabled();
expectTemporarilyDisabled("test-user@localhost", null, "invalid");
loginPage.resetPassword(); loginPage.resetPassword();
resetPasswordPage.assertCurrent(); resetPasswordPage.assertCurrent();
@ -468,12 +477,16 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
} }
public void expectTemporarilyDisabled() throws Exception { public void expectTemporarilyDisabled() throws Exception {
expectTemporarilyDisabled("test-user@localhost", null); expectTemporarilyDisabled("test-user@localhost", null, "password");
} }
public void expectTemporarilyDisabled(String username, String userId) throws Exception { public void expectTemporarilyDisabled(String username, String userId) throws Exception {
expectTemporarilyDisabled(username, userId, "password");
}
public void expectTemporarilyDisabled(String username, String userId, String password) throws Exception {
loginPage.open(); loginPage.open();
loginPage.login(username, "password"); loginPage.login(username, password);
loginPage.assertCurrent(); loginPage.assertCurrent();
String src = driver.getPageSource(); String src = driver.getPageSource();