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
17 changes: 17 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,16 @@ gh -R minekube/connect-java release view <version> --json tagName,targetCommitis
curl -I -L --fail https://github.com/minekube/connect-java/releases/download/<version>/connect-velocity.jar
```

## Injector Scoping (config availability)

- The parent injector binds `ConfigHolder`; `ConnectPlatform.init()` populates it
before `ConfigLoadedModule` binds `ConnectConfig` in the child injector.
- Anything reachable while constructing the parent/platform injector must avoid
injecting `ConnectConfig` directly. Depend on the parent-bound `ConfigHolder`
and read `configHolder.get()` lazily after initialization instead.
- The Bedrock identity graph follows this pattern; see
`core/.../module/BedrockParentInjectorStartupTest` for the regression guard.

## Velocity Join Bugs

- For Velocity proxy issues, test both `CONFIGURATION` and `PLAY` state packet
Expand All @@ -57,3 +67,10 @@ Run targeted module tests for packet/session fixes, then the broader build:
Do not call production fixed from a Connect Java release alone. Confirm the
released jar is in the hub image, the hub pod logs the expected plugin version,
and Moxy accepts a tunnel with the same `connectorVersion`.

## Maintaining this file

Keep this file for knowledge useful to almost every future agent session in this project.
Do not repeat what the codebase already shows; point to the authoritative file or command instead.
Prefer rewriting or pruning existing entries over appending new ones.
When updating this file, preserve this bar for all agents and keep entries concise.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.minekube.connect.api.player.bedrock.BedrockIdentityReplayCache;
import com.minekube.connect.api.player.bedrock.BedrockIdentityVerificationException;
import com.minekube.connect.api.player.bedrock.BedrockIdentityVerifier;
import com.minekube.connect.config.ConfigHolder;
import com.minekube.connect.config.ConnectConfig;
import com.minekube.connect.config.ConnectConfig.BedrockIdentityConfig;
import com.minekube.connect.network.netty.LocalSession;
Expand All @@ -28,14 +29,28 @@ public final class BedrockIdentityEnforcer {
private static final String PROTOCOL_BEDROCK = "bedrock";
private static final String REJECT_MESSAGE = "Bedrock identity verification failed";

private final ConnectConfig config;
private final Supplier<ConnectConfig> config;
private final ConnectLogger logger;
private final Supplier<Instant> now;
private final BedrockIdentityKeyProvider keyProvider;
private final BedrockAdmissionCoordinator admissionCoordinator;
private final BedrockIdentityReplayCache replayCache = new BedrockIdentityReplayCache();

/**
* Injection seam used by the plugin. The config is resolved lazily from {@link ConfigHolder} so
* this enforcer can be constructed by the (config-agnostic) parent injector while the platform
* injector is built, before {@code ConnectPlatform.init()} loads the configuration.
*/
@Inject
public BedrockIdentityEnforcer(
ConfigHolder configHolder,
ConnectLogger logger,
BedrockIdentityKeyProvider keyProvider,
BedrockAdmissionCoordinator admissionCoordinator) {
this(Objects.requireNonNull(configHolder, "configHolder")::get, logger, Instant::now,
keyProvider, admissionCoordinator);
}

public BedrockIdentityEnforcer(
ConnectConfig config,
ConnectLogger logger,
Expand Down Expand Up @@ -89,13 +104,31 @@ public BedrockIdentityEnforcer(
Supplier<Instant> now,
BedrockIdentityKeyProvider keyProvider,
BedrockAdmissionCoordinator admissionCoordinator) {
this(constant(config), logger, now, keyProvider, admissionCoordinator);
}

private BedrockIdentityEnforcer(
Supplier<ConnectConfig> config,
ConnectLogger logger,
Supplier<Instant> now,
BedrockIdentityKeyProvider keyProvider,
BedrockAdmissionCoordinator admissionCoordinator) {
this.config = Objects.requireNonNull(config, "config");
this.logger = Objects.requireNonNull(logger, "logger");
this.now = Objects.requireNonNull(now, "now");
this.keyProvider = Objects.requireNonNull(keyProvider, "keyProvider");
this.admissionCoordinator = admissionCoordinator;
}

private static Supplier<ConnectConfig> constant(ConnectConfig config) {
Objects.requireNonNull(config, "config");
return () -> config;
}

private ConnectConfig config() {
return Objects.requireNonNull(config.get(), "config");
}

BedrockIdentityEnforcer(
ConnectConfig config,
ConnectLogger logger,
Expand Down Expand Up @@ -180,7 +213,7 @@ Decision verify(
SessionProtocol protocol) {
Objects.requireNonNull(player, "player");
Objects.requireNonNull(protocol, "protocol");
BedrockIdentityConfig bedrockIdentity = config.getBedrockIdentity();
BedrockIdentityConfig bedrockIdentity = config().getBedrockIdentity();
BedrockIdentityConfiguration identityConfiguration = BedrockIdentityConfiguration.from(bedrockIdentity);
if (identityConfiguration.isDisabled()) {
return Decision.allowed(null);
Expand Down Expand Up @@ -280,7 +313,7 @@ private BedrockIdentityVerifier verifier(
.publicKey(publicKey)
.now(now)
.endpointId(endpointId)
.endpointName(config.getEndpoint())
.endpointName(config().getEndpoint())
.orgId(endpointOrgId)
.sessionId(player.getSessionId())
.protocol(PROTOCOL_BEDROCK)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

import com.google.gson.Gson;
import com.google.gson.JsonSyntaxException;
import com.google.inject.name.Named;
import com.minekube.connect.api.player.bedrock.BedrockIdentityVerifier;
import com.minekube.connect.config.ConfigHolder;
import com.minekube.connect.config.ConnectConfig;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
Expand All @@ -16,29 +18,49 @@
import java.util.Objects;
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
import javax.inject.Inject;
import javax.inject.Singleton;
import okhttp3.HttpUrl;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;
import okhttp3.ResponseBody;

@Singleton
public final class BedrockIdentityKeyProvider {
private static final Gson GSON = new Gson();
private static final long METADATA_BODY_LIMIT_BYTES = 64 * 1024;
private static final long METADATA_CALL_TIMEOUT_SECONDS = 5;
private static final long METADATA_FAILURE_BACKOFF_SECONDS = 5;

private final ConnectConfig config;
private final Supplier<ConnectConfig> config;
private final OkHttpClient httpClient;
private final Supplier<Instant> now;
private CachedKeys cachedKeys;
private Instant nextRefreshAt = Instant.MIN;

/**
* Injection seam used by the plugin. The config is resolved lazily from {@link ConfigHolder} so
* this provider can be constructed by the (config-agnostic) parent injector before
* {@code ConnectPlatform.init()} loads the configuration.
*/
@Inject
public BedrockIdentityKeyProvider(
ConfigHolder configHolder,
@Named("defaultHttpClient") OkHttpClient httpClient) {
this(Objects.requireNonNull(configHolder, "configHolder")::get, httpClient, Instant::now);
}

public BedrockIdentityKeyProvider(ConnectConfig config, OkHttpClient httpClient) {
this(config, httpClient, Instant::now);
}

BedrockIdentityKeyProvider(ConnectConfig config, OkHttpClient httpClient, Supplier<Instant> now) {
this(constant(config), httpClient, now);
}

private BedrockIdentityKeyProvider(
Supplier<ConnectConfig> config, OkHttpClient httpClient, Supplier<Instant> now) {
this.config = Objects.requireNonNull(config, "config");
this.httpClient = Objects.requireNonNull(httpClient, "httpClient").newBuilder()
.callTimeout(METADATA_CALL_TIMEOUT_SECONDS, TimeUnit.SECONDS)
Expand All @@ -48,9 +70,18 @@ public BedrockIdentityKeyProvider(ConnectConfig config, OkHttpClient httpClient)
this.now = Objects.requireNonNull(now, "now");
}

private static Supplier<ConnectConfig> constant(ConnectConfig config) {
Objects.requireNonNull(config, "config");
return () -> config;
}

private ConnectConfig config() {
return Objects.requireNonNull(config.get(), "config");
}

synchronized List<byte[]> keys() {
List<byte[]> staticKeys = staticKeys();
String metadataUrl = config.getBedrockIdentity().getMetadataUrl();
String metadataUrl = config().getBedrockIdentity().getMetadataUrl();
if (metadataUrl == null || metadataUrl.isEmpty()) {
return staticKeys;
}
Expand Down Expand Up @@ -83,7 +114,7 @@ synchronized List<byte[]> keys() {

synchronized boolean hasUsableKeys() {
List<byte[]> keys = keys();
String metadataUrl = config.getBedrockIdentity().getMetadataUrl();
String metadataUrl = config().getBedrockIdentity().getMetadataUrl();
if (metadataUrl == null || metadataUrl.isEmpty()) {
return !keys.isEmpty();
}
Expand All @@ -103,7 +134,7 @@ private RemoteKeys fetchKeys(String metadataUrl) throws IOException {
}
VerifierMetadata metadata = GSON.fromJson(readBody(body), VerifierMetadata.class);
if (metadata == null || !"Ed25519".equals(metadata.algorithm) ||
!config.getBedrockIdentity().getExpectedIssuer().equals(metadata.issuer)) {
!config().getBedrockIdentity().getExpectedIssuer().equals(metadata.issuer)) {
throw new IllegalArgumentException("metadata scope is invalid");
}
List<byte[]> keys = new ArrayList<>();
Expand Down Expand Up @@ -178,9 +209,9 @@ private Instant failureBackoffUntil(Instant current) {

private List<byte[]> staticKeys() {
List<byte[]> keys = new ArrayList<>();
addKey(keys, config.getBedrockIdentity().getPublicKey());
if (config.getBedrockIdentity().getPublicKeys() != null) {
for (String key : config.getBedrockIdentity().getPublicKeys()) {
addKey(keys, config().getBedrockIdentity().getPublicKey());
if (config().getBedrockIdentity().getPublicKeys() != null) {
for (String key : config().getBedrockIdentity().getPublicKeys()) {
addKey(keys, key);
}
}
Expand Down Expand Up @@ -213,15 +244,15 @@ private static void addMetadataKey(List<byte[]> keys, String encoded) {
}

private int metadataCacheSeconds(int remoteCacheSeconds) {
int configured = config.getBedrockIdentity().getMetadataCacheSeconds();
int configured = config().getBedrockIdentity().getMetadataCacheSeconds();
if (configured <= 0) {
return remoteCacheSeconds;
}
return remoteCacheSeconds <= 0 ? configured : Math.min(configured, remoteCacheSeconds);
}

private int metadataMaxStaleSeconds() {
return Math.max(0, config.getBedrockIdentity().getMetadataMaxStaleSeconds());
return Math.max(0, config().getBedrockIdentity().getMetadataMaxStaleSeconds());
}

private static final class CachedKeys {
Expand Down
14 changes: 5 additions & 9 deletions core/src/main/java/com/minekube/connect/module/CommonModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -125,20 +125,16 @@ public OkHttpClient defaultOkHttpClient() {
return HttpUtils.defaultOkHttpClient();
}

@Provides
@Singleton
public BedrockIdentityKeyProvider bedrockIdentityKeyProvider(
ConnectConfig config,
@Named("defaultHttpClient") OkHttpClient httpClient) {
return new BedrockIdentityKeyProvider(config, httpClient);
}
// BedrockIdentityKeyProvider is bound just-in-time (@Inject @Singleton) so it and the enforcer
// can be resolved by the config-agnostic parent injector without pulling ConnectConfig into it.
// See BedrockParentInjectorStartupTest.

@Provides
@Singleton
public BedrockIdentityReadiness bedrockIdentityReadiness(
ConnectConfig config,
ConfigHolder configHolder,
BedrockIdentityKeyProvider keyProvider) {
return new BedrockIdentityReadiness(config, keyProvider);
return new BedrockIdentityReadiness(configHolder.get(), keyProvider);
}

@Provides
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,21 @@ void retainsLegacyHttpClientConstructor() throws Exception {
new ConnectConfig(), mock(ConnectLogger.class), new OkHttpClient()));
}

@Test
void retainsLegacyConfigConstructor() throws Exception {
Constructor<BedrockIdentityEnforcer> constructor = BedrockIdentityEnforcer.class.getConstructor(
ConnectConfig.class,
ConnectLogger.class,
BedrockIdentityKeyProvider.class,
BedrockAdmissionCoordinator.class);

assertNotNull(constructor.newInstance(
new ConnectConfig(),
mock(ConnectLogger.class),
new BedrockIdentityKeyProvider(new ConnectConfig(), new OkHttpClient()),
null));
}

@Test
void legacyConstructorUsesContextAdmissionRegistry() throws Exception {
KeyPair keyPair = ed25519KeyPair();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package com.minekube.connect.module;

import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.mockito.Mockito.mock;

import com.google.inject.AbstractModule;
import com.google.inject.Guice;
import com.google.inject.Injector;
import com.google.inject.Key;
import com.google.inject.name.Names;
import com.minekube.connect.api.logger.ConnectLogger;
import com.minekube.connect.bedrock.BedrockIdentityEnforcer;
import com.minekube.connect.config.ConfigHolder;
import com.minekube.connect.config.ConnectConfig;
import okhttp3.OkHttpClient;
import org.junit.jupiter.api.Test;

/**
* Regression guard for resolving the Bedrock graph before configuration is loaded.
*
* <p>The production parent injector builds the platform before {@code ConnectPlatform.init()}
* creates the config child. The graph must resolve config lazily through the parent-bound
* {@link ConfigHolder}, so the loaded {@link ConnectConfig} can be bound in the child afterward.
*/
class BedrockParentInjectorStartupTest {

/**
* Mirrors the production ordering: a parent injector (with only pre-config bindings available)
* resolves the Bedrock enforcer the way {@code SpigotPlatformModule.platformInjector(...)} does,
* and only afterwards does {@code ConnectPlatform.init()} create the config-owning child injector.
*/
@Test
void resolvingBedrockGraphDoesNotBindConfigOnParentInjector() {
Injector parent = Guice.createInjector(new AbstractModule() {
@Override
protected void configure() {
// ConfigHolder is bound on the parent and populated by init() before config is read.
bind(ConfigHolder.class).toInstance(new ConfigHolder());
bind(ConnectLogger.class).toInstance(mock(ConnectLogger.class));
bind(OkHttpClient.class)
.annotatedWith(Names.named("defaultHttpClient"))
.toInstance(new OkHttpClient());
}
});

// The platform injector resolves the Bedrock enforcer before config load.
parent.getInstance(BedrockIdentityEnforcer.class);

// Regression guard: resolving the Bedrock graph must NOT create a ConnectConfig binding on
// the parent, otherwise the child ConfigLoadedModule below fails with JitBindingAlreadySet.
assertNull(
parent.getExistingBinding(Key.get(ConnectConfig.class)),
"resolving the Bedrock graph must not establish a ConnectConfig binding on the parent injector");

// Reproduces ConnectPlatform.init(): the loaded config is bound in a child injector.
ConnectConfig loaded = new ConnectConfig();
Injector child = parent.createChildInjector(new ConfigLoadedModule(loaded));
assertSame(loaded, child.getInstance(ConnectConfig.class));
}
}
Loading