Fix bug allowing unauthorized access to a client

Additionally token authentication is now used whenever the client is
online

client/src/main/java/envoy/client/data/LocalDB.java

@@ -44,6 +44,7 @@ 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
@@ -260,7 +261,7 @@ public final class LocalDB implements EventListener {
 						Context.getInstance().getClient().isOnline() ? Instant.now() : lastSync);
 
 			// Save last login information
-			if (authToken != null)
+			if (saveToken && authToken != null)
 				SerializationUtils.write(lastLoginFile, user, authToken);
 
 			// Save ID generator
@@ -488,4 +489,10 @@ 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; }
 }

client/src/main/java/envoy/client/net/Client.java

@@ -151,7 +151,11 @@ public final class Client implements EventListener, Closeable {
 		checkOnline();
 		logger.log(Level.FINE, "Sending " + obj);
 		try {
-			SerializationUtils.writeBytesWithLength(obj, socket.getOutputStream());
+			SerializationUtils.writeBytesWithLength(
+				new AuthenticatedRequest<>(obj,
+					Context.getInstance().getLocalDB().getUser().getID(),
+					Context.getInstance().getLocalDB().getAuthToken()),
+				socket.getOutputStream());
 		} catch (final IOException e) {
 			throw new RuntimeException(e);
 		}

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.ClientConfig;
+import envoy.client.data.*;
 import envoy.client.ui.Startup;
 import envoy.client.util.IconUtil;
 
@@ -79,9 +79,11 @@ 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();
-		final boolean	requestToken	= cbStaySignedIn.isSelected();
+
+		// Choose whether to persist the token or not
+		Context.getInstance().getLocalDB().setSaveToken(cbStaySignedIn.isSelected());
 
 		// Prevent registration with unequal passwords
 		if (registration && !pass.equals(repeatPass)) {
@@ -96,8 +98,8 @@ public final class LoginScene implements EventListener {
 		} else {
 			Instant lastSync = Startup.loadLastSync(userTextField.getText());
 			Startup.performHandshake(registration
-				? LoginCredentials.registration(user, pass, requestToken, Startup.VERSION, lastSync)
+				? LoginCredentials.registration(user, pass, Startup.VERSION, lastSync)
-				: LoginCredentials.login(user, pass, requestToken, Startup.VERSION, lastSync));
+				: LoginCredentials.login(user, pass, Startup.VERSION, lastSync));
 		}
 	}
 

common/src/main/java/envoy/data/AuthenticatedRequest.java

@@ -0,0 +1,70 @@
+package envoy.data;
+
+import java.io.Serializable;
+import java.util.Objects;
+
+/**
+ * Wraps any request sent to the server in the authentication details of a user.
+ *
+ * @author Leon Hofmeister
+ * @param <T> the type of object to be sent
+ * @since Envoy Common v0.3-beta
+ */
+public final class AuthenticatedRequest<T extends Serializable> implements Serializable {
+
+	private final T			request;
+	private final String	authentication;
+	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
+	 * @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;
+	}
+
+	/**
+	 * @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
+	 */
+	public T getRequest() { return request; }
+
+	/**
+	 * @return the userID
+	 * @since Envoy Common v0.3-beta
+	 */
+	public long getUserID() { return userID; }
+
+	@Override
+	public int hashCode() {
+		return Objects.hash(request, userID);
+	}
+
+	@Override
+	public boolean equals(Object obj) {
+		if (this == obj)
+			return true;
+		if (!(obj instanceof AuthenticatedRequest))
+			return false;
+		AuthenticatedRequest<?> other = (AuthenticatedRequest<?>) obj;
+		return userID == other.userID;
+	}
+
+	@Override
+	public String toString() {
+		return "AuthenticatedRequest [request=" + request + ", userID=" + userID + "]";
+	}
+}

common/src/main/java/envoy/data/LoginCredentials.java

