From d4c7813c977ef8e5bd928b37e8659c5aa6db2ba0 Mon Sep 17 00:00:00 2001 From: delvh Date: Fri, 23 Oct 2020 18:45:40 +0200 Subject: [PATCH] Fix unnecessary authentication token being sent in requests --- client/.classpath | 1 - .../main/java/envoy/client/data/LocalDB.java | 9 +---- .../main/java/envoy/client/net/Client.java | 3 +- .../client/ui/controller/LoginScene.java | 12 +++--- .../java/envoy/data/AuthenticatedRequest.java | 25 ++++-------- .../java/envoy/data/LoginCredentials.java | 28 ++++++++++---- .../server/net/ObjectMessageProcessor.java | 38 ++++++++----------- .../processors/LoginCredentialProcessor.java | 27 ++++++------- 8 files changed, 64 insertions(+), 79 deletions(-) diff --git a/client/.classpath b/client/.classpath index 524f8bb..4328dab 100644 --- a/client/.classpath +++ b/client/.classpath @@ -21,7 +21,6 @@ - diff --git a/client/src/main/java/envoy/client/data/LocalDB.java b/client/src/main/java/envoy/client/data/LocalDB.java index 1edede8..e3c4ac7 100644 --- a/client/src/main/java/envoy/client/data/LocalDB.java +++ b/client/src/main/java/envoy/client/data/LocalDB.java @@ -44,7 +44,6 @@ public final class LocalDB implements EventListener { private IDGenerator idGenerator; private CacheMap cacheMap = new CacheMap(); private String authToken; - private boolean saveToken; private boolean contactsChanged; // Auto save timer @@ -261,7 +260,7 @@ public final class LocalDB implements EventListener { Context.getInstance().getClient().isOnline() ? Instant.now() : lastSync); // Save last login information - if (saveToken && authToken != null) + if (authToken != null) SerializationUtils.write(lastLoginFile, user, authToken); // Save ID generator @@ -489,10 +488,4 @@ public final class LocalDB implements EventListener { * @since Envoy Client v0.2-beta */ public String getAuthToken() { return authToken; } - - /** - * @param saveToken whether the token will be persisted or deleted on shutdown - * @since Envoy Client v0.3-beta - */ - public void setSaveToken(boolean saveToken) { this.saveToken = saveToken; } } diff --git a/client/src/main/java/envoy/client/net/Client.java b/client/src/main/java/envoy/client/net/Client.java index 8c8e682..d4ed164 100644 --- a/client/src/main/java/envoy/client/net/Client.java +++ b/client/src/main/java/envoy/client/net/Client.java @@ -153,8 +153,7 @@ public final class Client implements EventListener, Closeable { try { SerializationUtils.writeBytesWithLength( new AuthenticatedRequest<>(obj, - Context.getInstance().getLocalDB().getUser().getID(), - Context.getInstance().getLocalDB().getAuthToken()), + Context.getInstance().getLocalDB().getUser().getID()), socket.getOutputStream()); } catch (final IOException e) { throw new RuntimeException(e); diff --git a/client/src/main/java/envoy/client/ui/controller/LoginScene.java b/client/src/main/java/envoy/client/ui/controller/LoginScene.java index fc9a71f..f9d062b 100644 --- a/client/src/main/java/envoy/client/ui/controller/LoginScene.java +++ b/client/src/main/java/envoy/client/ui/controller/LoginScene.java @@ -16,7 +16,7 @@ import envoy.data.LoginCredentials; import envoy.event.HandshakeRejection; import envoy.util.*; -import envoy.client.data.*; +import envoy.client.data.ClientConfig; import envoy.client.ui.Startup; import envoy.client.util.IconUtil; @@ -79,11 +79,9 @@ public final class LoginScene implements EventListener { @FXML private void loginButtonPressed() { - final String user = userTextField.getText(), pass = passwordField.getText(), + final String user = userTextField.getText(), pass = passwordField.getText(), repeatPass = repeatPasswordField.getText(); - - // Choose whether to persist the token or not - Context.getInstance().getLocalDB().setSaveToken(cbStaySignedIn.isSelected()); + final boolean requestToken = cbStaySignedIn.isSelected(); // Prevent registration with unequal passwords if (registration && !pass.equals(repeatPass)) { @@ -98,8 +96,8 @@ public final class LoginScene implements EventListener { } else { Instant lastSync = Startup.loadLastSync(userTextField.getText()); Startup.performHandshake(registration - ? LoginCredentials.registration(user, pass, Startup.VERSION, lastSync) - : LoginCredentials.login(user, pass, Startup.VERSION, lastSync)); + ? LoginCredentials.registration(user, pass, requestToken, Startup.VERSION, lastSync) + : LoginCredentials.login(user, pass, requestToken, Startup.VERSION, lastSync)); } } diff --git a/common/src/main/java/envoy/data/AuthenticatedRequest.java b/common/src/main/java/envoy/data/AuthenticatedRequest.java index fa2f6ce..3728be7 100644 --- a/common/src/main/java/envoy/data/AuthenticatedRequest.java +++ b/common/src/main/java/envoy/data/AuthenticatedRequest.java @@ -4,7 +4,7 @@ import java.io.Serializable; import java.util.Objects; /** - * Wraps any request sent to the server in the authentication details of a user. + * Enables checking for the server whether the client is really who he is supposed to be. * * @author Leon Hofmeister * @param the type of object to be sent @@ -12,30 +12,21 @@ import java.util.Objects; */ public final class AuthenticatedRequest implements Serializable { - private final T request; - private final String authentication; - private final long userID; + private final T request; + private final long userID; private static final long serialVersionUID = 1L; /** - * @param request the actual object that should be sent - * @param userID the ID of the currently logged in user - * @param authentication the authentication of the currently logged in user + * @param request the actual object that should be sent + * @param userID the ID of the currently logged in user * @since Envoy Common v0.3-beta */ - public AuthenticatedRequest(T request, long userID, String authentication) { - this.request = Objects.requireNonNull(request); - this.userID = userID; - this.authentication = authentication == null ? "" : authentication; + public AuthenticatedRequest(T request, long userID) { + this.request = Objects.requireNonNull(request); + this.userID = userID; } - /** - * @return the authentication token of the currently logged in user - * @since Envoy Common v0.3-beta - */ - public String getAuthentication() { return authentication; } - /** * @return the request * @since Envoy Common v0.3-beta diff --git a/common/src/main/java/envoy/data/LoginCredentials.java b/common/src/main/java/envoy/data/LoginCredentials.java index c0a455a..4ab448d 100644 --- a/common/src/main/java/envoy/data/LoginCredentials.java +++ b/common/src/main/java/envoy/data/LoginCredentials.java @@ -15,18 +15,18 @@ import java.util.Objects; public final class LoginCredentials implements Serializable { private final String identifier, password, clientVersion; - private final boolean registration, token; + private final boolean registration, token, requestToken; private final Instant lastSync; private static final long serialVersionUID = 4; private LoginCredentials(String identifier, String password, boolean registration, - boolean token, String clientVersion, - Instant lastSync) { + boolean token, boolean requestToken, String clientVersion, Instant lastSync) { this.identifier = Objects.requireNonNull(identifier); this.password = Objects.requireNonNull(password); this.registration = registration; this.token = token; + this.requestToken = requestToken; this.clientVersion = Objects.requireNonNull(clientVersion); this.lastSync = lastSync == null ? Instant.EPOCH : lastSync; } @@ -36,14 +36,15 @@ public final class LoginCredentials implements Serializable { * * @param identifier the identifier of the user * @param password the password of the user + * @param requestToken requests the server to generate an authentication token * @param clientVersion the version of the client sending these credentials * @param lastSync the timestamp of the last synchronization * @return the created login credentials * @since Envoy Common v0.2-beta */ - public static LoginCredentials login(String identifier, String password, + public static LoginCredentials login(String identifier, String password, boolean requestToken, String clientVersion, Instant lastSync) { - return new LoginCredentials(identifier, password, false, false, clientVersion, + return new LoginCredentials(identifier, password, false, false, requestToken, clientVersion, lastSync); } @@ -59,7 +60,7 @@ public final class LoginCredentials implements Serializable { */ public static LoginCredentials loginWithToken(String identifier, String token, String clientVersion, Instant lastSync) { - return new LoginCredentials(identifier, token, false, true, clientVersion, lastSync); + return new LoginCredentials(identifier, token, false, true, false, clientVersion, lastSync); } /** @@ -67,24 +68,27 @@ public final class LoginCredentials implements Serializable { * * @param identifier the identifier of the user * @param password the password of the user + * @param requestToken requests the server to generate an authentication token * @param clientVersion the version of the client sending these credentials * @param lastSync the timestamp of the last synchronization * @return the created login credentials * @since Envoy Common v0.2-beta */ public static LoginCredentials registration(String identifier, String password, + boolean requestToken, String clientVersion, Instant lastSync) { - return new LoginCredentials(identifier, password, true, false, clientVersion, + return new LoginCredentials(identifier, password, true, false, requestToken, clientVersion, lastSync); } @Override public String toString() { return String.format( - "LoginCredentials[identifier=%s,registration=%b,token=%b,clientVersion=%s,lastSync=%s]", + "LoginCredentials[identifier=%s,registration=%b,token=%b,requestToken=%b,clientVersion=%s,lastSync=%s]", identifier, registration, token, + requestToken, clientVersion, lastSync); } @@ -116,6 +120,14 @@ public final class LoginCredentials implements Serializable { return token; } + /** + * @return {@code true} if the server should generate a new authentication token + * @since Envoy Common v0.2-beta + */ + public boolean requestToken() { + return requestToken; + } + /** * @return the version of the client sending these credentials * @since Envoy Common v0.1-beta diff --git a/server/src/main/java/envoy/server/net/ObjectMessageProcessor.java b/server/src/main/java/envoy/server/net/ObjectMessageProcessor.java index 6bbde84..652b21a 100755 --- a/server/src/main/java/envoy/server/net/ObjectMessageProcessor.java +++ b/server/src/main/java/envoy/server/net/ObjectMessageProcessor.java @@ -6,12 +6,11 @@ import java.util.Set; import java.util.logging.*; import com.jenkov.nioserver.*; -import com.jenkov.nioserver.Message; import envoy.data.AuthenticatedRequest; import envoy.util.EnvoyLog; -import envoy.server.data.*; +import envoy.server.data.PersistenceManager; import envoy.server.processors.ObjectProcessor; /** @@ -49,32 +48,25 @@ public final class ObjectMessageProcessor implements IMessageProcessor { // authenticate requests if necessary boolean authenticated = false; + if (obj instanceof AuthenticatedRequest) + try { + authenticated = PersistenceManager + .getInstance().getUserByID(((AuthenticatedRequest) obj).getUserID()) + .getID() == ConnectionManager.getInstance() + .getUserIDBySocketID(message.socketId); - if (obj instanceof AuthenticatedRequest) { - Contact contact = PersistenceManager.getInstance() - .getContactByID(((AuthenticatedRequest) obj).getUserID()); - - // Validating the authenticity of the request - if (contact == null || contact instanceof Group - || !((AuthenticatedRequest) obj).getAuthentication() - .equals(((User) contact).getAuthToken())) { - - // Invalid request - logger.log(Level.INFO, - "A user tried to perform an authenticated request but could not identify himself. Discarding request."); - return; + // Class cast exception and NullPointerException are valid here and signify a + // failed authentication + } catch (ClassCastException | NullPointerException e) {} finally { + obj = ((AuthenticatedRequest) obj).getRequest(); } - - // Valid request - logger.log(Level.INFO, "A user successfully authenticated a request for " + obj); - authenticated = true; - obj = ((AuthenticatedRequest) obj).getRequest(); - } else - logger.log(Level.FINE, "Received unauthenticated " + obj); + logger.log(Level.INFO, + "Received " + (authenticated ? "" : "un") + "authenticated " + obj); refer(message.socketId, writeProxy, obj, authenticated); } catch (IOException | ClassNotFoundException e) { - e.printStackTrace(); + logger.log(Level.WARNING, + "An exception occurred when reading in an object: " + e); } } diff --git a/server/src/main/java/envoy/server/processors/LoginCredentialProcessor.java b/server/src/main/java/envoy/server/processors/LoginCredentialProcessor.java index 92c8578..77faaa5 100755 --- a/server/src/main/java/envoy/server/processors/LoginCredentialProcessor.java +++ b/server/src/main/java/envoy/server/processors/LoginCredentialProcessor.java @@ -124,22 +124,23 @@ public final class LoginCredentialProcessor implements ObjectProcessor