refactored client connection (more integrity checks)

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

@@ -81,7 +81,7 @@ public final class ClientConnection implements GrantEntity {
             @JsonProperty(Domain.CLIENT_CONNECTION.ATTR_EXAM_USER_SESSION_IDENTIFER) final String userSessionId,
             @JsonProperty(Domain.CLIENT_CONNECTION.ATTR_CLIENT_ADDRESS) final String clientAddress,
             @JsonProperty(Domain.CLIENT_CONNECTION.ATTR_VIRTUAL_CLIENT_ADDRESS) final String virtualClientAddress,
-            @JsonProperty(Domain.CLIENT_CONNECTION.ATTR_CREATION_TIME) final Long creationTim) {
+            @JsonProperty(Domain.CLIENT_CONNECTION.ATTR_CREATION_TIME) final Long creationTime) {
 
         this.id = id;
         this.institutionId = institutionId;
@@ -91,7 +91,7 @@ public final class ClientConnection implements GrantEntity {
         this.userSessionId = userSessionId;
         this.clientAddress = clientAddress;
         this.virtualClientAddress = virtualClientAddress;
-        this.creationTime = creationTim;
+        this.creationTime = creationTime;
     }
 
     @Override

src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/batch/BatchConfig.java

@@ -8,36 +8,29 @@
 
 package ch.ethz.seb.sebserver.webservice.servicelayer.batch;
 
-import org.quartz.CronScheduleBuilder;
-import org.quartz.JobBuilder;
-import org.quartz.JobDetail;
-import org.quartz.Trigger;
-import org.quartz.TriggerBuilder;
-import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 
 @Configuration
 public class BatchConfig {
 
-    @Bean
+//    @Bean
-    public JobDetail jobADetails() {
+//    public JobDetail jobADetails() {
-        return JobBuilder
+//        return JobBuilder
-                .newJob(SimpleBatchJob.class)
+//                .newJob(SimpleBatchJob.class)
-                .withIdentity("sampleJobA")
+//                .withIdentity("sampleJobA")
-                .build();
+//                .build();
-    }
+//    }
-
+//
-    @Bean
+//    @Bean
-    public Trigger jobATrigger(final JobDetail jobADetails) {
+//    public Trigger jobATrigger(final JobDetail jobADetails) {
-
+//
-        return TriggerBuilder
+//        return TriggerBuilder
-                .newTrigger()
+//                .newTrigger()
-                .forJob(jobADetails)
+//                .forJob(jobADetails)
-                .withIdentity("sampleTriggerA")
+//                .withIdentity("sampleTriggerA")
-
+//
-                .withSchedule(CronScheduleBuilder.cronSchedule("0/30 0 0 ? * * *"))
+//                .withSchedule(CronScheduleBuilder.cronSchedule("0/30 0 0 ? * * *"))
-                .startNow()
+//                .build();
-                .build();
+//    }
-    }
 
 }

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

@@ -15,11 +15,17 @@ import java.util.function.Predicate;
 import ch.ethz.seb.sebserver.gbl.model.exam.Exam;
 import ch.ethz.seb.sebserver.gbl.model.session.ClientConnectionData;
 import ch.ethz.seb.sebserver.gbl.util.Result;
+import ch.ethz.seb.sebserver.webservice.servicelayer.dao.ExamDAO;
 import ch.ethz.seb.sebserver.webservice.servicelayer.dao.FilterMap;
 
 /** A Service to handle running exam sessions */
 public interface ExamSessionService {
 
+    /** Get the underling ExamDAO service.
+     *
+     * @return the underling ExamDAO service. */
+    ExamDAO getExamDAO();
+
     /** Indicates whether an Exam is currently running or not.
      *
      * @param examId the PK of the Exam to test

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

@@ -8,6 +8,8 @@
 
 package ch.ethz.seb.sebserver.webservice.servicelayer.session;
 
+import java.security.Principal;
+
 import ch.ethz.seb.sebserver.gbl.model.session.ClientConnection;
 import ch.ethz.seb.sebserver.gbl.model.session.ClientEvent;
 import ch.ethz.seb.sebserver.gbl.util.Result;
@@ -25,12 +27,14 @@ public interface SebClientConnectionService {
      * A connection-token to identify the connection is generated and stored within the
      * returned ClientConnection.
      *
+     * @param principal the client connection Principal from REST controller interface
      * @param institutionId The institution identifier
      * @param clientAddress The clients remote IP address
      * @param examId the exam identifier (can be null)
      * @return A Result refer to the newly created ClientConnection in state: CONNECTION_REQUESTED, or refer to an error
      *         if happened */
     Result<ClientConnection> createClientConnection(
+            Principal principal,
             Long institutionId,
             String clientAddress,
             Long examId);

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

@@ -58,6 +58,11 @@ public class ExamSessionServiceImpl implements ExamSessionService {
         this.cacheManager = cacheManager;
     }
 
+    @Override
+    public ExamDAO getExamDAO() {
+        return this.examDAO;
+    }
+
     @Override
     public boolean isExamRunning(final Long examId) {
         return !getRunningExam(examId).hasError();
@@ -199,7 +204,7 @@ public class ExamSessionServiceImpl implements ExamSessionService {
                         // evict also cached ping record
                         this.examSessionCacheService.evictPingRecord(token);
                     });
-        } catch (Exception e) {
+        } catch (final Exception e) {
             log.error("Unexpected error while trying to flush cache for exam: ", exam, e);
         }
     }

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

