You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ji...@apache.org on 2018/11/06 19:05:41 UTC
[geode] branch develop updated: GEODE-5918 Geode function security
should be dynamically determined by function arguments (#2727)
This is an automated email from the ASF dual-hosted git repository.
jinmeiliao pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new d02315d GEODE-5918 Geode function security should be dynamically determined by function arguments (#2727)
d02315d is described below
commit d02315d1fc989779da6d364c92beaed947fe728f
Author: ivorzhou <iv...@gmail.com>
AuthorDate: Wed Nov 7 03:05:31 2018 +0800
GEODE-5918 Geode function security should be dynamically determined by function arguments (#2727)
---
.../execute/FunctionDynamicByArgsSecurityTest.java | 88 ++++++++++++++++++++++
.../org/apache/geode/cache/execute/Function.java | 58 +++++++++++---
.../tier/sockets/command/ExecuteFunction.java | 2 +-
.../tier/sockets/command/ExecuteFunction65.java | 2 +-
.../tier/sockets/command/ExecuteFunction66.java | 2 +-
.../sockets/command/ExecuteRegionFunction.java | 2 +-
.../sockets/command/ExecuteRegionFunction61.java | 2 +-
.../sockets/command/ExecuteRegionFunction65.java | 2 +-
.../sockets/command/ExecuteRegionFunction66.java | 2 +-
.../command/ExecuteRegionFunctionSingleHop.java | 2 +-
.../cli/functions/UserFunctionExecution.java | 2 +-
.../sockets/command/ExecuteFunction65Test.java | 1 +
.../sockets/command/ExecuteFunction66Test.java | 1 +
.../tier/sockets/command/ExecuteFunctionTest.java | 1 +
.../security/SecureFunctionServiceImpl.java | 11 +--
.../security/SecureFunctionServiceImplTest.java | 6 +-
.../web/controllers/FunctionAccessController.java | 13 ++--
17 files changed, 163 insertions(+), 34 deletions(-)
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/cache/execute/FunctionDynamicByArgsSecurityTest.java b/geode-core/src/integrationTest/java/org/apache/geode/cache/execute/FunctionDynamicByArgsSecurityTest.java
new file mode 100644
index 0000000..09a54aa
--- /dev/null
+++ b/geode-core/src/integrationTest/java/org/apache/geode/cache/execute/FunctionDynamicByArgsSecurityTest.java
@@ -0,0 +1,88 @@
+/*
+ * 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.cache.execute;
+
+import java.util.Collection;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.examples.SimpleSecurityManager;
+import org.apache.geode.security.ResourcePermission;
+import org.apache.geode.test.junit.rules.ConnectionConfiguration;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+
+public class FunctionDynamicByArgsSecurityTest {
+ private static final String RESULT_HEADER = "Message";
+
+ @ClassRule
+ public static ServerStarterRule server =
+ new ServerStarterRule().withJMXManager().withSecurityManager(SimpleSecurityManager.class)
+ .withRegion(RegionShortcut.PARTITION, "testRegion1")
+ .withRegion(RegionShortcut.PARTITION, "testRegion2")
+ .withAutoStart();
+
+ @Rule
+ public GfshCommandRule gfsh =
+ new GfshCommandRule(server::getJmxPort, GfshCommandRule.PortType.jmxManager);
+
+ private static DynamicSecurityFunction function = new DynamicSecurityFunction();
+
+ @BeforeClass
+ public static void setupClass() {
+ FunctionService.registerFunction(function);
+ }
+
+ @Test
+ @ConnectionConfiguration(user = "DATAREADtestRegion1", password = "DATAREADtestRegion1")
+ public void functionDynamicRequireExpectedPermission() throws Exception {
+ gfsh.executeAndAssertThat(
+ "execute function --id=" + function.getId() + " --arguments=testRegion1")
+ .tableHasRowCount(RESULT_HEADER, 1)
+ .tableHasRowWithValues(RESULT_HEADER, "[successfully invoked with argument:testRegion1]")
+ .statusIsSuccess();
+ gfsh.executeAndAssertThat(
+ "execute function --id=" + function.getId() + " --arguments=testRegion1,testRegion2")
+ .tableHasRowCount(RESULT_HEADER, 1)
+ .tableHasRowWithValues(RESULT_HEADER,
+ "Exception: DATAREADtestRegion1 not authorized for DATA:READ:testRegion2")
+ .statusIsError();
+ }
+
+ static class DynamicSecurityFunction implements Function {
+ @Override
+ public void execute(FunctionContext context) {
+ Object args = context.getArguments();
+ String[] regions = (String[]) args;
+ context.getResultSender()
+ .lastResult("successfully invoked with argument:" + String.join(",", regions));
+ }
+
+ @Override
+ public Collection<ResourcePermission> getRequiredPermissions(String regionName, Object args) {
+ String[] regions = (String[]) args;
+ return Stream.of(regions).map(s -> new ResourcePermission(ResourcePermission.Resource.DATA,
+ ResourcePermission.Operation.READ, s)).collect(Collectors.toSet());
+ }
+ }
+}
diff --git a/geode-core/src/main/java/org/apache/geode/cache/execute/Function.java b/geode-core/src/main/java/org/apache/geode/cache/execute/Function.java
index 2c4bf34..d05cefe 100755
--- a/geode-core/src/main/java/org/apache/geode/cache/execute/Function.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/execute/Function.java
@@ -30,9 +30,8 @@ import org.apache.geode.security.ResourcePermission;
* <p>
* Even though this interface extends Serializable, functions will only be serialized if they are
* not registered. For best performance it is recommended that you implement {@link #getId()} to
- * return a non-null identifier and register your function using
- * {@link FunctionService#registerFunction(Function)} or the cache.xml <code>function</code>
- * element.
+ * return a non-null identifier and register your function using {@link
+ * FunctionService#registerFunction(Function)} or the cache.xml <code>function</code> element.
* </p>
*
* @since GemFire 6.0
@@ -72,7 +71,8 @@ public interface Function<T> extends Identifiable<String> {
void execute(FunctionContext<T> context);
/**
- * Return a unique function identifier, used to register the function with {@link FunctionService}
+ * Return a unique function identifier, used to register the function with {@link
+ * FunctionService}
*
* @return string identifying this function
* @since GemFire 6.0
@@ -84,19 +84,20 @@ public interface Function<T> extends Identifiable<String> {
/**
* <p>
* Return true to indicate to GemFire the method requires optimization for writing the targeted
- * {@link FunctionService#onRegion(org.apache.geode.cache.Region)} and any associated
- * {@linkplain Execution#withFilter(java.util.Set) routing objects}.
+ * {@link FunctionService#onRegion(org.apache.geode.cache.Region)} and any associated {@linkplain
+ * Execution#withFilter(java.util.Set) routing objects}.
* </p>
*
* <p>
- * Returning false will optimize for read behavior on the targeted
- * {@link FunctionService#onRegion(org.apache.geode.cache.Region)} and any associated
- * {@linkplain Execution#withFilter(java.util.Set) routing objects}.
+ * Returning false will optimize for read behavior on the targeted {@link
+ * FunctionService#onRegion(org.apache.geode.cache.Region)} and any associated {@linkplain
+ * Execution#withFilter(java.util.Set) routing objects}.
* </p>
*
* <p>
* This method is only consulted when Region passed to
- * FunctionService#onRegion(org.apache.geode.cache.Region) is a partitioned region
+ * FunctionService#onRegion(org.apache.geode.cache.Region)
+ * is a partitioned region
* </p>
*
* @return false if the function is read only, otherwise returns true
@@ -137,12 +138,45 @@ public interface Function<T> extends Identifiable<String> {
*
* @param regionName the region this function will be executed on. The regionName is optional and
* will only be present when the function is executed by an onRegion() executor. In other
- * cases, it will be null. This method returns permissions appropriate to the context,
- * independent of the presence of the regionName parameter.
+ * cases,
+ * it will be null. This method returns permissions appropriate to the context, independent
+ * of the
+ * presence of the regionName parameter.
* @return a collection of {@link ResourcePermission}s indicating the permissions required to
* execute the function.
*/
default Collection<ResourcePermission> getRequiredPermissions(String regionName) {
return Collections.singletonList(ResourcePermissions.DATA_WRITE);
}
+
+ /**
+ * Returns the list of ResourcePermission this function requires.
+ * <p>
+ * By default, functions require DATA:WRITE permission. If your function requires other
+ * permissions, you will need to override this method.
+ * </p>
+ * <p>
+ * Please be as specific as possible when you set the required permissions for your function e.g.
+ * if your function reads from a region, it would be good to include the region name in your
+ * permission. It's better to return "DATA:READ:regionName" as the required permission other than
+ * "DATA:READ", because the latter means only users with read permission on ALL regions can
+ * execute your function.
+ * </p>
+ * <p>
+ * All the permissions returned from this method will be ANDed together.
+ * </p>
+ *
+ * @param regionName the region this function will be executed on. The regionName is optional and
+ * will only be present when the function is executed by an onRegion() executor. In other
+ * cases,
+ * it will be null. This method returns permissions appropriate to the context, independent
+ * of the
+ * presence of the regionName parameter.
+ * @param args the arguments to the function.
+ * @return a collection of {@link ResourcePermission}s indicating the permissions required to
+ * execute the function.
+ */
+ default Collection<ResourcePermission> getRequiredPermissions(String regionName, Object args) {
+ return getRequiredPermissions(regionName);
+ }
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction.java
index 9e2c6b4..3e09bab 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction.java
@@ -113,7 +113,7 @@ public class ExecuteFunction extends BaseCommand {
FunctionStats stats = FunctionStats.getFunctionStats(functionObject.getId());
// check if the caller is authorized to do this operation on server
- functionObject.getRequiredPermissions(null).forEach(securityService::authorize);
+ functionObject.getRequiredPermissions(null, args).forEach(securityService::authorize);
AuthorizeRequest authzRequest = serverConnection.getAuthzRequest();
ExecuteFunctionOperationContext executeContext = null;
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction65.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction65.java
index ee27adc..c03e821 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction65.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction65.java
@@ -142,7 +142,7 @@ public class ExecuteFunction65 extends BaseCommand {
FunctionStats stats = FunctionStats.getFunctionStats(functionObject.getId());
// check if the caller is authorized to do this operation on server
- functionObject.getRequiredPermissions(null).forEach(securityService::authorize);
+ functionObject.getRequiredPermissions(null, args).forEach(securityService::authorize);
AuthorizeRequest authzRequest = serverConnection.getAuthzRequest();
ExecuteFunctionOperationContext executeContext = null;
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction66.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction66.java
index 0ce6454..f5d7872 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction66.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction66.java
@@ -175,7 +175,7 @@ public class ExecuteFunction66 extends BaseCommand {
FunctionStats stats = FunctionStats.getFunctionStats(functionObject.getId());
// check if the caller is authorized to do this operation on server
- functionObject.getRequiredPermissions(null).forEach(securityService::authorize);
+ functionObject.getRequiredPermissions(null, args).forEach(securityService::authorize);
AuthorizeRequest authzRequest = serverConnection.getAuthzRequest();
ExecuteFunctionOperationContext executeContext = null;
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunction.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunction.java
index 1746898..7713457 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunction.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunction.java
@@ -155,7 +155,7 @@ public class ExecuteRegionFunction extends BaseCommand {
}
// check if the caller is authorized to do this operation on server
- functionObject.getRequiredPermissions(regionName).forEach(securityService::authorize);
+ functionObject.getRequiredPermissions(regionName, args).forEach(securityService::authorize);
AuthorizeRequest authzRequest = serverConnection.getAuthzRequest();
final String functionName = functionObject.getId();
final String regionPath = region.getFullPath();
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunction61.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunction61.java
index 5769c94..60e7ebe 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunction61.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunction61.java
@@ -167,7 +167,7 @@ public class ExecuteRegionFunction61 extends BaseCommand {
functionObject = (Function) function;
}
// check if the caller is authorized to do this operation on server
- functionObject.getRequiredPermissions(regionName).forEach(securityService::authorize);
+ functionObject.getRequiredPermissions(regionName, args).forEach(securityService::authorize);
AuthorizeRequest authzRequest = serverConnection.getAuthzRequest();
final String functionName = functionObject.getId();
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunction65.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunction65.java
index 1d0d29a..ae7a7e1 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunction65.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunction65.java
@@ -189,7 +189,7 @@ public class ExecuteRegionFunction65 extends BaseCommand {
}
// check if the caller is authorized to do this operation on server
- functionObject.getRequiredPermissions(regionName).forEach(securityService::authorize);
+ functionObject.getRequiredPermissions(regionName, args).forEach(securityService::authorize);
AuthorizeRequest authzRequest = serverConnection.getAuthzRequest();
final String functionName = functionObject.getId();
final String regionPath = region.getFullPath();
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunction66.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunction66.java
index beb9128..166d470 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunction66.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunction66.java
@@ -163,7 +163,7 @@ public class ExecuteRegionFunction66 extends BaseCommand {
}
// check if the caller is authorized to do this operation on server
- functionObject.getRequiredPermissions(regionName).forEach(securityService::authorize);
+ functionObject.getRequiredPermissions(regionName, args).forEach(securityService::authorize);
ExecuteFunctionOperationContext executeContext =
getAuthorizedExecuteFunctionOperationContext(args, filter,
functionObject.optimizeForWrite(), serverConnection.getAuthzRequest(),
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunctionSingleHop.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunctionSingleHop.java
index 38a4228..793589b 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunctionSingleHop.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteRegionFunctionSingleHop.java
@@ -208,7 +208,7 @@ public class ExecuteRegionFunctionSingleHop extends BaseCommand {
}
// check if the caller is authorized to do this operation on server
- functionObject.getRequiredPermissions(regionName).forEach(securityService::authorize);
+ functionObject.getRequiredPermissions(regionName, args).forEach(securityService::authorize);
AuthorizeRequest authzRequest = serverConnection.getAuthzRequest();
final String functionName = functionObject.getId();
final String regionPath = region.getFullPath();
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/UserFunctionExecution.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/UserFunctionExecution.java
index b92ab5a..06f1647 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/UserFunctionExecution.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/UserFunctionExecution.java
@@ -117,7 +117,7 @@ public class UserFunctionExecution implements InternalFunction<Object[]> {
}
// security check
- function.getRequiredPermissions(onRegion).forEach(securityService::authorize);
+ function.getRequiredPermissions(onRegion, functionArgs).forEach(securityService::authorize);
Execution execution = null;
if (onRegion != null && onRegion.length() > 0) {
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction65Test.java b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction65Test.java
index 0ba095d..f0bb5e7 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction65Test.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction65Test.java
@@ -131,6 +131,7 @@ public class ExecuteFunction65Test {
when(this.functionObject.getId()).thenReturn(FUNCTION_ID);
doCallRealMethod().when(this.functionObject).getRequiredPermissions(any());
+ doCallRealMethod().when(this.functionObject).getRequiredPermissions(any(), any());
when(this.functionPart.getStringOrObject()).thenReturn(FUNCTION);
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction66Test.java b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction66Test.java
index fc234c2..5b8f9f5 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction66Test.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunction66Test.java
@@ -131,6 +131,7 @@ public class ExecuteFunction66Test {
when(this.functionObject.getId()).thenReturn(FUNCTION_ID);
doCallRealMethod().when(this.functionObject).getRequiredPermissions(any());
+ doCallRealMethod().when(this.functionObject).getRequiredPermissions(any(), any());
when(this.functionPart.getStringOrObject()).thenReturn(FUNCTION);
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunctionTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunctionTest.java
index d9fd4aa..3fb79f7 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunctionTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteFunctionTest.java
@@ -131,6 +131,7 @@ public class ExecuteFunctionTest {
when(this.functionObject.getId()).thenReturn(FUNCTION_ID);
doCallRealMethod().when(this.functionObject).getRequiredPermissions(any());
+ doCallRealMethod().when(this.functionObject).getRequiredPermissions(any(), any());
when(this.functionPart.getStringOrObject()).thenReturn(FUNCTION);
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/security/SecureFunctionServiceImpl.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/security/SecureFunctionServiceImpl.java
index 94d9290..f0471ba 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/security/SecureFunctionServiceImpl.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/security/SecureFunctionServiceImpl.java
@@ -43,7 +43,7 @@ public class SecureFunctionServiceImpl implements SecureFunctionService {
public List<Object> executeFunctionOnRegion(String functionID, String regionName,
Object arguments, Set<?> keyFilter) {
- Function function = authorizeAndGetFunction(regionName, functionID);
+ Function function = authorizeAndGetFunction(regionName, functionID, arguments);
Region region = getRegion(regionName);
Execution execution = FunctionService.onRegion(region);
if (keyFilter != null) {
@@ -65,7 +65,8 @@ public class SecureFunctionServiceImpl implements SecureFunctionService {
}
}
- private Function<?> authorizeAndGetFunction(String regionName, String functionID) {
+ private Function<?> authorizeAndGetFunction(String regionName, String functionID,
+ Object arguments) {
final Function<?> function = FunctionService.getFunction(functionID);
if (function == null) {
throw new IllegalArgumentException(
@@ -73,7 +74,7 @@ public class SecureFunctionServiceImpl implements SecureFunctionService {
functionID));
}
- function.getRequiredPermissions(regionName).forEach(security::authorize);
+ function.getRequiredPermissions(regionName, arguments).forEach(security::authorize);
return function;
}
@@ -81,7 +82,7 @@ public class SecureFunctionServiceImpl implements SecureFunctionService {
public List<Object> executeFunctionOnMember(String functionID, Object arguments,
List<String> memberNameList) {
- Function function = authorizeAndGetFunction(null, functionID);
+ Function function = authorizeAndGetFunction(null, functionID, arguments);
Execution execution = FunctionService.onMembers(getMemberIDs(functionID, memberNameList));
return executeFunction(execution, functionID, function, arguments);
}
@@ -89,7 +90,7 @@ public class SecureFunctionServiceImpl implements SecureFunctionService {
@Override
public List<Object> executeFunctionOnGroups(String functionID, Object arguments,
List<String> groupNameList) {
- Function function = authorizeAndGetFunction(null, functionID);
+ Function function = authorizeAndGetFunction(null, functionID, arguments);
Execution execution = FunctionService.onMember(groupNameList.toArray(new String[0]));
return executeFunction(execution, functionID, function, arguments);
}
diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/security/SecureFunctionServiceImplTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/security/SecureFunctionServiceImplTest.java
index 10c631b..a36c114 100644
--- a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/security/SecureFunctionServiceImplTest.java
+++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/security/SecureFunctionServiceImplTest.java
@@ -71,7 +71,7 @@ public class SecureFunctionServiceImplTest {
@Test
public void executeFunctionOnRegionWithoutAuthorization() throws Exception {
- when(function.getRequiredPermissions(REGION))
+ when(function.getRequiredPermissions(REGION, null))
.thenReturn(Collections.singleton(new ResourcePermission(CLUSTER, WRITE, REGION, ALL)));
assertThatThrownBy(
() -> functionService.executeFunctionOnRegion(FUNCTION_ID, REGION, null, null))
@@ -80,7 +80,7 @@ public class SecureFunctionServiceImplTest {
@Test
public void executeFunctionOnMemberWithoutAuthorization() throws Exception {
- when(function.getRequiredPermissions(null))
+ when(function.getRequiredPermissions(null, null))
.thenReturn(Collections.singleton(new ResourcePermission(CLUSTER, WRITE, REGION, ALL)));
assertThatThrownBy(
() -> functionService.executeFunctionOnMember(FUNCTION_ID, null, Arrays.asList("member")))
@@ -89,7 +89,7 @@ public class SecureFunctionServiceImplTest {
@Test
public void executeFunctionOnGroupsWithoutAuthorization() throws Exception {
- when(function.getRequiredPermissions(null))
+ when(function.getRequiredPermissions(null, null))
.thenReturn(Collections.singleton(new ResourcePermission(CLUSTER, WRITE, REGION, ALL)));
assertThatThrownBy(
() -> functionService.executeFunctionOnGroups(FUNCTION_ID, null, Arrays.asList("group")))
diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java
index 1ade61d..a8faf59 100644
--- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java
+++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java
@@ -118,7 +118,6 @@ public class FunctionAccessController extends AbstractBaseController {
* @param filter list of keys which the function will use to determine on which node to execute
* the function.
* @param argsInBody function argument as a JSON document
- *
* @return result as a JSON document
*/
@RequestMapping(method = RequestMethod.POST, value = "/{functionId:.+}",
@@ -148,8 +147,14 @@ public class FunctionAccessController extends AbstractBaseController {
String.format("The function %s is not registered.", functionId));
}
+ Object[] args = null;
+ if (argsInBody != null) {
+ args = jsonToObjectArray(argsInBody);
+ }
+
// check for required permissions of the function
- Collection<ResourcePermission> requiredPermissions = function.getRequiredPermissions(region);
+ Collection<ResourcePermission> requiredPermissions =
+ function.getRequiredPermissions(region, args);
for (ResourcePermission requiredPermission : requiredPermissions) {
securityService.authorize(requiredPermission);
}
@@ -213,9 +218,7 @@ public class FunctionAccessController extends AbstractBaseController {
final ResultCollector<?, ?> results;
try {
- if (argsInBody != null) {
- Object[] args = jsonToObjectArray(argsInBody);
-
+ if (args != null) {
// execute function with specified arguments
if (args.length == 1) {
results = execution.setArguments(args[0]).execute(functionId);