geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jensde...@apache.org
Subject [geode] branch develop updated: GEODE-4570: Remove getInstance() singleton call from SecurityServiceF… (#1482)
Date Thu, 22 Feb 2018 22:42:05 GMT
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 <jdeppe@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
jensdeppe@apache.org.

Mime
View raw message