From 7a883861be141c2e59273e7b1a3ca1dc4b3c54ba Mon Sep 17 00:00:00 2001 From: delvh Date: Tue, 27 Oct 2020 23:01:44 +0100 Subject: [PATCH] Apply suggestions by @kske --- .../main/java/envoy/client/net/Client.java | 5 +- .../envoy/client/ui/controller/ChatScene.java | 15 +---- .../client/ui/settings/UserSettingsPane.java | 10 +-- .../java/envoy/data/AuthenticatedRequest.java | 61 ------------------- .../envoy/event/GroupMessageStatusChange.java | 2 +- .../src/main/java/envoy/event/IsTyping.java | 19 ++---- .../envoy/event/PasswordChangeRequest.java | 19 ++---- .../java/envoy/event/ProfilePicChange.java | 14 +---- .../server/net/ObjectMessageProcessor.java | 32 ++-------- .../processors/GroupMessageProcessor.java | 12 +++- .../GroupMessageStatusChangeProcessor.java | 9 +++ .../server/processors/IsTypingProcessor.java | 9 +-- .../processors/IssueProposalProcessor.java | 3 - .../processors/LoginCredentialProcessor.java | 3 - .../server/processors/MessageProcessor.java | 10 +++ .../server/processors/ObjectProcessor.java | 6 -- .../PasswordChangeRequestProcessor.java | 5 +- .../server/util/UserAuthenticationUtil.java | 24 ++++++++ 18 files changed, 85 insertions(+), 173 deletions(-) delete mode 100644 common/src/main/java/envoy/data/AuthenticatedRequest.java create mode 100644 server/src/main/java/envoy/server/util/UserAuthenticationUtil.java diff --git a/client/src/main/java/envoy/client/net/Client.java b/client/src/main/java/envoy/client/net/Client.java index d4ed164..fe1c9e2 100644 --- a/client/src/main/java/envoy/client/net/Client.java +++ b/client/src/main/java/envoy/client/net/Client.java @@ -151,10 +151,7 @@ public final class Client implements EventListener, Closeable { checkOnline(); logger.log(Level.FINE, "Sending " + obj); try { - SerializationUtils.writeBytesWithLength( - new AuthenticatedRequest<>(obj, - Context.getInstance().getLocalDB().getUser().getID()), - socket.getOutputStream()); + SerializationUtils.writeBytesWithLength(obj, socket.getOutputStream()); } catch (final IOException e) { throw new RuntimeException(e); } diff --git a/client/src/main/java/envoy/client/ui/controller/ChatScene.java b/client/src/main/java/envoy/client/ui/controller/ChatScene.java index c7d6034..ceb27cd 100644 --- a/client/src/main/java/envoy/client/ui/controller/ChatScene.java +++ b/client/src/main/java/envoy/client/ui/controller/ChatScene.java @@ -593,7 +593,7 @@ public final class ChatScene implements EventListener, Restorable { // IsTyping#millisecondsActive if (client.isOnline() && currentChat.getLastWritingEvent() + IsTyping.millisecondsActive <= System.currentTimeMillis()) { - client.send(new IsTyping(getChatID(), currentChat.getRecipient().getID())); + client.send(new IsTyping(currentChat.getRecipient().getID())); currentChat.lastWritingEventWasNow(); } @@ -607,19 +607,6 @@ public final class ChatScene implements EventListener, Restorable { checkPostConditions(false); } - /** - * Returns the id that should be used to send things to the server: the id of 'our' {@link User} - * if the recipient of that object is another User, else the id of the {@link Group} 'our' user - * is sending to. - * - * @return an id that can be sent to the server - * @since Envoy Client v0.2-beta - */ - private long getChatID() { - return currentChat.getRecipient() instanceof User ? client.getSender().getID() - : currentChat.getRecipient().getID(); - } - /** * @param e the keys that have been pressed * @since Envoy Client v0.1-beta diff --git a/client/src/main/java/envoy/client/ui/settings/UserSettingsPane.java b/client/src/main/java/envoy/client/ui/settings/UserSettingsPane.java index 75a97d1..c3790a2 100644 --- a/client/src/main/java/envoy/client/ui/settings/UserSettingsPane.java +++ b/client/src/main/java/envoy/client/ui/settings/UserSettingsPane.java @@ -137,7 +137,7 @@ public final class UserSettingsPane extends OnlineOnlySettingsPane { // Displaying the save button saveButton - .setOnAction(e -> save(client.getSender().getID(), currentPasswordField.getText())); + .setOnAction(e -> save(currentPasswordField.getText())); saveButton.setAlignment(Pos.BOTTOM_RIGHT); getChildren().add(saveButton); } @@ -148,11 +148,11 @@ public final class UserSettingsPane extends OnlineOnlySettingsPane { * @param username the new username * @since Envoy Client v0.2-beta */ - private void save(long userID, String oldPassword) { + private void save(String oldPassword) { // The profile pic was changed if (profilePicChanged) { - final var profilePicChangeEvent = new ProfilePicChange(currentImageBytes, userID); + final var profilePicChangeEvent = new ProfilePicChange(currentImageBytes); eventBus.dispatch(profilePicChangeEvent); client.send(profilePicChangeEvent); logger.log(Level.INFO, "The user just changed his profile pic."); @@ -161,7 +161,7 @@ public final class UserSettingsPane extends OnlineOnlySettingsPane { // The username was changed final var validContactName = Bounds.isValidContactName(newUsername); if (usernameChanged && validContactName) { - final var nameChangeEvent = new NameChange(userID, newUsername); + final var nameChangeEvent = new NameChange(client.getSender().getID(), newUsername); eventBus.dispatch(nameChangeEvent); client.send(nameChangeEvent); logger.log(Level.INFO, "The user just changed his name to " + newUsername + "."); @@ -178,7 +178,7 @@ public final class UserSettingsPane extends OnlineOnlySettingsPane { // The password was changed if (validPassword) { - client.send(new PasswordChangeRequest(newPassword, oldPassword, userID)); + client.send(new PasswordChangeRequest(newPassword, oldPassword)); logger.log(Level.INFO, "The user just tried to change his password!"); } else if (!(validPassword || newPassword.isBlank())) { final var alert = new Alert(AlertType.ERROR); diff --git a/common/src/main/java/envoy/data/AuthenticatedRequest.java b/common/src/main/java/envoy/data/AuthenticatedRequest.java deleted file mode 100644 index 3728be7..0000000 --- a/common/src/main/java/envoy/data/AuthenticatedRequest.java +++ /dev/null @@ -1,61 +0,0 @@ -package envoy.data; - -import java.io.Serializable; -import java.util.Objects; - -/** - * 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 - * @since Envoy Common v0.3-beta - */ -public final class AuthenticatedRequest implements Serializable { - - 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 - * @since Envoy Common v0.3-beta - */ - public AuthenticatedRequest(T request, long userID) { - this.request = Objects.requireNonNull(request); - this.userID = userID; - } - - /** - * @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 + "]"; - } -} diff --git a/common/src/main/java/envoy/event/GroupMessageStatusChange.java b/common/src/main/java/envoy/event/GroupMessageStatusChange.java index bd5f326..1e27a03 100644 --- a/common/src/main/java/envoy/event/GroupMessageStatusChange.java +++ b/common/src/main/java/envoy/event/GroupMessageStatusChange.java @@ -30,7 +30,7 @@ public final class GroupMessageStatusChange extends MessageStatusChange { } /** - * @return the memberID which the user who sends this event has + * @return the ID of the sender of this event * @since Envoy Common v0.1-beta */ public long getMemberID() { return memberID; } diff --git a/common/src/main/java/envoy/event/IsTyping.java b/common/src/main/java/envoy/event/IsTyping.java index 4e587b4..95cdd3c 100644 --- a/common/src/main/java/envoy/event/IsTyping.java +++ b/common/src/main/java/envoy/event/IsTyping.java @@ -8,8 +8,6 @@ package envoy.event; */ public final class IsTyping extends Event { - private final long destinationID; - private static final long serialVersionUID = 1L; /** @@ -22,20 +20,13 @@ public final class IsTyping extends Event { public static final int millisecondsActive = 3500; /** - * Creates a new {@code IsTyping} event with originator and recipient. + * Creates a new {@code IsTyping}. The client will only send the contact that should receive + * this event. The server will send the id of the contact who sent this event. * - * @param sourceID the ID of the originator - * @param destinationID the ID of the contact the user wrote to + * @param id the ID of the recipient (client)/ originator(server) * @since Envoy Common v0.2-beta */ - public IsTyping(long sourceID, long destinationID) { - super(sourceID); - this.destinationID = destinationID; + public IsTyping(long id) { + super(id); } - - /** - * @return the ID of the contact in whose chat the user typed something - * @since Envoy Common v0.2-beta - */ - public long getDestinationID() { return destinationID; } } diff --git a/common/src/main/java/envoy/event/PasswordChangeRequest.java b/common/src/main/java/envoy/event/PasswordChangeRequest.java index 023c482..54c83aa 100644 --- a/common/src/main/java/envoy/event/PasswordChangeRequest.java +++ b/common/src/main/java/envoy/event/PasswordChangeRequest.java @@ -2,37 +2,26 @@ package envoy.event; import java.util.Objects; -import envoy.data.Contact; - /** * @author Leon Hofmeister * @since Envoy Common v0.2-beta */ public final class PasswordChangeRequest extends Event { - private final long id; - private final String oldPassword; + private final String oldPassword; private static final long serialVersionUID = 0L; /** * @param newPassword the new password of that user * @param oldPassword the old password of that user - * @param userID the ID of the user who wants to change his password * @since Envoy Common v0.2-beta */ - public PasswordChangeRequest(String newPassword, String oldPassword, long userID) { + public PasswordChangeRequest(String newPassword, String oldPassword) { super(newPassword); - this.oldPassword = Objects.requireNonNull(oldPassword); - id = userID; + this.oldPassword = Objects.requireNonNull(oldPassword); } - /** - * @return the ID of the {@link Contact} this event is related to - * @since Envoy Common v0.2-alpha - */ - public long getID() { return id; } - /** * @return the old password of the underlying user * @since Envoy Common v0.2-beta @@ -41,6 +30,6 @@ public final class PasswordChangeRequest extends Event { @Override public String toString() { - return "PasswordChangeRequest[id=" + id + "]"; + return "PasswordChangeRequest[]"; } } diff --git a/common/src/main/java/envoy/event/ProfilePicChange.java b/common/src/main/java/envoy/event/ProfilePicChange.java index 3400dc4..248e2aa 100644 --- a/common/src/main/java/envoy/event/ProfilePicChange.java +++ b/common/src/main/java/envoy/event/ProfilePicChange.java @@ -6,23 +6,13 @@ package envoy.event; */ public final class ProfilePicChange extends Event { - private final long id; - private static final long serialVersionUID = 0L; /** - * @param value the byte[] of the new image - * @param userID the ID of the user who changed his profile pic + * @param value the byte[] of the new image * @since Envoy Common v0.2-beta */ - public ProfilePicChange(byte[] value, long userID) { + public ProfilePicChange(byte[] value) { super(value); - id = userID; } - - /** - * @return the ID of the user changing his profile pic - * @since Envoy Common v0.2-beta - */ - public long getId() { return id; } } diff --git a/server/src/main/java/envoy/server/net/ObjectMessageProcessor.java b/server/src/main/java/envoy/server/net/ObjectMessageProcessor.java index 652b21a..c40e866 100755 --- a/server/src/main/java/envoy/server/net/ObjectMessageProcessor.java +++ b/server/src/main/java/envoy/server/net/ObjectMessageProcessor.java @@ -7,10 +7,8 @@ import java.util.logging.*; import com.jenkov.nioserver.*; -import envoy.data.AuthenticatedRequest; import envoy.util.EnvoyLog; -import envoy.server.data.PersistenceManager; import envoy.server.processors.ObjectProcessor; /** @@ -46,24 +44,9 @@ public final class ObjectMessageProcessor implements IMessageProcessor { return; } - // 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); + logger.log(Level.INFO, "Received " + obj); - // Class cast exception and NullPointerException are valid here and signify a - // failed authentication - } catch (ClassCastException | NullPointerException e) {} finally { - obj = ((AuthenticatedRequest) obj).getRequest(); - } - logger.log(Level.INFO, - "Received " + (authenticated ? "" : "un") + "authenticated " + obj); - - refer(message.socketId, writeProxy, obj, authenticated); + refer(message.socketId, writeProxy, obj); } catch (IOException | ClassNotFoundException e) { logger.log(Level.WARNING, "An exception occurred when reading in an object: " + e); @@ -75,27 +58,20 @@ public final class ObjectMessageProcessor implements IMessageProcessor { * present. */ @SuppressWarnings("unchecked") - private void refer(long socketID, WriteProxy writeProxy, Object obj, boolean authenticated) { + private void refer(long socketID, WriteProxy writeProxy, Object obj) { // 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; - } - + if (c.equals(obj.getClass())) try { p.process(c.cast(obj), socketID, new ObjectWriteProxy(writeProxy)); break; } catch (IOException e) { logger.log(Level.SEVERE, "Exception during processor execution: ", e); } - } } } } diff --git a/server/src/main/java/envoy/server/processors/GroupMessageProcessor.java b/server/src/main/java/envoy/server/processors/GroupMessageProcessor.java index a3b5747..7e6d403 100644 --- a/server/src/main/java/envoy/server/processors/GroupMessageProcessor.java +++ b/server/src/main/java/envoy/server/processors/GroupMessageProcessor.java @@ -5,7 +5,7 @@ import static envoy.server.Startup.config; import java.time.Instant; import java.util.Collections; -import java.util.logging.Logger; +import java.util.logging.*; import javax.persistence.EntityExistsException; @@ -15,6 +15,7 @@ import envoy.util.EnvoyLog; import envoy.server.data.PersistenceManager; import envoy.server.net.*; +import envoy.server.util.UserAuthenticationUtil; /** * @author Maximilian Käfer @@ -29,6 +30,15 @@ public final class GroupMessageProcessor implements ObjectProcessor { throws IOException { final var contact = persistenceManager.getContactByID(event.get()); if (contact instanceof User) { - final var destinationID = event.getDestinationID(); - if (connectionManager.isOnline(destinationID)) - writeProxy.write(connectionManager.getSocketID(destinationID), event); + if (connectionManager.isOnline(event.get())) + writeProxy.write(connectionManager.getSocketID(event.get()), + new IsTyping(connectionManager.getUserIDBySocketID(socketID))); } else - writeProxy.writeToOnlineContacts(contact.getContacts(), event); + writeProxy.writeToOnlineContacts(contact.getContacts(), + new IsTyping(connectionManager.getUserIDBySocketID(socketID))); } } diff --git a/server/src/main/java/envoy/server/processors/IssueProposalProcessor.java b/server/src/main/java/envoy/server/processors/IssueProposalProcessor.java index 01e9b3b..cd3f857 100644 --- a/server/src/main/java/envoy/server/processors/IssueProposalProcessor.java +++ b/server/src/main/java/envoy/server/processors/IssueProposalProcessor.java @@ -22,9 +22,6 @@ public final class IssueProposalProcessor implements ObjectProcessor { @Override public void process(Message message, long socketID, ObjectWriteProxy writeProxy) { + + // Check whether the message has the expected parameters + if (!UserAuthenticationUtil.isExpectedUser(message.getSenderID(), socketID) + || persistenceManager.getContactByID(message.getRecipientID()) == null) { + logger.log(Level.INFO, + "Received a message with invalid parameters"); + return; + } + message.nextStatus(); // Convert to server message diff --git a/server/src/main/java/envoy/server/processors/ObjectProcessor.java b/server/src/main/java/envoy/server/processors/ObjectProcessor.java index 24aa2f0..ffae6bd 100755 --- a/server/src/main/java/envoy/server/processors/ObjectProcessor.java +++ b/server/src/main/java/envoy/server/processors/ObjectProcessor.java @@ -21,10 +21,4 @@ public interface ObjectProcessor { * @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; } } diff --git a/server/src/main/java/envoy/server/processors/PasswordChangeRequestProcessor.java b/server/src/main/java/envoy/server/processors/PasswordChangeRequestProcessor.java index eb0f453..101ad6b 100644 --- a/server/src/main/java/envoy/server/processors/PasswordChangeRequestProcessor.java +++ b/server/src/main/java/envoy/server/processors/PasswordChangeRequestProcessor.java @@ -7,7 +7,7 @@ import envoy.event.*; import envoy.util.EnvoyLog; import envoy.server.data.PersistenceManager; -import envoy.server.net.ObjectWriteProxy; +import envoy.server.net.*; import envoy.server.util.PasswordUtil; /** @@ -21,7 +21,8 @@ public final class PasswordChangeRequestProcessor public void process(PasswordChangeRequest event, long socketID, ObjectWriteProxy writeProxy) throws IOException { final var persistenceManager = PersistenceManager.getInstance(); - final var user = persistenceManager.getUserByID(event.getID()); + final var user = persistenceManager + .getUserByID(ConnectionManager.getInstance().getUserIDBySocketID(socketID)); final var logger = EnvoyLog.getLogger(PasswordChangeRequestProcessor.class); final var correctAuthentication = diff --git a/server/src/main/java/envoy/server/util/UserAuthenticationUtil.java b/server/src/main/java/envoy/server/util/UserAuthenticationUtil.java new file mode 100644 index 0000000..d4bf5b9 --- /dev/null +++ b/server/src/main/java/envoy/server/util/UserAuthenticationUtil.java @@ -0,0 +1,24 @@ +package envoy.server.util; + +import envoy.server.net.ConnectionManager; + +/** + * @author Leon Hofmeister + * @since Envoy Server v0.3-beta + */ +public final class UserAuthenticationUtil { + + private UserAuthenticationUtil() {} + + /** + * Checks whether a user is really who he claims to be. + * + * @param expectedID the expected user ID + * @param socketID the socket ID of the user making a request + * @return whether this user is who he claims to be + * @since Envoy Server v0.3-beta + */ + public static boolean isExpectedUser(long expectedID, long socketID) { + return ConnectionManager.getInstance().getUserIDBySocketID(socketID) == expectedID; + } +}