libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

Avoid NPE while fetching offline sessions (#17577)

Browse source
This commit is contained in:
mkrueger92 2023-05-18 13:32:02 +02:00 committed by GitHub
parent 41cf72d57f
commit 256bb84cc4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 70 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java
View file

@ -382,7 +382,8 @@ public class InfinispanUserSessionProvider implements UserSessionProvider {
// and then mapped locally to avoid serialization issues when trying to manipulate the cache stream directly.
return StreamSupport.stream(cache.entrySet().stream().filter(predicate).spliterator(), false)
.map(Mappers.userSessionEntity())
.map(entity -> this.wrap(realm, entity, offline));
.map(entity -> this.wrap(realm, entity, offline))
.filter(Objects::nonNull).map(Function.identity());
}
@Override
@ -739,7 +740,17 @@ public class InfinispanUserSessionProvider implements UserSessionProvider {
UserSessionAdapter wrap(RealmModel realm, UserSessionEntity entity, boolean offline) {
InfinispanChangelogBasedTransaction<String, UserSessionEntity> userSessionUpdateTx = getTransaction(offline);
InfinispanChangelogBasedTransaction<UUID, AuthenticatedClientSessionEntity> clientSessionUpdateTx = getClientSessionTransaction(offline);
return entity != null ? new UserSessionAdapter(session, this, userSessionUpdateTx, clientSessionUpdateTx, realm, entity, offline) : null;
if (entity == null) {
return null;
}
UserModel user = session.users().getUserById(realm, entity.getUser());
if (user == null) {
return null;
}
return new UserSessionAdapter(session, user, this, userSessionUpdateTx, clientSessionUpdateTx, realm, entity, offline);
}
AuthenticatedClientSessionAdapter wrap(UserSessionModel userSession, ClientModel client, AuthenticatedClientSessionEntity entity, boolean offline) {

32
model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/UserSessionAdapter.java
View file

@ -60,17 +60,20 @@ public class UserSessionAdapter implements UserSessionModel {
private final RealmModel realm;
private final UserModel user;
private final UserSessionEntity entity;
private final boolean offline;
private SessionPersistenceState persistenceState;
public UserSessionAdapter(KeycloakSession session, InfinispanUserSessionProvider provider,
public UserSessionAdapter(KeycloakSession session, UserModel user, InfinispanUserSessionProvider provider,
InfinispanChangelogBasedTransaction<String, UserSessionEntity> userSessionUpdateTx,
InfinispanChangelogBasedTransaction<UUID, AuthenticatedClientSessionEntity> clientSessionUpdateTx,
RealmModel realm, UserSessionEntity entity, boolean offline) {
this.session = session;
this.user = user;
this.provider = provider;
this.userSessionUpdateTx = userSessionUpdateTx;
this.clientSessionUpdateTx = clientSessionUpdateTx;
@ -138,20 +141,20 @@ public class UserSessionAdapter implements UserSessionModel {
if (removedClientUUIDS.size() >= MINIMUM_INACTIVE_CLIENT_SESSIONS_TO_CLEANUP) {
// Update user session
UserSessionUpdateTask task = new UserSessionUpdateTask() {
@Override
public void runUpdate(UserSessionEntity entity) {
removedClientUUIDS.forEach(entity.getAuthenticatedClientSessions()::remove);
}
};
@Override
public void runUpdate(UserSessionEntity entity) {
removedClientUUIDS.forEach(entity.getAuthenticatedClientSessions()::remove);
}
};
update(task);
}
// do not iterate the removedClientUUIDS and remove the clientSession directly as the addTask can manipulate
// the collection being iterated, and that can lead to unpredictable behaviour (e.g. NPE)
List<UUID> clientSessionUuids = removedClientUUIDS.stream()
.map(entity.getAuthenticatedClientSessions()::get)
.filter(Objects::nonNull)
.collect(Collectors.toList());
.map(entity.getAuthenticatedClientSessions()::get)
.filter(Objects::nonNull)
.collect(Collectors.toList());
clientSessionUuids.forEach(clientSessionId -> this.clientSessionUpdateTx.addTask(clientSessionId, Tasks.removeSync()));
}
@ -174,8 +177,9 @@ public class UserSessionAdapter implements UserSessionModel {
public String getBrokerUserId() {
return entity.getBrokerUserId();
}
public UserModel getUser() {
return session.users().getUserById(realm, entity.getUser());
return this.user;
}
@Override
@ -339,8 +343,12 @@ public class UserSessionAdapter implements UserSessionModel {
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || !(o instanceof UserSessionModel)) return false;
if (this == o) {
return true;
}
if (o == null || !(o instanceof UserSessionModel)) {
return false;
}
UserSessionModel that = (UserSessionModel) o;
return that.getId().equals(getId());

33
model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionAdapter.java
View file

@ -43,8 +43,11 @@ import static org.keycloak.models.map.userSession.SessionExpiration.setUserSessi
*/
public class MapUserSessionAdapter extends AbstractUserSessionModel {
public MapUserSessionAdapter(KeycloakSession session, RealmModel realm, MapUserSessionEntity entity) {
private final UserModel user;
public MapUserSessionAdapter(KeycloakSession session, RealmModel realm, UserModel userModel, MapUserSessionEntity entity) {
super(session, realm, entity);
this.user = userModel;
}
@Override
@ -69,7 +72,7 @@ public class MapUserSessionAdapter extends AbstractUserSessionModel {
@Override
public UserModel getUser() {
return session.users().getUserById(getRealm(), entity.getUserId());
return this.user;
}
@Override
@ -127,11 +130,11 @@ public class MapUserSessionAdapter extends AbstractUserSessionModel {
}
return authenticatedClientSessions
.stream()
.filter(this::filterAndRemoveExpiredClientSessions)
.filter(this::matchingOfflineFlag)
.filter(this::filterAndRemoveClientSessionWithoutClient)
.collect(Collectors.toMap(MapAuthenticatedClientSessionEntity::getClientId, this::clientSessionEntityToModel));
.stream()
.filter(this::filterAndRemoveExpiredClientSessions)
.filter(this::matchingOfflineFlag)
.filter(this::filterAndRemoveClientSessionWithoutClient)
.collect(Collectors.toMap(MapAuthenticatedClientSessionEntity::getClientId, this::clientSessionEntityToModel));
}
private AuthenticatedClientSessionModel clientSessionEntityToModel(MapAuthenticatedClientSessionEntity clientSessionEntity) {
@ -176,7 +179,9 @@ public class MapUserSessionAdapter extends AbstractUserSessionModel {
Boolean isClientSessionOffline = clientSession.isOffline();
// If client session doesn't have offline flag default to false
if (isClientSessionOffline == null) return !isOffline();
if (isClientSessionOffline == null) {
return !isOffline();
}
return isOffline() == isClientSessionOffline;
}
@ -190,6 +195,7 @@ public class MapUserSessionAdapter extends AbstractUserSessionModel {
.map(this::clientSessionEntityToModel)
.orElse(null);
}
@Override
public void removeAuthenticatedClientSessions(Collection<String> removedClientUKS) {
removedClientUKS.forEach(entity::removeAuthenticatedClientSession);
@ -254,8 +260,9 @@ public class MapUserSessionAdapter extends AbstractUserSessionModel {
String correspondingSessionId = entity.getNote(CORRESPONDING_SESSION_ID);
entity.setNotes(new ConcurrentHashMap<>());
if (correspondingSessionId != null)
if (correspondingSessionId != null) {
entity.setNote(CORRESPONDING_SESSION_ID, correspondingSessionId);
}
entity.clearAuthenticatedClientSessions();
}
@ -267,8 +274,12 @@ public class MapUserSessionAdapter extends AbstractUserSessionModel {
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof UserSessionModel)) return false;
if (this == o) {
return true;
}
if (!(o instanceof UserSessionModel)) {
return false;
}
UserSessionModel that = (UserSessionModel) o;
return Objects.equals(that.getId(), getId());

6
model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProvider.java
View file

@ -88,7 +88,11 @@ public class MapUserSessionProvider implements UserSessionProvider {
}
return null;
} else {
return new MapUserSessionAdapter(session, realm, origEntity);
UserModel userModel = session.users().getUserById(realm, origEntity.getUserId());
if (userModel != null) {
return new MapUserSessionAdapter(session, realm, userModel, origEntity);
}
return null;
}
};
}

8
server-spi/src/main/java/org/keycloak/models/UserSessionProvider.java
View file

@ -97,7 +97,7 @@ public interface UserSessionProvider extends Provider {
* Obtains the online user sessions associated with the specified client, starting from the {@code firstResult} and containing
* at most {@code maxResults}.
*
* @param realm a reference tot he realm.
* @param realm a reference to the realm.
* @param client the client whose user sessions are being searched.
* @param firstResult first result to return. Ignored if negative or {@code null}.
* @param maxResults maximum number of results to return. Ignored if negative or {@code null}.
@ -117,8 +117,8 @@ public interface UserSessionProvider extends Provider {
UserSessionModel getUserSessionByBrokerSessionId(RealmModel realm, String brokerSessionId);
/**
* Return userSession of specified ID as long as the predicate passes. Otherwise returns {@code null}.
* If predicate doesn't pass, implementation can do some best-effort actions to try have predicate passing (eg. download userSession from other DC)
* Return userSession of specified ID as long as the predicate passes. Otherwise, returns {@code null}.
* If predicate doesn't pass, implementation can do some best-effort actions to try to have predicate passing (e.g. download userSession from other DC)
*/
UserSessionModel getUserSessionWithPredicate(RealmModel realm, String id, boolean offline, Predicate<UserSessionModel> predicate);
@ -192,7 +192,7 @@ public interface UserSessionProvider extends Provider {
* Obtains the offline user sessions associated with the specified client, starting from the {@code firstResult} and
* containing at most {@code maxResults}.
*
* @param realm a reference tot he realm.
* @param realm a reference to the realm.
* @param client the client whose user sessions are being searched.
* @param firstResult first result to return. Ignored if negative or {@code null}.
* @param maxResults maximum number of results to return. Ignored if negative or {@code null}.

9
services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java
View file

@ -281,13 +281,16 @@ public class AuthenticationManager {
backchannelLogoutResponse.setLocalLogoutSucceeded(true);
return backchannelLogoutResponse;
}
UserModel user = userSession.getUser();
if (userSession.getState() != UserSessionModel.State.LOGGING_OUT) {
userSession.setState(UserSessionModel.State.LOGGING_OUT);
}
logger.debugv("Logging out: {0} ({1}) offline: {2}", user.getUsername(), userSession.getId(),
userSession.isOffline());
if (logger.isDebugEnabled()) {
UserModel user = userSession.getUser();
logger.debugv("Logging out: {0} ({1}) offline: {2}", user.getUsername(), userSession.getId(),
userSession.isOffline());
}
boolean expireUserSessionCookieSucceeded =
expireUserSessionCookie(session, userSession, realm, uriInfo, headers, connection);