fix(proxy): re-assert Connect's login decision after other plugins

AGENTS.md

@@ -80,6 +80,39 @@ curl -I -L --fail https://github.com/minekube/connect-java/releases/download/<ve
   commitment). Pinned by
   `core/.../network/netty/ConnectPlayerAttributeBoundaryTest`.
 
+## Defensive login re-assert (proxy)
+
+- Connect registers a **second, late** pre-login handler that restores its own decision after
+  every other plugin has run where the platform supports strict-after ordering, fixing the whole
+  "a login plugin forces online mode after Connect" class (LibreLogin, AuthMe, nLogin) with no
+  runtime dependency on any of them. On older Velocity builds, the fallback is `PostOrder.LAST`
+  plus the optional LibreLogin load-order edge, so other plugins retain the old last-writer
+  behavior.
+  Preserve the property
+  that makes it safe: it reacts **only** to Connect's own result on the event object, never
+  reads/links against/version-checks a third-party plugin, never overrides a deny, and no-ops
+  when nothing changed the decision. The existing `EARLY`/`LOWEST` handlers stay as they are -
+  this only adds a floor. If late-handler registration throws, `VelocityListenerRegistration`
+  catches `Throwable` locally, logs the failure, and continues with the pre-existing behavior;
+  ordinary listener registration remains unchanged. Authoritative:
+  `velocity/.../listener/VelocityLateEventRegistrar.java` (the two layered ordering levers and
+  why each exists), `VelocityLateReassertListener`, `BungeeLateReassertListener`,
+  `core/src/main/resources/proxy-config.yml` (`login-reassert`), `docs/login-plugin-integration.md`.
+- **Do not bump velocity-api past `3.2.0-SNAPSHOT`** to reach `PostOrder.CUSTOM`: Velocity reads
+  the annotation while collecting a listener's methods, so an enum constant an older runtime
+  lacks throws `EnumConstantNotPresentException` there and kills *all* of Connect's handlers on
+  pre-2024-09-16 proxies, unguardably. The reflective short-`register` lookup buys the same
+  ordering with a catchable failure.
+- Default profile scope is properties-only (skin) deliberately; restoring Connect's UUID breaks
+  login plugins that key their storage on the proxy UUID, so it is opt-in with its
+  `new-uuid-creator: MOJANG` prerequisite documented next to the option.
+- `:velocity:eventOrderTest` is a separate source set running the **real** `VelocityEventManager`
+  and `PluginDependencyUtils` against a Velocity proxy jar pinned by sha256 (ivy repo in
+  `settings.gradle.kts`; PaperMC publishes no proxy artifact to Maven). Separate because that
+  shaded jar carries its own velocity-api and `com.velocitypowered.proxy` classes, which must not
+  shadow the 3.2.0 API or the stubs in `velocity/src/test`. It runs under `check`, so a
+  fill-data.papermc.io outage fails `./gradlew build` until the artifact resolves or is cached.
+
 ## Injector Scoping (config availability)
 
 - The parent injector binds `ConfigHolder`; `ConnectPlatform.init()` populates it

bungee/src/main/java/com/minekube/connect/listener/BungeeConnections.java

@@ -0,0 +1,64 @@
+/*
+ * Copyright (c) 2021-2022 Minekube. https://minekube.com
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * @author Minekube
+ * @link https://github.com/minekube/connect-java
+ */
+
+package com.minekube.connect.listener;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import com.minekube.connect.util.ReflectionUtils;
+import io.netty.channel.Channel;
+import java.lang.reflect.Field;
+import net.md_5.bungee.api.connection.PendingConnection;
+import net.md_5.bungee.connection.InitialHandler;
+import net.md_5.bungee.netty.ChannelWrapper;
+
+/**
+ * The BungeeCord internals Connect needs during login, which its public API does not expose.
+ */
+final class BungeeConnections {
+    private static final Field CHANNEL_WRAPPER;
+    private static final Field PLAYER_NAME;
+
+    static {
+        CHANNEL_WRAPPER =
+                ReflectionUtils.getFieldOfType(InitialHandler.class, ChannelWrapper.class);
+        checkNotNull(CHANNEL_WRAPPER, "ChannelWrapper field cannot be null");
+
+        PLAYER_NAME = ReflectionUtils.getField(InitialHandler.class, "name");
+        checkNotNull(PLAYER_NAME, "Initial name field cannot be null");
+    }
+
+    private BungeeConnections() {
+    }
+
+    static Channel channel(PendingConnection connection) {
+        ChannelWrapper wrapper = ReflectionUtils.getCastedValue(connection, CHANNEL_WRAPPER);
+        return wrapper.getHandle();
+    }
+
+    static void setName(PendingConnection connection, String name) {
+        ReflectionUtils.setValue(connection, PLAYER_NAME, name);
+    }
+}

