From d55321b0f2d56c229c91c123150ffad8f5a77621 Mon Sep 17 00:00:00 2001 From: Xiaojian Zhou Date: Mon, 1 Jul 2019 16:19:43 -0700 Subject: [PATCH] =?UTF-8?q?GEODE-6930:=20Need=20to=20specify=20required=20?= =?UTF-8?q?resource=20permission=20DATA=5FREAD=20fo=E2=80=A6=20(#3763)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GEODE-6930: Need to specify required resource permission DATA_READ for lucene user functions. Co-authored-by: Xiaojian Zhou Evans Co-authored-by: Donal Evans --- .../test/LuceneFunctionSecurityTest.java | 119 ++++++++++++++---- .../IndexingInProgressFunction.java | 10 ++ .../distributed/LuceneQueryFunction.java | 8 ++ .../distributed/WaitUntilFlushedFunction.java | 12 ++ .../results/LuceneGetPageFunction.java | 9 ++ 5 files changed, 133 insertions(+), 25 deletions(-) diff --git a/geode-lucene/src/integrationTest/java/org/apache/geode/cache/lucene/test/LuceneFunctionSecurityTest.java b/geode-lucene/src/integrationTest/java/org/apache/geode/cache/lucene/test/LuceneFunctionSecurityTest.java index bf623de19138..a0448cfbc42c 100644 --- a/geode-lucene/src/integrationTest/java/org/apache/geode/cache/lucene/test/LuceneFunctionSecurityTest.java +++ b/geode-lucene/src/integrationTest/java/org/apache/geode/cache/lucene/test/LuceneFunctionSecurityTest.java @@ -15,8 +15,10 @@ package org.apache.geode.cache.lucene.test; -import java.util.HashMap; -import java.util.Map; +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Collection; +import java.util.HashSet; import org.junit.BeforeClass; import org.junit.ClassRule; @@ -33,10 +35,14 @@ import org.apache.geode.cache.lucene.internal.cli.functions.LuceneListIndexFunction; import org.apache.geode.cache.lucene.internal.cli.functions.LuceneSearchIndexFunction; import org.apache.geode.cache.lucene.internal.directory.DumpDirectoryFiles; +import org.apache.geode.cache.lucene.internal.distributed.IndexingInProgressFunction; import org.apache.geode.cache.lucene.internal.distributed.LuceneQueryFunction; import org.apache.geode.cache.lucene.internal.distributed.WaitUntilFlushedFunction; import org.apache.geode.cache.lucene.internal.results.LuceneGetPageFunction; import org.apache.geode.examples.SimpleSecurityManager; +import org.apache.geode.management.internal.security.ResourcePermissions; +import org.apache.geode.security.ResourcePermission; +import org.apache.geode.test.dunit.IgnoredException; import org.apache.geode.test.junit.categories.LuceneTest; import org.apache.geode.test.junit.categories.SecurityTest; import org.apache.geode.test.junit.rules.ConnectionConfiguration; @@ -46,61 +52,124 @@ @Category({SecurityTest.class, LuceneTest.class}) public class LuceneFunctionSecurityTest { private static final String RESULT_HEADER = "Message"; + private static final String REGION_NAME = "testRegion"; @ClassRule public static ServerStarterRule server = - new ServerStarterRule().withJMXManager().withSecurityManager(SimpleSecurityManager.class) - .withRegion(RegionShortcut.PARTITION, "testRegion").withAutoStart(); + new ServerStarterRule().withJMXManager() + .withSecurityManager(SimpleSecurityManager.class) + .withRegion(RegionShortcut.PARTITION, REGION_NAME).withAutoStart(); @Rule public GfshCommandRule gfsh = - new GfshCommandRule(server::getJmxPort, GfshCommandRule.PortType.jmxManager); + new GfshCommandRule(server::getJmxPort, + GfshCommandRule.PortType.jmxManager); - private static Map functionStringMap = new HashMap<>(); + private static HashSet functions = new HashSet<>(); + private static HashSet functionsWithDataRead = new HashSet<>(); @BeforeClass - public static void setupClass() { - functionStringMap.put(new LuceneCreateIndexFunction(), "*"); - functionStringMap.put(new LuceneDescribeIndexFunction(), "*"); - functionStringMap.put(new LuceneDestroyIndexFunction(), "*"); - functionStringMap.put(new LuceneListIndexFunction(), "*"); - functionStringMap.put(new LuceneSearchIndexFunction(), "*"); - functionStringMap.put(new LuceneQueryFunction(), "*"); - functionStringMap.put(new WaitUntilFlushedFunction(), "*"); - functionStringMap.put(new LuceneGetPageFunction(), "*"); - - functionStringMap.keySet().forEach(FunctionService::registerFunction); + public static void setupFunctions() { + functions.add(new LuceneCreateIndexFunction()); + functions.add(new LuceneDescribeIndexFunction()); + functions.add(new LuceneDestroyIndexFunction()); + functions.add(new LuceneListIndexFunction()); + functions.add(new LuceneSearchIndexFunction()); + functions.add(new LuceneQueryFunction()); + functions.add(new WaitUntilFlushedFunction()); + functions.add(new LuceneGetPageFunction()); + functions.add(new IndexingInProgressFunction()); + + functions.forEach(FunctionService::registerFunction); FunctionService.registerFunction(new DumpDirectoryFiles()); + + for (Function function : functions) { + Collection permissions = function + .getRequiredPermissions(REGION_NAME); + if (permissions.contains(ResourcePermissions.DATA_READ)) { + functionsWithDataRead.add(function); + } + } } @Test @ConnectionConfiguration(user = "user", password = "user") public void functionRequireExpectedPermission() throws Exception { - functionStringMap.entrySet().stream().forEach(entry -> { - Function function = entry.getKey(); - String permission = entry.getValue(); - gfsh.executeAndAssertThat("execute function --region=testRegion --id=" + function.getId()) + functions.stream().forEach(function -> { + Collection permissions = function + .getRequiredPermissions(REGION_NAME); + ResourcePermission resourcePermission = (ResourcePermission) permissions + .toArray()[0]; + String permission = resourcePermission.toString(); + + gfsh.executeAndAssertThat( + "execute function --region=" + REGION_NAME + " --id=" + function.getId()) .tableHasRowCount(1) .tableHasRowWithValues(RESULT_HEADER, "Exception: user not authorized for " + permission) .statusIsError(); }); } + @Test + @ConnectionConfiguration(user = "DATAREAD", password = "DATAREAD") + public void functionRequireExpectedReadPermissionToPass() throws Exception { + IgnoredException.addIgnoredException("did not send last result"); + assertThat(functionsWithDataRead).isNotEmpty(); + + functionsWithDataRead.stream().forEach(function -> { + String permission = "*"; + Collection permissions = function + .getRequiredPermissions(REGION_NAME); + for (ResourcePermission resourcePermission : permissions) { + if (permission.equals(ResourcePermissions.DATA_READ)) { + permission = resourcePermission.toString(); + } + } + gfsh.executeAndAssertThat( + "execute function --region=" + REGION_NAME + " --id=" + function.getId()) + .doesNotContainOutput("Exception: user not authorized for ") + .statusIsError(); + }); + } + + @Test + @ConnectionConfiguration(user = "user", password = "user") + public void functionWithoutExpectedPermissionWillFail() throws Exception { + IgnoredException.addIgnoredException("did not send last result"); + assertThat(functionsWithDataRead).isNotEmpty(); + + functionsWithDataRead.stream().forEach(function -> { + Collection permissions = function + .getRequiredPermissions(REGION_NAME); + ResourcePermission resourcePermission = (ResourcePermission) permissions + .toArray()[0]; + String permission = resourcePermission.toString(); + gfsh.executeAndAssertThat( + "execute function --region=" + REGION_NAME + " --id=" + function.getId()) + .statusIsError().hasTableSection().hasRowSize(1).hasRow(0).asList().last() + .isEqualTo("Exception: user not authorized for " + permission); + }); + } + // use DumpDirectoryFile function to verify that all the permissions returned by the // getRequiredPermission are all enforced before trying to execute @Test @ConnectionConfiguration(user = "clusterManage", password = "clusterManage") public void dumpDirectoryFileRequiresAll_insufficientUser() { - gfsh.executeAndAssertThat("execute function --region=testRegion --id=" + DumpDirectoryFiles.ID) + gfsh.executeAndAssertThat( + "execute function --region=" + REGION_NAME + " --id=" + DumpDirectoryFiles.ID) .tableHasRowCount(1) - .tableHasRowWithValues(RESULT_HEADER, "Exception: clusterManage not authorized for *") + .tableHasRowWithValues(RESULT_HEADER, + "Exception: clusterManage not authorized for *") .statusIsError(); } @Test @ConnectionConfiguration(user = "*", password = "*") public void dumpDirectoryFileRequiresAll_validUser() { - gfsh.executeAndAssertThat("execute function --region=testRegion --id=" + DumpDirectoryFiles.ID) - .tableHasRowCount(1).doesNotContainOutput("not authorized").statusIsError(); + gfsh.executeAndAssertThat( + "execute function --region=" + REGION_NAME + " --id=" + DumpDirectoryFiles.ID) + .tableHasRowCount(1).doesNotContainOutput("not authorized") + .statusIsError(); } } diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/IndexingInProgressFunction.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/IndexingInProgressFunction.java index 934e7757a6ee..33f19733b106 100644 --- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/IndexingInProgressFunction.java +++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/IndexingInProgressFunction.java @@ -15,6 +15,9 @@ package org.apache.geode.cache.lucene.internal.distributed; +import java.util.Collection; +import java.util.Collections; + import org.apache.geode.cache.Cache; import org.apache.geode.cache.Region; import org.apache.geode.cache.execute.FunctionContext; @@ -24,6 +27,8 @@ import org.apache.geode.cache.lucene.LuceneService; import org.apache.geode.cache.lucene.LuceneServiceProvider; import org.apache.geode.internal.cache.execute.InternalFunction; +import org.apache.geode.management.internal.security.ResourcePermissions; +import org.apache.geode.security.ResourcePermission; public class IndexingInProgressFunction implements InternalFunction { @@ -60,4 +65,9 @@ public String getId() { public boolean optimizeForWrite() { return true; } + + @Override + public Collection getRequiredPermissions(String regionName) { + return Collections.singletonList(ResourcePermissions.DATA_READ); + } } diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/LuceneQueryFunction.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/LuceneQueryFunction.java index 33ae0c322f7c..caa07722f3e7 100644 --- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/LuceneQueryFunction.java +++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/LuceneQueryFunction.java @@ -18,6 +18,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.Set; import org.apache.logging.log4j.Logger; @@ -51,6 +52,8 @@ import org.apache.geode.internal.cache.execute.InternalFunctionInvocationTargetException; import org.apache.geode.internal.cache.execute.PartitionedRegionFunctionResultSender; import org.apache.geode.internal.logging.LogService; +import org.apache.geode.management.internal.security.ResourcePermissions; +import org.apache.geode.security.ResourcePermission; /** * {@link LuceneQueryFunction} coordinates text search on a member. It receives text search query @@ -227,4 +230,9 @@ public String getId() { public boolean optimizeForWrite() { return true; } + + @Override + public Collection getRequiredPermissions(String regionName) { + return Collections.singletonList(ResourcePermissions.DATA_READ); + } } diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/WaitUntilFlushedFunction.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/WaitUntilFlushedFunction.java index 0e84ad38d3e9..7d8281c046f8 100644 --- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/WaitUntilFlushedFunction.java +++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/WaitUntilFlushedFunction.java @@ -15,6 +15,8 @@ package org.apache.geode.cache.lucene.internal.distributed; +import java.util.Collection; +import java.util.Collections; import java.util.concurrent.TimeUnit; import org.apache.geode.cache.Cache; @@ -25,6 +27,8 @@ import org.apache.geode.cache.execute.ResultSender; import org.apache.geode.cache.lucene.internal.LuceneServiceImpl; import org.apache.geode.internal.cache.execute.InternalFunction; +import org.apache.geode.management.internal.security.ResourcePermissions; +import org.apache.geode.security.ResourcePermission; /** * {@link WaitUntilFlushedFunction} will check all the members with index to wait until the events @@ -43,6 +47,9 @@ public void execute(FunctionContext context) { Region region = ctx.getDataSet(); Cache cache = region.getCache(); WaitUntilFlushedFunctionContext arg = (WaitUntilFlushedFunctionContext) ctx.getArguments(); + if (arg == null) { + return; + } String indexName = arg.getIndexName(); if (indexName == null) { throw new IllegalArgumentException("Missing index name"); @@ -75,4 +82,9 @@ public String getId() { public boolean optimizeForWrite() { return true; } + + @Override + public Collection getRequiredPermissions(String regionName) { + return Collections.singletonList(ResourcePermissions.DATA_READ); + } } diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunction.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunction.java index 3ddbff3f8527..62423e7c4747 100644 --- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunction.java +++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunction.java @@ -15,6 +15,8 @@ package org.apache.geode.cache.lucene.internal.results; +import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Set; @@ -31,6 +33,8 @@ import org.apache.geode.internal.cache.execute.InternalFunction; import org.apache.geode.internal.cache.execute.InternalFunctionInvocationTargetException; import org.apache.geode.internal.logging.LogService; +import org.apache.geode.management.internal.security.ResourcePermissions; +import org.apache.geode.security.ResourcePermission; /** * {@link LuceneGetPageFunction} Returns the values of entries back to the user This behaves @@ -86,4 +90,9 @@ public String getId() { public boolean optimizeForWrite() { return false; } + + @Override + public Collection getRequiredPermissions(String regionName) { + return Collections.singletonList(ResourcePermissions.DATA_READ); + } }