Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 1ABE6200CAD for ; Mon, 5 Jun 2017 18:46:33 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 1983D160BBB; Mon, 5 Jun 2017 16:46:33 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id BA6C8160BD4 for ; Mon, 5 Jun 2017 18:46:30 +0200 (CEST) Received: (qmail 34622 invoked by uid 500); 5 Jun 2017 16:46:25 -0000 Mailing-List: contact commits-help@geode.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@geode.apache.org Delivered-To: mailing list commits@geode.apache.org Received: (qmail 34401 invoked by uid 99); 5 Jun 2017 16:46:24 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 05 Jun 2017 16:46:24 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id A6637E000B; Mon, 5 Jun 2017 16:46:24 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: klund@apache.org To: commits@geode.apache.org Date: Mon, 05 Jun 2017 16:46:27 -0000 Message-Id: <7951c83608a34d469c5125b3878332a6@git.apache.org> In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [4/9] geode git commit: Use immutable SecurityService archived-at: Mon, 05 Jun 2017 16:46:33 -0000 http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommands.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommands.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommands.java index cb9c4fe..62113de 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommands.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommands.java @@ -33,8 +33,6 @@ import org.apache.geode.cache.execute.ResultCollector; import org.apache.geode.cache.partition.PartitionRebalanceInfo; import org.apache.geode.distributed.DistributedMember; import org.apache.geode.internal.cache.InternalCache; -import org.apache.geode.internal.security.IntegratedSecurityService; -import org.apache.geode.internal.security.SecurityService; import org.apache.geode.management.DistributedRegionMXBean; import org.apache.geode.management.ManagementService; import org.apache.geode.management.cli.CliMetaData; @@ -49,6 +47,7 @@ import org.apache.geode.management.internal.cli.functions.DataCommandFunction; import org.apache.geode.management.internal.cli.functions.ExportDataFunction; import org.apache.geode.management.internal.cli.functions.ImportDataFunction; import org.apache.geode.management.internal.cli.functions.RebalanceFunction; +import org.apache.geode.management.internal.cli.functions.SelectExecStep; import org.apache.geode.management.internal.cli.i18n.CliStrings; import org.apache.geode.management.internal.cli.multistep.CLIMultiStepHelper; import org.apache.geode.management.internal.cli.multistep.CLIStep; @@ -86,13 +85,21 @@ import java.util.concurrent.TimeoutException; */ public class DataCommands implements GfshCommand { - final int resultItemCount = 9; + private final int resultItemCount = 9; private final ExportDataFunction exportDataFunction = new ExportDataFunction(); private final ImportDataFunction importDataFunction = new ImportDataFunction(); - private SecurityService securityService = IntegratedSecurityService.getSecurityService(); + @Override + public Gfsh getGfsh() { + return Gfsh.getCurrentInstance(); + } + + @Override + public InternalCache getCache() { + return (InternalCache) CacheFactory.getAnyInstance(); + } @CliCommand(value = CliStrings.REBALANCE, help = CliStrings.REBALANCE__HELP) @CliMetaData(relatedTopic = {CliStrings.TOPIC_GEODE_DATA, CliStrings.TOPIC_GEODE_REGION}) @@ -146,15 +153,15 @@ public class DataCommands implements GfshCommand { return executeRebalanceWithTimeout(includeRegions, excludeRegions, simulate); } - public ExecuteRebalanceWithTimeout(String[] includedRegions, String[] excludedRegions, - boolean toSimulate) { + ExecuteRebalanceWithTimeout(String[] includedRegions, String[] excludedRegions, + boolean toSimulate) { includeRegions = includedRegions; excludeRegions = excludedRegions; simulate = toSimulate; } - public Result executeRebalanceWithTimeout(String[] includeRegions, String[] excludeRegions, - boolean simulate) { + Result executeRebalanceWithTimeout(String[] includeRegions, String[] excludeRegions, + boolean simulate) { Result result = null; try { @@ -451,14 +458,14 @@ public class DataCommands implements GfshCommand { return result; } - public boolean checkMemberPresence(DistributedMember dsMember, InternalCache cache) { + private boolean checkMemberPresence(DistributedMember dsMember, InternalCache cache) { // check if member's presence just before executing function // this is to avoid running a function on departed members #47248 Set dsMemberList = CliUtil.getAllNormalMembers(cache); return dsMemberList.contains(dsMember); } - public String listOfAllMembers(ArrayList dsMemberList) { + private String listOfAllMembers(ArrayList dsMemberList) { StringBuilder listMembersId = new StringBuilder(); for (int j = 0; j < dsMemberList.size() - 1; j++) { listMembersId.append(dsMemberList.get(j).getId()); @@ -467,8 +474,9 @@ public class DataCommands implements GfshCommand { return listMembersId.toString(); } - protected CompositeResultData toCompositeResultData(CompositeResultData rebalanceResulteData, - ArrayList rstlist, int index, boolean simulate, InternalCache cache) { + private CompositeResultData toCompositeResultData(CompositeResultData rebalanceResulteData, + ArrayList rstlist, int index, + boolean simulate, InternalCache cache) { // add only if there are any valid regions in results if (rstlist.size() > resultItemCount && StringUtils.isNotEmpty(rstlist.get(resultItemCount))) { @@ -623,7 +631,7 @@ public class DataCommands implements GfshCommand { return rebalanceResultData; } - public DistributedMember getAssociatedMembers(String region, final InternalCache cache) { + private DistributedMember getAssociatedMembers(String region, final InternalCache cache) { DistributedRegionMXBean bean = ManagementService.getManagementService(cache).getDistributedRegionMXBean(region); @@ -741,7 +749,7 @@ public class DataCommands implements GfshCommand { optionContext = ConverterHint.MEMBERIDNAME, mandatory = true, help = CliStrings.EXPORT_DATA__MEMBER__HELP) String memberNameOrId) { - this.securityService.authorizeRegionRead(regionName); + getCache().getSecurityService().authorizeRegionRead(regionName); final DistributedMember targetMember = CliUtil.getDistributedMemberByNameOrId(memberNameOrId); Result result; @@ -799,7 +807,7 @@ public class DataCommands implements GfshCommand { @CliOption(key = CliStrings.IMPORT_DATA__INVOKE_CALLBACKS, unspecifiedDefaultValue = "false", help = CliStrings.IMPORT_DATA__INVOKE_CALLBACKS__HELP) boolean invokeCallbacks) { - this.securityService.authorizeRegionWrite(regionName); + getCache().getSecurityService().authorizeRegionWrite(regionName); Result result; @@ -860,8 +868,8 @@ public class DataCommands implements GfshCommand { @CliOption(key = {CliStrings.PUT__PUTIFABSENT}, help = CliStrings.PUT__PUTIFABSENT__HELP, unspecifiedDefaultValue = "false") boolean putIfAbsent) { - this.securityService.authorizeRegionWrite(regionPath); InternalCache cache = getCache(); + cache.getSecurityService().authorizeRegionWrite(regionPath); DataCommandResult dataResult; if (StringUtils.isEmpty(regionPath)) { return makePresentationResult(DataCommandResult.createPutResult(key, null, null, @@ -931,9 +939,9 @@ public class DataCommands implements GfshCommand { @CliOption(key = CliStrings.GET__LOAD, unspecifiedDefaultValue = "true", specifiedDefaultValue = "true", help = CliStrings.GET__LOAD__HELP) Boolean loadOnCacheMiss) { - this.securityService.authorizeRegionRead(regionPath, key); InternalCache cache = getCache(); + cache.getSecurityService().authorizeRegionRead(regionPath, key); DataCommandResult dataResult; if (StringUtils.isEmpty(regionPath)) { @@ -959,7 +967,7 @@ public class DataCommands implements GfshCommand { request.setRegionName(regionPath); request.setValueClass(valueClass); request.setLoadOnCacheMiss(loadOnCacheMiss); - Subject subject = this.securityService.getSubject(); + Subject subject = cache.getSecurityService().getSubject(); if (subject != null) { request.setPrincipal(subject.getPrincipal()); } @@ -970,7 +978,8 @@ public class DataCommands implements GfshCommand { false); } } else { - dataResult = getfn.get(null, key, keyClass, valueClass, regionPath, loadOnCacheMiss); + dataResult = getfn.get(null, key, keyClass, valueClass, regionPath, loadOnCacheMiss, + cache.getSecurityService()); } dataResult.setKeyClass(keyClass); if (valueClass != null) { @@ -996,7 +1005,7 @@ public class DataCommands implements GfshCommand { help = CliStrings.LOCATE_ENTRY__RECURSIVE__HELP, unspecifiedDefaultValue = "false") boolean recursive) { - this.securityService.authorizeRegionRead(regionPath, key); + getCache().getSecurityService().authorizeRegionRead(regionPath, key); DataCommandResult dataResult; @@ -1059,9 +1068,9 @@ public class DataCommands implements GfshCommand { } if (removeAllKeys) { - this.securityService.authorizeRegionWrite(regionPath); + cache.getSecurityService().authorizeRegionWrite(regionPath); } else { - this.securityService.authorizeRegionWrite(regionPath, key); + cache.getSecurityService().authorizeRegionWrite(regionPath, key); } @SuppressWarnings("rawtypes") @@ -1107,7 +1116,7 @@ public class DataCommands implements GfshCommand { } Object[] arguments = new Object[] {query, stepName, interactive}; - CLIStep exec = new DataCommandFunction.SelectExecStep(arguments); + CLIStep exec = new SelectExecStep(arguments); CLIStep display = new DataCommandFunction.SelectDisplayStep(arguments); CLIStep move = new DataCommandFunction.SelectMoveStep(arguments); CLIStep quit = new DataCommandFunction.SelectQuitStep(arguments); @@ -1127,14 +1136,15 @@ public class DataCommands implements GfshCommand { } private static class MemberPRInfo { - public ArrayList dsMemberList; + ArrayList dsMemberList; public String region; - public MemberPRInfo() { + MemberPRInfo() { region = ""; dsMemberList = new ArrayList<>(); } + @Override public boolean equals(Object o2) { return o2 != null && this.region.equals(((MemberPRInfo) o2).region); } http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java index 30d840a..268dc7a 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java @@ -31,7 +31,6 @@ import org.apache.geode.management.internal.cli.functions.ListDeployedFunction; import org.apache.geode.management.internal.cli.functions.UndeployFunction; import org.apache.geode.management.internal.cli.i18n.CliStrings; import org.apache.geode.management.internal.cli.remote.CommandExecutionContext; -import org.apache.geode.management.internal.cli.result.CommandResultException; import org.apache.geode.management.internal.cli.result.FileResult; import org.apache.geode.management.internal.cli.result.ResultBuilder; import org.apache.geode.management.internal.cli.result.TabularResultData; @@ -50,11 +49,9 @@ import java.util.List; import java.util.Map; import java.util.Set; - /** * Commands for deploying, un-deploying and listing files deployed using the command line shell. - *

- * + * * @see GfshCommand * @since GemFire 7.0 */ @@ -85,7 +82,7 @@ public class DeployCommands implements GfshCommand { // since deploy function can potentially do a lot of damage to security, this action should // require these following privileges - SecurityService securityService = SecurityService.getSecurityService(); + SecurityService securityService = getCache().getSecurityService(); securityService.authorizeClusterManage(); securityService.authorizeClusterWrite(); securityService.authorizeDataManage(); @@ -282,7 +279,7 @@ public class DeployCommands implements GfshCommand { return true; } - return (getGfsh() != null && getGfsh().isConnectedAndReady()); + return getGfsh() != null && getGfsh().isConnectedAndReady(); } /** @@ -296,15 +293,15 @@ public class DeployCommands implements GfshCommand { Map paramValueMap = parseResult.getParamValueStrings(); String jar = paramValueMap.get("jar"); - jar = (jar == null) ? null : jar.trim(); + jar = jar == null ? null : jar.trim(); String dir = paramValueMap.get("dir"); - dir = (dir == null) ? null : dir.trim(); + dir = dir == null ? null : dir.trim(); String group = paramValueMap.get("group"); - group = (group == null) ? null : group.trim(); + group = group == null ? null : group.trim(); - String jarOrDir = (jar != null ? jar : dir); + String jarOrDir = jar != null ? jar : dir; if (jar == null && dir == null) { return ResultBuilder.createUserErrorResult( @@ -325,7 +322,7 @@ public class DeployCommands implements GfshCommand { if (dir != null) { String message = "\nDeploying files: " + fileResult.getFormattedFileList() + "\nTotal file size is: " - + this.numFormatter.format(((double) fileResult.computeFileSizeTotal() / 1048576)) + + this.numFormatter.format((double) fileResult.computeFileSizeTotal() / 1048576) + "MB\n\nContinue? "; if (readYesNo(message, Response.YES) == Response.NO) { http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java index d46024d..86296b4 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java @@ -37,8 +37,7 @@ import java.util.Set; /** * Encapsulates common functionality for implementing command classes for the Geode shell (gfsh). - *

- * + * * @see org.apache.geode.cache.Cache * @see org.apache.geode.cache.execute.FunctionService * @see org.apache.geode.distributed.DistributedMember @@ -48,13 +47,13 @@ import java.util.Set; @SuppressWarnings("unused") public interface GfshCommand extends CommandMarker { default String convertDefaultValue(final String from, final String to) { - return (CliMetaData.ANNOTATION_DEFAULT_VALUE.equals(from) ? to : from); + return CliMetaData.ANNOTATION_DEFAULT_VALUE.equals(from) ? to : from; } default String toString(final Boolean condition, final String trueValue, final String falseValue) { - return (Boolean.TRUE.equals(condition) ? StringUtils.defaultIfBlank(trueValue, "true") - : StringUtils.defaultIfBlank(falseValue, "false")); + return Boolean.TRUE.equals(condition) ? StringUtils.defaultIfBlank(trueValue, "true") + : StringUtils.defaultIfBlank(falseValue, "false"); } default String toString(final Throwable t, final boolean printStackTrace) { @@ -70,12 +69,12 @@ public interface GfshCommand extends CommandMarker { } default boolean isConnectedAndReady() { - return (getGfsh() != null && getGfsh().isConnectedAndReady()); + return getGfsh() != null && getGfsh().isConnectedAndReady(); } default ClusterConfigurationService getSharedConfiguration() { InternalLocator locator = InternalLocator.getLocator(); - return (locator == null) ? null : locator.getSharedConfiguration(); + return locator == null ? null : locator.getSharedConfiguration(); } default void persistClusterConfiguration(Result result, Runnable runnable) { @@ -92,11 +91,11 @@ public interface GfshCommand extends CommandMarker { } default boolean isDebugging() { - return (getGfsh() != null && getGfsh().getDebug()); + return getGfsh() != null && getGfsh().getDebug(); } default boolean isLogging() { - return (getGfsh() != null); + return getGfsh() != null; } default InternalCache getCache() { @@ -122,7 +121,7 @@ public interface GfshCommand extends CommandMarker { /** * Gets all members in the GemFire distributed system/cache. - * + * * @param cache the GemFire cache. * @return all members in the GemFire distributed system/cache. * @see org.apache.geode.management.internal.cli.CliUtil#getAllMembers(org.apache.geode.internal.cache.InternalCache) http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java index b3d9675..628547e 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java @@ -26,8 +26,6 @@ import org.apache.geode.distributed.DistributedMember; import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.internal.cache.execute.AbstractExecution; import org.apache.geode.internal.lang.StringUtils; -import org.apache.geode.internal.security.IntegratedSecurityService; -import org.apache.geode.internal.security.SecurityService; import org.apache.geode.management.cli.CliMetaData; import org.apache.geode.management.cli.ConverterHint; import org.apache.geode.management.cli.Result; @@ -41,7 +39,6 @@ import org.apache.geode.management.internal.cli.functions.CreateIndexFunction; import org.apache.geode.management.internal.cli.functions.DestroyIndexFunction; import org.apache.geode.management.internal.cli.functions.ListIndexFunction; 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.ErrorResultData; import org.apache.geode.management.internal.cli.result.InfoResultData; import org.apache.geode.management.internal.cli.result.ResultBuilder; @@ -67,8 +64,7 @@ import java.util.concurrent.atomic.AtomicReference; /** * The IndexCommands class encapsulates all GemFire shell (Gfsh) commands related to indexes defined * in GemFire. - *

- * + * * @see GfshCommand * @see org.apache.geode.management.internal.cli.domain.IndexDetails * @see org.apache.geode.management.internal.cli.functions.ListIndexFunction @@ -84,7 +80,11 @@ public class IndexCommands implements GfshCommand { private static final Set indexDefinitions = Collections.synchronizedSet(new HashSet()); - private SecurityService securityService = IntegratedSecurityService.getSecurityService(); + @Override + public Set getMembers(final InternalCache cache) { + // TODO determine what this does (as it is untested and unmockable!) + return CliUtil.getAllMembers(cache); + } @CliCommand(value = CliStrings.LIST_INDEX, help = CliStrings.LIST_INDEX__HELP) @CliMetaData(shellOnly = false, @@ -196,7 +196,7 @@ public class IndexCommands implements GfshCommand { Result result = null; AtomicReference xmlEntity = new AtomicReference<>(); - this.securityService.authorizeRegionManage(regionPath); + getCache().getSecurityService().authorizeRegionManage(regionPath); try { final Cache cache = CacheFactory.getAnyInstance(); @@ -312,7 +312,6 @@ public class IndexCommands implements GfshCommand { result = ResultBuilder.createGemFireErrorResult(e.getMessage()); } - if (xmlEntity.get() != null) { persistClusterConfiguration(result, () -> getSharedConfiguration().addXmlEntity(xmlEntity.get(), group)); @@ -355,9 +354,9 @@ public class IndexCommands implements GfshCommand { // requires data manage permission on all regions if (StringUtils.isNotBlank(regionPath)) { regionName = regionPath.startsWith("/") ? regionPath.substring(1) : regionPath; - this.securityService.authorizeRegionManage(regionName); + getCache().getSecurityService().authorizeRegionManage(regionName); } else { - this.securityService.authorizeDataManage(); + getCache().getSecurityService().authorizeDataManage(); } IndexInfo indexInfo = new IndexInfo(indexName, regionName); @@ -479,7 +478,7 @@ public class IndexCommands implements GfshCommand { Result result = null; XmlEntity xmlEntity = null; - this.securityService.authorizeRegionManage(regionPath); + getCache().getSecurityService().authorizeRegionManage(regionPath); int idxType = IndexInfo.RANGE_INDEX; @@ -641,14 +640,13 @@ public class IndexCommands implements GfshCommand { final InfoResultData infoResult = ResultBuilder.createInfoResultData(); infoResult.addLine(CliStrings.CLEAR_DEFINED_INDEX__SUCCESS__MSG); return ResultBuilder.buildResult(infoResult); - } @CliAvailabilityIndicator({CliStrings.LIST_INDEX, CliStrings.CREATE_INDEX, CliStrings.DESTROY_INDEX, CliStrings.CREATE_DEFINED_INDEXES, CliStrings.CLEAR_DEFINED_INDEXES, CliStrings.DEFINE_INDEX}) public boolean indexCommandsAvailable() { - return (!CliUtil.isGfshVM() || (getGfsh() != null && getGfsh().isConnectedAndReady())); + return !CliUtil.isGfshVM() || getGfsh() != null && getGfsh().isConnectedAndReady(); } protected static class IndexStatisticsDetailsAdapter { @@ -664,28 +662,28 @@ public class IndexCommands implements GfshCommand { } public String getNumberOfKeys() { - return (getIndexStatisticsDetails() != null - ? StringUtils.defaultString(getIndexStatisticsDetails().getNumberOfKeys()) : ""); + return getIndexStatisticsDetails() != null + ? StringUtils.defaultString(getIndexStatisticsDetails().getNumberOfKeys()) : ""; } public String getNumberOfUpdates() { - return (getIndexStatisticsDetails() != null - ? StringUtils.defaultString(getIndexStatisticsDetails().getNumberOfUpdates()) : ""); + return getIndexStatisticsDetails() != null + ? StringUtils.defaultString(getIndexStatisticsDetails().getNumberOfUpdates()) : ""; } public String getNumberOfValues() { - return (getIndexStatisticsDetails() != null - ? StringUtils.defaultString(getIndexStatisticsDetails().getNumberOfValues()) : ""); + return getIndexStatisticsDetails() != null + ? StringUtils.defaultString(getIndexStatisticsDetails().getNumberOfValues()) : ""; } public String getTotalUpdateTime() { - return (getIndexStatisticsDetails() != null - ? StringUtils.defaultString(getIndexStatisticsDetails().getTotalUpdateTime()) : ""); + return getIndexStatisticsDetails() != null + ? StringUtils.defaultString(getIndexStatisticsDetails().getTotalUpdateTime()) : ""; } public String getTotalUses() { - return (getIndexStatisticsDetails() != null - ? StringUtils.defaultString(getIndexStatisticsDetails().getTotalUses()) : ""); + return getIndexStatisticsDetails() != null + ? StringUtils.defaultString(getIndexStatisticsDetails().getTotalUses()) : ""; } } http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java index e2164a3..ff7aa02 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java @@ -94,9 +94,6 @@ public class DataCommandFunction extends FunctionAdapter implements InternalEnti protected static final String SELECT_STEP_EXEC = "SELECT_EXEC"; private static final int NESTED_JSON_LENGTH = 20; - // this needs to be static so that it won't get serialized - private static SecurityService securityService = SecurityService.getSecurityService(); - @Override public String getId() { return DataCommandFunction.class.getName(); @@ -136,7 +133,7 @@ public class DataCommandFunction extends FunctionAdapter implements InternalEnti } DataCommandResult result = null; if (request.isGet()) { - result = get(request); + result = get(request, cache.getSecurityService()); } else if (request.isLocateEntry()) { result = locateEntry(request); } else if (request.isPut()) { @@ -169,13 +166,14 @@ public class DataCommandFunction extends FunctionAdapter implements InternalEnti return remove(key, keyClass, regionName, removeAllKeys); } - public DataCommandResult get(DataCommandRequest request) { + public DataCommandResult get(DataCommandRequest request, SecurityService securityService) { String key = request.getKey(); String keyClass = request.getKeyClass(); String valueClass = request.getValueClass(); String regionName = request.getRegionName(); Boolean loadOnCacheMiss = request.isLoadOnCacheMiss(); - return get(request.getPrincipal(), key, keyClass, valueClass, regionName, loadOnCacheMiss); + return get(request.getPrincipal(), key, keyClass, valueClass, regionName, loadOnCacheMiss, + securityService); } public DataCommandResult locateEntry(DataCommandRequest request) { @@ -296,7 +294,7 @@ public class DataCommandFunction extends FunctionAdapter implements InternalEnti List list, AtomicInteger nestedObjectCount) throws GfJsonException { for (Object object : selectResults) { // Post processing - object = securityService.postProcess(principal, null, null, object, false); + object = getCache().getSecurityService().postProcess(principal, null, null, object, false); if (object instanceof Struct) { StructImpl impl = (StructImpl) object; @@ -434,7 +432,7 @@ public class DataCommandFunction extends FunctionAdapter implements InternalEnti @SuppressWarnings({"rawtypes"}) public DataCommandResult get(Object principal, String key, String keyClass, String valueClass, - String regionName, Boolean loadOnCacheMiss) { + String regionName, Boolean loadOnCacheMiss, SecurityService securityService) { InternalCache cache = getCache(); @@ -836,7 +834,7 @@ public class DataCommandFunction extends FunctionAdapter implements InternalEnti return list; } - private static DataCommandResult cachedResult = null; + static DataCommandResult cachedResult = null; public static class SelectDisplayStep extends CLIMultiStepHelper.LocalStep { @@ -915,107 +913,6 @@ public class DataCommandFunction extends FunctionAdapter implements InternalEnti } } - public static class SelectExecStep extends CLIMultiStepHelper.RemoteStep { - - private static final long serialVersionUID = 1L; - - private static SecurityService securityService = SecurityService.getSecurityService(); - - public SelectExecStep(Object[] arguments) { - super(SELECT_STEP_EXEC, arguments); - } - - @Override - public Result exec() { - String remainingQuery = (String) commandArguments[0]; - boolean interactive = (Boolean) commandArguments[2]; - DataCommandResult result = _select(remainingQuery); - int endCount = 0; - cachedResult = result; - if (interactive) { - endCount = getPageSize(); - } else { - if (result.getSelectResult() != null) { - endCount = result.getSelectResult().size(); - } - } - if (interactive) { - return result.pageResult(0, endCount, SELECT_STEP_DISPLAY); - } else { - return CLIMultiStepHelper.createBannerResult(new String[] {}, new Object[] {}, - SELECT_STEP_END); - } - } - - public DataCommandResult _select(String query) { - InternalCache cache = (InternalCache) CacheFactory.getAnyInstance(); - DataCommandResult dataResult; - - if (StringUtils.isEmpty(query)) { - dataResult = DataCommandResult.createSelectInfoResult(null, null, -1, null, - CliStrings.QUERY__MSG__QUERY_EMPTY, false); - return dataResult; - } - - Object array[] = DataCommands.replaceGfshEnvVar(query, CommandExecutionContext.getShellEnv()); - query = (String) array[1]; - query = addLimit(query); - - @SuppressWarnings("deprecation") - QCompiler compiler = new QCompiler(); - Set regionsInQuery; - try { - CompiledValue compiledQuery = compiler.compileQuery(query); - Set regions = new HashSet<>(); - compiledQuery.getRegionsInQuery(regions, null); - - // authorize data read on these regions - for (String region : regions) { - securityService.authorizeRegionRead(region); - } - - regionsInQuery = Collections.unmodifiableSet(regions); - if (regionsInQuery.size() > 0) { - Set members = - DataCommands.getQueryRegionsAssociatedMembers(regionsInQuery, cache, false); - if (members != null && members.size() > 0) { - DataCommandFunction function = new DataCommandFunction(); - DataCommandRequest request = new DataCommandRequest(); - request.setCommand(CliStrings.QUERY); - request.setQuery(query); - Subject subject = securityService.getSubject(); - if (subject != null) { - request.setPrincipal(subject.getPrincipal()); - } - dataResult = DataCommands.callFunctionForRegion(request, function, members); - dataResult.setInputQuery(query); - return dataResult; - } else { - return DataCommandResult.createSelectInfoResult(null, null, -1, null, CliStrings.format( - CliStrings.QUERY__MSG__REGIONS_NOT_FOUND, regionsInQuery.toString()), false); - } - } else { - return DataCommandResult.createSelectInfoResult(null, null, -1, null, - CliStrings.format(CliStrings.QUERY__MSG__INVALID_QUERY, - "Region mentioned in query probably missing /"), - false); - } - } catch (QueryInvalidException qe) { - logger.error("{} Failed Error {}", query, qe.getMessage(), qe); - return DataCommandResult.createSelectInfoResult(null, null, -1, null, - CliStrings.format(CliStrings.QUERY__MSG__INVALID_QUERY, qe.getMessage()), false); - } - } - - private String addLimit(String query) { - if (StringUtils.containsIgnoreCase(query, " limit") - || StringUtils.containsIgnoreCase(query, " count(")) { - return query; - } - return query + " limit " + getFetchSize(); - } - } - public static class SelectQuitStep extends CLIMultiStepHelper.RemoteStep { public SelectQuitStep(Object[] arguments) { @@ -1063,7 +960,7 @@ public class DataCommandFunction extends FunctionAdapter implements InternalEnti return pageSize; } - private static int getFetchSize() { + static int getFetchSize() { return CommandExecutionContext.getShellFetchSize(); } http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SelectExecStep.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SelectExecStep.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SelectExecStep.java new file mode 100644 index 0000000..8d9e57b --- /dev/null +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SelectExecStep.java @@ -0,0 +1,137 @@ +/* + * 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.management.internal.cli.functions; + +import org.apache.commons.lang.StringUtils; +import org.apache.geode.cache.CacheFactory; +import org.apache.geode.cache.query.QueryInvalidException; +import org.apache.geode.cache.query.internal.CompiledValue; +import org.apache.geode.cache.query.internal.QCompiler; +import org.apache.geode.distributed.DistributedMember; +import org.apache.geode.internal.cache.InternalCache; +import org.apache.geode.internal.logging.LogService; +import org.apache.geode.management.cli.Result; +import org.apache.geode.management.internal.cli.commands.DataCommands; +import org.apache.geode.management.internal.cli.domain.DataCommandRequest; +import org.apache.geode.management.internal.cli.domain.DataCommandResult; +import org.apache.geode.management.internal.cli.i18n.CliStrings; +import org.apache.geode.management.internal.cli.multistep.CLIMultiStepHelper; +import org.apache.geode.management.internal.cli.remote.CommandExecutionContext; +import org.apache.logging.log4j.Logger; +import org.apache.shiro.subject.Subject; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +public class SelectExecStep extends CLIMultiStepHelper.RemoteStep { + private static final Logger logger = LogService.getLogger(); + + private static final long serialVersionUID = 1L; + + public SelectExecStep(Object[] arguments) { + super(DataCommandFunction.SELECT_STEP_EXEC, arguments); + } + + @Override + public Result exec() { + String remainingQuery = (String) commandArguments[0]; + boolean interactive = (Boolean) commandArguments[2]; + DataCommandResult result = _select(remainingQuery); + int endCount = 0; + DataCommandFunction.cachedResult = result; + if (interactive) { + endCount = DataCommandFunction.getPageSize(); + } else { + if (result.getSelectResult() != null) { + endCount = result.getSelectResult().size(); + } + } + if (interactive) { + return result.pageResult(0, endCount, DataCommandFunction.SELECT_STEP_DISPLAY); + } else { + return CLIMultiStepHelper.createBannerResult(new String[] {}, new Object[] {}, + DataCommandFunction.SELECT_STEP_END); + } + } + + public DataCommandResult _select(String query) { + InternalCache cache = (InternalCache) CacheFactory.getAnyInstance(); + DataCommandResult dataResult; + + if (StringUtils.isEmpty(query)) { + dataResult = DataCommandResult.createSelectInfoResult(null, null, -1, null, + CliStrings.QUERY__MSG__QUERY_EMPTY, false); + return dataResult; + } + + Object array[] = DataCommands.replaceGfshEnvVar(query, CommandExecutionContext.getShellEnv()); + query = (String) array[1]; + query = addLimit(query); + + @SuppressWarnings("deprecation") + QCompiler compiler = new QCompiler(); + Set regionsInQuery; + try { + CompiledValue compiledQuery = compiler.compileQuery(query); + Set regions = new HashSet<>(); + compiledQuery.getRegionsInQuery(regions, null); + + // authorize data read on these regions + for (String region : regions) { + cache.getSecurityService().authorizeRegionRead(region); + } + + regionsInQuery = Collections.unmodifiableSet(regions); + if (regionsInQuery.size() > 0) { + Set members = + DataCommands.getQueryRegionsAssociatedMembers(regionsInQuery, cache, false); + if (members != null && members.size() > 0) { + DataCommandFunction function = new DataCommandFunction(); + DataCommandRequest request = new DataCommandRequest(); + request.setCommand(CliStrings.QUERY); + request.setQuery(query); + Subject subject = cache.getSecurityService().getSubject(); + if (subject != null) { + request.setPrincipal(subject.getPrincipal()); + } + dataResult = DataCommands.callFunctionForRegion(request, function, members); + dataResult.setInputQuery(query); + return dataResult; + } else { + return DataCommandResult.createSelectInfoResult(null, null, -1, null, CliStrings + .format(CliStrings.QUERY__MSG__REGIONS_NOT_FOUND, regionsInQuery.toString()), false); + } + } else { + return DataCommandResult.createSelectInfoResult(null, null, -1, null, + CliStrings.format(CliStrings.QUERY__MSG__INVALID_QUERY, + "Region mentioned in query probably missing /"), + false); + } + } catch (QueryInvalidException qe) { + logger.error("{} Failed Error {}", query, qe.getMessage(), qe); + return DataCommandResult.createSelectInfoResult(null, null, -1, null, + CliStrings.format(CliStrings.QUERY__MSG__INVALID_QUERY, qe.getMessage()), false); + } + } + + private String addLimit(String query) { + if (StringUtils.containsIgnoreCase(query, " limit") + || StringUtils.containsIgnoreCase(query, " count(")) { + return query; + } + return query + " limit " + DataCommandFunction.getFetchSize(); + } +} http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/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 c2c6e14..6bc5904 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 @@ -14,6 +14,7 @@ */ package org.apache.geode.management.internal.cli.remote; +import org.apache.geode.internal.security.DisabledSecurityService; import org.apache.geode.internal.security.IntegratedSecurityService; import org.apache.geode.internal.security.SecurityService; import org.apache.geode.management.cli.CommandProcessingException; @@ -49,16 +50,18 @@ public class CommandProcessor { private volatile boolean isStopped = false; - private SecurityService securityService = IntegratedSecurityService.getSecurityService(); + private final SecurityService securityService; public CommandProcessor() throws ClassNotFoundException, IOException { - this(null); + this(null, new DisabledSecurityService()); } - public CommandProcessor(Properties cacheProperties) throws ClassNotFoundException, IOException { + public CommandProcessor(Properties cacheProperties, SecurityService securityService) + throws ClassNotFoundException, IOException { this.gfshParser = new GfshParser(cacheProperties); this.executionStrategy = new RemoteExecutionStrategy(); this.logWrapper = LogWrapper.getInstance(); + this.securityService = securityService; } protected RemoteExecutionStrategy getExecutionStrategy() { http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/MemberCommandService.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/MemberCommandService.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/MemberCommandService.java index a19c5cb..25ff549 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/MemberCommandService.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/MemberCommandService.java @@ -17,24 +17,23 @@ package org.apache.geode.management.internal.cli.remote; import java.io.IOException; import java.util.Map; -import org.apache.geode.cache.Cache; +import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.management.cli.CommandService; import org.apache.geode.management.cli.CommandServiceException; import org.apache.geode.management.cli.CommandStatement; import org.apache.geode.management.cli.Result; -/** - */ public class MemberCommandService extends CommandService { private final Object modLock = new Object(); - private Cache cache; + private InternalCache cache; private CommandProcessor commandProcessor; - public MemberCommandService(Cache cache) throws CommandServiceException { + public MemberCommandService(InternalCache cache) throws CommandServiceException { this.cache = cache; try { - this.commandProcessor = new CommandProcessor(cache.getDistributedSystem().getProperties()); + this.commandProcessor = new CommandProcessor(cache.getDistributedSystem().getProperties(), + cache.getSecurityService()); } catch (ClassNotFoundException e) { throw new CommandServiceException("Could not load commands.", e); } catch (IOException e) { @@ -70,12 +69,4 @@ public class MemberCommandService extends CommandService { return (this.cache != null && !this.cache.isClosed()); } - // @Override - // public void stop() { - // cache = null; - // synchronized (modLock) { - // this.commandProcessor.stop(); - // this.commandProcessor = null; - // } - // } } http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/RecreateCacheFunction.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/RecreateCacheFunction.java b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/RecreateCacheFunction.java index a00a79e..f25d1af 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/RecreateCacheFunction.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/RecreateCacheFunction.java @@ -16,7 +16,7 @@ package org.apache.geode.management.internal.configuration.functions; import org.apache.geode.cache.execute.Function; import org.apache.geode.cache.execute.FunctionContext; -import org.apache.geode.distributed.DistributedSystem; +import org.apache.geode.distributed.internal.InternalDistributedSystem; import org.apache.geode.internal.InternalEntity; import org.apache.geode.internal.cache.CacheConfig; import org.apache.geode.internal.cache.GemFireCacheImpl; @@ -28,7 +28,7 @@ public class RecreateCacheFunction implements Function, InternalEntity { public void execute(FunctionContext context) { CliFunctionResult result = null; InternalCache cache = GemFireCacheImpl.getInstance(); - DistributedSystem ds = cache.getDistributedSystem(); + InternalDistributedSystem ds = cache.getInternalDistributedSystem(); CacheConfig cacheConfig = cache.getCacheConfig(); try { cache.close("Re-create Cache", true, true); http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/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 6514a33..dbc6c6b 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 @@ -26,7 +26,11 @@ import org.apache.geode.security.GemFireSecurityException; */ public class AccessControlMBean implements AccessControlMXBean { - private SecurityService securityService = IntegratedSecurityService.getSecurityService(); + private final SecurityService securityService; + + public AccessControlMBean(SecurityService securityService) { + this.securityService = securityService; + } @Override public boolean authorize(String resource, String permission) { http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/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 fe79efb..345d688 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,6 +14,11 @@ */ package org.apache.geode.management.internal.security; +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 java.io.ObjectInputStream; import java.util.Set; import javax.management.Attribute; @@ -42,25 +47,22 @@ import javax.management.ReflectionException; import javax.management.loading.ClassLoaderRepository; import javax.management.remote.MBeanServerForwarder; -import org.apache.geode.internal.security.IntegratedSecurityService; -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; - /** * This class intercepts all MBean requests for GemFire MBeans and passed it to * ManagementInterceptor for authorization * * @since Geode 1.0 - * */ public class MBeanServerWrapper implements MBeanServerForwarder { + + // TODO: make volatile or verify this is thread-safe private MBeanServer mbs; - private SecurityService securityService = IntegratedSecurityService.getSecurityService(); + private final SecurityService securityService; - public MBeanServerWrapper() {} + public MBeanServerWrapper(SecurityService securityService) { + this.securityService = securityService; + } private void checkDomain(ObjectName name) { if (ManagementConstants.OBJECTNAME__DEFAULTDOMAIN.equals(name.getDomain())) http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/AbstractCommandsController.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/AbstractCommandsController.java b/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/AbstractCommandsController.java index 54c29f8..0a18ec5 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/AbstractCommandsController.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/AbstractCommandsController.java @@ -12,7 +12,6 @@ * or implied. See the License for the specific language governing permissions and limitations under * the License. */ - package org.apache.geode.management.internal.web.controllers; import org.apache.geode.internal.cache.GemFireCacheImpl; @@ -20,8 +19,6 @@ import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.internal.lang.StringUtils; import org.apache.geode.internal.logging.LogService; import org.apache.geode.internal.logging.log4j.LogMarker; -import org.apache.geode.internal.security.IntegratedSecurityService; -import org.apache.geode.internal.security.SecurityService; import org.apache.geode.internal.util.ArrayUtils; import org.apache.geode.management.DistributedSystemMXBean; import org.apache.geode.management.ManagementService; @@ -85,8 +82,6 @@ public abstract class AbstractCommandsController { private MemberMXBean managingMemberMXBeanProxy; - private SecurityService securityService = IntegratedSecurityService.getSecurityService(); - private Class accessControlKlass; private InternalCache getCache() { @@ -576,10 +571,9 @@ public abstract class AbstractCommandsController { return new ResponseEntity(result, HttpStatus.OK); } }; - return this.securityService.associateWith(callable); + return getCache().getSecurityService().associateWith(callable); } - /** * Executes the specified command as entered by the user using the GemFire Shell (Gfsh). Note, * Gfsh performs validation of the command during parsing before sending the command to the http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptor.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptor.java b/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptor.java index 56d9b9e..ffe1895 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptor.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptor.java @@ -14,10 +14,11 @@ */ package org.apache.geode.management.internal.web.controllers.support; -import org.apache.geode.cache.Cache; import org.apache.geode.distributed.internal.DistributionConfig; +import org.apache.geode.internal.cache.GemFireCacheImpl; +import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.internal.logging.LogService; -import org.apache.geode.internal.security.IntegratedSecurityService; +import org.apache.geode.internal.security.DisabledSecurityService; import org.apache.geode.internal.security.SecurityService; import org.apache.geode.management.internal.cli.multistep.CLIMultiStepHelper; import org.apache.geode.management.internal.security.ResourceConstants; @@ -48,9 +49,7 @@ public class LoginHandlerInterceptor extends HandlerInterceptorAdapter { private static final Logger logger = LogService.getLogger(); - private Cache cache; - - private SecurityService securityService = IntegratedSecurityService.getSecurityService(); + private final SecurityService securityService; private static final ThreadLocal> ENV = new ThreadLocal>() { @@ -65,10 +64,26 @@ public class LoginHandlerInterceptor extends HandlerInterceptorAdapter { protected static final String SECURITY_VARIABLE_REQUEST_HEADER_PREFIX = DistributionConfig.SECURITY_PREFIX_NAME; + public LoginHandlerInterceptor() { + this(findSecurityService()); + } + + LoginHandlerInterceptor(SecurityService securityService) { + this.securityService = securityService; + } + public static Map getEnvironment() { return ENV.get(); } + private static SecurityService findSecurityService() { + InternalCache cache = GemFireCacheImpl.getInstance(); + if (cache != null) { + return cache.getSecurityService(); + } + return new DisabledSecurityService(); + } + @Override public boolean preHandle(final HttpServletRequest request, final HttpServletResponse response, final Object handler) throws Exception { @@ -104,11 +119,6 @@ public class LoginHandlerInterceptor extends HandlerInterceptorAdapter { return true; } - public void setSecurityService(SecurityService securityService) { - this.securityService = securityService; - } - - @Override public void afterCompletion(final HttpServletRequest request, final HttpServletResponse response, final Object handler, final Exception ex) throws Exception { http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/geode-core/src/main/java/org/apache/geode/security/PostProcessor.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/security/PostProcessor.java b/geode-core/src/main/java/org/apache/geode/security/PostProcessor.java index 707e3cf..bad58d8 100644 --- a/geode-core/src/main/java/org/apache/geode/security/PostProcessor.java +++ b/geode-core/src/main/java/org/apache/geode/security/PostProcessor.java @@ -28,7 +28,7 @@ public interface PostProcessor { * Given the security props of the server, properly initialize the post processor for the server. * Initialized at cache creation * - * @param securityProps + * @param securityProps security properties */ default void init(Properties securityProps) {} http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/geode-core/src/test/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTest.java b/geode-core/src/test/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTest.java index dc73f04..c9b8d6b 100644 --- a/geode-core/src/test/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTest.java @@ -55,6 +55,9 @@ public class AnalyzeSerializablesJUnitTest { /** all loaded classes */ private Map classes; + private File expectedDataSerializablesFile; + private File expectedSerializablesFile; + private List expectedDataSerializables; private List expectedSerializables; @@ -76,18 +79,18 @@ public class AnalyzeSerializablesJUnitTest { // setup expectedDataSerializables - File expectedDataSerializablesFile = getResourceAsFile("sanctionedDataSerializables.txt"); - assertThat(expectedDataSerializablesFile).exists().canRead(); + this.expectedDataSerializablesFile = getResourceAsFile("sanctionedDataSerializables.txt"); + assertThat(this.expectedDataSerializablesFile).exists().canRead(); - this.expectedDataSerializables = loadClassesAndMethods(expectedDataSerializablesFile); + this.expectedDataSerializables = loadClassesAndMethods(this.expectedDataSerializablesFile); Collections.sort(this.expectedDataSerializables); // setup expectedSerializables - File expectedSerializablesFile = getResourceAsFile("sanctionedSerializables.txt"); - assertThat(expectedSerializablesFile).exists().canRead(); + this.expectedSerializablesFile = getResourceAsFile("sanctionedSerializables.txt"); + assertThat(this.expectedSerializablesFile).exists().canRead(); - this.expectedSerializables = loadClassesAndVariables(expectedSerializablesFile); + this.expectedSerializables = loadClassesAndVariables(this.expectedSerializablesFile); Collections.sort(this.expectedSerializables); // setup empty actual files @@ -119,8 +122,8 @@ public class AnalyzeSerializablesJUnitTest { fail( diff + "\n\nIf the class is not persisted or sent over the wire add it to the excludedClasses.txt file in the " + "\norg/apache/geode/codeAnalysis directory. Otherwise if this doesn't " - + "\nbreak backward compatibility move the file actualDataSerializables.dat to the codeAnalysis " - + "\ntest directory and rename to sanctionedDataSerializables.txt"); + + "\nbreak backward compatibility move the file " + this.actualDataSerializablesFile.getAbsolutePath() + " to the codeAnalysis " + + "\ntest directory and rename to " + this.expectedDataSerializablesFile.getAbsolutePath()); } } @@ -139,8 +142,8 @@ public class AnalyzeSerializablesJUnitTest { fail( diff + "\n\nIf the class is not persisted or sent over the wire add it to the excludedClasses.txt file in the " + "\n/org/apache/geode/codeAnalysis/ directory. Otherwise if this doesn't " - + "\nbreak backward compatibility move the file actualSerializables.dat to the " - + "\ncodeAnalysis test directory and rename to sanctionedSerializables.txt"); + + "\nbreak backward compatibility move the file " + this.actualSerializablesFile.getAbsolutePath() + " to the " + + "\ncodeAnalysis test directory and rename to " + this.expectedSerializablesFile.getAbsolutePath()); } } http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java index f112d1a..a0c3cf3 100755 --- a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java @@ -14,6 +14,13 @@ */ package org.apache.geode.distributed.internal.membership; +import static org.apache.geode.distributed.ConfigurationProperties.DISABLE_TCP; +import static org.apache.geode.distributed.ConfigurationProperties.GROUPS; +import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS; +import static org.apache.geode.distributed.ConfigurationProperties.LOG_FILE; +import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL; +import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT; +import static org.apache.geode.distributed.ConfigurationProperties.MEMBER_TIMEOUT; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -22,57 +29,48 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import java.io.File; -import java.net.InetAddress; -import java.util.List; -import java.util.Properties; - -import org.apache.logging.log4j.Level; -import org.junit.AfterClass; -import org.junit.BeforeClass; -import org.junit.Test; -import org.junit.experimental.categories.Category; - import org.apache.geode.GemFireConfigException; import org.apache.geode.distributed.ConfigurationProperties; import org.apache.geode.distributed.Locator; -import org.apache.geode.distributed.internal.*; +import org.apache.geode.distributed.internal.DMStats; +import org.apache.geode.distributed.internal.DistributionConfig; +import org.apache.geode.distributed.internal.DistributionConfigImpl; +import org.apache.geode.distributed.internal.DistributionManager; +import org.apache.geode.distributed.internal.InternalLocator; +import org.apache.geode.distributed.internal.SerialAckedMessage; import org.apache.geode.distributed.internal.membership.gms.GMSUtil; import org.apache.geode.distributed.internal.membership.gms.ServiceConfig; import org.apache.geode.distributed.internal.membership.gms.Services; import org.apache.geode.distributed.internal.membership.gms.interfaces.JoinLeave; import org.apache.geode.distributed.internal.membership.gms.membership.GMSJoinLeave; -import org.apache.geode.distributed.internal.membership.gms.messages.*; +import org.apache.geode.distributed.internal.membership.gms.messages.HeartbeatMessage; +import org.apache.geode.distributed.internal.membership.gms.messages.HeartbeatRequestMessage; +import org.apache.geode.distributed.internal.membership.gms.messages.InstallViewMessage; +import org.apache.geode.distributed.internal.membership.gms.messages.JoinRequestMessage; +import org.apache.geode.distributed.internal.membership.gms.messages.JoinResponseMessage; +import org.apache.geode.distributed.internal.membership.gms.messages.LeaveRequestMessage; +import org.apache.geode.distributed.internal.membership.gms.messages.RemoveMemberMessage; +import org.apache.geode.distributed.internal.membership.gms.messages.SuspectMembersMessage; +import org.apache.geode.distributed.internal.membership.gms.messages.ViewAckMessage; import org.apache.geode.distributed.internal.membership.gms.mgr.GMSMembershipManager; import org.apache.geode.internal.AvailablePortHelper; -import org.apache.geode.internal.net.SocketCreator; import org.apache.geode.internal.admin.remote.RemoteTransportConfig; +import org.apache.geode.internal.net.SocketCreator; +import org.apache.geode.internal.security.SecurityServiceFactory; import org.apache.geode.test.junit.categories.IntegrationTest; +import org.apache.logging.log4j.Level; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.experimental.categories.Category; -import static org.apache.geode.distributed.ConfigurationProperties.*; +import java.io.File; +import java.net.InetAddress; +import java.util.List; +import java.util.Properties; @Category({IntegrationTest.class, MembershipJUnitTest.class}) public class MembershipJUnitTest { - static Level baseLogLevel; - - @BeforeClass - public static void setupClass() { - // baseLogLevel = LogService.getBaseLogLevel(); - // LogService.setBaseLogLevel(Level.DEBUG); - } - - @AfterClass - public static void tearDown() throws Exception { - // LogService.setBaseLogLevel(baseLogLevel); - } - - // @Test - // public void testRepeat() throws Exception { - // for (int i=0; i<50; i++) { - // System.out.println("--------------------run #" + i); - // testMultipleManagersInSameProcess(); - // } - // } /** * This test creates a locator with a colocated membership manager and then creates a second @@ -152,7 +150,8 @@ public class MembershipJUnitTest { DistributedMembershipListener listener1 = mock(DistributedMembershipListener.class); DMStats stats1 = mock(DMStats.class); System.out.println("creating 1st membership manager"); - m1 = MemberFactory.newMembershipManager(listener1, config, transport, stats1); + m1 = MemberFactory.newMembershipManager(listener1, config, transport, stats1, + SecurityServiceFactory.create()); m1.startEventProcessing(); } finally { System.getProperties().remove(GMSJoinLeave.BYPASS_DISCOVERY_PROPERTY); @@ -162,7 +161,8 @@ public class MembershipJUnitTest { DistributedMembershipListener listener2 = mock(DistributedMembershipListener.class); DMStats stats2 = mock(DMStats.class); System.out.println("creating 2nd membership manager"); - m2 = MemberFactory.newMembershipManager(listener2, config, transport, stats2); + m2 = MemberFactory.newMembershipManager(listener2, config, transport, stats2, + SecurityServiceFactory.create()); m2.startEventProcessing(); // we have to check the views with JoinLeave because the membership @@ -292,7 +292,8 @@ public class MembershipJUnitTest { DistributedMembershipListener listener1 = mock(DistributedMembershipListener.class); DMStats stats1 = mock(DMStats.class); System.out.println("creating 1st membership manager"); - m1 = MemberFactory.newMembershipManager(listener1, config, transport, stats1); + m1 = MemberFactory.newMembershipManager(listener1, config, transport, stats1, + SecurityServiceFactory.create()); m1.startEventProcessing(); } finally { System.getProperties().remove(GMSJoinLeave.BYPASS_DISCOVERY_PROPERTY); @@ -302,7 +303,8 @@ public class MembershipJUnitTest { DistributedMembershipListener listener2 = mock(DistributedMembershipListener.class); DMStats stats2 = mock(DMStats.class); System.out.println("creating 2nd membership manager"); - m2 = MemberFactory.newMembershipManager(listener2, config, transport, stats2); + m2 = MemberFactory.newMembershipManager(listener2, config, transport, stats2, + SecurityServiceFactory.create()); m2.startEventProcessing(); // we have to check the views with JoinLeave because the membership http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/AbstractGMSAuthenticatorTestCase.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/AbstractGMSAuthenticatorTestCase.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/AbstractGMSAuthenticatorTestCase.java index f17b40b..e8c4e73 100644 --- a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/AbstractGMSAuthenticatorTestCase.java +++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/AbstractGMSAuthenticatorTestCase.java @@ -65,18 +65,19 @@ public abstract class AbstractGMSAuthenticatorTestCase { clearStatics(); MockitoAnnotations.initMocks(this); - props = new Properties(); - securityProps = new Properties(); - - when(securityService.isIntegratedSecurity()).thenReturn(isIntegratedSecurity()); - when(securityService.isPeerSecurityRequired()).thenReturn(true); - when(securityService.login(securityProps)).thenReturn(subject); - when(distributionConfig.getSecurityProps()).thenReturn(securityProps); - when(serviceConfig.getDistributionConfig()).thenReturn(distributionConfig); - when(services.getSecurityLogWriter()).thenReturn(mock(InternalLogWriter.class)); - when(services.getConfig()).thenReturn(serviceConfig); - - authenticator.init(services); + this.props = new Properties(); + this.securityProps = new Properties(); + + when(this.securityService.isIntegratedSecurity()).thenReturn(isIntegratedSecurity()); + when(this.securityService.isPeerSecurityRequired()).thenReturn(true); + when(this.securityService.login(this.securityProps)).thenReturn(this.subject); + when(this.distributionConfig.getSecurityProps()).thenReturn(this.securityProps); + when(this.serviceConfig.getDistributionConfig()).thenReturn(this.distributionConfig); + when(this.services.getSecurityLogWriter()).thenReturn(mock(InternalLogWriter.class)); + when(this.services.getConfig()).thenReturn(this.serviceConfig); + when(this.services.getSecurityService()).thenReturn(this.securityService); + + this.authenticator.init(this.services); } protected abstract boolean isIntegratedSecurity(); @@ -135,11 +136,11 @@ public abstract class AbstractGMSAuthenticatorTestCase { @Override public void close() { - closed = true; + this.closed = true; } public boolean isClosed() { - return closed; + return this.closed; } public static int getCreateCount() { @@ -284,11 +285,11 @@ public abstract class AbstractGMSAuthenticatorTestCase { @Override public void close() { - closed = true; + this.closed = true; } public boolean isClosed() { - return closed; + return this.closed; } public static int getCreateCount() { http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithAuthenticatorTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithAuthenticatorTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithAuthenticatorTest.java index b9f6429..96a9a5f 100644 --- a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithAuthenticatorTest.java +++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithAuthenticatorTest.java @@ -38,26 +38,28 @@ public class GMSAuthenticatorWithAuthenticatorTest extends AbstractGMSAuthentica @Test public void nullAuthenticatorShouldReturnNull() throws Exception { - assertThat(securityProps).doesNotContainKey(SECURITY_PEER_AUTHENTICATOR); - String result = authenticator.authenticate(member, securityProps, securityProps); + assertThat(this.securityProps).doesNotContainKey(SECURITY_PEER_AUTHENTICATOR); + String result = + this.authenticator.authenticate(this.member, this.securityProps, this.securityProps); // assertThat(result).isNull(); NOTE: old security used to return null assertThat(result).contains("Security check failed"); } @Test public void emptyAuthenticatorShouldReturnNull() throws Exception { - securityProps.setProperty(SECURITY_PEER_AUTHENTICATOR, ""); - String result = authenticator.authenticate(member, securityProps, securityProps); + this.securityProps.setProperty(SECURITY_PEER_AUTHENTICATOR, ""); + String result = + this.authenticator.authenticate(this.member, this.securityProps, this.securityProps); // assertThat(result).isNull(); NOTE: old security used to return null assertThat(result).contains("Security check failed"); } @Test public void shouldGetSecurityPropsFromDistributionConfig() throws Exception { - securityProps.setProperty(SECURITY_PEER_AUTH_INIT, "dummy1"); - securityProps.setProperty(SECURITY_PEER_AUTHENTICATOR, "dummy2"); + this.securityProps.setProperty(SECURITY_PEER_AUTH_INIT, "dummy1"); + this.securityProps.setProperty(SECURITY_PEER_AUTHENTICATOR, "dummy2"); - Properties secProps = authenticator.getSecurityProps(); + Properties secProps = this.authenticator.getSecurityProps(); assertThat(secProps.size()).isEqualTo(2); assertThat(secProps.getProperty(SECURITY_PEER_AUTH_INIT)).isEqualTo("dummy1"); @@ -66,73 +68,74 @@ public class GMSAuthenticatorWithAuthenticatorTest extends AbstractGMSAuthentica @Test public void usesPeerAuthInitToGetCredentials() throws Exception { - props.setProperty(SECURITY_PEER_AUTH_INIT, SpyAuthInit.class.getName() + ".create"); + this.props.setProperty(SECURITY_PEER_AUTH_INIT, SpyAuthInit.class.getName() + ".create"); SpyAuthInit auth = new SpyAuthInit(); assertThat(auth.isClosed()).isFalse(); SpyAuthInit.setAuthInitialize(auth); - Properties credentials = authenticator.getCredentials(member, props); + Properties credentials = this.authenticator.getCredentials(this.member, this.props); - assertThat(credentials).isEqualTo(props); + assertThat(credentials).isEqualTo(this.props); assertThat(auth.isClosed()).isTrue(); assertThat(SpyAuthInit.getCreateCount()).isEqualTo(1); } @Test public void getCredentialsShouldReturnNullIfNoPeerAuthInit() throws Exception { - Properties credentials = authenticator.getCredentials(member, props); + Properties credentials = this.authenticator.getCredentials(this.member, this.props); assertThat(credentials).isNull(); } @Test public void getCredentialsShouldReturnNullIfEmptyPeerAuthInit() throws Exception { - props.setProperty(SECURITY_PEER_AUTH_INIT, ""); - Properties credentials = authenticator.getCredentials(member, props); + this.props.setProperty(SECURITY_PEER_AUTH_INIT, ""); + Properties credentials = this.authenticator.getCredentials(this.member, this.props); assertThat(credentials).isNull(); } @Test public void getCredentialsShouldThrowIfPeerAuthInitDoesNotExist() throws Exception { String authInit = getClass().getName() + "$NotExistAuth.create"; - props.setProperty(SECURITY_PEER_AUTH_INIT, authInit); - assertThatThrownBy(() -> authenticator.getCredentials(member, props)) + this.props.setProperty(SECURITY_PEER_AUTH_INIT, authInit); + assertThatThrownBy(() -> this.authenticator.getCredentials(this.member, this.props)) .hasMessageContaining("Instance could not be obtained from"); } @Test public void getCredentialsShouldThrowIfPeerAuthInitCreateReturnsNull() throws Exception { - props.setProperty(SECURITY_PEER_AUTH_INIT, + this.props.setProperty(SECURITY_PEER_AUTH_INIT, AuthInitCreateReturnsNull.class.getName() + ".create"); - assertThatThrownBy(() -> authenticator.getCredentials(member, props)) + assertThatThrownBy(() -> this.authenticator.getCredentials(this.member, this.props)) .hasMessageContaining("Instance could not be obtained from"); } @Test public void getCredentialsShouldThrowIfPeerAuthInitGetCredentialsAndInitThrow() throws Exception { - props.setProperty(SECURITY_PEER_AUTH_INIT, + this.props.setProperty(SECURITY_PEER_AUTH_INIT, AuthInitGetCredentialsAndInitThrow.class.getName() + ".create"); - assertThatThrownBy(() -> authenticator.getCredentials(member, props)) + assertThatThrownBy(() -> this.authenticator.getCredentials(this.member, this.props)) .hasMessageContaining("expected init error"); } @Test public void getCredentialsShouldThrowIfPeerAuthInitGetCredentialsThrows() throws Exception { - props.setProperty(SECURITY_PEER_AUTH_INIT, + this.props.setProperty(SECURITY_PEER_AUTH_INIT, AuthInitGetCredentialsThrows.class.getName() + ".create"); - assertThatThrownBy(() -> authenticator.getCredentials(member, props)) + assertThatThrownBy(() -> this.authenticator.getCredentials(this.member, this.props)) .hasMessageContaining("expected get credential error"); } @Test public void authenticateShouldReturnNullIfSuccessful() throws Exception { - props.setProperty(SECURITY_PEER_AUTHENTICATOR, SpyAuthenticator.class.getName() + ".create"); + this.props.setProperty(SECURITY_PEER_AUTHENTICATOR, + SpyAuthenticator.class.getName() + ".create"); SpyAuthenticator auth = new SpyAuthenticator(); assertThat(auth.isClosed()).isFalse(); SpyAuthenticator.setAuthenticator(auth); - String result = authenticator.authenticate(member, props, props); + String result = this.authenticator.authenticate(this.member, this.props, this.props); assertThat(result).isNull(); assertThat(auth.isClosed()).isTrue(); @@ -141,15 +144,15 @@ public class GMSAuthenticatorWithAuthenticatorTest extends AbstractGMSAuthentica @Test public void authenticateShouldReturnNullIfPeerAuthenticatorIsNull() throws Exception { - String result = authenticator.authenticate(member, props, props); + String result = this.authenticator.authenticate(this.member, this.props, this.props); // assertThat(result).isNull(); // NOTE: old security used to return null assertThat(result).contains("Security check failed. Instance could not be obtained from null"); } @Test public void authenticateShouldReturnNullIfPeerAuthenticatorIsEmpty() throws Exception { - props.setProperty(SECURITY_PEER_AUTHENTICATOR, ""); - String result = authenticator.authenticate(member, props, props); + this.props.setProperty(SECURITY_PEER_AUTHENTICATOR, ""); + String result = this.authenticator.authenticate(this.member, this.props, this.props); // assertThat(result).isNull(); // NOTE: old security used to return null assertThat(result).contains("Security check failed. Instance could not be obtained from"); } @@ -157,40 +160,41 @@ public class GMSAuthenticatorWithAuthenticatorTest extends AbstractGMSAuthentica @Test public void authenticateShouldReturnFailureMessageIfPeerAuthenticatorDoesNotExist() throws Exception { - props.setProperty(SECURITY_PEER_AUTHENTICATOR, getClass().getName() + "$NotExistAuth.create"); - String result = authenticator.authenticate(member, props, props); + this.props.setProperty(SECURITY_PEER_AUTHENTICATOR, + getClass().getName() + "$NotExistAuth.create"); + String result = this.authenticator.authenticate(this.member, this.props, this.props); assertThat(result).startsWith("Security check failed. Instance could not be obtained from"); } @Test public void authenticateShouldReturnFailureMessageIfAuthenticateReturnsNull() throws Exception { - props.setProperty(SECURITY_PEER_AUTHENTICATOR, + this.props.setProperty(SECURITY_PEER_AUTHENTICATOR, AuthenticatorReturnsNulls.class.getName() + ".create"); - String result = authenticator.authenticate(member, props, props); + String result = this.authenticator.authenticate(this.member, this.props, this.props); assertThat(result).startsWith("Security check failed. Instance could not be obtained"); } @Test public void authenticateShouldReturnFailureMessageIfNullCredentials() throws Exception { - props.setProperty(SECURITY_PEER_AUTHENTICATOR, + this.props.setProperty(SECURITY_PEER_AUTHENTICATOR, AuthenticatorReturnsNulls.class.getName() + ".create"); - String result = authenticator.authenticate(member, null, props); + String result = this.authenticator.authenticate(this.member, null, this.props); assertThat(result).startsWith("Failed to find credentials from"); } @Test public void authenticateShouldReturnFailureMessageIfAuthenticateInitThrows() throws Exception { - props.setProperty(SECURITY_PEER_AUTHENTICATOR, + this.props.setProperty(SECURITY_PEER_AUTHENTICATOR, AuthenticatorInitThrows.class.getName() + ".create"); - String result = authenticator.authenticate(member, props, props); + String result = this.authenticator.authenticate(this.member, this.props, this.props); assertThat(result).startsWith("Security check failed. expected init error"); } @Test public void authenticateShouldReturnFailureMessageIfAuthenticateThrows() throws Exception { - props.setProperty(SECURITY_PEER_AUTHENTICATOR, + this.props.setProperty(SECURITY_PEER_AUTHENTICATOR, AuthenticatorAuthenticateThrows.class.getName() + ".create"); - String result = authenticator.authenticate(member, props, props); + String result = this.authenticator.authenticate(this.member, this.props, this.props); assertThat(result).startsWith("Security check failed. expected authenticate error"); } } http://git-wip-us.apache.org/repos/asf/geode/blob/eab9e6e0/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithSecurityManagerTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithSecurityManagerTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithSecurityManagerTest.java index 0cb6994..01ebc81 100644 --- a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithSecurityManagerTest.java +++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithSecurityManagerTest.java @@ -40,24 +40,26 @@ public class GMSAuthenticatorWithSecurityManagerTest extends AbstractGMSAuthenti @Test public void nullManagerShouldReturnNull() throws Exception { - assertThat(securityProps).doesNotContainKey(SECURITY_MANAGER); - String result = authenticator.authenticate(member, securityProps, securityProps); + assertThat(this.securityProps).doesNotContainKey(SECURITY_MANAGER); + String result = + this.authenticator.authenticate(this.member, this.securityProps, this.securityProps); assertThat(result).isNull(); } @Test public void emptyAuthenticatorShouldReturnNull() throws Exception { - securityProps.setProperty(SECURITY_MANAGER, ""); - String result = authenticator.authenticate(member, securityProps, securityProps); + this.securityProps.setProperty(SECURITY_MANAGER, ""); + String result = + this.authenticator.authenticate(this.member, this.securityProps, this.securityProps); assertThat(result).isNull(); } @Test public void shouldGetSecurityPropsFromDistributionConfig() throws Exception { - securityProps.setProperty(SECURITY_PEER_AUTH_INIT, "dummy1"); - securityProps.setProperty(SECURITY_MANAGER, "dummy2"); + this.securityProps.setProperty(SECURITY_PEER_AUTH_INIT, "dummy1"); + this.securityProps.setProperty(SECURITY_MANAGER, "dummy2"); - Properties secProps = authenticator.getSecurityProps(); + Properties secProps = this.authenticator.getSecurityProps(); assertThat(secProps.size()).isEqualTo(2); assertThat(secProps.getProperty(SECURITY_PEER_AUTH_INIT)).isEqualTo("dummy1"); @@ -66,91 +68,91 @@ public class GMSAuthenticatorWithSecurityManagerTest extends AbstractGMSAuthenti @Test public void usesPeerAuthInitToGetCredentials() throws Exception { - props.setProperty(SECURITY_PEER_AUTH_INIT, SpyAuthInit.class.getName() + ".create"); - props.setProperty(SECURITY_MANAGER, "dummy"); + this.props.setProperty(SECURITY_PEER_AUTH_INIT, SpyAuthInit.class.getName() + ".create"); + this.props.setProperty(SECURITY_MANAGER, "dummy"); SpyAuthInit auth = new SpyAuthInit(); assertThat(auth.isClosed()).isFalse(); SpyAuthInit.setAuthInitialize(auth); - Properties credentials = authenticator.getCredentials(member, props); + Properties credentials = this.authenticator.getCredentials(this.member, this.props); - assertThat(credentials).isEqualTo(props); + assertThat(credentials).isEqualTo(this.props); assertThat(auth.isClosed()).isTrue(); assertThat(SpyAuthInit.getCreateCount() == 1).isTrue(); } @Test public void getCredentialsShouldReturnNullIfNoPeerAuthInit() throws Exception { - Properties credentials = authenticator.getCredentials(member, props); + Properties credentials = this.authenticator.getCredentials(this.member, this.props); assertThat(credentials).isNull(); } @Test public void getCredentialsShouldReturnNullIfEmptyPeerAuthInit() throws Exception { - props.setProperty(SECURITY_PEER_AUTH_INIT, ""); - Properties credentials = authenticator.getCredentials(member, props); + this.props.setProperty(SECURITY_PEER_AUTH_INIT, ""); + Properties credentials = this.authenticator.getCredentials(this.member, this.props); assertThat(credentials).isNull(); } @Test public void getCredentialsShouldThrowIfPeerAuthInitDoesNotExist() throws Exception { String authInit = getClass().getName() + "$NotExistAuth.create"; - props.setProperty(SECURITY_PEER_AUTH_INIT, authInit); - assertThatThrownBy(() -> authenticator.getCredentials(member, props)) + this.props.setProperty(SECURITY_PEER_AUTH_INIT, authInit); + assertThatThrownBy(() -> this.authenticator.getCredentials(this.member, this.props)) .hasMessageContaining("Instance could not be obtained"); } @Test public void getCredentialsShouldThrowIfPeerAuthInitCreateReturnsNull() throws Exception { - props.setProperty(SECURITY_PEER_AUTH_INIT, + this.props.setProperty(SECURITY_PEER_AUTH_INIT, AuthInitCreateReturnsNull.class.getName() + ".create"); - assertThatThrownBy(() -> authenticator.getCredentials(member, props)) + assertThatThrownBy(() -> this.authenticator.getCredentials(this.member, this.props)) .hasMessageContaining("Instance could not be obtained from"); } @Test public void getCredentialsShouldThrowIfPeerAuthInitGetCredentialsAndInitThrow() throws Exception { - props.setProperty(SECURITY_PEER_AUTH_INIT, + this.props.setProperty(SECURITY_PEER_AUTH_INIT, AuthInitGetCredentialsAndInitThrow.class.getName() + ".create"); - assertThatThrownBy(() -> authenticator.getCredentials(member, props)) + assertThatThrownBy(() -> this.authenticator.getCredentials(this.member, this.props)) .hasMessage("expected init error"); } @Test public void getCredentialsShouldThrowIfPeerAuthInitGetCredentialsThrows() throws Exception { - props.setProperty(SECURITY_PEER_AUTH_INIT, + this.props.setProperty(SECURITY_PEER_AUTH_INIT, AuthInitGetCredentialsThrows.class.getName() + ".create"); - assertThatThrownBy(() -> authenticator.getCredentials(member, props)) + assertThatThrownBy(() -> this.authenticator.getCredentials(this.member, this.props)) .hasMessage("expected get credential error"); } @Test public void authenticateShouldReturnNullIfSuccessful() throws Exception { - props.setProperty(SECURITY_MANAGER, "dummy"); - String result = authenticator.authenticate(member, props, props); + this.props.setProperty(SECURITY_MANAGER, "dummy"); + String result = this.authenticator.authenticate(this.member, this.props, this.props); assertThat(result).isNull(); } @Test public void authenticateShouldReturnNullIfNoSecurityManager() throws Exception { - String result = authenticator.authenticate(member, props, props); + String result = this.authenticator.authenticate(this.member, this.props, this.props); assertThat(result).isNull(); } @Test public void authenticateShouldReturnFailureMessageIfLoginThrows() throws Exception { - when(securityService.login(any(Properties.class))) + when(this.securityService.login(any(Properties.class))) .thenThrow(new GemFireSecurityException("dummy")); - props.setProperty(SECURITY_MANAGER, "dummy"); - String result = authenticator.authenticate(member, props, props); + this.props.setProperty(SECURITY_MANAGER, "dummy"); + String result = this.authenticator.authenticate(this.member, this.props, this.props); assertThat(result).startsWith("Security check failed. dummy"); } @Test public void authenticateShouldReturnFailureMessageIfNullCredentials() throws Exception { - props.setProperty(SECURITY_MANAGER, "dummy"); - String result = authenticator.authenticate(member, null, props); + this.props.setProperty(SECURITY_MANAGER, "dummy"); + String result = this.authenticator.authenticate(this.member, null, this.props); assertThat(result).startsWith("Failed to find credentials from"); }