Fix Paper shared connection access before plugin enable

dataprovider-platform-paper/src/main/java/nl/hauntedmc/dataprovider/platform/bukkit/identity/BukkitCallerContextResolver.java

@@ -10,7 +10,10 @@
 import org.bukkit.plugin.Plugin;
 
 import java.util.List;
+import java.util.Locale;
 import java.util.Objects;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Supplier;
 
 /**
@@ -19,6 +22,7 @@
 public final class BukkitCallerContextResolver implements CallerContextResolver {
 
     private final PluginIdentityRegistry identities = new PluginIdentityRegistry();
+    private final Set<String> installedPluginIds = ConcurrentHashMap.newKeySet();
     private final Supplier<List<ClassLoader>> callerChain;
 
     public BukkitCallerContextResolver(ClassLoader ownClassLoader) {
@@ -58,23 +62,38 @@ public CallerContext resolveCallerIfPresent() {
 
     @Override
     public boolean isKnownPlugin(String pluginId) {
-        return identities.isKnownPlugin(pluginId);
+        if (pluginId == null || pluginId.isBlank()) {
+            return false;
+        }
+        return installedPluginIds.contains(normalizePluginId(pluginId));
     }
 
-    /** Called from Paper's lifecycle thread before APIs are handed to plugins. */
+    /**
+     * Called from Paper's lifecycle thread before APIs are handed to plugins.
+     *
+     * <p>All installed plugin names must be known here, not only plugins that have already completed
+     * {@code onEnable}. Access policies are configuration declarations and may legitimately name a
+     * plugin that enables later in Paper's dependency order. Only enabled plugins receive an active
+     * lifecycle identity; disabled plugins are known solely for configuration validation.</p>
+     */
     public void synchronizePlugins() {
-        for (Plugin plugin : Bukkit.getPluginManager().getPlugins()) {
+        synchronizePlugins(List.of(Bukkit.getPluginManager().getPlugins()));
+    }
+
+    void synchronizePlugins(Iterable<? extends Plugin> plugins) {
+        Objects.requireNonNull(plugins, "Plugins cannot be null.");
+        for (Plugin plugin : plugins) {
+            rememberInstalled(plugin);
             if (plugin.isEnabled()) {
-                register(plugin);
+                registerActiveIdentity(plugin);
             }
         }
     }
 
     public PluginIdentity register(Plugin plugin) {
         Objects.requireNonNull(plugin, "Plugin cannot be null.");
-        ClassLoader classLoader = plugin.getClass().getClassLoader();
-        PluginIdentity existing = identities.find(classLoader);
-        return existing != null ? existing : identities.register(plugin.getName(), classLoader);
+        rememberInstalled(plugin);
+        return registerActiveIdentity(plugin);
     }
 
     public void invalidate(Plugin plugin) {
@@ -89,6 +108,7 @@ public PluginIdentity find(Plugin plugin) {
 
     public void invalidateAll() {
         identities.invalidateAll();
+        installedPluginIds.clear();
     }
 
     @Override
@@ -102,7 +122,7 @@ public PluginIdentity issueIdentity(Object platformPlugin) {
         // Bukkit fires PluginEnableEvent after JavaPlugin.onEnable. Register here as
         // well so a plugin can bind the API from its own onEnable callback.
         PluginIdentity identity = register(plugin);
-        if (identity == null || !identity.pluginId().equals(plugin.getName().trim().toLowerCase(java.util.Locale.ROOT))) {
+        if (!identity.pluginId().equals(normalizePluginId(plugin.getName()))) {
             throw new SecurityException("Bukkit plugin is not active in DataProvider's identity registry.");
         }
         requireBindingCaller(identity);
@@ -114,10 +134,39 @@ public boolean isIdentityActive(PluginIdentity identity) {
         return identities.isActive(identity);
     }
 
+    private PluginIdentity registerActiveIdentity(Plugin plugin) {
+        String pluginId = normalizePluginId(plugin.getName());
+        ClassLoader classLoader = plugin.getClass().getClassLoader();
+        PluginIdentity existing = identities.find(classLoader);
+        PluginIdentity identity = existing != null ? existing : identities.register(pluginId, classLoader);
+        if (!identity.pluginId().equals(pluginId)) {
+            throw new IllegalStateException(
+                    "Cannot securely distinguish Bukkit plugins '" + identity.pluginId() + "' and '"
+                            + pluginId + "' because they share one class loader."
+            );
+        }
+        return identity;
+    }
+
+    private void rememberInstalled(Plugin plugin) {
+        Objects.requireNonNull(plugin, "Plugin cannot be null.");
+        installedPluginIds.add(normalizePluginId(plugin.getName()));
+    }
+
     private void requireBindingCaller(PluginIdentity identity) {
         CallerContext caller = resolveCaller();
         if (!identity.pluginId().equals(caller.pluginId()) || identity.classLoader() != caller.classLoader()) {
             throw new SecurityException("A Bukkit plugin can bind DataProvider only to its own plugin instance.");
         }
     }
+
+    private static String normalizePluginId(String pluginId) {
+        String normalized = Objects.requireNonNull(pluginId, "Plugin id cannot be null.")
+                .trim()
+                .toLowerCase(Locale.ROOT);
+        if (normalized.isEmpty()) {
+            throw new IllegalArgumentException("Plugin id cannot be blank.");
+        }
+        return normalized;
+    }
 }

dataprovider-platform-paper/src/test/java/nl/hauntedmc/dataprovider/platform/bukkit/identity/BukkitCallerContextResolverTest.java

@@ -7,18 +7,19 @@
 import java.util.List;
 
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.clearInvocations;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
 
 class BukkitCallerContextResolverTest {
 
     @Test
-    void issuesAndInvalidatesLifecycleIdentityWithoutBukkitAccessDuringUse() {
+    void invalidatesLifecycleIdentityButRetainsInstalledPluginKnowledge() {
         Plugin plugin = mock(Plugin.class);
         BukkitCallerContextResolver resolver = resolverFor(plugin.getClass().getClassLoader());
         when(plugin.getName()).thenReturn("Example");
@@ -34,10 +35,47 @@ void issuesAndInvalidatesLifecycleIdentityWithoutBukkitAccessDuringUse() {
         verifyNoInteractions(plugin);
 
         resolver.invalidate(plugin);
+        assertFalse(resolver.isIdentityActive(identity));
+        assertTrue(resolver.isKnownPlugin("example"));
+    }
+
+    @Test
+    void invalidateAllClearsLifecycleAndInstalledPluginKnowledge() {
+        Plugin plugin = mock(Plugin.class);
+        when(plugin.getName()).thenReturn("Example");
+        BukkitCallerContextResolver resolver = resolverFor(plugin.getClass().getClassLoader());
+        PluginIdentity identity = resolver.register(plugin);
+
+        resolver.invalidateAll();
+
         assertFalse(resolver.isIdentityActive(identity));
         assertFalse(resolver.isKnownPlugin("example"));
     }
 
+    @Test
+    void synchronizesInstalledPluginsBeforeTheyAreEnabled() {
+        Plugin dataRegistryDelegate = mock(Plugin.class);
+        Plugin serverFeaturesDelegate = mock(Plugin.class);
+        when(dataRegistryDelegate.getName()).thenReturn("DataRegistry");
+        when(serverFeaturesDelegate.getName()).thenReturn("ServerFeatures");
+        when(dataRegistryDelegate.isEnabled()).thenReturn(true);
+        when(serverFeaturesDelegate.isEnabled()).thenReturn(false);
+        ClassLoader dataRegistryLoader = new ClassLoader() {
+        };
+        ClassLoader serverFeaturesLoader = new ClassLoader() {
+        };
+        Plugin dataRegistry = pluginWithLoader(dataRegistryDelegate, dataRegistryLoader);
+        Plugin serverFeatures = pluginWithLoader(serverFeaturesDelegate, serverFeaturesLoader);
+
+        BukkitCallerContextResolver resolver = resolverFor(dataRegistryLoader);
+        resolver.synchronizePlugins(List.of(dataRegistry, serverFeatures));
+
+        assertTrue(resolver.isKnownPlugin("dataregistry"));
+        assertTrue(resolver.isKnownPlugin("serverfeatures"));
+        assertNull(resolver.find(serverFeatures));
+        assertThrows(SecurityException.class, () -> resolver.issueIdentity(serverFeatures));
+    }
+
     @Test
     void issuesAnIdentityDuringPluginEnableBeforeTheLifecycleEventIsFired() {
         Plugin plugin = mock(Plugin.class);