Deleted authentication sessions should not be re-surrected with an update

Closes #31829

Signed-off-by: Alexander Schwartz <aschwart@redhat.com>
This commit is contained in:
Alexander Schwartz 2024-08-01 12:57:28 +02:00 committed by Pedro Igor
parent 8e11987341
commit 07a168cb14
3 changed files with 29 additions and 8 deletions

View file

@ -122,11 +122,12 @@ public class InfinispanKeycloakTransaction implements KeycloakTransaction {
Object taskKey = getTaskKey(cache, key); Object taskKey = getTaskKey(cache, key);
CacheTask current = tasks.get(taskKey); CacheTask current = tasks.get(taskKey);
if (current != null && current != TOMBSTONE && current.getOperation() != Operation.REMOVE) { if (current != null) {
if (current instanceof CacheTaskWithValue) { if (current instanceof CacheTaskWithValue) {
((CacheTaskWithValue<V>) current).setValue(value); ((CacheTaskWithValue<V>) current).setValue(value);
((CacheTaskWithValue<V>) current).updateLifespan(lifespan, lifespanUnit); ((CacheTaskWithValue<V>) 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"); throw new IllegalStateException("Can't replace entry: task " + current + " in progress for session");
} }
} else { } else {
@ -152,7 +153,7 @@ public class InfinispanKeycloakTransaction implements KeycloakTransaction {
CacheTask current = tasks.get(taskKey); CacheTask current = tasks.get(taskKey);
if (current != null) { if (current != null) {
if (current instanceof CacheTaskWithValue && ((CacheTaskWithValue<?>) current).getOperation() == Operation.PUT) { if (current instanceof CacheTaskWithValue && current.getOperation() == Operation.PUT) {
tasks.put(taskKey, TOMBSTONE); tasks.put(taskKey, TOMBSTONE);
return; return;
} }

View file

@ -77,10 +77,11 @@ class RemoteInfinispanKeycloakTransaction<K, V, R extends ConditionalRemover<K,
logger.tracef("Adding %s.replace(%S)", cache.getName(), key); logger.tracef("Adding %s.replace(%S)", cache.getName(), key);
Operation<K, V> existing = tasks.get(key); Operation<K, V> existing = tasks.get(key);
if (existing != null && existing != TOMBSTONE && !(existing instanceof RemoteInfinispanKeycloakTransaction.RemoveOperation<K,V>)) { if (existing != null) {
if (existing.hasValue()) { if (existing.hasValue()) {
tasks.put(key, existing.update(value, lifespan, timeUnit)); tasks.put(key, existing.update(value, lifespan, timeUnit));
} else { } else if (!(existing == TOMBSTONE || existing instanceof RemoveOperation<K, V>)) {
// A previous delete operation will take precedence over any new replace
throw new IllegalStateException("Can't replace entry: task " + existing + " in progress for session"); throw new IllegalStateException("Can't replace entry: task " + existing + " in progress for session");
} }
return; return;

View file

@ -23,8 +23,10 @@ import org.junit.Assert;
import org.junit.Rule; import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.keycloak.admin.client.resource.UserResource; import org.keycloak.admin.client.resource.UserResource;
import org.keycloak.cookie.CookieType;
import org.keycloak.events.Details; import org.keycloak.events.Details;
import org.keycloak.events.EventType; import org.keycloak.events.EventType;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel; import org.keycloak.models.UserModel;
import org.keycloak.representations.idm.EventRepresentation; import org.keycloak.representations.idm.EventRepresentation;
import org.keycloak.representations.idm.RealmRepresentation; 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.GreenMailRule;
import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.OAuthClient;
import org.keycloak.testsuite.util.SecondBrowser; import org.keycloak.testsuite.util.SecondBrowser;
import org.openqa.selenium.Cookie;
import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebDriver;
import java.util.List; import java.util.List;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
/** /**
@ -85,8 +90,22 @@ public class AppInitiatedActionResetPasswordTest extends AbstractAppInitiatedAct
changePasswordPage.assertCurrent(); changePasswordPage.assertCurrent();
assertTrue(changePasswordPage.isCancelDisplayed()); 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"); 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(); events.expectRequiredAction(EventType.UPDATE_PASSWORD).assertEvent();
assertKcActionStatus(SUCCESS); assertKcActionStatus(SUCCESS);