@@ -14,19 +14,18 @@ import java.time.Instant;
 public final class LoginCredentials implements Serializable {
 
 	private final String	identifier, password, clientVersion;
-	private final boolean	registration, token, requestToken;
+	private final boolean	registration, token;
 	private final Instant	lastSync;
 
 	private static final long serialVersionUID = 4;
 
 	private LoginCredentials(String identifier, String password, boolean registration,
-		boolean token, boolean requestToken, String clientVersion,
+		boolean token, String clientVersion,
 		Instant lastSync) {
 		this.identifier		= identifier;
 		this.password		= password;
 		this.registration	= registration;
 		this.token			= token;
-		this.requestToken	= requestToken;
 		this.clientVersion	= clientVersion;
 		this.lastSync		= lastSync;
 	}
@@ -36,15 +35,14 @@ 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, boolean requestToken,
+	public static LoginCredentials login(String identifier, String password,
 		String clientVersion, Instant lastSync) {
-		return new LoginCredentials(identifier, password, false, false, requestToken, clientVersion,
+		return new LoginCredentials(identifier, password, false, false, clientVersion,
 			lastSync);
 	}
 
@@ -60,7 +58,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, false, clientVersion, lastSync);
+		return new LoginCredentials(identifier, token, false, true, clientVersion, lastSync);
 	}
 
 	/**
@@ -68,26 +66,24 @@ 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) {
+		String clientVersion, Instant lastSync) {
-		return new LoginCredentials(identifier, password, true, false, requestToken, clientVersion,
+		return new LoginCredentials(identifier, password, true, false, clientVersion,
 			lastSync);
 	}
 
 	@Override
 	public String toString() {
 		return String.format(
-			"LoginCredentials[identifier=%s,registration=%b,token=%b,requestToken=%b,clientVersion=%s,lastSync=%s]",
+			"LoginCredentials[identifier=%s,registration=%b,token=%b,clientVersion=%s,lastSync=%s]",
 			identifier,
 			registration,
 			token,
-			requestToken,
 			clientVersion,
 			lastSync);
 	}
@@ -119,14 +115,6 @@ 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

server/src/main/java/envoy/server/net/ObjectMessageProcessor.java

@@ -6,9 +6,12 @@ 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.processors.ObjectProcessor;
 
 /**
@@ -33,7 +36,6 @@ public final class ObjectMessageProcessor implements IMessageProcessor {
 		this.processors = processors;
 	}
 
-	@SuppressWarnings("unchecked")
 	@Override
 	public void process(Message message, WriteProxy writeProxy) {
 		try (ObjectInputStream in =
@@ -45,23 +47,63 @@ public final class ObjectMessageProcessor implements IMessageProcessor {
 				return;
 			}
 
-			logger.fine("Received " + obj);
+			// authenticate requests if necessary
+			boolean authenticated = false;
 
-			// Get processor and input class and process object
+			if (obj instanceof AuthenticatedRequest) {
-			for (@SuppressWarnings("rawtypes")
+				Contact contact = PersistenceManager.getInstance()
-			ObjectProcessor p : processors) {
+					.getContactByID(((AuthenticatedRequest<?>) obj).getUserID());
-				Class<?> c = (Class<?>) ((ParameterizedType) p.getClass().getGenericInterfaces()[0])
+
-					.getActualTypeArguments()[0];
+				// Validating the authenticity of the request
-				if (c.equals(obj.getClass()))
+				if (contact == null || contact instanceof Group
-					try {
+					|| !((AuthenticatedRequest<?>) obj).getAuthentication()
-						p.process(c.cast(obj), message.socketId, new ObjectWriteProxy(writeProxy));
+						.equals(((User) contact).getAuthToken())) {
-						break;
+
-					} catch (IOException e) {
+					// Invalid request
-						logger.log(Level.SEVERE, "Exception during processor execution: ", e);
+					logger.log(Level.INFO,
-					}
+						"A user tried to perform an authenticated request but could not identify himself. Discarding request.");
-			}
+					return;
+				}
+
+				// 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);
+
+			refer(message.socketId, writeProxy, obj, authenticated);
 		} catch (IOException | ClassNotFoundException e) {
 			e.printStackTrace();
 		}
 	}
+
+	/**
+	 * Executes the appropriate {@link ObjectProcessor} for the given input ({@code obj}), if any is
+	 * present.
+	 */
+	@SuppressWarnings("unchecked")
+	private void refer(long socketID, WriteProxy writeProxy, Object obj, boolean authenticated) {
+
+		// Get processor and input class and process object
+		for (@SuppressWarnings("rawtypes")
+		ObjectProcessor p : processors) {
+			Class<?> c = (Class<?>) ((ParameterizedType) p.getClass().getGenericInterfaces()[0])
+				.getActualTypeArguments()[0];
+			if (c.equals(obj.getClass())) {
+				if (!authenticated && p.isAuthenticationRequired()) {
+					logger.log(Level.INFO,
+						"Discarding request as no authentication has been provided");
+					return;
+				}
+
+				try {
+					p.process(c.cast(obj), socketID, new ObjectWriteProxy(writeProxy));
+					break;
+				} catch (IOException e) {
+					logger.log(Level.SEVERE, "Exception during processor execution: ", e);
+				}
+			}
+		}
+	}
 }

