Repository: geode Updated Branches: refs/heads/develop 122b07afd -> f54104358
GEODE-2925: add target for resource operation for finer grained security * add finer authorize* call in security service * add target to the MXBean authorization check * use enum type instead of raw strings for resource/operation Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/f5410435 Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/f5410435 Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/f5410435 Branch: refs/heads/develop Commit: f541043580bf245fbd4b62b2300c494ebc58ab6b Parents: 122b07a Author: Jinmei Liao <[email protected]> Authored: Wed May 31 13:50:23 2017 -0700 Committer: Jinmei Liao <[email protected]> Committed: Mon Jun 12 11:05:23 2017 -0700 ---------------------------------------------------------------------- .../security/ExampleSecurityManager.java | 6 +- .../security/CustomSecurityService.java | 93 +++++++++++--------- .../security/DisabledSecurityService.java | 59 ++++++++----- .../security/EnabledSecurityService.java | 80 +++++++++-------- .../security/LegacySecurityService.java | 59 ++++++++----- .../internal/security/SecurityService.java | 23 +++-- .../cli/commands/DiskStoreCommands.java | 52 +++++------ .../internal/cli/remote/CommandProcessor.java | 6 +- .../internal/security/AccessControlMBean.java | 6 +- .../internal/security/MBeanServerWrapper.java | 13 ++- .../internal/security/ResourceOperation.java | 14 +-- .../geode/security/ResourcePermission.java | 75 ++++++++++------ .../security/ResourcePermissionTest.java | 67 ++++++++------ .../internal/security/TestCommand.java | 33 ++++--- .../security/SimpleSecurityManagerTest.java | 11 ++- .../geode/security/TestSecurityManager.java | 6 +- .../geode/codeAnalysis/excludedClasses.txt | 1 + .../web/security/RestSecurityService.java | 6 +- 18 files changed, 370 insertions(+), 240 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java b/geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java index 84f97de..c1d7ebf 100644 --- a/geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java +++ b/geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java @@ -21,6 +21,8 @@ import org.apache.geode.management.internal.security.ResourceConstants; import org.apache.geode.security.AuthenticationFailedException; import org.apache.geode.security.NotAuthorizedException; import org.apache.geode.security.ResourcePermission; +import org.apache.geode.security.ResourcePermission.Operation; +import org.apache.geode.security.ResourcePermission.Resource; import org.apache.geode.security.SecurityManager; import org.apache.shiro.authz.Permission; @@ -243,8 +245,8 @@ public class ExampleSecurityManager implements SecurityManager { String regionPart = (regionNames != null) ? regionNames : "*"; String keyPart = (keys != null) ? keys : "*"; - role.permissions - .add(new ResourcePermission(resourcePart, operationPart, regionPart, keyPart)); + role.permissions.add(new ResourcePermission(Resource.valueOf(resourcePart), + Operation.valueOf(operationPart), regionPart, keyPart)); } roleMap.put(role.name, role); http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java b/geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java index c4946e7..0ba1cb6 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java +++ b/geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java @@ -14,13 +14,6 @@ */ package org.apache.geode.internal.security; -import java.io.IOException; -import java.io.Serializable; -import java.security.AccessController; -import java.util.Properties; -import java.util.Set; -import java.util.concurrent.Callable; - import org.apache.commons.lang.SerializationException; import org.apache.commons.lang.StringUtils; import org.apache.geode.GemFireIOException; @@ -32,8 +25,12 @@ import org.apache.geode.internal.util.BlobHelper; import org.apache.geode.security.AuthenticationFailedException; import org.apache.geode.security.GemFireSecurityException; import org.apache.geode.security.NotAuthorizedException; +import org.apache.geode.security.PostProcessor; +import org.apache.geode.security.ResourcePermission; import org.apache.geode.security.ResourcePermission.Operation; import org.apache.geode.security.ResourcePermission.Resource; +import org.apache.geode.security.ResourcePermission.Target; +import org.apache.geode.security.SecurityManager; import org.apache.logging.log4j.Logger; import org.apache.shiro.SecurityUtils; import org.apache.shiro.ShiroException; @@ -42,10 +39,12 @@ import org.apache.shiro.subject.support.SubjectThreadState; import org.apache.shiro.util.ThreadContext; import org.apache.shiro.util.ThreadState; -import org.apache.geode.management.internal.security.ResourceOperation; -import org.apache.geode.security.PostProcessor; -import org.apache.geode.security.ResourcePermission; -import org.apache.geode.security.SecurityManager; +import java.io.IOException; +import java.io.Serializable; +import java.security.AccessController; +import java.util.Properties; +import java.util.Set; +import java.util.concurrent.Callable; public class CustomSecurityService implements SecurityService { private static Logger logger = LogService.getLogger(LogService.SECURITY_LOGGER_NAME); @@ -154,90 +153,96 @@ public class CustomSecurityService implements SecurityService { return currentUser.associateWith(callable); } - @Override - public void authorize(final ResourceOperation resourceOperation) { - if (resourceOperation == null) { - return; - } - - authorize(resourceOperation.resource().name(), resourceOperation.operation().name(), null); - } - - @Override public void authorizeClusterManage() { - authorize("CLUSTER", "MANAGE"); + authorize(Resource.CLUSTER, Operation.MANAGE, Target.ALL, ResourcePermission.ALL); } @Override public void authorizeClusterWrite() { - authorize("CLUSTER", "WRITE"); + authorize(Resource.CLUSTER, Operation.WRITE, Target.ALL, ResourcePermission.ALL); } @Override public void authorizeClusterRead() { - authorize("CLUSTER", "READ"); + authorize(Resource.CLUSTER, Operation.READ, Target.ALL, ResourcePermission.ALL); } @Override public void authorizeDataManage() { - authorize("DATA", "MANAGE"); + authorize(Resource.DATA, Operation.MANAGE, Target.ALL, ResourcePermission.ALL); } @Override public void authorizeDataWrite() { - authorize("DATA", "WRITE"); + authorize(Resource.DATA, Operation.WRITE, Target.ALL, ResourcePermission.ALL); } @Override public void authorizeDataRead() { - authorize("DATA", "READ"); + authorize(Resource.DATA, Operation.READ, Target.ALL, ResourcePermission.ALL); + } + + @Override + public void authorizeDiskManage() { + authorize(Resource.CLUSTER, Operation.MANAGE, Target.DISK, ResourcePermission.ALL); + } + + @Override + public void authorizeGatewayManage() { + authorize(Resource.CLUSTER, Operation.MANAGE, Target.GATEWAY, ResourcePermission.ALL); + } + + @Override + public void authorizeJarManage() { + authorize(Resource.CLUSTER, Operation.MANAGE, Target.JAR, ResourcePermission.ALL); + } + + @Override + public void authorizeQueryManage() { + authorize(Resource.CLUSTER, Operation.MANAGE, Target.QUERY, ResourcePermission.ALL); } @Override public void authorizeRegionManage(final String regionName) { - authorize("DATA", "MANAGE", regionName); + authorize(Resource.DATA, Operation.MANAGE, regionName, ResourcePermission.ALL); } @Override public void authorizeRegionManage(final String regionName, final String key) { - authorize("DATA", "MANAGE", regionName, key); + authorize(Resource.DATA, Operation.MANAGE, regionName, key); } @Override public void authorizeRegionWrite(final String regionName) { - authorize("DATA", "WRITE", regionName); + authorize(Resource.DATA, Operation.WRITE, regionName, ResourcePermission.ALL); } @Override public void authorizeRegionWrite(final String regionName, final String key) { - authorize("DATA", "WRITE", regionName, key); + authorize(Resource.DATA, Operation.WRITE, regionName, key); } @Override public void authorizeRegionRead(final String regionName) { - authorize("DATA", "READ", regionName); + authorize(Resource.DATA, Operation.READ, regionName, ResourcePermission.ALL); } @Override public void authorizeRegionRead(final String regionName, final String key) { - authorize("DATA", "READ", regionName, key); + authorize(Resource.DATA, Operation.READ, regionName, key); } - @Override - public void authorize(final String resource, final String operation) { - authorize(resource, operation, null); + public void authorize(Resource resource, Operation operation, ResourcePermission.Target target, + String key) { + authorize(resource, operation, target.getName(), key); } - @Override - public void authorize(final String resource, final String operation, final String regionName) { - authorize(resource, operation, regionName, null); + public void authorize(Resource resource, Operation operation, ResourcePermission.Target target) { + authorize(resource, operation, target, ResourcePermission.ALL); } - @Override - public void authorize(final String resource, final String operation, String regionName, - final String key) { - regionName = StringUtils.stripStart(regionName, "/"); - authorize(new ResourcePermission(resource, operation, regionName, key)); + public void authorize(Resource resource, Operation operation, String target, String key) { + authorize(new ResourcePermission(resource, operation, target, key)); } @Override http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java b/geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java index d328946..b505690 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java +++ b/geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java @@ -14,17 +14,15 @@ */ package org.apache.geode.internal.security; -import java.util.Properties; -import java.util.concurrent.Callable; - +import org.apache.geode.security.PostProcessor; +import org.apache.geode.security.ResourcePermission; +import org.apache.geode.security.SecurityManager; import org.apache.shiro.subject.Subject; import org.apache.shiro.subject.support.SubjectThreadState; import org.apache.shiro.util.ThreadState; -import org.apache.geode.management.internal.security.ResourceOperation; -import org.apache.geode.security.PostProcessor; -import org.apache.geode.security.ResourcePermission; -import org.apache.geode.security.SecurityManager; +import java.util.Properties; +import java.util.concurrent.Callable; /** * No-op security service that does nothing. @@ -72,8 +70,21 @@ public class DisabledSecurityService implements SecurityService { } @Override - public void authorize(final ResourceOperation resourceOperation) { - // nothing + public void authorize(ResourcePermission.Resource resource, + ResourcePermission.Operation operation, String target, String key) { + + } + + @Override + public void authorize(ResourcePermission.Resource resource, + ResourcePermission.Operation operation, ResourcePermission.Target target, String key) { + + } + + @Override + public void authorize(ResourcePermission.Resource resource, + ResourcePermission.Operation operation, ResourcePermission.Target target) { + } @Override @@ -107,48 +118,52 @@ public class DisabledSecurityService implements SecurityService { } @Override - public void authorizeRegionManage(final String regionName) { - // nothing + public void authorizeDiskManage() { + } @Override - public void authorizeRegionManage(final String regionName, final String key) { - // nothing + public void authorizeGatewayManage() { + } @Override - public void authorizeRegionWrite(final String regionName) { - // nothing + public void authorizeJarManage() { + } @Override - public void authorizeRegionWrite(final String regionName, final String key) { + public void authorizeQueryManage() { + + } + + @Override + public void authorizeRegionManage(final String regionName) { // nothing } @Override - public void authorizeRegionRead(final String regionName) { + public void authorizeRegionManage(final String regionName, final String key) { // nothing } @Override - public void authorizeRegionRead(final String regionName, final String key) { + public void authorizeRegionWrite(final String regionName) { // nothing } @Override - public void authorize(final String resource, final String operation) { + public void authorizeRegionWrite(final String regionName, final String key) { // nothing } @Override - public void authorize(final String resource, final String operation, final String regionName) { + public void authorizeRegionRead(final String regionName) { // nothing } @Override - public void authorize(final String resource, final String operation, final String regionName, - final String key) { + public void authorizeRegionRead(final String regionName, final String key) { // nothing } http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java b/geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java index f971dee..f0568c0 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java +++ b/geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java @@ -31,6 +31,7 @@ import org.apache.geode.security.PostProcessor; import org.apache.geode.security.ResourcePermission; import org.apache.geode.security.ResourcePermission.Operation; import org.apache.geode.security.ResourcePermission.Resource; +import org.apache.geode.security.ResourcePermission.Target; import org.apache.geode.security.SecurityManager; import org.apache.logging.log4j.Logger; import org.apache.shiro.SecurityUtils; @@ -187,98 +188,99 @@ public class EnabledSecurityService implements SecurityService { return threadState; } - @Override - public void authorize(final ResourceOperation resourceOperation) { - if (resourceOperation == null) { - return; - } - - authorize(resourceOperation.resource().name(), resourceOperation.operation().name(), null); - } - - @Override public void authorizeClusterManage() { - authorize("CLUSTER", "MANAGE"); + authorize(Resource.CLUSTER, Operation.MANAGE, Target.ALL, ResourcePermission.ALL); } @Override public void authorizeClusterWrite() { - authorize("CLUSTER", "WRITE"); + authorize(Resource.CLUSTER, Operation.WRITE, Target.ALL, ResourcePermission.ALL); } @Override public void authorizeClusterRead() { - authorize("CLUSTER", "READ"); + authorize(Resource.CLUSTER, Operation.READ, Target.ALL, ResourcePermission.ALL); } @Override public void authorizeDataManage() { - authorize("DATA", "MANAGE"); + authorize(Resource.DATA, Operation.MANAGE, Target.ALL, ResourcePermission.ALL); } @Override public void authorizeDataWrite() { - authorize("DATA", "WRITE"); + authorize(Resource.DATA, Operation.WRITE, Target.ALL, ResourcePermission.ALL); } @Override public void authorizeDataRead() { - authorize("DATA", "READ"); + authorize(Resource.DATA, Operation.READ, Target.ALL, ResourcePermission.ALL); + } + + @Override + public void authorizeDiskManage() { + authorize(Resource.CLUSTER, Operation.MANAGE, Target.DISK, ResourcePermission.ALL); + } + + @Override + public void authorizeGatewayManage() { + authorize(Resource.CLUSTER, Operation.MANAGE, Target.GATEWAY, ResourcePermission.ALL); + } + + @Override + public void authorizeJarManage() { + authorize(Resource.CLUSTER, Operation.MANAGE, Target.JAR, ResourcePermission.ALL); + } + + @Override + public void authorizeQueryManage() { + authorize(Resource.CLUSTER, Operation.MANAGE, Target.QUERY, ResourcePermission.ALL); } @Override public void authorizeRegionManage(final String regionName) { - authorize("DATA", "MANAGE", regionName); + authorize(Resource.DATA, Operation.MANAGE, regionName, ResourcePermission.ALL); } @Override public void authorizeRegionManage(final String regionName, final String key) { - authorize("DATA", "MANAGE", regionName, key); + authorize(Resource.DATA, Operation.MANAGE, regionName, key); } @Override public void authorizeRegionWrite(final String regionName) { - authorize("DATA", "WRITE", regionName); + authorize(Resource.DATA, Operation.WRITE, regionName, ResourcePermission.ALL); } @Override public void authorizeRegionWrite(final String regionName, final String key) { - authorize("DATA", "WRITE", regionName, key); + authorize(Resource.DATA, Operation.WRITE, regionName, key); } @Override public void authorizeRegionRead(final String regionName) { - authorize("DATA", "READ", regionName); + authorize(Resource.DATA, Operation.READ, regionName, ResourcePermission.ALL); } @Override public void authorizeRegionRead(final String regionName, final String key) { - authorize("DATA", "READ", regionName, key); + authorize(Resource.DATA, Operation.READ, regionName, key); } - @Override - public void authorize(final String resource, final String operation) { - authorize(resource, operation, null); + public void authorize(Resource resource, Operation operation, Target target, String key) { + authorize(resource, operation, target.getName(), key); } - @Override - public void authorize(final String resource, final String operation, final String regionName) { - authorize(resource, operation, regionName, null); + public void authorize(Resource resource, Operation operation, Target target) { + authorize(resource, operation, target, ResourcePermission.ALL); } - @Override - public void authorize(final String resource, final String operation, String regionName, - final String key) { - regionName = StringUtils.stripStart(regionName, "/"); - authorize(new ResourcePermission(resource, operation, regionName, key)); + public void authorize(Resource resource, Operation operation, String target, String key) { + authorize(new ResourcePermission(resource, operation, target, key)); } @Override public void authorize(final ResourcePermission context) { - Subject currentUser = getSubject(); - if (currentUser == null) { - return; - } if (context == null) { return; } @@ -286,6 +288,10 @@ public class EnabledSecurityService implements SecurityService { return; } + // if currentUser is null, let it throw NPE, since in a EnabledSecurityService, + // user can not be null + Subject currentUser = getSubject(); + try { currentUser.checkPermission(context); } catch (ShiroException e) { http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java b/geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java index ef92bb7..4456253 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java +++ b/geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java @@ -14,16 +14,14 @@ */ package org.apache.geode.internal.security; -import java.util.Properties; -import java.util.concurrent.Callable; - -import org.apache.shiro.subject.Subject; -import org.apache.shiro.util.ThreadState; - -import org.apache.geode.management.internal.security.ResourceOperation; import org.apache.geode.security.PostProcessor; import org.apache.geode.security.ResourcePermission; import org.apache.geode.security.SecurityManager; +import org.apache.shiro.subject.Subject; +import org.apache.shiro.util.ThreadState; + +import java.util.Properties; +import java.util.concurrent.Callable; /** * Legacy security service with ClientAuthenticator and/or PeerAuthenticator. @@ -70,8 +68,21 @@ public class LegacySecurityService implements SecurityService { } @Override - public void authorize(final ResourceOperation resourceOperation) { - // nothing + public void authorize(ResourcePermission.Resource resource, + ResourcePermission.Operation operation, String target, String key) { + + } + + @Override + public void authorize(ResourcePermission.Resource resource, + ResourcePermission.Operation operation, ResourcePermission.Target target, String key) { + + } + + @Override + public void authorize(ResourcePermission.Resource resource, + ResourcePermission.Operation operation, ResourcePermission.Target target) { + } @Override @@ -105,48 +116,52 @@ public class LegacySecurityService implements SecurityService { } @Override - public void authorizeRegionManage(final String regionName) { - // nothing + public void authorizeDiskManage() { + } @Override - public void authorizeRegionManage(final String regionName, final String key) { - // nothing + public void authorizeGatewayManage() { + } @Override - public void authorizeRegionWrite(final String regionName) { - // nothing + public void authorizeJarManage() { + } @Override - public void authorizeRegionWrite(final String regionName, final String key) { + public void authorizeQueryManage() { + + } + + @Override + public void authorizeRegionManage(final String regionName) { // nothing } @Override - public void authorizeRegionRead(final String regionName) { + public void authorizeRegionManage(final String regionName, final String key) { // nothing } @Override - public void authorizeRegionRead(final String regionName, final String key) { + public void authorizeRegionWrite(final String regionName) { // nothing } @Override - public void authorize(final String resource, final String operation) { + public void authorizeRegionWrite(final String regionName, final String key) { // nothing } @Override - public void authorize(final String resource, final String operation, final String regionName) { + public void authorizeRegionRead(final String regionName) { // nothing } @Override - public void authorize(final String resource, final String operation, final String regionName, - final String key) { + public void authorizeRegionRead(final String regionName, final String key) { // nothing } http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java b/geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java index be81582..a4041e1 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java +++ b/geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java @@ -17,6 +17,9 @@ package org.apache.geode.internal.security; import org.apache.geode.management.internal.security.ResourceOperation; import org.apache.geode.security.PostProcessor; import org.apache.geode.security.ResourcePermission; +import org.apache.geode.security.ResourcePermission.Resource; +import org.apache.geode.security.ResourcePermission.Operation; +import org.apache.geode.security.ResourcePermission.Target; import org.apache.geode.security.SecurityManager; import org.apache.shiro.subject.Subject; import org.apache.shiro.util.ThreadState; @@ -38,7 +41,11 @@ public interface SecurityService { Callable associateWith(Callable callable); - void authorize(ResourceOperation resourceOperation); + void authorize(Resource resource, Operation operation, String target, String key); + + void authorize(Resource resource, Operation operation, Target target, String key); + + void authorize(Resource resource, Operation operation, Target target); void authorizeClusterManage(); @@ -52,6 +59,14 @@ public interface SecurityService { void authorizeDataRead(); + void authorizeDiskManage(); + + void authorizeGatewayManage(); + + void authorizeJarManage(); + + void authorizeQueryManage(); + void authorizeRegionManage(String regionName); void authorizeRegionManage(String regionName, String key); @@ -64,12 +79,6 @@ public interface SecurityService { void authorizeRegionRead(String regionName, String key); - void authorize(String resource, String operation); - - void authorize(String resource, String operation, String regionName); - - void authorize(String resource, String operation, String regionName, String key); - void authorize(ResourcePermission context); void close(); http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java index 64fafda..16124aa 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java @@ -14,30 +14,6 @@ */ package org.apache.geode.management.internal.cli.commands; -import java.io.BufferedReader; -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.io.PrintStream; -import java.net.URL; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Map.Entry; -import java.util.Set; -import java.util.concurrent.atomic.AtomicReference; - -import org.springframework.shell.core.annotation.CliAvailabilityIndicator; -import org.springframework.shell.core.annotation.CliCommand; -import org.springframework.shell.core.annotation.CliOption; - import org.apache.geode.GemFireIOException; import org.apache.geode.SystemFailure; import org.apache.geode.admin.BackupStatus; @@ -77,7 +53,6 @@ import org.apache.geode.management.internal.cli.functions.DestroyDiskStoreFuncti import org.apache.geode.management.internal.cli.functions.ListDiskStoresFunction; import org.apache.geode.management.internal.cli.functions.ShowMissingDiskStoresFunction; import org.apache.geode.management.internal.cli.i18n.CliStrings; -import org.apache.geode.management.internal.cli.result.CommandResultException; import org.apache.geode.management.internal.cli.result.CompositeResultData; import org.apache.geode.management.internal.cli.result.CompositeResultData.SectionResultData; import org.apache.geode.management.internal.cli.result.ErrorResultData; @@ -93,8 +68,32 @@ import org.apache.geode.management.internal.cli.util.MemberNotFoundException; import org.apache.geode.management.internal.configuration.domain.XmlEntity; import org.apache.geode.management.internal.messages.CompactRequest; import org.apache.geode.management.internal.security.ResourceOperation; +import org.apache.geode.security.ResourcePermission.Target; import org.apache.geode.security.ResourcePermission.Operation; import org.apache.geode.security.ResourcePermission.Resource; +import org.springframework.shell.core.annotation.CliAvailabilityIndicator; +import org.springframework.shell.core.annotation.CliCommand; +import org.springframework.shell.core.annotation.CliOption; + +import java.io.BufferedReader; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.PrintStream; +import java.net.URL; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; /** * The DiskStoreCommands class encapsulates all GemFire Disk Store commands in Gfsh. @@ -400,7 +399,8 @@ public class DiskStoreCommands implements GfshCommand { @CliCommand(value = CliStrings.COMPACT_DISK_STORE, help = CliStrings.COMPACT_DISK_STORE__HELP) @CliMetaData(shellOnly = false, relatedTopic = {CliStrings.TOPIC_GEODE_DISKSTORE}) - @ResourceOperation(resource = Resource.DATA, operation = Operation.MANAGE) + @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.MANAGE, + target = Target.DISK) public Result compactDiskStore( @CliOption(key = CliStrings.COMPACT_DISK_STORE__NAME, mandatory = true, optionContext = ConverterHint.DISKSTORE, http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java index 6e5b650..e9c2736 100755 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java @@ -26,6 +26,7 @@ import org.apache.geode.management.internal.cli.result.ResultBuilder; import org.apache.geode.management.internal.cli.util.CommentSkipHelper; import org.apache.geode.management.internal.security.ResourceOperation; import org.apache.geode.security.NotAuthorizedException; +import org.apache.geode.security.ResourcePermission; import org.springframework.shell.core.Parser; import org.springframework.shell.event.ParseResult; @@ -109,7 +110,10 @@ public class CommandProcessor { // do general authorization check here Method method = parseResult.getMethod(); ResourceOperation resourceOperation = method.getAnnotation(ResourceOperation.class); - this.securityService.authorize(resourceOperation); + if (resourceOperation != null) { + this.securityService.authorize(resourceOperation.resource(), + resourceOperation.operation(), resourceOperation.target(), ResourcePermission.ALL); + } result = executionStrategy.execute(parseResult); if (result instanceof Result) { http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java b/geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java index 365c6ae..7f088ce 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java @@ -16,6 +16,9 @@ package org.apache.geode.management.internal.security; import org.apache.geode.internal.security.SecurityService; import org.apache.geode.security.GemFireSecurityException; +import org.apache.geode.security.ResourcePermission; +import org.apache.geode.security.ResourcePermission.Resource; +import org.apache.geode.security.ResourcePermission.Operation; /** * AccessControlMBean Implementation. This retrieves JMXPrincipal from AccessController and performs @@ -34,7 +37,8 @@ public class AccessControlMBean implements AccessControlMXBean { @Override public boolean authorize(String resource, String permission) { try { - this.securityService.authorize(resource, permission); + this.securityService.authorize(Resource.valueOf(resource), Operation.valueOf(permission), + ResourcePermission.ALL, ResourcePermission.ALL); return true; } catch (GemFireSecurityException e) { return false; http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java b/geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java index 345d688..47260bc 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java @@ -14,10 +14,14 @@ */ package org.apache.geode.management.internal.security; +import org.apache.commons.lang.StringUtils; import org.apache.geode.internal.security.SecurityService; import org.apache.geode.management.internal.ManagementConstants; import org.apache.geode.security.GemFireSecurityException; import org.apache.geode.security.ResourcePermission; +import org.apache.geode.security.ResourcePermission.Resource; +import org.apache.geode.security.ResourcePermission.Operation; +import org.apache.geode.security.ResourcePermission.Target; import java.io.ObjectInputStream; import java.util.Set; @@ -255,8 +259,15 @@ public class MBeanServerWrapper implements MBeanServerForwarder { ResourcePermission defaultValue) { String resource = (String) descriptor.getFieldValue("resource"); String operationCode = (String) descriptor.getFieldValue("operation"); + String targetCode = (String) descriptor.getFieldValue("target"); if (resource != null && operationCode != null) { - return new ResourcePermission(resource, operationCode); + if (StringUtils.isBlank(targetCode)) { + return new ResourcePermission(Resource.valueOf(resource), Operation.valueOf(operationCode), + targetCode); + } else { + return new ResourcePermission(Resource.valueOf(resource), Operation.valueOf(operationCode), + Target.valueOf(targetCode).getName()); + } } return defaultValue; } http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java b/geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java index db3a187..1eb684f 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java @@ -14,17 +14,17 @@ */ package org.apache.geode.management.internal.security; +import org.apache.geode.security.ResourcePermission.Target; +import org.apache.geode.security.ResourcePermission.Operation; +import org.apache.geode.security.ResourcePermission.Resource; + import java.lang.annotation.ElementType; import java.lang.annotation.Inherited; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; import javax.management.DescriptorKey; -import org.apache.geode.security.ResourcePermission.Operation; -import org.apache.geode.security.ResourcePermission.Resource; - -@Target({ElementType.METHOD, ElementType.TYPE}) [email protected]({ElementType.METHOD, ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) @Inherited public @interface ResourceOperation { @@ -33,4 +33,8 @@ public @interface ResourceOperation { @DescriptorKey("operation") Operation operation() default Operation.NULL; + + @DescriptorKey("target") + Target target() default Target.ALL; + } http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java b/geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java index 45da464..6a920d7 100644 --- a/geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java +++ b/geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java @@ -14,6 +14,9 @@ */ package org.apache.geode.security; +import org.apache.commons.lang.StringUtils; +import org.apache.geode.cache.Region; +import org.apache.geode.management.internal.security.ResourceOperation; import org.apache.shiro.authz.permission.WildcardPermission; /** @@ -25,8 +28,7 @@ import org.apache.shiro.authz.permission.WildcardPermission; */ public class ResourcePermission extends WildcardPermission { - public static String ALL_REGIONS = "*"; - public static String ALL_KEYS = "*"; + public static String ALL = "*"; public enum Resource { NULL, CLUSTER, DATA @@ -36,49 +38,64 @@ public class ResourcePermission extends WildcardPermission { NULL, MANAGE, WRITE, READ } + // when ALL is specified, we need it to convert to string "*" instead of "ALL". + public enum Target { + ALL(ResourcePermission.ALL), DISK, GATEWAY, QUERY, JAR; + + private String name; + + Target() {} + + Target(String name) { + this.name = name; + } + + public String getName() { + if (name != null) { + return name; + } + return name(); + } + } + // these default values are used when creating a lock around an operation private Resource resource = Resource.NULL; private Operation operation = Operation.NULL; - private String regionName = ALL_REGIONS; - private String key = ALL_KEYS; + private String target = ALL; + private String key = ALL; public ResourcePermission() { - this(Resource.NULL, Operation.NULL); - } - - public ResourcePermission(String resource, String operation) { - this(resource, operation, ALL_REGIONS); + this(Resource.NULL, Operation.NULL, ALL, ALL); } - public ResourcePermission(String resource, String operation, String regionName) { - this(resource, operation, regionName, ALL_KEYS); + public ResourcePermission(Resource resource, Operation operation) { + this(resource, operation, ALL, ALL); } - public ResourcePermission(String resource, String operation, String regionName, String key) { - this((resource == null) ? Resource.NULL : Resource.valueOf(resource.toUpperCase()), - (operation == null) ? Operation.NULL : Operation.valueOf(operation.toUpperCase()), - regionName, key); + public ResourcePermission(Resource resource, Operation operation, String target) { + this(resource, operation, target, ALL); } - public ResourcePermission(Resource resource, Operation operation) { - this(resource, operation, ALL_REGIONS); + public ResourcePermission(Resource resource, Operation operation, Target target) { + this(resource, operation, target, ALL); } - public ResourcePermission(Resource resource, Operation operation, String regionName) { - this(resource, operation, regionName, ALL_KEYS); + public ResourcePermission(Resource resource, Operation operation, Target target, + String targetKey) { + this(resource, operation, target.getName(), targetKey); } - public ResourcePermission(Resource resource, Operation operation, String regionName, String key) { + public ResourcePermission(Resource resource, Operation operation, String target, String key) { if (resource != null) this.resource = resource; if (operation != null) this.operation = operation; - if (regionName != null) - this.regionName = regionName; + if (target != null) + this.target = StringUtils.stripStart(target, Region.SEPARATOR); if (key != null) this.key = key; - setParts(this.resource + ":" + this.operation + ":" + this.regionName + ":" + this.key, true); + setParts(this.resource + ":" + this.operation + ":" + this.target + ":" + this.key, true); } /** @@ -98,8 +115,8 @@ public class ResourcePermission extends WildcardPermission { /** * returns the regionName, could be "*", meaning all regions */ - public String getRegionName() { - return regionName; + public String getTarget() { + return target; } /** @@ -111,12 +128,12 @@ public class ResourcePermission extends WildcardPermission { @Override public String toString() { - if (ALL_REGIONS.equals(regionName)) { + if (ALL.equals(target)) { return getResource() + ":" + getOperation(); - } else if (ALL_KEYS.equals(key)) { - return resource + ":" + operation + ":" + regionName; + } else if (ALL.equals(key)) { + return resource + ":" + operation + ":" + target; } else { - return resource + ":" + operation + ":" + regionName + ":" + key; + return resource + ":" + operation + ":" + target + ":" + key; } } http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java index b728b27..51f8c5e 100644 --- a/geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java +++ b/geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java @@ -14,17 +14,20 @@ */ package org.apache.geode.management.internal.security; -import static org.junit.Assert.*; - -import org.apache.shiro.authz.permission.WildcardPermission; -import org.junit.Test; -import org.junit.experimental.categories.Category; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import org.apache.geode.security.ResourcePermission; +import org.apache.geode.security.ResourcePermission.Target; import org.apache.geode.security.ResourcePermission.Operation; import org.apache.geode.security.ResourcePermission.Resource; import org.apache.geode.test.junit.categories.SecurityTest; import org.apache.geode.test.junit.categories.UnitTest; +import org.apache.shiro.authz.permission.WildcardPermission; +import org.junit.Test; +import org.junit.experimental.categories.Category; @Category({UnitTest.class, SecurityTest.class}) public class ResourcePermissionTest { @@ -35,7 +38,7 @@ public class ResourcePermissionTest { context = new ResourcePermission(); assertEquals(Resource.NULL, context.getResource()); assertEquals(Operation.NULL, context.getOperation()); - assertEquals(ResourcePermission.ALL_REGIONS, context.getRegionName()); + assertEquals(ResourcePermission.ALL, context.getTarget()); } @Test @@ -49,32 +52,46 @@ public class ResourcePermissionTest { context = new ResourcePermission(); assertEquals(Resource.NULL, context.getResource()); assertEquals(Operation.NULL, context.getOperation()); - assertEquals(ResourcePermission.ALL_REGIONS, context.getRegionName()); + assertEquals(ResourcePermission.ALL, context.getTarget()); context = new ResourcePermission(); assertEquals(Resource.NULL, context.getResource()); assertEquals(Operation.NULL, context.getOperation()); - assertEquals(ResourcePermission.ALL_REGIONS, context.getRegionName()); + assertEquals(ResourcePermission.ALL, context.getTarget()); - context = new ResourcePermission("DATA", null, null); + context = new ResourcePermission(Resource.DATA, null); assertEquals(Resource.DATA, context.getResource()); assertEquals(Operation.NULL, context.getOperation()); - assertEquals(ResourcePermission.ALL_REGIONS, context.getRegionName()); + assertEquals(ResourcePermission.ALL, context.getTarget()); - context = new ResourcePermission("CLUSTER", null, null); + context = new ResourcePermission(Resource.CLUSTER, null); assertEquals(Resource.CLUSTER, context.getResource()); assertEquals(Operation.NULL, context.getOperation()); - assertEquals(ResourcePermission.ALL_REGIONS, context.getRegionName()); + assertEquals(ResourcePermission.ALL, context.getTarget()); - context = new ResourcePermission(null, "MANAGE", "REGIONA"); + context = new ResourcePermission(null, Operation.MANAGE, "REGIONA"); assertEquals(Resource.NULL, context.getResource()); assertEquals(Operation.MANAGE, context.getOperation()); - assertEquals("REGIONA", context.getRegionName()); + assertEquals("REGIONA", context.getTarget()); - context = new ResourcePermission("DATA", "MANAGE", "REGIONA"); + context = new ResourcePermission(Resource.DATA, Operation.MANAGE, "REGIONA"); assertEquals(Resource.DATA, context.getResource()); assertEquals(Operation.MANAGE, context.getOperation()); - assertEquals("REGIONA", context.getRegionName()); + assertEquals("REGIONA", context.getTarget()); + + context = new ResourcePermission(Resource.CLUSTER, Operation.MANAGE); + assertEquals(Resource.CLUSTER, context.getResource()); + assertEquals(Operation.MANAGE, context.getOperation()); + assertEquals(ResourcePermission.ALL, context.getTarget()); + + // make sure "ALL" in the resource "DATA" means regionName won't be converted to * + context = new ResourcePermission(Resource.DATA, Operation.READ, "ALL"); + assertEquals(Resource.DATA, context.getResource()); + assertEquals(Operation.READ, context.getOperation()); + assertEquals("ALL", context.getTarget()); + + context = new ResourcePermission(Resource.CLUSTER, Operation.READ, Target.ALL); + assertEquals(context.getTarget(), ResourcePermission.ALL); } @Test @@ -82,28 +99,28 @@ public class ResourcePermissionTest { context = new ResourcePermission(); assertEquals("NULL:NULL", context.toString()); - context = new ResourcePermission("DATA", "MANAGE"); + context = new ResourcePermission(Resource.DATA, Operation.MANAGE); assertEquals("DATA:MANAGE", context.toString()); - context = new ResourcePermission("DATA", "MANAGE", "REGIONA"); + context = new ResourcePermission(Resource.DATA, Operation.MANAGE, "REGIONA"); assertEquals("DATA:MANAGE:REGIONA", context.toString()); - context = new ResourcePermission("data", "manage"); + context = new ResourcePermission(Resource.DATA, Operation.MANAGE); assertEquals("DATA:MANAGE", context.toString()); } @Test public void testImples() { WildcardPermission role = new WildcardPermission("*:read"); - role.implies(new ResourcePermission("data", "read")); - role.implies(new ResourcePermission("cluster", "read")); + role.implies(new ResourcePermission(Resource.DATA, Operation.READ)); + role.implies(new ResourcePermission(Resource.CLUSTER, Operation.READ)); role = new WildcardPermission("*:read:*"); - role.implies(new ResourcePermission("data", "read", "testRegion")); - role.implies(new ResourcePermission("cluster", "read", "anotherRegion", "key1")); + role.implies(new ResourcePermission(Resource.DATA, Operation.READ, "testRegion")); + role.implies(new ResourcePermission(Resource.CLUSTER, Operation.READ, "anotherRegion", "key1")); role = new WildcardPermission("data:*:testRegion"); - role.implies(new ResourcePermission("data", "read", "testRegion")); - role.implies(new ResourcePermission("data", "write", "testRegion")); + role.implies(new ResourcePermission(Resource.DATA, Operation.READ, "testRegion")); + role.implies(new ResourcePermission(Resource.DATA, Operation.WRITE, "testRegion")); } } http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java b/geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java index 3f8f4d9..ea4b60d 100644 --- a/geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java +++ b/geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java @@ -15,6 +15,9 @@ package org.apache.geode.management.internal.security; import org.apache.geode.security.ResourcePermission; +import org.apache.geode.security.ResourcePermission.Target; +import org.apache.geode.security.ResourcePermission.Operation; +import org.apache.geode.security.ResourcePermission.Resource; import org.apache.shiro.authz.Permission; import java.util.ArrayList; @@ -24,19 +27,27 @@ public class TestCommand { public static ResourcePermission none = null; public static ResourcePermission everyOneAllowed = new ResourcePermission(); - public static ResourcePermission dataRead = new ResourcePermission("DATA", "READ"); - public static ResourcePermission dataWrite = new ResourcePermission("DATA", "WRITE"); - public static ResourcePermission dataManage = new ResourcePermission("DATA", "MANAGE"); - - public static ResourcePermission regionARead = new ResourcePermission("DATA", "READ", "RegionA"); + public static ResourcePermission dataRead = new ResourcePermission(Resource.DATA, Operation.READ); + public static ResourcePermission dataWrite = + new ResourcePermission(Resource.DATA, Operation.WRITE); + public static ResourcePermission dataManage = + new ResourcePermission(Resource.DATA, Operation.MANAGE); + public static ResourcePermission diskManage = + new ResourcePermission(Resource.CLUSTER, Operation.MANAGE, Target.DISK); + + public static ResourcePermission regionARead = + new ResourcePermission(Resource.DATA, Operation.READ, "RegionA"); public static ResourcePermission regionAWrite = - new ResourcePermission("DATA", "WRITE", "RegionA"); + new ResourcePermission(Resource.DATA, Operation.WRITE, "RegionA"); public static ResourcePermission regionAManage = - new ResourcePermission("DATA", "MANAGE", "RegionA"); + new ResourcePermission(Resource.DATA, Operation.MANAGE, "RegionA"); - public static ResourcePermission clusterRead = new ResourcePermission("CLUSTER", "READ"); - public static ResourcePermission clusterWrite = new ResourcePermission("CLUSTER", "WRITE"); - public static ResourcePermission clusterManage = new ResourcePermission("CLUSTER", "MANAGE"); + public static ResourcePermission clusterRead = + new ResourcePermission(Resource.CLUSTER, Operation.READ); + public static ResourcePermission clusterWrite = + new ResourcePermission(Resource.CLUSTER, Operation.WRITE); + public static ResourcePermission clusterManage = + new ResourcePermission(Resource.CLUSTER, Operation.MANAGE); private static List<TestCommand> testCommands = new ArrayList<>(); @@ -116,7 +127,7 @@ public class TestCommand { createTestCommand("backup disk-store --dir=foo", dataRead); createTestCommand("list disk-stores", clusterRead); createTestCommand("create disk-store --name=foo --dir=bar", dataManage); - createTestCommand("compact disk-store --name=foo", dataManage); + createTestCommand("compact disk-store --name=foo", diskManage); createTestCommand("compact offline-disk-store --name=foo --disk-dirs=bar", null); createTestCommand("upgrade offline-disk-store --name=foo --disk-dirs=bar", null); createTestCommand("describe disk-store --name=foo --member=baz", clusterRead); http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/test/java/org/apache/geode/security/SimpleSecurityManagerTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/security/SimpleSecurityManagerTest.java b/geode-core/src/test/java/org/apache/geode/security/SimpleSecurityManagerTest.java index 1c73b57..bb22588 100644 --- a/geode-core/src/test/java/org/apache/geode/security/SimpleSecurityManagerTest.java +++ b/geode-core/src/test/java/org/apache/geode/security/SimpleSecurityManagerTest.java @@ -24,6 +24,9 @@ import org.apache.geode.test.junit.categories.UnitTest; import org.junit.Before; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.apache.geode.security.ResourcePermission.Target; +import org.apache.geode.security.ResourcePermission.Operation; +import org.apache.geode.security.ResourcePermission.Resource; import java.util.Properties; @@ -61,12 +64,12 @@ public class SimpleSecurityManagerTest { @Test public void testAuthorization() { - ResourcePermission permission = new ResourcePermission("CLUSTER", "READ"); + ResourcePermission permission = new ResourcePermission(Resource.CLUSTER, Operation.READ); assertTrue(manager.authorize("clusterRead", permission)); assertTrue(manager.authorize("cluster", permission)); assertFalse(manager.authorize("data", permission)); - permission = new ResourcePermission("DATA", "WRITE", "regionA", "key1"); + permission = new ResourcePermission(Resource.DATA, Operation.WRITE, "regionA", "key1"); assertTrue(manager.authorize("data", permission)); assertTrue(manager.authorize("dataWrite", permission)); assertTrue(manager.authorize("dataWriteRegionA", permission)); @@ -76,12 +79,12 @@ public class SimpleSecurityManagerTest { @Test public void testMultipleRoleAuthorization() { - ResourcePermission permission = new ResourcePermission("CLUSTER", "READ"); + ResourcePermission permission = new ResourcePermission(Resource.CLUSTER, Operation.READ); assertTrue(manager.authorize("clusterRead,clusterWrite", permission)); assertTrue(manager.authorize("cluster,data", permission)); assertFalse(manager.authorize("clusterWrite,data", permission)); - permission = new ResourcePermission("DATA", "WRITE", "regionA", "key1"); + permission = new ResourcePermission(Resource.DATA, Operation.WRITE, "regionA", "key1"); assertTrue(manager.authorize("data,cluster", permission)); assertTrue(manager.authorize("dataWrite,clusterWrite", permission)); } http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/test/java/org/apache/geode/security/TestSecurityManager.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/security/TestSecurityManager.java b/geode-core/src/test/java/org/apache/geode/security/TestSecurityManager.java index 6080b5d..e42cf6b 100644 --- a/geode-core/src/test/java/org/apache/geode/security/TestSecurityManager.java +++ b/geode-core/src/test/java/org/apache/geode/security/TestSecurityManager.java @@ -19,6 +19,8 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.commons.io.IOUtils; import org.apache.geode.management.internal.security.ResourceConstants; import org.apache.shiro.authz.Permission; +import org.apache.geode.security.ResourcePermission.Operation; +import org.apache.geode.security.ResourcePermission.Resource; import java.io.IOException; import java.io.InputStream; @@ -238,8 +240,8 @@ public class TestSecurityManager implements SecurityManager { String regionPart = (regionNames != null) ? regionNames : "*"; String keyPart = (keys != null) ? keys : "*"; - role.permissions - .add(new ResourcePermission(resourcePart, operationPart, regionPart, keyPart)); + role.permissions.add(new ResourcePermission(Resource.valueOf(resourcePart), + Operation.valueOf(operationPart), regionPart, keyPart)); } roleMap.put(role.name, role); http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt ---------------------------------------------------------------------- diff --git a/geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt b/geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt index 9cff80d..6a6e0c1 100644 --- a/geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt +++ b/geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt @@ -150,6 +150,7 @@ org/apache/geode/internal/cache/operations/ContainsKeyOperationContext org/apache/geode/security/ResourcePermission org/apache/geode/security/ResourcePermission$Operation org/apache/geode/security/ResourcePermission$Resource +org/apache/geode/security/ResourcePermission$Target org/apache/geode/distributed/internal/tcpserver/LocatorCancelException org.apache.geode.internal.security.SecurableCommunicationChannel org/apache/geode/internal/security/shiro/GeodeAuthenticationToken http://git-wip-us.apache.org/repos/asf/geode/blob/f5410435/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java ---------------------------------------------------------------------- diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java index 6d66947..04d74ce 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java @@ -21,6 +21,9 @@ import org.apache.geode.internal.security.SecurityServiceFactory; import org.apache.geode.security.GemFireSecurityException; import org.springframework.stereotype.Component; +import org.apache.geode.security.ResourcePermission.Operation; +import org.apache.geode.security.ResourcePermission.Resource; + @Component("securityService") public class RestSecurityService { @@ -45,7 +48,8 @@ public class RestSecurityService { public boolean authorize(String resource, String operation, String region, String key) { try { - securityService.authorize(resource, operation, region, key); + securityService.authorize(Resource.valueOf(resource), Operation.valueOf(operation), region, + key); return true; } catch (GemFireSecurityException ex) { return false;
