This is an automated email from the ASF dual-hosted git repository.
jensdeppe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 9a26976 GEODE-4570: Remove getInstance() singleton call from
SecurityServiceF… (#1482)
9a26976 is described below
commit 9a26976e2e30b4277b920a9c7c73432b2dfa6d61
Author: Jens Deppe <jde...@pivotal.io>
AuthorDate: Thu Feb 22 14:41:57 2018 -0800
GEODE-4570: Remove getInstance() singleton call from SecurityServiceF…
(#1482)
* Also now need to pass a SecurityContext into the Spring environment
---
.../geode/internal/cache/GemFireCacheImpl.java | 2 +-
.../internal/security/SecurityServiceFactory.java | 10 --------
.../geode/management/internal/JettyHelper.java | 30 +++++-----------------
.../geode/management/internal/ManagementAgent.java | 19 +++++++-------
.../geode/management/internal/RestAgent.java | 11 +++++---
.../internal/beans/DistributedSystemBridge.java | 10 +++-----
.../internal/beans/DistributedSystemMBean.java | 3 +--
.../support/LoginHandlerInterceptor.java | 28 +++++++++++++++-----
.../geode/tools/pulse/tests/rules/ServerRule.java | 2 +-
.../support/LoginHandlerInterceptorJUnitTest.java | 16 ++++++++++--
10 files changed, 67 insertions(+), 64 deletions(-)
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
index cc4c879..efb6f07 100755
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
@@ -1262,7 +1262,7 @@ public class GemFireCacheImpl implements InternalCache,
InternalClientCache, Has
private void startRestAgentServer(GemFireCacheImpl cache) {
if (this.system.getConfig().getStartDevRestApi() && isNotJmxManager() &&
isServerNode()) {
- this.restAgent = new RestAgent(this.system.getConfig());
+ this.restAgent = new RestAgent(this.system.getConfig(),
this.securityService);
this.restAgent.start(cache);
} else {
this.restAgent = null;
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 ed5fe72..bb43125 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
@@ -25,8 +25,6 @@ import org.apache.shiro.SecurityUtils;
import org.apache.shiro.UnavailableSecurityManagerException;
import org.apache.geode.internal.cache.CacheConfig;
-import org.apache.geode.internal.cache.GemFireCacheImpl;
-import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.security.shiro.SecurityManagerProvider;
import org.apache.geode.security.PostProcessor;
import org.apache.geode.security.SecurityManager;
@@ -90,14 +88,6 @@ public class SecurityServiceFactory {
return new LegacySecurityService(clientAuthenticatorConfig,
peerAuthenticatorConfig);
}
- public static SecurityService findSecurityService() {
- InternalCache cache = GemFireCacheImpl.getInstance();
- if (cache != null) {
- return cache.getSecurityService();
- }
- return SecurityServiceFactory.create();
- }
-
private static boolean isShiroInUse() {
try {
return SecurityUtils.getSecurityManager() != null;
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java
b/geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java
index 9281154..92950c0 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java
@@ -33,7 +33,9 @@ import org.eclipse.jetty.webapp.WebAppContext;
import org.apache.geode.GemFireConfigException;
import org.apache.geode.internal.admin.SSLConfig;
+import org.apache.geode.internal.cache.GemFireCacheImpl;
import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.internal.security.SecurityService;
/**
* @since GemFire 8.1
@@ -54,6 +56,9 @@ public class JettyHelper {
private static int port = 0;
+ public static final String SECURITY_SERVICE_SERVLET_CONTEXT_PARAM =
+ "org.apache.geode.securityService";
+
public static Server initJetty(final String bindAddress, final int port,
SSLConfig sslConfig) {
final Server jettyServer = new Server();
@@ -152,12 +157,13 @@ public class JettyHelper {
}
public static Server addWebApplication(final Server jetty, final String
webAppContext,
- final String warFilePath) {
+ final String warFilePath, SecurityService securityService) {
WebAppContext webapp = new WebAppContext();
webapp.setContextPath(webAppContext);
webapp.setWar(warFilePath);
webapp.setParentLoaderPriority(false);
webapp.setInitParameter("org.eclipse.jetty.servlet.Default.dirAllowed",
"false");
+ webapp.setAttribute(SECURITY_SERVICE_SERVLET_CONTEXT_PARAM,
securityService);
File tmpPath = new File(getWebAppBaseDirectory(webAppContext));
tmpPath.mkdirs();
@@ -191,26 +197,4 @@ public class JettyHelper {
return (webAppContext.startsWith("/") ? webAppContext : "/" +
webAppContext);
}
- public static void main(final String... args) throws Exception {
- if (args.length > 1) {
- System.out.printf("Temporary Directory @ ($1%s)%n", USER_DIR);
-
- final Server jetty = JettyHelper.initJetty(null, 8090, new SSLConfig());
-
- for (int index = 0; index < args.length; index += 2) {
- final String webAppContext = args[index];
- final String webAppArchivePath = args[index + 1];
-
- JettyHelper.addWebApplication(jetty,
normalizeWebAppContext(webAppContext),
- normalizeWebAppArchivePath(webAppArchivePath));
- }
-
- JettyHelper.startJetty(jetty);
- latch.await();
- } else {
- System.out.printf(
- "usage:%n>java org.apache.geode.management.internal.TomcatHelper
<web-app-context> <war-file-path> [<web-app-context> <war-file-path>]*");
- }
- }
-
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java
b/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java
index 55df263..3f47a25 100755
---
a/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java
@@ -249,22 +249,23 @@ public class ManagementAgent {
.getSSLConfigForComponent(config,
SecurableCommunicationChannel.WEB));
if (agentUtil.isWebApplicationAvailable(gemfireWar)) {
- this.httpServer =
- JettyHelper.addWebApplication(this.httpServer, "/gemfire",
gemfireWar);
- this.httpServer =
- JettyHelper.addWebApplication(this.httpServer, "/geode-mgmt",
gemfireWar);
+ this.httpServer = JettyHelper.addWebApplication(this.httpServer,
"/gemfire", gemfireWar,
+ securityService);
+ this.httpServer = JettyHelper.addWebApplication(this.httpServer,
"/geode-mgmt",
+ gemfireWar, securityService);
}
if (agentUtil.isWebApplicationAvailable(pulseWar)) {
- this.httpServer = JettyHelper.addWebApplication(this.httpServer,
"/pulse", pulseWar);
+ this.httpServer =
+ JettyHelper.addWebApplication(this.httpServer, "/pulse",
pulseWar, securityService);
}
if (isServer && this.config.getStartDevRestApi()) {
if (agentUtil.isWebApplicationAvailable(gemfireAPIWar)) {
- this.httpServer =
- JettyHelper.addWebApplication(this.httpServer, "/geode",
gemfireAPIWar);
- this.httpServer =
- JettyHelper.addWebApplication(this.httpServer,
"/gemfire-api", gemfireAPIWar);
+ this.httpServer = JettyHelper.addWebApplication(this.httpServer,
"/geode",
+ gemfireAPIWar, securityService);
+ this.httpServer = JettyHelper.addWebApplication(this.httpServer,
"/gemfire-api",
+ gemfireAPIWar, securityService);
isRestWebAppAdded = true;
}
} else {
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java
b/geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java
index 3cf4915..dccf883 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java
@@ -34,6 +34,7 @@ import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.net.SSLConfigurationFactory;
import org.apache.geode.internal.net.SocketCreator;
import org.apache.geode.internal.security.SecurableCommunicationChannel;
+import org.apache.geode.internal.security.SecurityService;
import org.apache.geode.management.ManagementService;
/**
@@ -50,9 +51,11 @@ public class RestAgent {
private boolean running = false;
private final DistributionConfig config;
+ private final SecurityService securityService;
- public RestAgent(DistributionConfig config) {
+ public RestAgent(DistributionConfig config, SecurityService securityService)
{
this.config = config;
+ this.securityService = securityService;
}
public synchronized boolean isRunning() {
@@ -134,8 +137,10 @@ public class RestAgent {
this.httpServer = JettyHelper.initJetty(httpServiceBindAddress, port,
SSLConfigurationFactory.getSSLConfigForComponent(SecurableCommunicationChannel.WEB));
- this.httpServer = JettyHelper.addWebApplication(httpServer,
"/gemfire-api", gemfireAPIWar);
- this.httpServer = JettyHelper.addWebApplication(httpServer, "/geode",
gemfireAPIWar);
+ this.httpServer = JettyHelper.addWebApplication(httpServer,
"/gemfire-api", gemfireAPIWar,
+ securityService);
+ this.httpServer =
+ JettyHelper.addWebApplication(httpServer, "/geode", gemfireAPIWar,
securityService);
if (logger.isDebugEnabled()) {
logger.debug("Starting HTTP embedded server on port ({}) at
bind-address ({})...",
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java
b/geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java
index 09403a9..bfb8d68 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java
@@ -54,7 +54,6 @@ import
org.apache.geode.internal.admin.remote.MissingPersistentIDsRequest;
import org.apache.geode.internal.admin.remote.PrepareRevokePersistentIDRequest;
import org.apache.geode.internal.admin.remote.RevokePersistentIDRequest;
import org.apache.geode.internal.admin.remote.ShutdownAllRequest;
-import org.apache.geode.internal.cache.GemFireCacheImpl;
import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.cache.backup.BackupDataStoreHelper;
import org.apache.geode.internal.cache.backup.BackupDataStoreResult;
@@ -147,11 +146,6 @@ public class DistributedSystemBridge {
private volatile int gatewayReceiverSetSize;
/**
- * Member MBean for current member
- */
- private MemberMXBean thisMember;
-
- /**
* Cache instance
*/
private InternalCache cache;
@@ -266,6 +260,10 @@ public class DistributedSystemBridge {
this.receiverMonitor = new GatewayReceiverClusterStatsMonitor();
}
+ public InternalCache getCache() {
+ return cache;
+ }
+
/**
* Add a proxy to the map to be used by bridge.
*
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemMBean.java
b/geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemMBean.java
index 28edbf6..ea5d110 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemMBean.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemMBean.java
@@ -19,7 +19,6 @@ import java.util.Map;
import javax.management.NotificationBroadcasterSupport;
import javax.management.ObjectName;
-import org.apache.geode.internal.security.SecurityServiceFactory;
import org.apache.geode.management.DiskBackupStatus;
import org.apache.geode.management.DiskMetrics;
import org.apache.geode.management.DistributedSystemMXBean;
@@ -53,7 +52,7 @@ public class DistributedSystemMBean extends
NotificationBroadcasterSupport
@Override
public DiskBackupStatus backupAllMembers(String targetDirPath, String
baselineDirPath)
throws Exception {
- SecurityServiceFactory.findSecurityService().authorize(Resource.CLUSTER,
Operation.WRITE,
+ bridge.getCache().getSecurityService().authorize(Resource.CLUSTER,
Operation.WRITE,
Target.DISK);
return bridge.backupAllMembers(targetDirPath, baselineDirPath);
}
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 d609105..ef182d2 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,23 +14,25 @@
*/
package org.apache.geode.management.internal.web.controllers.support;
-import static
org.apache.geode.internal.security.SecurityServiceFactory.findSecurityService;
-
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Map;
import java.util.Properties;
+import javax.servlet.ServletContext;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.logging.log4j.Logger;
+import org.springframework.web.context.ServletContextAware;
import org.springframework.web.servlet.handler.HandlerInterceptorAdapter;
+import org.apache.geode.annotations.TestingOnly;
import org.apache.geode.distributed.internal.DistributionConfig;
import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.security.SecurityService;
+import org.apache.geode.management.internal.JettyHelper;
import org.apache.geode.management.internal.security.ResourceConstants;
/**
@@ -44,11 +46,12 @@ import
org.apache.geode.management.internal.security.ResourceConstants;
* @since GemFire 8.0
*/
@SuppressWarnings("unused")
-public class LoginHandlerInterceptor extends HandlerInterceptorAdapter {
+public class LoginHandlerInterceptor extends HandlerInterceptorAdapter
+ implements ServletContextAware {
private static final Logger logger = LogService.getLogger();
- private final SecurityService securityService;
+ private SecurityService securityService;
private static final ThreadLocal<Map<String, String>> ENV =
new ThreadLocal<Map<String, String>>() {
@@ -63,10 +66,9 @@ public class LoginHandlerInterceptor extends
HandlerInterceptorAdapter {
protected static final String SECURITY_VARIABLE_REQUEST_HEADER_PREFIX =
DistributionConfig.SECURITY_PREFIX_NAME;
- public LoginHandlerInterceptor() {
- this(findSecurityService());
- }
+ public LoginHandlerInterceptor() {}
+ @TestingOnly
LoginHandlerInterceptor(SecurityService securityService) {
this.securityService = securityService;
}
@@ -117,4 +119,16 @@ public class LoginHandlerInterceptor extends
HandlerInterceptorAdapter {
HttpServletResponse response, Object handler) throws Exception {
ENV.remove();
}
+
+ /**
+ * This is used to pass attributes into the Spring/Jetty environment from
the instantiating Geode
+ * environment.
+ *
+ * @param servletContext
+ */
+ @Override
+ public void setServletContext(ServletContext servletContext) {
+ securityService = (SecurityService) servletContext
+ .getAttribute(JettyHelper.SECURITY_SERVICE_SERVLET_CONTEXT_PARAM);
+ }
}
diff --git
a/geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
b/geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
index 389e3d0..e5e4488 100644
---
a/geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
+++
b/geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
@@ -46,7 +46,7 @@ public class ServerRule extends ExternalResource {
int httpPort = AvailablePort.getRandomAvailablePort(AvailablePort.SOCKET);
jetty = JettyHelper.initJetty(LOCALHOST, httpPort, new SSLConfig());
- JettyHelper.addWebApplication(jetty, PULSE_CONTEXT, getPulseWarPath());
+ JettyHelper.addWebApplication(jetty, PULSE_CONTEXT, getPulseWarPath(),
null);
pulseURL = "http://"; + LOCALHOST + ":" + httpPort + PULSE_CONTEXT;
System.out.println("Pulse started at " + pulseURL);
}
diff --git
a/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptorJUnitTest.java
b/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptorJUnitTest.java
index 52b09e2..1cf4484 100644
---
a/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptorJUnitTest.java
+++
b/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptorJUnitTest.java
@@ -20,6 +20,7 @@ import java.util.Enumeration;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
+import java.util.Properties;
import javax.servlet.http.HttpServletRequest;
@@ -34,6 +35,7 @@ import org.junit.Before;
import org.junit.Test;
import org.junit.experimental.categories.Category;
+import org.apache.geode.internal.security.SecurityService;
import org.apache.geode.management.internal.security.ResourceConstants;
import org.apache.geode.test.junit.categories.UnitTest;
@@ -51,12 +53,16 @@ public class LoginHandlerInterceptorJUnitTest {
private Mockery mockContext;
+ private SecurityService securityService;
+
@Before
public void setUp() {
mockContext = new Mockery();
mockContext.setImposteriser(ClassImposteriser.INSTANCE);
mockContext.setThreadingPolicy(new Synchroniser());
LoginHandlerInterceptor.getEnvironment().clear();
+
+ securityService = mockContext.mock(SecurityService.class);
}
@After
@@ -102,10 +108,12 @@ public class LoginHandlerInterceptorJUnitTest {
will(returnValue("password"));
oneOf(mockHttpRequest).getParameter(with(equal(createEnvironmentVariable("variable"))));
will(returnValue(requestParameters.get(createEnvironmentVariable("variable"))));
+ oneOf(securityService).login(with(aNonNull(Properties.class)));
+ oneOf(securityService).logout();
}
});
- LoginHandlerInterceptor handlerInterceptor = new LoginHandlerInterceptor();
+ LoginHandlerInterceptor handlerInterceptor = new
LoginHandlerInterceptor(securityService);
Map<String, String> envBefore = LoginHandlerInterceptor.getEnvironment();
@@ -167,6 +175,8 @@ public class LoginHandlerInterceptorJUnitTest {
oneOf(mockHttpRequestOne)
.getParameter(with(equal(createEnvironmentVariable("GEODE_HOME"))));
will(returnValue(requestParametersOne.get(createEnvironmentVariable("GEODE_HOME"))));
+ oneOf(securityService).login(with(aNonNull(Properties.class)));
+ oneOf(securityService).logout();
}
});
@@ -192,10 +202,12 @@ public class LoginHandlerInterceptorJUnitTest {
oneOf(mockHttpRequestTwo)
.getParameter(with(equal(createEnvironmentVariable("GEODE_HOME"))));
will(returnValue(requestParametersTwo.get(createEnvironmentVariable("GEODE_HOME"))));
+ oneOf(securityService).login(with(aNonNull(Properties.class)));
+ oneOf(securityService).logout();
}
});
- handlerInterceptor = new LoginHandlerInterceptor();
+ handlerInterceptor = new LoginHandlerInterceptor(securityService);
}
public void thread1() throws Exception {
--
To stop receiving notification emails like this one, please contact
jensde...@apache.org.