server/src/main/java/envoy/server/processors/IssueProposalProcessor.java

@@ -22,6 +22,9 @@ public final class IssueProposalProcessor implements ObjectProcessor<IssuePropos
 
 	private static final Logger logger = EnvoyLog.getLogger(IssueProposalProcessor.class);
 
+	@Override
+	public boolean isAuthenticationRequired() { return false; }
+
 	@Override
 	public void process(IssueProposal issueProposal, long socketID, ObjectWriteProxy writeProxy)
 		throws IOException {

server/src/main/java/envoy/server/processors/LoginCredentialProcessor.java

@@ -34,6 +34,9 @@ public final class LoginCredentialProcessor implements ObjectProcessor<LoginCred
 
 	private static final Logger logger = EnvoyLog.getLogger(LoginCredentialProcessor.class);
 
+	@Override
+	public boolean isAuthenticationRequired() { return false; }
+
 	@Override
 	public void process(LoginCredentials credentials, long socketID, ObjectWriteProxy writeProxy) {
 
@@ -121,24 +124,21 @@ public final class LoginCredentialProcessor implements ObjectProcessor<LoginCred
 		UserStatusChangeProcessor.updateUserStatus(user, ONLINE);
 
 		// Process token request
-		if (credentials.requestToken()) {
+		String token;
-			String token;
+		if (user.getAuthToken() != null && user.getAuthTokenExpiration().isAfter(Instant.now()))
 
-			if (user.getAuthToken() != null && user.getAuthTokenExpiration().isAfter(Instant.now()))
+			// Reuse existing token and delay expiration date
+			token = user.getAuthToken();
+		else {
 
-				// Reuse existing token and delay expiration date
+			// Generate new token
-				token = user.getAuthToken();
+			token = AuthTokenGenerator.nextToken();
-			else {
+			user.setAuthToken(token);
-
-				// Generate new token
-				token = AuthTokenGenerator.nextToken();
-				user.setAuthToken(token);
-			}
-			user.setAuthTokenExpiration(Instant.now().plus(
-				ServerConfig.getInstance().getAuthTokenExpiration().longValue(), ChronoUnit.DAYS));
-			persistenceManager.updateContact(user);
-			writeProxy.write(socketID, new NewAuthToken(token));
 		}
+		user.setAuthTokenExpiration(Instant.now().plus(
+			ServerConfig.getInstance().getAuthTokenExpiration().longValue(), ChronoUnit.DAYS));
+		persistenceManager.updateContact(user);
+		writeProxy.write(socketID, new NewAuthToken(token));
 
 		final var pendingMessages =
 			PersistenceManager.getInstance().getPendingMessages(user, credentials.getLastSync());

server/src/main/java/envoy/server/processors/ObjectProcessor.java

@@ -21,4 +21,10 @@ public interface ObjectProcessor<T> {
 	 * @since Envoy Server Standalone v0.1-alpha
 	 */
 	void process(T input, long socketID, ObjectWriteProxy writeProxy) throws IOException;
+
+	/**
+	 * @return whether authentication is required for the given processor. Defaults to {@code true}.
+	 * @since Envoy Server v0.3-beta
+	 */
+	default boolean isAuthenticationRequired() { return true; }
 }