ARTEMIS-5894 Fix RBAC address mismatch between canInvoke and invoke

artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/ArtemisMBeanServerGuard.java

@@ -142,6 +142,13 @@ public boolean canInvoke(String object, String operationName) {
       if (operationName == null || canBypassRBAC(objectName)) {
          return true;
       }
+
+      // Strip the parameter list from operationName.
+      int paramListIndex = operationName.indexOf('(');
+      if (paramListIndex > 0) {
+         operationName = operationName.substring(0, paramListIndex);
+      }
+
       List<String> requiredRoles = getRequiredRoles(objectName, operationName);
       for (String role : requiredRoles) {
          if (currentUserHasRole(role)) {

artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/ArtemisRbacInvocationHandler.java

@@ -123,8 +123,23 @@ public boolean canInvoke(String name, String operationName) {
       try {
          final ObjectName objectName = ObjectName.getInstance(name);
          if (!isUncheckedDomain(objectName)) {
-            final SimpleString rbacAddress = addressFrom(objectName, operationName);
-            securityStoreCheck(rbacAddress, permissionFrom(operationName));
+            if (operationName != null) {
+               // Strip the parameter list from operationName.
+               int paramListIndex = operationName.indexOf('(');
+               if (paramListIndex > 0) {
+                  operationName = operationName.substring(0, paramListIndex);
+               }
+
+               final SimpleString rbacAddress = addressFrom(objectName, operationName);
+               securityStoreCheck(rbacAddress, permissionFrom(operationName));
+            } else {
+               // When operationName is null, canInvoke checks if the user has any access to the MBean.
+               // We use VIEW permission rather than EDIT because hawtio-react's hasInvokeRights method
+               // checks both canInvoke(mbean, null) AND canInvoke(mbean, method). This allows users with
+               // VIEW permission to see MBeans and then determine which specific operations they can invoke.
+               final SimpleString rbacAddress = addressFrom(objectName, null);
+               securityStoreCheck(rbacAddress, CheckType.VIEW);
+            }
          }
          okInvoke = true;
       } catch (Throwable expectedOnCheckFailOrInvalidObjectName) {

artemis-server/src/test/java/org/apache/activemq/artemis/core/server/management/ArtemisMBeanServerGuardTest.java

@@ -100,4 +100,49 @@ public void testCanInvokeMethodDoeNotHasRole() throws Throwable {
       });
       assertFalse((Boolean) result);
    }
+
+   @Test
+   public void testCanInvokeStripsParameterList() throws Throwable {
+      ArtemisMBeanServerGuard guard = new ArtemisMBeanServerGuard();
+      JMXAccessControlList controlList = new JMXAccessControlList();
+      guard.setJMXAccessControlList(controlList);
+      ObjectNameBuilder objectNameBuilder = ObjectNameBuilder.create("testdomain", "myBroker");
+      ObjectName activeMQServerObjectName = objectNameBuilder.getActiveMQServerObjectName();
+
+      // Configure permission for operation without parameter list
+      controlList.addToRoleAccess("testdomain", "broker=myBroker", "deleteAddress", "admin");
+
+      Subject subject = new Subject();
+      subject.getPrincipals().add(new RolePrincipal("admin"));
+
+      // Test with operation name including parameter list
+      Object result = SecurityManagerShim.callAs(subject, (Callable<Object>) () -> {
+         try {
+            return guard.canInvoke(activeMQServerObjectName.getCanonicalName(), "deleteAddress(java.lang.String)");
+         } catch (Exception e1) {
+            return e1;
+         }
+      });
+      assertTrue((Boolean) result, "Should be able to invoke deleteAddress(java.lang.String) - parameter list should be stripped");
+
+      // Test with empty parameter list
+      result = SecurityManagerShim.callAs(subject, (Callable<Object>) () -> {
+         try {
+            return guard.canInvoke(activeMQServerObjectName.getCanonicalName(), "deleteAddress()");
+         } catch (Exception e1) {
+            return e1;
+         }
+      });
+      assertTrue((Boolean) result, "Should be able to invoke deleteAddress() - parameter list should be stripped");
+
+      // Test without parameter list (should also work)
+      result = SecurityManagerShim.callAs(subject, (Callable<Object>) () -> {
+         try {
+            return guard.canInvoke(activeMQServerObjectName.getCanonicalName(), "deleteAddress");
+         } catch (Exception e1) {
+            return e1;
+         }
+      });
+      assertTrue((Boolean) result, "Should be able to invoke deleteAddress without parameters");
+   }
 }

artemis-server/src/test/java/org/apache/activemq/artemis/core/server/management/ArtemisRbacMBeanServerBuilderTest.java

@@ -18,7 +18,6 @@
 package org.apache.activemq.artemis.core.server.management;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -666,7 +665,7 @@ public void testCanInvoke() throws Exception {
          }
       });
       assertNotNull(result);
