Add resource permission type and update view test cases

Signed-off-by: Peter Nied <petern@amazon.com>
peternied · Dec 20, 2023 · 07a32f4 · 07a32f4
1 parent 1fb29ad
commit 07a32f4
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 13 deletions.
diff --git a/src/integrationTest/java/org/opensearch/security/http/ViewsTests.java b/src/integrationTest/java/org/opensearch/security/http/ViewsTests.java
@@ -35,7 +35,7 @@
 @ThreadLeakScope(ThreadLeakScope.Scope.NONE)
 public class ViewsTests {
     private static final TestSecurityConfig.User ADMIN_USER = new TestSecurityConfig.User("admin").backendRoles("admin");
-    private static final TestSecurityConfig.User VIEW_USER = new TestSecurityConfig.User("view_user").roles(new TestSecurityConfig.Role("see views").clusterPermissions("cluster:views:search"));
+    private static final TestSecurityConfig.User VIEW_USER = new TestSecurityConfig.User("view_user").roles(new TestSecurityConfig.Role("see views").clusterPermissions("cluster:views:search").resourcePermissions("view_id", "songs"));
 
     @ClassRule
     public static LocalCluster cluster = new LocalCluster.Builder().singleNode()
@@ -48,9 +48,7 @@ public class ViewsTests {
     public static void createTestData() {
         try (final Client client = cluster.getInternalNodeClient()) {
             final var doc1 = client.prepareIndex("songs-2022").setRefreshPolicy(IMMEDIATE).setSource(SONGS[0].asMap()).get();
-            System.err.println("Created doc1:\r\n" + doc1);
             final var doc2 = client.prepareIndex("songs-2023").setRefreshPolicy(IMMEDIATE).setSource(SONGS[1].asMap()).get();
-            System.err.println("Created doc2:\r\n" + doc2);
         }
     }
 
@@ -66,10 +64,8 @@ public void createView() {
 
             final HttpResponse createView = adminClient.postJson("views", createViewBody());
             createView.assertStatusCode(SC_CREATED);
-            System.err.println("View created:\r\n" + createView.getBody());
 
             final HttpResponse search = adminClient.postJson("songs-*/_search", createQueryString());
-            System.err.println("Search result:\r\n" + search.getBody());
 
             final HttpResponse searchView = adminClient.postJson("views/songs/_search", createQueryString());
             assertThat("Search response was:\r\n" + searchView.getBody(), searchView.getIntFromJsonBody("/hits/total/value"), equalTo(2));

diff --git a/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java b/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java
@@ -336,13 +336,13 @@ public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params
     }
 
     public static class Role implements ToXContentObject {
-        public static Role ALL_ACCESS = new Role("all_access").clusterPermissions("*").indexPermissions("*").on("*");
+        public static Role ALL_ACCESS = new Role("all_access").clusterPermissions("*").indexPermissions("*").on("*").resourcePermissions("*", "*");
 
         private String name;
         private List<String> clusterPermissions = new ArrayList<>();
 
         private List<IndexPermission> indexPermissions = new ArrayList<>();
-        private List<String, User> resourcePermissions;
+        private List<ResourcePermission> resourcePermissions = new ArrayList<>();
 
         public Role(String name) {
             this.name = name;
@@ -357,6 +357,11 @@ public IndexPermission indexPermissions(String... indexPermissions) {
             return new IndexPermission(this, indexPermissions);
         }
 
+        public Role resourcePermissions(final String resourceType, final String... resourceIds) {
+            resourcePermissions.add(new ResourcePermission(resourceType).resourceIds(resourceIds));
+            return this;
+        }
+
         public Role name(String name) {
             this.name = name;
             return this;
@@ -370,6 +375,7 @@ public Role clone() {
             Role role = new Role(this.name);
             role.clusterPermissions.addAll(this.clusterPermissions);
             role.indexPermissions.addAll(this.indexPermissions);
+            role.resourcePermissions.addAll(this.resourcePermissions);
             return role;
         }
 
@@ -458,6 +464,31 @@ public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params
         }
     }
 
+    public static class ResourcePermission implements ToXContentObject {
+        private String resourceType;
+        private List<String> resourceIds;
+
+        ResourcePermission(final String resourceType) {
+            this.resourceType = resourceType;
+        }
+
+        public ResourcePermission resourceIds(final String... resourceIds) {
+            this.resourceIds = Arrays.asList(resourceIds);
+            return this;
+        }
+
+        @Override
+        public XContentBuilder toXContent(final XContentBuilder xContentBuilder, final Params params) throws IOException {
+            xContentBuilder.startObject();
+
+            xContentBuilder.field("resource_type", resourceType);
+            xContentBuilder.field("resource_ids", resourceIds);
+
+            xContentBuilder.endObject();
+            return xContentBuilder;
+        }
+    }
+
     public static class AuthcDomain implements ToXContentObject {
 
         private static String PUBLIC_KEY =

diff --git a/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java
@@ -26,6 +26,7 @@
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.concurrent.Callable;
@@ -216,7 +217,14 @@ public SecurityRole call() throws Exception {
 
                     final Set<String> permittedClusterActions = agr.resolvedActions(securityRole.getValue().getCluster_permissions());
                     _securityRole.addClusterPerms(permittedClusterActions);
-                    _securityRole.addResourcePermissions(null)
+
+                    final Set<ResourcePermission> resourcePermissions = new HashSet<>();
+                    for (final RoleV7.ResourcePermission rp : securityRole.getValue().getResource_permissions()) {
+                        final String resourceType = rp.getResource_type();
+                        final List<String> resourceIds = rp.getResource_ids();
+                        resourcePermissions.add(new ResourcePermission(resourceType, resourceIds));
+                    }
+                    _securityRole.addResourcePermissions(resourcePermissions);
 
                     /*for(RoleV7.Tenant tenant: securityRole.getValue().getTenant_permissions()) {
 
@@ -568,8 +576,9 @@ private boolean containsDlsFlsConfig() {
         @Override
         public boolean hasResourcePermission(final String resourceType, final String resourceId) {
             for (final SecurityRole role : roles) {
-                final ResourcePermission resourcePermission = role.resourcePermissions.get(resourceType);
-                if (resourcePermission != null && resourcePermission.permmittedResourceIds.contains(resourceId)) {
+                final ResourcePermission resourcePermission = Optional.ofNullable(role.resourcePermissions.get(resourceType))
+                    .orElseGet(() -> role.resourcePermissions.get("*"));
+                if (resourcePermission != null && (resourcePermission.permmittedResourceIds.contains(resourceId) || resourcePermission.permmittedResourceIds.contains("*"))) {
                     return true;
                 }
             }
@@ -726,7 +735,10 @@ public String toString() {
                 + ipatterns
                 + System.lineSeparator()
                 + "    clusterPerms="
-                + clusterPerms;
+                + clusterPerms
+                + System.lineSeparator()
+                + "    resourcePermissions="
+                + resourcePermissions;
         }
 
         // public Set<Tenant> getTenants(User user) {
@@ -759,6 +771,16 @@ public String getResourceType() {
         public Set<String> getPermittedResourceIds() {
             return permmittedResourceIds;
         }
+
+        @Override
+        public String toString() {
+            return System.lineSeparator()
+                + "        resourceType="
+                + resourceType
+                + System.lineSeparator()
+                + "        permmittedResourceIds="
+                + permmittedResourceIds;
+        }
     }
 
     // sg roles

diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java
@@ -227,17 +227,25 @@ public String toString() {
     }
 
     public static class ResourcePermission {
-        private String resourceType;
+        private String resource_type;
         private List<String> resource_ids = Collections.emptyList();
 
         public ResourcePermission() {
         }
 
+        public String getResource_type() {
+            return resource_type;
+        }
+
+        public void setResource_type(final String resource_type) {
+            this.resource_type = resource_type;
+        }
+
         public List<String> getResource_ids() {
             return resource_ids;
         }
 
-        public void setResource_ids(List<String> resource_ids) {
+        public void setResource_ids(final List<String> resource_ids) {
             this.resource_ids = resource_ids;
         }
     }
@@ -274,6 +282,14 @@ public void setIndex_permissions(List<Index> index_permissions) {
         this.index_permissions = index_permissions;
     }
 
+    public List<ResourcePermission> getResource_permissions() {
+        return resource_permissions;
+    }
+
+    public void setResource_permissions(final List<ResourcePermission> resources_permissions) {
+        this.resource_permissions = resources_permissions;
+    }
+
     public List<Tenant> getTenant_permissions() {
         return tenant_permissions;
     }

diff --git a/src/main/resources/static_config/static_roles.yml b/src/main/resources/static_config/static_roles.yml
@@ -20,6 +20,9 @@ all_access:
         - "*"
       allowed_actions:
         - "kibana_all_write"
+  resource_permissions:
+    - resource_type: "*"
+      resource_ids: ["*"]
 
 kibana_user:
   reserved: true