More scoping coverage

Signed-off-by: Peter Nied <[email protected]>
stephen-crawford · May 15, 2023 · 1baee4f · 1baee4f
1 parent 80bfeb5
commit 1baee4f
Show file tree

Hide file tree

Showing 11 changed files with 106 additions and 49 deletions.
diff --git a/plugins/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroSubject.java b/plugins/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroSubject.java
@@ -88,4 +88,15 @@ public void authenticate(AuthToken authenticationToken) {
             .orElseThrow(() -> new UnsupportedAuthenticationToken());
         shiroSubject.login(authToken);
     }
+
+    @Override
+    public Principal getAssociatedApplication() {
+        return null;
+    }
+
+    @Override
+    public boolean isAllowed(final List<Scope> scope) {
+        // Noop subject is always allowed for any scope checks
+        return true;
+    }
 }
diff --git a/server/src/main/java/org/opensearch/action/ActionScopes.java b/server/src/main/java/org/opensearch/action/ActionScopes.java
@@ -16,6 +16,7 @@
  */
 public enum ActionScopes implements Scope {
     Index_Read(),
+    // TODO: Write REST / Transport actions have not been covered
     Index_ReadWrite();
 
     public String getNamespace() {

diff --git a/server/src/main/java/org/opensearch/client/node/NodeClient.java b/server/src/main/java/org/opensearch/client/node/NodeClient.java
@@ -51,6 +51,7 @@
 import org.opensearch.transport.RemoteClusterService;
 
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
 
 import javax.management.InvalidApplicationException;
 
@@ -142,7 +143,7 @@ private <Request extends ActionRequest, Response extends ActionResponse> Transpo
         ActionType<Response> action
     ) {
         if (!IdentityService.getInstance().getSubject().isAllowed(action.allowedScopes())) {
-            final String scopeList = action.allowedScopes().stream().map(s -> s.toString()).collect(Collectors.joining(",");
+            final String scopeList = action.allowedScopes().stream().map(s -> s.toString()).collect(Collectors.joining(","));
             logger.debug("Request did not have any of the required scopes, " + scopeList);
             throw new OpenSearchSecurityException("Unauthorized, at least of these scopes is required, " + scopeList);
         }

diff --git a/server/src/main/java/org/opensearch/identity/Scope.java b/server/src/main/java/org/opensearch/identity/Scope.java
@@ -9,7 +9,7 @@
 package org.opensearch.identity;
 
 /**
- * Any kind of limitation for an extension
+ * Limitation for the scope of an application in OpenSearch
  *
  * @opensearch.experimental
  */
@@ -20,9 +20,5 @@ public interface Scope {
     default String asPermissionString() {
         return getNamespace() + "." + getArea() + "." + getAction();
     }
-
-    default String toString() {
-        return asPermissionString();
-    }
 }
 
diff --git a/server/src/main/java/org/opensearch/identity/Subject.java b/server/src/main/java/org/opensearch/identity/Subject.java
@@ -5,12 +5,13 @@
 
 package org.opensearch.identity;
 
-import org.opensearch.identity.Scope;
 import org.opensearch.identity.Scope;
 import org.opensearch.identity.tokens.AuthToken;
 
 import java.security.Principal;
 import java.util.List;
+import org.opensearch.common.Nullable;
+
 /**
  * An individual, process, or device that causes information to flow among objects or change to the system state.
  *
@@ -32,7 +33,18 @@ public interface Subject {
      */
     void authenticate(final AuthToken token);
 
-    default boolean isAllowed(final List<Scope> scope) {
-        return false;
-    }
+
+    /**
+     * Get the application-wide uniquely identifying associated application's principal
+     * @return Null, if there is not associated application
+     */
+    @Nullable
+    Principal getAssociatedApplication();
+
+    /**
+     * Checks scopes of the current subject if they are allowed for any of the listed scopes
+     * @param scope The scopes to check against the subject
+     * @return true if allowed, false if none of the scopes are allowed.
+     */
+    boolean isAllowed(final List<Scope> scope);
 }
diff --git a/server/src/main/java/org/opensearch/identity/noop/NoopSubject.java b/server/src/main/java/org/opensearch/identity/noop/NoopSubject.java
@@ -9,9 +9,11 @@
 package org.opensearch.identity.noop;
 
 import java.security.Principal;
+import java.util.List;
 import java.util.Objects;
 
 import org.opensearch.identity.NamedPrincipal;
+import org.opensearch.identity.Scope;
 import org.opensearch.identity.tokens.AuthToken;
 import org.opensearch.identity.Subject;
 
@@ -54,4 +56,16 @@ public String toString() {
     public void authenticate(AuthToken AuthToken) {
         // Do nothing as noop subject is always logged in
     }
+
+    @Override
+    public Principal getAssociatedApplication() {
+        // Noop subject is not aware of an application
+        return null;
+    }
+
+    @Override
+    public boolean isAllowed(final List<Scope> scope) {
+        // Noop subject is always allowed for any scope checks
+        return true;
+    }
 }
diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java
@@ -188,6 +188,7 @@
 import org.opensearch.plugins.Plugin;
 import org.opensearch.plugins.PluginsService;
 import org.opensearch.plugins.RepositoryPlugin;
+import org.opensearch.plugins.ScopeProtectedActionPlugin;
 import org.opensearch.plugins.ScriptPlugin;
 import org.opensearch.plugins.SearchPlugin;
 import org.opensearch.plugins.SystemIndexPlugin;
@@ -785,7 +786,7 @@ protected Node(
             final List<ActionPlugin> scopeProtectedActionPlugin = pluginsService
                 .filterPlugins(ActionPlugin.class)
                 .stream()
-                .map(plugin -> ScopeProtectedActionPlugin(plugin, identityService))
+                .map(plugin -> new ScopeProtectedActionPlugin(plugin, identityService))
                 .collect(Collectors.toList());
 
             ActionModule actionModule = new ActionModule(

diff --git a/server/src/main/java/org/opensearch/plugins/ExtensionPointScopes.java b/server/src/main/java/org/opensearch/plugins/ExtensionPointScopes.java
@@ -8,6 +8,7 @@
 
 package org.opensearch.plugins;
 
+import org.opensearch.OpenSearchException;
 import org.opensearch.identity.Scope;
 
 /**
@@ -16,7 +17,8 @@
  * @opensearch.experimental
  */
 public enum ExtensionPointScopes implements Scope  {
-    ActionPlugin,
+    Action,
+    // TODO: implement checks for all other scopes
     Analysis,
     CircuitBreaker,
     Cluster,
@@ -47,10 +49,15 @@ public String getArea() {
     public String getAction() {
         return "Allowed";
     }
-}
 
-public static class ExtensionPointScopeException extends OpenSearchException {
-    public ExtensionPointScopeException(final ExtensionPointScopes missingScope) {
-        super("Missing scope for this extension point " + missingScope.toString());
+    /**
+     * Exception raised when an ExtensionPointScopes is missing
+     *
+     * @opensearch.experimental
+     */
+    public static class ExtensionPointScopeException extends OpenSearchException {
+        public ExtensionPointScopeException(final ExtensionPointScopes missingScope) {
+            super("Missing scope for this extension point " + missingScope.asPermissionString());
+        }
     }
 }
diff --git a/server/src/main/java/org/opensearch/plugins/wrappers/ScopeProtectedActionPlugin.java b/server/src/main/java/org/opensearch/plugins/wrappers/ScopeProtectedActionPlugin.java
@@ -32,35 +32,37 @@
 
  package org.opensearch.plugins;
 
- import org.opensearch.action.ActionType;
- import org.opensearch.action.ActionRequest;
- import org.opensearch.action.ActionResponse;
- import org.opensearch.action.RequestValidators;
- import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest;
- import org.opensearch.action.admin.indices.mapping.put.PutMappingRequest;
- import org.opensearch.action.support.ActionFilter;
- import org.opensearch.action.support.TransportAction;
- import org.opensearch.action.support.TransportActions;
- import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
- import org.opensearch.cluster.node.DiscoveryNodes;
- import org.opensearch.common.Strings;
- import org.opensearch.common.settings.ClusterSettings;
- import org.opensearch.common.settings.IndexScopedSettings;
- import org.opensearch.common.settings.Settings;
- import org.opensearch.common.settings.SettingsFilter;
- import org.opensearch.common.util.concurrent.ThreadContext;
+import org.opensearch.action.ActionType;
+import org.opensearch.action.ActionRequest;
+import org.opensearch.action.ActionResponse;
+import org.opensearch.action.RequestValidators;
+import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest;
+import org.opensearch.action.admin.indices.mapping.put.PutMappingRequest;
+import org.opensearch.action.support.ActionFilter;
+import org.opensearch.action.support.TransportAction;
+import org.opensearch.action.support.TransportActions;
+import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
+import org.opensearch.cluster.node.DiscoveryNodes;
+import org.opensearch.common.Strings;
+import org.opensearch.common.settings.ClusterSettings;
+import org.opensearch.common.settings.IndexScopedSettings;
+import org.opensearch.common.settings.Settings;
+import org.opensearch.common.settings.SettingsFilter;
+import org.opensearch.common.util.concurrent.ThreadContext;
+import org.opensearch.identity.IdentityService;
 import org.opensearch.plugins.ActionPlugin;
+import org.opensearch.plugins.ExtensionPointScopes;
+import org.opensearch.rest.RestHandler;
+import org.opensearch.rest.RestHeaderDefinition;
 import org.opensearch.rest.RestController;
- import org.opensearch.rest.RestHandler;
- import org.opensearch.rest.RestHeaderDefinition;
-
- import java.util.Collection;
- import java.util.Collections;
- import java.util.List;
- import java.util.Objects;
- import java.util.function.Supplier;
- import java.util.function.UnaryOperator;
- import java.util.stream.Collectors;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import java.util.function.Supplier;
+import java.util.function.UnaryOperator;
+import java.util.stream.Collectors;
 
 /**
  * Only allowed plugins are able able to response
@@ -76,9 +78,9 @@ public ScopeProtectedActionPlugin(final ActionPlugin plugin, final IdentityServi
         this.identity = identity;
     }
 
-    private throwIfNotAllowed() {
-        if (!identity.getInstance().getSubject().isAllowed(List.of(ExtensionPointScopes.ActionPlugin))) {
-            throw new ExtensionPointScopeException(ExtensionPointScopes.ActionPlugin);
+    private void throwIfNotAllowed() {
+        if (!identity.getSubject().isAllowed(List.of(ExtensionPointScopes.Action))) {
+            throw new ExtensionPointScopes.ExtensionPointScopeException(ExtensionPointScopes.Action);
         }
     }
 
@@ -94,7 +96,7 @@ public List<ActionType<? extends ActionResponse>> getClientActions() {
 
     public List<ActionFilter> getActionFilters() {
         throwIfNotAllowed();
-        return plugin.getClientActions();
+        return plugin.getActionFilters();
     }
 
     public List<RestHandler> getRestHandlers(
@@ -117,7 +119,7 @@ public Collection<RestHeaderDefinition> getRestHeaders() {
 
     public Collection<String> getTaskHeaders() {
         throwIfNotAllowed();
-        return plugin.getRestHeaders();
+        return plugin.getTaskHeaders();
     }
 
     public UnaryOperator<RestHandler> getRestHandlerWrapper(final ThreadContext threadContext) {

diff --git a/server/src/main/java/org/opensearch/plugins/wrappers/package-info.java b/server/src/main/java/org/opensearch/plugins/wrappers/package-info.java
@@ -0,0 +1,12 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+/**
+ * Wrappers for plugin interfaces
+ * */
+package org.opensearch.plugins.wrappers;
diff --git a/server/src/main/java/org/opensearch/rest/BaseRestHandler.java b/server/src/main/java/org/opensearch/rest/BaseRestHandler.java
@@ -104,7 +104,7 @@ public final long getUsageCount() {
     public final void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
         final IdentityService identityService = IdentityService.getInstance();
         if (!identityService.getSubject().isAllowed(allowedScopes())) {
-            final String scopeList = allowedScopes().stream().map(s -> s.toString()).collect(Collectors.joining(",");
+            final String scopeList = allowedScopes().stream().map(s -> s.toString()).collect(Collectors.joining(","));
             logger.debug("Request did not have any of the required scopes, " + scopeList);
             throw new IllegalArgumentException("Unauthorized, at least of these scopes is required, " + scopeList);
         }