GEODE-3685: MBean wrappers are properly invoked over http

This closes #838.

geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StopServerAcceptanceTest.java

@@ -0,0 +1,66 @@
+/*
+ * 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.commands;
+
+import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.util.Properties;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.examples.SimpleSecurityManager;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.junit.categories.AcceptanceTest;
+import org.apache.geode.test.junit.rules.gfsh.GfshExecution;
+import org.apache.geode.test.junit.rules.gfsh.GfshRule;
+import org.apache.geode.test.junit.rules.gfsh.GfshScript;
+
+@Category(AcceptanceTest.class)
+public class StopServerAcceptanceTest {
+
+  @Rule
+  public GfshRule gfshRule = new GfshRule();
+
+
+  @Before
+  public void startCluster() {
+    gfshRule.execute("start locator --name=locator", "start server --name=server");
+  }
+
+  @Test
+  public void canStopServerByNameWhenConnectedOverJmx() throws Exception {
+
+    gfshRule.execute("connect", "stop server --name=server");
+  }
+
+  @Test
+  public void canStopServerByNameWhenConnectedOverHttp() throws Exception {
+
+    gfshRule.execute("connect --use-http", "stop server --name=server");
+  }
+
+  @Test
+  public void cannotStopServerByNameWhenNotConnected() throws Exception {
+    startCluster();
+
+    gfshRule.execute(GfshScript.of("stop server --name=server").expectFailure());
+  }
+}

geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StopServerWithSecurityAcceptanceTest.java

@@ -0,0 +1,139 @@
+/*
+ * 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.commands;
+
+import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.util.Properties;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.examples.SimpleSecurityManager;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.junit.categories.AcceptanceTest;
+import org.apache.geode.test.junit.rules.gfsh.GfshExecution;
+import org.apache.geode.test.junit.rules.gfsh.GfshRule;
+import org.apache.geode.test.junit.rules.gfsh.GfshScript;
+
+@Category(AcceptanceTest.class)
+public class StopServerWithSecurityAcceptanceTest {
+
+  @Rule
+  public GfshRule gfshRule = new GfshRule();
+
+  private static final Properties securityProps = new Properties();
+
+  static {
+    securityProps.setProperty(SECURITY_MANAGER, SimpleSecurityManager.class.getName());
+    securityProps.setProperty("security-username", "cluster");
+    securityProps.setProperty("security-password", "cluster");
+  }
+
+  private File securityPropertiesFile;
+
+  @Before
+  public void before() throws Exception {
+    securityPropertiesFile = gfshRule.getTemporaryFolder().newFile("security.properties");
+    securityProps.store(new FileOutputStream(securityPropertiesFile), null);
+  }
+
+  @Test
+  public void cannotStopServerAsDataReaderOverHttp() throws Exception {
+    startCluster();
+
+    GfshExecution stopServer = dataReaderCannotStopServer(true);
+    assertThat(stopServer.getStdErrText()).contains("dataReader not authorized for CLUSTER:READ");
+  }
+
+  @Test
+  public void canStopServerAsClusterAdminOverHttp() throws Exception {
+    startCluster();
+
+    clusterAdminCanStopServer(true);
+  }
+
+  @Test
+  public void cannotStopServerAsDataReaderOverJmx() throws Exception {
+    startCluster();
+
+    GfshExecution stopServer = dataReaderCannotStopServer(false);
+    assertThat(stopServer.getStdErrText()).contains("dataReader not authorized for CLUSTER:READ");
+  }
+
+  @Test
+  public void canStopServerAsClusterAdminOverJmx() throws Exception {
+    startCluster();
+
+    clusterAdminCanStopServer(false);
+  }
+
+  @Test
+  public void cannotStopServerAsClusterReaderOverJmx() throws Exception {
+    startCluster();
+
+    GfshExecution stopServer = clusterReaderCannotStopServer(false);
+    assertThat(stopServer.getStdErrText())
+        .contains("clusterRead not authorized for CLUSTER:MANAGE");
+  }
+
+  @Test
+  public void cannotStopServerAsClusterReaderOverHttp() throws Exception {
+    startCluster();
+
+    GfshExecution stopServer = clusterReaderCannotStopServer(true);
+    assertThat(stopServer.getStdErrText())
+        .contains("clusterRead not authorized for CLUSTER:MANAGE");
+  }
+
+  private GfshExecution startCluster() {
+    String startLocator = new CommandStringBuilder("start locator").addOption("name", "locator")
+        .addOption("security-properties-file", securityPropertiesFile.getAbsolutePath()).toString();
+
+    String startServer = new CommandStringBuilder("start server").addOption("name", "server")
+        .addOption("security-properties-file", securityPropertiesFile.getAbsolutePath()).toString();
+
+    return GfshScript.of(startLocator, startServer).withName("cluster-setup").execute(gfshRule);
+  }
+
+  private GfshExecution dataReaderCannotStopServer(boolean useHttp) {
+    return GfshScript.of(connectCommand("dataReader", useHttp), "stop server --name=server")
+        .expectFailure().execute(gfshRule);
+  }
+
+  private GfshExecution clusterAdminCanStopServer(boolean useHttp) {
+    return GfshScript.of(connectCommand("cluster", useHttp), "stop server --name=server")
+        .execute(gfshRule);
+  }
+
+  private GfshExecution clusterReaderCannotStopServer(boolean useHttp) {
+    return GfshScript.of(connectCommand("clusterRead", useHttp), "stop server --name=server")
+        .expectFailure().execute(gfshRule);
+  }
+
+  private String connectCommand(String permission, boolean useHttp) {
+    CommandStringBuilder cmd = new CommandStringBuilder("connect").addOption("user", permission)
+        .addOption("password", permission);
+    if (useHttp) {
+      cmd.addOption("use-http");
+    }
+    return cmd.getCommandString();
+  }
+}

geode-core/src/main/java/org/apache/geode/examples/SimpleSecurityManager.java

@@ -0,0 +1,59 @@
+/*
+ * 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.examples;
+
+import java.util.Properties;
+
+import org.apache.geode.security.AuthenticationFailedException;
+import org.apache.geode.security.ResourcePermission;
+import org.apache.geode.security.SecurityManager;
+
+/**
+ * Intended for implementation testing, this class authenticates a user when the username matches
+ * the password, which also represents the permissions the user is granted.
+ */
+public class SimpleSecurityManager implements SecurityManager {
+
+  @Override
+  public void init(final Properties securityProps) {
+    // nothing
+  }
+
+  @Override
+  public Object authenticate(final Properties credentials) throws AuthenticationFailedException {
+    String username = credentials.getProperty("security-username");
+    String password = credentials.getProperty("security-password");
+    if (username != null && username.equals(password)) {
+      return username;
+    }
+    throw new AuthenticationFailedException("invalid username/password");
+  }
+
+  @Override
+  public boolean authorize(final Object principal, final ResourcePermission permission) {
+    String[] principals = principal.toString().toLowerCase().split(",");
+    for (String role : principals) {
+      String permissionString = permission.toString().replace(":", "").toLowerCase();
+      if (permissionString.startsWith(role))
+        return true;
+    }
+    return false;
+  }
+
+  @Override
+  public void close() {
+    // nothing
+  }
+}

geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java

@@ -19,7 +19,6 @@
 import java.lang.management.ManagementFactory;
 import java.net.InetAddress;
 import java.net.ServerSocket;
-import java.net.Socket;
 import java.net.UnknownHostException;
 import java.rmi.AlreadyBoundException;
 import java.rmi.registry.LocateRegistry;
@@ -90,12 +89,12 @@ public class ManagementAgent {
    */
   private boolean running = false;
   private Registry registry;
+
   private JMXConnectorServer jmxConnectorServer;
   private JMXShiroAuthenticator shiroAuthenticator;
   private final DistributionConfig config;
   private final SecurityService securityService;
   private boolean isHttpServiceRunning = false;
-
   /**
    * This system property is set to true when the embedded HTTP server is started so that the
    * embedded pulse webapp can use a local MBeanServer instead of a remote JMX connection.
@@ -115,11 +114,11 @@ public synchronized boolean isRunning() {
     return this.running;
   }
 
-  public synchronized boolean isHttpServiceRunning() {
+  synchronized boolean isHttpServiceRunning() {
     return isHttpServiceRunning;
   }
 
-  public synchronized void setHttpServiceRunning(boolean isHttpServiceRunning) {
+  private synchronized void setHttpServiceRunning(boolean isHttpServiceRunning) {
     this.isHttpServiceRunning = isHttpServiceRunning;
   }
 
@@ -180,7 +179,7 @@ public synchronized void stopAgent() {
 
   private Server httpServer;
   private final String GEMFIRE_VERSION = GemFireVersion.getGemFireVersion();
-  private AgentUtil agentUtil = new AgentUtil(GEMFIRE_VERSION);
+  private final AgentUtil agentUtil = new AgentUtil(GEMFIRE_VERSION);
 
   private void startHttpService(boolean isServer) {
     final SystemManagementService managementService = (SystemManagementService) ManagementService
@@ -461,8 +460,7 @@ public synchronized void start() throws IOException {
             try {
               registry.bind("jmxrmi", stub);
             } catch (AlreadyBoundException x) {
-              final IOException io = new IOException(x.getMessage(), x);
-              throw io;
+              throw new IOException(x.getMessage(), x);
             }
             super.start();
           }
@@ -513,30 +511,18 @@ private void registerAccessControlMBean() {
           logger.info("Registered AccessControlMBean on " + accessControlMBeanON);
         } catch (InstanceAlreadyExistsException | MBeanRegistrationException
             | NotCompliantMBeanException e) {
-          throw new GemFireConfigException("Error while configuring accesscontrol for jmx resource",
-              e);
+          throw new GemFireConfigException(
+              "Error while configuring access control for jmx resource", e);
         }
       }
     } catch (MalformedObjectNameException e) {
-      throw new GemFireConfigException("Error while configuring accesscontrol for jmx resource", e);
+      throw new GemFireConfigException("Error while configuring access control for jmx resource",
+          e);
     }
   }
 
-  private static class GemFireRMIClientSocketFactory
-      implements RMIClientSocketFactory, Serializable {
-
-    private static final long serialVersionUID = -7604285019188827617L;
-
-    private transient SocketCreator sc;
-
-    public GemFireRMIClientSocketFactory(SocketCreator sc) {
-      this.sc = sc;
-    }
-
-    @Override
-    public Socket createSocket(String host, int port) throws IOException {
-      return this.sc.connectForClient(host, port, 0);
-    }
+  public JMXConnectorServer getJmxConnectorServer() {
+    return jmxConnectorServer;
   }
 
   private static class GemFireRMIServerSocketFactory