Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions core/src/main/java/com/minekube/connect/config/ConfigLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,33 @@ public final class ConfigLoader {

private final ConnectLogger logger;

/**
* {@code login-reassert.restore-full-profile} is read only from inside the late pre-login and
* game-profile handlers, and every one of them returns early while
* {@code login-reassert.enabled} is off. An operator who flips only the second key therefore
* keeps the UUID they were trying to replace while every log line still looks healthy.
*/
static final String INERT_RESTORE_FULL_PROFILE_WARNING =
"login-reassert.restore-full-profile has no effect while login-reassert is disabled "
+ "(login-reassert.enabled: false). Set login-reassert.enabled: true to restore"
+ " the full game profile, or leave restore-full-profile: false if another"
+ " plugin is meant to keep changing Connect's login decision.";

/**
* One startup warning for the combination that looks configured but cannot act on a proxy.
* Spigot has no proxy config at all, so this stays silent there.
*/
private void warnAboutInertLoginReassert(ConnectConfig loaded) {
if (!(loaded instanceof ProxyConnectConfig)) {
return;
}
ProxyConnectConfig.LoginReassertConfig loginReassert =
((ProxyConnectConfig) loaded).getLoginReassert();
if (!loginReassert.isEnabled() && loginReassert.isRestoreFullProfile()) {
logger.warn(INERT_RESTORE_FULL_PROFILE_WARNING);
}
}

private static void fixPlaceholderIssue(Path path) throws IOException {
Charset charset = StandardCharsets.UTF_8;
String content = new String(Files.readAllBytes(path), charset)
Expand Down Expand Up @@ -122,6 +149,7 @@ public <T extends ConnectConfig> T load() {
if (useMinekubeBedrockIdentityDefaults) {
loaded.getBedrockIdentity().useMinekubeDefaults();
}
warnAboutInertLoginReassert(loaded);
return loaded;
} catch (Throwable throwable) {
throw new RuntimeException(
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/resources/proxy-config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ login-reassert:
# Also restore Connect's full game profile - the player's Mojang UUID and username - and
# not just the skin properties.
#
# Only has an effect while `login-reassert.enabled` is true above: with `enabled: false` the
# re-assert never runs, so this option changes nothing. Connect logs a warning at startup when
# both are set that way.
#
# PREREQUISITE, read before enabling: only turn this on if every login plugin on this proxy
# keys its own database on the player's Mojang UUID. For LibreLogin that means setting
# `new-uuid-creator: MOJANG` in LibreLogin's config first. With its default `CRACKED` it
Expand Down
106 changes: 100 additions & 6 deletions core/src/test/java/com/minekube/connect/config/ConfigLoaderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockingDetails;
import static org.mockito.Mockito.verify;

import com.minekube.connect.api.logger.ConnectLogger;
import java.nio.file.Files;
Expand Down Expand Up @@ -201,11 +203,7 @@ void loadsLoginReassertOverrides() throws Exception {
*/
@Test
void shippedProxyTemplateDocumentsTheFullProfilePrerequisite() throws Exception {
String template;
try (java.io.InputStream in =
ConfigLoader.class.getResourceAsStream("/proxy-config.yml")) {
template = new String(in.readAllBytes(), java.nio.charset.StandardCharsets.UTF_8);
}
String template = shippedProxyTemplate();

int optionAt = template.indexOf("restore-full-profile:");
assertTrue(optionAt > 0, "proxy-config.yml must ship the restore-full-profile option");
Expand All @@ -214,11 +212,107 @@ void shippedProxyTemplateDocumentsTheFullProfilePrerequisite() throws Exception
"the LibreLogin-side prerequisite must be documented next to the option");
}

/**
* Field report 2026-10-21 (endpoint {@code servermc4321}): the operator ran
* {@code login-reassert.enabled: false} together with {@code restore-full-profile: true} and
* expected Mojang UUIDs. The second key is read only from inside the late handlers, which all
* return early while the first is off, so it was silently inert - nothing in either log named
* the combination and the join was diagnosed on both sides for hours. The config loader is the
* single startup path on every proxy platform, so exactly one warning belongs here.
*/
@Test
void warnsWhenRestoreFullProfileCannotRunBecauseLoginReassertIsDisabled() throws Exception {
Files.writeString(tempDir.resolve("config.yml"),
proxyConfigWithLoginReassert(" enabled: false", " restore-full-profile: true"));
ConnectLogger logger = mock(ConnectLogger.class);

load(ProxyConnectConfig.class, logger);

assertTrue(ConfigLoader.INERT_RESTORE_FULL_PROFILE_WARNING.contains(
"restore-full-profile has no effect while login-reassert is disabled"),
"the warning must name the exact inert combination an operator has to undo");
verify(logger).warn(ConfigLoader.INERT_RESTORE_FULL_PROFILE_WARNING);
}

/** The shipped default (`enabled: true`, `restore-full-profile: false`) must stay quiet. */
@Test
void doesNotWarnForTheShippedLoginReassertDefaults() throws Exception {
Files.writeString(tempDir.resolve("config.yml"),
proxyConfigWithLoginReassert(" enabled: true", " restore-full-profile: false"));
ConnectLogger logger = mock(ConnectLogger.class);

load(ProxyConnectConfig.class, logger);

assertNoWarnings(logger);
}

/**
* Either key alone is a valid configuration: turning the floor off without asking for the
* full profile is the documented way to let another plugin keep changing the decision.
*/
@Test
void doesNotWarnWhenOnlyOneLoginReassertKeyIsSet() throws Exception {
Files.writeString(tempDir.resolve("config.yml"),
proxyConfigWithLoginReassert(" enabled: false", " restore-full-profile: false"));
ConnectLogger logger = mock(ConnectLogger.class);

load(ProxyConnectConfig.class, logger);

assertNoWarnings(logger);
}

/**
* The template is where an operator reads the option before flipping it, and it used to
* present {@code restore-full-profile} as if it acted on its own. Pin that it states the
* dependency in the option's own comment, not only somewhere else in the file.
*/
@Test
void shippedProxyTemplateMarksRestoreFullProfileAsConditionalOnEnabled() throws Exception {
String documentation = shippedProxyTemplate();

int optionAt = documentation.indexOf("restore-full-profile:");
assertTrue(optionAt > 0, "proxy-config.yml must ship the restore-full-profile option");
assertTrue(documentation.substring(0, optionAt)
.contains("Only has an effect while `login-reassert.enabled` is true"),
"the option's comment must say it is inert while login-reassert is disabled");
}

private static String proxyConfigWithLoginReassert(String enabledLine, String restoreLine) {
return String.join("\n",
"endpoint: codexp2p3",
"allow-offline-mode-players: false",
"login-reassert:",
enabledLine,
restoreLine,
"metrics:",
" disabled: true",
" uuid: 00000000-0000-0000-0000-000000000000",
"config-version: 1",
"");
}

private static String shippedProxyTemplate() throws Exception {
try (java.io.InputStream in =
ConfigLoader.class.getResourceAsStream("/proxy-config.yml")) {
return new String(in.readAllBytes(), java.nio.charset.StandardCharsets.UTF_8);
}
}

private static void assertNoWarnings(ConnectLogger logger) {
boolean warned = mockingDetails(logger).getInvocations().stream()
.anyMatch(invocation -> invocation.getMethod().getName().equals("warn"));
assertFalse(warned, "a valid login-reassert combination must not warn");
}

private <T extends ConnectConfig> T load(Class<T> configClass) {
return load(configClass, mock(ConnectLogger.class));
}

private <T extends ConnectConfig> T load(Class<T> configClass, ConnectLogger logger) {
return new ConfigLoader(
tempDir,
configClass,
new ConfigLoader.EndpointNameGenerator(new OkHttpClient()),
mock(ConnectLogger.class)).load();
logger).load();
}
}
4 changes: 4 additions & 0 deletions docs/login-plugin-integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ able to change Connect's login decision. Off restores exactly the previous behav
restores the player's **Mojang UUID and username**, which is what makes
`ConnectApi.isConnectPlayer(uuid)` and `getPlayer(uuid)` resolve for these players.

> **It only has an effect while `enabled` is `true`.** With `login-reassert.enabled: false` the
> re-assert never runs, so `restore-full-profile` changes nothing — players keep the UUID the
> login plugin set. Connect logs one startup warning when both are set that way.

If late-handler registration fails, Connect logs the failure, skips the re-assert floor, and
continues with its pre-existing handlers. Ordinary listener registration is unaffected.

Expand Down
6 changes: 6 additions & 0 deletions velocity/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ The root `AGENTS.md` still applies; this file adds Velocity-specific guidance.
- 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.
- `restore-full-profile` is inert while `login-reassert.enabled` is false, because every late
handler returns before it reads the option. Keep that combination operator-visible: one startup
WARN from `ConfigLoader` (pinned by `ConfigLoaderTest`), the dependency stated in the
`proxy-config.yml` comment, and the Velocity game-profile re-assert logging Connect's UUID vs
the event's UUID at DEBUG so a "right player, wrong UUID - only through the tunnel" report is
diagnosable from the endpoint log alone.
- `: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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,14 @@ void onGameProfileRequestLate(GameProfileRequestEvent event) {
}
GameProfile current = event.getGameProfile();
GameProfile connectProfile = VelocityGameProfiles.fromConnectPlayer(current, player);
if (!current.getId().equals(connectProfile.getId())) {
// "Same player, wrong UUID - but only through the tunnel" is only diagnosable if the
// endpoint log names both UUIDs; comparing the two sides out of band took hours.
logger.debug("Connect supplied profile UUID {} but the game profile event carries {}"
+ " (login-reassert.restore-full-profile: {})",
connectProfile.getId(), current.getId(),
config.getLoginReassert().isRestoreFullProfile());
}
GameProfile wanted = config.getLoginReassert().isRestoreFullProfile()
? connectProfile
: current.withProperties(connectProfile.getProperties());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.velocitypowered.api.util.GameProfile.Property;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.UUID;
Expand Down Expand Up @@ -177,6 +178,45 @@ void gameProfileIgnoresConnectionsConnectDidNotAuthenticate() throws Exception {
assertSame(before, event.getGameProfile());
}

/**
* The {@code servermc4321} report was "right player, wrong UUID - but only through the
* tunnel": the endpoint log showed a join and never named either UUID, so the two sides had
* to be compared out of band. DEBUG now carries both, which is what makes that class of
* report diagnosable from the proxy's own log.
*/
@Test
void gameProfileLogsConnectAndEventUuidsAtDebugWhenALoginPluginLeftItsOwn() throws Exception {
RecordingLogger logger = new RecordingLogger();
Fixture fixture = Fixture.connectSession(true, false, logger);
GameProfileRequestEvent event = fixture.gameProfileRequest();

fixture.listener.onGameProfileRequestLate(event);

assertTrue(logger.debugMessages.stream().anyMatch(message ->
message.contains(CONNECT_UUID.toString())
&& message.contains(PROXY_UUID.toString())),
"DEBUG must name Connect's UUID and the UUID the game profile event carried: "
+ logger.debugMessages);
}

/** No mismatch, no line: the normal case with no conflicting plugin installed stays quiet. */
@Test
void gameProfileDoesNotLogUuidsWhenTheProfileAlreadyMatches() throws Exception {
RecordingLogger logger = new RecordingLogger();
Fixture fixture = Fixture.connectSession(true, false, logger);
GameProfileRequestEvent event = fixture.gameProfileRequest();
// What VelocityListener#onGameProfileRequest already did at PostOrder.EARLY.
event.setGameProfile(
VelocityGameProfiles.fromConnectPlayer(event.getGameProfile(), connectPlayer()));

fixture.listener.onGameProfileRequestLate(event);

assertFalse(logger.debugMessages.stream().anyMatch(message ->
message.contains(CONNECT_UUID.toString())
&& message.contains(PROXY_UUID.toString())),
"an unchanged profile needs no UUID diagnostic: " + logger.debugMessages);
}

@Test
void partialRegistrationRollsBackOnlyTheHandlerThatWasRegistered() {
PartialFailureEventManager eventManager = new PartialFailureEventManager();
Expand Down Expand Up @@ -204,18 +244,28 @@ private static final class Fixture {

private Fixture(boolean enabled, boolean restoreFullProfile, boolean connectSession)
throws Exception {
this(enabled, restoreFullProfile, connectSession, mock(ConnectLogger.class));
}

private Fixture(boolean enabled, boolean restoreFullProfile, boolean connectSession,
ConnectLogger logger) throws Exception {
VelocityConnectPlayers players = new VelocityConnectPlayers();
if (connectSession) {
players.remember(connection, connectPlayer());
}
listener = new VelocityLateReassertListener(
players, config(enabled, restoreFullProfile), mock(ConnectLogger.class));
players, config(enabled, restoreFullProfile), logger);
}

static Fixture connectSession(boolean enabled, boolean restoreFullProfile) throws Exception {
return new Fixture(enabled, restoreFullProfile, true);
}

static Fixture connectSession(boolean enabled, boolean restoreFullProfile,
ConnectLogger logger) throws Exception {
return new Fixture(enabled, restoreFullProfile, true, logger);
}

static Fixture foreignSession(boolean enabled, boolean restoreFullProfile) throws Exception {
return new Fixture(enabled, restoreFullProfile, false);
}
Expand Down Expand Up @@ -304,4 +354,54 @@ public <E> void unregister(Object plugin, EventHandler<E> handler) {
unregisteredHandlers.add(handler);
}
}

/**
* Captures DEBUG lines as {@code message + args}, so a test can assert on the values a log
* line actually carries instead of on Mockito's varargs matching.
*/
private static final class RecordingLogger implements ConnectLogger {
final List<String> debugMessages = new ArrayList<>();

@Override
public void error(String message, Object... args) {
}

@Override
public void error(String message, Throwable throwable, Object... args) {
}

@Override
public void warn(String message, Object... args) {
}

@Override
public void info(String message, Object... args) {
}

@Override
public void translatedInfo(String message, Object... args) {
}

@Override
public void debug(String message, Object... args) {
debugMessages.add(message + " " + Arrays.toString(args));
}

@Override
public void trace(String message, Object... args) {
}

@Override
public void enableDebug() {
}

@Override
public void disableDebug() {
}

@Override
public boolean isDebug() {
return true;
}
}
}
Loading