Add getPluginSubject(plugin) to IdentityPlugin

Signed-off-by: Craig Perkins <[email protected]>
cwperks · Aug 16, 2024 · ca1f297 · ca1f297
1 parent be9a89a
commit ca1f297
Show file tree

Hide file tree

Showing 22 changed files with 135 additions and 69 deletions.
diff --git a/plugins/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroIdentityPlugin.java b/plugins/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroIdentityPlugin.java
@@ -12,16 +12,27 @@
 import org.apache.logging.log4j.Logger;
 import org.apache.shiro.SecurityUtils;
 import org.apache.shiro.mgt.SecurityManager;
+import org.opensearch.client.Client;
+import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
+import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.settings.Settings;
+import org.opensearch.core.common.io.stream.NamedWriteableRegistry;
+import org.opensearch.core.xcontent.NamedXContentRegistry;
+import org.opensearch.env.Environment;
+import org.opensearch.env.NodeEnvironment;
 import org.opensearch.identity.Subject;
 import org.opensearch.identity.noop.NoopPluginSubject;
 import org.opensearch.identity.tokens.TokenManager;
-import org.opensearch.plugins.IdentityAwarePlugin;
 import org.opensearch.plugins.IdentityPlugin;
 import org.opensearch.plugins.Plugin;
+import org.opensearch.repositories.RepositoriesService;
+import org.opensearch.script.ScriptService;
 import org.opensearch.threadpool.ThreadPool;
+import org.opensearch.watcher.ResourceWatcherService;
 
