GEODE-10679: Refactor access checks in MBeanServerWrapper

geode-core/src/integrationTest/java/org/apache/geode/management/internal/security/MBeanServerWrapperJUnitTest.java

@@ -0,0 +1,118 @@
+/*
+ * 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.security;
+
+import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import javax.management.MBeanServer;
+import javax.management.MBeanServerConnection;
+import javax.management.ObjectName;
+
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.management.internal.MBeanJMXAdapter;
+import org.apache.geode.security.TestSecurityManager;
+import org.apache.geode.test.junit.categories.SecurityTest;
+import org.apache.geode.test.junit.rules.ConnectionConfiguration;
+import org.apache.geode.test.junit.rules.MBeanServerConnectionRule;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+/**
+ * Exercises MBean creation, registration and invocation through the JMX connector for users with
+ * different resource permissions, drawn from {@code cacheServer.json}.
+ */
+@Category({SecurityTest.class})
+public class MBeanServerWrapperJUnitTest {
+
+  private static final String DISTRIBUTED_SYSTEM_MBEAN =
+      "GemFire:service=System,type=Distributed";
+
+  private static final ObjectName OTHER_DOMAIN_BEAN = objectName("other:type=timer");
+
+  @ClassRule
+  public static ServerStarterRule server = new ServerStarterRule().withJMXManager()
+      .withProperty(SECURITY_MANAGER, TestSecurityManager.class.getName())
+      .withProperty(TestSecurityManager.SECURITY_JSON,
+          "org/apache/geode/management/internal/security/cacheServer.json")
+      .withAutoStart();
+
+  @Rule
+  public MBeanServerConnectionRule connectionRule =
+      new MBeanServerConnectionRule(server::getJmxPort);
+
+  private static ObjectName objectName(String name) {
+    try {
+      return new ObjectName(name);
+    } catch (Exception e) {
+      throw new RuntimeException(e);
+    }
+  }
+
+  @BeforeClass
+  public static void registerBeanForInvocation() throws Exception {
+    MBeanServer platform = MBeanJMXAdapter.mbeanServer;
+    if (!platform.isRegistered(OTHER_DOMAIN_BEAN)) {
+      platform.createMBean("javax.management.timer.Timer", OTHER_DOMAIN_BEAN);
+    }
+  }
+
+  @Test
+  @ConnectionConfiguration(user = "stranger", password = "1234567")
+  public void memberCountReadRequiresClusterRead() throws Exception {
+    MBeanServerConnection con = connectionRule.getMBeanServerConnection();
+    assertThatThrownBy(
+        () -> con.getAttribute(new ObjectName(DISTRIBUTED_SYSTEM_MBEAN), "MemberCount"))
+            .hasMessageContaining("CLUSTER:READ");
+  }
+
+  @Test
+  @ConnectionConfiguration(user = "cluster-reader", password = "1234567")
+  public void clusterReaderCanReadMemberCount() throws Exception {
+    MBeanServerConnection con = connectionRule.getMBeanServerConnection();
+    Object memberCount = con.getAttribute(new ObjectName(DISTRIBUTED_SYSTEM_MBEAN), "MemberCount");
+    assertThat(((Number) memberCount).intValue()).isGreaterThanOrEqualTo(1);
+  }
+
+  @Test
+  @ConnectionConfiguration(user = "stranger", password = "1234567")
+  public void createMBeanInOtherDomainRequiresClusterManage() throws Exception {
+    MBeanServerConnection con = connectionRule.getMBeanServerConnection();
+    ObjectName name = objectName("other:type=created");
+    assertThatThrownBy(() -> con.createMBean("javax.management.timer.Timer", name))
+        .satisfiesAnyOf(
+            t -> assertThat(t).isInstanceOf(SecurityException.class),
+            t -> assertThat(t).hasMessageContaining("not authorized"),
+            t -> assertThat(t).hasMessageContaining("Access Denied"));
+  }
+
+  @Test
+  @ConnectionConfiguration(user = "stranger", password = "1234567")
+  public void invokeInOtherDomainRequiresClusterManage() throws Exception {
+    MBeanServerConnection con = connectionRule.getMBeanServerConnection();
+    assertThatThrownBy(
+        () -> con.invoke(OTHER_DOMAIN_BEAN, "removeAllNotifications", new Object[] {},
+            new String[] {}))
+                .satisfiesAnyOf(
+                    t -> assertThat(t).isInstanceOf(SecurityException.class),
+                    t -> assertThat(t).hasMessageContaining("not authorized"),
+                    t -> assertThat(t).hasMessageContaining("Access Denied"));
+  }
+}

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

@@ -78,10 +78,15 @@ private void checkDomain(ObjectName name) {
     }
   }
 
+  private void checkClusterManageResource() {
+    securityService.authorize(new ResourcePermission(Resource.CLUSTER, Operation.MANAGE));
+  }
+
   @Override
   public ObjectInstance createMBean(String className, ObjectName name) throws ReflectionException,
       InstanceAlreadyExistsException, MBeanException, NotCompliantMBeanException {
     checkDomain(name);
+    checkClusterManageResource();
     return mbs.createMBean(className, name);
   }
 
@@ -90,6 +95,7 @@ public ObjectInstance createMBean(String className, ObjectName name, ObjectName
       throws ReflectionException, InstanceAlreadyExistsException, MBeanException,
       NotCompliantMBeanException, InstanceNotFoundException {
     checkDomain(name);
+    checkClusterManageResource();
     return mbs.createMBean(className, name, loaderName);
   }
 
@@ -98,6 +104,7 @@ public ObjectInstance createMBean(String className, ObjectName name, Object[] pa
       String[] signature) throws ReflectionException, InstanceAlreadyExistsException,
       MBeanException, NotCompliantMBeanException {
     checkDomain(name);
+    checkClusterManageResource();
     return mbs.createMBean(className, name, params, signature);
   }
 
@@ -107,6 +114,7 @@ public ObjectInstance createMBean(String className, ObjectName name, ObjectName
       throws ReflectionException, InstanceAlreadyExistsException, MBeanException,
       NotCompliantMBeanException, InstanceNotFoundException {
     checkDomain(name);
+    checkClusterManageResource();
     return mbs.createMBean(className, name, loaderName, params, signature);
   }
 
@@ -115,13 +123,15 @@ public ObjectInstance registerMBean(Object object, ObjectName name)
       throws InstanceAlreadyExistsException, MBeanRegistrationException,
       NotCompliantMBeanException {
     checkDomain(name);
+    checkClusterManageResource();
     return mbs.registerMBean(object, name);
   }
 
   @Override
   public void unregisterMBean(ObjectName name)
       throws InstanceNotFoundException, MBeanRegistrationException {
     checkDomain(name);
+    checkClusterManageResource();
     mbs.unregisterMBean(name);
   }
 
@@ -235,6 +245,10 @@ public Object invoke(ObjectName name, String operationName, Object[] params, Str
       throws InstanceNotFoundException, MBeanException, ReflectionException {
 
     ResourcePermission ctx = getOperationContext(name, operationName, true);
+    if (ctx == null
+        && !ManagementConstants.OBJECTNAME__DEFAULTDOMAIN.equals(name.getDomain())) {
+      ctx = new ResourcePermission(Resource.CLUSTER, Operation.MANAGE);
+    }
     securityService.authorize(ctx);
 
     return mbs.invoke(name, operationName, params, signature);