bungee/src/main/java/com/minekube/connect/listener/BungeeLateReassertListener.java

@@ -0,0 +1,101 @@
+/*
+ * Copyright (c) 2021-2022 Minekube. https://minekube.com
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * @author Minekube
+ * @link https://github.com/minekube/connect-java
+ */
+
+package com.minekube.connect.listener;
+
+import com.google.inject.Inject;
+import com.minekube.connect.api.logger.ConnectLogger;
+import com.minekube.connect.api.player.ConnectPlayer;
+import com.minekube.connect.config.ProxyConnectConfig;
+import com.minekube.connect.network.netty.LocalSession;
+import net.md_5.bungee.api.connection.PendingConnection;
+import net.md_5.bungee.api.event.PreLoginEvent;
+import net.md_5.bungee.api.plugin.Listener;
+import net.md_5.bungee.event.EventHandler;
+
+/**
+ * The BungeeCord counterpart of {@code VelocityLateReassertListener}: re-asserts Connect's own
+ * pre-login decision after every other plugin has had its say, so that a login plugin forcing
+ * online mode on a Connect-tunneled connection cannot leave the player hanging at
+ * "Logging in..." forever.
+ *
+ * <p>{@link EventHandler#priority()} is a plain {@code byte} and BungeeCord's event bus
+ * dispatches the whole byte range, so {@link Byte#MAX_VALUE} runs strictly after
+ * {@code EventPriority.HIGHEST} (64). That is the only correct lever here: BungeeCord breaks
+ * ties between equal priorities with a {@code HashMap} keyed by listener identity, so
+ * registration order - and therefore any {@code softDepends} - decides nothing.
+ *
+ * <p>Same guarantees as on Velocity: only connections Connect itself authenticated are touched,
+ * another plugin's kick is never overridden, and {@code login-reassert.enabled: false} turns
+ * the whole thing off.
+ *
+ * <p>BungeeCord's {@code PendingConnection} exposes no profile-properties API at pre-login, so
+ * the profile half of this is limited to the UUID and username, behind the same opt-in
+ * {@code login-reassert.restore-full-profile}.
+ */
+public final class BungeeLateReassertListener implements Listener {
+    @Inject private ProxyConnectConfig config;
+    @Inject private ConnectLogger logger;
+
+    @EventHandler(priority = Byte.MAX_VALUE)
+    public void onPreLoginLate(PreLoginEvent event) {
+        if (!config.getLoginReassert().isEnabled()) {
+            return;
+        }
+        if (event.isCancelled()) {
+            return; // a plugin denied this login; never convert a kick into a join
+        }
+        try {
+            PendingConnection connection = event.getConnection();
+            LocalSession.context(BungeeConnections.channel(connection),
+                    ctx -> reassert(connection, ctx.getPlayer()));
+        } catch (Exception exception) {
+            // Never let the defensive floor itself break a login: without it the connection is
+            // exactly where it would have been before this listener existed.
+            logger.error("Failed to re-assert Connect's pre-login decision", exception);
+        }
+    }
+
+    private void reassert(PendingConnection connection, ConnectPlayer player) {
+        if (player.getAuth().isPassthrough()) {
+            return; // not authenticated by Connect - none of our business
+        }
+        if (connection.isOnlineMode()) {
+            connection.setOnlineMode(false);
+            logger.debug("Re-asserted offline mode for Connect session {} at pre-login; another "
+                            + "plugin had changed it (set login-reassert.enabled to false to allow that)",
+                    player.getUsername());
+        }
+        if (!config.getLoginReassert().isRestoreFullProfile()) {
+            return;
+        }
+        if (!player.getUniqueId().equals(connection.getUniqueId())) {
+            connection.setUniqueId(player.getUniqueId());
+        }
+        if (!player.getUsername().equals(connection.getName())) {
+            BungeeConnections.setName(connection, player.getUsername());
+        }
+    }
+}

