Make SessionWrapper related fields immutable that are part of the equals method

The cache replace logic depends on it, as values returned by reference from a local cache must never be modified on those critical fields directly.

Closes #28906

Signed-off-by: Alexander Schwartz <aschwart@redhat.com>
This commit is contained in:
Alexander Schwartz 2024-04-19 08:56:28 +02:00 committed by Alexander Schwartz
parent ee93561706
commit 6cc8d653f3
14 changed files with 32 additions and 63 deletions

View file

@ -76,8 +76,7 @@ public class InfinispanAuthenticationSessionProvider implements AuthenticationSe
@Override
public RootAuthenticationSessionModel createRootAuthenticationSession(RealmModel realm, String id) {
RootAuthenticationSessionEntity entity = new RootAuthenticationSessionEntity();
entity.setId(id);
RootAuthenticationSessionEntity entity = new RootAuthenticationSessionEntity(id);
entity.setRealmId(realm.getId());
entity.setTimestamp(Time.currentTime());

View file

@ -236,8 +236,7 @@ public class InfinispanUserSessionProvider implements UserSessionProvider, Sessi
id = keyGenerator.generateKeyString(session, sessionCache);
}
UserSessionEntity entity = new UserSessionEntity();
entity.setId(id);
UserSessionEntity entity = new UserSessionEntity(id);
updateSessionEntity(entity, realm, user, loginUsername, ipAddress, authMethod, rememberMe, brokerSessionId, brokerUserId);
SessionUpdateTask<UserSessionEntity> createSessionTask = Tasks.addIfAbsentSync();
@ -1042,8 +1041,7 @@ public class InfinispanUserSessionProvider implements UserSessionProvider, Sessi
private UserSessionEntity createUserSessionEntityInstance(UserSessionModel userSession) {
UserSessionEntity entity = new UserSessionEntity();
entity.setId(userSession.getId());
UserSessionEntity entity = new UserSessionEntity(userSession.getId());
entity.setRealmId(userSession.getRealm().getId());
entity.setAuthMethod(userSession.getAuthMethod());

View file

@ -238,8 +238,7 @@ public class PersistentUserSessionProvider implements UserSessionProvider, Sessi
id = keyGenerator.generateKeyString(session, sessionCache);
}
UserSessionEntity entity = new UserSessionEntity();
entity.setId(id);
UserSessionEntity entity = new UserSessionEntity(id);
updateSessionEntity(entity, realm, user, loginUsername, ipAddress, authMethod, rememberMe, brokerSessionId, brokerUserId);
SessionUpdateTask<UserSessionEntity> createSessionTask = Tasks.addIfAbsentSync();
@ -852,8 +851,7 @@ public class PersistentUserSessionProvider implements UserSessionProvider, Sessi
}
private UserSessionEntity createUserSessionEntityInstance(UserSessionModel userSession) {
UserSessionEntity entity = new UserSessionEntity();
entity.setId(userSession.getId());
UserSessionEntity entity = new UserSessionEntity(userSession.getId());
entity.setRealmId(userSession.getRealm().getId());
entity.setAuthMethod(userSession.getAuthMethod());

View file

@ -550,7 +550,7 @@ public class JpaChangesPerformer<K, V extends SessionEntity> implements SessionC
} else {
PersistentUserSessionAdapter userSessionModel = (PersistentUserSessionAdapter) userSessionPersister.loadUserSession(realm, entry.getKey().toString(), entity.isOffline());
if (userSessionModel != null) {
UserSessionEntity userSessionEntity = new UserSessionEntity() {
UserSessionEntity userSessionEntity = new UserSessionEntity(userSessionModel.getId()) {
@Override
public Map<String, String> getNotes() {
return new HashMap<>() {
@ -611,16 +611,6 @@ public class JpaChangesPerformer<K, V extends SessionEntity> implements SessionC
userSessionModel.setRealm(innerSession.realms().getRealm(realmId));
}
@Override
public String getId() {
return userSessionModel.getId();
}
@Override
public void setId(String id) {
throw new IllegalStateException("not supported");
}
@Override
public String getUser() {
return userSessionModel.getUser().getId();

View file

@ -33,11 +33,12 @@ import org.infinispan.commons.marshall.SerializeWith;
@SerializeWith(RootAuthenticationSessionEntity.ExternalizerImpl.class)
public class RootAuthenticationSessionEntity extends SessionEntity {
private String id;
private final String id;
private int timestamp;
private Map<String, AuthenticationSessionEntity> authenticationSessions = new ConcurrentHashMap<>();
public RootAuthenticationSessionEntity() {
public RootAuthenticationSessionEntity(String id) {
this.id = id;
}
protected RootAuthenticationSessionEntity(String realmId, String id, int timestamp, Map<String, AuthenticationSessionEntity> authenticationSessions) {
@ -51,10 +52,6 @@ public class RootAuthenticationSessionEntity extends SessionEntity {
return id;
}
public void setId(String id) {
this.id = id;
}
public int getTimestamp() {
return timestamp;
}

View file

@ -47,7 +47,7 @@ public class UserSessionEntity extends SessionEntity {
// Metadata attribute, which contains the lastSessionRefresh available on remoteCache. Used in decide whether we need to write to remoteCache (DC) or not
public static final String LAST_SESSION_REFRESH_REMOTE = "lsrr";
private String id;
private final String id;
private String user;
@ -68,12 +68,12 @@ public class UserSessionEntity extends SessionEntity {
private UserSessionModel.State state;
public String getId() {
return id;
public UserSessionEntity(String id) {
this.id = id;
}
public void setId(String id) {
this.id = id;
public String getId() {
return id;
}
private Map<String, String> notes = new ConcurrentHashMap<>();
@ -279,13 +279,13 @@ public class UserSessionEntity extends SessionEntity {
}
public UserSessionEntity readObjectVersion1(ObjectInput input) throws IOException, ClassNotFoundException {
UserSessionEntity sessionEntity = new UserSessionEntity();
sessionEntity.setAuthMethod(MarshallUtil.unmarshallString(input));
sessionEntity.setBrokerSessionId(MarshallUtil.unmarshallString(input));
sessionEntity.setBrokerUserId(MarshallUtil.unmarshallString(input));
final String userSessionId = MarshallUtil.unmarshallString(input);
sessionEntity.setId(userSessionId);
String authMethod = MarshallUtil.unmarshallString(input);
String brokerSessionId = MarshallUtil.unmarshallString(input);
String brokerUserId = MarshallUtil.unmarshallString(input);
UserSessionEntity sessionEntity = new UserSessionEntity(MarshallUtil.unmarshallString(input));
sessionEntity.setAuthMethod(authMethod);
sessionEntity.setBrokerSessionId(brokerSessionId);
sessionEntity.setBrokerUserId(brokerUserId);
sessionEntity.setIpAddress(MarshallUtil.unmarshallString(input));
sessionEntity.setLoginUsername(MarshallUtil.unmarshallString(input));
sessionEntity.setRealmId(MarshallUtil.unmarshallString(input));

View file

@ -127,8 +127,7 @@ public class ConcurrencyDistributedRemoveSessionTest {
private static SessionEntityWrapper<UserSessionEntity> createSessionEntity(String sessionId) {
// Create 100 initial sessions
UserSessionEntity session = new UserSessionEntity();
session.setId(sessionId);
UserSessionEntity session = new UserSessionEntity(sessionId);
session.setRealmId("foo");
session.setBrokerSessionId("!23123123");
session.setBrokerUserId(null);

View file

@ -81,8 +81,7 @@ public class ConcurrencyJDGCacheReplaceTest {
Cache<String, SessionEntityWrapper<UserSessionEntity>> cache2 = createManager(2).getCache(InfinispanConnectionProvider.USER_SESSION_CACHE_NAME);
// Create initial item
UserSessionEntity session = new UserSessionEntity();
session.setId("123");
UserSessionEntity session = new UserSessionEntity("123");
session.setRealmId("foo");
session.setBrokerSessionId("!23123123");
session.setBrokerUserId(null);

View file

@ -43,8 +43,7 @@ public class ConcurrencyJDGOfflineBackupsTest {
try {
// Create initial item
UserSessionEntity session = new UserSessionEntity();
session.setId("123");
UserSessionEntity session = new UserSessionEntity("123");
session.setRealmId("foo");
session.setBrokerSessionId("!23123123");
session.setBrokerUserId(null);

View file

@ -188,8 +188,7 @@ public class ConcurrencyJDGRemoveSessionTest {
private static SessionEntityWrapper<UserSessionEntity> createSessionEntity(String sessionId) {
// Create 100 initial sessions
UserSessionEntity session = new UserSessionEntity();
session.setId(sessionId);
UserSessionEntity session = new UserSessionEntity(sessionId);
session.setRealmId("foo");
session.setBrokerSessionId("!23123123");
session.setBrokerUserId(null);

View file

@ -63,8 +63,7 @@ public class RemoteCacheSessionsLoaderTest {
for (int i=0 ; i<COUNT ; i++) {
// Create initial item
UserSessionEntity session = new UserSessionEntity();
session.setId("loader-key-" + i);
UserSessionEntity session = new UserSessionEntity("loader-key-" + i);
session.setRealmId("master");
session.setBrokerSessionId("!23123123");
session.setBrokerUserId(null);

View file

@ -75,8 +75,7 @@ public class DistributedCacheConcurrentWritesTest {
private static SessionEntityWrapper<UserSessionEntity> createEntityInstance(String id) {
// Create initial item
UserSessionEntity session = new UserSessionEntity();
session.setId(id);
UserSessionEntity session = new UserSessionEntity(id);
session.setRealmId("foo");
session.setBrokerSessionId("!23123123");
session.setBrokerUserId(null);

View file

@ -55,8 +55,7 @@ public class DistributedCacheWriteSkewTest {
Cache<String, UserSessionEntity> cache2 = createManager("node2").getCache(InfinispanConnectionProvider.USER_SESSION_CACHE_NAME);
// Create initial item
UserSessionEntity session = new UserSessionEntity();
session.setId("123");
UserSessionEntity session = new UserSessionEntity("123");
session.setRealmId("foo");
session.setBrokerSessionId("!23123123");
session.setBrokerUserId(null);

View file

@ -107,14 +107,11 @@ public abstract class AbstractSessionCacheCommand extends AbstractCommand {
@Override
protected void doRunCacheCommand(KeycloakSession session, Cache<String, SessionEntityWrapper> cache) {
UserSessionEntity userSession = new UserSessionEntity();
String id = getArg(1);
userSession.setId(id);
UserSessionEntity userSession = new UserSessionEntity(getArg(1));
userSession.setRealmId(getArg(2));
userSession.setLastSessionRefresh(Time.currentTime());
cache.put(id, new SessionEntityWrapper(userSession));
cache.put(userSession.getId(), new SessionEntityWrapper(userSession));
}
@Override
@ -294,14 +291,11 @@ public abstract class AbstractSessionCacheCommand extends AbstractCommand {
BatchTaskRunner.runInBatches(0, count, batchCount, session.getKeycloakSessionFactory(), (KeycloakSession batchSession, int firstInIteration, int countInIteration) -> {
for (int i=0 ; i<countInIteration ; i++) {
UserSessionEntity userSession = new UserSessionEntity();
String id = KeycloakModelUtils.generateId();
userSession.setId(id);
UserSessionEntity userSession = new UserSessionEntity(KeycloakModelUtils.generateId());
userSession.setRealmId(realmName);
userSession.setLastSessionRefresh(Time.currentTime());
cache.put(id, new SessionEntityWrapper(userSession));
cache.put(userSession.getId(), new SessionEntityWrapper(userSession));
}
log.infof("Created '%d' sessions started from offset '%d'", countInIteration, firstInIteration);