From d4a270781992eb708860fd9912302121fd8db25e Mon Sep 17 00:00:00 2001 From: Lukas Jost Date: Sat, 31 Oct 2026 20:35:17 +0100 Subject: [PATCH 1/2] fix(velocity): consume the verified runtime transfer admission --- .../presence/HttpPlayerPresenceClient.kt | 35 +++++++++- .../presence/HttpPlayerPresenceClientTest.kt | 66 ++++++++++++++++++ .../listener/PlayerConnectionListener.kt | 33 ++++++++- .../grounds/presence/PlayerPresenceService.kt | 45 ++++++++---- .../presence/TransferAdmissionBridge.kt | 40 +++++++++++ .../presence/PlayerPresenceServiceTest.kt | 68 +++++++++++++++++++ .../presence/TransferAdmissionBridgeTest.kt | 48 +++++++++++++ 7 files changed, 320 insertions(+), 15 deletions(-) create mode 100644 velocity/src/main/kotlin/gg/grounds/presence/TransferAdmissionBridge.kt create mode 100644 velocity/src/test/kotlin/gg/grounds/presence/PlayerPresenceServiceTest.kt create mode 100644 velocity/src/test/kotlin/gg/grounds/presence/TransferAdmissionBridgeTest.kt diff --git a/common/src/main/kotlin/gg/grounds/player/presence/HttpPlayerPresenceClient.kt b/common/src/main/kotlin/gg/grounds/player/presence/HttpPlayerPresenceClient.kt index c758ab1..7a6c55d 100644 --- a/common/src/main/kotlin/gg/grounds/player/presence/HttpPlayerPresenceClient.kt +++ b/common/src/main/kotlin/gg/grounds/player/presence/HttpPlayerPresenceClient.kt @@ -211,6 +211,23 @@ class HttpPlayerPresenceClient( return SessionHandoff(token, parseInstantMillis(expiry)) } + private fun abortHandoff(playerId: UUID, token: String, connectionId: UUID, proxyId: String) { + val body = + mapper.writeValueAsString( + mapOf( + "playerId" to playerId.toString(), + "handoffToken" to token, + "connectionId" to connectionId.toString(), + "destinationProxyId" to proxyId, + ) + ) + send( + request("/v2/players/sessions/transfer/admission/abort") + .header("Content-Type", APPLICATION_JSON) + .POST(HttpRequest.BodyPublishers.ofString(body)) + ) + } + fun consumeHandoff( playerId: UUID, sessionId: UUID, @@ -218,6 +235,7 @@ class HttpPlayerPresenceClient( playerName: String, proxyId: String, region: String, + admissionConnectionId: UUID? = null, ): ExactPlayerLoginResult { val body = mapper.writeValueAsString( @@ -228,6 +246,7 @@ class HttpPlayerPresenceClient( "playerName" to playerName, "proxyId" to proxyId, "region" to region, + "connectionId" to admissionConnectionId?.toString(), ) ) val response = @@ -235,7 +254,15 @@ class HttpPlayerPresenceClient( request("/v2/players/sessions/transfer") .header("Content-Type", APPLICATION_JSON) .POST(HttpRequest.BodyPublishers.ofString(body)) - ) ?: return ExactPlayerLoginResult.Unavailable("presence service did not answer") + ) + if (response == null || response.statusCode() in 500..599) { + if (admissionConnectionId != null) + abortHandoff(playerId, token, admissionConnectionId, proxyId) + logoutExactSession(playerId, sessionId) + return ExactPlayerLoginResult.Unavailable( + response?.let { problemDetail(it.body()) } ?: "presence service did not answer" + ) + } return when (response.statusCode()) { 201 -> { val json = parse(response.body()) @@ -246,10 +273,14 @@ class HttpPlayerPresenceClient( ExactPlayerLoginResult.Accepted( AdmittedPlayerSession(sessionId, parseInstantMillis(expiry)) ) - else + else { + if (admissionConnectionId != null) + abortHandoff(playerId, token, admissionConnectionId, proxyId) + logoutExactSession(playerId, sessionId) ExactPlayerLoginResult.Error( "presence service returned an invalid transferred lease" ) + } } 409 -> ExactPlayerLoginResult.AlreadyOnline in 500..599 -> ExactPlayerLoginResult.Unavailable(problemDetail(response.body())) diff --git a/common/src/test/kotlin/gg/grounds/player/presence/HttpPlayerPresenceClientTest.kt b/common/src/test/kotlin/gg/grounds/player/presence/HttpPlayerPresenceClientTest.kt index bfbe334..eb77f36 100644 --- a/common/src/test/kotlin/gg/grounds/player/presence/HttpPlayerPresenceClientTest.kt +++ b/common/src/test/kotlin/gg/grounds/player/presence/HttpPlayerPresenceClientTest.kt @@ -177,6 +177,72 @@ class HttpPlayerPresenceClientTest { ) } + @Test + fun `reserved consume carries the runtime connection binding`() { + routes["/v2/players/sessions/transfer"] = + 201 to + """{"playerId":"$PLAYER_ID","sessionId":"$DESTINATION_SESSION_ID","leaseExpiresAt":"2026-08-04T10:01:00Z"}""" + client.consumeHandoff( + PLAYER_ID, + DESTINATION_SESSION_ID, + "token", + "Notch", + "target", + "eu", + DESTINATION_SESSION_ID, + ) + assertTrue( + seen["/v2/players/sessions/transfer"]!! + .body + .contains("\"connectionId\":\"$DESTINATION_SESSION_ID\"") + ) + } + + @Test + fun `uncertain reserved consume fences its admission before exact cleanup`() { + routes["/v2/players/sessions/transfer"] = 503 to "" + routes["/v2/players/sessions/transfer/admission/abort"] = 204 to "" + routes["/v2/players/$PLAYER_ID/session"] = 204 to "" + client.consumeHandoff( + PLAYER_ID, + DESTINATION_SESSION_ID, + "opaque-token", + "Notch", + "target", + "eu", + DESTINATION_SESSION_ID, + ) + val abort = seen["/v2/players/sessions/transfer/admission/abort"]!! + assertEquals("POST", abort.method) + assertTrue(abort.body.contains("\"connectionId\":\"$DESTINATION_SESSION_ID\"")) + assertTrue(abort.body.contains("\"destinationProxyId\":\"target\"")) + assertEquals( + "sessionId=$DESTINATION_SESSION_ID", + seen["/v2/players/$PLAYER_ID/session"]?.query, + ) + } + + @Test + fun `unknown transfer result cleans only its exact destination lease`() { + routes["/v2/players/sessions/transfer"] = 503 to "" + routes["/v2/players/$PLAYER_ID/session"] = 204 to "" + val result = + client.consumeHandoff( + PLAYER_ID, + DESTINATION_SESSION_ID, + "token", + "Notch", + "target", + "eu", + ) + assertTrue(result is ExactPlayerLoginResult.Unavailable) + assertEquals("DELETE", seen["/v2/players/$PLAYER_ID/session"]?.method) + assertEquals( + "sessionId=$DESTINATION_SESSION_ID", + seen["/v2/players/$PLAYER_ID/session"]?.query, + ) + } + @Test fun `409 is already-online, not an error`() { routes["/v1/players/sessions"] = diff --git a/velocity/src/main/kotlin/gg/grounds/listener/PlayerConnectionListener.kt b/velocity/src/main/kotlin/gg/grounds/listener/PlayerConnectionListener.kt index b823ef9..ca53388 100644 --- a/velocity/src/main/kotlin/gg/grounds/listener/PlayerConnectionListener.kt +++ b/velocity/src/main/kotlin/gg/grounds/listener/PlayerConnectionListener.kt @@ -15,6 +15,7 @@ import gg.grounds.config.MessagesConfig import gg.grounds.player.presence.ExactPlayerLoginResult import gg.grounds.player.presence.PlayerLogoutResult import gg.grounds.presence.PlayerPresenceService +import gg.grounds.presence.TransferAdmissionBridge import java.util.Collections import java.util.IdentityHashMap import java.util.concurrent.CompletableFuture @@ -48,7 +49,18 @@ class PlayerConnectionListener( if ( player.handshakeIntent == com.velocitypowered.api.network.HandshakeIntent.TRANSFER ) { - // A transfer is admitted only after the destination consumes the cookie handoff. + if ( + TransferAdmissionBridge.required && + runCatching { TransferAdmissionBridge.admission(player) }.getOrNull() == + null + ) { + deny( + event, + Component.text("This transfer could not be verified. Please reconnect."), + ) + return@async + } + // Ownership still waits for the final authenticated profile and exact consume. pendingTransfers.add(player) return@async } @@ -124,6 +136,25 @@ class PlayerConnectionListener( handoffCookies[player] = cookie return EventTask.async { try { + val admission = TransferAdmissionBridge.admission(player) + if (TransferAdmissionBridge.required && admission == null) + throw IllegalStateException("transfer admission is missing") + if (admission != null) { + val result = + playerPresenceService.consumeHandoff( + player, + admission.token, + admission.connectionId, + admission.connectionId, + ) + if (result is ExactPlayerLoginResult.Accepted) + TransferAdmissionBridge.complete(player) + if (result !is ExactPlayerLoginResult.Accepted) + player.disconnect( + Component.text("This transfer could not be verified. Please reconnect.") + ) + return@async + } player.requestCookie(HANDOFF_COOKIE) // Null when the sending proxy stored no handoff: a bare transfer packet. val bytes = cookie.get(COOKIE_TIMEOUT_SECONDS, TimeUnit.SECONDS) diff --git a/velocity/src/main/kotlin/gg/grounds/presence/PlayerPresenceService.kt b/velocity/src/main/kotlin/gg/grounds/presence/PlayerPresenceService.kt index 1c9d447..8c1d5db 100644 --- a/velocity/src/main/kotlin/gg/grounds/presence/PlayerPresenceService.kt +++ b/velocity/src/main/kotlin/gg/grounds/presence/PlayerPresenceService.kt @@ -28,6 +28,8 @@ class PlayerPresenceService : AutoCloseable { private var client: HttpPlayerPresenceClient? = null private val connections = Collections.synchronizedMap(IdentityHashMap()) + private val outboundTransfers = + Collections.newSetFromMap(Collections.synchronizedMap(IdentityHashMap())) private val mapper = JsonMapper.builder().build() data class ConnectionLease( @@ -54,7 +56,12 @@ class PlayerPresenceService : AutoCloseable { return withClient(null) { it.issueHandoff(lease.playerId, lease.sessionId) } } - fun consumeHandoff(player: Player, token: String, sessionId: UUID): ExactPlayerLoginResult { + fun consumeHandoff( + player: Player, + token: String, + sessionId: UUID, + admissionConnectionId: UUID? = null, + ): ExactPlayerLoginResult { val result = withClient(ExactPlayerLoginResult.Unavailable("presence service is not configured")) { it.consumeHandoff( @@ -64,18 +71,25 @@ class PlayerPresenceService : AutoCloseable { player.username, proxyId(), region(), + admissionConnectionId, ) } if (result is ExactPlayerLoginResult.Accepted) { - connections[player] = + val lease = ConnectionLease(player.uniqueId, sessionId, result.session.leaseExpiresAtMillis) + synchronized(connections) { + if (player.isActive) connections[player] = lease else logoutExact(lease) + } } return result } fun lease(player: Player): ConnectionLease? = connections[player] - fun remove(player: Player): ConnectionLease? = connections.remove(player) + fun remove(player: Player): ConnectionLease? { + outboundTransfers.remove(player) + return connections.remove(player) + } fun leasesFor(players: Collection): List = players.mapNotNull(::lease) @@ -113,15 +127,22 @@ class PlayerPresenceService : AutoCloseable { fun transferToHost(player: Player, host: String, port: Int): Boolean { val lease = lease(player) ?: return false - val handoff = issueHandoff(player) ?: return false - player.storeCookie( - HANDOFF_COOKIE, - mapper.writeValueAsBytes( - mapOf("playerId" to lease.playerId.toString(), "token" to handoff.token) - ), - ) - player.transferToHost(InetSocketAddress.createUnresolved(host, port)) - return true + if (!outboundTransfers.add(player)) return false + var sent = false + try { + val handoff = issueHandoff(player) ?: return false + player.storeCookie( + HANDOFF_COOKIE, + mapper.writeValueAsBytes( + mapOf("playerId" to lease.playerId.toString(), "token" to handoff.token) + ), + ) + player.transferToHost(InetSocketAddress.createUnresolved(host, port)) + sent = true + return true + } finally { + if (!sent) outboundTransfers.remove(player) + } } fun configure(baseUrl: String) { diff --git a/velocity/src/main/kotlin/gg/grounds/presence/TransferAdmissionBridge.kt b/velocity/src/main/kotlin/gg/grounds/presence/TransferAdmissionBridge.kt new file mode 100644 index 0000000..0fab9ad --- /dev/null +++ b/velocity/src/main/kotlin/gg/grounds/presence/TransferAdmissionBridge.kt @@ -0,0 +1,40 @@ +package gg.grounds.presence + +import com.velocitypowered.api.proxy.Player +import java.util.UUID + +/** Optional runtime bridge; enabling the gate requires the patched runtime, never a fallback. */ +object TransferAdmissionBridge { + val required: Boolean + get() = System.getenv("GROUNDS_TRANSFER_ADMISSION_ENABLED") == "true" + + data class Admission(val token: String, val connectionId: UUID) + + private fun runtime() = Class.forName("gg.grounds.runtime.transfer.TransferAdmission") + + fun admission(player: Player): Admission? { + if (!required) return null + val raw = runtime().getMethod("admission", Player::class.java).invoke(null, player) + return parse(raw as? Map<*, *>, player.uniqueId, System.getenv("PROXY_ID")) + } + + internal fun parse(raw: Map<*, *>?, profile: UUID, proxy: String?): Admission? { + if ( + raw?.get("playerId") != profile.toString() || + raw["destinationProxyId"] != proxy || + proxy.isNullOrBlank() + ) + return null + val token = + (raw["handoffToken"] as? String)?.takeIf { it.matches(Regex("[A-Za-z0-9_-]{43}")) } + ?: return null + val connection = + runCatching { UUID.fromString(raw["connectionId"] as? String) }.getOrNull() + ?: return null + return Admission(token, connection) + } + + fun complete(player: Player) { + if (required) runtime().getMethod("complete", Player::class.java).invoke(null, player) + } +} diff --git a/velocity/src/test/kotlin/gg/grounds/presence/PlayerPresenceServiceTest.kt b/velocity/src/test/kotlin/gg/grounds/presence/PlayerPresenceServiceTest.kt new file mode 100644 index 0000000..e609ce1 --- /dev/null +++ b/velocity/src/test/kotlin/gg/grounds/presence/PlayerPresenceServiceTest.kt @@ -0,0 +1,68 @@ +package gg.grounds.presence + +import com.sun.net.httpserver.HttpServer +import com.velocitypowered.api.proxy.Player +import java.lang.reflect.Proxy +import java.net.InetSocketAddress +import java.time.Instant +import java.util.UUID +import java.util.concurrent.atomic.AtomicInteger +import org.junit.jupiter.api.Assertions.* +import org.junit.jupiter.api.Test +import tools.jackson.databind.json.JsonMapper + +class PlayerPresenceServiceTest { + @Test + fun `a source connection can send only one outstanding transfer`() { + val profile = UUID.randomUUID() + val issued = AtomicInteger() + val transferred = AtomicInteger() + val server = HttpServer.create(InetSocketAddress("127.0.0.1", 0), 0) + val mapper = JsonMapper.builder().build() + server.createContext("/") { exchange -> + val body = mapper.readTree(exchange.requestBody.readAllBytes()) + val response = + if (exchange.requestURI.path.endsWith("/transfer")) { + issued.incrementAndGet() + """{"token":"${"v".repeat(43)}","expiresAt":"${Instant.now().plusSeconds(15)}"}""" + } else { + """{"playerId":"$profile","sessionId":"${body.get("sessionId").asString()}","leaseExpiresAt":"${Instant.now().plusSeconds(90)}"}""" + } + val bytes = response.toByteArray() + exchange.sendResponseHeaders(201, bytes.size.toLong()) + exchange.responseBody.use { it.write(bytes) } + } + server.start() + val player = + Proxy.newProxyInstance(Player::class.java.classLoader, arrayOf(Player::class.java)) { + proxy, + method, + args -> + when (method.name) { + "getUniqueId" -> profile + "getUsername" -> "TransferProbe" + "isActive" -> true + "hashCode" -> System.identityHashCode(proxy) + "equals" -> proxy === args?.get(0) + "storeCookie" -> null + "transferToHost" -> { + transferred.incrementAndGet() + null + } + else -> throw UnsupportedOperationException(method.name) + } + } as Player + val presence = PlayerPresenceService() + try { + presence.configure("http://127.0.0.1:${server.address.port}") + presence.claim(player, "source", "eu") + assertTrue(presence.transferToHost(player, "target.invalid", 25565)) + assertFalse(presence.transferToHost(player, "other.invalid", 25565)) + assertEquals(1, issued.get()) + assertEquals(1, transferred.get()) + } finally { + presence.close() + server.stop(0) + } + } +} diff --git a/velocity/src/test/kotlin/gg/grounds/presence/TransferAdmissionBridgeTest.kt b/velocity/src/test/kotlin/gg/grounds/presence/TransferAdmissionBridgeTest.kt new file mode 100644 index 0000000..ab660ba --- /dev/null +++ b/velocity/src/test/kotlin/gg/grounds/presence/TransferAdmissionBridgeTest.kt @@ -0,0 +1,48 @@ +package gg.grounds.presence + +import java.util.UUID +import org.junit.jupiter.api.Assertions.* +import org.junit.jupiter.api.Test + +class TransferAdmissionBridgeTest { + private val profile = UUID.randomUUID() + private val connection = UUID.randomUUID() + + private fun valid() = + mapOf( + "playerId" to profile.toString(), + "handoffToken" to "v".repeat(43), + "connectionId" to connection.toString(), + "destinationProxyId" to "target", + ) + + @Test + fun `bridge keeps the exact runtime connection identity`() { + assertEquals( + TransferAdmissionBridge.Admission("v".repeat(43), connection), + TransferAdmissionBridge.parse(valid(), profile, "target"), + ) + } + + @Test + fun `foreign authenticated profiles and proxies cannot use the reservation`() { + assertNull(TransferAdmissionBridge.parse(valid(), UUID.randomUUID(), "target")) + assertNull(TransferAdmissionBridge.parse(valid(), profile, "other")) + assertNull(TransferAdmissionBridge.parse(valid(), profile, null)) + } + + @Test + fun `malformed or missing runtime data fails closed`() { + assertNull(TransferAdmissionBridge.parse(null, profile, "target")) + assertNull( + TransferAdmissionBridge.parse( + valid() + ("connectionId" to "invalid"), + profile, + "target", + ) + ) + assertNull( + TransferAdmissionBridge.parse(valid() + ("handoffToken" to ""), profile, "target") + ) + } +} From 755c21c761397db7e8063a23cab18892f7df70c3 Mon Sep 17 00:00:00 2001 From: Lukas Jost Date: Sat, 31 Oct 2026 20:44:17 +0100 Subject: [PATCH 2/2] fix: address transfer admission review regressions --- .../player/presence/HttpPlayerPresenceClient.kt | 11 +++++++++-- .../gg/grounds/presence/PlayerPresenceService.kt | 4 +++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/common/src/main/kotlin/gg/grounds/player/presence/HttpPlayerPresenceClient.kt b/common/src/main/kotlin/gg/grounds/player/presence/HttpPlayerPresenceClient.kt index 7a6c55d..5504852 100644 --- a/common/src/main/kotlin/gg/grounds/player/presence/HttpPlayerPresenceClient.kt +++ b/common/src/main/kotlin/gg/grounds/player/presence/HttpPlayerPresenceClient.kt @@ -196,8 +196,15 @@ class HttpPlayerPresenceClient( return response.statusCode() == 204 } - fun issueHandoff(playerId: UUID, sessionId: UUID): SessionHandoff? { - val body = mapper.writeValueAsString(mapOf("sessionId" to sessionId.toString())) + fun issueHandoff( + playerId: UUID, + sessionId: UUID, + boundedAdmission: Boolean = false, + ): SessionHandoff? { + val body = + mapper.writeValueAsString( + mapOf("sessionId" to sessionId.toString(), "boundedAdmission" to boundedAdmission) + ) val response = send( request("/v2/players/$playerId/session/transfer") diff --git a/velocity/src/main/kotlin/gg/grounds/presence/PlayerPresenceService.kt b/velocity/src/main/kotlin/gg/grounds/presence/PlayerPresenceService.kt index 8c1d5db..39a3d33 100644 --- a/velocity/src/main/kotlin/gg/grounds/presence/PlayerPresenceService.kt +++ b/velocity/src/main/kotlin/gg/grounds/presence/PlayerPresenceService.kt @@ -53,7 +53,9 @@ class PlayerPresenceService : AutoCloseable { fun issueHandoff(player: Player): SessionHandoff? { val lease = lease(player) ?: return null - return withClient(null) { it.issueHandoff(lease.playerId, lease.sessionId) } + return withClient(null) { + it.issueHandoff(lease.playerId, lease.sessionId, TransferAdmissionBridge.required) + } } fun consumeHandoff(