-      assertFalse((Boolean) result, "in the absence of an operation to check, update required");
+      assertTrue((Boolean) result, "in the absence of an operation to check, view is sufficient");
 
       result = SecurityManagerShim.callAs(viewSubject, (Callable<Object>) () -> {
          try {
@@ -707,4 +706,92 @@ public void testCanInvoke() throws Exception {
       assertEquals("getVmName()", cd.get("Method"));
       assertEquals(true, cd.get("CanInvoke"));
    }
+
+   @Test
+   public void testCanInvokeStripsParameterList() throws Exception {
+
+      MBeanServer proxy = underTest.newMBeanServer("d", mbeanServer, mBeanServerDelegate);
+
+      final ActiveMQServer server = createServer(false);
+      server.setMBeanServer(proxy);
+      server.getConfiguration().setJMXManagementEnabled(true).setSecurityEnabled(true);
+
+      Set<Role> editRoles = new HashSet<>();
+      editRoles.add(new Role("editors", false, false, false, false, false, false, false, false, false, false, false, true));
+
+      // Grant edit permission to deleteAddress operation WITHOUT parameter list
+      // This simulates the typical permission setup where operations are configured without parameter signatures
+      server.getConfiguration().putSecurityRoles("mops.broker.deleteAddress", editRoles);
+
+      server.start();
+
+      final HawtioSecurityControl securityControl = JMX.newMBeanProxy(
+         proxy, ObjectNameBuilder.DEFAULT.getSecurityObjectName(), HawtioSecurityControl.class, false);
+
+      ObjectName serverObjectName = ObjectNameBuilder.DEFAULT.getActiveMQServerObjectName();
+      final ActiveMQServerControl serverControl = JMX.newMBeanProxy(
+         proxy, serverObjectName, ActiveMQServerControl.class, false);
+      assertNotNull(serverControl);
+
+      Subject editSubject = new Subject();
+      editSubject.getPrincipals().add(new UserPrincipal("e"));
+      editSubject.getPrincipals().add(new RolePrincipal("editors"));
+
+      // Test with operation name including parameter list as sent by console
+      Object result = SecurityManagerShim.callAs(editSubject, (Callable<Object>) () -> {
+         try {
+            return securityControl.canInvoke(serverObjectName.toString(), "deleteAddress(java.lang.String)");
+         } catch (Exception e1) {
+            return e1.getCause();
+         }
+      });
+      assertNotNull(result);
+      assertTrue((Boolean) result, "Should be able to invoke deleteAddress(java.lang.String) - parameter list should be stripped to match permission");
+
+      // Test with empty parameter list
+      result = SecurityManagerShim.callAs(editSubject, (Callable<Object>) () -> {
+         try {
+            return securityControl.canInvoke(serverObjectName.toString(), "deleteAddress()");
+         } catch (Exception e1) {
+            return e1.getCause();
+         }
+      });
+      assertNotNull(result);
+      assertTrue((Boolean) result, "Should be able to invoke deleteAddress() - parameter list should be stripped");
+
+      // Test without parameter list (should also work)
+      result = SecurityManagerShim.callAs(editSubject, (Callable<Object>) () -> {
+         try {
+            return securityControl.canInvoke(serverObjectName.toString(), "deleteAddress");
+         } catch (Exception e1) {
+            return e1.getCause();
+         }
+      });
+      assertNotNull(result);
+      assertTrue((Boolean) result, "Should be able to invoke deleteAddress without parameters");
+
+      // Test bulk query with parameter lists
+      Map<String, List<String>> bulkQuery = new HashMap<>();
+      bulkQuery.put(serverObjectName.toString(), List.of("deleteAddress(java.lang.String)", "deleteAddress()"));
+
+      result = SecurityManagerShim.callAs(editSubject, (Callable<Object>) () -> {
+         try {
+            return securityControl.canInvoke(bulkQuery);
+         } catch (Exception e1) {
+            return e1.getCause();
+         }
+      });
+      assertNotNull(result);
+      assertEquals(2, ((TabularData)result).size());
+
+      CompositeData cd = ((TabularData)result).get(new Object[]{serverObjectName.toString(), "deleteAddress(java.lang.String)"});
+      assertEquals(serverObjectName.toString(), cd.get("ObjectName"));
+      assertEquals("deleteAddress(java.lang.String)", cd.get("Method"));
+      assertEquals(true, cd.get("CanInvoke"));
+
+      cd = ((TabularData)result).get(new Object[]{serverObjectName.toString(), "deleteAddress()"});
+      assertEquals(serverObjectName.toString(), cd.get("ObjectName"));
+      assertEquals("deleteAddress()", cd.get("Method"));
+      assertEquals(true, cd.get("CanInvoke"));
+   }
 }

tests/smoke-tests/pom.xml

@@ -302,29 +302,6 @@
             <groupId>org.apache.artemis</groupId>
             <artifactId>artemis-maven-plugin</artifactId>
             <executions>
-               <execution>
-                  <phase>test-compile</phase>
-                  <id>create-create-console</id>
-                  <goals>
-                     <goal>create</goal>
-                  </goals>
-                  <configuration>
-                     <role>amq,connections,sessions,consumers,producers,addresses,queues</role>
-                     <user>admin</user>
-                     <password>admin</password>
-                     <allowAnonymous>false</allowAnonymous>
-                     <noWeb>false</noWeb>
-                     <instance>${basedir}/target/console</instance>
-                     <configuration>${basedir}/target/classes/servers/console</configuration>
-                     <args>
-                        <arg>--http-host</arg>
-                        <arg>${sts-http-host}</arg>
-                        <arg>--http-port</arg>
-                        <arg>8161</arg>
-                     </args>
-                  </configuration>
-               </execution>
-
                <execution>
                   <phase>test-compile</phase>
                   <id>upgrade-linux</id>

tests/smoke-tests/src/main/resources/servers/console-broker-security/artemis-roles.properties

@@ -0,0 +1,25 @@
+#
+# 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.
+#
+
+amq=admin
+connections=connections
+sessions=sessions
+consumers=consumers
+producers=producers
+addresses=addresses,deleteAddresses
+deleteAddresses=deleteAddresses
+queues=queues

tests/smoke-tests/src/main/resources/servers/console-broker-security/artemis-users.properties

@@ -0,0 +1,25 @@
+#
+# 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.
+#
+
+admin=admin
+connections=connections
+sessions=sessions
+consumers=consumers
+producers=producers
+addresses=addresses
+deleteAddresses=deleteAddresses
+queues=queues