fix(security): keep quit processing fail-closed so no stale login survives by sgdc3 · Pull Request #3154 · AuthMe/AuthMeReloaded

sgdc3 · 2026-09-28T22:18:58Z

Summary

Makes quit processing fail-closed, so a player can never stay authenticated in the PlayerCache after leaving, and hardens related paths found while auditing the issue.

An exception during quit processing could abort it before the player was removed from the PlayerCache, leaving the name authenticated after the player had left. On Folia this happened whenever an ender pearl was owned by another region, because the quit handler read the shooter of every pearl in every world.

Quit processing

PlayerListener.onPlayerQuit: performQuit runs in a finally; only pearls owned by the current region are read; names in the antibot kick list no longer skip it
AsynchronousQuit: cache removal in a finally, best-effort quit-location save, drops the name-keyed proxy/premium auto-login grants, resets a pending 2FA limbo state
AsyncPlayerPreLoginEvent (LOWEST): discards the cached authentication of a name that is not online
onPlayerKick no longer processes the quit: PlayerQuitEvent is fired for kicked players too, and the kick can still be cancelled by another listener

Related hardening

SchedulingAdapter / BukkitService: isOwnedByCurrentThread(Entity|Location), also used by the pearl/vehicle restore at login
console commands are dispatched on the global region (Folia)
the pearl/vehicle snapshot saved on quit no longer records the operator flag, which the limbo merge re-applied on the next login even after a deop
SpawnLoader falls back to the spawn if the respawn location can't be read; OnShutdownPlayerSaver handles each player separately; the email recall task no longer uses the Bukkit scheduler
unauthenticated players: teleports by pearls, chorus fruit, portals or the spectator menu, arrow pickup, breaking hanging entities and vehicles, and item use on air clicks are now blocked

Testing

mvn -B test (JDK 21): all modules pass, with new tests in AsynchronousQuitTest, PlayerListenerTest, EntityListenerTest, LimboServiceTest and BukkitServiceTest
Smoke test on a Folia/Canvas server: quit with a pearl in another region, quit while de

…vives An exception thrown while a player quit could abort the quit processing before the player was removed from the PlayerCache, leaving the name authenticated after the player had left. On Folia this happened whenever an ender pearl was owned by another region, since the quit handler read the shooter of every pearl. Quit processing: - PlayerListener.onPlayerQuit runs performQuit in a finally block and only reads pearls owned by the current region; the antibot kick list no longer skips it - AsynchronousQuit removes the player from the PlayerCache in a finally block, treats the quit-location save as best-effort, drops the name-keyed proxy and premium auto-login grants and resets a pending 2FA limbo state - a connecting player discards the cached authentication of a name that is not online (AsyncPlayerPreLoginEvent, LOWEST) - onPlayerKick no longer processes the quit: PlayerQuitEvent is fired for kicked players too, and the kick may still be cancelled by another listener Related hardening: - SchedulingAdapter/BukkitService: isOwnedByCurrentThread(Entity|Location), also used by the pearl and vehicle restore at login - console commands are dispatched on the global region (Folia) - the pearl/vehicle snapshot saved on quit no longer records the operator flag, which the limbo merge re-applied on the next login even after a deop - SpawnLoader falls back to the spawn when the respawn location cannot be read, OnShutdownPlayerSaver handles each player separately and the email recall task no longer uses the Bukkit scheduler - unauthenticated players are no longer moved by pearls, chorus fruit, portals or spectator teleports, and can no longer pick up arrows, break hanging entities or vehicles, or use items by clicking in the air Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Xephi · 2026-10-01T06:43:45Z

