geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jinmeil...@apache.org
Subject incubator-geode git commit: GEODE-17: added DataCommandSecurityTest
Date Tue, 15 Mar 2016 17:43:53 GMT
Repository: incubator-geode
Updated Branches:
  refs/heads/feature/GEODE-17-2 ea8beec02 -> 374e20cef


GEODE-17: added DataCommandSecurityTest

* added the test
* added the command in the CliOperationContext


Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/374e20ce
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/374e20ce
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/374e20ce

Branch: refs/heads/feature/GEODE-17-2
Commit: 374e20cef143569d73be56ae8927c68e97535dfa
Parents: ea8beec
Author: Jinmei Liao <jiliao@pivotal.io>
Authored: Tue Mar 15 10:43:32 2016 -0700
Committer: Jinmei Liao <jiliao@pivotal.io>
Committed: Tue Mar 15 10:43:32 2016 -0700

----------------------------------------------------------------------
 .../internal/cli/commands/DataCommands.java     |  2 +-
 .../internal/security/CLIOperationContext.java  | 14 +--
 .../internal/security/MBeanServerWrapper.java   | 11 +--
 .../security/ManagementInterceptor.java         |  4 +-
 .../security/ResourceOperationContext.java      |  4 +
 .../security/DataCommandsSecurityTest.java      | 52 +++++++++++
 .../internal/security/JSONAuthorization.java    | 16 +++-
 ...tionCodesForDataCommandsIntegrationTest.java | 95 --------------------
 .../internal/security/cacheServer.json          | 20 +++--
 9 files changed, 104 insertions(+), 114 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/374e20ce/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/DataCommands.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/DataCommands.java
b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/DataCommands.java
index 45ecb7f..25bb3fa 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/DataCommands.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/DataCommands.java
@@ -1173,7 +1173,7 @@ public class DataCommands implements CommandMarker {
       CliStrings.TOPIC_GEMFIRE_DATA, CliStrings.TOPIC_GEMFIRE_REGION })
   @MultiStepCommand
   @CliCommand(value = { CliStrings.QUERY }, help = CliStrings.QUERY__HELP)
