diff --git a/server-spi-private/src/main/java/org/keycloak/events/Errors.java b/server-spi-private/src/main/java/org/keycloak/events/Errors.java index 531c7ff6d4cd..f0d513e564d4 100755 --- a/server-spi-private/src/main/java/org/keycloak/events/Errors.java +++ b/server-spi-private/src/main/java/org/keycloak/events/Errors.java @@ -38,6 +38,7 @@ public interface Errors { String USER_DISABLED = "user_disabled"; String USER_TEMPORARILY_DISABLED = "user_temporarily_disabled"; String INVALID_USER_CREDENTIALS = "invalid_user_credentials"; + String INVALID_AUTHENTICATION_SESSION = "invalid_authentication_session"; String DIFFERENT_USER_AUTHENTICATING = "different_user_authenticating"; String DIFFERENT_USER_AUTHENTICATED = "different_user_authenticated"; String USER_DELETE_ERROR = "user_delete_error"; diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java index 481df3f3b172..cbd703747a90 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java @@ -53,6 +53,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth public static final String REGISTRATION_FORM_ACTION = "registration_form"; public static final String ATTEMPTED_USERNAME = "ATTEMPTED_USERNAME"; + public static final String SESSION_INVALID = "SESSION_INVALID"; // Flag is true if user was already set in the authContext before this authenticator was triggered. In this case we skip clearing of the user after unsuccessful password authentication protected static final String USER_SET_BEFORE_USERNAME_PASSWORD_AUTH = "USER_SET_BEFORE_USERNAME_PASSWORD_AUTH"; diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java index 37649c7cf6c9..5df0216740df 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java @@ -35,7 +35,6 @@ import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserModel; import org.keycloak.models.credential.OTPCredentialModel; -import org.keycloak.services.managers.AuthenticationSessionManager; import org.keycloak.services.messages.Messages; import org.keycloak.services.validation.Validation; import org.keycloak.sessions.AuthenticationSessionModel; @@ -88,9 +87,15 @@ public void validateOTP(AuthenticationFlowContext context) { context.form().setAttribute(SELECTED_OTP_CREDENTIAL_ID, credentialId); UserModel userModel = context.getUser(); + if("true".equals(context.getAuthenticationSession().getAuthNote(AbstractUsernameFormAuthenticator.SESSION_INVALID))) { + context.getEvent().user(context.getUser()).error(Errors.INVALID_AUTHENTICATION_SESSION); + Response challengeResponse = challenge(context, Messages.INVALID_TOTP, Validation.FIELD_OTP_CODE); + context.forceChallenge(challengeResponse); + return; + } if (!enabledUser(context, userModel)) { // error in context is set in enabledUser/isDisabledByBruteForce - new AuthenticationSessionManager(context.getSession()).removeAuthenticationSession(context.getRealm(), context.getAuthenticationSession(), true); + context.getAuthenticationSession().setAuthNote(AbstractUsernameFormAuthenticator.SESSION_INVALID, "true"); return; } 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 dca0a3a755b6..d7f27ae09811 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 @@ -212,7 +212,7 @@ public void testGrantInvalidPassword() throws Exception { Assert.assertNotNull(response.getError()); Assert.assertEquals("invalid_grant", response.getError()); Assert.assertEquals("Invalid user credentials", response.getErrorDescription()); - assertUserDisabledEvent(); + assertUserDisabledEvent(Errors.USER_TEMPORARILY_DISABLED); events.clear(); } clearUserFailures(); @@ -256,7 +256,7 @@ public void testGrantInvalidOtp() throws Exception { Assert.assertNotNull(response.getError()); Assert.assertEquals(response.getError(), "invalid_grant"); Assert.assertEquals("Invalid user credentials", response.getErrorDescription()); - assertUserDisabledEvent(); + assertUserDisabledEvent(Errors.USER_TEMPORARILY_DISABLED); events.clear(); } clearUserFailures(); @@ -304,7 +304,7 @@ public void testGrantMissingOtp() throws Exception { Assert.assertNotNull(response.getError()); Assert.assertEquals(response.getError(), "invalid_grant"); Assert.assertEquals("Invalid user credentials", response.getErrorDescription()); - assertUserDisabledEvent(); + assertUserDisabledEvent(Errors.USER_TEMPORARILY_DISABLED); events.clear(); } clearUserFailures(); @@ -466,11 +466,14 @@ public void testBrowserMissingTotp() throws Exception { } @Test - public void testBrowserTotpSessionClosedAfterLockout() throws Exception { + public void testBrowserTotpSessionInvalidAfterLockout() throws Exception { long start = System.currentTimeMillis(); loginWithTotpFailure(); continueLoginWithInvalidTotp(); - loginPage.assertCurrent(); + continueLoginWithInvalidTotp(); + events.clear(); + continueLoginWithInvalidTotp(); + assertUserDisabledEvent(Errors.INVALID_AUTHENTICATION_SESSION); } @Test @@ -724,7 +727,6 @@ public void continueLoginWithInvalidTotp() { loginTotpPage.assertCurrent(); Assert.assertEquals("Invalid authenticator code.", loginTotpPage.getInputError()); - events.clear(); } public void continueLoginWithMissingTotp() { @@ -793,8 +795,8 @@ public void registerUser(String username) { events.clear(); } - private void assertUserDisabledEvent() { - events.expect(EventType.LOGIN_ERROR).error(Errors.USER_TEMPORARILY_DISABLED).assertEvent(); + private void assertUserDisabledEvent(String error) { + events.expect(EventType.LOGIN_ERROR).error(error).assertEvent(); } private void assertUserDisabledReason(String expected) {