@@ -8,6 +8,7 @@
 
 package ch.ethz.seb.sebserver.webservice.servicelayer.session.impl;
 
+import java.security.Principal;
 import java.util.UUID;
 
 import org.slf4j.Logger;
@@ -23,10 +24,12 @@ import ch.ethz.seb.sebserver.gbl.profile.WebServiceProfile;
 import ch.ethz.seb.sebserver.gbl.util.Result;
 import ch.ethz.seb.sebserver.gbl.util.Utils;
 import ch.ethz.seb.sebserver.webservice.servicelayer.dao.ClientConnectionDAO;
+import ch.ethz.seb.sebserver.webservice.servicelayer.dao.SebClientConfigDAO;
 import ch.ethz.seb.sebserver.webservice.servicelayer.session.EventHandlingStrategy;
 import ch.ethz.seb.sebserver.webservice.servicelayer.session.ExamSessionService;
 import ch.ethz.seb.sebserver.webservice.servicelayer.session.PingHandlingStrategy;
 import ch.ethz.seb.sebserver.webservice.servicelayer.session.SebClientConnectionService;
+import ch.ethz.seb.sebserver.webservice.weblayer.api.APIConstraintViolationException;
 
 @Lazy
 @Service
@@ -40,29 +43,51 @@ public class SebClientConnectionServiceImpl implements SebClientConnectionServic
     private final EventHandlingStrategy eventHandlingStrategy;
     private final ClientConnectionDAO clientConnectionDAO;
     private final PingHandlingStrategy pingHandlingStrategy;
