From 44d30829583f6f7daebdf5689e6a963f4199f993 Mon Sep 17 00:00:00 2001 From: delvh Date: Thu, 22 Oct 2020 23:05:51 +0200 Subject: [PATCH 1/5] Fix bug allowing unauthorized access to a client Additionally token authentication is now used whenever the client is online --- .../main/java/envoy/client/data/LocalDB.java | 9 ++- .../main/java/envoy/client/net/Client.java | 6 +- .../client/ui/controller/LoginScene.java | 12 ++-- .../java/envoy/data/AuthenticatedRequest.java | 70 ++++++++++++++++++ .../java/envoy/data/LoginCredentials.java | 28 +++----- .../server/net/ObjectMessageProcessor.java | 72 +++++++++++++++---- .../processors/IssueProposalProcessor.java | 3 + .../processors/LoginCredentialProcessor.java | 30 ++++---- .../server/processors/ObjectProcessor.java | 6 ++ 9 files changed, 179 insertions(+), 57 deletions(-) create mode 100644 common/src/main/java/envoy/data/AuthenticatedRequest.java diff --git a/client/src/main/java/envoy/client/data/LocalDB.java b/client/src/main/java/envoy/client/data/LocalDB.java index e3c4ac7..1edede8 100644 --- a/client/src/main/java/envoy/client/data/LocalDB.java +++ b/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; } } diff --git a/client/src/main/java/envoy/client/net/Client.java b/client/src/main/java/envoy/client/net/Client.java index fe1c9e2..8c8e682 100644 --- a/client/src/main/java/envoy/client/net/Client.java +++ b/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); } 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 f9d062b..fc9a71f 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.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.login(user, pass, requestToken, Startup.VERSION, lastSync)); + ? LoginCredentials.registration(user, pass, Startup.VERSION, lastSync) + : LoginCredentials.login(user, pass, Startup.VERSION, lastSync)); } } diff --git a/common/src/main/java/envoy/data/AuthenticatedRequest.java b/common/src/main/java/envoy/data/AuthenticatedRequest.java new file mode 100644 index 0000000..fa2f6ce --- /dev/null +++ b/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 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 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 + "]"; + } +} diff --git a/common/src/main/java/envoy/data/LoginCredentials.java b/common/src/main/java/envoy/data/LoginCredentials.java index 292c2e2..f121005 100644 --- a/common/src/main/java/envoy/data/LoginCredentials.java +++ b/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) { - return new LoginCredentials(identifier, password, true, false, requestToken, clientVersion, + String clientVersion, Instant lastSync) { + 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 diff --git a/server/src/main/java/envoy/server/net/ObjectMessageProcessor.java b/server/src/main/java/envoy/server/net/ObjectMessageProcessor.java index 0148be7..6bbde84 100755 --- a/server/src/main/java/envoy/server/net/ObjectMessageProcessor.java +++ b/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 - for (@SuppressWarnings("rawtypes") - ObjectProcessor p : processors) { - Class c = (Class) ((ParameterizedType) p.getClass().getGenericInterfaces()[0]) - .getActualTypeArguments()[0]; - if (c.equals(obj.getClass())) - try { - p.process(c.cast(obj), message.socketId, new ObjectWriteProxy(writeProxy)); - break; - } catch (IOException e) { - logger.log(Level.SEVERE, "Exception during processor execution: ", e); - } - } + 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; + } + + // 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); + } + } + } + } } diff --git a/server/src/main/java/envoy/server/processors/IssueProposalProcessor.java b/server/src/main/java/envoy/server/processors/IssueProposalProcessor.java index cd3f857..01e9b3b 100644 --- a/server/src/main/java/envoy/server/processors/IssueProposalProcessor.java +++ b/server/src/main/java/envoy/server/processors/IssueProposalProcessor.java @@ -22,6 +22,9 @@ public final class IssueProposalProcessor implements 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; } } From 2eeb55ed52479719761456ca44dd611eafbcce32 Mon Sep 17 00:00:00 2001 From: delvh Date: Thu, 22 Oct 2020 23:58:55 +0200 Subject: [PATCH 2/5] Add client side errors in case of data initialization with null values --- .../src/main/java/envoy/data/Attachment.java | 7 +-- common/src/main/java/envoy/data/Contact.java | 4 +- .../main/java/envoy/data/GroupMessage.java | 3 +- .../java/envoy/data/LoginCredentials.java | 9 ++-- common/src/main/java/envoy/data/Message.java | 5 ++- common/src/main/java/envoy/data/User.java | 2 +- common/src/main/java/envoy/event/Event.java | 14 +++++- .../java/envoy/event/GroupCreationResult.java | 2 +- .../main/java/envoy/event/GroupResize.java | 4 +- .../src/main/java/envoy/event/IsTyping.java | 2 +- .../main/java/envoy/event/IssueProposal.java | 6 +-- .../java/envoy/event/MessageStatusChange.java | 3 +- .../envoy/event/PasswordChangeRequest.java | 4 +- .../envoy/event/contact/UserOperation.java | 4 +- .../envoy/server/data/PersistenceManager.java | 44 ++++++++++++------- 15 files changed, 72 insertions(+), 41 deletions(-) diff --git a/common/src/main/java/envoy/data/Attachment.java b/common/src/main/java/envoy/data/Attachment.java index 0be0e1f..fc79d9f 100644 --- a/common/src/main/java/envoy/data/Attachment.java +++ b/common/src/main/java/envoy/data/Attachment.java @@ -1,6 +1,7 @@ package envoy.data; import java.io.Serializable; +import java.util.Objects; /** * This interface should be used for any type supposed to be a {@link Message} attachment (i.e. @@ -63,9 +64,9 @@ public final class Attachment implements Serializable { * @since Envoy Common v0.1-beta */ public Attachment(byte[] data, String name, AttachmentType type) { - this.data = data; - this.name = name; - this.type = type; + this.data = Objects.requireNonNull(data); + this.name = Objects.requireNonNull(name); + this.type = Objects.requireNonNull(type); } /** diff --git a/common/src/main/java/envoy/data/Contact.java b/common/src/main/java/envoy/data/Contact.java index 8e250dc..18e0e4f 100644 --- a/common/src/main/java/envoy/data/Contact.java +++ b/common/src/main/java/envoy/data/Contact.java @@ -29,8 +29,8 @@ public abstract class Contact implements Serializable { */ public Contact(long id, String name, Set contacts) { this.id = id; - this.name = name; - this.contacts = contacts; + this.name = Objects.requireNonNull(name); + this.contacts = contacts == null ? new HashSet<>() : contacts; } /** diff --git a/common/src/main/java/envoy/data/GroupMessage.java b/common/src/main/java/envoy/data/GroupMessage.java index d6a30a9..8f2f28a 100644 --- a/common/src/main/java/envoy/data/GroupMessage.java +++ b/common/src/main/java/envoy/data/GroupMessage.java @@ -38,7 +38,8 @@ public final class GroupMessage extends Message { Map memberStatuses) { super(id, senderID, groupID, creationDate, receivedDate, readDate, text, attachment, status, forwarded); - this.memberStatuses = memberStatuses; + this.memberStatuses = + memberStatuses == null ? new HashMap<>() : memberStatuses; } /** diff --git a/common/src/main/java/envoy/data/LoginCredentials.java b/common/src/main/java/envoy/data/LoginCredentials.java index f121005..c0a455a 100644 --- a/common/src/main/java/envoy/data/LoginCredentials.java +++ b/common/src/main/java/envoy/data/LoginCredentials.java @@ -2,6 +2,7 @@ package envoy.data; import java.io.Serializable; import java.time.Instant; +import java.util.Objects; /** * Contains a {@link User}'s login / registration information as well as the client version. @@ -22,12 +23,12 @@ public final class LoginCredentials implements Serializable { private LoginCredentials(String identifier, String password, boolean registration, boolean token, String clientVersion, Instant lastSync) { - this.identifier = identifier; - this.password = password; + this.identifier = Objects.requireNonNull(identifier); + this.password = Objects.requireNonNull(password); this.registration = registration; this.token = token; - this.clientVersion = clientVersion; - this.lastSync = lastSync; + this.clientVersion = Objects.requireNonNull(clientVersion); + this.lastSync = lastSync == null ? Instant.EPOCH : lastSync; } /** diff --git a/common/src/main/java/envoy/data/Message.java b/common/src/main/java/envoy/data/Message.java index b8965a3..78a5ad6 100644 --- a/common/src/main/java/envoy/data/Message.java +++ b/common/src/main/java/envoy/data/Message.java @@ -2,6 +2,7 @@ package envoy.data; import java.io.Serializable; import java.time.Instant; +import java.util.Objects; import dev.kske.eventbus.IEvent; @@ -80,9 +81,9 @@ public class Message implements Serializable, IEvent { this.creationDate = creationDate; this.receivedDate = receivedDate; this.readDate = readDate; - this.text = text; + this.text = text == null ? "" : text; this.attachment = attachment; - this.status = status; + this.status = Objects.requireNonNull(status); this.forwarded = forwarded; } diff --git a/common/src/main/java/envoy/data/User.java b/common/src/main/java/envoy/data/User.java index f0f2a62..6b7c580 100644 --- a/common/src/main/java/envoy/data/User.java +++ b/common/src/main/java/envoy/data/User.java @@ -86,7 +86,7 @@ public final class User extends Contact { */ public User(long id, String name, UserStatus status, Set contacts) { super(id, name, contacts); - this.status = status; + this.status = Objects.requireNonNull(status); } @Override diff --git a/common/src/main/java/envoy/event/Event.java b/common/src/main/java/envoy/event/Event.java index e12b882..5812878 100644 --- a/common/src/main/java/envoy/event/Event.java +++ b/common/src/main/java/envoy/event/Event.java @@ -1,6 +1,7 @@ package envoy.event; import java.io.Serializable; +import java.util.Objects; import dev.kske.eventbus.IEvent; @@ -20,7 +21,16 @@ public abstract class Event implements IEvent, Serializable { private static final long serialVersionUID = 0L; protected Event(T value) { - this.value = value; + this(value, false); + } + + /** + * This constructor is reserved for {@link Valueless} events. No other event should contain null + * values. Only use if really necessary. Using this constructor with {@code true} implies that + * the user has to manually check if the value of the event is null. + */ + protected Event(T value, boolean canBeNull) { + this.value = canBeNull ? value : Objects.requireNonNull(value); } /** @@ -46,7 +56,7 @@ public abstract class Event implements IEvent, Serializable { private static final long serialVersionUID = 0L; protected Valueless() { - super(null); + super(null, true); } @Override diff --git a/common/src/main/java/envoy/event/GroupCreationResult.java b/common/src/main/java/envoy/event/GroupCreationResult.java index c1a002b..2c01b44 100644 --- a/common/src/main/java/envoy/event/GroupCreationResult.java +++ b/common/src/main/java/envoy/event/GroupCreationResult.java @@ -20,7 +20,7 @@ public class GroupCreationResult extends Event { * @since Envoy Common v0.2-beta */ public GroupCreationResult() { - super(null); + super(null, true); } /** diff --git a/common/src/main/java/envoy/event/GroupResize.java b/common/src/main/java/envoy/event/GroupResize.java index 69e0447..3679f14 100644 --- a/common/src/main/java/envoy/event/GroupResize.java +++ b/common/src/main/java/envoy/event/GroupResize.java @@ -2,6 +2,8 @@ package envoy.event; import static envoy.event.ElementOperation.*; +import java.util.Objects; + import envoy.data.*; /** @@ -30,7 +32,7 @@ public final class GroupResize extends Event { */ public GroupResize(User user, Group group, ElementOperation operation) { super(user); - this.operation = operation; + this.operation = Objects.requireNonNull(operation); final var contained = group.getContacts().contains(user); if (contained && operation.equals(ADD)) throw new IllegalArgumentException(String.format("Cannot add %s to %s!", user, group)); diff --git a/common/src/main/java/envoy/event/IsTyping.java b/common/src/main/java/envoy/event/IsTyping.java index 4c5e93e..4e587b4 100644 --- a/common/src/main/java/envoy/event/IsTyping.java +++ b/common/src/main/java/envoy/event/IsTyping.java @@ -28,7 +28,7 @@ public final class IsTyping extends Event { * @param destinationID the ID of the contact the user wrote to * @since Envoy Common v0.2-beta */ - public IsTyping(Long sourceID, long destinationID) { + public IsTyping(long sourceID, long destinationID) { super(sourceID); this.destinationID = destinationID; } diff --git a/common/src/main/java/envoy/event/IssueProposal.java b/common/src/main/java/envoy/event/IssueProposal.java index d015e88..0b2cc5f 100644 --- a/common/src/main/java/envoy/event/IssueProposal.java +++ b/common/src/main/java/envoy/event/IssueProposal.java @@ -23,7 +23,7 @@ public final class IssueProposal extends Event { */ public IssueProposal(String title, String description, boolean isBug) { super(escape(title)); - this.description = sanitizeDescription(description); + this.description = description == null ? "" : sanitizeDescription(description); bug = isBug; } @@ -37,8 +37,8 @@ public final class IssueProposal extends Event { */ public IssueProposal(String title, String description, String user, boolean isBug) { super(escape(title)); - this.description = - sanitizeDescription(description) + String.format("
Submitted by user %s.", user); + this.description = description == null ? "" + : sanitizeDescription(description) + String.format("
Submitted by user %s.", user); bug = isBug; } diff --git a/common/src/main/java/envoy/event/MessageStatusChange.java b/common/src/main/java/envoy/event/MessageStatusChange.java index 5bdacde..5581f67 100644 --- a/common/src/main/java/envoy/event/MessageStatusChange.java +++ b/common/src/main/java/envoy/event/MessageStatusChange.java @@ -1,6 +1,7 @@ package envoy.event; import java.time.Instant; +import java.util.Objects; import envoy.data.Message; @@ -26,7 +27,7 @@ public class MessageStatusChange extends Event { public MessageStatusChange(long id, Message.MessageStatus status, Instant date) { super(status); this.id = id; - this.date = date; + this.date = Objects.requireNonNull(date); } /** diff --git a/common/src/main/java/envoy/event/PasswordChangeRequest.java b/common/src/main/java/envoy/event/PasswordChangeRequest.java index b1b860c..023c482 100644 --- a/common/src/main/java/envoy/event/PasswordChangeRequest.java +++ b/common/src/main/java/envoy/event/PasswordChangeRequest.java @@ -1,5 +1,7 @@ package envoy.event; +import java.util.Objects; + import envoy.data.Contact; /** @@ -21,7 +23,7 @@ public final class PasswordChangeRequest extends Event { */ public PasswordChangeRequest(String newPassword, String oldPassword, long userID) { super(newPassword); - this.oldPassword = oldPassword; + this.oldPassword = Objects.requireNonNull(oldPassword); id = userID; } diff --git a/common/src/main/java/envoy/event/contact/UserOperation.java b/common/src/main/java/envoy/event/contact/UserOperation.java index bbec1c9..fd4de7f 100644 --- a/common/src/main/java/envoy/event/contact/UserOperation.java +++ b/common/src/main/java/envoy/event/contact/UserOperation.java @@ -1,5 +1,7 @@ package envoy.event.contact; +import java.util.Objects; + import envoy.data.User; import envoy.event.*; @@ -24,7 +26,7 @@ public final class UserOperation extends Event { */ public UserOperation(User contact, ElementOperation operationType) { super(contact); - this.operationType = operationType; + this.operationType = Objects.requireNonNull(operationType); } /** diff --git a/server/src/main/java/envoy/server/data/PersistenceManager.java b/server/src/main/java/envoy/server/data/PersistenceManager.java index ddabe7b..d2e1f22 100755 --- a/server/src/main/java/envoy/server/data/PersistenceManager.java +++ b/server/src/main/java/envoy/server/data/PersistenceManager.java @@ -1,7 +1,7 @@ package envoy.server.data; import java.time.Instant; -import java.util.List; +import java.util.*; import java.util.logging.Level; import javax.persistence.*; @@ -223,6 +223,9 @@ public final class PersistenceManager { * @since Envoy Server Standalone v0.2-beta */ public List getPendingMessages(User user, Instant lastSync) { + if (user == null) + return new ArrayList<>(); + lastSync = Objects.requireNonNullElse(lastSync, Instant.EPOCH); return entityManager.createNamedQuery(Message.getPending).setParameter("user", user) .setParameter("lastSeen", lastSync).getResultList(); } @@ -236,6 +239,9 @@ public final class PersistenceManager { * @since Envoy Server Standalone v0.2-beta */ public List getPendingGroupMessages(User user, Instant lastSync) { + if (user == null) + return new ArrayList<>(); + lastSync = Objects.requireNonNullElse(lastSync, Instant.EPOCH); return entityManager.createNamedQuery(GroupMessage.getPendingGroupMsg) .setParameter("userId", user.getID()) .setParameter("lastSeen", lastSync) @@ -277,16 +283,18 @@ public final class PersistenceManager { * @since Envoy Server v0.3-beta */ public void addContactBidirectional(Contact contact1, Contact contact2) { + if (!(contact1 == null || contact2 == null)) { - // Add users to each others contact list - contact1.getContacts().add(contact2); - contact2.getContacts().add(contact1); + // Add users to each others contact list + contact1.getContacts().add(contact2); + contact2.getContacts().add(contact1); - // Synchronize changes with the database - transaction(() -> { - entityManager.merge(contact1); - entityManager.merge(contact2); - }); + // Synchronize changes with the database + transaction(() -> { + entityManager.merge(contact1); + entityManager.merge(contact2); + }); + } } /** @@ -308,16 +316,18 @@ public final class PersistenceManager { * @since Envoy Server v0.3-beta */ public void removeContactBidirectional(Contact contact1, Contact contact2) { + if (!(contact1 == null || contact2 == null)) { - // Remove users from each others contact list - contact1.getContacts().remove(contact2); - contact2.getContacts().remove(contact1); + // Remove users from each others contact list + contact1.getContacts().remove(contact2); + contact2.getContacts().remove(contact1); - // Synchronize changes with the database - transaction(() -> { - entityManager.merge(contact1); - entityManager.merge(contact2); - }); + // Synchronize changes with the database + transaction(() -> { + entityManager.merge(contact1); + entityManager.merge(contact2); + }); + } } /** From fccd7e70b19c09983abc396a05dc4f4a4451b7f7 Mon Sep 17 00:00:00 2001 From: delvh Date: Fri, 23 Oct 2020 00:15:37 +0200 Subject: [PATCH 3/5] Disable crashing the server when Hibernate panics after oopsing --- .../envoy/server/data/PersistenceManager.java | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/envoy/server/data/PersistenceManager.java b/server/src/main/java/envoy/server/data/PersistenceManager.java index d2e1f22..c79296b 100755 --- a/server/src/main/java/envoy/server/data/PersistenceManager.java +++ b/server/src/main/java/envoy/server/data/PersistenceManager.java @@ -341,15 +341,36 @@ public final class PersistenceManager { } private void persist(Object obj) { - transaction(() -> entityManager.persist(obj)); + try { + transaction(() -> entityManager.persist(obj)); + } catch (EntityExistsException e) { + if (transaction.isActive()) + transaction.rollback(); + EnvoyLog.getLogger(PersistenceManager.class).log(Level.WARNING, + String.format("Could not persist %s: entity exists already.", obj)); + } } private void merge(Object obj) { - transaction(() -> entityManager.merge(obj)); + try { + transaction(() -> entityManager.merge(obj)); + } catch (IllegalArgumentException e) { + if (transaction.isActive()) + transaction.rollback(); + EnvoyLog.getLogger(PersistenceManager.class).log(Level.WARNING, + String.format("Could not merge %s: entity doesn't exist.", obj)); + } } private void remove(Object obj) { - transaction(() -> entityManager.remove(obj)); + try { + transaction(() -> entityManager.remove(obj)); + } catch (IllegalArgumentException e) { + if (transaction.isActive()) + transaction.rollback(); + EnvoyLog.getLogger(PersistenceManager.class).log(Level.WARNING, + String.format("Could not remove %s: entity didn't exist (for the database).", obj)); + } } /** From d4c7813c977ef8e5bd928b37e8659c5aa6db2ba0 Mon Sep 17 00:00:00 2001 From: delvh Date: Fri, 23 Oct 2020 18:45:40 +0200 Subject: [PATCH 4/5] 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 Date: Tue, 27 Oct 2020 23:01:44 +0100 Subject: [PATCH 5/5] 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; + } +}