From 07a32f44ee033e947fb113e80a081dac2f3296c9 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Wed, 20 Dec 2023 20:47:22 +0000 Subject: [PATCH] Add resource permission type and update view test cases Signed-off-by: Peter Nied --- .../opensearch/security/http/ViewsTests.java | 6 +--- .../test/framework/TestSecurityConfig.java | 35 +++++++++++++++++-- .../security/securityconf/ConfigModelV7.java | 30 +++++++++++++--- .../security/securityconf/impl/v7/RoleV7.java | 20 +++++++++-- .../resources/static_config/static_roles.yml | 3 ++ 5 files changed, 81 insertions(+), 13 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/http/ViewsTests.java b/src/integrationTest/java/org/opensearch/security/http/ViewsTests.java index 985f9c0f10..8d4dcf1003 100644 --- 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 index 7aac0b9c34..4e23517065 100644 --- 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 clusterPermissions = new ArrayList<>(); private List indexPermissions = new ArrayList<>(); - private List resourcePermissions; + private List 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 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 index d1a2db5aec..2bddd8f601 100644 --- 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 permittedClusterActions = agr.resolvedActions(securityRole.getValue().getCluster_permissions()); _securityRole.addClusterPerms(permittedClusterActions); - _securityRole.addResourcePermissions(null) + + final Set resourcePermissions = new HashSet<>(); + for (final RoleV7.ResourcePermission rp : securityRole.getValue().getResource_permissions()) { + final String resourceType = rp.getResource_type(); + final List 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 getTenants(User user) { @@ -759,6 +771,16 @@ public String getResourceType() { public Set 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 index d7e5b2f54c..fb8cd5cb98 100644 --- 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 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 getResource_ids() { return resource_ids; } - public void setResource_ids(List resource_ids) { + public void setResource_ids(final List resource_ids) { this.resource_ids = resource_ids; } } @@ -274,6 +282,14 @@ public void setIndex_permissions(List index_permissions) { this.index_permissions = index_permissions; } + public List getResource_permissions() { + return resource_permissions; + } + + public void setResource_permissions(final List resources_permissions) { + this.resource_permissions = resources_permissions; + } + public List 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 index c7820ab627..a29d118f66 100644 --- 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