Invalidate session secretly

Signed-off-by: Douglas Palmer <[email protected]>
tnorimat · Jan 19, 2024 · e7d842e · e7d842e
1 parent 18d0105
commit e7d842e
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 10 deletions.
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
@@ -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/...org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java b/...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/...rc/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java b/...rc/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/...tion-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java b/...tion-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) {