Merge pull request 'Made Server Less Error Prone' (#107) from f/secure-server into develop

Reviewed-on: https://git.kske.dev/zdm/envoy/pulls/107
Reviewed-by: kske <kai@kske.dev>
This commit is contained in:
Kai S. K. Engelbart 2020-10-31 16:30:44 +01:00
commit f6c772a655
Signed by: Käfer & Engelbart Git
GPG Key ID: 70F2F9206EDC1FCE
28 changed files with 202 additions and 131 deletions

View File

@ -21,7 +21,6 @@
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-11">
<attributes>
<attribute name="maven.pomderived" value="true"/>
<attribute name="module" value="true"/>
</attributes>
</classpathentry>
<classpathentry kind="con" path="org.eclipse.m2e.MAVEN2_CLASSPATH_CONTAINER">

View File

@ -586,7 +586,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();
}
@ -600,19 +600,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

View File

@ -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);

View File

@ -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);
}
/**

View File

@ -29,8 +29,8 @@ public abstract class Contact implements Serializable {
*/
public Contact(long id, String name, Set<? extends Contact> contacts) {
this.id = id;
this.name = name;
this.contacts = contacts;
this.name = Objects.requireNonNull(name);
this.contacts = contacts == null ? new HashSet<>() : contacts;
}
/**

View File

@ -38,7 +38,8 @@ public final class GroupMessage extends Message {
Map<Long, MessageStatus> memberStatuses) {
super(id, senderID, groupID, creationDate, receivedDate, readDate, text, attachment, status,
forwarded);
this.memberStatuses = memberStatuses;
this.memberStatuses =
memberStatuses == null ? new HashMap<>() : memberStatuses;
}
/**

View File

@ -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.
@ -20,15 +21,14 @@ public final class LoginCredentials implements Serializable {
private static final long serialVersionUID = 4;
private LoginCredentials(String identifier, String password, boolean registration,
boolean token, boolean requestToken, String clientVersion,
Instant lastSync) {
this.identifier = identifier;
this.password = password;
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 = clientVersion;
this.lastSync = lastSync;
this.clientVersion = Objects.requireNonNull(clientVersion);
this.lastSync = lastSync == null ? Instant.EPOCH : lastSync;
}
/**
@ -75,7 +75,8 @@ public final class LoginCredentials implements Serializable {
* @since Envoy Common v0.2-beta
*/
public static LoginCredentials registration(String identifier, String password,
boolean requestToken, String clientVersion, Instant lastSync) {
boolean requestToken,
String clientVersion, Instant lastSync) {
return new LoginCredentials(identifier, password, true, false, requestToken, clientVersion,
lastSync);
}

View File

@ -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;
}

View File

