Swap from static to isntance based Handler and make Interface

Signed-off-by: Stephen Crawford <[email protected]>

sandbox/libs/authn/src/main/java/org/opensearch/authn/Permission.java

@@ -23,16 +23,9 @@ public class Permission {
     public final String permissionString;
 
     public Permission(final String permission) {
-        try {
-            this.permissionString = permission;
-            this.permissionChunks = permission.split(PERMISSION_DELIMITER);
-            if (!permissionIsValidFormat()) {
-                throw new Exception();
-            }
-            ;
-        } catch (final Exception e) {
-            throw new InvalidPermissionName(permission);
-        }
+
+        this.permissionString = permission;
+        this.permissionChunks = permission.split(PERMISSION_DELIMITER);
     }
 
     /**

sandbox/libs/authn/src/main/java/org/opensearch/authn/PermissionHandler.java

@@ -14,12 +14,21 @@
 import java.util.HashMap;
 import java.util.Objects;
 
-public class PermissionHandler {
+public class PermissionHandler implements PermissionInterface {
+
+    InternalSubject subject;
+    HashMap<String, ArrayList<Permission>> permissionStore;
+
+    public PermissionHandler(InternalSubject subject) {
+        this.subject = subject;
+        this.permissionStore = new HashMap<String, ArrayList<Permission>>();
+
+    }
 
     /**
      * Check that the permission required matches the permission available
      */
-    public static boolean matchPermission(final Permission permission, final String permissionRequired) {
+    public boolean matchPermission(final Permission permission, final String permissionRequired) {
 
         Objects.nonNull(permissionRequired);
 
@@ -32,29 +41,29 @@ public static boolean matchPermission(final Permission permission, final String
         return true;
     }
 
-    public static void grantPermission(final Permission permission, final String resource, final InternalSubject subject) {
+    public void grantPermission(final Permission permission, final String resource, final InternalSubject subject) {
 
         // Principal represents the user or plugin which requires the permission on the target resource
-        subject.grantedPermissions.putIfAbsent(resource, new ArrayList<Permission>());
-        subject.grantedPermissions.get(resource).add(permission);
+        this.permissionStore.putIfAbsent(resource, new ArrayList<Permission>());
+        this.permissionStore.get(resource).add(permission);
     }
 
-    public static HashMap<String, ArrayList<Permission>> getPermissions(InternalSubject subject) {
+    public HashMap<String, ArrayList<Permission>> getPermissions(InternalSubject subject) {
 
-        return subject.grantedPermissions;
+        return this.permissionStore;
     }
 
-    public static boolean checkPermission(final Permission permission, final String resource, final InternalSubject subject) {
+    public boolean checkPermission(final Permission permission, final String resource, final InternalSubject subject) {
 
-        if (subject.grantedPermissions.get(resource).contains(permission)) {
+        if (this.permissionStore.get(resource).contains(permission)) {
             return true;
         }
 
         return false;
     }
 
-    public static void deletePermissions(final Permission permission, final String resource, final InternalSubject subject) {
+    public void deletePermissions(final Permission permission, final String resource, final InternalSubject subject) {
 
-        subject.grantedPermissions.get(resource).remove(permission);
+        this.permissionStore.get(resource).remove(permission);
     }
 }

sandbox/libs/authn/src/main/java/org/opensearch/authn/PermissionInterface.java

@@ -0,0 +1,21 @@
+/*
+ * 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.
+ */
+
+package org.opensearch.authn;
+
+import org.opensearch.authn.internal.InternalSubject;
+
+public interface PermissionInterface {
+
+    void grantPermission(final Permission permission, final String resource, final InternalSubject subject);
+
+    boolean checkPermission(final Permission permission, final String resource, final InternalSubject subject);
+
+    void deletePermissions(final Permission permission, final String resource, final InternalSubject subject);
+
+}

sandbox/libs/authn/src/main/java/org/opensearch/authn/internal/InternalSubject.java

@@ -6,12 +6,10 @@
 package org.opensearch.authn.internal;
 
 import java.security.Principal;
-import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.Objects;
 
 import org.opensearch.authn.AuthenticationTokenHandler;
-import org.opensearch.authn.Permission;
+import org.opensearch.authn.PermissionHandler;
 import org.opensearch.authn.tokens.AuthenticationToken;
 import org.opensearch.authn.Subject;
 
@@ -25,12 +23,12 @@
 public class InternalSubject implements Subject {
     private final org.apache.shiro.subject.Subject shiroSubject;
 
-    public HashMap<String, ArrayList<Permission>> grantedPermissions; // Not sure how we plan to store the permissions
+    public PermissionHandler permissionHandler; // Not sure how we plan to store the permissions
 
     public InternalSubject(org.apache.shiro.subject.Subject subject) {
 
         shiroSubject = subject;
-        this.grantedPermissions = new HashMap<String, ArrayList<Permission>>();
+        this.permissionHandler = new PermissionHandler(this);
     }
 
     @Override

sandbox/libs/authn/src/test/java/org/opensearch/authn/PermissionHandlerTests.java

@@ -25,23 +25,64 @@ public class PermissionHandlerTests extends OpenSearchIntegTestCase {
     public void testInternalSubjectPermissionGrant() {
 
         InternalSubject testInternalSubject = new InternalSubject(SecurityUtils.getSubject());
-        PermissionHandler.grantPermission(new Permission("opensearch.indexing.index.create"), "cluster", testInternalSubject);
-        assertTrue(PermissionHandler.checkPermission(new Permission("opensearch.indexing.index.create"), "cluster", testInternalSubject));
+        testInternalSubject.permissionHandler.grantPermission(
+            new Permission("opensearch.indexing.index.create"),
+            "cluster",
+            testInternalSubject
+        );
+        assertTrue(
+            testInternalSubject.permissionHandler.checkPermission(
+                new Permission("opensearch.indexing.index.create"),
+                "cluster",
+                testInternalSubject
+            )
+        );
     }
 
     public void testInternalSubjectPermissionGrantThenDeny() {
 
         InternalSubject testInternalSubject = new InternalSubject(SecurityUtils.getSubject());
-        PermissionHandler.grantPermission(new Permission("opensearch.indexing.index.create"), "cluster", testInternalSubject);
-        assertTrue(PermissionHandler.checkPermission(new Permission("opensearch.indexing.index.create"), "cluster", testInternalSubject));
-        PermissionHandler.deletePermissions(new Permission("opensearch.indexing.index.create"), "cluster", testInternalSubject);
-        assertFalse(PermissionHandler.checkPermission(new Permission("opensearch.indexing.index.create"), "cluster", testInternalSubject));
+        testInternalSubject.permissionHandler.grantPermission(
+            new Permission("opensearch.indexing.index.create"),
+            "cluster",
+            testInternalSubject
+        );
+        assertTrue(
+            testInternalSubject.permissionHandler.checkPermission(
+                new Permission("opensearch.indexing.index.create"),
+                "cluster",
+                testInternalSubject
+            )
+        );
+        testInternalSubject.permissionHandler.deletePermissions(
+            new Permission("opensearch.indexing.index.create"),
+            "cluster",
+            testInternalSubject
+        );
+        assertFalse(
+            testInternalSubject.permissionHandler.checkPermission(
+                new Permission("opensearch.indexing.index.create"),
+                "cluster",
+                testInternalSubject
+            )
+        );
     }
 
     public void testInternalSubjectPermissionGrantSuccess() throws IOException {
         InternalSubject testInternalSubject = new InternalSubject(SecurityUtils.getSubject());
-        PermissionHandler.grantPermission(new Permission("opensearch.indexing.index.create"), "cluster", testInternalSubject);
-        assertTrue(PermissionHandler.checkPermission(new Permission("opensearch.indexing.index.create"), "cluster", testInternalSubject));
+
+        testInternalSubject.permissionHandler.grantPermission(
+            new Permission("opensearch.indexing.index.create"),
+            "cluster",
+            testInternalSubject
+        );
+        assertTrue(
+            testInternalSubject.permissionHandler.checkPermission(
+                new Permission("opensearch.indexing.index.create"),
+                "cluster",
+                testInternalSubject
+            )
+        );
 
         // TODO: Need to make it so that this request is called from the context of the user
         Request request = new Request("PUT", "/sample_index1");
@@ -53,8 +94,18 @@ public void testInternalSubjectPermissionGrantSuccess() throws IOException {
 
     public void testInternalSubjectPermissionGrantThenDenyFailure() throws IOException {
         InternalSubject testInternalSubject = new InternalSubject(SecurityUtils.getSubject());
-        PermissionHandler.grantPermission(new Permission("opensearch.indexing.index.create"), "cluster", testInternalSubject);
-        assertTrue(PermissionHandler.checkPermission(new Permission("opensearch.indexing.index.create"), "cluster", testInternalSubject));
+        testInternalSubject.permissionHandler.grantPermission(
+            new Permission("opensearch.indexing.index.create"),
+            "cluster",
+            testInternalSubject
+        );
+        assertTrue(
+            testInternalSubject.permissionHandler.checkPermission(
+                new Permission("opensearch.indexing.index.create"),
+                "cluster",
+                testInternalSubject
+            )
+        );
 
         // TODO: Need to make it so that this request is called from the context of the user
         Request request = new Request("PUT", "/sample_index1");
@@ -63,8 +114,18 @@ public void testInternalSubjectPermissionGrantThenDenyFailure() throws IOExcepti
         assertEquals(RestStatus.OK.getStatus(), response.getStatusLine().getStatusCode());
         assertThat(content, containsString("green"));
 
-        PermissionHandler.deletePermissions(new Permission("opensearch.indexing.index.create"), "cluster", testInternalSubject);
-        assertFalse(PermissionHandler.checkPermission(new Permission("opensearch.indexing.index.create"), "cluster", testInternalSubject));
+        testInternalSubject.permissionHandler.deletePermissions(
+            new Permission("opensearch.indexing.index.create"),
+            "cluster",
+            testInternalSubject
+        );
+        assertFalse(
+            testInternalSubject.permissionHandler.checkPermission(
+                new Permission("opensearch.indexing.index.create"),
+                "cluster",
+                testInternalSubject
+            )
+        );
 
         // TODO: Need to make it so that this request is called from the context of the user
         Request request2 = new Request("PUT", "/sample_index2");