-  @ResourceOperation(resource = Resource.QUERY, operation = OperationCode.QUERY)
+  @ResourceOperation(resource = Resource.QUERY, operation = OperationCode.EXECUTE)
   public Object query(
       @CliOption(key = CliStrings.QUERY__QUERY, help = CliStrings.QUERY__QUERY__HELP, mandatory
= true) final String query,
       @CliOption(key = CliStrings.QUERY__STEPNAME, mandatory = false, help = "Step name",
unspecifiedDefaultValue = CliStrings.QUERY__STEPNAME__DEFAULTVALUE) String stepName,

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/374e20ce/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/CLIOperationContext.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/CLIOperationContext.java
b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/CLIOperationContext.java
index fdc683d..7e125ef 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/CLIOperationContext.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/CLIOperationContext.java
@@ -39,6 +39,7 @@ import java.util.Map;
 public class CLIOperationContext extends ResourceOperationContext {
 
 	private Map<String,String> commandOptions = null;
+	private String command = null;
 	
 	private static Map<String,ResourceOperation> commandToCodeMapping = new HashMap<String,ResourceOperation>();
 	private static CommandManager commandManager = null;
@@ -46,9 +47,10 @@ public class CLIOperationContext extends ResourceOperationContext {
 	
 	public CLIOperationContext(String commandString) throws CommandProcessingException, IllegalStateException{
 		GfshParseResult parseResult = (GfshParseResult) parseCommand(commandString);
-		ResourceOperation op = findResourceCode(parseResult.getCommandName());
-		setResourceOperation(op);
+		this.command = parseResult.getCommandName();
 		this.commandOptions = parseResult.getParamValueStrings();
+		ResourceOperation op = findResourceCode(this.command);
+		setResourceOperation(op);
   }
 	
 	private static ParseResult parseCommand(String commentLessLine) throws CommandProcessingException,
IllegalStateException {
@@ -79,13 +81,15 @@ public class CLIOperationContext extends ResourceOperationContext {
 		return commandOptions;
 	}
 
+	public String getCommand(){
+		return command;
+	}
+
 	private static ResourceOperation findResourceCode(String commandName) {
 		return commandToCodeMapping.get(commandName);
 	}
 	
 	public String toString(){
-	  String str;
-	  str = "CLIOperationContext(resourceCode=" + getOperationCode() + ") options=" + commandOptions+")";
-	  return str;
+	  return getResource() + ":"+ getOperationCode() + " commmand=" + command + " options="
+ commandOptions;
 	}
 }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/374e20ce/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/MBeanServerWrapper.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/MBeanServerWrapper.java
b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/MBeanServerWrapper.java
index ee52070..bbf81d1 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/MBeanServerWrapper.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/MBeanServerWrapper.java
@@ -213,13 +213,14 @@ public class MBeanServerWrapper implements MBeanServerForwarder {
   @Override
   public Object invoke(ObjectName name, String operationName, Object[] params, String[] signature)
       throws InstanceNotFoundException, MBeanException, ReflectionException {
-    ResourceOperationContext ctx = getOperationContext(name, operationName, true);
-    doAuthorization(ctx);
-    // further authorize the processCommand call
+    ResourceOperationContext ctx = null;
     if("processCommand".equals(operationName) && params.length==1){
-      CLIOperationContext cliContext = new CLIOperationContext((String)params[0]);
-      doAuthorization(cliContext);
+      ctx = new CLIOperationContext((String)params[0]);
+    }
+    else {
+      ctx = getOperationContext(name, operationName, true);
     }
+    doAuthorization(ctx);
 
     Object result = mbs.invoke(name, operationName, params, signature);
     if(ctx!=null)

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/374e20ce/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/ManagementInterceptor.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/ManagementInterceptor.java
b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/ManagementInterceptor.java
index 2362f4d..7e6befb 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/ManagementInterceptor.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/ManagementInterceptor.java
@@ -168,7 +168,7 @@ public class ManagementInterceptor implements JMXAuthenticator{
     Set<JMXPrincipal> principals = subject.getPrincipals(JMXPrincipal.class);
 
     if (principals == null || principals.isEmpty()) {
-      throw new SecurityException(ACCESS_DENIED_MESSAGE);
+      throw new SecurityException(ACCESS_DENIED_MESSAGE + ": No princial found.");
 		}
 
 		Principal principal = principals.iterator().next();
@@ -176,7 +176,7 @@ public class ManagementInterceptor implements JMXAuthenticator{
     AccessControl accessControl = getAccessControl(principal, false);
 
     if (!accessControl.authorizeOperation(null, context)) {
-      throw new SecurityException(ACCESS_DENIED_MESSAGE);
+      throw new SecurityException(ACCESS_DENIED_MESSAGE + ": Not authorized for "+context);
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/374e20ce/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContext.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContext.java
b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContext.java
index 9ef458d..9e2b1b4 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContext.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContext.java
@@ -77,4 +77,8 @@ public class ResourceOperationContext extends OperationContext {
     return this.opResult;
   }
 
+  public String toString(){
+    return getResource() + ":"+ getOperationCode();
+  }
+
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/374e20ce/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/DataCommandsSecurityTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/DataCommandsSecurityTest.java
b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/DataCommandsSecurityTest.java
index ab42e58..7d1564b 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/DataCommandsSecurityTest.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/DataCommandsSecurityTest.java
@@ -59,4 +59,56 @@ public class DataCommandsSecurityTest {
     bean.processCommand("locate entry --key=k1 --region=secureRegion");
   }
 
+  @JMXConnectionConfiguration(user = "superuser", password = "1234567")
+  @Test
+  public void testAllAccess(){
+    bean.processCommand("rebalance --include-region=region1");
+    bean.processCommand("export data --region=region1 --file=foo.txt --member=value");
+    bean.processCommand("import data --region=region1 --file=foo.txt --member=value");
+    bean.processCommand("put --key=key1 --value=value1 --region=region1");
+    bean.processCommand("get --key=key1 --region=region1");
+    bean.processCommand("remove --region=region1");
+    bean.processCommand("query --query='SELECT * FROM /region1'");
+  }
+
+  // stranger has no permission granted
+  @JMXConnectionConfiguration(user = "stranger", password = "1234567")
+  @Test
+  public void testNoAccess(){
+    assertThatThrownBy(() -> bean.processCommand("rebalance --include-region=region1")).isInstanceOf(SecurityException.class)
+    .hasMessageStartingWith("Access Denied: Not authorized for REGION:REBALANCE");
+
+    assertThatThrownBy(() -> bean.processCommand("export data --region=region1 --file=foo.txt
--member=value")).isInstanceOf(SecurityException.class);
+    assertThatThrownBy(() -> bean.processCommand("import data --region=region1 --file=foo.txt
--member=value")).isInstanceOf(SecurityException.class);
+
+    assertThatThrownBy(() -> bean.processCommand("put --key=key1 --value=value1 --region=region1")).isInstanceOf(SecurityException.class)
+        .hasMessageStartingWith("Access Denied: Not authorized for REGION:PUT");
+
+    assertThatThrownBy(() -> bean.processCommand("get --key=key1 --region=region1")).isInstanceOf(SecurityException.class)
+        .hasMessageStartingWith("Access Denied: Not authorized for REGION:GET");
+
+    assertThatThrownBy(() -> bean.processCommand("query --query='SELECT * FROM /region1'")).isInstanceOf(SecurityException.class)
+        .hasMessageStartingWith("Access Denied: Not authorized for QUERY:EXECUTE");
+  }
+
+  // dataUser has all the permissions granted, but not to region2 (only to region1)
+  @JMXConnectionConfiguration(user = "dataUser", password = "1234567")
+  @Test
+  public void testNoAccessToRegion(){
+    assertThatThrownBy(() -> bean.processCommand("rebalance --include-region=region2")).isInstanceOf(SecurityException.class)
+        .hasMessageStartingWith("Access Denied: Not authorized for REGION:REBALANCE");
+
+    assertThatThrownBy(() -> bean.processCommand("export data --region=region2 --file=foo.txt
--member=value")).isInstanceOf(SecurityException.class);
+    assertThatThrownBy(() -> bean.processCommand("import data --region=region2 --file=foo.txt
--member=value")).isInstanceOf(SecurityException.class);
+
+    assertThatThrownBy(() -> bean.processCommand("put --key=key1 --value=value1 --region=region2")).isInstanceOf(SecurityException.class)
+        .hasMessageStartingWith("Access Denied: Not authorized for REGION:PUT");
+
+    assertThatThrownBy(() -> bean.processCommand("get --key=key1 --region=region2")).isInstanceOf(SecurityException.class)
+        .hasMessageStartingWith("Access Denied: Not authorized for REGION:GET");
+
+    assertThatThrownBy(() -> bean.processCommand("query --query='SELECT * FROM /region2'")).isInstanceOf(SecurityException.class)
+        .hasMessageStartingWith("Access Denied: Not authorized for QUERY:EXECUTE");
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/374e20ce/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/JSONAuthorization.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/JSONAuthorization.java
b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/JSONAuthorization.java
index 40497f7..8b79c1c 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/JSONAuthorization.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/JSONAuthorization.java
@@ -43,6 +43,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 public class JSONAuthorization implements AccessControl, Authenticator {
 
@@ -210,7 +212,19 @@ public class JSONAuthorization implements AccessControl, Authenticator
{
           // If this is a Command operation context, we need to further check if the region
is allowed in this role
           CLIOperationContext ctx = (CLIOperationContext) context;
           String region = ctx.getCommandOptions().get("region");
-          if(role.regionNames == null || role.regionNames.contains(region)){
+          if(region==null) {
+            region = ctx.getCommandOptions().get("include-region");
+          }
+          if(region==null) {
+            String query = ctx.getCommandOptions().get("query");
+            if(query!=null) {
+              Matcher matcher = Pattern.compile("/\\s*(\\w+)").matcher(query);
+              if (matcher.find())
+                region = matcher.group(1);
+            }
+          }
+
+          if(role.regionNames == null || region == null || role.regionNames.contains(region)){
             // if regionName is null, i.e. all regions are allowed
             return true;
           }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/374e20ce/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/OperationCodesForDataCommandsIntegrationTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/OperationCodesForDataCommandsIntegrationTest.java
b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/OperationCodesForDataCommandsIntegrationTest.java
deleted file mode 100755
index 435e6e7..0000000
--- a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/OperationCodesForDataCommandsIntegrationTest.java
+++ /dev/null
@@ -1,95 +0,0 @@
-/*
- * 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 com.gemstone.gemfire.management.internal.security;
-
-import com.gemstone.gemfire.cache.CacheFactory;
-import com.gemstone.gemfire.cache.operations.OperationContext.OperationCode;
-import com.gemstone.gemfire.distributed.DistributedSystem;
-import com.gemstone.gemfire.distributed.internal.DistributionConfig;
-import com.gemstone.gemfire.internal.cache.GemFireCacheImpl;
-import com.gemstone.gemfire.test.junit.categories.IntegrationTest;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.contrib.java.lang.system.RestoreSystemProperties;
-import org.junit.experimental.categories.Category;
-import org.junit.rules.TestName;
-
-import java.io.IOException;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Properties;
-
-import static org.assertj.core.api.Assertions.assertThat;
-
-/**
- * Tests operation codes for data commands.
- */
-@Category(IntegrationTest.class)
-@SuppressWarnings("deprecation")
-public class OperationCodesForDataCommandsIntegrationTest {
-
-  private GemFireCacheImpl cache;
-  private DistributedSystem ds;
-  private Map<String, OperationCode> commands = new HashMap<String, OperationCode>();
-
-  @Rule
-  public TestName testName = new TestName();
-
-  @Rule
-  public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();
-
-  @Before
-  public void setUp() {
-    Properties properties = new Properties();
-    properties.put("name", testName.getMethodName());
-    properties.put(DistributionConfig.LOCATORS_NAME, "");
-    properties.put(DistributionConfig.MCAST_PORT_NAME, "0");
-    properties.put(DistributionConfig.HTTP_SERVICE_PORT_NAME, "0");
-
-    this.ds = DistributedSystem.connect(properties);
-    this.cache = (GemFireCacheImpl) CacheFactory.create(ds);
-
-    this.commands.put("put --key=k1 --value=v1 --region=/region1", OperationCode.PUT);
-    this.commands.put("locate entry --key=k1 --region=/region1", OperationCode.GET);
-    this.commands.put("query --query=\"select * from /region1\"", OperationCode.QUERY);
-    this.commands.put("export data --region=value --file=value --member=value", OperationCode.EXPORT);
-    this.commands.put("import data --region=value --file=value --member=value", OperationCode.IMPORT);
-    this.commands.put("rebalance", OperationCode.REBALANCE);
-  }
-
-  @After
-  public void tearDown() throws IOException {
-    if (this.cache != null) {
-      this.cache.close();
-      this.cache = null;
-    }
-    if (this.ds != null) {
-      this.ds.disconnect();
-      this.ds = null;
-    }
-  }
-
-  @Test
-  public void commandsShouldMapToCorrectResourceCodes() throws Exception {
-    for (String command : this.commands.keySet()) {
-      CLIOperationContext ctx = new CLIOperationContext(command);
-      assertThat(ctx.getOperationCode()).isEqualTo(this.commands.get(command));
-    }
-  }
-}

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/374e20ce/geode-core/src/test/resources/com/gemstone/gemfire/management/internal/security/cacheServer.json
----------------------------------------------------------------------
diff --git a/geode-core/src/test/resources/com/gemstone/gemfire/management/internal/security/cacheServer.json
b/geode-core/src/test/resources/com/gemstone/gemfire/management/internal/security/cacheServer.json
index 092767d..dd7f830 100644
--- a/geode-core/src/test/resources/com/gemstone/gemfire/management/internal/security/cacheServer.json
+++ b/geode-core/src/test/resources/com/gemstone/gemfire/management/internal/security/cacheServer.json
@@ -9,11 +9,16 @@
         "CONTINUOUS_QUERY:EXECUTE",
         "CONTINUOUS_QUERY:STOP",
         "MEMBER:SHUTDOWN",
+        "MEMBER:STATUS",
         "DISKSTORE:COMPACT",
         "MANAGER:CREATE",
         "REGION:CREATE",
-        "MEMBER:PROCESS_COMMAND",
-        "MEMBER:STATUS"
+        "REGION:REBALANCE",
+        "REGION:EXPORT",
+        "REGION:IMPORT",
+        "REGION:PUT",
+        "REGION:GET",
+        "REGION:DELETE"
       ]
     },
     {
@@ -31,15 +36,20 @@
       "name": "dataUsers",
       "operationsAllowed": [
         "REGION:GET",
-        "MEMBER:PROCESS_COMMAND"
+        "REGION:REBALANCE",
+        "REGION:EXPORT",
+        "REGION:IMPORT",
+        "REGION:PUT",
+        "REGION:GET",
+        "REGION:DELETE",
+        "QUERY:EXECUTE"
       ],
       "region": "region1"
     },
     {
       "name": "secureDataUsers",
       "operationsAllowed": [
-        "REGION:GET",
-        "MEMBER:PROCESS_COMMAND"
+        "REGION:GET"
       ],
       "regions": ["region1", "secureRegion"]
     }


Mime
View raw message