+    private final SebClientConfigDAO sebClientConfigDAO;
 
     protected SebClientConnectionServiceImpl(
             final ExamSessionService examSessionService,
             final ExamSessionCacheService examSessionCacheService,
             final ClientConnectionDAO clientConnectionDAO,
             final EventHandlingStrategyFactory eventHandlingStrategyFactory,
-            final PingHandlingStrategyFactory pingHandlingStrategyFactory) {
+            final PingHandlingStrategyFactory pingHandlingStrategyFactory,
+            final SebClientConfigDAO sebClientConfigDAO) {
 
         this.examSessionService = examSessionService;
         this.examSessionCacheService = examSessionCacheService;
         this.clientConnectionDAO = clientConnectionDAO;
         this.pingHandlingStrategy = pingHandlingStrategyFactory.get();
         this.eventHandlingStrategy = eventHandlingStrategyFactory.get();
+        this.sebClientConfigDAO = sebClientConfigDAO;
     }
 
     @Override
     public Result<ClientConnection> createClientConnection(
+            final Principal principal,
             final Long institutionId,
             final String clientAddress,
             final Long examId) {
 
         return Result.tryCatch(() -> {
 
+            final Long clientsInstitution = getInstitutionId(principal);
+            if (!clientsInstitution.equals(institutionId)) {
+                log.error("Institutional integrity violation: requested institution: {} authenticated institution: {}",
+                        institutionId,
+                        clientsInstitution);
+                throw new APIConstraintViolationException("Institutional integrity violation");
+            }
+
+            if (log.isDebugEnabled()) {
+                log.debug("Request received on Exam Client Connection create endpoint: "
+                        + "institution: {} "
+                        + "exam: {} "
+                        + "client-address: {}",
+                        institutionId,
+                        examId,
+                        clientAddress);
+            }
+
             if (log.isDebugEnabled()) {
                 log.debug("SEB client connection attempt, create ClientConnection for "
                         + "instituion {} "
@@ -131,21 +156,16 @@ public class SebClientConnectionServiceImpl implements SebClientConnectionServic
 
             final ClientConnection clientConnection = getClientConnection(connectionToken);
 
-            checkInstitutionalIntegrity(
+            checkInstitutionalIntegrity(institutionId, clientConnection);
-                    institutionId,
+            checkExamIntegrity(examId, clientConnection);
-                    clientConnection);
 
-            // examId integrity check
+            // connection integrity check
-            if (examId != null &&
+            if (clientConnection.status != ConnectionStatus.CONNECTION_REQUESTED) {
-                    clientConnection.examId != null &&
+                log.error("ClientConnection integrity violation: client connection is not in expected state: {}",
-                    !examId.equals(clientConnection.examId)) {
-
-                log.error("Exam integrity violation: another examId is already set for the connection: {}",
                         clientConnection);
                 throw new IllegalArgumentException(
-                        "Exam integrity violation: another examId is already set for the connection");
+                        "ClientConnection integrity violation: client connection is not in expected state");
             }
-            checkExamRunning(examId);
 
             // userSessionId integrity check
             if (userSessionId != null &&
@@ -169,7 +189,7 @@ public class SebClientConnectionServiceImpl implements SebClientConnectionServic
                             clientConnection.id,
                             null,
                             examId,
-                            null,
+                            (userSessionId != null) ? ConnectionStatus.AUTHENTICATED : null,
                             null,
                             userSessionId,
                             null,
@@ -219,20 +239,21 @@ public class SebClientConnectionServiceImpl implements SebClientConnectionServic
                         userSessionId);
             }
 
-            checkExamRunning(examId);
-
             final ClientConnection clientConnection = getClientConnection(connectionToken);
+            checkInstitutionalIntegrity(institutionId, clientConnection);
+            checkExamIntegrity(examId, clientConnection);
 
-            checkInstitutionalIntegrity(
+            // connection integrity check
-                    institutionId,
+            if (clientConnection.status == ConnectionStatus.CONNECTION_REQUESTED) {
+                // TODO discuss if we need a flag on exam domain level that indicates whether unauthenticated connection
+                //      are allowed or not
+                log.warn("ClientConnection integrity warning: client connection is not authenticated: {}",
                         clientConnection);
-
+            } else if (clientConnection.status != ConnectionStatus.AUTHENTICATED) {
-            // Exam integrity
+                log.error("ClientConnection integrity violation: client connection is not in expected state: {}",
-            if (clientConnection.examId != null && examId != null && !examId.equals(clientConnection.examId)) {
-                log.error("Exam integrity violation with examId: {} on clientConnection: {}",
-                        examId,
                         clientConnection);
-                throw new IllegalAccessError("Exam integrity violation");
+                throw new IllegalArgumentException(
+                        "ClientConnection integrity violation: client connection is not in expected state");
             }
 
             final String virtualClientAddress = getVirtualClientAddress(
@@ -240,6 +261,7 @@ public class SebClientConnectionServiceImpl implements SebClientConnectionServic
                     clientAddress,
                     clientConnection.clientAddress);
 
+            // create new ClientConnection for update
             final ClientConnection establishedClientConnection = new ClientConnection(
                     clientConnection.id,
                     null,
@@ -312,7 +334,9 @@ public class SebClientConnectionServiceImpl implements SebClientConnectionServic
                     .byConnectionToken(connectionToken)
                     .getOrThrow();
 
-            final ClientConnection updatedClientConnection = this.clientConnectionDAO.save(new ClientConnection(
+            ClientConnection updatedClientConnection;
+            if (clientConnection.status != ConnectionStatus.CLOSED) {
+                updatedClientConnection = this.clientConnectionDAO.save(new ClientConnection(
                         clientConnection.id,
                         null,
                         null,
@@ -327,6 +351,10 @@ public class SebClientConnectionServiceImpl implements SebClientConnectionServic
                     log.debug("SEB client connection: successfully closed ClientConnection: {}",
                             clientConnection);
                 }
+            } else {
+                log.warn("SEB client connection is already closed: {}", clientConnection);
+                updatedClientConnection = clientConnection;
+            }
 
             // evict cached ClientConnection
             this.examSessionCacheService.evictClientConnection(connectionToken);
@@ -390,9 +418,10 @@ public class SebClientConnectionServiceImpl implements SebClientConnectionServic
         return clientConnection;
     }
 
-    private void checkInstitutionalIntegrity(final Long institutionId, final ClientConnection clientConnection)
+    private void checkInstitutionalIntegrity(
-            throws IllegalAccessError {
+            final Long institutionId,
-        // Institutional integrity
+            final ClientConnection clientConnection) throws IllegalAccessError {
+
         if (!institutionId.equals(clientConnection.institutionId)) {
             log.error("Instituion integrity violation with institution: {} on clientConnection: {}",
                     institutionId,
@@ -437,4 +466,23 @@ public class SebClientConnectionServiceImpl implements SebClientConnectionServic
                 .getType() == ExamType.VDI;
     }
 
+    private Long getInstitutionId(final Principal principal) {
+        final String clientId = principal.getName();
+        return this.sebClientConfigDAO.byClientName(clientId)
+                .getOrThrow().institutionId;
+    }
+
+    private void checkExamIntegrity(final Long examId, final ClientConnection clientConnection) {
+        if (examId != null &&
+                clientConnection.examId != null &&
+                !examId.equals(clientConnection.examId)) {
+
+            log.error("Exam integrity violation: another examId is already set for the connection: {}",
+                    clientConnection);
+            throw new IllegalArgumentException(
+                    "Exam integrity violation: another examId is already set for the connection");
+        }
+        checkExamRunning(examId);
+    }
+
 }

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

@@ -43,7 +43,6 @@ import ch.ethz.seb.sebserver.gbl.model.session.PingResponse;
 import ch.ethz.seb.sebserver.gbl.model.session.RunningExamInfo;
 import ch.ethz.seb.sebserver.gbl.profile.WebServiceProfile;
 import ch.ethz.seb.sebserver.gbl.util.Utils;
-import ch.ethz.seb.sebserver.webservice.servicelayer.dao.ExamDAO;
 import ch.ethz.seb.sebserver.webservice.servicelayer.dao.LmsSetupDAO;
 import ch.ethz.seb.sebserver.webservice.servicelayer.dao.SebClientConfigDAO;
 import ch.ethz.seb.sebserver.webservice.servicelayer.session.ExamSessionService;
@@ -56,7 +55,6 @@ public class ExamAPI_V1_Controller {
 
     private static final Logger log = LoggerFactory.getLogger(ExamAPI_V1_Controller.class);
 
-    private final ExamDAO examDAO;
     private final LmsSetupDAO lmsSetupDAO;
     private final ExamSessionService examSessionService;
     private final SebClientConnectionService sebClientConnectionService;
@@ -64,14 +62,12 @@ public class ExamAPI_V1_Controller {
     private final JSONMapper jsonMapper;
 
     protected ExamAPI_V1_Controller(
-            final ExamDAO examDAO,
             final LmsSetupDAO lmsSetupDAO,
             final ExamSessionService examSessionService,
             final SebClientConnectionService sebClientConnectionService,
             final SebClientConfigDAO sebClientConfigDAO,
             final JSONMapper jsonMapper) {
 
-        this.examDAO = examDAO;
         this.lmsSetupDAO = lmsSetupDAO;
         this.examSessionService = examSessionService;
         this.sebClientConnectionService = sebClientConnectionService;
@@ -101,25 +97,17 @@ public class ExamAPI_V1_Controller {
         final Long examId = (examIdRequestParam != null)
                 ? examIdRequestParam
                 : mapper.getLong(API.EXAM_API_PARAM_EXAM_ID);
-        final Long clientsInstitution = getInstitutionId(principal);
 
-        if (!clientsInstitution.equals(institutionId)) {
+        // Create and get new ClientConnection if all integrity checks passes
-            log.error("Institutional integrity violation: requested institution: {} authenticated institution: {}",
+        final ClientConnection clientConnection = this.sebClientConnectionService
-                    institutionId,
+                .createClientConnection(principal, institutionId, remoteAddr, examId)
-                    clientsInstitution);
+                .getOrThrow();
-            throw new APIConstraintViolationException("Institutional integrity violation");
-        }
 
-        if (log.isDebugEnabled()) {
+        response.setHeader(
-            log.debug("Request received on Exam Client Connection create endpoint: "
+                API.EXAM_API_SEB_CONNECTION_TOKEN,
-                    + "institution: {} "
+                clientConnection.connectionToken);
-                    + "exam: {} "
-                    + "client-address: {}",
-                    institutionId,
-                    examId,
-                    remoteAddr);
-        }
 
+        // Crate list of running exams
         List<RunningExamInfo> result;
         if (examId == null) {
             result = this.examSessionService.getRunningExamsForInstitution(institutionId)
@@ -128,7 +116,7 @@ public class ExamAPI_V1_Controller {
                     .map(this::createRunningExamInfo)
                     .collect(Collectors.toList());
         } else {
-            final Exam exam = this.examDAO.byPK(examId)
+            final Exam exam = this.examSessionService.getExamDAO().byPK(examId)
                     .getOrThrow();
 
             result = Arrays.asList(createRunningExamInfo(exam));
@@ -139,25 +127,9 @@ public class ExamAPI_V1_Controller {
             throw new IllegalStateException("There are no currently running exams");
         }
 
-        final ClientConnection clientConnection = this.sebClientConnectionService
-                .createClientConnection(institutionId, remoteAddr, examId)
-                .getOrThrow();
-
-        response.setHeader(
-                API.EXAM_API_SEB_CONNECTION_TOKEN,
-                clientConnection.connectionToken);
-
         return result;
     }
 
-    private RunningExamInfo createRunningExamInfo(final Exam exam) {
-        return new RunningExamInfo(
-                exam,
-                this.lmsSetupDAO.byPK(exam.lmsSetupId)
-                        .map(lms -> lms.lmsType)
-                        .getOr(null));
-    }
-
     @RequestMapping(
             path = API.EXAM_API_HANDSHAKE_ENDPOINT,
             method = RequestMethod.PATCH,
@@ -352,4 +324,12 @@ public class ExamAPI_V1_Controller {
                 .getOrThrow().institutionId;
     }
 
+    private RunningExamInfo createRunningExamInfo(final Exam exam) {
+        return new RunningExamInfo(
+                exam,
+                this.lmsSetupDAO.byPK(exam.lmsSetupId)
+                        .map(lms -> lms.lmsType)
+                        .getOr(null));
+    }
+
 }

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

@@ -216,7 +216,7 @@ public class SebConnectionTest extends ExamAPIIntegrationTester {
         final ClientConnectionRecord clientConnectionRecord = records.get(0);
         assertEquals("1", String.valueOf(clientConnectionRecord.getInstitutionId()));
         assertEquals("2", String.valueOf(clientConnectionRecord.getExamId()));
-        assertEquals("CONNECTION_REQUESTED", String.valueOf(clientConnectionRecord.getStatus()));
+        assertEquals("AUTHENTICATED", String.valueOf(clientConnectionRecord.getStatus()));
         assertNotNull(clientConnectionRecord.getConnectionToken());
         assertNotNull(clientConnectionRecord.getClientAddress());
         assertEquals("userSessionId", clientConnectionRecord.getExamUserSessionIdentifer());