-import java.util.List;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.function.Supplier;
 
 /**
  * Identity implementation with Shiro
@@ -34,6 +45,8 @@ public final class ShiroIdentityPlugin extends Plugin implements IdentityPlugin
     private final Settings settings;
     private final ShiroTokenManager authTokenHandler;
 
+    private ThreadPool threadPool;
+
     /**
      * Create a new instance of the Shiro Identity Plugin
      *
@@ -47,6 +60,24 @@ public ShiroIdentityPlugin(final Settings settings) {
         SecurityUtils.setSecurityManager(securityManager);
     }
 
+    @Override
+    public Collection<Object> createComponents(
+        Client client,
+        ClusterService clusterService,
+        ThreadPool threadPool,
+        ResourceWatcherService resourceWatcherService,
+        ScriptService scriptService,
+        NamedXContentRegistry xContentRegistry,
+        Environment environment,
+        NodeEnvironment nodeEnvironment,
+        NamedWriteableRegistry namedWriteableRegistry,
+        IndexNameExpressionResolver expressionResolver,
+        Supplier<RepositoriesService> repositoriesServiceSupplier
+    ) {
+        this.threadPool = threadPool;
+        return Collections.emptyList();
+    }
+
     /**
      * Return a Shiro Subject based on the provided authTokenHandler and current subject
      *
@@ -68,12 +99,7 @@ public TokenManager getTokenManager() {
     }
 
     @Override
-    public void initializeIdentityAwarePlugins(List<IdentityAwarePlugin> identityAwarePlugins, ThreadPool threadPool) {
-        if (identityAwarePlugins != null) {
-            for (IdentityAwarePlugin plugin : identityAwarePlugins) {
-                Subject subject = new NoopPluginSubject(threadPool);
-                plugin.assignSubject(subject);
-            }
-        }
+    public Subject getPluginSubject(Plugin plugin) {
+        return new NoopPluginSubject(threadPool);
     }
 }
diff --git a/.../identity-shiro/src/test/java/org/opensearch/identity/shiro/ShiroIdentityPluginTests.java b/.../identity-shiro/src/test/java/org/opensearch/identity/shiro/ShiroIdentityPluginTests.java
@@ -13,6 +13,7 @@
 import org.opensearch.identity.IdentityService;
 import org.opensearch.plugins.IdentityPlugin;
 import org.opensearch.test.OpenSearchTestCase;
+import org.opensearch.threadpool.TestThreadPool;
 
 import java.util.List;
 
@@ -24,18 +25,20 @@
 public class ShiroIdentityPluginTests extends OpenSearchTestCase {
 
     public void testSingleIdentityPluginSucceeds() {
+        TestThreadPool threadPool = new TestThreadPool(getTestName());
         IdentityPlugin identityPlugin1 = new ShiroIdentityPlugin(Settings.EMPTY);
         List<IdentityPlugin> pluginList1 = List.of(identityPlugin1);
-        IdentityService identityService1 = new IdentityService(Settings.EMPTY, pluginList1);
+        IdentityService identityService1 = new IdentityService(Settings.EMPTY, threadPool, pluginList1);
         assertThat(identityService1.getTokenManager(), is(instanceOf(ShiroTokenManager.class)));
     }
 
     public void testMultipleIdentityPluginsFail() {
+        TestThreadPool threadPool = new TestThreadPool(getTestName());
         IdentityPlugin identityPlugin1 = new ShiroIdentityPlugin(Settings.EMPTY);
         IdentityPlugin identityPlugin2 = new ShiroIdentityPlugin(Settings.EMPTY);
         IdentityPlugin identityPlugin3 = new ShiroIdentityPlugin(Settings.EMPTY);
         List<IdentityPlugin> pluginList = List.of(identityPlugin1, identityPlugin2, identityPlugin3);
-        Exception ex = assertThrows(OpenSearchException.class, () -> new IdentityService(Settings.EMPTY, pluginList));
+        Exception ex = assertThrows(OpenSearchException.class, () -> new IdentityService(Settings.EMPTY, threadPool, pluginList));
         assert (ex.getMessage().contains("Multiple identity plugins are not supported,"));
     }
 

diff --git a/server/src/main/java/org/opensearch/extensions/NoopExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/NoopExtensionsManager.java
@@ -20,7 +20,6 @@
 import org.opensearch.transport.TransportService;
 
 import java.io.IOException;
-import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 
@@ -31,8 +30,8 @@
  */
 public class NoopExtensionsManager extends ExtensionsManager {
 
-    public NoopExtensionsManager() throws IOException {
-        super(Set.of(), new IdentityService(Settings.EMPTY, List.of()));
+    public NoopExtensionsManager(IdentityService identityService) throws IOException {
+        super(Set.of(), identityService);
     }
 
     @Override

diff --git a/server/src/main/java/org/opensearch/identity/IdentityService.java b/server/src/main/java/org/opensearch/identity/IdentityService.java
@@ -13,6 +13,7 @@
 import org.opensearch.identity.tokens.TokenManager;
 import org.opensearch.plugins.IdentityAwarePlugin;
 import org.opensearch.plugins.IdentityPlugin;
+import org.opensearch.plugins.Plugin;
 import org.opensearch.threadpool.ThreadPool;
 
 import java.util.List;
@@ -29,12 +30,12 @@ public class IdentityService {
     private final Settings settings;
     private final IdentityPlugin identityPlugin;
 
-    public IdentityService(final Settings settings, final List<IdentityPlugin> identityPlugins) {
+    public IdentityService(final Settings settings, final ThreadPool threadPool, final List<IdentityPlugin> identityPlugins) {
         this.settings = settings;
 
         if (identityPlugins.size() == 0) {
             log.debug("Identity plugins size is 0");
-            identityPlugin = new NoopIdentityPlugin();
+            identityPlugin = new NoopIdentityPlugin(threadPool);
         } else if (identityPlugins.size() == 1) {
             log.debug("Identity plugins size is 1");
             identityPlugin = identityPlugins.get(0);
@@ -60,7 +61,12 @@ public TokenManager getTokenManager() {
         return identityPlugin.getTokenManager();
     }
 
-    public void initializeIdentityAwarePlugins(final List<IdentityAwarePlugin> identityAwarePlugins, ThreadPool threadPool) {
-        identityPlugin.initializeIdentityAwarePlugins(identityAwarePlugins, threadPool);
+    public void initializeIdentityAwarePlugins(final List<IdentityAwarePlugin> identityAwarePlugins) {
+        if (identityAwarePlugins != null) {
+            for (IdentityAwarePlugin plugin : identityAwarePlugins) {
+                Subject subject = identityPlugin.getPluginSubject((Plugin) plugin);
+                plugin.assignSubject(subject);
+            }
+        }
     }
 }
diff --git a/server/src/main/java/org/opensearch/identity/Subject.java b/server/src/main/java/org/opensearch/identity/Subject.java
@@ -5,6 +5,7 @@
 
 package org.opensearch.identity;
 
+import org.opensearch.common.annotation.ExperimentalApi;
 import org.opensearch.identity.tokens.AuthToken;
 
 import java.security.Principal;
@@ -15,6 +16,7 @@
  *
  * @opensearch.experimental
  */
+@ExperimentalApi
 public interface Subject {
 
     /**

diff --git a/server/src/main/java/org/opensearch/identity/noop/NoopIdentityPlugin.java b/server/src/main/java/org/opensearch/identity/noop/NoopIdentityPlugin.java
@@ -10,12 +10,10 @@
 
 import org.opensearch.identity.Subject;
 import org.opensearch.identity.tokens.TokenManager;
-import org.opensearch.plugins.IdentityAwarePlugin;
 import org.opensearch.plugins.IdentityPlugin;
+import org.opensearch.plugins.Plugin;
 import org.opensearch.threadpool.ThreadPool;
 
-import java.util.List;
-
 /**
  * Implementation of identity plugin that does not enforce authentication or authorization
  * <p>
@@ -25,6 +23,12 @@
  */
 public class NoopIdentityPlugin implements IdentityPlugin {
 
+    private final ThreadPool threadPool;
+
+    public NoopIdentityPlugin(ThreadPool threadPool) {
+        this.threadPool = threadPool;
+    }
+
     /**
      * Get the current subject
      * @return Must never return null
@@ -44,12 +48,7 @@ public TokenManager getTokenManager() {
     }
 
     @Override
-    public void initializeIdentityAwarePlugins(List<IdentityAwarePlugin> identityAwarePlugins, ThreadPool threadPool) {
-        if (identityAwarePlugins != null) {
-            for (IdentityAwarePlugin plugin : identityAwarePlugins) {
-                Subject subject = new NoopPluginSubject(threadPool);
-                plugin.assignSubject(subject);
-            }
-        }
+    public Subject getPluginSubject(Plugin plugin) {
+        return new NoopPluginSubject(threadPool);
     }
 }
diff --git a/server/src/main/java/org/opensearch/identity/tokens/AuthToken.java b/server/src/main/java/org/opensearch/identity/tokens/AuthToken.java
@@ -8,11 +8,14 @@
 
 package org.opensearch.identity.tokens;
 
+import org.opensearch.common.annotation.ExperimentalApi;
+
 /**
  * Interface for all token formats to support to authenticate user such as UserName/Password tokens, Access tokens, and more.
  *
  * @opensearch.experimental
  */
+@ExperimentalApi
 public interface AuthToken {
 
     String asAuthHeaderValue();

diff --git a/server/src/main/java/org/opensearch/identity/tokens/OnBehalfOfClaims.java b/server/src/main/java/org/opensearch/identity/tokens/OnBehalfOfClaims.java
@@ -8,9 +8,14 @@
 
 package org.opensearch.identity.tokens;
 
+import org.opensearch.common.annotation.ExperimentalApi;
+
 /**
  * This class represents the claims of an OnBehalfOf token.
+ *
+ * @opensearch.experimental
  */
+@ExperimentalApi
 public class OnBehalfOfClaims {
 
     private final String audience;

diff --git a/server/src/main/java/org/opensearch/identity/tokens/TokenManager.java b/server/src/main/java/org/opensearch/identity/tokens/TokenManager.java
@@ -8,11 +8,15 @@
 
 package org.opensearch.identity.tokens;
 
+import org.opensearch.common.annotation.ExperimentalApi;
 import org.opensearch.identity.Subject;
 
 /**
  * This interface defines the expected methods of a token manager
+ *
+ * @opensearch.experimental
  */
+@ExperimentalApi
 public interface TokenManager {
 
     /**

diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java
@@ -526,19 +526,6 @@ protected Node(
                 identityPlugins.addAll(pluginsService.filterPlugins(IdentityPlugin.class));
             }
 
-            final IdentityService identityService = new IdentityService(settings, identityPlugins);
-
-            if (FeatureFlags.isEnabled(FeatureFlags.EXTENSIONS)) {
-                final List<ExtensionAwarePlugin> extensionAwarePlugins = pluginsService.filterPlugins(ExtensionAwarePlugin.class);
-                Set<Setting<?>> additionalSettings = new HashSet<>();
-                for (ExtensionAwarePlugin extAwarePlugin : extensionAwarePlugins) {
-                    additionalSettings.addAll(extAwarePlugin.getExtensionSettings());
-                }
-                this.extensionsManager = new ExtensionsManager(additionalSettings, identityService);
-            } else {
-                this.extensionsManager = new NoopExtensionsManager();
-            }
-
             final Set<DiscoveryNodeRole> additionalRoles = pluginsService.filterPlugins(Plugin.class)
                 .stream()
                 .map(Plugin::getRoles)
@@ -576,6 +563,19 @@ protected Node(
             runnableTaskListener = new AtomicReference<>();
             final ThreadPool threadPool = new ThreadPool(settings, runnableTaskListener, executorBuilders.toArray(new ExecutorBuilder[0]));
 
+            final IdentityService identityService = new IdentityService(settings, threadPool, identityPlugins);
+
+            if (FeatureFlags.isEnabled(FeatureFlags.EXTENSIONS)) {
+                final List<ExtensionAwarePlugin> extensionAwarePlugins = pluginsService.filterPlugins(ExtensionAwarePlugin.class);
+                Set<Setting<?>> additionalSettings = new HashSet<>();
+                for (ExtensionAwarePlugin extAwarePlugin : extensionAwarePlugins) {
+                    additionalSettings.addAll(extAwarePlugin.getExtensionSettings());
+                }
+                this.extensionsManager = new ExtensionsManager(additionalSettings, identityService);
+            } else {
+                this.extensionsManager = new NoopExtensionsManager(identityService);
+            }
+
             final SetOnce<RepositoriesService> repositoriesServiceReference = new SetOnce<>();
             final RemoteStoreNodeService remoteStoreNodeService = new RemoteStoreNodeService(repositoriesServiceReference::get, threadPool);
             localNodeFactory = new LocalNodeFactory(settings, nodeEnvironment.nodeId(), remoteStoreNodeService);
@@ -714,7 +714,7 @@ protected Node(
                 rerouteServiceReference::get
             );
             List<IdentityAwarePlugin> identityAwarePlugins = pluginsService.filterPlugins(IdentityAwarePlugin.class);
-            identityService.initializeIdentityAwarePlugins(identityAwarePlugins, threadPool);
+            identityService.initializeIdentityAwarePlugins(identityAwarePlugins);
             final Map<String, Collection<SystemIndexDescriptor>> systemIndexDescriptorMap = Collections.unmodifiableMap(
                 pluginsService.filterPlugins(SystemIndexPlugin.class)
                     .stream()

diff --git a/server/src/main/java/org/opensearch/plugins/IdentityPlugin.java b/server/src/main/java/org/opensearch/plugins/IdentityPlugin.java
@@ -8,17 +8,16 @@
 
 package org.opensearch.plugins;
 
+import org.opensearch.common.annotation.ExperimentalApi;
 import org.opensearch.identity.Subject;
 import org.opensearch.identity.tokens.TokenManager;
-import org.opensearch.threadpool.ThreadPool;
-
-import java.util.List;
 
 /**
  * Plugin that provides identity and access control for OpenSearch
  *
  * @opensearch.experimental
  */
+@ExperimentalApi
 public interface IdentityPlugin {
 
     /**
@@ -33,5 +32,12 @@ public interface IdentityPlugin {
      */
     TokenManager getTokenManager();
 
-    void initializeIdentityAwarePlugins(final List<IdentityAwarePlugin> systemIndexPlugins, ThreadPool threadPool);
+    /**
+     * Gets a subject corresponding to the passed plugin that can be utilized to perform transport actions
+     * in the plugin system context
+     *
+     * @param plugin The corresponding plugin
+     * @return Subject corresponding to the plugin
+     */
+    Subject getPluginSubject(Plugin plugin);
 }
diff --git a/server/src/test/java/org/opensearch/action/ActionModuleTests.java b/server/src/test/java/org/opensearch/action/ActionModuleTests.java
@@ -130,6 +130,7 @@ protected FakeAction() {
     public void testSetupRestHandlerContainsKnownBuiltin() throws IOException {
         SettingsModule settings = new SettingsModule(Settings.EMPTY);
         UsageService usageService = new UsageService();
+        ThreadPool threadPool = new TestThreadPool(getTestName());
         ActionModule actionModule = new ActionModule(
             settings.getSettings(),
             new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)),
@@ -142,8 +143,8 @@ public void testSetupRestHandlerContainsKnownBuiltin() throws IOException {
             null,
             usageService,
             null,
-            new IdentityService(Settings.EMPTY, new ArrayList<>()),
-            new ExtensionsManager(Set.of(), new IdentityService(Settings.EMPTY, List.of()))
+            new IdentityService(Settings.EMPTY, threadPool, new ArrayList<>()),
+            new ExtensionsManager(Set.of(), new IdentityService(Settings.EMPTY, threadPool, List.of()))
         );
         actionModule.initRestHandlers(null);
         // At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail