Initial PR feedback

Signed-off-by: Peter Nied <petern@amazon.com>
opensearch-project · Jan 11, 2023 · ba6f5cb · ba6f5cb
1 parent c42fbe1
commit ba6f5cb
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 4 deletions.
diff --git a/sandbox/libs/authn/src/main/java/org/opensearch/authn/Permission.java b/sandbox/libs/authn/src/main/java/org/opensearch/authn/Permission.java
@@ -9,10 +9,15 @@
 import java.util.Objects;
 
 public class Permission {
+    private final static String PERMISSION_DELIMITER = "\\.";  
     private final String[] permissionChunks; 
 
     public Permission(final String permission) {
-        this.permissionChunks = permission.split("\\.");
+        try {
+            this.permissionChunks = permission.split(PERMISSION_DELIMITER);
+        } catch (Exception) {
+            throw new InvalidPermissionName(permission);
+        }
     }
 
     public boolean matches(final String permissionRequired) {
@@ -26,4 +31,14 @@ public boolean matches(final String permissionRequired) {
         }
         return true;
     }
+
+    public static void checkIsValid(final String permission) {
+        new Permission(permission);
+    }
+
+    public static class InvalidPermissionName extends RuntimeException {
+        public InvalidPermissionName(final String name) {
+            super("The name '" + name + "' is not a valid permission name");
+        }
+    }
 }
diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java
@@ -272,6 +272,7 @@
 import org.opensearch.action.termvectors.TransportTermVectorsAction;
 import org.opensearch.action.update.TransportUpdateAction;
 import org.opensearch.action.update.UpdateAction;
+import org.opensearch.authn.Permission;
 import org.opensearch.client.node.NodeClient;
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
 import org.opensearch.cluster.node.DiscoveryNodes;
@@ -441,6 +442,7 @@
 import org.opensearch.usage.UsageService;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -540,6 +542,12 @@ class ActionRegistry extends NamedRegistry<ActionHandler<?, ?>> {
             }
 
             public void register(ActionHandler<?, ?> handler) {
+                // Ensure all permissions are valid
+                handler.getTransportAction().requiredPermissions()
+                    .stream()
+                    .flatMap(Collection::stream)
+                    .forEach(Permission::checkIsValid);
+                // TODO: AuthenticationManager gets a list of permission for permission validity / error messaging
                 register(handler.getAction().name(), handler);
             }
 

diff --git a/server/src/main/java/org/opensearch/action/ActionRequest.java b/server/src/main/java/org/opensearch/action/ActionRequest.java
@@ -34,7 +34,6 @@
 
 import org.opensearch.common.io.stream.StreamInput;
 import org.opensearch.common.io.stream.StreamOutput;
-import org.opensearch.authn.Permission;
 import org.opensearch.transport.TransportRequest;
 
 import java.io.IOException;

diff --git a/server/src/main/java/org/opensearch/action/ActionType.java b/server/src/main/java/org/opensearch/action/ActionType.java
@@ -32,8 +32,6 @@
 
 package org.opensearch.action;
 
-import java.util.List;
-
 import org.opensearch.common.io.stream.Writeable;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.transport.TransportRequestOptions;