-
-        final Player player = event.getPlayer();
-        if (!antiBotService.wasPlayerKicked(player.getName())) {
-            management.performQuit(player);


This change reopens the stale-auth race it is meant to fix. By removing management.performQuit(player) from onPlayerKick, a second connection using the same name can reach pre-login while the original authenticated player is still online. removeStaleAuthentication then keeps the cache entry, and before the eventual PlayerQuitEvent cleanup runs asynchronously, AsynchronousJoin sees playerCache.isAuthenticated(name) and returns without creating limbo or requiring a password.

That allows the new connection to inherit the existing session. We should take care of a potentially cancelled kick.

Addresses review feedback: with ForceSingleSession disabled, a connection taking over a name through a duplicate login could reach the join before the previous connection's asynchronous quit had removed it from the PlayerCache, and AsynchronousJoin would then treat it as already logged in. A player who has just joined has not logged in yet, so any authentication cached for their name belongs to another connection. PlayerListener now discards it on PlayerJoinEvent (LOWEST), before anything relies on it. The server only lets a duplicate login complete once the old player has been removed, so by then the old session is gone; kicks still need no handling since nothing is processed until an actual quit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Delayed quit tasks can corrupt replacement-connection state, and pre-login cleanup iterates Bukkit’s player collection asynchronously.

2 open findings

Stale quit task removes replacement connection grants · New
Unsafe asynchronous access to online player list · New

What changed in this PR

Hardens authentication cleanup during player disconnects and improves Folia-safe behavior.

Changes:

Makes quit cleanup fail-closed and clears connection-scoped authentication state.
Adds Folia ownership checks and global-region scheduling.
Expands unauthenticated-player restrictions and regression tests.

File	Description
`FoliaPlatformAdapter.java`	Adds location ownership checks.
`BukkitServiceTest.java`	Tests global console dispatch.
`AsynchronousQuitTest.java`	Tests fail-closed quit cleanup.
`PlayerListenerTest.java`	Tests quit, login, and movement restrictions.
`EntityListenerTest.java`	Tests entity protection.
`LimboServiceTest.java`	Tests region-safe restoration and OP handling.
`SpawnLoader.java`	Falls back when respawn lookup fails.
`PremiumLoginVerifier.java`	Adds verified-session removal.
`BungeeReceiver.java`	Consumes proxy login requests.
`BukkitService.java`	Adds ownership helpers and global dispatch.
`AsynchronousQuit.java`	Guarantees cache cleanup and resets transient state.
`SchedulingAdapter.java`	Extends thread-ownership API.
`AbstractSpigotPlatformAdapter.java`	Implements location ownership checks.
`PlayerListener.java`	Hardens quit, pre-login, teleport, and interaction handling.
`EntityListener.java`	Blocks additional unauthenticated entity actions.
`OnStartupTasks.java`	Moves email recalls to global scheduling.
`OnShutdownPlayerSaver.java`	Isolates per-player shutdown failures.
`LimboService.java`	Makes entity restoration region-aware.

🧠 Review effort: Balanced

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

+        proxySessionManager.removeLoginRequest(name);
+        premiumLoginVerifier.removeVerified(name);


+        for (Player player : bukkitService.getOnlinePlayers()) {
+            if (player.getName().equalsIgnoreCase(name)) {
+                return true;


PlayerQuitEvent is also fired for kicked players, so the quit process ran twice. It also ran when a listener after ours cancelled the kick, leaving the player online but logged out and without any limbo restriction. Ported from AuthMe#3154.

A player who has just joined has not logged in yet, so a cached authentication for their name belongs to another connection, e.g. one taken over through a duplicate login whose asynchronous quit cleanup has not run yet. Discard it at the lowest priority, before anything relies on it. Ported from AuthMe#3154.

An exception while saving the quit state of a logged-in player aborted the quit process before the player was removed from the PlayerCache, leaving the name authenticated after the player had left. The removal now runs in a finally block, and the quit location is saved on a best-effort basis: reading the respawn location of a player who quit while dead can throw on Folia. Ported from AuthMe#3154.

A queued perform.login was kept after it had already logged the online player in, and a premium verification outlived the connection it was made for. Both are keyed by name only, so a later connection using that name could inherit them. Drop the queued request once it has been completed, and discard the premium verification when the player quits. Ported from AuthMe#3154.

…t region On Folia, the quit handler read the shooter of every ender pearl in every world, and the login restore touched pearls and vehicles wherever they were. Accessing an entity owned by another region throws, which aborted the quit handling. Skip entities and pearl locations outside the current region, through a new SchedulingAdapter#isOwnedByCurrentThread(Location). Ported from AuthMe#3154.

…e read The respawn location is validated against the bed or anchor block. On Folia, reading that block throws when it is not owned by the current thread, e.g. during the async quit process or for a bed in another region, which aborted the callers. Use the spawn instead. Ported from AuthMe#3154.

On Folia, a console command must be dispatched from the global region thread, but the configured commands run on the player's region thread (e.g. on login). Hand the dispatch off to the global thread when called from another thread. Ported from AuthMe#3154.

The email reminder was handed to the Bukkit scheduler, which is not supported on Folia and throws on every run. Ported from AuthMe#3154.

An exception while saving one player, e.g. a region-owned read on Folia's shutdown thread, stopped the loop: the limbo data of the remaining players was not restored, and that player was left in the PlayerCache. Ported from AuthMe#3154.

Unauthenticated players could still trigger these actions. PlayerTeleportEvent and PlayerPortalEvent have their own handler lists, so onPlayerMove never saw ender pearl, chorus fruit, portal or spectator teleports. Arrows and tridents are picked up through PlayerPickupArrowEvent, not EntityPickupItemEvent. Clicks on air are fired already cancelled, so the ignoreCancelled interact handler never saw them and the item in hand could still be used. Ported from AuthMe#3154.

Non-living entities hit by a player do not go through EntityDamageByEntityEvent: paintings, item frames and leash knots are broken through HangingBreakByEntityEvent, boats and minecarts through the vehicle events, so unauthenticated players could still break them. Ported from AuthMe#3154.

sgdc3 requested review from Xephi and ljacqu September 28, 2026 22:18

Xephi requested changes Oct 1, 2026

View reviewed changes

sgdc3 requested a review from Xephi October 1, 2026 19:46

Xephi requested a balanced review from Copilot October 8, 2026 12:33

Copilot started reviewing on behalf of Xephi October 8, 2026 12:33 View session

Copilot AI reviewed Oct 8, 2026

View reviewed changes

Xephi closed this Oct 13, 2026

Navigation Menu

Repository navigation

fix(security): keep quit processing fail-closed so no stale login survives - #3154

fix(security): keep quit processing fail-closed so no stale login survives#3154
sgdc3 wants to merge 2 commits into
masterfrom
fix/quit-auth-bypass

sgdc3 commented Sep 28, 2026

Uh oh!

Xephi Oct 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

		proxySessionManager.removeLoginRequest(name);
		premiumLoginVerifier.removeVerified(name);

Conversation

sgdc3 commented Sep 28, 2026

Summary

Quit processing

Related hardening

Testing

Uh oh!

Xephi Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants