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 531c7ff6d4..f0d513e564 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 481df3f3b1..cbd703747a 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 37649c7cf6..5df0216740 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.RealmModel; 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 class OTPFormAuthenticator extends AbstractUsernameFormAuthenticator impl 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 dca0a3a755..d7f27ae098 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 class BruteForceTest extends AbstractTestRealmKeycloakTest { 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 class BruteForceTest extends AbstractTestRealmKeycloakTest { 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 class BruteForceTest extends AbstractTestRealmKeycloakTest { 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 class BruteForceTest extends AbstractTestRealmKeycloakTest { } @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 class BruteForceTest extends AbstractTestRealmKeycloakTest { loginTotpPage.assertCurrent(); Assert.assertEquals("Invalid authenticator code.", loginTotpPage.getInputError()); - events.clear(); } public void continueLoginWithMissingTotp() { @@ -793,8 +795,8 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { 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) {