bungee/src/main/java/com/minekube/connect/listener/BungeeListener.java

@@ -25,8 +25,6 @@
 
 package com.minekube.connect.listener;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-
 import com.google.inject.Inject;
 import com.minekube.connect.api.ProxyConnectApi;
 import com.minekube.connect.api.logger.ConnectLogger;
@@ -35,34 +33,17 @@
 import com.minekube.connect.bedrock.BedrockIdentityEnforcer.Decision;
 import com.minekube.connect.network.netty.LocalSession;
 import com.minekube.connect.util.LanguageManager;
-import com.minekube.connect.util.ReflectionUtils;
-import io.netty.channel.Channel;
-import java.lang.reflect.Field;
 import java.util.UUID;
 import net.md_5.bungee.api.connection.PendingConnection;
 import net.md_5.bungee.api.event.LoginEvent;
 import net.md_5.bungee.api.event.PlayerDisconnectEvent;
 import net.md_5.bungee.api.event.PreLoginEvent;
 import net.md_5.bungee.api.plugin.Listener;
-import net.md_5.bungee.connection.InitialHandler;
 import net.md_5.bungee.event.EventHandler;
 import net.md_5.bungee.event.EventPriority;
-import net.md_5.bungee.netty.ChannelWrapper;
 
 @SuppressWarnings("ConstantConditions")
 public final class BungeeListener implements Listener {
-    private static final Field CHANNEL_WRAPPER;
-    private static final Field PLAYER_NAME;
-
-    static {
-        CHANNEL_WRAPPER =
-                ReflectionUtils.getFieldOfType(InitialHandler.class, ChannelWrapper.class);
-        checkNotNull(CHANNEL_WRAPPER, "ChannelWrapper field cannot be null");
-
-        PLAYER_NAME = ReflectionUtils.getField(InitialHandler.class, "name");
-        checkNotNull(PLAYER_NAME, "Initial name field cannot be null");
-    }
-
     @Inject private ProxyConnectApi api;
     @Inject private LanguageManager languageManager;
     @Inject private ConnectLogger logger;
@@ -77,10 +58,7 @@ public void onPreLogin(PreLoginEvent event) {
 
         PendingConnection connection = event.getConnection();
 
-        ChannelWrapper wrapper = ReflectionUtils.getCastedValue(connection, CHANNEL_WRAPPER);
-        Channel channel = wrapper.getHandle();
-
-        LocalSession.context(channel, ctx -> {
+        LocalSession.context(BungeeConnections.channel(connection), ctx -> {
             Decision decision = bedrockIdentityEnforcer.verify(ctx);
             if (!decision.allowed()) {
                 bedrockIdentityEnforcer.reject(ctx, decision);
@@ -90,7 +68,7 @@ public void onPreLogin(PreLoginEvent event) {
             }
             connection.setOnlineMode(false);
             connection.setUniqueId(ctx.getPlayer().getUniqueId());
-            ReflectionUtils.setValue(connection, PLAYER_NAME, ctx.getPlayer().getUsername());
+            BungeeConnections.setName(connection, ctx.getPlayer().getUsername());
             // TODO robin: what about profile properties? (but why is skin already showing)
         });
     }

bungee/src/main/java/com/minekube/connect/module/BungeeListenerModule.java

@@ -29,6 +29,7 @@
 import com.google.inject.Singleton;
 import com.google.inject.TypeLiteral;
 import com.google.inject.multibindings.ProvidesIntoSet;
+import com.minekube.connect.listener.BungeeLateReassertListener;
 import com.minekube.connect.listener.BungeeListener;
 import com.minekube.connect.register.ListenerRegister;
 import net.md_5.bungee.api.plugin.Listener;
@@ -45,4 +46,10 @@ public Listener bungeeListener() {
         return new BungeeListener();
     }
 
+    @Singleton
+    @ProvidesIntoSet
+    public Listener bungeeLateReassertListener() {
+        return new BungeeLateReassertListener();
+    }
+
 }

bungee/src/test/java/com/minekube/connect/BungeePluginStartupTest.java

@@ -41,6 +41,7 @@
 import com.minekube.connect.bedrock.BedrockIdentityKeyProvider;
 import com.minekube.connect.inject.CommonPlatformInjector;
 import com.minekube.connect.inject.bungee.BungeeInjector;
+import com.minekube.connect.listener.BungeeLateReassertListener;
 import com.minekube.connect.listener.BungeeListener;
 import com.minekube.connect.listener.BungeeListenerRegistration;
 import com.minekube.connect.module.ProxyCommonModule;
@@ -80,6 +81,7 @@ class BungeePluginStartupTest {
     private static List<Class<?>> bungeeGraphRoots() {
         List<Class<?>> roots = new ArrayList<>(StartupGraphProvisioning.coreRuntimeGraphRoots());
         roots.add(BungeeListener.class);
+        roots.add(BungeeLateReassertListener.class);
         roots.add(BungeeCommandUtil.class);
         roots.add(BungeePlatformUtils.class);
         roots.add(BungeeInjector.class);

bungee/src/test/java/com/minekube/connect/listener/BungeeLateReassertListenerTest.java

@@ -0,0 +1,49 @@
+package com.minekube.connect.listener;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.lang.reflect.Method;
+import net.md_5.bungee.api.event.PreLoginEvent;
+import net.md_5.bungee.event.EventHandler;
+import net.md_5.bungee.event.EventPriority;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Pins BungeeCord's half of the ordering lever.
+ *
+ * <p>{@code EventPriority} is not an enum - it is a set of {@code byte} constants, the highest of
+ * which is {@code HIGHEST = 64}, while {@code EventHandler#priority()} is a plain {@code byte}
+ * and BungeeCord's event bus bakes handlers across the entire byte range. So
+ * {@link Byte#MAX_VALUE} is inside the annotation's declared domain and runs strictly after
+ * {@code HIGHEST}.
+ *
+ * <p>This is the only correct lever on BungeeCord: handlers of equal priority live in a
+ * {@code HashMap} keyed by listener identity, so the tie-break is not registration order and no
+ * {@code softDepends} in {@code plugin.yml} can influence it - unlike on Velocity, where the
+ * declared dependency graph is a real fallback.
+ */
+class BungeeLateReassertListenerTest {
+    @Test
+    void theReassertRunsAfterEveryOtherPluginsPreLoginHandler() throws Exception {
+        Method handler =
+                BungeeLateReassertListener.class.getMethod("onPreLoginLate", PreLoginEvent.class);
+
+        byte priority = handler.getAnnotation(EventHandler.class).priority();
+
+        assertEquals(Byte.MAX_VALUE, priority,
+                "the re-assert must run after HIGHEST; nothing lower is a floor");
+        assertTrue(priority > EventPriority.HIGHEST);
+    }
+
+    /**
+     * The re-assert only adds a floor. Connect's original pre-login handler keeps running first,
+     * so Connect's ordering relative to every other plugin is otherwise unchanged.
+     */
+    @Test
+    void theOriginalPreLoginHandlerStillRunsFirst() throws Exception {
+        Method original = BungeeListener.class.getMethod("onPreLogin", PreLoginEvent.class);
+
+        assertEquals(EventPriority.LOWEST, original.getAnnotation(EventHandler.class).priority());
+    }
+}

core/src/main/java/com/minekube/connect/config/ProxyConnectConfig.java

@@ -32,4 +32,33 @@
  */
 @Getter
 public final class ProxyConnectConfig extends ConnectConfig {
+    private static final LoginReassertConfig DEFAULT_LOGIN_REASSERT = new LoginReassertConfig();
+
+    /**
+     * Whether Connect re-asserts its own pre-login decision after every other plugin has run.
+     */
+    private LoginReassertConfig loginReassert = new LoginReassertConfig();
+
+    /**
+     * Never {@code null}: a config file written before this option existed keeps the defaults.
+     */
+    public LoginReassertConfig getLoginReassert() {
+        return loginReassert != null ? loginReassert : DEFAULT_LOGIN_REASSERT;
+    }
+
+    @Getter
+    public static class LoginReassertConfig {
+        /**
+         * Default on. Turning it off restores the behaviour of leaving Connect's pre-login
+         * decision to whichever plugin writes last, which is what an operator who deliberately
+         * wants another plugin to override Connect needs.
+         */
+        private boolean enabled = true;
+        /**
+         * Default off. Also restores Connect's UUID and username, not just the skin properties.
+         * Requires every login plugin on the proxy to key its own storage on the Mojang UUID -
+         * see the prerequisite documented next to this option in {@code proxy-config.yml}.
+         */
+        private boolean restoreFullProfile = false;
+    }
 }