From c0b4a1052690e17da6fb054a54f14c42c39e31ea Mon Sep 17 00:00:00 2001 From: Tadayoshi Sato Date: Thu, 3 Dec 2020 19:27:39 +0900 Subject: [PATCH] ARTEMIS-3017 ArtemisJMXSecurity bulk canInvoke operation always returns true --- .../impl/HawtioSecurityControlImpl.java | 36 +++- .../impl/HawtioSecurityControlImplTest.java | 181 ++++++++++++++++++ 2 files changed, 214 insertions(+), 3 deletions(-) create mode 100644 artemis-server/src/test/java/org/apache/activemq/artemis/core/server/management/impl/HawtioSecurityControlImplTest.java diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/impl/HawtioSecurityControlImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/impl/HawtioSecurityControlImpl.java index c40e3af938..ff8447c3ef 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/impl/HawtioSecurityControlImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/impl/HawtioSecurityControlImpl.java @@ -19,6 +19,7 @@ package org.apache.activemq.artemis.core.server.management.impl; import org.apache.activemq.artemis.core.management.impl.AbstractControl; import org.apache.activemq.artemis.core.management.impl.MBeanInfoHelper; import org.apache.activemq.artemis.core.persistence.StorageManager; +import org.apache.activemq.artemis.core.server.ActiveMQServerLogger; import org.apache.activemq.artemis.core.server.management.ArtemisMBeanServerGuard; import org.apache.activemq.artemis.core.server.management.HawtioSecurityControl; @@ -28,12 +29,15 @@ import javax.management.NotCompliantMBeanException; import javax.management.openmbean.CompositeData; import javax.management.openmbean.CompositeDataSupport; import javax.management.openmbean.CompositeType; +import javax.management.openmbean.KeyAlreadyExistsException; import javax.management.openmbean.OpenDataException; import javax.management.openmbean.OpenType; import javax.management.openmbean.SimpleType; import javax.management.openmbean.TabularData; import javax.management.openmbean.TabularDataSupport; import javax.management.openmbean.TabularType; +import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; @@ -104,14 +108,27 @@ public class HawtioSecurityControlImpl extends AbstractControl implements Hawtio boolean res = canInvoke(objectName); CompositeData data = new CompositeDataSupport(CAN_INVOKE_RESULT_ROW_TYPE, CAN_INVOKE_RESULT_COLUMNS, - new Object[]{objectName, "", true}); + new Object[]{objectName, "", res}); table.put(data); } else { for (String method : methods) { + List argTypes = new ArrayList<>(); + String name = parseMethodName(method, argTypes); + + boolean res; + if (name.equals(method)) { + res = canInvoke(objectName, name); + } else { + res = canInvoke(objectName, name, argTypes.toArray(new String[]{})); + } CompositeData data = new CompositeDataSupport(CAN_INVOKE_RESULT_ROW_TYPE, CAN_INVOKE_RESULT_COLUMNS, - new Object[]{objectName, method, true}); - table.put(data); + new Object[]{objectName, method, res}); + try { + table.put(data); + } catch (KeyAlreadyExistsException e) { + ActiveMQServerLogger.LOGGER.debugv("Key already exists: {0} (objectName = \"{1}\", method = \"{2}\")", e.getMessage(), objectName, method); + } } } } @@ -119,6 +136,19 @@ public class HawtioSecurityControlImpl extends AbstractControl implements Hawtio return table; } + private String parseMethodName(String method, List argTypes) { + method = method.trim(); + int index = method.indexOf('('); + if (index < 0) { + return method; + } + + String args = method.substring(index + 1, method.length() - 1); + argTypes.addAll(Arrays.asList(args.split(","))); + + return method.substring(0, index); + } + // A member class is used to initialize final fields, as this needs to do some exception handling... static class SecurityMBeanOpenTypeInitializer { private static final String[] COLUMNS = new String[]{"ObjectName", "Method", "CanInvoke"}; diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/management/impl/HawtioSecurityControlImplTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/management/impl/HawtioSecurityControlImplTest.java new file mode 100644 index 0000000000..feff848453 --- /dev/null +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/management/impl/HawtioSecurityControlImplTest.java @@ -0,0 +1,181 @@ +/* + * 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.activemq.artemis.core.server.management.impl; + +import javax.management.openmbean.CompositeData; +import javax.management.openmbean.TabularData; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.apache.activemq.artemis.core.persistence.StorageManager; +import org.apache.activemq.artemis.core.server.management.ArtemisMBeanServerGuard; +import org.junit.Test; +import org.mockito.Mockito; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class HawtioSecurityControlImplTest { + + @Test + public void testCanInvokeMBean() throws Exception { + String objectName = "foo.bar.testing:type=SomeMBean"; + ArtemisMBeanServerGuard guard = Mockito.mock(ArtemisMBeanServerGuard.class); + StorageManager storageManager = Mockito.mock(StorageManager.class); + Mockito.when(guard.canInvoke(objectName, null)).thenReturn(true); + + HawtioSecurityControlImpl control = new HawtioSecurityControlImpl(guard, storageManager); + assertTrue(control.canInvoke(objectName)); + } + + @Test + public void testCanInvokeMBean2() throws Exception { + String objectName = "foo.bar.testing:type=SomeMBean"; + ArtemisMBeanServerGuard guard = Mockito.mock(ArtemisMBeanServerGuard.class); + StorageManager storageManager = Mockito.mock(StorageManager.class); + Mockito.when(guard.canInvoke(objectName, null)).thenReturn(false); + + HawtioSecurityControlImpl control = new HawtioSecurityControlImpl(guard, storageManager); + assertFalse(control.canInvoke(objectName)); + } + + @Test(expected = Exception.class) + public void testCanInvokeMBeanThrowsException() throws Exception { + String objectName = "foo.bar.testing:type=SomeMBean"; + ArtemisMBeanServerGuard guard = Mockito.mock(ArtemisMBeanServerGuard.class); + StorageManager storageManager = Mockito.mock(StorageManager.class); + Mockito.when(guard.canInvoke(objectName, null)).thenThrow(new Exception()); + + HawtioSecurityControlImpl control = new HawtioSecurityControlImpl(guard, storageManager); + control.canInvoke(objectName); + fail("Should have thrown an exception"); + } + + @Test + public void testCanInvokeMBeanNoGuard() throws Exception { + HawtioSecurityControlImpl control = new HawtioSecurityControlImpl(null, null); + assertTrue(control.canInvoke("foo.bar.testing:type=SomeMBean")); + } + + @Test + public void testCanInvokeMethod() throws Exception { + String objectName = "foo.bar.testing:type=SomeMBean"; + ArtemisMBeanServerGuard guard = Mockito.mock(ArtemisMBeanServerGuard.class); + StorageManager storageManager = Mockito.mock(StorageManager.class); + Mockito.when(guard.canInvoke(objectName, "testMethod")).thenReturn(true); + Mockito.when(guard.canInvoke(objectName, "otherMethod")).thenReturn(false); + + HawtioSecurityControlImpl control = new HawtioSecurityControlImpl(guard, storageManager); + assertTrue(control.canInvoke(objectName, "testMethod", new String[]{"long"})); + assertTrue(control.canInvoke(objectName, "testMethod", new String[]{"java.lang.String"})); + assertFalse(control.canInvoke(objectName, "otherMethod", new String[]{"java.lang.String", "java.lang.String"})); + } + + @Test(expected = Exception.class) + public void testCanInvokeMethodException() throws Exception { + String objectName = "foo.bar.testing:type=SomeMBean"; + ArtemisMBeanServerGuard guard = Mockito.mock(ArtemisMBeanServerGuard.class); + StorageManager storageManager = Mockito.mock(StorageManager.class); + Mockito.when(guard.canInvoke(objectName, "testMethod")).thenThrow(new Exception()); + + HawtioSecurityControlImpl control = new HawtioSecurityControlImpl(guard, storageManager); + control.canInvoke(objectName, "testMethod", new String[]{}); + fail("Should have thrown an exception"); + } + + @Test + public void testCanInvokeMethodNoGuard() throws Exception { + HawtioSecurityControlImpl control = new HawtioSecurityControlImpl(null, null); + assertTrue(control.canInvoke("foo.bar.testing:type=SomeMBean", "someMethod", new String[]{})); + } + + @Test + public void testCanInvokeBulk() throws Exception { + ArtemisMBeanServerGuard guard = Mockito.mock(ArtemisMBeanServerGuard.class); + StorageManager storageManager = Mockito.mock(StorageManager.class); + String objectName = "foo.bar.testing:type=SomeMBean"; + Mockito.when(guard.canInvoke(objectName, "testMethod")).thenReturn(true); + Mockito.when(guard.canInvoke(objectName, "testMethod2")).thenReturn(false); + Mockito.when(guard.canInvoke(objectName, "otherMethod")).thenReturn(true); + String objectName2 = "foo.bar.testing:type=SomeOtherMBean"; + Mockito.when(guard.canInvoke(objectName2, null)).thenReturn(true); + String objectName3 = "foo.bar.foo.testing:type=SomeOtherMBean"; + Mockito.when(guard.canInvoke(objectName3, null)).thenReturn(false); + + HawtioSecurityControlImpl control = new HawtioSecurityControlImpl(guard, storageManager); + Map> query = new HashMap<>(); + query.put(objectName, Arrays.asList("otherMethod", "testMethod(long)", "testMethod2(java.lang.String)")); + query.put(objectName2, Collections.emptyList()); + query.put(objectName3, Collections.emptyList()); + TabularData result = control.canInvoke(query); + assertEquals(5, result.size()); + + CompositeData cd = result.get(new Object[]{objectName, "testMethod(long)"}); + assertEquals(objectName, cd.get("ObjectName")); + assertEquals("testMethod(long)", cd.get("Method")); + assertEquals(true, cd.get("CanInvoke")); + CompositeData cd2 = result.get(new Object[]{objectName, "testMethod2(java.lang.String)"}); + assertEquals(objectName, cd2.get("ObjectName")); + assertEquals("testMethod2(java.lang.String)", cd2.get("Method")); + assertEquals(false, cd2.get("CanInvoke")); + CompositeData cd3 = result.get(new Object[]{objectName, "otherMethod"}); + assertEquals(objectName, cd3.get("ObjectName")); + assertEquals("otherMethod", cd3.get("Method")); + assertEquals(true, cd3.get("CanInvoke")); + CompositeData cd4 = result.get(new Object[]{objectName2, ""}); + assertEquals(objectName2, cd4.get("ObjectName")); + assertEquals("", cd4.get("Method")); + assertEquals(true, cd4.get("CanInvoke")); + CompositeData cd5 = result.get(new Object[]{objectName3, ""}); + assertEquals(objectName3, cd5.get("ObjectName")); + assertEquals("", cd5.get("Method")); + assertEquals(false, cd5.get("CanInvoke")); + } + + @Test + public void testCanInvokeBulkWithDuplicateMethods() throws Exception { + ArtemisMBeanServerGuard guard = Mockito.mock(ArtemisMBeanServerGuard.class); + StorageManager storageManager = Mockito.mock(StorageManager.class); + String objectName = "foo.bar.testing:type=SomeMBean"; + Mockito.when(guard.canInvoke(objectName, "duplicateMethod1")).thenReturn(true); + Mockito.when(guard.canInvoke(objectName, "duplicateMethod2")).thenReturn(false); + + HawtioSecurityControlImpl control = new HawtioSecurityControlImpl(guard, storageManager); + Map> query = new HashMap<>(); + query.put(objectName, Arrays.asList("duplicateMethod1(long)", "duplicateMethod1(java.lang.String)", "duplicateMethod1(long)", "duplicateMethod2", "duplicateMethod2")); + TabularData result = control.canInvoke(query); + assertEquals(3, result.size()); + + CompositeData cd = result.get(new Object[]{objectName, "duplicateMethod1(long)"}); + assertEquals(objectName, cd.get("ObjectName")); + assertEquals("duplicateMethod1(long)", cd.get("Method")); + assertEquals(true, cd.get("CanInvoke")); + CompositeData cd2 = result.get(new Object[]{objectName, "duplicateMethod1(java.lang.String)"}); + assertEquals(objectName, cd2.get("ObjectName")); + assertEquals("duplicateMethod1(java.lang.String)", cd2.get("Method")); + assertEquals(true, cd2.get("CanInvoke")); + CompositeData cd3 = result.get(new Object[]{objectName, "duplicateMethod2"}); + assertEquals(objectName, cd3.get("ObjectName")); + assertEquals("duplicateMethod2", cd3.get("Method")); + assertEquals(false, cd3.get("CanInvoke")); + } +}