From 0d5d7b38946ce1df4d9eb13839fec8baf37f56f9 Mon Sep 17 00:00:00 2001 From: anhefti Date: Wed, 22 Nov 2023 12:12:53 +0100 Subject: [PATCH] SEBSERV-475 implemented --- .../gbl/model/session/ClientConnection.java | 14 +-- .../dao/impl/ClientConnectionDAOImpl.java | 4 +- .../session/ExamSessionService.java | 4 +- .../session/SEBClientSessionService.java | 2 +- .../impl/SEBClientConnectionServiceImpl.java | 101 ++++++++++++------ .../impl/SEBClientEventBatchService.java | 2 +- 6 files changed, 81 insertions(+), 46 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/model/session/ClientConnection.java b/src/main/java/ch/ethz/seb/sebserver/gbl/model/session/ClientConnection.java index 23157cd2..9fb3c3c4 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/model/session/ClientConnection.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/model/session/ClientConnection.java @@ -131,7 +131,7 @@ public final class ClientConnection implements GrantEntity { @JsonIgnore public final Long remoteProctoringRoomId; @JsonIgnore - public final String virtualClientId; + public final String sebClientUserId; @JsonIgnore public final Long creationTime; @JsonIgnore @@ -170,7 +170,7 @@ public final class ClientConnection implements GrantEntity { this.userSessionId = userSessionId; this.info = info; this.vdi = false; - this.virtualClientId = null; + this.sebClientUserId = null; this.vdiPairToken = null; this.creationTime = 0L; this.updateTime = 0L; @@ -199,7 +199,7 @@ public final class ClientConnection implements GrantEntity { final String seb_os_name, final String seb_machine_name, final String seb_version, - final String virtualClientId, + final String sebClientUserId, final Boolean vdi, final String vdiPairToken, final Long creationTime, @@ -222,7 +222,7 @@ public final class ClientConnection implements GrantEntity { this.sebOSName = seb_os_name; this.sebMachineName = seb_machine_name; this.sebVersion = seb_version; - this.virtualClientId = virtualClientId; + this.sebClientUserId = sebClientUserId; this.vdi = vdi; this.vdiPairToken = vdiPairToken; this.creationTime = creationTime; @@ -296,8 +296,8 @@ public final class ClientConnection implements GrantEntity { } @JsonIgnore - public String getVirtualClientId() { - return this.virtualClientId; + public String getSebClientUserId() { + return this.sebClientUserId; } @JsonIgnore // not used yet on GUI side @@ -441,7 +441,7 @@ public final class ClientConnection implements GrantEntity { builder.append(", remoteProctoringRoomId="); builder.append(this.remoteProctoringRoomId); builder.append(", virtualClientId="); - builder.append(this.virtualClientId); + builder.append(this.sebClientUserId); builder.append(", creationTime="); builder.append(this.creationTime); builder.append(", updateTime="); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientConnectionDAOImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientConnectionDAOImpl.java index 32cdc4fa..545b07a4 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientConnectionDAOImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientConnectionDAOImpl.java @@ -366,7 +366,7 @@ public class ClientConnectionDAOImpl implements ClientConnectionDAO { data.connectionToken, null, data.clientAddress, - data.virtualClientId, + data.sebClientUserId, BooleanUtils.toInteger(data.vdi, 1, 0, 0), data.vdiPairToken, millisecondsNow, @@ -408,7 +408,7 @@ public class ClientConnectionDAOImpl implements ClientConnectionDAO { null, data.userSessionId, data.clientAddress, - data.virtualClientId, + data.sebClientUserId, BooleanUtils.toInteger(data.vdi, 1, 0, 0), data.vdiPairToken, null, diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/ExamSessionService.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/ExamSessionService.java index 5e391601..7d181e16 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/ExamSessionService.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/ExamSessionService.java @@ -84,7 +84,7 @@ public interface ExamSessionService { Result> checkExamConsistency(Long examId); /** Use this to check if a specified Exam has currently active SEB Client connections. - * + *

* Active SEB Client connections are established connections that are not yet closed and * open connection attempts. * @@ -163,7 +163,7 @@ public interface ExamSessionService { OutputStream out); /** Get current ClientConnectionData for a specified active SEB client connection. - * + *

* active SEB client connections are connections that were initialized by a SEB client * on the particular server instance. * diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/SEBClientSessionService.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/SEBClientSessionService.java index 3253e51e..30adf21b 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/SEBClientSessionService.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/SEBClientSessionService.java @@ -57,7 +57,7 @@ public interface SEBClientSessionService extends ExamUpdateTask, SessionUpdateTa /** Notify a SEB client event for live indication and storing to database. * * @param connectionToken the connection token - * @param event The SEB client event data */ + * @param jsonBody The SEB client event JSON data */ void notifyClientEvent(String connectionToken, String jsonBody); /** This is used to confirm SEB instructions that must be confirmed by the SEB client. diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientConnectionServiceImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientConnectionServiceImpl.java index c92ae347..e51be127 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientConnectionServiceImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientConnectionServiceImpl.java @@ -10,11 +10,15 @@ package ch.ethz.seb.sebserver.webservice.servicelayer.session.impl; import java.security.Principal; import java.util.Collection; +import java.util.Objects; import java.util.UUID; import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; +import ch.ethz.seb.sebserver.gbl.model.Entity; +import ch.ethz.seb.sebserver.gbl.model.session.ClientEvent; +import ch.ethz.seb.sebserver.gbl.util.Utils; import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; @@ -67,6 +71,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic private final ExamAdminService examAdminService; private final DistributedIndicatorValueService distributedPingCache; private final SecurityKeyService securityKeyService; + private final SEBClientEventBatchService sebClientEventBatchService; private final boolean isDistributedSetup; protected SEBClientConnectionServiceImpl( @@ -76,7 +81,8 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic final DistributedIndicatorValueService distributedPingCache, final ClientIndicatorFactory clientIndicatorFactory, final SecurityKeyService securityKeyService, - final WebserviceInfo webserviceInfo) { + final WebserviceInfo webserviceInfo, + final SEBClientEventBatchService sebClientEventBatchService) { this.examSessionService = examSessionService; this.examSessionCacheService = examSessionService.getExamSessionCacheService(); @@ -87,6 +93,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic this.distributedPingCache = distributedPingCache; this.securityKeyService = securityKeyService; this.isDistributedSetup = webserviceInfo.isDistributed(); + this.sebClientEventBatchService = sebClientEventBatchService; } @Override @@ -113,7 +120,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic if (clientConfig == null) { log.error("Illegal client connection request: requested connection config name: {}", - principal.getName()); + (principal != null) ? principal.getName() : clientAddress); throw new AccessDeniedException("Unknown or illegal client access"); } @@ -226,12 +233,12 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic if (StringUtils.isNoneBlank(clientAddress) && StringUtils.isNotBlank(clientConnection.clientAddress) && !clientAddress.equals(clientConnection.clientAddress)) { + // log SEB client IP address change log.error( "ClientConnection integrity violation: client address mismatch: {}, {}", clientAddress, clientConnection.clientAddress); - throw new IllegalArgumentException( - "ClientConnection integrity violation: client address mismatch"); + sebLogClientAddressMismatch(clientAddress, clientConnection); } if (examId != null) { @@ -327,26 +334,26 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic ClientConnection clientConnection = getClientConnection(connectionToken); - // connection integrity check - if (clientConnection.status == ConnectionStatus.ACTIVE) { - // connection already established. Check if IP is the same - if (StringUtils.isNoneBlank(clientAddress) && - StringUtils.isNotBlank(clientConnection.clientAddress) && - !clientAddress.equals(clientConnection.clientAddress)) { - log.warn( - "ClientConnection integrity violation: client address mismatch: {}, {}", - clientAddress, - clientConnection.clientAddress); - throw new IllegalArgumentException( - "ClientConnection integrity violation: client address mismatch"); - } - } else if (!clientConnection.status.clientActiveStatus) { + // overall connection status integrity check + if (!clientConnection.status.clientActiveStatus) { log.warn("ClientConnection integrity violation: client connection is not in expected state: {}", clientConnection); throw new IllegalArgumentException( "ClientConnection integrity violation: client connection is not in expected state"); } + // check IP address change + if (StringUtils.isNoneBlank(clientAddress) && + StringUtils.isNotBlank(clientConnection.clientAddress) && + !clientAddress.equals(clientConnection.clientAddress)) { + // log client IP address change + log.warn( + "ClientConnection integrity violation: client address mismatch: {}, {}", + clientAddress, + clientConnection.clientAddress); + sebLogClientAddressMismatch(clientAddress, clientConnection); + } + if (log.isDebugEnabled()) { log.debug( "SEB client connection, establish ClientConnection for " @@ -391,7 +398,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic final Long currentExamId = (examId != null) ? examId : clientConnection.examId; final String currentVdiConnectionId = (clientId != null) ? clientId - : clientConnection.virtualClientId; + : clientConnection.sebClientUserId; // create new ClientConnection for update final ClientConnection establishedClientConnection = new ClientConnection( @@ -438,12 +445,13 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic throw new IllegalStateException("ClientConnection integrity violation"); } - final ClientConnection connectionToSave = handleVDISetup( - currentVdiConnectionId, - establishedClientConnection); - +// Removed this since VDI integration was postponed and has not been reactivated since then. +// final ClientConnection connectionToSave = handleVDISetup( +// currentVdiConnectionId, +// establishedClientConnection); +// final ClientConnection updatedClientConnection = this.clientConnectionDAO - .save(connectionToSave) + .save(establishedClientConnection) .getOrThrow(); // check exam integrity for established connection @@ -467,13 +475,15 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic log.warn("Failed to load ClientConnectionDataInternal into cache on update"); } else if (log.isDebugEnabled()) { log.debug("SEB client connection, successfully established ClientConnection: {}", - updatedClientConnection); + establishedClientConnection); } - return updatedClientConnection; + return establishedClientConnection; }); } + + private ClientConnection handleVDISetup( final String currentVdiConnectionId, final ClientConnection establishedClientConnection) { @@ -485,7 +495,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic final Result vdiPairConnectionResult = this.clientConnectionDAO.getVDIPairCompanion( establishedClientConnection.examId, - establishedClientConnection.virtualClientId); + establishedClientConnection.sebClientUserId); if (!vdiPairConnectionResult.hasValue()) { return establishedClientConnection; @@ -506,7 +516,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic null, null, null, - establishedClientConnection.virtualClientId, + establishedClientConnection.sebClientUserId, null, vdiPairCompanion.getConnectionToken(), null, @@ -554,7 +564,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic .byConnectionToken(connectionToken) .getOrThrow(); - ClientConnection updatedClientConnection; + final ClientConnection updatedClientConnection; if (clientConnection.status != ConnectionStatus.CLOSED) { updatedClientConnection = saveInState( clientConnection, @@ -614,7 +624,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic .byConnectionToken(connectionToken) .getOrThrow(); - ClientConnection updatedClientConnection; + final ClientConnection updatedClientConnection; if (DISABLE_STATE_PREDICATE.test(clientConnection)) { updatedClientConnection = saveInState( @@ -658,12 +668,37 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic .map(token -> disableConnection(token, institutionId) .onError(error -> log.error("Failed to disable SEB client connection: {}", token)) .getOr(null)) - .filter(clientConnection -> clientConnection != null) - .map(clientConnection -> clientConnection.getEntityKey()) + .filter(Objects::nonNull) + .map(Entity::getEntityKey) .collect(Collectors.toList()); }); } + // SEBSERV-475 IP address change during handshake is possible but is logged within SEB logs + private void sebLogClientAddressMismatch( + final String clientAddress, + final ClientConnection clientConnection) { + + try { + final long now = Utils.getMillisecondsNow(); + this.sebClientEventBatchService.accept(new SEBClientEventBatchService.EventData( + clientConnection.connectionToken, + now, + new ClientEvent( + null, + clientConnection.id, + ClientEvent.EventType.WARN_LOG, + now, now, null, + "SEB Client IP address changed: " + + clientConnection.clientAddress + + " -> " + + clientAddress + ))); + } catch (final Exception e) { + log.error("Failed to log SEB client IP address change: ", e); + } + } + private void checkExamRunning(final Long examId, final String user, final String address) { if (examId != null && !this.examSessionService.isExamRunning(examId)) { examNotRunningException(examId, user, address); @@ -730,7 +765,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic } } - // try to get user account display name + // try to get user account display name (SEBSERV-228) String accountId = userSessionId; try { final String newAccountId = this.examSessionService diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientEventBatchService.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientEventBatchService.java index f7fad510..4481ccc9 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientEventBatchService.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientEventBatchService.java @@ -176,7 +176,7 @@ public class SEBClientEventBatchService { log.debug("SEBClientEventBatchService worker {} processes batch of size {} in {} ms", workerName, size, - start - Utils.getMillisecondsNow()); + Utils.getMillisecondsNow() - start); } } catch (final Exception e) {