finished up improved SEB client connection handshake

src/main/java/ch/ethz/seb/sebserver/gbl/model/session/ClientConnection.java

@@ -32,7 +32,7 @@ public final class ClientConnection implements GrantEntity {
     public enum ConnectionStatus {
         UNDEFINED(0, false, false, false),
         CONNECTION_REQUESTED(1, true, false, false),
-        AUTHENTICATED(2, true, true, true),
+        READY(2, true, true, true),
         ACTIVE(3, false, true, true),
         CLOSED(4, false, false, true),
         DISABLED(5, false, false, false);
@@ -59,18 +59,18 @@ public enum ConnectionStatus {
 
     public final static List<String> ACTIVE_STATES = Utils.immutableListOf(
             ConnectionStatus.ACTIVE.name(),
-            ConnectionStatus.AUTHENTICATED.name(),
+            ConnectionStatus.READY.name(),
             ConnectionStatus.CONNECTION_REQUESTED.name());
 
     public final static List<String> SECURE_STATES = Utils.immutableListOf(
             ConnectionStatus.ACTIVE.name(),
-            ConnectionStatus.AUTHENTICATED.name(),
+            ConnectionStatus.READY.name(),
             ConnectionStatus.CLOSED.name());
 
     public final static List<String> SECURE_CHECK_STATES = Utils.immutableListOf(
             ConnectionStatus.CONNECTION_REQUESTED.name(),
             ConnectionStatus.ACTIVE.name(),
-            ConnectionStatus.AUTHENTICATED.name());
+            ConnectionStatus.READY.name());
 
     public static final ClientConnection EMPTY_CLIENT_CONNECTION = new ClientConnection(
             -1L, -1L, -1L,

src/main/java/ch/ethz/seb/sebserver/gui/content/monitoring/MonitoringRunningExam.java

@@ -546,7 +546,7 @@ private void addClientGroupFilterAction(
     }
 
     /** This holds the filter action items and implements the specific GUI update for it */
-    private class FilterGUIUpdate implements FullPageMonitoringGUIUpdate {
+    private static class FilterGUIUpdate implements FullPageMonitoringGUIUpdate {
 
         private final PolyglotPageService polyglotPageService;
         private final TreeItem[] actionItemPerStateFilter = new TreeItem[ConnectionStatus.values().length];

src/main/java/ch/ethz/seb/sebserver/gui/service/session/ClientConnectionTable.java

@@ -629,6 +629,12 @@ public int compareTo(final UpdatableTableItem other) {
                     .thenComparingInt(UpdatableTableItem::thresholdsWeight)
                     .thenComparing(UpdatableTableItem::getConnectionIdentifier)
                     .compare(this, other);
+
+//            return Comparator.comparing(UpdatableTableItem::getConnectionIdentifier)
+//                    .thenComparingInt(UpdatableTableItem::thresholdsWeight)
+//                    .thenComparingInt(UpdatableTableItem::notificationWeight)
+//                    .thenComparingInt(UpdatableTableItem::statusWeight)
+//                    .compare(this, other);
         }
 
         @Override

src/main/java/ch/ethz/seb/sebserver/gui/service/session/ColorData.java

@@ -42,7 +42,7 @@ Color getStatusColor(final MonitoringEntry entry) {
 
         switch (status) {
             case CONNECTION_REQUESTED:
-            case AUTHENTICATED:
+            case READY:
             case ACTIVE: {
                 if (entry.grantDenied()) {
                     return this.color3;
@@ -62,23 +62,11 @@ Color getStatusTextColor(final Color statusColor) {
     }
 
     int statusWeight(final MonitoringEntry entry) {
-        if (entry == null) {
+        if (entry == null || entry.getStatus() == null) {
             return 100;
         }
 
-        switch (entry.getStatus()) {
-            case CONNECTION_REQUESTED:
-            case AUTHENTICATED: {
-                return 0;
-            }
-            case ACTIVE: {
-                return 2;
-            }
-            case CLOSED:
-                return 3;
-            default:
-                return 5;
-        }
+        return entry.getStatus().code;
     }
 
 }

src/main/java/ch/ethz/seb/sebserver/gui/service/session/InstructionProcessor.java

@@ -48,6 +48,14 @@ public class InstructionProcessor {
 
     private static final Logger log = LoggerFactory.getLogger(InstructionProcessor.class);
 
+    private static final Predicate<ClientMonitoringDataView> ALL_BUT_DISABLED_STATES =
+            ClientMonitoringDataView.getStatusPredicate(
+                    ConnectionStatus.CONNECTION_REQUESTED,
+                    ConnectionStatus.UNDEFINED,
+                    ConnectionStatus.CLOSED,
+                    ConnectionStatus.ACTIVE,
+                    ConnectionStatus.READY);
+
     private final RestService restService;
     private final JSONMapper jsonMapper;
 
@@ -77,6 +85,7 @@ public void propagateSEBQuitInstruction(
             final Set<String> connectionTokens = selectionFunction
                     .apply(ClientMonitoringDataView.getStatusPredicate(
                             ConnectionStatus.CONNECTION_REQUESTED,
+                            ConnectionStatus.READY,
                             ConnectionStatus.ACTIVE));
 
             if (connectionTokens.isEmpty()) {
@@ -97,7 +106,7 @@ public void propagateSEBQuitInstruction(
                     null);
 
             processInstruction(() -> this.restService.getBuilder(PropagateInstruction.class)
-                    .withURIVariable(API.PARAM_PARENT_MODEL_ID, String.valueOf(examId))
+                    .withURIVariable(API.PARAM_PARENT_MODEL_ID, examId)
                     .withBody(clientInstruction)
                     .call()
                     .getOrThrow(),
@@ -154,19 +163,14 @@ public void propagateSEBLockInstruction(
         }
     }
 
+
+
     public void disableConnection(
             final Long examId,
             final Function<Predicate<ClientMonitoringDataView>, Set<String>> selectionFunction,
             final PageContext pageContext) {
 
-        final Set<String> connectionTokens = selectionFunction
-                .apply(ClientMonitoringDataView.getStatusPredicate(
-                        ConnectionStatus.CONNECTION_REQUESTED,
-                        ConnectionStatus.UNDEFINED,
-                        ConnectionStatus.CLOSED,
-                        ConnectionStatus.ACTIVE,
-                        ConnectionStatus.AUTHENTICATED));
-
+        final Set<String> connectionTokens = selectionFunction.apply(ALL_BUT_DISABLED_STATES);
         if (connectionTokens.isEmpty()) {
             return;
         }

src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientConnectionDAOImpl.java

@@ -286,7 +286,7 @@ public Result<Collection<String>> getInactiveConnectionTokens(final Set<String>
                     .and(ClientConnectionRecordDynamicSqlSupport.status, isNotEqualTo(ConnectionStatus.ACTIVE.name()))
                     .and(
                             ClientConnectionRecordDynamicSqlSupport.status,
-                            isNotEqualTo(ConnectionStatus.AUTHENTICATED.name()))
+                            isNotEqualTo(ConnectionStatus.READY.name()))
                     .and(ClientConnectionRecordDynamicSqlSupport.status,
                             isNotEqualTo(ConnectionStatus.CONNECTION_REQUESTED.name()))
                     .build()
@@ -777,10 +777,14 @@ public Result<Set<String>> filterForInstructionStatus(final Long examId, final S
                         SqlBuilder.isEqualTo(examId))
                 .and(
                         ClientConnectionRecordDynamicSqlSupport.status,
-                        SqlBuilder.isEqualTo(ConnectionStatus.ACTIVE.name()),
-                        SqlBuilder.or(
-                                ClientConnectionRecordDynamicSqlSupport.status,
-                                SqlBuilder.isEqualTo(ConnectionStatus.CONNECTION_REQUESTED.name())))
+                        SqlBuilder.isIn(ClientConnection.ACTIVE_STATES)
+                )
+//                .and(
+//                        ClientConnectionRecordDynamicSqlSupport.status,
+//                        SqlBuilder.isEqualTo(ConnectionStatus.ACTIVE.name()),
+//                        SqlBuilder.or(
+//                                ClientConnectionRecordDynamicSqlSupport.status,
+//                                SqlBuilder.isEqualTo(ConnectionStatus.CONNECTION_REQUESTED.name())))
                 .build()
                 .execute()
                 .stream()

src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientConnectionServiceImpl.java

@@ -25,7 +25,6 @@
 import ch.ethz.seb.sebserver.webservice.servicelayer.session.SEBClientInstructionService;
 import org.apache.commons.lang3.BooleanUtils;
 import org.apache.commons.lang3.StringUtils;
-import org.apache.tomcat.jni.Address;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.context.annotation.Lazy;
@@ -64,7 +63,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic
             .getStatusPredicate(
                     ConnectionStatus.UNDEFINED,
                     ConnectionStatus.CONNECTION_REQUESTED,
-                    ConnectionStatus.AUTHENTICATED,
+                    ConnectionStatus.READY,
                     ConnectionStatus.ACTIVE,
                     ConnectionStatus.CLOSED);
 
@@ -266,8 +265,8 @@ public Result<ClientConnection> updateClientConnection(
                     clientConnection.id,
                     null,
                     examId,
-                    (userSessionId != null && currentStatus == ConnectionStatus.CONNECTION_REQUESTED)
-                        ? ConnectionStatus.AUTHENTICATED
+                    (StringUtils.isNotBlank(userSessionId) && currentStatus == ConnectionStatus.READY)
+                        ? ConnectionStatus.ACTIVE
                         : null,
                     null,
                     updateUserSessionId,
@@ -345,7 +344,6 @@ public Result<ClientConnection> establishClientConnection(
                         connectionToken, institutionId, examId, clientAddress, userSessionId, clientId);
             }
 
-
             final String updateUserSessionId = updateUserSessionId(
                     _examId,
                     userSessionId,
@@ -364,12 +362,16 @@ public Result<ClientConnection> establishClientConnection(
                     ? clientId
                     : clientConnection.sebClientUserId;
 
+
+
             // create new ClientConnection for update
             final ClientConnection establishedClientConnection = new ClientConnection(
                     clientConnection.id,
                     null,
                     currentExamId,
-                    ConnectionStatus.ACTIVE,
+                    StringUtils.isNotBlank(userSessionId) || alreadyAuthenticated(clientConnection)
+                            ? ConnectionStatus.ACTIVE
+                            : ConnectionStatus.READY,
                     null,
                     updateUserSessionId,
                     StringUtils.isNotBlank(clientAddress) ? clientAddress : null,
@@ -390,20 +392,16 @@ public Result<ClientConnection> establishClientConnection(
                     null);
 
             // ClientConnection integrity check
-            // institutionId, connectionToken and clientAddress must be set
-            // The status ins not already active
-            // and if this is not a VDI prime connection, the examId must also be set
             if (clientConnection.institutionId == null ||
                     clientConnection.connectionToken == null ||
-                    establishedClientConnection.examId == null ||
-                    clientConnection.clientAddress == null ||
-                    establishedClientConnection.status != ConnectionStatus.ACTIVE ||
-                    (!BooleanUtils.isTrue(clientConnection.vdi) && currentExamId == null)) {
+                    currentExamId == null ||
+                    (clientConnection.clientAddress == null && clientAddress == null)) {
 
-                log.error("ClientConnection integrity violation, clientConnection: {}, establishedClientConnection: {}",
+                log.error("ClientConnection integrity violation, clientConnection: {}, updatedClientConnection: {}",
                         clientConnection,
                         establishedClientConnection);
-                throw new IllegalStateException("ClientConnection integrity violation");
+
+                throw new APIConstraintViolationException("ClientConnection integrity violation");
             }
 
             final ClientConnection updatedClientConnection = this.clientConnectionDAO
@@ -635,7 +633,10 @@ public void streamExamConfig(
         }
     }
 
-    private void writeSEBClientErrors(HttpServletResponse response, Collection<APIMessage> errorMessages) {
+    private void writeSEBClientErrors(
+            final HttpServletResponse response,
+            final Collection<APIMessage> errorMessages) {
+
         try {
             response.getOutputStream().write(Utils.toByteArray(this.jsonMapper.writeValueAsString(errorMessages)));
         } catch (final Exception e1) {
@@ -663,7 +664,7 @@ private void connectionStatusIntegrityCheck(
                     false
             );
 
-            throw new IllegalArgumentException(
+            throw new APIConstraintViolationException(
                     "ClientConnection integrity violation: client connection is not in expected state");
         }
 
@@ -842,7 +843,7 @@ private void checkExamIntegrity(
 
         if (currentExamId != null && !examId.equals(currentExamId)) {
             log.error("Exam integrity violation: another examId is already set for the connection: {}", ccToken);
-            throw new IllegalArgumentException(
+            throw new APIConstraintViolationException(
                     "Exam integrity violation: another examId is already set for the connection");
         }
 
@@ -914,4 +915,11 @@ private String getSignatureHash(
                 .onError(error -> log.error("Failed to get hash signature from sent app signature key: ", error))
                 .getOr(null);
     }
+
+    private boolean alreadyAuthenticated(final ClientConnection clientConnection) {
+        return clientConnection.userSessionId != null &&
+                !clientConnection.userSessionId.equals(clientConnection.clientAddress) &&
+                !clientConnection.userSessionId.equals(clientConnection.sebClientUserId) &&
+                !clientConnection.userSessionId.equals(clientConnection.sebMachineName);
+    }
 }

src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamAPI_V1_Controller.java

@@ -237,7 +237,7 @@ public CompletableFuture<Void> handshakeEstablish(
             @RequestParam(name = API.EXAM_API_USER_SESSION_ID, required = false) final String userSessionId,
             @RequestParam(name = API.EXAM_API_PARAM_SEB_VERSION, required = false) final String sebVersion,
             @RequestParam(name = API.EXAM_API_PARAM_SEB_OS_NAME, required = false) final String sebOSName,
-            @RequestParam(name = API.EXAM_API_PARAM_SEB_MACHINE_NAME, required = false) final String sebMachinName,
+            @RequestParam(name = API.EXAM_API_PARAM_SEB_MACHINE_NAME, required = false) final String setMachineName,
             @RequestParam(
                     name = API.EXAM_API_PARAM_SIGNATURE_KEY,
                     required = false) final String browserSignatureKey,
@@ -260,7 +260,7 @@ public CompletableFuture<Void> handshakeEstablish(
                                     remoteAddr,
                                     sebVersion,
                                     sebOSName,
-                                    sebMachinName,
+                                    setMachineName,
                                     userSessionId,
                                     clientId,
                                     browserSignatureKey)

src/main/resources/messages.properties

@@ -2320,6 +2320,7 @@ sebserver.monitoring.exam.connection.notificationlist.type=Notification Type
 sebserver.monitoring.exam.connection.status.UNDEFINED=Undefined
 sebserver.monitoring.exam.connection.status.CONNECTION_REQUESTED=Connecting
 sebserver.monitoring.exam.connection.status.AUTHENTICATED=Authenticated
+sebserver.monitoring.exam.connection.status.READY=Ready
 sebserver.monitoring.exam.connection.status.ACTIVE=Active
 sebserver.monitoring.exam.connection.status.CLOSED=Closed
 sebserver.monitoring.exam.connection.status.ABORTED=Aborted

src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/exam/SEBConnectionAPITest.java

@@ -162,7 +162,7 @@ public void testOrdinarySEBHandshakeWithKnownExam() throws Exception {
                 .byConnectionToken(connectionToken)
                 .getOrThrow();
         assertNotNull(clientConnection);
-        assertEquals("AUTHENTICATED", clientConnection.getStatus().name());
+        assertEquals("CONNECTION_REQUESTED", clientConnection.getStatus().name());
         assertEquals(connectionToken, clientConnection.connectionToken);
         assertEquals(1, (long) clientConnection.institutionId);
         assertEquals(2, (long) clientConnection.examId);

src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/exam/SebConnectionTest.java

@@ -238,7 +238,7 @@ public void testUpdateConnection() throws Exception {
         final ClientConnectionRecord clientConnectionRecord = records.get(0);
         assertEquals("1", String.valueOf(clientConnectionRecord.getInstitutionId()));
         assertEquals("2", String.valueOf(clientConnectionRecord.getExamId()));
-        assertEquals("AUTHENTICATED", String.valueOf(clientConnectionRecord.getStatus()));
+        assertEquals("CONNECTION_REQUESTED", String.valueOf(clientConnectionRecord.getStatus()));
         assertNotNull(clientConnectionRecord.getConnectionToken());
         assertNotNull(clientConnectionRecord.getClientAddress());
         assertEquals("-- (userSessionId)", clientConnectionRecord.getExamUserSessionId());
@@ -361,14 +361,14 @@ public void testEstablishConnectionNoExamLeadsToError() throws Exception {
                 null);
 
         // check correct response
-        assertTrue(HttpStatus.OK.value() != updatedConnection.getStatus());
+        assertTrue(HttpStatus.BAD_REQUEST.value() == updatedConnection.getStatus());
         final String contentAsString = updatedConnection.getContentAsString();
         final Collection<APIMessage> errorMessage = this.jsonMapper.readValue(
                 contentAsString,
                 new TypeReference<Collection<APIMessage>>() {
                 });
         final APIMessage error = errorMessage.iterator().next();
-        assertEquals(ErrorMessage.UNEXPECTED.messageCode, error.messageCode);
+        assertEquals(ErrorMessage.ILLEGAL_API_ARGUMENT.messageCode, error.messageCode);
         assertEquals("ClientConnection integrity violation", error.details);
 
         // check correct stored (no changes)
@@ -394,6 +394,17 @@ public void testEstablishConnectionNoExamLeadsToError() throws Exception {
         assertNotNull(ccdi);
         assertNull(ccdi.clientConnection.examId);
         assertTrue(ccdi.indicatorValues.isEmpty());
+
+        // check update to active
+        MockHttpServletResponse updateResponse = super.updateConnection(
+                accessToken,
+                connectionToken,
+                null,
+                "test-user");
+
+
+
+
     }
 
     @Test