[
https://issues.apache.org/jira/browse/GEODE-3974?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16329404#comment-16329404
]
ASF GitHub Bot commented on GEODE-3974:
---------------------------------------
PurelyApplied commented on a change in pull request #1287: GEODE-3974: function
security improvement
URL: https://github.com/apache/geode/pull/1287#discussion_r162167928
##########
File path:
geode-lucene/src/test/java/org/apache/geode/cache/lucene/test/LuceneFunctionSecurityTest.java
##########
@@ -105,366 +67,45 @@
@ClassRule
public static ServerStarterRule server =
new
ServerStarterRule().withJMXManager().withSecurityManager(SimpleSecurityManager.class)
- .withRegion(RegionShortcut.PARTITION, regionName).withAutoStart();
+ .withRegion(RegionShortcut.PARTITION, "testRegion").withAutoStart();
@Rule
public GfshCommandRule gfsh =
new GfshCommandRule(server::getJmxPort,
GfshCommandRule.PortType.jmxManager);
+ private static Map<Function, String> functionStringMap = new HashMap<>();
+
@BeforeClass
public static void setupClass() {
- FunctionService.registerFunction(luceneCreateIndexFunction);
- FunctionService.registerFunction(luceneDescribeIndexFunction);
- FunctionService.registerFunction(luceneDestroyIndexFunction);
- FunctionService.registerFunction(luceneListIndexFunction);
- FunctionService.registerFunction(luceneSearchIndexFunction);
- FunctionService.registerFunction(dumpDirectoryFiles);
- FunctionService.registerFunction(luceneQueryFunction);
- FunctionService.registerFunction(waitUntilFlushedFunction);
- FunctionService.registerFunction(luceneGetPageFunction);
- }
-
- /* Command authorized tests */
- @Test
- @ConnectionConfiguration(user = "clusterManageLucene", password =
"clusterManageLucene")
- public void testValidPermissionsForLuceneCreateIndexFunction() {
- Function thisFunction = luceneCreateIndexFunction;
-
- gfsh.executeAndAssertThat("execute function --id=" + thisFunction.getId())
- .doesNotContainOutput("not authorized for").statusIsSuccess();
- }
-
- @Test
- @ConnectionConfiguration(user = "clusterReadLucene", password =
"clusterReadLucene")
- public void testValidPermissionsForLuceneDescribeIndexFunction() {
- Function thisFunction = luceneDescribeIndexFunction;
-
- gfsh.executeAndAssertThat("execute function --id=" + thisFunction.getId())
- .doesNotContainOutput("not authorized for").statusIsSuccess();
- }
-
- @Test
- @ConnectionConfiguration(user = "clusterManageLucene", password =
"clusterManageLucene")
- public void testValidPermissionsForLuceneDestroyIndexFunction() {
- Function thisFunction = luceneDestroyIndexFunction;
-
- gfsh.executeAndAssertThat("execute function --id=" + thisFunction.getId())
- .doesNotContainOutput("not authorized for").statusIsSuccess();
- }
-
- @Test
- @ConnectionConfiguration(user = "clusterReadLucene", password =
"clusterReadLucene")
- public void testValidPermissionsForLuceneListIndexFunction() {
- Function thisFunction = luceneListIndexFunction;
-
- gfsh.executeAndAssertThat("execute function --id=" + thisFunction.getId())
- .doesNotContainOutput("not authorized for").statusIsSuccess();
- }
-
- @Test
- @ConnectionConfiguration(user = "dataReadThis_test_region,clusterManage",
- password = "dataReadThis_test_region,clusterManage")
- public void testValidPermissionsForDumpDirectoryFilesWithRegionParameter() {
- Function thisFunction = dumpDirectoryFiles;
-
- gfsh.executeAndAssertThat(
- "execute function --region=" + regionName + " --id=" +
thisFunction.getId())
- .doesNotContainOutput("not authorized for").statusIsSuccess();
- }
-
- @Test
- @ConnectionConfiguration(user = "dataRead,clusterManage", password =
"dataRead,clusterManage")
- public void
testValidPermissionsForDumpDirectoryFilesWithoutRegionParameter() {
- Function thisFunction = dumpDirectoryFiles;
-
- gfsh.executeAndAssertThat("execute function --id=" + thisFunction.getId())
- .doesNotContainOutput("not authorized for").statusIsSuccess();
- }
-
-
- @Test
- @ConnectionConfiguration(user = "dataRead", password = "dataRead")
- public void
testValidPermissionsForLuceneSearchIndexFunctionWithoutRegionParameter() {
- Function thisFunction = luceneSearchIndexFunction;
-
- gfsh.executeAndAssertThat("execute function --id=" + thisFunction.getId())
- .doesNotContainOutput("not authorized for").statusIsSuccess();
- }
-
- @Test
- @ConnectionConfiguration(user = "dataReadThis_test_region", password =
"dataReadThis_test_region")
- public void
testValidPermissionsForLuceneSearchIndexFunctionWithRegionParameter() {
- Function thisFunction = luceneSearchIndexFunction;
-
- gfsh.executeAndAssertThat(
- "execute function --region=" + regionName + " --id=" +
thisFunction.getId())
- .doesNotContainOutput("not authorized for").statusIsSuccess();
- }
-
- @Test
- @ConnectionConfiguration(user = "dataRead", password = "dataRead")
- public void
testValidPermissionsForLuceneQueryFunctionWithoutRegionParameter() {
- Function thisFunction = luceneQueryFunction;
-
- gfsh.executeAndAssertThat("execute function --id=" + thisFunction.getId())
- .doesNotContainOutput("not authorized for").statusIsSuccess();
- }
-
- @Test
- @ConnectionConfiguration(user = "dataReadThis_test_region", password =
"dataReadThis_test_region")
- public void testValidPermissionsForLuceneQueryFunctionWithRegionParameter() {
- Function thisFunction = luceneQueryFunction;
-
- gfsh.executeAndAssertThat(
- "execute function --region=" + regionName + " --id=" +
thisFunction.getId())
- .doesNotContainOutput("not authorized for").statusIsSuccess();
- }
-
- @Test
- @ConnectionConfiguration(user = "dataRead", password = "dataRead")
- public void
testValidPermissionsForWaitUntilFlushedFunctionWithoutRegionParameter() {
- Function thisFunction = waitUntilFlushedFunction;
-
- gfsh.executeAndAssertThat("execute function --id=" + thisFunction.getId())
- .doesNotContainOutput("not authorized for").statusIsSuccess();
- }
-
- @Test
- @ConnectionConfiguration(user = "dataReadThis_test_region", password =
"dataReadThis_test_region")
- public void
testValidPermissionsForWaitUntilFlushedFunctionWithRegionParameter() {
- Function thisFunction = waitUntilFlushedFunction;
-
- gfsh.executeAndAssertThat(
- "execute function --region=" + regionName + " --id=" +
thisFunction.getId())
- .doesNotContainOutput("not authorized for").statusIsSuccess();
- }
-
- @Test
- @ConnectionConfiguration(user = "dataRead", password = "dataRead")
- public void
testValidPermissionsForLuceneGetPageFunctionWithoutRegionParameter() {
- Function thisFunction = luceneGetPageFunction;
-
- gfsh.executeAndAssertThat("execute function --id=" + thisFunction.getId())
- .doesNotContainOutput("not authorized for").statusIsSuccess();
- }
-
- @Test
- @ConnectionConfiguration(user = "dataReadThis_test_region", password =
"dataReadThis_test_region")
- public void
testValidPermissionsForLuceneGetPageFunctionWithRegionParameter() {
- Function thisFunction = luceneGetPageFunction;
-
- gfsh.executeAndAssertThat(
- "execute function --region=" + regionName + " --id=" +
thisFunction.getId())
- .doesNotContainOutput("not authorized for").statusIsSuccess();
- }
-
-
- /* Command refused tests */
- @Test
- @ConnectionConfiguration(user = "noPermissions", password = "noPermissions")
- public void testInvalidPermissionsForLuceneCreateIndexFunction() {
- Function thisFunction = luceneCreateIndexFunction;
- ResourcePermission thisRequiredPermission = CLUSTER_MANAGE_LUCENE;
+ functionStringMap.put(new LuceneCreateIndexFunction(),
"CLUSTER:MANAGE:LUCENE");
+ functionStringMap.put(new LuceneDescribeIndexFunction(),
"CLUSTER:READ:LUCENE");
+ functionStringMap.put(new LuceneDestroyIndexFunction(),
"CLUSTER:MANAGE:LUCENE");
+ functionStringMap.put(new LuceneListIndexFunction(),
"CLUSTER:READ:LUCENE");
+ functionStringMap.put(new LuceneSearchIndexFunction(),
"DATA:READ:testRegion");
+ functionStringMap.put(new LuceneQueryFunction(), "DATA:READ:testRegion");
+ functionStringMap.put(new WaitUntilFlushedFunction(),
"DATA:READ:testRegion");
+ functionStringMap.put(new LuceneGetPageFunction(), "DATA:READ:testRegion");
- gfsh.executeAndAssertThat("execute function --id=" + thisFunction.getId())
- .containsOutput("not authorized for " +
thisRequiredPermission.toString()).statusIsError();
+ functionStringMap.keySet().forEach(FunctionService::registerFunction);
}
- @Test
- @ConnectionConfiguration(user = "noPermissions", password = "noPermissions")
- public void testInvalidPermissionsForLuceneDescribeIndexFunction() {
- Function thisFunction = luceneDescribeIndexFunction;
- ResourcePermission thisRequiredPermission = CLUSTER_READ_LUCENE;
-
- gfsh.executeAndAssertThat("execute function --id=" + thisFunction.getId())
- .containsOutput("not authorized for " +
thisRequiredPermission.toString()).statusIsError();
- }
-
- @Test
- @ConnectionConfiguration(user = "noPermissions", password = "noPermissions")
- public void testInvalidPermissionsForLuceneDestroyIndexFunction() {
- Function thisFunction = luceneDestroyIndexFunction;
- ResourcePermission thisRequiredPermission = CLUSTER_MANAGE_LUCENE;
-
- gfsh.executeAndAssertThat("execute function --id=" + thisFunction.getId())
- .containsOutput("not authorized for " +
thisRequiredPermission.toString()).statusIsError();
- }
-
- @Test
- @ConnectionConfiguration(user = "noPermissions", password = "noPermissions")
- public void testInvalidPermissionsForLuceneListIndexFunction() {
- Function thisFunction = luceneListIndexFunction;
- ResourcePermission thisRequiredPermission = CLUSTER_READ_LUCENE;
-
- gfsh.executeAndAssertThat("execute function --id=" + thisFunction.getId())
- .containsOutput("not authorized for " +
thisRequiredPermission.toString()).statusIsError();
- }
-
- @Test
- @ConnectionConfiguration(user = "noPermissions", password = "noPermissions")
- public void
testInvalidPermissionsForDumpDirectoryFilesWithoutRegionParameter_noPermission()
- throws Exception {
- Function thisFunction = dumpDirectoryFiles;
-
- Predicate<String> notAuthForDataRead =
- s -> s.contains("not authorized for " + DATA_READ.toString());
- Predicate<String> notAuthForClusterManage =
- s -> s.contains("not authorized for " + CLUSTER_MANAGE.toString());
- Predicate<String> notAuthForSomePermission =
- s -> notAuthForDataRead.test(s) || notAuthForClusterManage.test(s);
-
- String output = gfsh.execute("execute function --id=" +
thisFunction.getId());
-
- Condition<String> containsSomeAuthFailure = new
Condition<>(notAuthForSomePermission,
- "not authorized for for [DATA:MANAGE|CLUSTER:MANAGE]", output);
- assertThat(output).has(containsSomeAuthFailure);
- }
-
- @Test
- @ConnectionConfiguration(user = "dataRead", password = "dataRead")
- public void
testInvalidPermissionsForDumpDirectoryFilesWithoutRegionParameter_withDataRead()
{
- Function thisFunction = dumpDirectoryFiles;
- ResourcePermission thisMissingPermission = CLUSTER_MANAGE;
-
- gfsh.executeAndAssertThat("execute function --id=" + thisFunction.getId())
- .containsOutput("not authorized for " +
thisMissingPermission.toString()).statusIsError();
- }
-
- @Test
- @ConnectionConfiguration(user = "clusterManage", password = "clusterManage")
- public void
testInvalidPermissionsForDumpDirectoryFilesWithoutRegionParameter_withClusterManage()
{
- Function thisFunction = dumpDirectoryFiles;
- ResourcePermission thisMissingPermission = DATA_READ;
-
- gfsh.executeAndAssertThat("execute function --id=" + thisFunction.getId())
- .containsOutput("not authorized for " +
thisMissingPermission.toString()).statusIsError();
- }
-
- @Test
- @ConnectionConfiguration(user = "noPermissions", password = "noPermissions")
- public void
testInvalidPermissionsForDumpDirectoryFilesWithRegionParameter_noPermission()
- throws Exception {
- Function thisFunction = dumpDirectoryFiles;
-
- Predicate<String> notAuthForDataReadRegion =
- s -> s.contains("not authorized for " + DATA_READ_REGION.toString());
- Predicate<String> notAuthForClusterManage =
- s -> s.contains("not authorized for " + CLUSTER_MANAGE.toString());
- Predicate<String> notAuthForSomePermission =
- s -> notAuthForDataReadRegion.test(s) ||
notAuthForClusterManage.test(s);
-
- String output =
- gfsh.execute("execute function --region=" + regionName + " --id=" +
thisFunction.getId());
-
- Condition<String> containsSomeAuthFailure =
- new Condition<>(notAuthForSomePermission, "D:R or C:M:L auth failure",
output);
- assertThat(output).has(containsSomeAuthFailure);
- }
-
- @Test
- @ConnectionConfiguration(user = "dataReadThis_test_region", password =
"dataReadThis_test_region")
- public void
testInvalidPermissionsForDumpDirectoryFilesWithRegionParameter_withDataReadRegion()
{
- Function thisFunction = dumpDirectoryFiles;
- ResourcePermission thisMissingPermission = CLUSTER_MANAGE;
-
- gfsh.executeAndAssertThat(
- "execute function --region=" + regionName + " --id=" +
thisFunction.getId())
- .containsOutput("not authorized for " +
thisMissingPermission.toString()).statusIsError();
- }
@Test
- @ConnectionConfiguration(user = "clusterManage", password = "clusterManage")
- public void
testInvalidPermissionsForDumpDirectoryFilesWithRegionParameter_withClusterManage()
{
- Function thisFunction = dumpDirectoryFiles;
- ResourcePermission thisMissingPermission = DATA_READ;
-
- gfsh.executeAndAssertThat(
- "execute function --region=" + regionName + " --id=" +
thisFunction.getId())
- .containsOutput("not authorized for " +
thisMissingPermission.toString()).statusIsError();
+ @ConnectionConfiguration(user = "user", password = "user")
+ public void functionRequireExpectedPermission() throws Exception {
Review comment:
I understand that the testing of the security service itself is rather
beyond the scope of this test class, but do we have any positive-tests for
permissions with region-level specificity?
`ExecuteFunctionCommandSecurityTest` and
`ExecuteFunctionCommandWitSecurityDUnitTest` appear to stop at the second tier,
i.e., `clusterManage` and `dataRead`.
I think we need the positive-test coverage that these `testValid*` tests
have, even if they belong in a different class / ticket.
----------------------------------------------------------------
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:
[email protected]
> improve permission for Internal functions
> -----------------------------------------
>
> Key: GEODE-3974
> URL: https://issues.apache.org/jira/browse/GEODE-3974
> Project: Geode
> Issue Type: Bug
> Components: docs, management
> Reporter: Jinmei Liao
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.5.0
>
>
> Internal functions needs to be updated to require appropriate permissions
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)