Repository: geode Updated Branches: refs/heads/feature/GEODE-2632-21 a9f939bb0 -> 288158d1f
fixup Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/288158d1 Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/288158d1 Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/288158d1 Branch: refs/heads/feature/GEODE-2632-21 Commit: 288158d1fd084463cc508d2c9bae6ec298a11642 Parents: a9f939b Author: Kirk Lund <[email protected]> Authored: Mon Jun 5 18:04:24 2017 -0700 Committer: Kirk Lund <[email protected]> Committed: Mon Jun 5 18:04:24 2017 -0700 ---------------------------------------------------------------------- .../org/apache/geode/cache/CacheFactory.java | 40 ++- .../internal/InternalDistributedSystem.java | 12 +- .../distributed/internal/SecurityConfig.java | 51 ++++ .../security/CustomSecurityService.java | 158 +++++++++-- .../internal/security/PostProcessorService.java | 264 +++++++++++++++++++ .../security/SecurityServiceFactory.java | 23 +- .../internal/security/SecurityServiceType.java | 2 + ...urityServiceFactoryShiroIntegrationTest.java | 92 +++++++ .../security/SecurityServiceFactoryTest.java | 97 ++++--- ...urityServiceWithShiroIniIntegrationTest.java | 3 +- .../CacheFactoryWithSecurityObjectTest.java | 28 +- ...curityServiceFactoryShiroIntegrationTest.ini | 30 +++ 12 files changed, 708 insertions(+), 92 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/288158d1/geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java b/geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java index d9faf6b..9b23f6c 100644 --- a/geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java +++ b/geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java @@ -17,6 +17,7 @@ package org.apache.geode.cache; import org.apache.geode.distributed.ConfigurationProperties; import org.apache.geode.distributed.DistributedSystem; import org.apache.geode.distributed.internal.InternalDistributedSystem; +import org.apache.geode.distributed.internal.SecurityConfig; import org.apache.geode.internal.GemFireVersion; import org.apache.geode.internal.cache.CacheConfig; import org.apache.geode.internal.cache.GemFireCacheImpl; @@ -27,6 +28,7 @@ import org.apache.geode.pdx.PdxInstance; import org.apache.geode.pdx.PdxSerializer; import org.apache.geode.security.AuthenticationFailedException; import org.apache.geode.security.AuthenticationRequiredException; +import org.apache.geode.security.GemFireSecurityException; import org.apache.geode.security.PostProcessor; import org.apache.geode.security.SecurityManager; @@ -205,17 +207,45 @@ public class CacheFactory { if (this.dsProps.isEmpty()) { // any ds will do ds = InternalDistributedSystem.getConnectedInstance(); - // TODO:if already connected AND used setSecurityManager or setPostProcessor then throw - // Error + validateUsabilityOfSecurityCallbacks(ds); } if (ds == null) { - ds = DistributedSystem.connect(this.dsProps); + // use ThreadLocal to avoid exposing new User API in DistributedSystem + SecurityConfig.set(this.cacheConfig.getSecurityManager(), + this.cacheConfig.getPostProcessor()); + try { + ds = DistributedSystem.connect(this.dsProps); + } finally { + SecurityConfig.remove(); + } } return create(ds, true, this.cacheConfig); } } /** + * Throws GemFireSecurityException if existing DistributedSystem connection cannot use specified + * SecurityManager or PostProcessor. + */ + private void validateUsabilityOfSecurityCallbacks(DistributedSystem ds) + throws GemFireSecurityException { + if (ds == null) { + return; + } + // pre-existing DistributedSystem already has an incompatible SecurityService in use + if (this.cacheConfig.getSecurityManager() != null) { + // invalid configuration + throw new GemFireSecurityException( + "Existing DistributedSystem connection cannot use specified SecurityManager"); + } + if (this.cacheConfig.getPostProcessor() != null) { + // invalid configuration + throw new GemFireSecurityException( + "Existing DistributedSystem connection cannot use specified PostProcessor"); + } + } + + /** * Gets the instance of {@link Cache} produced by an earlier call to {@link #create()}. * * @param system the {@code DistributedSystem} the cache was created with. @@ -323,7 +353,7 @@ public class CacheFactory { } /** - * sets the securityManager for the cache. If this securityManager is set. It will override the + * Sets the securityManager for the cache. If this securityManager is set, it will override the * security-manager property you set in your gemfire system properties. * * This is provided mostly for container to inject an already initialized securityManager. An @@ -338,7 +368,7 @@ public class CacheFactory { } /** - * sets the postProcessor for the cache. If this postProcessor is set. It will override thie + * Sets the postProcessor for the cache. If this postProcessor is set, it will override the * security-post-processor setting in the gemfire system properties. * * This is provided mostly for container to inject an already initialized post processor. An http://git-wip-us.apache.org/repos/asf/geode/blob/288158d1/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java index bcabe3d..22edb6f 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java @@ -303,7 +303,17 @@ public class InternalDistributedSystem extends DistributedSystem * Creates a new instance of <code>InternalDistributedSystem</code> with the given configuration. */ public static InternalDistributedSystem newInstance(Properties config) { - return newInstance(config, null, null); + return newInstance(config, SecurityConfig.get()); + } + + public static InternalDistributedSystem newInstance(Properties config, + SecurityConfig securityConfig) { + if (securityConfig == null) { + return newInstance(config, null, null); + } else { + return newInstance(config, securityConfig.getSecurityManager(), + securityConfig.getPostProcessor()); + } } public static InternalDistributedSystem newInstance(Properties config, http://git-wip-us.apache.org/repos/asf/geode/blob/288158d1/geode-core/src/main/java/org/apache/geode/distributed/internal/SecurityConfig.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/SecurityConfig.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/SecurityConfig.java new file mode 100644 index 0000000..deea55f --- /dev/null +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/SecurityConfig.java @@ -0,0 +1,51 @@ +/* + * 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.distributed.internal; + +import org.apache.geode.security.PostProcessor; +import org.apache.geode.security.SecurityManager; + +public class SecurityConfig { + + private static final ThreadLocal<SecurityConfig> THREAD_LOCAL = new ThreadLocal<>(); + + public static void set(SecurityManager securityManager, PostProcessor postProcessor) { + THREAD_LOCAL.set(new SecurityConfig(securityManager, postProcessor)); + } + + public static SecurityConfig get() { + return THREAD_LOCAL.get(); + } + + public static void remove() { + THREAD_LOCAL.remove(); + } + + private final SecurityManager securityManager; + private final PostProcessor postProcessor; + + public SecurityConfig(SecurityManager securityManager, PostProcessor postProcessor) { + this.securityManager = securityManager; + this.postProcessor = postProcessor; + } + + public SecurityManager getSecurityManager() { + return this.securityManager; + } + + public PostProcessor getPostProcessor() { + return this.postProcessor; + } +} http://git-wip-us.apache.org/repos/asf/geode/blob/288158d1/geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java b/geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java index f4b0ceb..1fcf38f 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java +++ b/geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java @@ -14,10 +14,26 @@ */ package org.apache.geode.internal.security; +import java.security.AccessController; import java.util.Properties; +import java.util.Set; import java.util.concurrent.Callable; +import org.apache.commons.lang.StringUtils; +import org.apache.geode.internal.logging.LogService; +import org.apache.geode.internal.security.shiro.GeodeAuthenticationToken; +import org.apache.geode.internal.security.shiro.ShiroPrincipal; +import org.apache.geode.security.AuthenticationFailedException; +import org.apache.geode.security.GemFireSecurityException; +import org.apache.geode.security.NotAuthorizedException; +import org.apache.geode.security.ResourcePermission.Operation; +import org.apache.geode.security.ResourcePermission.Resource; +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.management.internal.security.ResourceOperation; @@ -26,6 +42,7 @@ import org.apache.geode.security.ResourcePermission; import org.apache.geode.security.SecurityManager; public class CustomSecurityService implements SecurityService { + private static Logger logger = LogService.getLogger(LogService.SECURITY_LOGGER_NAME); CustomSecurityService() { // nothing @@ -33,133 +50,222 @@ public class CustomSecurityService implements SecurityService { @Override public void initSecurity(final Properties securityProps) { - + // nothing } @Override public void setSecurityManager(final SecurityManager securityManager) { - + // nothing } @Override public void setPostProcessor(final PostProcessor postProcessor) { - + // nothing } @Override public ThreadState bindSubject(final Subject subject) { - return null; + if (subject == null) { + return null; + } + + ThreadState threadState = new SubjectThreadState(subject); + threadState.bind(); + return threadState; } @Override public Subject getSubject() { - return null; + Subject currentUser; + + // First try get the principal out of AccessControlContext instead of Shiro's Thread context + // since threads can be shared between JMX clients. + javax.security.auth.Subject jmxSubject = + javax.security.auth.Subject.getSubject(AccessController.getContext()); + + if (jmxSubject != null) { + Set<ShiroPrincipal> principals = jmxSubject.getPrincipals(ShiroPrincipal.class); + if (principals.size() > 0) { + ShiroPrincipal principal = principals.iterator().next(); + currentUser = principal.getSubject(); + ThreadContext.bind(currentUser); + return currentUser; + } + } + + // in other cases like rest call, client operations, we get it from the current thread + currentUser = SecurityUtils.getSubject(); + + if (currentUser == null || currentUser.getPrincipal() == null) { + throw new GemFireSecurityException("Error: Anonymous User"); + } + + return currentUser; } @Override public Subject login(final Properties credentials) { - return null; + if (credentials == null) { + return null; + } + + // this makes sure it starts with a clean user object + ThreadContext.remove(); + + Subject currentUser = SecurityUtils.getSubject(); + GeodeAuthenticationToken token = new GeodeAuthenticationToken(credentials); + try { + logger.debug("Logging in " + token.getPrincipal()); + currentUser.login(token); + } catch (ShiroException e) { + logger.info(e.getMessage(), e); + throw new AuthenticationFailedException( + "Authentication error. Please check your credentials.", e); + } + + return currentUser; } @Override public void logout() { + Subject currentUser = getSubject(); + if (currentUser == null) { + return; + } + try { + logger.debug("Logging out " + currentUser.getPrincipal()); + currentUser.logout(); + } catch (ShiroException e) { + logger.info(e.getMessage(), e); + throw new GemFireSecurityException(e.getMessage(), e); + } + // clean out Shiro's thread local content + ThreadContext.remove(); } @Override public Callable associateWith(final Callable callable) { - return null; + Subject currentUser = getSubject(); + if (currentUser == null) { + return callable; + } + + return currentUser.associateWith(callable); } @Override public void authorize(final ResourceOperation resourceOperation) { + if (resourceOperation == null) { + return; + } + authorize(resourceOperation.resource().name(), resourceOperation.operation().name(), null); } @Override public void authorizeClusterManage() { - + authorize("CLUSTER", "MANAGE"); } @Override public void authorizeClusterWrite() { - + authorize("CLUSTER", "WRITE"); } @Override public void authorizeClusterRead() { - + authorize("CLUSTER", "READ"); } @Override public void authorizeDataManage() { - + authorize("DATA", "MANAGE"); } @Override public void authorizeDataWrite() { - + authorize("DATA", "WRITE"); } @Override public void authorizeDataRead() { - + authorize("DATA", "READ"); } @Override public void authorizeRegionManage(final String regionName) { - + authorize("DATA", "MANAGE", regionName); } @Override public void authorizeRegionManage(final String regionName, final String key) { - + authorize("DATA", "MANAGE", regionName, key); } @Override public void authorizeRegionWrite(final String regionName) { - + authorize("DATA", "WRITE", regionName); } @Override public void authorizeRegionWrite(final String regionName, final String key) { - + authorize("DATA", "WRITE", regionName, key); } @Override public void authorizeRegionRead(final String regionName) { - + authorize("DATA", "READ", regionName); } @Override public void authorizeRegionRead(final String regionName, final String key) { - + authorize("DATA", "READ", regionName, key); } @Override public void authorize(final String resource, final String operation) { - + authorize(resource, operation, null); } @Override public void authorize(final String resource, final String operation, final String regionName) { - + authorize(resource, operation, regionName, null); } @Override - public void authorize(final String resource, final String operation, final String regionName, - final String key) { - + public void authorize(final String resource, final String operation, String regionName, + final String key) { + regionName = StringUtils.stripStart(regionName, "/"); + authorize(new ResourcePermission(resource, operation, regionName, key)); } @Override public void authorize(final ResourcePermission context) { - + Subject currentUser = getSubject(); + if (currentUser == null) { + return; + } + if (context == null) { + return; + } + if (context.getResource() == Resource.NULL && context.getOperation() == Operation.NULL) { + return; + } + + try { + currentUser.checkPermission(context); + } catch (ShiroException e) { + String msg = currentUser.getPrincipal() + " not authorized for " + context; + logger.info(msg); + throw new NotAuthorizedException(msg, e); + } } @Override public void close() { - + ThreadContext.remove(); + SecurityUtils.setSecurityManager(null); } @Override http://git-wip-us.apache.org/repos/asf/geode/blob/288158d1/geode-core/src/main/java/org/apache/geode/internal/security/PostProcessorService.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/security/PostProcessorService.java b/geode-core/src/main/java/org/apache/geode/internal/security/PostProcessorService.java new file mode 100644 index 0000000..3c1e4d8 --- /dev/null +++ b/geode-core/src/main/java/org/apache/geode/internal/security/PostProcessorService.java @@ -0,0 +1,264 @@ +/* + * 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.internal.security; + +import org.apache.commons.lang.SerializationException; +import org.apache.commons.lang.StringUtils; +import org.apache.geode.GemFireIOException; +import org.apache.geode.internal.cache.EntryEventImpl; +import org.apache.geode.internal.logging.LogService; +import org.apache.geode.internal.util.BlobHelper; +import org.apache.geode.management.internal.security.ResourceOperation; +import org.apache.geode.security.PostProcessor; +import org.apache.geode.security.ResourcePermission; +import org.apache.geode.security.SecurityManager; +import org.apache.logging.log4j.Logger; +import org.apache.shiro.subject.Subject; +import org.apache.shiro.util.ThreadState; + +import java.io.IOException; +import java.io.Serializable; +import java.util.Properties; +import java.util.concurrent.Callable; + +/** + * Security service with PostProcessor but no SecurityManager + */ +public class PostProcessorService implements SecurityService { + private static Logger logger = LogService.getLogger(LogService.SECURITY_LOGGER_NAME); + + private final PostProcessor postProcessor; + + PostProcessorService(final PostProcessor postProcessor) { + if (postProcessor == null) { + throw new IllegalArgumentException("PostProcessor must not be null"); + } + this.postProcessor = postProcessor; + } + + @Override + public void initSecurity(final Properties securityProps) { + this.postProcessor.init(securityProps); + } + + @Override + public void setSecurityManager(final SecurityManager securityManager) { + // nothing + } + + @Override + public void setPostProcessor(final PostProcessor postProcessor) { + // nothing + } + + /** + * It first looks the shiro subject in AccessControlContext since JMX will use multiple threads to + * process operations from the same client, then it looks into Shiro's thead context. + * + * @return the shiro subject, null if security is not enabled + */ + @Override + public Subject getSubject() { + return null; + } + + /** + * @return null if security is not enabled, otherwise return a shiro subject + */ + @Override + public Subject login(final Properties credentials) { + return null; + } + + @Override + public void logout() { + // nothing + } + + @Override + public Callable associateWith(final Callable callable) { + return null; + } + + @Override + public ThreadState bindSubject(final Subject subject) { + return null; + } + + @Override + public void authorize(final ResourceOperation resourceOperation) { + // nothing + } + + @Override + public void authorizeClusterManage() { + // nothing + } + + @Override + public void authorizeClusterWrite() { + // nothing + } + + @Override + public void authorizeClusterRead() { + // nothing + } + + @Override + public void authorizeDataManage() { + // nothing + } + + @Override + public void authorizeDataWrite() { + // nothing + } + + @Override + public void authorizeDataRead() { + // nothing + } + + @Override + public void authorizeRegionManage(final String regionName) { + // nothing + } + + @Override + public void authorizeRegionManage(final String regionName, final String key) { + // nothing + } + + @Override + public void authorizeRegionWrite(final String regionName) { + // nothing + } + + @Override + public void authorizeRegionWrite(final String regionName, final String key) { + // nothing + } + + @Override + public void authorizeRegionRead(final String regionName) { + // nothing + } + + @Override + public void authorizeRegionRead(final String regionName, final String key) { + // nothing + } + + @Override + public void authorize(final String resource, final String operation) { + // nothing + } + + @Override + public void authorize(final String resource, final String operation, final String regionName) { + // nothing + } + + @Override + public void authorize(final String resource, final String operation, String regionName, + final String key) { + // nothing + } + + @Override + public void authorize(final ResourcePermission context) { + // nothing + } + + @Override + public void close() { + if (this.postProcessor != null) { + this.postProcessor.close(); + } + } + + /** + * postProcess call already has this logic built in, you don't need to call this everytime you + * call postProcess. But if your postProcess is pretty involved with preparations and you need to + * bypass it entirely, call this first. + */ + @Override + public boolean needPostProcess() { + return true; + } + + @Override + public Object postProcess(final String regionPath, final Object key, final Object value, + final boolean valueIsSerialized) { + return postProcess(null, regionPath, key, value, valueIsSerialized); + } + + @Override + public Object postProcess(Object principal, final String regionPath, final Object key, + final Object value, final boolean valueIsSerialized) { + if (principal == null) { + Subject subject = getSubject(); + if (subject == null) { + return value; + } + principal = (Serializable) subject.getPrincipal(); + } + + String regionName = StringUtils.stripStart(regionPath, "/"); + Object newValue; + + // if the data is a byte array, but the data itself is supposed to be an object, we need to + // deserialize it before we pass it to the callback. + if (valueIsSerialized && value instanceof byte[]) { + try { + Object oldObj = EntryEventImpl.deserialize((byte[]) value); + Object newObj = this.postProcessor.processRegionValue(principal, regionName, key, oldObj); + newValue = BlobHelper.serializeToBlob(newObj); + } catch (IOException | SerializationException e) { + throw new GemFireIOException("Exception de/serializing entry value", e); + } + } else { + newValue = this.postProcessor.processRegionValue(principal, regionName, key, value); + } + + return newValue; + } + + @Override + public SecurityManager getSecurityManager() { + return null; + } + + @Override + public PostProcessor getPostProcessor() { + return this.postProcessor; + } + + @Override + public boolean isIntegratedSecurity() { + return false; + } + + @Override + public boolean isClientSecurityRequired() { + return false; + } + + @Override + public boolean isPeerSecurityRequired() { + return false; + } +} http://git-wip-us.apache.org/repos/asf/geode/blob/288158d1/geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java b/geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java index 4453651..481c385 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java +++ b/geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java @@ -14,12 +14,13 @@ */ package org.apache.geode.internal.security; -import static org.apache.geode.distributed.ConfigurationProperties.*; - -import java.util.Properties; +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_CLIENT_AUTHENTICATOR; +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER; +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_PEER_AUTHENTICATOR; +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_POST_PROCESSOR; +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_SHIRO_INIT; import org.apache.commons.lang.StringUtils; - import org.apache.geode.distributed.internal.DistributionConfig; import org.apache.geode.internal.cache.CacheConfig; import org.apache.geode.internal.security.shiro.ConfigInitialization; @@ -28,6 +29,8 @@ import org.apache.geode.security.SecurityManager; import org.apache.shiro.SecurityUtils; import org.apache.shiro.UnavailableSecurityManagerException; +import java.util.Properties; + public class SecurityServiceFactory { private SecurityServiceFactory() { @@ -63,7 +66,7 @@ public class SecurityServiceFactory { public static SecurityService create(Properties securityConfig, SecurityManager securityManager, PostProcessor postProcessor) { - SecurityServiceType type = determineType(securityConfig, securityManager); + SecurityServiceType type = determineType(securityConfig, securityManager, postProcessor); switch (type) { case CUSTOM: String shiroConfig = getProperty(securityConfig, SECURITY_SHIRO_INIT); @@ -74,6 +77,8 @@ public class SecurityServiceFactory { return new CustomSecurityService(); case ENABLED: return new EnabledSecurityService(securityManager, postProcessor); + case POST_PROCESSOR: + return new PostProcessorService(postProcessor); case LEGACY: String clientAuthenticator = getProperty(securityConfig, SECURITY_CLIENT_AUTHENTICATOR); String peerAuthenticator = getProperty(securityConfig, SECURITY_PEER_AUTHENTICATOR); @@ -84,7 +89,7 @@ public class SecurityServiceFactory { } static SecurityServiceType determineType(Properties securityConfig, - SecurityManager securityManager) { + SecurityManager securityManager, PostProcessor postProcessor) { boolean hasShiroConfig = hasProperty(securityConfig, SECURITY_SHIRO_INIT); if (hasShiroConfig) { return SecurityServiceType.CUSTOM; @@ -106,6 +111,11 @@ public class SecurityServiceFactory { return SecurityServiceType.CUSTOM; } + boolean hasPostProcessor = postProcessor != null; + if (hasPostProcessor) { + return SecurityServiceType.POST_PROCESSOR; + } + return SecurityServiceType.DISABLED; } @@ -184,5 +194,4 @@ public class SecurityServiceFactory { securityService.initSecurity(distributionConfig.getSecurityProps()); } } - } http://git-wip-us.apache.org/repos/asf/geode/blob/288158d1/geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java b/geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java index 8ae76d2..73fde6b 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java +++ b/geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java @@ -19,6 +19,8 @@ public enum SecurityServiceType { ENABLED, /** Security is Disabled */ DISABLED, + /** Has PostProcessor only */ + POST_PROCESSOR, /** Legacy Security is Enabled */ LEGACY, /** Shiro Config is specified */ http://git-wip-us.apache.org/repos/asf/geode/blob/288158d1/geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java b/geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java new file mode 100644 index 0000000..83cea46 --- /dev/null +++ b/geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java @@ -0,0 +1,92 @@ +/* + * 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.internal.security; + +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_CLIENT_AUTHENTICATOR; +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_PEER_AUTHENTICATOR; +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_SHIRO_INIT; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +import org.apache.geode.security.PostProcessor; +import org.apache.geode.security.SecurityManager; +import org.apache.geode.test.junit.categories.IntegrationTest; +import org.apache.shiro.SecurityUtils; +import org.apache.shiro.util.ThreadContext; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.rules.TemporaryFolder; + +import java.util.Properties; + +@Category(IntegrationTest.class) +public class SecurityServiceFactoryShiroIntegrationTest { + + private static final String SHIRO_INI_FILE = "SecurityServiceFactoryShiroIntegrationTest.ini"; + + private String shiroIniInClasspath; + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @Before + public void before() throws Exception { + assertThat(getClass().getResource(SHIRO_INI_FILE)).isNotNull(); + this.shiroIniInClasspath = getResourcePackage(getClass()) + SHIRO_INI_FILE; + } + + @After + public void after() throws Exception { + ThreadContext.remove(); + SecurityUtils.setSecurityManager(null); + } + + @Test + public void getResourcePackage_shouldReturnPackageWithSlashes() throws Exception { + String expected = "org/apache/geode/internal/security/"; + assertThat(getResourcePackage(getClass())).isEqualTo(expected); + } + + @Test + public void create_shiro_createsCustomSecurityService() throws Exception { + Properties securityConfig = new Properties(); + securityConfig.setProperty(SECURITY_SHIRO_INIT, this.shiroIniInClasspath); + + assertThat(SecurityServiceFactory.create(securityConfig, null, null)) + .isInstanceOf(CustomSecurityService.class); + } + + @Test + public void create_all_createsCustomSecurityService() throws Exception { + Properties securityConfig = new Properties(); + securityConfig.setProperty(SECURITY_SHIRO_INIT, this.shiroIniInClasspath); + securityConfig.setProperty(SECURITY_CLIENT_AUTHENTICATOR, "value"); + securityConfig.setProperty(SECURITY_PEER_AUTHENTICATOR, "value"); + + SecurityManager mockSecurityManager = mock(SecurityManager.class); + PostProcessor mockPostProcessor = mock(PostProcessor.class); + + assertThat( + SecurityServiceFactory.create(securityConfig, mockSecurityManager, mockPostProcessor)) + .isInstanceOf(CustomSecurityService.class); + } + + private String getResourcePackage(Class classInPackage) { + return classInPackage.getName().replace(classInPackage.getSimpleName(), "").replace(".", "/"); + } +} http://git-wip-us.apache.org/repos/asf/geode/blob/288158d1/geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java b/geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java index 0f2a5d9..2029642 100644 --- a/geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java +++ b/geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java @@ -14,23 +14,34 @@ */ package org.apache.geode.internal.security; -import static org.apache.geode.distributed.ConfigurationProperties.*; -import static org.assertj.core.api.Assertions.*; -import static org.mockito.Mockito.*; - -import java.util.Properties; +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_CLIENT_AUTHENTICATOR; +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER; +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_PEER_AUTHENTICATOR; +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_POST_PROCESSOR; +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_SHIRO_INIT; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; import org.apache.geode.security.PostProcessor; import org.apache.geode.security.SecurityManager; import org.apache.geode.test.junit.categories.UnitTest; - -import org.junit.Ignore; +import org.apache.shiro.SecurityUtils; +import org.apache.shiro.util.ThreadContext; +import org.junit.After; import org.junit.Test; import org.junit.experimental.categories.Category; +import java.util.Properties; + @Category(UnitTest.class) public class SecurityServiceFactoryTest { + @After + public void after() throws Exception { + ThreadContext.remove(); + SecurityUtils.setSecurityManager(null); + } + @Test public void getPostProcessor_null_returnsNull() throws Exception { assertThat(SecurityServiceFactory.getPostProcessor(null, null)).isNull(); @@ -110,7 +121,7 @@ public class SecurityServiceFactoryTest { @Test public void determineType_null_returnsDISABLED() throws Exception { - assertThat(SecurityServiceFactory.determineType(null, null)) + assertThat(SecurityServiceFactory.determineType(null, null, null)) .isSameAs(SecurityServiceType.DISABLED); } @@ -119,7 +130,7 @@ public class SecurityServiceFactoryTest { Properties securityConfig = new Properties(); securityConfig.setProperty(SECURITY_SHIRO_INIT, "value"); - assertThat(SecurityServiceFactory.determineType(securityConfig, null)) + assertThat(SecurityServiceFactory.determineType(securityConfig, null, null)) .isSameAs(SecurityServiceType.CUSTOM); } @@ -128,11 +139,30 @@ public class SecurityServiceFactoryTest { Properties securityConfig = new Properties(); SecurityManager mockSecurityManager = mock(SecurityManager.class); - assertThat(SecurityServiceFactory.determineType(securityConfig, mockSecurityManager)) + assertThat(SecurityServiceFactory.determineType(securityConfig, mockSecurityManager, null)) .isSameAs(SecurityServiceType.ENABLED); } @Test + public void determineType_postProcessor_returnsPOST_PROCESSOR() throws Exception { + Properties securityConfig = new Properties(); + PostProcessor mockPostProcessor = mock(PostProcessor.class); + + assertThat(SecurityServiceFactory.determineType(securityConfig, null, mockPostProcessor)) + .isSameAs(SecurityServiceType.POST_PROCESSOR); + } + + @Test + public void determineType_both_returnsENABLED() throws Exception { + Properties securityConfig = new Properties(); + SecurityManager mockSecurityManager = mock(SecurityManager.class); + PostProcessor mockPostProcessor = mock(PostProcessor.class); + + assertThat(SecurityServiceFactory.determineType(securityConfig, mockSecurityManager, + mockPostProcessor)).isSameAs(SecurityServiceType.ENABLED); + } + + @Test public void determineType_prefersCUSTOM() throws Exception { Properties securityConfig = new Properties(); securityConfig.setProperty(SECURITY_SHIRO_INIT, "value"); @@ -140,7 +170,7 @@ public class SecurityServiceFactoryTest { securityConfig.setProperty(SECURITY_PEER_AUTHENTICATOR, "value"); SecurityManager mockSecurityManager = mock(SecurityManager.class); - assertThat(SecurityServiceFactory.determineType(securityConfig, mockSecurityManager)) + assertThat(SecurityServiceFactory.determineType(securityConfig, mockSecurityManager, null)) .isSameAs(SecurityServiceType.CUSTOM); } @@ -149,7 +179,7 @@ public class SecurityServiceFactoryTest { Properties securityConfig = new Properties(); securityConfig.setProperty(SECURITY_CLIENT_AUTHENTICATOR, "value"); - assertThat(SecurityServiceFactory.determineType(securityConfig, null)) + assertThat(SecurityServiceFactory.determineType(securityConfig, null, null)) .isSameAs(SecurityServiceType.LEGACY); } @@ -158,7 +188,7 @@ public class SecurityServiceFactoryTest { Properties securityConfig = new Properties(); securityConfig.setProperty(SECURITY_PEER_AUTHENTICATOR, "value"); - assertThat(SecurityServiceFactory.determineType(securityConfig, null)) + assertThat(SecurityServiceFactory.determineType(securityConfig, null, null)) .isSameAs(SecurityServiceType.LEGACY); } @@ -168,7 +198,7 @@ public class SecurityServiceFactoryTest { securityConfig.setProperty(SECURITY_CLIENT_AUTHENTICATOR, "value"); securityConfig.setProperty(SECURITY_PEER_AUTHENTICATOR, "value"); - assertThat(SecurityServiceFactory.determineType(securityConfig, null)) + assertThat(SecurityServiceFactory.determineType(securityConfig, null, null)) .isSameAs(SecurityServiceType.LEGACY); } @@ -176,21 +206,11 @@ public class SecurityServiceFactoryTest { public void determineType_empty_returnsDISABLED() throws Exception { Properties securityConfig = new Properties(); - assertThat(SecurityServiceFactory.determineType(securityConfig, null)) + assertThat(SecurityServiceFactory.determineType(securityConfig, null, null)) .isSameAs(SecurityServiceType.DISABLED); } @Test - @Ignore("Move to IntegrationTest with shiro config") - public void create_shiro_createsCustomSecurityService() throws Exception { - Properties securityConfig = new Properties(); - securityConfig.setProperty(SECURITY_SHIRO_INIT, "value"); - - assertThat(SecurityServiceFactory.create(securityConfig, null, null)) - .isInstanceOf(CustomSecurityService.class); - } - - @Test public void create_clientAuthenticator_createsLegacySecurityService() throws Exception { Properties securityConfig = new Properties(); securityConfig.setProperty(SECURITY_CLIENT_AUTHENTICATOR, "value"); @@ -219,26 +239,29 @@ public class SecurityServiceFactoryTest { } @Test - @Ignore("Move to IntegrationTest with shiro config") - public void create_all_createsCustomSecurityService() throws Exception { + public void create_none_createsDisabledSecurityService() throws Exception { Properties securityConfig = new Properties(); - securityConfig.setProperty(SECURITY_SHIRO_INIT, "value"); - securityConfig.setProperty(SECURITY_CLIENT_AUTHENTICATOR, "value"); - securityConfig.setProperty(SECURITY_PEER_AUTHENTICATOR, "value"); - SecurityManager mockSecurityManager = mock(SecurityManager.class); + assertThat(SecurityServiceFactory.create(securityConfig, null, null)) + .isInstanceOf(DisabledSecurityService.class); + } + + @Test + public void create_postProcessor_createsPostProcessorService() throws Exception { + Properties securityConfig = new Properties(); PostProcessor mockPostProcessor = mock(PostProcessor.class); - assertThat( - SecurityServiceFactory.create(securityConfig, mockSecurityManager, mockPostProcessor)) - .isInstanceOf(CustomSecurityService.class); + assertThat(SecurityServiceFactory.create(securityConfig, null, mockPostProcessor)) + .isInstanceOf(PostProcessorService.class); } @Test - public void create_none_createsDisabledSecurityService() throws Exception { + public void create_securityManager_createsPostProcessorService() throws Exception { Properties securityConfig = new Properties(); + SecurityManager mockSecurityManager = mock(SecurityManager.class); - assertThat(SecurityServiceFactory.create(securityConfig, null, null)) - .isInstanceOf(DisabledSecurityService.class); + assertThat(SecurityServiceFactory.create(securityConfig, mockSecurityManager, null)) + .isInstanceOf(EnabledSecurityService.class); } + } http://git-wip-us.apache.org/repos/asf/geode/blob/288158d1/geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java index 47188f2..55df41b 100644 --- a/geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java +++ b/geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java @@ -48,8 +48,7 @@ public class SecurityServiceWithShiroIniIntegrationTest { @Before public void before() { - this.securityService = SecurityServiceFactory.create(null, null); - this.securityService.initSecurity(props); + this.securityService = SecurityServiceFactory.create(props, null, null); } @Test http://git-wip-us.apache.org/repos/asf/geode/blob/288158d1/geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java b/geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java index 8cabb1c..827ae3d 100644 --- a/geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java +++ b/geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.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.security; import static org.junit.Assert.assertFalse; @@ -44,15 +43,15 @@ public class CacheFactoryWithSecurityObjectTest { @Before public void before() throws Exception { - simpleSecurityManager = new SimpleTestSecurityManager(); - properties.setProperty("mcast-port", "0"); + this.simpleSecurityManager = new SimpleTestSecurityManager(); + this.properties.setProperty("mcast-port", "0"); } @Test public void testCreateCacheWithSecurityManager() throws Exception { - cache = (InternalCache) new CacheFactory(properties).setSecurityManager(simpleSecurityManager) - .setPostProcessor(null).create(); - SecurityService securityService = cache.getSecurityService(); + this.cache = (InternalCache) new CacheFactory(this.properties) + .setSecurityManager(this.simpleSecurityManager).setPostProcessor(null).create(); + SecurityService securityService = this.cache.getSecurityService(); assertTrue(securityService.isIntegratedSecurity()); assertFalse(securityService.needPostProcess()); assertNotNull(securityService.getSecurityManager()); @@ -60,9 +59,9 @@ public class CacheFactoryWithSecurityObjectTest { @Test public void testCreateCacheWithPostProcessor() throws Exception { - cache = (InternalCache) new CacheFactory(properties).setPostProcessor(new TestPostProcessor()) - .setSecurityManager(null).create(); - SecurityService securityService = cache.getSecurityService(); + this.cache = (InternalCache) new CacheFactory(this.properties) + .setPostProcessor(new TestPostProcessor()).setSecurityManager(null).create(); + SecurityService securityService = this.cache.getSecurityService(); assertFalse(securityService.isIntegratedSecurity()); assertFalse(securityService.needPostProcess()); assertNotNull(securityService.getPostProcessor()); @@ -70,13 +69,14 @@ public class CacheFactoryWithSecurityObjectTest { @Test public void testOverride() throws Exception { - properties.setProperty(ConfigurationProperties.SECURITY_CLIENT_AUTHENTICATOR, + this.properties.setProperty(ConfigurationProperties.SECURITY_CLIENT_AUTHENTICATOR, DummyAuthenticator.class.getName()); - cache = (InternalCache) new CacheFactory(properties).setSecurityManager(simpleSecurityManager) - .setPostProcessor(new TestPostProcessor()).create(); + this.cache = (InternalCache) new CacheFactory(this.properties) + .setSecurityManager(this.simpleSecurityManager).setPostProcessor(new TestPostProcessor()) + .create(); - SecurityService securityService = cache.getSecurityService(); + SecurityService securityService = this.cache.getSecurityService(); assertTrue(securityService.isIntegratedSecurity()); assertTrue(securityService.isClientSecurityRequired()); @@ -86,7 +86,7 @@ public class CacheFactoryWithSecurityObjectTest { @After public void after() { - cache.close(); + this.cache.close(); } } http://git-wip-us.apache.org/repos/asf/geode/blob/288158d1/geode-core/src/test/resources/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.ini ---------------------------------------------------------------------- diff --git a/geode-core/src/test/resources/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.ini b/geode-core/src/test/resources/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.ini new file mode 100644 index 0000000..8f7ffa7 --- /dev/null +++ b/geode-core/src/test/resources/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.ini @@ -0,0 +1,30 @@ +# 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. + +# the users and roles in this file needs to be kept in sync with shiro.ini +# since they are used by the same test to test ShiroUtil +# ----------------------------------------------------------------------------- +# Users and their (optional) assigned roles +# username = password, role1, role2, ..., roleN +# ----------------------------------------------------------------------------- +[users] +root = secret, admin + +# ----------------------------------------------------------------------------- +# Roles with assigned permissions +# roleName = perm1, perm2, ..., permN +# ----------------------------------------------------------------------------- +[roles] +admin = *
