Merge pull request #348 from jacwah/develop-names

Refuse login if user name is already taken

cardshifter-api/src/main/java/com/cardshifter/api/ClientServerInterface.java

@@ -2,6 +2,8 @@
 
 import com.cardshifter.api.messages.Message;
 
+import java.util.Collection;
+
 public interface ClientServerInterface {
 
 	void handleMessage(ClientIO clientIO, String message);

cardshifter-api/src/main/java/com/cardshifter/api/outgoing/WelcomeMessage.java

@@ -11,12 +11,12 @@ public class WelcomeMessage extends Message {
 	private String message;
 
 	public WelcomeMessage() {
-		this(-42, true);
+		this(-42, true, "");
 	}
-	public WelcomeMessage(int id, boolean success) {
+	public WelcomeMessage(int id, boolean success, String message) {
 		super("loginresponse");
 		this.status = success ? STATUS_OK : 404;
-		this.message = success ? "OK" : "Wrong username or password";
+		this.message = message;
 		this.userId = id;
 	}
 

cardshifter-fx/src/main/java/com/cardshifter/client/GameClientLauncherController.java

@@ -1,11 +1,8 @@
 package com.cardshifter.client;
 
 import java.net.URL;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.*;
 import java.util.Map.Entry;
-import java.util.ResourceBundle;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.prefs.Preferences;
@@ -180,6 +177,7 @@ public void handleMessage(ClientIO clientIO, String message) {
             public LogInterface getLogger() {
                 return new Log4jAdapter();
             }
+
         };
 
 		try {

cardshifter-server/src/main/java/com/cardshifter/server/model/Handlers.java

@@ -66,13 +66,17 @@ public void query(ServerQueryMessage message, ClientIO client) {
 
 	public void loginMessage(LoginMessage message, ClientIO client) {
 		logger.info("Login request: " + message.getUsername() + " for client " + client);
-		if (message.getUsername().startsWith("x")) {
-			client.sendToClient(new WelcomeMessage(0, false));
+
+		try {
+			server.trySetClientName(client, message.getUsername());
+		}
+		catch (UserNameAlreadyInUseException | InvalidUserNameException e) {
+			client.sendToClient(new WelcomeMessage(0, false, e.getMessage()));
 			return;
 		}
+
 		logger.info("Client is welcome!");
-		client.setName(message.getUsername());
-		client.sendToClient(new WelcomeMessage(client.getId(), true));
+		client.sendToClient(new WelcomeMessage(client.getId(), true, "OK"));
 		UserStatusMessage statusMessage = new UserStatusMessage(client.getId(), client.getName(), Status.ONLINE);
 		server.getClients().values().stream()
 			.filter(cl -> cl != client)

cardshifter-server/src/main/java/com/cardshifter/server/model/InvalidUserNameException.java

@@ -0,0 +1,8 @@
+package com.cardshifter.server.model;
+
+public class InvalidUserNameException extends Exception {
+
+    public InvalidUserNameException() {
+        super("User name is invalid");
+    }
+}

cardshifter-server/src/main/java/com/cardshifter/server/model/Server.java

@@ -1,25 +1,17 @@
 package com.cardshifter.server.model;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Set;
+import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
-import com.cardshifter.api.LogInterface;
+import com.cardshifter.api.*;
 import com.cardshifter.core.Log4jAdapter;
 import org.apache.log4j.LogManager;
 import org.apache.log4j.Logger;
 
-import com.cardshifter.api.ClientIO;
-import com.cardshifter.api.ClientServerInterface;
 import com.cardshifter.api.both.ChatMessage;
 import com.cardshifter.api.both.InviteResponse;
 import com.cardshifter.api.both.PlayerConfigMessage;
@@ -119,6 +111,42 @@ public ChatArea newChatRoom(String name) {
 	public Map<Integer, ClientIO> getClients() {
 		return Collections.unmodifiableMap(clients);
 	}
+
+	/**
+	 * Validate a user name
+	 *
+	 * @param name The user name to be validated
+	 * @return true if user name is valid
+	 */
+	public static boolean isValidUserName(String name) {
+		return   name.length() > 0 &&
+				!name.startsWith("x");
+	}
+
+	/**
+	 * Set the user name of a client or fail
+	 *
+	 * @param client The client
+	 * @param name The user name to set
+	 * @throws UserNameAlreadyInUseException If name is already used by another client
+	 * @throws InvalidUserNameException If name is not a valid user name as determined by isValidUserName
+	 */
+	public void trySetClientName(ClientIO client, String name)
+			throws UserNameAlreadyInUseException, InvalidUserNameException {
+		if (!isValidUserName(name)) {
+			throw new InvalidUserNameException();
+		}
+
+		synchronized (this) {
+			for (ClientIO other : clients.values()) {
+				if (other.getName().equals(name)) {
+					throw new UserNameAlreadyInUseException();
+				}
+			}
+
+			client.setName(name);
+		}
+	}
 
 	/**
 	 * 

cardshifter-server/src/main/java/com/cardshifter/server/model/UserNameAlreadyInUseException.java

@@ -0,0 +1,9 @@
+package com.cardshifter.server.model;
+
+
+public class UserNameAlreadyInUseException extends Exception {
+
+    UserNameAlreadyInUseException() {
+        super("User name already in use by another client");
+    }
+}

cardshifter-server/src/test/java/com/cardshifter/server/main/ServerConnectionTest.java

@@ -50,6 +50,7 @@ private String getTestMod() {
 	private Server server;
 	private TestClient client1;
 	private int userId;
+	private final String client1UserName = "Tester1";
     private AvailableModsMessage mods;
 
     @Before
@@ -61,7 +62,7 @@ public void setup() throws IOException, InterruptedException {
 		assertTrue("Server did not start correctly. Perhaps it is already running?", server.getClients().size() > 0);
 
 		client1 = new TestClient();
-		client1.send(new LoginMessage("Tester"));
+		client1.send(new LoginMessage(client1UserName));
 
 		WelcomeMessage welcome = client1.await(WelcomeMessage.class);
 		assertEquals(200, welcome.getStatus());
@@ -104,7 +105,7 @@ public void testUserOnlineOffline() throws InterruptedException, UnknownHostExce
 		List<UserStatusMessage> users = client2.awaitMany(6, UserStatusMessage.class);
 		System.out.println("Online users: " + users);
 		// There is no determined order in which the UserStatusMessages are received, so it is harder to make any assertions.
-		assertTrue(users.stream().filter(mess -> mess.getName().equals("Tester")).findAny().isPresent());
+		assertTrue(users.stream().filter(mess -> mess.getName().equals(client1UserName)).findAny().isPresent());
 		assertTrue(users.stream().filter(mess -> mess.getName().equals("Test2")).findAny().isPresent());
 		assertTrue(users.stream().filter(mess -> mess.getName().equals("AI Fighter")).findAny().isPresent());
 		assertTrue(users.stream().filter(mess -> mess.getName().equals("AI Loser")).findAny().isPresent());
@@ -119,6 +120,14 @@ public void testUserOnlineOffline() throws InterruptedException, UnknownHostExce
 		assertEquals(client2id, statusMessage.getUserId());
 		assertEquals("Test2", statusMessage.getName());
 	}
+
+	@Test(timeout = 5000)
+	public void testSameUserName() throws IOException, InterruptedException {
+		TestClient client2 = new TestClient();
+		client2.send(new LoginMessage(client1UserName));
+		WelcomeMessage welcomeMessage = client2.await(WelcomeMessage.class);
+		assertEquals(false, welcomeMessage.isOK());
+	}
 
 	@Test(timeout = 10000)
 	public void testStartGame() throws InterruptedException, IOException {