diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanKeycloakTransaction.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanKeycloakTransaction.java index 313799a3b6..6856b9da90 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanKeycloakTransaction.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanKeycloakTransaction.java @@ -122,11 +122,12 @@ public class InfinispanKeycloakTransaction implements KeycloakTransaction { Object taskKey = getTaskKey(cache, key); CacheTask current = tasks.get(taskKey); - if (current != null && current != TOMBSTONE && current.getOperation() != Operation.REMOVE) { + if (current != null) { if (current instanceof CacheTaskWithValue) { ((CacheTaskWithValue) current).setValue(value); ((CacheTaskWithValue) current).updateLifespan(lifespan, lifespanUnit); - } else { + } else if (current != TOMBSTONE && current.getOperation() != Operation.REMOVE) { + // A previous delete operation will take precedence over any new replace throw new IllegalStateException("Can't replace entry: task " + current + " in progress for session"); } } else { @@ -152,7 +153,7 @@ public class InfinispanKeycloakTransaction implements KeycloakTransaction { CacheTask current = tasks.get(taskKey); if (current != null) { - if (current instanceof CacheTaskWithValue && ((CacheTaskWithValue) current).getOperation() == Operation.PUT) { + if (current instanceof CacheTaskWithValue && current.getOperation() == Operation.PUT) { tasks.put(taskKey, TOMBSTONE); return; } diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/remote/transaction/RemoteInfinispanKeycloakTransaction.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/remote/transaction/RemoteInfinispanKeycloakTransaction.java index 7c74f45536..b65ba85812 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/remote/transaction/RemoteInfinispanKeycloakTransaction.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/remote/transaction/RemoteInfinispanKeycloakTransaction.java @@ -77,10 +77,11 @@ class RemoteInfinispanKeycloakTransaction existing = tasks.get(key); - if (existing != null && existing != TOMBSTONE && !(existing instanceof RemoteInfinispanKeycloakTransaction.RemoveOperation)) { + if (existing != null) { if (existing.hasValue()) { tasks.put(key, existing.update(value, lifespan, timeUnit)); - } else { + } else if (!(existing == TOMBSTONE || existing instanceof RemoveOperation)) { + // A previous delete operation will take precedence over any new replace throw new IllegalStateException("Can't replace entry: task " + existing + " in progress for session"); } return; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionResetPasswordTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionResetPasswordTest.java index fb7da92085..001a768184 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionResetPasswordTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionResetPasswordTest.java @@ -23,8 +23,10 @@ import org.junit.Assert; import org.junit.Rule; import org.junit.Test; import org.keycloak.admin.client.resource.UserResource; +import org.keycloak.cookie.CookieType; import org.keycloak.events.Details; import org.keycloak.events.EventType; +import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.representations.idm.EventRepresentation; import org.keycloak.representations.idm.RealmRepresentation; @@ -35,12 +37,15 @@ import org.keycloak.testsuite.pages.LoginPasswordUpdatePage; import org.keycloak.testsuite.util.GreenMailRule; import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.SecondBrowser; +import org.openqa.selenium.Cookie; import org.openqa.selenium.WebDriver; import java.util.List; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; /** @@ -85,8 +90,22 @@ public class AppInitiatedActionResetPasswordTest extends AbstractAppInitiatedAct changePasswordPage.assertCurrent(); assertTrue(changePasswordPage.isCancelDisplayed()); + Cookie authSessionCookie = driver.manage().getCookieNamed(CookieType.AUTH_SESSION_ID.getName()); + String authSessionId = authSessionCookie.getValue().split("\\.")[0]; + testingClient.server().run(session -> { + // ensure that our logic to detect the authentication session works as expected + RealmModel realm = session.realms().getRealm(TEST_REALM_NAME); + assertNotNull(session.authenticationSessions().getRootAuthenticationSession(realm, authSessionId)); + }); + changePasswordPage.changePassword("new-password", "new-password"); + testingClient.server().run(session -> { + // ensure that the authentication session has been terminated + RealmModel realm = session.realms().getRealm(TEST_REALM_NAME); + assertNull(session.authenticationSessions().getRootAuthenticationSession(realm, authSessionId)); + }); + events.expectRequiredAction(EventType.UPDATE_PASSWORD).assertEvent(); assertKcActionStatus(SUCCESS); @@ -180,12 +199,12 @@ public class AppInitiatedActionResetPasswordTest extends AbstractAppInitiatedAct @Test public void cancelChangePassword() throws Exception { doAIA(); - + loginPage.login("test-user@localhost", "password"); - + changePasswordPage.assertCurrent(); changePasswordPage.cancel(); - + assertKcActionStatus(CANCELLED); }