[ https://issues.apache.org/jira/browse/GEODE-3974?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16320862#comment-16320862 ]
ASF GitHub Bot commented on GEODE-3974: --------------------------------------- jdeppe-pivotal closed pull request #1258: GEODE-3974: Improve permissions for geode-modules functions URL: https://github.com/apache/geode/pull/1258 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/BootstrappingFunction.java b/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/BootstrappingFunction.java index 82ca3e3bee..4d64a7817f 100644 --- a/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/BootstrappingFunction.java +++ b/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/BootstrappingFunction.java @@ -17,6 +17,8 @@ import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; +import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Set; @@ -32,6 +34,8 @@ import org.apache.geode.distributed.internal.InternalDistributedSystem; import org.apache.geode.distributed.internal.MembershipListener; import org.apache.geode.distributed.internal.membership.InternalDistributedMember; +import org.apache.geode.management.internal.security.ResourcePermissions; +import org.apache.geode.security.ResourcePermission; public class BootstrappingFunction implements Function, MembershipListener, DataSerializable { @@ -87,6 +91,11 @@ private Cache verifyCacheExists() { return cache; } + @Override + public Collection<ResourcePermission> getRequiredPermissions(String regionName) { + return Collections.singletonList(ResourcePermissions.CLUSTER_MANAGE); + } + private void registerAsMembershipListener(Cache cache) { DistributionManager dm = ((InternalDistributedSystem) cache.getDistributedSystem()).getDistributionManager(); diff --git a/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/CreateRegionFunction.java b/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/CreateRegionFunction.java index 7c89cb0605..bf998d1703 100644 --- a/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/CreateRegionFunction.java +++ b/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/CreateRegionFunction.java @@ -20,10 +20,10 @@ import java.io.FileWriter; import java.io.IOException; import java.io.PrintWriter; +import java.util.Collection; +import java.util.Collections; import java.util.Properties; -import javax.xml.crypto.Data; - import org.apache.geode.DataSerializable; import org.apache.geode.InternalGemFireError; import org.apache.geode.cache.AttributesFactory; @@ -32,8 +32,6 @@ import org.apache.geode.cache.Declarable; import org.apache.geode.cache.Region; import org.apache.geode.cache.RegionAttributes; -import org.apache.geode.cache.RegionFactory; -import org.apache.geode.cache.RegionShortcut; import org.apache.geode.cache.Scope; import org.apache.geode.cache.client.ClientCache; import org.apache.geode.cache.execute.Function; @@ -46,6 +44,8 @@ import org.apache.geode.internal.cache.PartitionedRegion; import org.apache.geode.internal.cache.xmlcache.CacheXmlGenerator; import org.apache.geode.internal.i18n.LocalizedStrings; +import org.apache.geode.management.internal.security.ResourcePermissions; +import org.apache.geode.security.ResourcePermission; public class CreateRegionFunction implements Function, Declarable, DataSerializable { @@ -96,6 +96,11 @@ public void execute(FunctionContext context) { context.getResultSender().lastResult(status); } + @Override + public Collection<ResourcePermission> getRequiredPermissions(String regionName) { + return Collections.singletonList(ResourcePermissions.DATA_MANAGE); + } + private RegionStatus createOrRetrieveRegion(RegionConfiguration configuration) { RegionStatus status = null; String regionName = configuration.getRegionName(); diff --git a/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/RegionSizeFunction.java b/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/RegionSizeFunction.java index 328d490b4a..a45e4727b3 100644 --- a/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/RegionSizeFunction.java +++ b/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/RegionSizeFunction.java @@ -17,6 +17,8 @@ import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; +import java.util.Collection; +import java.util.Collections; import java.util.Properties; import org.apache.geode.DataSerializable; @@ -24,6 +26,7 @@ import org.apache.geode.cache.execute.Function; import org.apache.geode.cache.execute.FunctionContext; import org.apache.geode.cache.execute.RegionFunctionContext; +import org.apache.geode.security.ResourcePermission; public class RegionSizeFunction implements Function, Declarable, DataSerializable { @@ -38,6 +41,12 @@ public void execute(FunctionContext context) { context.getResultSender().lastResult(rfc.getDataSet().size()); } + @Override + public Collection<ResourcePermission> getRequiredPermissions(String regionName) { + return Collections.singletonList(new ResourcePermission(ResourcePermission.Resource.DATA, + ResourcePermission.Operation.READ, regionName)); + } + public String getId() { return ID; } diff --git a/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/TouchPartitionedRegionEntriesFunction.java b/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/TouchPartitionedRegionEntriesFunction.java index 01b1685614..d503359364 100644 --- a/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/TouchPartitionedRegionEntriesFunction.java +++ b/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/TouchPartitionedRegionEntriesFunction.java @@ -17,6 +17,8 @@ import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; +import java.util.Collection; +import java.util.Collections; import java.util.Properties; import java.util.Set; @@ -29,6 +31,7 @@ import org.apache.geode.cache.execute.FunctionContext; import org.apache.geode.cache.execute.RegionFunctionContext; import org.apache.geode.cache.partition.PartitionRegionHelper; +import org.apache.geode.security.ResourcePermission; /** * Touches the keys contained in the set of keys by performing a get on the partitioned region. @@ -76,6 +79,12 @@ public void execute(FunctionContext context) { context.getResultSender().lastResult(true); } + @Override + public Collection<ResourcePermission> getRequiredPermissions(String regionName) { + return Collections.singletonList(new ResourcePermission(ResourcePermission.Resource.DATA, + ResourcePermission.Operation.READ, regionName)); + } + public String getId() { return ID; } diff --git a/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/TouchReplicatedRegionEntriesFunction.java b/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/TouchReplicatedRegionEntriesFunction.java index f48c117f4d..7fcb0e9e49 100644 --- a/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/TouchReplicatedRegionEntriesFunction.java +++ b/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/TouchReplicatedRegionEntriesFunction.java @@ -17,6 +17,8 @@ import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; +import java.util.Collection; +import java.util.Collections; import java.util.Properties; import java.util.Set; @@ -27,6 +29,7 @@ import org.apache.geode.cache.Region; import org.apache.geode.cache.execute.Function; import org.apache.geode.cache.execute.FunctionContext; +import org.apache.geode.security.ResourcePermission; /** * Touches the keys contained in the set of keys by performing a get on the replicated region. This @@ -71,6 +74,12 @@ public void execute(FunctionContext context) { context.getResultSender().lastResult(true); } + @Override + public Collection<ResourcePermission> getRequiredPermissions(String regionName) { + return Collections.singletonList(new ResourcePermission(ResourcePermission.Resource.DATA, + ResourcePermission.Operation.READ, regionName)); + } + public String getId() { return ID; } diff --git a/extensions/geode-modules/src/test/java/org/apache/geode/modules/util/ModuleFunctionsSecurityTest.java b/extensions/geode-modules/src/test/java/org/apache/geode/modules/util/ModuleFunctionsSecurityTest.java new file mode 100644 index 0000000000..1a0548d397 --- /dev/null +++ b/extensions/geode-modules/src/test/java/org/apache/geode/modules/util/ModuleFunctionsSecurityTest.java @@ -0,0 +1,91 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. You may obtain a + * copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package org.apache.geode.modules.util; + +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import org.apache.geode.cache.RegionShortcut; +import org.apache.geode.cache.execute.FunctionService; +import org.apache.geode.examples.SimpleSecurityManager; +import org.apache.geode.test.junit.categories.IntegrationTest; +import org.apache.geode.test.junit.categories.SecurityTest; +import org.apache.geode.test.junit.rules.ConnectionConfiguration; +import org.apache.geode.test.junit.rules.GfshCommandRule; +import org.apache.geode.test.junit.rules.ServerStarterRule; + +@Category({IntegrationTest.class, SecurityTest.class}) +public class ModuleFunctionsSecurityTest { + + @ClassRule + public static ServerStarterRule server = + new ServerStarterRule().withJMXManager().withSecurityManager(SimpleSecurityManager.class) + .withRegion(RegionShortcut.REPLICATE, "REPLICATE_1") + .withRegion(RegionShortcut.PARTITION, "PARTITION_1").withAutoStart(); + + @Rule + public GfshCommandRule gfsh = + new GfshCommandRule(server::getJmxPort, GfshCommandRule.PortType.jmxManager); + + @BeforeClass + public static void setupClass() { + FunctionService.registerFunction(new BootstrappingFunction()); + FunctionService.registerFunction(new CreateRegionFunction()); + FunctionService.registerFunction(new RegionSizeFunction()); + FunctionService.registerFunction(new TouchPartitionedRegionEntriesFunction()); + FunctionService.registerFunction(new TouchReplicatedRegionEntriesFunction()); + } + + @Test + @ConnectionConfiguration(user = "dataWrite", password = "dataWrite") + public void testInvalidPermissionsForBootstrappingFunction() throws Exception { + gfsh.executeAndAssertThat("execute function --id=" + BootstrappingFunction.ID) + .containsOutput("not authorized for CLUSTER:MANAGE").statusIsSuccess(); + } + + @Test + @ConnectionConfiguration(user = "dataWrite", password = "dataWrite") + public void testInvalidPermissionsForCreateRegionFunction() throws Exception { + gfsh.executeAndAssertThat("execute function --id=" + CreateRegionFunction.ID) + .containsOutput("not authorized for DATA:MANAGE").statusIsSuccess(); + } + + @Test + @ConnectionConfiguration(user = "dataWrite", password = "dataWrite") + public void testInvalidPermissionsForRegionSizeFunction() throws Exception { + gfsh.executeAndAssertThat("execute function --region=REPLICATE_1 --id=" + RegionSizeFunction.ID) + .containsOutput("not authorized for DATA:READ:REPLICATE_1").statusIsSuccess(); + } + + @Test + @ConnectionConfiguration(user = "dataWrite", password = "dataWrite") + public void testInvalidPermissionsForTouchPartitionedRegionEntriesFunction() throws Exception { + gfsh.executeAndAssertThat( + "execute function --region=PARTITION_1 --id=" + TouchPartitionedRegionEntriesFunction.ID) + .containsOutput("not authorized for DATA:READ:PARTITION_1").statusIsSuccess(); + } + + @Test + @ConnectionConfiguration(user = "dataWrite", password = "dataWrite") + public void testInvalidPermissionsForTouchReplicatedRegionEntriesFunction() throws Exception { + gfsh.executeAndAssertThat( + "execute function --region=REPLICATE_1 --id=" + TouchReplicatedRegionEntriesFunction.ID) + .containsOutput("not authorized for DATA:READ:REPLICATE_1").statusIsSuccess(); + } +} diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java index 80985660b7..3729d75b4b 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java @@ -14,6 +14,8 @@ */ package org.apache.geode.management.internal.cli.functions; +import java.util.Collection; +import java.util.Collections; import java.util.Set; import org.apache.commons.lang.StringUtils; @@ -49,6 +51,8 @@ import org.apache.geode.management.internal.cli.i18n.CliStrings; import org.apache.geode.management.internal.cli.util.RegionPath; import org.apache.geode.management.internal.configuration.domain.XmlEntity; +import org.apache.geode.management.internal.security.ResourcePermissions; +import org.apache.geode.security.ResourcePermission; /** * @@ -121,6 +125,11 @@ public void execute(FunctionContext context) { } } + @Override + public Collection<ResourcePermission> getRequiredPermissions(String regionName) { + return Collections.singletonList(ResourcePermissions.DATA_MANAGE); + } + private CliFunctionResult handleException(final String memberNameOrId, final String exceptionMsg, final Exception e) { if (e != null && logger.isDebugEnabled()) { ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > improve permission for Internal functions > ----------------------------------------- > > Key: GEODE-3974 > URL: https://issues.apache.org/jira/browse/GEODE-3974 > Project: Geode > Issue Type: Bug > Components: management > Reporter: Jinmei Liao > > Internal functions needs to be updated to require appropriate permissions -- This message was sent by Atlassian JIRA (v6.4.14#64029)