Skip to content

Commit 00be4f9

Browse files
jinmeiliaojinmeiliao
authored
GEODE-3974: function security improvement (#1287)
* GEODE-3974: function security improvement * function executed on a local member does not log out user accidentally * Mark some internal functions as InternalEntity * test refactor
1 parent 081aa75 commit 00be4f9

File tree

22 files changed

+315
-753
lines changed

22 files changed

+315
-753
lines changed

‎extensions/geode-modules/src/main/java/org/apache/geode/modules/util/CreateRegionFunction.java

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
import org.apache.geode.cache.Region;
3434
import org.apache.geode.cache.RegionAttributes;
3535
import org.apache.geode.cache.Scope;
36-
import org.apache.geode.cache.client.ClientCache;
3736
import org.apache.geode.cache.execute.Function;
3837
import org.apache.geode.cache.execute.FunctionContext;
3938
import org.apache.geode.cache.partition.PartitionRegionHelper;
@@ -64,19 +63,10 @@ public class CreateRegionFunction implements Function, Declarable, DataSerializa
6463
"__regionConfigurationMetadata";
6564

6665
public CreateRegionFunction() {
67-
this(CacheFactory.getAnyInstance());
68-
}
69-
70-
public CreateRegionFunction(Cache cache) {
71-
this.cache = cache;
66+
this.cache = CacheFactory.getAnyInstance();
7267
this.regionConfigurationsRegion = createRegionConfigurationMetadataRegion();
7368
}
7469

75-
public CreateRegionFunction(ClientCache cache) {
76-
this.cache = null;
77-
this.regionConfigurationsRegion = null;
78-
}
79-
8070
public void execute(FunctionContext context) {
8171
RegionConfiguration configuration = (RegionConfiguration) context.getArguments();
8272
if (this.cache.getLogger().fineEnabled()) {

‎extensions/geode-modules/src/main/java/org/apache/geode/modules/util/TouchPartitionedRegionEntriesFunction.java

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424

2525
import org.apache.geode.DataSerializable;
2626
import org.apache.geode.cache.Cache;
27-
import org.apache.geode.cache.CacheFactory;
2827
import org.apache.geode.cache.Declarable;
2928
import org.apache.geode.cache.Region;
3029
import org.apache.geode.cache.execute.Function;
@@ -42,31 +41,22 @@ public class TouchPartitionedRegionEntriesFunction
4241

4342
private static final long serialVersionUID = -3700389655056961153L;
4443

45-
private final Cache cache;
46-
4744
public static final String ID = "touch-partitioned-region-entries";
4845

49-
public TouchPartitionedRegionEntriesFunction() {
50-
this(CacheFactory.getAnyInstance());
51-
}
52-
53-
public TouchPartitionedRegionEntriesFunction(Cache cache) {
54-
this.cache = cache;
55-
}
56-
5746
@SuppressWarnings("unchecked")
5847
public void execute(FunctionContext context) {
5948
RegionFunctionContext rfc = (RegionFunctionContext) context;
6049
Set<String> keys = (Set<String>) rfc.getFilter();
6150

51+
Cache cache = context.getCache();
6252
// Get local (primary) data for the context
6353
Region primaryDataSet = PartitionRegionHelper.getLocalDataForContext(rfc);
6454

65-
if (this.cache.getLogger().fineEnabled()) {
55+
if (cache.getLogger().fineEnabled()) {
6656
StringBuilder builder = new StringBuilder();
6757
builder.append("Function ").append(ID).append(" received request to touch ")
6858
.append(primaryDataSet.getFullPath()).append("->").append(keys);
69-
this.cache.getLogger().fine(builder.toString());
59+
cache.getLogger().fine(builder.toString());
7060
}
7161

7262
// Retrieve each value to update the lastAccessedTime.

‎extensions/geode-modules/src/main/java/org/apache/geode/modules/util/TouchReplicatedRegionEntriesFunction.java

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@
2424

2525
import org.apache.geode.DataSerializable;
2626
import org.apache.geode.cache.Cache;
27-
import org.apache.geode.cache.CacheFactory;
2827
import org.apache.geode.cache.Declarable;
2928
import org.apache.geode.cache.Region;
3029
import org.apache.geode.cache.execute.Function;
3130
import org.apache.geode.cache.execute.FunctionContext;
31+
import org.apache.geode.management.internal.security.ResourcePermissions;
3232
import org.apache.geode.security.ResourcePermission;
3333

3434
/**
@@ -41,31 +41,22 @@ public class TouchReplicatedRegionEntriesFunction
4141

4242
private static final long serialVersionUID = -7424895036162243564L;
4343

44-
private final Cache cache;
45-
4644
public static final String ID = "touch-replicated-region-entries";
4745

48-
public TouchReplicatedRegionEntriesFunction() {
49-
this(CacheFactory.getAnyInstance());
50-
}
51-
52-
public TouchReplicatedRegionEntriesFunction(Cache cache) {
53-
this.cache = cache;
54-
}
55-
5646
public void execute(FunctionContext context) {
5747
Object[] arguments = (Object[]) context.getArguments();
48+
Cache cache = context.getCache();
5849
String regionName = (String) arguments[0];
5950
Set<String> keys = (Set<String>) arguments[1];
60-
if (this.cache.getLogger().fineEnabled()) {
51+
if (cache.getLogger().fineEnabled()) {
6152
StringBuilder builder = new StringBuilder();
6253
builder.append("Function ").append(ID).append(" received request to touch ")
6354
.append(regionName).append("->").append(keys);
64-
this.cache.getLogger().fine(builder.toString());
55+
cache.getLogger().fine(builder.toString());
6556
}
6657

6758
// Retrieve the appropriate Region and value to update the lastAccessedTime
68-
Region region = this.cache.getRegion(regionName);
59+
Region region = cache.getRegion(regionName);
6960
if (region != null) {
7061
region.getAll(keys);
7162
}
@@ -75,9 +66,10 @@ public void execute(FunctionContext context) {
7566
}
7667

7768
@Override
69+
// the actual regionName used in the function body is passed in as an function arugment,
70+
// this regionName is not really used in function. Hence requiring DATA:READ on all regions
7871
public Collection<ResourcePermission> getRequiredPermissions(String regionName) {
79-
return Collections.singletonList(new ResourcePermission(ResourcePermission.Resource.DATA,
80-
ResourcePermission.Operation.READ, regionName));
72+
return Collections.singletonList(ResourcePermissions.DATA_READ);
8173
}
8274

8375
public String getId() {

‎extensions/geode-modules/src/test/java/org/apache/geode/modules/util/ModuleFunctionsSecurityTest.java

Lines changed: 22 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,17 @@
1515

1616
package org.apache.geode.modules.util;
1717

18+
import java.util.HashMap;
19+
import java.util.Map;
20+
1821
import org.junit.BeforeClass;
1922
import org.junit.ClassRule;
2023
import org.junit.Rule;
2124
import org.junit.Test;
2225
import org.junit.experimental.categories.Category;
2326

2427
import org.apache.geode.cache.RegionShortcut;
28+
import org.apache.geode.cache.execute.Function;
2529
import org.apache.geode.cache.execute.FunctionService;
2630
import org.apache.geode.examples.SimpleSecurityManager;
2731
import org.apache.geode.test.junit.categories.IntegrationTest;
@@ -38,66 +42,34 @@ public class ModuleFunctionsSecurityTest {
3842
@ClassRule
3943
public static ServerStarterRule server =
4044
new ServerStarterRule().withJMXManager().withSecurityManager(SimpleSecurityManager.class)
41-
.withRegion(RegionShortcut.REPLICATE, "REPLICATE_1")
42-
.withRegion(RegionShortcut.PARTITION, "PARTITION_1").withAutoStart();
45+
.withRegion(RegionShortcut.REPLICATE, "AuthRegion").withAutoStart();
4346

4447
@Rule
4548
public GfshCommandRule gfsh =
4649
new GfshCommandRule(server::getJmxPort, GfshCommandRule.PortType.jmxManager);
4750

51+
private static Map<Function, String> functionStringMap = new HashMap<>();
52+
4853
@BeforeClass
4954
public static void setupClass() {
50-
FunctionService.registerFunction(new BootstrappingFunction());
51-
FunctionService.registerFunction(new CreateRegionFunction());
52-
FunctionService.registerFunction(new RegionSizeFunction());
53-
FunctionService.registerFunction(new TouchPartitionedRegionEntriesFunction());
54-
FunctionService.registerFunction(new TouchReplicatedRegionEntriesFunction());
55-
}
55+
functionStringMap.put(new BootstrappingFunction(), "CLUSTER:MANAGE");
56+
functionStringMap.put(new CreateRegionFunction(), "DATA:MANAGE");
57+
functionStringMap.put(new RegionSizeFunction(), "DATA:READ:AuthRegion");
58+
functionStringMap.put(new TouchPartitionedRegionEntriesFunction(), "DATA:READ:AuthRegion");
59+
functionStringMap.put(new TouchReplicatedRegionEntriesFunction(), "DATA:READ");
5660

57-
@Test
58-
@ConnectionConfiguration(user = "dataWrite", password = "dataWrite")
59-
public void testInvalidPermissionsForBootstrappingFunction() throws Exception {
60-
gfsh.executeAndAssertThat("execute function --id=" + BootstrappingFunction.ID)
61-
.tableHasColumnWithExactValuesInAnyOrder(RESULT_HEADER,
62-
"Exception: dataWrite not authorized for CLUSTER:MANAGE")
63-
.statusIsError();
64-
}
65-
66-
@Test
67-
@ConnectionConfiguration(user = "dataWrite", password = "dataWrite")
68-
public void testInvalidPermissionsForCreateRegionFunction() throws Exception {
69-
gfsh.executeAndAssertThat("execute function --id=" + CreateRegionFunction.ID)
70-
.tableHasColumnWithExactValuesInAnyOrder(RESULT_HEADER,
71-
"Exception: dataWrite not authorized for DATA:MANAGE")
72-
.statusIsError();
73-
}
74-
75-
@Test
76-
@ConnectionConfiguration(user = "dataWrite", password = "dataWrite")
77-
public void testInvalidPermissionsForRegionSizeFunction() throws Exception {
78-
gfsh.executeAndAssertThat("execute function --region=REPLICATE_1 --id=" + RegionSizeFunction.ID)
79-
.tableHasColumnWithExactValuesInAnyOrder(RESULT_HEADER,
80-
"Exception: dataWrite not authorized for DATA:READ:REPLICATE_1")
81-
.statusIsError();
82-
}
83-
84-
@Test
85-
@ConnectionConfiguration(user = "dataWrite", password = "dataWrite")
86-
public void testInvalidPermissionsForTouchPartitionedRegionEntriesFunction() throws Exception {
87-
gfsh.executeAndAssertThat(
88-
"execute function --region=PARTITION_1 --id=" + TouchPartitionedRegionEntriesFunction.ID)
89-
.tableHasColumnWithExactValuesInAnyOrder(RESULT_HEADER,
90-
"Exception: dataWrite not authorized for DATA:READ:PARTITION_1")
91-
.statusIsError();
61+
functionStringMap.keySet().forEach(FunctionService::registerFunction);
9262
}
9363

9464
@Test
95-
@ConnectionConfiguration(user = "dataWrite", password = "dataWrite")
96-
public void testInvalidPermissionsForTouchReplicatedRegionEntriesFunction() throws Exception {
97-
gfsh.executeAndAssertThat(
98-
"execute function --region=REPLICATE_1 --id=" + TouchReplicatedRegionEntriesFunction.ID)
99-
.tableHasColumnWithExactValuesInAnyOrder(RESULT_HEADER,
100-
"Exception: dataWrite not authorized for DATA:READ:REPLICATE_1")
101-
.statusIsError();
65+
@ConnectionConfiguration(user = "user", password = "user")
66+
public void functionRequireExpectedPermission() throws Exception {
67+
functionStringMap.entrySet().stream().forEach(entry -> {
68+
Function function = entry.getKey();
69+
String permission = entry.getValue();
70+
gfsh.executeAndAssertThat("execute function --region=AuthRegion --id=" + function.getId())
71+
.tableHasRowCount(RESULT_HEADER, 1)
72+
.tableHasColumnWithValuesContaining(RESULT_HEADER, permission).statusIsError();
73+
});
10274
}
10375
}

‎geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/JDBCConnectorFunctionsSecurityTest.java

Lines changed: 27 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515

1616
package org.apache.geode.connectors.jdbc.internal.cli;
1717

18+
import java.util.HashMap;
19+
import java.util.Map;
20+
1821
import org.junit.BeforeClass;
1922
import org.junit.ClassRule;
2023
import org.junit.Rule;
@@ -48,20 +51,6 @@ CliFunctionResult getFunctionResult(JdbcConnectorService service,
4851

4952
@Category({IntegrationTest.class, SecurityException.class})
5053
public class JDBCConnectorFunctionsSecurityTest {
51-
52-
private static Function alterConnectionFunction = new AlterConnectionFunction();
53-
private static Function alterMappingFunction = new AlterMappingFunction();
54-
private static Function createConnectionFunction = new CreateConnectionFunction();
55-
private static Function createMappingFunction = new CreateMappingFunction();
56-
private static Function describeConnectionFunction = new DescribeConnectionFunction();
57-
private static Function describeMappingFunction = new DescribeMappingFunction();
58-
private static Function destroyConnectionFunction = new DestroyConnectionFunction();
59-
private static Function destroyMappingFunction = new DestroyMappingFunction();
60-
private static Function listConnectionFunction = new ListConnectionFunction();
61-
private static Function listMappingFunction = new ListMappingFunction();
62-
private static Function inheritsDefaultPermissionsFunction =
63-
new InheritsDefaultPermissionsJDBCFunction();
64-
6554
@ClassRule
6655
public static ServerStarterRule server = new ServerStarterRule().withJMXManager()
6756
.withSecurityManager(SimpleSecurityManager.class).withAutoStart();
@@ -70,81 +59,35 @@ public class JDBCConnectorFunctionsSecurityTest {
7059
public GfshCommandRule gfsh =
7160
new GfshCommandRule(server::getJmxPort, GfshCommandRule.PortType.jmxManager);
7261

62+
private static Map<Function, String> functionStringMap = new HashMap<>();
63+
7364
@BeforeClass
7465
public static void setupClass() {
75-
FunctionService.registerFunction(alterConnectionFunction);
76-
FunctionService.registerFunction(alterMappingFunction);
77-
FunctionService.registerFunction(createConnectionFunction);
78-
FunctionService.registerFunction(createMappingFunction);
79-
FunctionService.registerFunction(describeConnectionFunction);
80-
FunctionService.registerFunction(describeMappingFunction);
81-
FunctionService.registerFunction(destroyConnectionFunction);
82-
FunctionService.registerFunction(destroyMappingFunction);
83-
FunctionService.registerFunction(listConnectionFunction);
84-
FunctionService.registerFunction(listMappingFunction);
85-
FunctionService.registerFunction(inheritsDefaultPermissionsFunction);
86-
}
87-
88-
@Test
89-
@ConnectionConfiguration(user = "dataWrite", password = "dataWrite")
90-
public void testInvalidPermissionsForAlterConnectionFunction() {
91-
gfsh.executeAndAssertThat("execute function --id=" + alterConnectionFunction.getId())
92-
.containsOutput("not authorized for CLUSTER:MANAGE").statusIsError();
66+
functionStringMap.put(new AlterConnectionFunction(), "CLUSTER:MANAGE");
67+
functionStringMap.put(new AlterMappingFunction(), "CLUSTER:MANAGE");
68+
functionStringMap.put(new CreateConnectionFunction(), "CLUSTER:MANAGE");
69+
functionStringMap.put(new CreateMappingFunction(), "CLUSTER:MANAGE");
70+
functionStringMap.put(new DescribeConnectionFunction(), "CLUSTER:READ");
71+
functionStringMap.put(new DescribeMappingFunction(), "CLUSTER:READ");
72+
functionStringMap.put(new DestroyConnectionFunction(), "CLUSTER:MANAGE");
73+
functionStringMap.put(new DestroyMappingFunction(), "CLUSTER:MANAGE");
74+
functionStringMap.put(new ListConnectionFunction(), "CLUSTER:READ");
75+
functionStringMap.put(new ListMappingFunction(), "CLUSTER:READ");
76+
functionStringMap.put(new InheritsDefaultPermissionsJDBCFunction(), "CLUSTER:READ");
77+
functionStringMap.keySet().forEach(FunctionService::registerFunction);
9378
}
9479

95-
@Test
96-
@ConnectionConfiguration(user = "dataWrite", password = "dataWrite")
97-
public void testInvalidPermissionsForAlterMappingFunction() {
98-
gfsh.executeAndAssertThat("execute function --id=" + alterMappingFunction.getId())
99-
.containsOutput("not authorized for CLUSTER:MANAGE").statusIsError();
100-
}
101-
102-
@Test
103-
@ConnectionConfiguration(user = "dataWrite", password = "dataWrite")
104-
public void testInvalidPermissionsForCreateConnectionFunction() {
105-
gfsh.executeAndAssertThat("execute function --id=" + createConnectionFunction.getId())
106-
.containsOutput("not authorized for CLUSTER:MANAGE").statusIsError();
107-
}
108-
109-
@Test
110-
@ConnectionConfiguration(user = "dataWrite", password = "dataWrite")
111-
public void testInvalidPermissionsForCreateMappingFunction() {
112-
gfsh.executeAndAssertThat("execute function --id=" + createMappingFunction.getId())
113-
.containsOutput("not authorized for CLUSTER:MANAGE").statusIsError();
114-
}
115-
116-
@Test
117-
@ConnectionConfiguration(user = "dataWrite", password = "dataWrite")
118-
public void testInvalidPermissionsForDescribeConnectionFunction() {
119-
gfsh.executeAndAssertThat("execute function --id=" + describeConnectionFunction.getId())
120-
.containsOutput("not authorized for CLUSTER:READ").statusIsError();
121-
}
122-
123-
@Test
124-
@ConnectionConfiguration(user = "dataWrite", password = "dataWrite")
125-
public void testInvalidPermissionsForDescribeMappingFunction() {
126-
gfsh.executeAndAssertThat("execute function --id=" + describeMappingFunction.getId())
127-
.containsOutput("not authorized for CLUSTER:READ").statusIsError();
128-
}
129-
130-
@Test
131-
@ConnectionConfiguration(user = "dataWrite", password = "dataWrite")
132-
public void testInvalidPermissionsForDestroyConnectionFunction() {
133-
gfsh.executeAndAssertThat("execute function --id=" + destroyConnectionFunction.getId())
134-
.containsOutput("not authorized for CLUSTER:MANAGE").statusIsError();
135-
}
136-
137-
@Test
138-
@ConnectionConfiguration(user = "dataWrite", password = "dataWrite")
139-
public void testInvalidPermissionsForDestroyMappingFunction() {
140-
gfsh.executeAndAssertThat("execute function --id=" + destroyMappingFunction.getId())
141-
.containsOutput("not authorized for CLUSTER:MANAGE").statusIsError();
142-
}
14380

14481
@Test
145-
@ConnectionConfiguration(user = "dataWrite", password = "dataWrite")
146-
public void testInvalidPermissionsForFunctionInheritingDefaultPermissions() {
147-
gfsh.executeAndAssertThat("execute function --id=" + inheritsDefaultPermissionsFunction.getId())
148-
.containsOutput("not authorized for CLUSTER:READ").statusIsError();
82+
@ConnectionConfiguration(user = "user", password = "user")
83+
public void functionRequireExpectedPermission() throws Exception {
84+
functionStringMap.entrySet().stream().forEach(entry -> {
85+
Function function = entry.getKey();
86+
String permission = entry.getValue();
87+
gfsh.executeAndAssertThat("execute function --id=" + function.getId())
88+
.tableHasRowCount("Function Execution Result", 1)
89+
.tableHasColumnWithValuesContaining("Function Execution Result", permission)
90+
.statusIsError();
91+
});
14992
}
15093
}

0 commit comments

Comments
 (0)