API breakage: Revamped some login stuff.

I have cleaned up some logic in the client login session handler and
revamped the GameProfile class somewhat. The most notable breaking
change is that Velocity now returns an UUID for getId() instead of an
undashed UUID, which was moved to a getUndashedId() method.

api/src/main/java/com/velocitypowered/api/util/GameProfile.java

@@ -15,11 +15,23 @@ public final class GameProfile {
   private final String name;
   private final List<Property> properties;
 
+  /**
+   * Creates a new Mojang game profile.
+   * @param id the UUID for the profile
+   * @param name the profile's username
+   * @param properties properties for the profile
+   */
   public GameProfile(UUID id, String name, List<Property> properties) {
     this(Preconditions.checkNotNull(id, "id"), UuidUtils.toUndashed(id),
         Preconditions.checkNotNull(name, "name"), ImmutableList.copyOf(properties));
   }
 
+  /**
+   * Creates a new Mojang game profile.
+   * @param undashedId the undashed, Mojang-style UUID for the profile
+   * @param name the profile's username
+   * @param properties properties for the profile
+   */
   public GameProfile(String undashedId, String name, List<Property> properties) {
     this(UuidUtils.fromUndashed(Preconditions.checkNotNull(undashedId, "undashedId")), undashedId,
         Preconditions.checkNotNull(name, "name"), ImmutableList.copyOf(properties));
@@ -32,18 +44,34 @@ public final class GameProfile {
     this.properties = properties;
   }
 
-  public String getId() {
+  /**
+   * Returns the undashed, Mojang-style UUID.
+   * @return the undashed UUID
+   */
+  public String getUndashedId() {
     return undashedId;
   }
 
-  public UUID idAsUuid() {
+  /**
+   * Returns the UUID associated with this game profile.
+   * @return the UUID
+   */
+  public UUID getId() {
     return id;
   }
 
+  /**
+   * Returns the username associated with this profile.
+   * @return the username
+   */
   public String getName() {
     return name;
   }
 
+  /**
+   * Returns an immutable list of profile properties associated with this profile.
+   * @return the properties associated with this profile
+   */
   public List<Property> getProperties() {
     return properties;
   }

proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java

@@ -157,7 +157,7 @@ public class LoginSessionHandler implements MinecraftSessionHandler {
     try {
       ProtocolUtils.writeVarInt(dataToForward, VelocityConstants.FORWARDING_VERSION);
       ProtocolUtils.writeString(dataToForward, address);
-      ProtocolUtils.writeUuid(dataToForward, profile.idAsUuid());
+      ProtocolUtils.writeUuid(dataToForward, profile.getId());
       ProtocolUtils.writeString(dataToForward, profile.getName());
       ProtocolUtils.writeProperties(dataToForward, profile.getProperties());
 

proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java

@@ -109,7 +109,7 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation,
         .append('\0')
         .append(proxyPlayer.getRemoteAddress().getHostString())
         .append('\0')
-        .append(proxyPlayer.getProfile().getId())
+        .append(proxyPlayer.getProfile().getUndashedId())
         .append('\0');
     GSON.toJson(proxyPlayer.getProfile().getProperties(), data);
     return data.toString();

proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java

@@ -100,7 +100,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player {
 
   @Override
   public UUID getUniqueId() {
-    return profile.idAsUuid();
+    return profile.getId();
   }
 
   @Override

proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java

@@ -1,6 +1,9 @@
 package com.velocitypowered.proxy.connection.client;
 
+import static com.velocitypowered.proxy.VelocityServer.GSON;
 import static com.velocitypowered.proxy.connection.VelocityConstants.EMPTY_BYTE_ARRAY;
+import static com.velocitypowered.proxy.connection.VelocityConstants.VELOCITY_IP_FORWARDING_CHANNEL;
+import static com.velocitypowered.proxy.protocol.ProtocolConstants.*;
 
 import com.google.common.base.Preconditions;
 import com.velocitypowered.api.event.connection.LoginEvent;
@@ -28,6 +31,7 @@ import com.velocitypowered.proxy.protocol.packet.ServerLogin;
 import com.velocitypowered.proxy.protocol.packet.ServerLoginSuccess;
 import com.velocitypowered.proxy.protocol.packet.SetCompression;
 import com.velocitypowered.proxy.util.EncryptionUtils;
+import com.velocitypowered.proxy.util.VelocityMessages;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBufUtil;
 import io.netty.buffer.Unpooled;
@@ -36,9 +40,7 @@ import java.net.MalformedURLException;
 import java.net.URL;
 import java.security.GeneralSecurityException;
 import java.security.KeyPair;
-import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.ThreadLocalRandom;
 import net.kyori.text.Component;
@@ -51,8 +53,10 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
 public class LoginSessionHandler implements MinecraftSessionHandler {
 
   private static final Logger logger = LogManager.getLogger(LoginSessionHandler.class);
-  private static final String MOJANG_SERVER_AUTH_URL =
+  private static final String MOJANG_HASJOINED_URL =
       "https://sessionserver.mojang.com/session/minecraft/hasJoined?username=%s&serverId=%s&ip=%s";
+  private static final GameProfile.Property IS_FORGE_CLIENT_PROPERTY =
+      new GameProfile.Property("forgeClient", "true", "");
 
   private final VelocityServer server;
   private final MinecraftConnection inbound;
@@ -62,7 +66,7 @@ public class LoginSessionHandler implements MinecraftSessionHandler {
   private int playerInfoId;
   private @MonotonicNonNull ConnectedPlayer connectedPlayer;
 
-  public LoginSessionHandler(VelocityServer server, MinecraftConnection inbound,
+  LoginSessionHandler(VelocityServer server, MinecraftConnection inbound,
       InboundConnection apiInbound) {
     this.server = Preconditions.checkNotNull(server, "server");
     this.inbound = Preconditions.checkNotNull(inbound, "inbound");
@@ -72,11 +76,10 @@ public class LoginSessionHandler implements MinecraftSessionHandler {
   @Override
   public boolean handle(ServerLogin packet) {
     this.login = packet;
-    if (inbound.getProtocolVersion() >= ProtocolConstants.MINECRAFT_1_13) {
+    if (inbound.getProtocolVersion() >= MINECRAFT_1_13) {
       playerInfoId = ThreadLocalRandom.current().nextInt();
-      inbound.write(
+      inbound.write(new LoginPluginMessage(playerInfoId, VELOCITY_IP_FORWARDING_CHANNEL,
-          new LoginPluginMessage(playerInfoId, VelocityConstants.VELOCITY_IP_FORWARDING_CHANNEL,
+          Unpooled.EMPTY_BUFFER));
-              Unpooled.EMPTY_BUFFER));
     } else {
       beginPreLogin();
     }
@@ -88,9 +91,7 @@ public class LoginSessionHandler implements MinecraftSessionHandler {
     if (packet.getId() == playerInfoId) {
       if (packet.isSuccess()) {
         // Uh oh, someone's trying to run Velocity behind Velocity. We don't want that happening.
-        inbound.closeWith(Disconnect.create(
+        inbound.closeWith(Disconnect.create(VelocityMessages.NO_PROXY_BEHIND_PROXY));
-            TextComponent.of("Running Velocity behind Velocity isn't supported.", TextColor.RED)
-        ));
       } else {
         // Proceed with the regular login process.
         beginPreLogin();
@@ -124,9 +125,9 @@ public class LoginSessionHandler implements MinecraftSessionHandler {
           .generateServerId(decryptedSharedSecret, serverKeyPair.getPublic());
 
       String playerIp = ((InetSocketAddress) inbound.getRemoteAddress()).getHostString();
+      String url = String.format(MOJANG_HASJOINED_URL, login.getUsername(), serverId, playerIp);
       server.getHttpClient()
-          .get(new URL(
+          .get(new URL(url))
-              String.format(MOJANG_SERVER_AUTH_URL, login.getUsername(), serverId, playerIp)))
           .thenAcceptAsync(profileResponse -> {
             if (inbound.isClosed()) {
               // The player disconnected after we authenticated them.
@@ -143,14 +144,12 @@ public class LoginSessionHandler implements MinecraftSessionHandler {
 
             if (profileResponse.getCode() == 200) {
               // All went well, initialize the session.
-              initializePlayer(
+              initializePlayer(GSON.fromJson(profileResponse.getBody(), GameProfile.class), true);
-                  VelocityServer.GSON.fromJson(profileResponse.getBody(), GameProfile.class), true);
             } else if (profileResponse.getCode() == 204) {
               // Apparently an offline-mode user logged onto this online-mode proxy.
               logger.warn("An offline-mode client ({} from {}) tried to connect!",
                   login.getUsername(), playerIp);
-              inbound.closeWith(Disconnect.create(TextComponent
+              inbound.closeWith(Disconnect.create(VelocityMessages.ONLINE_MODE_ONLY));
-                  .of("This server only accepts connections from online-mode clients.")));
             } else {
               // Something else went wrong
               logger.error(
@@ -216,10 +215,13 @@ public class LoginSessionHandler implements MinecraftSessionHandler {
   }
 
   private void initializePlayer(GameProfile profile, boolean onlineMode) {
-    if (inbound.isLegacyForge()
+    if (inbound.isLegacyForge() && server.getConfiguration().getPlayerInfoForwardingMode()
-        && server.getConfiguration().getPlayerInfoForwardingMode() == PlayerInfoForwarding.LEGACY) {
+        == PlayerInfoForwarding.LEGACY) {
-      // We want to add the FML token to the properties
+      // We can't forward the FML token to the server when we are running in legacy forwarding mode,
-      profile = profile.addProperty(new GameProfile.Property("forgeClient", "true", ""));
+      // since both use the "hostname" field in the handshake. We add a special property to the
+      // profile instead, which will be ignored by non-Forge servers and can be intercepted by a
+      // Forge coremod, such as SpongeForge.
+      profile = profile.addProperty(IS_FORGE_CLIENT_PROPERTY);
     }
     GameProfileRequestEvent profileRequestEvent = new GameProfileRequestEvent(apiInbound, profile,
         onlineMode);

proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/PlayerListItem.java

@@ -142,7 +142,7 @@ public class PlayerListItem implements MinecraftPacket {
     }
 
     public static Item from(TabListEntry entry) {
-      return new Item(entry.getProfile().idAsUuid())
+      return new Item(entry.getProfile().getId())
           .setName(entry.getProfile().getName())
           .setProperties(entry.getProfile().getProperties())
           .setLatency(entry.getLatency())

proxy/src/main/java/com/velocitypowered/proxy/protocol/util/GameProfileSerializer.java

@@ -29,7 +29,7 @@ public class GameProfileSerializer implements JsonSerializer<GameProfile>,
   @Override
   public JsonElement serialize(GameProfile src, Type typeOfSrc, JsonSerializationContext context) {
     JsonObject obj = new JsonObject();
-    obj.add("id", new JsonPrimitive(src.getId()));
+    obj.add("id", new JsonPrimitive(src.getUndashedId()));
     obj.add("name", new JsonPrimitive(src.getName()));
     obj.add("properties", context.serialize(src.getProperties(), propertyList));
     return obj;

proxy/src/main/java/com/velocitypowered/proxy/tablist/VelocityTabList.java

@@ -45,13 +45,13 @@ public class VelocityTabList implements TabList {
     Preconditions.checkNotNull(entry, "entry");
     Preconditions.checkArgument(entry.getTabList().equals(this),
         "The provided entry was not created by this tab list");
-    Preconditions.checkArgument(!entries.containsKey(entry.getProfile().idAsUuid()),
+    Preconditions.checkArgument(!entries.containsKey(entry.getProfile().getId()),
         "this TabList already contains an entry with the same uuid");
 
     PlayerListItem.Item packetItem = PlayerListItem.Item.from(entry);
     connection.write(
         new PlayerListItem(PlayerListItem.ADD_PLAYER, Collections.singletonList(packetItem)));
-    entries.put(entry.getProfile().idAsUuid(), entry);
+    entries.put(entry.getProfile().getId(), entry);
   }
 
   @Override
@@ -141,7 +141,7 @@ public class VelocityTabList implements TabList {
   }
 
   void updateEntry(int action, TabListEntry entry) {
-    if (entries.containsKey(entry.getProfile().idAsUuid())) {
+    if (entries.containsKey(entry.getProfile().getId())) {
       PlayerListItem.Item packetItem = PlayerListItem.Item.from(entry);
       connection.write(new PlayerListItem(action, Collections.singletonList(packetItem)));
     }

proxy/src/main/java/com/velocitypowered/proxy/util/VelocityMessages.java

@@ -0,0 +1,17 @@
+package com.velocitypowered.proxy.util;
+
+import net.kyori.text.Component;
+import net.kyori.text.TextComponent;
+import net.kyori.text.format.TextColor;
+
+public class VelocityMessages {
+
+  public static final Component ONLINE_MODE_ONLY = TextComponent
+      .of("This server only accepts connections from online-mode clients.", TextColor.RED);
+  public static final Component NO_PROXY_BEHIND_PROXY = TextComponent
+      .of("Running Velocity behind Velocity isn't supported.", TextColor.RED);
+
+  private VelocityMessages() {
+    throw new AssertionError();
+  }
+}