@ -86,7 +86,7 @@ public final class User extends Contact {
*/
public User(long id, String name, UserStatus status, Set<Contact> contacts) {
super(id, name, contacts);
this.status = status;
this.status = Objects.requireNonNull(status);
}
@Override

View File

@ -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<T> 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<T> implements IEvent, Serializable {
private static final long serialVersionUID = 0L;
protected Valueless() {
super(null);
super(null, true);
}
@Override

View File

@ -20,7 +20,7 @@ public class GroupCreationResult extends Event<Group> {
* @since Envoy Common v0.2-beta
*/
public GroupCreationResult() {
super(null);
super(null, true);
}
/**

View File

@ -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; }

View File

@ -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<User> {
*/
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));

View File

@ -8,8 +8,6 @@ package envoy.event;
*/
public final class IsTyping extends Event<Long> {
private final long destinationID;
private static final long serialVersionUID = 1L;
/**
@ -22,20 +20,13 @@ public final class IsTyping extends Event<Long> {
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; }
}

View File

@ -23,7 +23,7 @@ public final class IssueProposal extends Event<String> {
*/
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<String> {
*/
public IssueProposal(String title, String description, String user, boolean isBug) {
super(escape(title));
this.description =
sanitizeDescription(description) + String.format("<br>Submitted by user %s.", user);
this.description = description == null ? ""
: sanitizeDescription(description) + String.format("<br>Submitted by user %s.", user);
bug = isBug;
}

View File

@ -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<Message.MessageStatus> {
public MessageStatusChange(long id, Message.MessageStatus status, Instant date) {
super(status);
this.id = id;
this.date = date;
this.date = Objects.requireNonNull(date);
}
/**

View File

@ -1,6 +1,6 @@
package envoy.event;
import envoy.data.Contact;
import java.util.Objects;
/**
* @author Leon Hofmeister
@ -8,7 +8,6 @@ import envoy.data.Contact;
*/
public final class PasswordChangeRequest extends Event<String> {
private final long id;
private final String oldPassword;
private static final long serialVersionUID = 0L;
@ -16,21 +15,13 @@ public final class PasswordChangeRequest extends Event<String> {
/**
* @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 = 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
@ -39,6 +30,6 @@ public final class PasswordChangeRequest extends Event<String> {
@Override
public String toString() {
return "PasswordChangeRequest[id=" + id + "]";
return "PasswordChangeRequest[]";
}
}

View File

@ -6,23 +6,13 @@ package envoy.event;
*/
public final class ProfilePicChange extends Event<byte[]> {
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
* @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; }
}

View File

@ -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<User> {
*/
public UserOperation(User contact, ElementOperation operationType) {
super(contact);
this.operationType = operationType;
this.operationType = Objects.requireNonNull(operationType);
}
/**

View File

@ -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<Message> 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<GroupMessage> 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,6 +283,7 @@ 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);
@ -288,6 +295,7 @@ public final class PersistenceManager {
entityManager.merge(contact2);
});
}
}
/**
* Removes a contact from the contact list of another contact and vice versa.
@ -308,6 +316,7 @@ 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);
@ -319,6 +328,7 @@ public final class PersistenceManager {
entityManager.merge(contact2);
});
}
}
/**
* @param user the User whose contacts should be retrieved
@ -331,15 +341,36 @@ public final class PersistenceManager {
}
private void persist(Object 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) {
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) {
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));
}
}
/**

View File

@ -33,7 +33,6 @@ public final class ObjectMessageProcessor implements IMessageProcessor {
this.processors = processors;
}
@SuppressWarnings("unchecked")
@Override
public void process(Message message, WriteProxy writeProxy) {
try (ObjectInputStream in =
@ -45,7 +44,21 @@ public final class ObjectMessageProcessor implements IMessageProcessor {
return;
}
logger.fine("Received " + obj);
logger.log(Level.INFO, "Received " + obj);
refer(message.socketId, writeProxy, obj);
} catch (IOException | ClassNotFoundException e) {
logger.log(Level.WARNING,
"An exception occurred when reading in an object: " + e);
}
}
/**
* 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) {
// Get processor and input class and process object
for (@SuppressWarnings("rawtypes")
@ -54,14 +67,11 @@ public final class ObjectMessageProcessor implements IMessageProcessor {
.getActualTypeArguments()[0];
if (c.equals(obj.getClass()))
try {
p.process(c.cast(obj), message.socketId, new ObjectWriteProxy(writeProxy));
p.process(c.cast(obj), socketID, new ObjectWriteProxy(writeProxy));
break;
} catch (IOException e) {
logger.log(Level.SEVERE, "Exception during processor execution: ", e);
}
}
} catch (IOException | ClassNotFoundException e) {
e.printStackTrace();
}
}
}

View File

@ -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&auml;fer
@ -29,6 +30,15 @@ public final class GroupMessageProcessor implements ObjectProcessor<GroupMessage
@Override
public void process(GroupMessage groupMessage, long socketID, ObjectWriteProxy writeProxy) {
// Check whether the message has the expected parameters
if (!UserAuthenticationUtil.isExpectedUser(groupMessage.getSenderID(), socketID)
|| persistenceManager.getContactByID(groupMessage.getRecipientID()) == null) {
logger.log(Level.INFO,
"Received a group message with invalid parameters");
return;
}
groupMessage.nextStatus();
// Update statuses to SENT / RECEIVED depending on online status

View File

@ -12,6 +12,7 @@ import envoy.util.EnvoyLog;
import envoy.server.data.*;
import envoy.server.net.*;
import envoy.server.util.UserAuthenticationUtil;
/**
* @author Maximilian K&auml;fer
@ -28,6 +29,14 @@ public final class GroupMessageStatusChangeProcessor
@Override
public void process(GroupMessageStatusChange statusChange, long socketID,
ObjectWriteProxy writeProxy) {
// Check whether the message has the expected parameters
if (!UserAuthenticationUtil.isExpectedUser(statusChange.getMemberID(), socketID)) {
logger.log(Level.INFO,
"Received a group message with invalid parameters");
return;
}
GroupMessage gmsg = (GroupMessage) persistenceManager.getMessageByID(statusChange.getID());
// Any other status than READ is not supposed to be sent to the server

View File

@ -23,10 +23,11 @@ public final class IsTypingProcessor implements ObjectProcessor<IsTyping> {
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)));
}
}

View File

@ -123,7 +123,6 @@ public final class LoginCredentialProcessor implements ObjectProcessor<LoginCred
// Process token request
if (credentials.requestToken()) {
String token;
if (user.getAuthToken() != null && user.getAuthTokenExpiration().isAfter(Instant.now()))
// Reuse existing token and delay expiration date
@ -139,7 +138,6 @@ public final class LoginCredentialProcessor implements ObjectProcessor<LoginCred
persistenceManager.updateContact(user);
writeProxy.write(socketID, new NewAuthToken(token));
}
final var pendingMessages =
PersistenceManager.getInstance().getPendingMessages(user, credentials.getLastSync());
pendingMessages.removeIf(GroupMessage.class::isInstance);

View File

@ -12,6 +12,7 @@ import envoy.util.EnvoyLog;
import envoy.server.data.PersistenceManager;
import envoy.server.net.*;
import envoy.server.util.UserAuthenticationUtil;
/**
* This {@link ObjectProcessor} handles incoming {@link Message}s.
@ -29,6 +30,15 @@ public final class MessageProcessor implements ObjectProcessor<Message> {
@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

View File

@ -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 =

View File

@ -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;
}
}