Repository: incubator-geode Updated Branches: refs/heads/develop 617c9fd47 -> bbc402723
GEODE-17: change default regionName to * instead of null. * operations with no regionName specified in its permission string will need a higher level of role than the operations with regionName specified. This makes more sense than null. Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/bbc40272 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/bbc40272 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/bbc40272 Branch: refs/heads/develop Commit: bbc402723b28bac92c31bda070118ec778dbcba5 Parents: 617c9fd Author: Jinmei Liao <jil...@pivotal.io> Authored: Fri May 20 10:25:53 2016 -0700 Committer: Jinmei Liao <jil...@pivotal.io> Committed: Mon May 23 07:55:04 2016 -0700 ---------------------------------------------------------------------- .../gemfire/cache/operations/OperationContext.java | 5 +++-- .../security/ResourceOperationContext.java | 17 ++++++----------- .../security/MemberMBeanSecurityJUnitTest.java | 2 +- .../ResourceOperationContextJUnitTest.java | 13 +++++++------ 4 files changed, 17 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/bbc40272/geode-core/src/main/java/com/gemstone/gemfire/cache/operations/OperationContext.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/cache/operations/OperationContext.java b/geode-core/src/main/java/com/gemstone/gemfire/cache/operations/OperationContext.java index b81016d..317fee6 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/cache/operations/OperationContext.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/cache/operations/OperationContext.java @@ -31,6 +31,7 @@ import org.apache.shiro.authz.permission.WildcardPermission; * @since 5.5 */ public abstract class OperationContext extends WildcardPermission{ + public static String ALL_REGIONS="*"; public enum Resource { NULL, @@ -304,7 +305,7 @@ public abstract class OperationContext extends WildcardPermission{ } public String getRegionName(){ - return null; + return ALL_REGIONS; } /** @@ -360,7 +361,7 @@ public abstract class OperationContext extends WildcardPermission{ @Override public String toString(){ - if(getRegionName()==null) + if(ALL_REGIONS.equals(getRegionName())) return getResource()+":"+getOperationCode(); else return getResource()+":"+getOperationCode()+":"+getRegionName(); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/bbc40272/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 580b6c0..99da1f1 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 @@ -25,17 +25,20 @@ public class ResourceOperationContext extends OperationContext { private boolean isPostOperation = false; private Object opResult = null; + + // these default values are used when creating a lock around an operation private Resource resource = Resource.NULL; private OperationCode operation = OperationCode.NULL; - - private String regionName = null; + private String regionName = OperationContext.ALL_REGIONS; public ResourceOperationContext() { this(null, null, null); } + // When only specified a resource and operation, it's assumed that you need access to all regions in order to perform the operations + // guarded by this ResourceOperationConext public ResourceOperationContext(String resource, String operation) { - this(resource, operation, null); + this(resource, operation, OperationContext.ALL_REGIONS); } public ResourceOperationContext(String resource, String operation, String regionName) { @@ -43,14 +46,6 @@ public class ResourceOperationContext extends OperationContext { if (operation != null) this.operation = OperationCode.valueOf(operation); if (regionName !=null ) this.regionName = regionName; - //for DATA resource, when we construct the lock to guard the operations, there should always be a 3rd part (regionName), - // if no regionName is specified, we need to add "NULL" to it. - // this means, for general data operations, or operations that we can't put a regionName on yet, like backup diskstore, query data, create regions - // it will require DATA:READ/WRITE:NULL role - if(this.resource==Resource.DATA && this.regionName==null){ - this.regionName = "NULL"; - } - setParts(this.resource.name()+":"+this.operation.name()+":"+this.regionName, true); } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/bbc40272/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/MemberMBeanSecurityJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/MemberMBeanSecurityJUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/MemberMBeanSecurityJUnitTest.java index cabf555..e32b6ca 100644 --- a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/MemberMBeanSecurityJUnitTest.java +++ b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/MemberMBeanSecurityJUnitTest.java @@ -103,7 +103,7 @@ public class MemberMBeanSecurityJUnitTest { assertThatThrownBy(() -> bean.isCacheServer()).hasMessageContaining(TestCommand.clusterRead.toString()); assertThatThrownBy(() -> bean.isServer()).hasMessageContaining(TestCommand.clusterRead.toString()); assertThatThrownBy(() -> bean.listConnectedGatewayReceivers()).hasMessageContaining(TestCommand.clusterRead.toString()); - assertThatThrownBy(() -> bean.processCommand("create region --name=Region_A")).hasMessageContaining("DATA:MANAGE"); + assertThatThrownBy(() -> bean.processCommand("create region --name=Region_A")).hasMessageContaining(TestCommand.dataManage.toString()); assertThatThrownBy(() -> bean.showJVMMetrics()).hasMessageContaining(TestCommand.clusterRead.toString()); assertThatThrownBy(() -> bean.status()).hasMessageContaining(TestCommand.clusterRead.toString()); } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/bbc40272/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContextJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContextJUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContextJUnitTest.java index ec89aaa..46c0e1d 100644 --- a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContextJUnitTest.java +++ b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContextJUnitTest.java @@ -19,6 +19,7 @@ package com.gemstone.gemfire.management.internal.security; import static org.junit.Assert.*; +import com.gemstone.gemfire.cache.operations.OperationContext; import com.gemstone.gemfire.cache.operations.OperationContext.OperationCode; import com.gemstone.gemfire.cache.operations.OperationContext.Resource; import com.gemstone.gemfire.test.junit.categories.UnitTest; @@ -37,7 +38,7 @@ public class ResourceOperationContextJUnitTest { context = new ResourceOperationContext(); assertEquals(Resource.NULL, context.getResource()); assertEquals(OperationCode.NULL, context.getOperationCode()); - assertEquals(null, context.getRegionName()); + assertEquals(OperationContext.ALL_REGIONS, context.getRegionName()); } @Test @@ -51,22 +52,22 @@ public class ResourceOperationContextJUnitTest { context = new ResourceOperationContext(null, null, null); assertEquals(Resource.NULL, context.getResource()); assertEquals(OperationCode.NULL, context.getOperationCode()); - assertEquals(null, context.getRegionName()); + assertEquals(OperationContext.ALL_REGIONS, context.getRegionName()); context = new ResourceOperationContext(null, null); assertEquals(Resource.NULL, context.getResource()); assertEquals(OperationCode.NULL, context.getOperationCode()); - assertEquals(null, context.getRegionName()); + assertEquals(OperationContext.ALL_REGIONS, context.getRegionName()); context = new ResourceOperationContext("DATA", null, null); assertEquals(Resource.DATA, context.getResource()); assertEquals(OperationCode.NULL, context.getOperationCode()); - assertEquals("NULL", context.getRegionName()); + assertEquals(OperationContext.ALL_REGIONS, context.getRegionName()); context = new ResourceOperationContext("CLUSTER", null, null); assertEquals(Resource.CLUSTER, context.getResource()); assertEquals(OperationCode.NULL, context.getOperationCode()); - assertEquals(null, context.getRegionName()); + assertEquals(OperationContext.ALL_REGIONS, context.getRegionName()); context = new ResourceOperationContext(null, "MANAGE", "REGIONA"); assertEquals(Resource.NULL, context.getResource()); @@ -85,7 +86,7 @@ public class ResourceOperationContextJUnitTest { assertEquals("NULL:NULL", context.toString()); context = new ResourceOperationContext("DATA", "MANAGE"); - assertEquals("DATA:MANAGE:NULL", context.toString()); + assertEquals("DATA:MANAGE", context.toString()); context = new ResourceOperationContext("DATA", "MANAGE", "REGIONA"); assertEquals("DATA:MANAGE:REGIONA", context.toString());