Apply suggestions by @kske

This commit is contained in:
Leon Hofmeister 2020-10-27 23:01:44 +01:00
parent d4c7813c97
commit 7a883861be
Signed by: delvh
GPG Key ID: 3DECE05F6D9A647C
18 changed files with 85 additions and 173 deletions

View File

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

View File

@ -593,7 +593,7 @@ public final class ChatScene implements EventListener, Restorable {
// IsTyping#millisecondsActive // IsTyping#millisecondsActive
if (client.isOnline() && currentChat.getLastWritingEvent() if (client.isOnline() && currentChat.getLastWritingEvent()
+ IsTyping.millisecondsActive <= System.currentTimeMillis()) { + IsTyping.millisecondsActive <= System.currentTimeMillis()) {
client.send(new IsTyping(getChatID(), currentChat.getRecipient().getID())); client.send(new IsTyping(currentChat.getRecipient().getID()));
currentChat.lastWritingEventWasNow(); currentChat.lastWritingEventWasNow();
} }
@ -607,19 +607,6 @@ public final class ChatScene implements EventListener, Restorable {
checkPostConditions(false); 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 * @param e the keys that have been pressed
* @since Envoy Client v0.1-beta * @since Envoy Client v0.1-beta

View File

@ -137,7 +137,7 @@ public final class UserSettingsPane extends OnlineOnlySettingsPane {
// Displaying the save button // Displaying the save button
saveButton saveButton
.setOnAction(e -> save(client.getSender().getID(), currentPasswordField.getText())); .setOnAction(e -> save(currentPasswordField.getText()));
saveButton.setAlignment(Pos.BOTTOM_RIGHT); saveButton.setAlignment(Pos.BOTTOM_RIGHT);
getChildren().add(saveButton); getChildren().add(saveButton);
} }
@ -148,11 +148,11 @@ public final class UserSettingsPane extends OnlineOnlySettingsPane {
* @param username the new username * @param username the new username
* @since Envoy Client v0.2-beta * @since Envoy Client v0.2-beta
*/ */
private void save(long userID, String oldPassword) { private void save(String oldPassword) {
// The profile pic was changed // The profile pic was changed
if (profilePicChanged) { if (profilePicChanged) {
final var profilePicChangeEvent = new ProfilePicChange(currentImageBytes, userID); final var profilePicChangeEvent = new ProfilePicChange(currentImageBytes);
eventBus.dispatch(profilePicChangeEvent); eventBus.dispatch(profilePicChangeEvent);
client.send(profilePicChangeEvent); client.send(profilePicChangeEvent);
logger.log(Level.INFO, "The user just changed his profile pic."); 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 // The username was changed
final var validContactName = Bounds.isValidContactName(newUsername); final var validContactName = Bounds.isValidContactName(newUsername);
if (usernameChanged && validContactName) { if (usernameChanged && validContactName) {
final var nameChangeEvent = new NameChange(userID, newUsername); final var nameChangeEvent = new NameChange(client.getSender().getID(), newUsername);
eventBus.dispatch(nameChangeEvent); eventBus.dispatch(nameChangeEvent);
client.send(nameChangeEvent); client.send(nameChangeEvent);
logger.log(Level.INFO, "The user just changed his name to " + newUsername + "."); 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 // The password was changed
if (validPassword) { 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!"); logger.log(Level.INFO, "The user just tried to change his password!");
} else if (!(validPassword || newPassword.isBlank())) { } else if (!(validPassword || newPassword.isBlank())) {
final var alert = new Alert(AlertType.ERROR); final var alert = new Alert(AlertType.ERROR);

View File

@ -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 <T> the type of object to be sent
* @since Envoy Common v0.3-beta
*/
public final class AuthenticatedRequest<T extends Serializable> implements Serializable {
private final T request;
private final 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 + "]";
}
}

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 * @since Envoy Common v0.1-beta
*/ */
public long getMemberID() { return memberID; } public long getMemberID() { return memberID; }

View File

@ -8,8 +8,6 @@ package envoy.event;
*/ */
public final class IsTyping extends Event<Long> { public final class IsTyping extends Event<Long> {
private final long destinationID;
private static final long serialVersionUID = 1L; private static final long serialVersionUID = 1L;
/** /**
@ -22,20 +20,13 @@ public final class IsTyping extends Event<Long> {
public static final int millisecondsActive = 3500; 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 id the ID of the recipient (client)/ originator(server)
* @param destinationID the ID of the contact the user wrote to
* @since Envoy Common v0.2-beta * @since Envoy Common v0.2-beta
*/ */
public IsTyping(long sourceID, long destinationID) { public IsTyping(long id) {
super(sourceID); super(id);
this.destinationID = destinationID;
} }
/**
* @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

@ -2,37 +2,26 @@ package envoy.event;
import java.util.Objects; import java.util.Objects;
import envoy.data.Contact;
/** /**
* @author Leon Hofmeister * @author Leon Hofmeister
* @since Envoy Common v0.2-beta * @since Envoy Common v0.2-beta
*/ */
public final class PasswordChangeRequest extends Event<String> { public final class PasswordChangeRequest extends Event<String> {
private final long id; private final String oldPassword;
private final String oldPassword;
private static final long serialVersionUID = 0L; private static final long serialVersionUID = 0L;
/** /**
* @param newPassword the new password of that user * @param newPassword the new password of that user
* @param oldPassword the old 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 * @since Envoy Common v0.2-beta
*/ */
public PasswordChangeRequest(String newPassword, String oldPassword, long userID) { public PasswordChangeRequest(String newPassword, String oldPassword) {
super(newPassword); super(newPassword);
this.oldPassword = Objects.requireNonNull(oldPassword); this.oldPassword = Objects.requireNonNull(oldPassword);
id = userID;
} }
/**
* @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 * @return the old password of the underlying user
* @since Envoy Common v0.2-beta * @since Envoy Common v0.2-beta
@ -41,6 +30,6 @@ public final class PasswordChangeRequest extends Event<String> {
@Override @Override
public String toString() { 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[]> { public final class ProfilePicChange extends Event<byte[]> {
private final long id;
private static final long serialVersionUID = 0L; private static final long serialVersionUID = 0L;
/** /**
* @param value the byte[] of the new image * @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 * @since Envoy Common v0.2-beta
*/ */
public ProfilePicChange(byte[] value, long userID) { public ProfilePicChange(byte[] value) {
super(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

@ -7,10 +7,8 @@ import java.util.logging.*;
import com.jenkov.nioserver.*; import com.jenkov.nioserver.*;
import envoy.data.AuthenticatedRequest;
import envoy.util.EnvoyLog; import envoy.util.EnvoyLog;
import envoy.server.data.PersistenceManager;
import envoy.server.processors.ObjectProcessor; import envoy.server.processors.ObjectProcessor;
/** /**
@ -46,24 +44,9 @@ public final class ObjectMessageProcessor implements IMessageProcessor {
return; return;
} }
// authenticate requests if necessary logger.log(Level.INFO, "Received " + obj);
boolean authenticated = false;
if (obj instanceof AuthenticatedRequest)
try {
authenticated = PersistenceManager
.getInstance().getUserByID(((AuthenticatedRequest<?>) obj).getUserID())
.getID() == ConnectionManager.getInstance()
.getUserIDBySocketID(message.socketId);
// Class cast exception and NullPointerException are valid here and signify a refer(message.socketId, writeProxy, obj);
// 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);
} catch (IOException | ClassNotFoundException e) { } catch (IOException | ClassNotFoundException e) {
logger.log(Level.WARNING, logger.log(Level.WARNING,
"An exception occurred when reading in an object: " + e); "An exception occurred when reading in an object: " + e);
@ -75,27 +58,20 @@ public final class ObjectMessageProcessor implements IMessageProcessor {
* present. * present.
*/ */
@SuppressWarnings("unchecked") @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 // Get processor and input class and process object
for (@SuppressWarnings("rawtypes") for (@SuppressWarnings("rawtypes")
ObjectProcessor p : processors) { ObjectProcessor p : processors) {
Class<?> c = (Class<?>) ((ParameterizedType) p.getClass().getGenericInterfaces()[0]) Class<?> c = (Class<?>) ((ParameterizedType) p.getClass().getGenericInterfaces()[0])
.getActualTypeArguments()[0]; .getActualTypeArguments()[0];
if (c.equals(obj.getClass())) { if (c.equals(obj.getClass()))
if (!authenticated && p.isAuthenticationRequired()) {
logger.log(Level.INFO,
"Discarding request as no authentication has been provided");
return;
}
try { try {
p.process(c.cast(obj), socketID, new ObjectWriteProxy(writeProxy)); p.process(c.cast(obj), socketID, new ObjectWriteProxy(writeProxy));
break; break;
} catch (IOException e) { } catch (IOException e) {
logger.log(Level.SEVERE, "Exception during processor execution: ", e); logger.log(Level.SEVERE, "Exception during processor execution: ", e);
} }
}
} }
} }
} }

View File

@ -5,7 +5,7 @@ import static envoy.server.Startup.config;
import java.time.Instant; import java.time.Instant;
import java.util.Collections; import java.util.Collections;
import java.util.logging.Logger; import java.util.logging.*;
import javax.persistence.EntityExistsException; import javax.persistence.EntityExistsException;
@ -15,6 +15,7 @@ import envoy.util.EnvoyLog;
import envoy.server.data.PersistenceManager; import envoy.server.data.PersistenceManager;
import envoy.server.net.*; import envoy.server.net.*;
import envoy.server.util.UserAuthenticationUtil;
/** /**
* @author Maximilian K&auml;fer * @author Maximilian K&auml;fer
@ -29,6 +30,15 @@ public final class GroupMessageProcessor implements ObjectProcessor<GroupMessage
@Override @Override
public void process(GroupMessage groupMessage, long socketID, ObjectWriteProxy writeProxy) { 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(); groupMessage.nextStatus();
// Update statuses to SENT / RECEIVED depending on online status // 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.data.*;
import envoy.server.net.*; import envoy.server.net.*;
import envoy.server.util.UserAuthenticationUtil;
/** /**
* @author Maximilian K&auml;fer * @author Maximilian K&auml;fer
@ -28,6 +29,14 @@ public final class GroupMessageStatusChangeProcessor
@Override @Override
public void process(GroupMessageStatusChange statusChange, long socketID, public void process(GroupMessageStatusChange statusChange, long socketID,
ObjectWriteProxy writeProxy) { 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()); GroupMessage gmsg = (GroupMessage) persistenceManager.getMessageByID(statusChange.getID());
// Any other status than READ is not supposed to be sent to the server // 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 { throws IOException {
final var contact = persistenceManager.getContactByID(event.get()); final var contact = persistenceManager.getContactByID(event.get());
if (contact instanceof User) { if (contact instanceof User) {
final var destinationID = event.getDestinationID(); if (connectionManager.isOnline(event.get()))
if (connectionManager.isOnline(destinationID)) writeProxy.write(connectionManager.getSocketID(event.get()),
writeProxy.write(connectionManager.getSocketID(destinationID), event); new IsTyping(connectionManager.getUserIDBySocketID(socketID)));
} else } else
writeProxy.writeToOnlineContacts(contact.getContacts(), event); writeProxy.writeToOnlineContacts(contact.getContacts(),
new IsTyping(connectionManager.getUserIDBySocketID(socketID)));
} }
} }

View File

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

View File

@ -34,9 +34,6 @@ public final class LoginCredentialProcessor implements ObjectProcessor<LoginCred
private static final Logger logger = EnvoyLog.getLogger(LoginCredentialProcessor.class); private static final Logger logger = EnvoyLog.getLogger(LoginCredentialProcessor.class);
@Override
public boolean isAuthenticationRequired() { return false; }
@Override @Override
public void process(LoginCredentials credentials, long socketID, ObjectWriteProxy writeProxy) { public void process(LoginCredentials credentials, long socketID, ObjectWriteProxy writeProxy) {

View File

@ -12,6 +12,7 @@ import envoy.util.EnvoyLog;
import envoy.server.data.PersistenceManager; import envoy.server.data.PersistenceManager;
import envoy.server.net.*; import envoy.server.net.*;
import envoy.server.util.UserAuthenticationUtil;
/** /**
* This {@link ObjectProcessor} handles incoming {@link Message}s. * This {@link ObjectProcessor} handles incoming {@link Message}s.
@ -29,6 +30,15 @@ public final class MessageProcessor implements ObjectProcessor<Message> {
@Override @Override
public void process(Message message, long socketID, ObjectWriteProxy writeProxy) { 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(); message.nextStatus();
// Convert to server message // Convert to server message

View File

@ -21,10 +21,4 @@ public interface ObjectProcessor<T> {
* @since Envoy Server Standalone v0.1-alpha * @since Envoy Server Standalone v0.1-alpha
*/ */
void process(T input, long socketID, ObjectWriteProxy writeProxy) throws IOException; 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; }
} }

View File

@ -7,7 +7,7 @@ import envoy.event.*;
import envoy.util.EnvoyLog; import envoy.util.EnvoyLog;
import envoy.server.data.PersistenceManager; import envoy.server.data.PersistenceManager;
import envoy.server.net.ObjectWriteProxy; import envoy.server.net.*;
import envoy.server.util.PasswordUtil; import envoy.server.util.PasswordUtil;
/** /**
@ -21,7 +21,8 @@ public final class PasswordChangeRequestProcessor
public void process(PasswordChangeRequest event, long socketID, ObjectWriteProxy writeProxy) public void process(PasswordChangeRequest event, long socketID, ObjectWriteProxy writeProxy)
throws IOException { throws IOException {
final var persistenceManager = PersistenceManager.getInstance(); final var persistenceManager = PersistenceManager.getInstance();
final var user = persistenceManager.getUserByID(event.getID()); final var user = persistenceManager
.getUserByID(ConnectionManager.getInstance().getUserIDBySocketID(socketID));
final var logger = final var logger =
EnvoyLog.getLogger(PasswordChangeRequestProcessor.class); EnvoyLog.getLogger(PasswordChangeRequestProcessor.class);
final var correctAuthentication = 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;
}
}