finished up improved SEB client connection handshake

This commit is contained in:
anhefti 2023-12-04 12:01:19 +01:00
parent 32567a220a
commit a055e23538
11 changed files with 80 additions and 58 deletions

View file

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

View file

@ -546,7 +546,7 @@ public class MonitoringRunningExam implements TemplateComposer {
} }
/** This holds the filter action items and implements the specific GUI update for it */ /** 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 PolyglotPageService polyglotPageService;
private final TreeItem[] actionItemPerStateFilter = new TreeItem[ConnectionStatus.values().length]; private final TreeItem[] actionItemPerStateFilter = new TreeItem[ConnectionStatus.values().length];

View file

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

View file

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

View file

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

View file

@ -286,7 +286,7 @@ public class ClientConnectionDAOImpl implements ClientConnectionDAO {
.and(ClientConnectionRecordDynamicSqlSupport.status, isNotEqualTo(ConnectionStatus.ACTIVE.name())) .and(ClientConnectionRecordDynamicSqlSupport.status, isNotEqualTo(ConnectionStatus.ACTIVE.name()))
.and( .and(
ClientConnectionRecordDynamicSqlSupport.status, ClientConnectionRecordDynamicSqlSupport.status,
isNotEqualTo(ConnectionStatus.AUTHENTICATED.name())) isNotEqualTo(ConnectionStatus.READY.name()))
.and(ClientConnectionRecordDynamicSqlSupport.status, .and(ClientConnectionRecordDynamicSqlSupport.status,
isNotEqualTo(ConnectionStatus.CONNECTION_REQUESTED.name())) isNotEqualTo(ConnectionStatus.CONNECTION_REQUESTED.name()))
.build() .build()
@ -777,10 +777,14 @@ public class ClientConnectionDAOImpl implements ClientConnectionDAO {
SqlBuilder.isEqualTo(examId)) SqlBuilder.isEqualTo(examId))
.and( .and(
ClientConnectionRecordDynamicSqlSupport.status, ClientConnectionRecordDynamicSqlSupport.status,
SqlBuilder.isEqualTo(ConnectionStatus.ACTIVE.name()), SqlBuilder.isIn(ClientConnection.ACTIVE_STATES)
SqlBuilder.or( )
ClientConnectionRecordDynamicSqlSupport.status, // .and(
SqlBuilder.isEqualTo(ConnectionStatus.CONNECTION_REQUESTED.name()))) // ClientConnectionRecordDynamicSqlSupport.status,
// SqlBuilder.isEqualTo(ConnectionStatus.ACTIVE.name()),
// SqlBuilder.or(
// ClientConnectionRecordDynamicSqlSupport.status,
// SqlBuilder.isEqualTo(ConnectionStatus.CONNECTION_REQUESTED.name())))
.build() .build()
.execute() .execute()
.stream() .stream()

View file

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

View file

@ -237,7 +237,7 @@ public class ExamAPI_V1_Controller {
@RequestParam(name = API.EXAM_API_USER_SESSION_ID, required = false) final String userSessionId, @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_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_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( @RequestParam(
name = API.EXAM_API_PARAM_SIGNATURE_KEY, name = API.EXAM_API_PARAM_SIGNATURE_KEY,
required = false) final String browserSignatureKey, required = false) final String browserSignatureKey,
@ -260,7 +260,7 @@ public class ExamAPI_V1_Controller {
remoteAddr, remoteAddr,
sebVersion, sebVersion,
sebOSName, sebOSName,
sebMachinName, setMachineName,
userSessionId, userSessionId,
clientId, clientId,
browserSignatureKey) browserSignatureKey)

View file

@ -2320,6 +2320,7 @@ sebserver.monitoring.exam.connection.notificationlist.type=Notification Type
sebserver.monitoring.exam.connection.status.UNDEFINED=Undefined sebserver.monitoring.exam.connection.status.UNDEFINED=Undefined
sebserver.monitoring.exam.connection.status.CONNECTION_REQUESTED=Connecting sebserver.monitoring.exam.connection.status.CONNECTION_REQUESTED=Connecting
sebserver.monitoring.exam.connection.status.AUTHENTICATED=Authenticated 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.ACTIVE=Active
sebserver.monitoring.exam.connection.status.CLOSED=Closed sebserver.monitoring.exam.connection.status.CLOSED=Closed
sebserver.monitoring.exam.connection.status.ABORTED=Aborted sebserver.monitoring.exam.connection.status.ABORTED=Aborted

View file

@ -162,7 +162,7 @@ public class SEBConnectionAPITest extends ExamAPIIntegrationTester {
.byConnectionToken(connectionToken) .byConnectionToken(connectionToken)
.getOrThrow(); .getOrThrow();
assertNotNull(clientConnection); assertNotNull(clientConnection);
assertEquals("AUTHENTICATED", clientConnection.getStatus().name()); assertEquals("CONNECTION_REQUESTED", clientConnection.getStatus().name());
assertEquals(connectionToken, clientConnection.connectionToken); assertEquals(connectionToken, clientConnection.connectionToken);
assertEquals(1, (long) clientConnection.institutionId); assertEquals(1, (long) clientConnection.institutionId);
assertEquals(2, (long) clientConnection.examId); assertEquals(2, (long) clientConnection.examId);

View file

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