GEODE-3108: update lucene security permission to use "LUCENE" target
* have lucene module define it's own target for plugability * added string based target support for SecurityService Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/b71f3a63 Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/b71f3a63 Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/b71f3a63 Branch: refs/heads/feature/GEM-1483 Commit: b71f3a636175a26974fae88a5090ed4d94ccce73 Parents: 2913494 Author: Jinmei Liao <jil...@pivotal.io> Authored: Mon Jul 17 11:10:31 2017 -0700 Committer: Jinmei Liao <jil...@pivotal.io> Committed: Thu Jul 20 11:16:29 2017 -0700 ---------------------------------------------------------------------- .../security/IntegratedSecurityService.java | 32 ++++++++++++-------- .../internal/security/SecurityService.java | 15 +++++---- .../test/dunit/rules/MemberStarterRule.java | 9 ++---- .../internal/cli/LuceneIndexCommands.java | 26 ++++++++++------ .../functions/LuceneCreateIndexFunction.java | 8 ----- .../internal/security/LucenePermission.java | 21 +++++++++++++ .../lucene/LuceneCommandsSecurityDUnitTest.java | 18 +++++------ 7 files changed, 76 insertions(+), 53 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/b71f3a63/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java b/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java index 323ab67..da8a466 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java +++ b/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java @@ -14,8 +14,22 @@ */ package org.apache.geode.internal.security; +import java.io.IOException; +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.logging.log4j.Logger; +import org.apache.shiro.SecurityUtils; +import org.apache.shiro.ShiroException; +import org.apache.shiro.subject.Subject; +import org.apache.shiro.subject.support.SubjectThreadState; +import org.apache.shiro.util.ThreadContext; +import org.apache.shiro.util.ThreadState; + import org.apache.geode.GemFireIOException; import org.apache.geode.internal.cache.EntryEventImpl; import org.apache.geode.internal.logging.LogService; @@ -33,19 +47,6 @@ 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; -import org.apache.shiro.subject.Subject; -import org.apache.shiro.subject.support.SubjectThreadState; -import org.apache.shiro.util.ThreadContext; -import org.apache.shiro.util.ThreadState; - -import java.io.IOException; -import java.security.AccessController; -import java.util.Properties; -import java.util.Set; -import java.util.concurrent.Callable; /** * Security service with SecurityManager and an optional PostProcessor. @@ -285,6 +286,11 @@ public class IntegratedSecurityService implements SecurityService { } @Override + public void authorize(Resource resource, Operation operation, String target) { + authorize(new ResourcePermission(resource, operation, target, ResourcePermission.ALL)); + } + + @Override public void authorize(final ResourcePermission context) { if (context == null) { return; http://git-wip-us.apache.org/repos/asf/geode/blob/b71f3a63/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 b00e0f4..0fdb57d 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 @@ -14,17 +14,18 @@ */ 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.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.Resource; 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; - -import java.util.Properties; -import java.util.concurrent.Callable; public interface SecurityService { @@ -52,6 +53,8 @@ public interface SecurityService { default void authorize(Resource resource, Operation operation, Target target) {} + default void authorize(Resource resource, Operation operation, String target) {} + default void authorizeClusterManage() {} default void authorizeClusterWrite() {} http://git-wip-us.apache.org/repos/asf/geode/blob/b71f3a63/geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java index 5b4f2c1..226fd6c 100644 --- a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java +++ b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java @@ -27,13 +27,6 @@ import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT; import static org.apache.geode.distributed.ConfigurationProperties.NAME; import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER; -import org.apache.geode.distributed.DistributedSystem; -import org.apache.geode.distributed.internal.InternalDistributedSystem; -import org.apache.geode.internal.AvailablePortHelper; -import org.apache.geode.security.SecurityManager; -import org.junit.rules.ExternalResource; -import org.junit.rules.TemporaryFolder; - import java.io.File; import java.io.IOException; import java.util.Properties; @@ -41,6 +34,8 @@ import java.util.Properties; import org.junit.rules.ExternalResource; import org.junit.rules.TemporaryFolder; +import org.apache.geode.distributed.DistributedSystem; +import org.apache.geode.distributed.internal.InternalDistributedSystem; import org.apache.geode.internal.AvailablePortHelper; import org.apache.geode.security.SecurityManager; http://git-wip-us.apache.org/repos/asf/geode/blob/b71f3a63/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java index 6ddacf1..30bef69 100755 --- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java +++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java @@ -40,6 +40,7 @@ import org.apache.geode.cache.lucene.internal.cli.functions.LuceneDescribeIndexF import org.apache.geode.cache.lucene.internal.cli.functions.LuceneDestroyIndexFunction; import org.apache.geode.cache.lucene.internal.cli.functions.LuceneListIndexFunction; import org.apache.geode.cache.lucene.internal.cli.functions.LuceneSearchIndexFunction; +import org.apache.geode.cache.lucene.internal.security.LucenePermission; import org.apache.geode.distributed.DistributedMember; import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.internal.cache.execute.AbstractExecution; @@ -57,11 +58,9 @@ import org.apache.geode.management.internal.cli.result.ResultBuilder; import org.apache.geode.management.internal.cli.result.TabularResultData; import org.apache.geode.management.internal.cli.shell.Gfsh; import org.apache.geode.management.internal.configuration.domain.XmlEntity; -import org.apache.geode.management.internal.security.ResourceOperation; import org.apache.geode.security.GemFireSecurityException; import org.apache.geode.security.ResourcePermission.Operation; import org.apache.geode.security.ResourcePermission.Resource; -import org.apache.geode.security.ResourcePermission.Target; /** * The LuceneIndexCommands class encapsulates all Geode shell (Gfsh) commands related to Lucene @@ -86,11 +85,12 @@ public class LuceneIndexCommands implements GfshCommand { @CliCommand(value = LuceneCliStrings.LUCENE_LIST_INDEX, help = LuceneCliStrings.LUCENE_LIST_INDEX__HELP) @CliMetaData(relatedTopic = {CliStrings.TOPIC_GEODE_REGION, CliStrings.TOPIC_GEODE_DATA}) - @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.READ, target = Target.QUERY) public Result listIndex(@CliOption(key = LuceneCliStrings.LUCENE_LIST_INDEX__STATS, specifiedDefaultValue = "true", unspecifiedDefaultValue = "false", help = LuceneCliStrings.LUCENE_LIST_INDEX__STATS__HELP) final boolean stats) { + getSecurityService().authorize(Resource.CLUSTER, Operation.READ, LucenePermission.TARGET); + try { return toTabularResult(getIndexListing(), stats); } catch (FunctionInvocationTargetException ignore) { @@ -170,8 +170,6 @@ public class LuceneIndexCommands implements GfshCommand { help = LuceneCliStrings.LUCENE_CREATE_INDEX__HELP) @CliMetaData(relatedTopic = {CliStrings.TOPIC_GEODE_REGION, CliStrings.TOPIC_GEODE_DATA}) // TODO : Add optionContext for indexName - @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.MANAGE, - target = Target.QUERY) public Result createIndex(@CliOption(key = LuceneCliStrings.LUCENE__INDEX_NAME, mandatory = true, help = LuceneCliStrings.LUCENE_CREATE_INDEX__NAME__HELP) final String indexName, @@ -188,12 +186,17 @@ public class LuceneIndexCommands implements GfshCommand { Result result; XmlEntity xmlEntity = null; + // Every lucene index potentially writes to disk. + getSecurityService().authorize(Resource.CLUSTER, Operation.MANAGE, LucenePermission.TARGET); + try { final InternalCache cache = getCache(); + // trim fields for any leading trailing spaces. String[] trimmedFields = Arrays.stream(fields).map(String::trim).toArray(String[]::new); LuceneIndexInfo indexInfo = new LuceneIndexInfo(indexName, regionPath, trimmedFields, analyzers); + final ResultCollector<?, ?> rc = this.executeFunctionOnAllMembers(createIndexFunction, indexInfo); final List<CliFunctionResult> funcResults = (List<CliFunctionResult>) rc.getResult(); @@ -232,7 +235,6 @@ public class LuceneIndexCommands implements GfshCommand { @CliCommand(value = LuceneCliStrings.LUCENE_DESCRIBE_INDEX, help = LuceneCliStrings.LUCENE_DESCRIBE_INDEX__HELP) @CliMetaData(relatedTopic = {CliStrings.TOPIC_GEODE_REGION, CliStrings.TOPIC_GEODE_DATA}) - @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.READ, target = Target.QUERY) public Result describeIndex( @CliOption(key = LuceneCliStrings.LUCENE__INDEX_NAME, mandatory = true, help = LuceneCliStrings.LUCENE_DESCRIBE_INDEX__NAME__HELP) final String indexName, @@ -240,6 +242,9 @@ public class LuceneIndexCommands implements GfshCommand { @CliOption(key = LuceneCliStrings.LUCENE__REGION_PATH, mandatory = true, optionContext = ConverterHint.REGION_PATH, help = LuceneCliStrings.LUCENE_DESCRIBE_INDEX__REGION_HELP) final String regionPath) { + + getSecurityService().authorize(Resource.CLUSTER, Operation.READ, LucenePermission.TARGET); + try { LuceneIndexInfo indexInfo = new LuceneIndexInfo(indexName, regionPath); return toTabularResult(getIndexDetails(indexInfo), true); @@ -292,8 +297,10 @@ public class LuceneIndexCommands implements GfshCommand { @CliOption(key = LuceneCliStrings.LUCENE_SEARCH_INDEX__KEYSONLY, unspecifiedDefaultValue = "false", help = LuceneCliStrings.LUCENE_SEARCH_INDEX__KEYSONLY__HELP) boolean keysOnly) { + + getSecurityService().authorizeRegionRead(regionPath); + try { - getSecurityService().authorizeRegionRead(regionPath); LuceneQueryInfo queryInfo = new LuceneQueryInfo(indexName, regionPath, queryString, defaultField, limit, keysOnly); int pageSize = Integer.MAX_VALUE; @@ -320,14 +327,13 @@ public class LuceneIndexCommands implements GfshCommand { @CliCommand(value = LuceneCliStrings.LUCENE_DESTROY_INDEX, help = LuceneCliStrings.LUCENE_DESTROY_INDEX__HELP) @CliMetaData(relatedTopic = {CliStrings.TOPIC_GEODE_REGION, CliStrings.TOPIC_GEODE_DATA}) - @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.MANAGE, - target = Target.QUERY) public Result destroyIndex(@CliOption(key = LuceneCliStrings.LUCENE__INDEX_NAME, help = LuceneCliStrings.LUCENE_DESTROY_INDEX__NAME__HELP) final String indexName, @CliOption(key = LuceneCliStrings.LUCENE__REGION_PATH, mandatory = true, optionContext = ConverterHint.REGION_PATH, help = LuceneCliStrings.LUCENE_DESTROY_INDEX__REGION_HELP) final String regionPath) { + if (StringUtils.isBlank(regionPath) || regionPath.equals(Region.SEPARATOR)) { return ResultBuilder.createInfoResult( CliStrings.format(LuceneCliStrings.LUCENE_DESTROY_INDEX__MSG__REGION_CANNOT_BE_EMPTY)); @@ -338,6 +344,8 @@ public class LuceneIndexCommands implements GfshCommand { CliStrings.format(LuceneCliStrings.LUCENE_DESTROY_INDEX__MSG__INDEX_CANNOT_BE_EMPTY)); } + getSecurityService().authorize(Resource.CLUSTER, Operation.MANAGE, LucenePermission.TARGET); + Result result; try { List<CliFunctionResult> accumulatedResults = new ArrayList<>(); http://git-wip-us.apache.org/repos/asf/geode/blob/b71f3a63/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/functions/LuceneCreateIndexFunction.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/functions/LuceneCreateIndexFunction.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/functions/LuceneCreateIndexFunction.java index 2d3f8d2..65cc361 100644 --- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/functions/LuceneCreateIndexFunction.java +++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/functions/LuceneCreateIndexFunction.java @@ -33,14 +33,10 @@ import org.apache.geode.cache.lucene.internal.cli.LuceneCliStrings; import org.apache.geode.cache.lucene.internal.cli.LuceneIndexDetails; import org.apache.geode.cache.lucene.internal.cli.LuceneIndexInfo; import org.apache.geode.internal.InternalEntity; -import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.management.internal.cli.CliUtil; import org.apache.geode.management.internal.cli.functions.CliFunctionResult; import org.apache.geode.management.internal.cli.i18n.CliStrings; import org.apache.geode.management.internal.configuration.domain.XmlEntity; -import org.apache.geode.security.ResourcePermission.Operation; -import org.apache.geode.security.ResourcePermission.Resource; -import org.apache.geode.security.ResourcePermission.Target; /** @@ -94,10 +90,6 @@ public class LuceneCreateIndexFunction extends FunctionAdapter implements Intern REGION_PATH.validateName(indexInfo.getRegionPath()); - // Every lucene index potentially writes to disk. - ((InternalCache) cache).getSecurityService().authorize(Resource.CLUSTER, Operation.WRITE, - Target.DISK); - indexFactory.create(indexInfo.getIndexName(), indexInfo.getRegionPath()); // TODO - update cluster configuration by returning a valid XmlEntity http://git-wip-us.apache.org/repos/asf/geode/blob/b71f3a63/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/security/LucenePermission.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/security/LucenePermission.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/security/LucenePermission.java new file mode 100644 index 0000000..d1d7935 --- /dev/null +++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/security/LucenePermission.java @@ -0,0 +1,21 @@ +/* + * 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 org.apache.geode.cache.lucene.internal.security; + +public class LucenePermission { + + public static final String TARGET = "LUCENE"; +} http://git-wip-us.apache.org/repos/asf/geode/blob/b71f3a63/geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneCommandsSecurityDUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneCommandsSecurityDUnitTest.java b/geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneCommandsSecurityDUnitTest.java index 7116601..c2ba153 100644 --- a/geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneCommandsSecurityDUnitTest.java +++ b/geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneCommandsSecurityDUnitTest.java @@ -26,7 +26,6 @@ import java.util.Properties; import junitparams.JUnitParamsRunner; import junitparams.Parameters; import org.junit.Before; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -47,7 +46,6 @@ import org.apache.geode.test.dunit.rules.MemberVM; import org.apache.geode.test.junit.categories.DistributedTest; import org.apache.geode.test.junit.categories.SecurityTest; -@Ignore("GEODE-3134") @Category({DistributedTest.class, SecurityTest.class}) @RunWith(JUnitParamsRunner.class) public class LuceneCommandsSecurityDUnitTest { @@ -77,8 +75,8 @@ public class LuceneCommandsSecurityDUnitTest { protected UserNameAndExpectedResponse[] getCreateIndexUserNameAndExpectedResponses() { return new UserNameAndExpectedResponse[] { new UserNameAndExpectedResponse("noPermissions", true, - "Unauthorized. Reason : noPermissions not authorized for CLUSTER:MANAGE:QUERY"), - new UserNameAndExpectedResponse("clusterManageQuery", false, + "Unauthorized. Reason : noPermissions not authorized for CLUSTER:MANAGE:LUCENE"), + new UserNameAndExpectedResponse("clusterManageLucene", false, "Successfully created lucene index")}; } @@ -123,8 +121,8 @@ public class LuceneCommandsSecurityDUnitTest { protected UserNameAndExpectedResponse[] getListIndexesUserNameAndExpectedResponses() { return new UserNameAndExpectedResponse[] { new UserNameAndExpectedResponse("noPermissions", true, - "Unauthorized. Reason : noPermissions not authorized for CLUSTER:READ:QUERY"), - new UserNameAndExpectedResponse("clusterReadQuery", false, "Index Name")}; + "Unauthorized. Reason : noPermissions not authorized for CLUSTER:READ:LUCENE"), + new UserNameAndExpectedResponse("clusterReadLucene", false, "Index Name")}; } @Test @@ -147,8 +145,8 @@ public class LuceneCommandsSecurityDUnitTest { protected UserNameAndExpectedResponse[] getDescribeIndexUserNameAndExpectedResponses() { return new UserNameAndExpectedResponse[] { new UserNameAndExpectedResponse("noPermissions", true, - "Unauthorized. Reason : noPermissions not authorized for CLUSTER:READ:QUERY"), - new UserNameAndExpectedResponse("clusterReadQuery", false, "Index Name")}; + "Unauthorized. Reason : noPermissions not authorized for CLUSTER:READ:LUCENE"), + new UserNameAndExpectedResponse("clusterReadLucene", false, "Index Name")}; } @Test @@ -171,8 +169,8 @@ public class LuceneCommandsSecurityDUnitTest { protected UserNameAndExpectedResponse[] getDestroyIndexUserNameAndExpectedResponses() { return new UserNameAndExpectedResponse[] { new UserNameAndExpectedResponse("noPermissions", true, - "Unauthorized. Reason : noPermissions not authorized for CLUSTER:MANAGE:QUERY"), - new UserNameAndExpectedResponse("clusterManageQuery", false, + "Unauthorized. Reason : noPermissions not authorized for CLUSTER:MANAGE:LUCENE"), + new UserNameAndExpectedResponse("clusterManageLucene", false, "Successfully destroyed lucene index")}; }
