QPID-3858: Updated code to include recent refactoring by Gordon (gsim) - see QPID-4178.

git-svn-id: https://svn.apache.org/repos/asf/qpid/branches/asyncstore@1377715 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 249 insertions, 52 deletions

diff --git a/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java b/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
index 49f06d5121..a2a0d2d093 100644
--- a/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
+++ b/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
@@ -22,6 +22,7 @@ package org.apache.qpid.server.jmx;
 
 import org.apache.log4j.Logger;
 
+import org.apache.qpid.server.logging.LogActor;
 import org.apache.qpid.server.logging.actors.CurrentActor;
 import org.apache.qpid.server.logging.actors.ManagementActor;
 import org.apache.qpid.server.logging.messages.ManagementConsoleMessages;
@@ -38,6 +39,7 @@ import javax.management.MBeanServer;
 import javax.management.Notification;
 import javax.management.NotificationListener;
 import javax.management.ObjectName;
+import javax.management.RuntimeErrorException;
 import javax.management.remote.JMXConnectionNotification;
 import javax.management.remote.JMXPrincipal;
 import javax.management.remote.MBeanServerForwarder;
@@ -48,6 +50,7 @@ import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
 import java.security.AccessControlContext;
 import java.security.AccessController;
+import java.util.Arrays;
 import java.util.Map;
 import java.util.Set;
 
@@ -157,77 +160,98 @@ public class MBeanInvocationHandlerImpl implements InvocationHandler, Notificati
 
             // Save the subject
             SecurityManager.setThreadSubject(subject);
-
-            // Get the component, type and impact, which may be null
-            String type = getType(method, args);
-            String vhost = getVirtualHost(method, args);
-            int impact = getImpact(method, args);
-
-            // Get the security manager for the virtual host (if set)
-            SecurityManager security;
-            if (vhost == null)
+            CurrentActor.set(_logActor);
+            try
             {
-                security = _appRegistry.getSecurityManager();
+                return authoriseAndInvoke(method, args);
             }
-            else
+            finally
             {
-                security = _appRegistry.getVirtualHostRegistry().getVirtualHost(vhost).getSecurityManager();
+                CurrentActor.remove();
             }
+        }
+        catch (InvocationTargetException e)
+        {
+            Throwable targetException =  e.getCause();
+            logTargetException(method, args, targetException);
+            throw targetException;
+        }
+    }
 
-            methodName = getMethodName(method, args);
-            if (isAccessMethod(methodName) || impact == MBeanOperationInfo.INFO)
-            {
-                // Check for read-only method invocation permission
-                if (!security.authoriseMethod(Operation.ACCESS, type, methodName))
-                {
-                    throw new SecurityException("Permission denied: Access " + methodName);
-                }
-            }
-            else
-            {
-                // Check for setting properties permission
-                if (!security.authoriseMethod(Operation.UPDATE, type, methodName))
-                {
-                    throw new SecurityException("Permission denied: Update " + methodName);
-                }
-            }
+    private void logTargetException(Method method, Object[] args, Throwable targetException)
+    {
+        Throwable error = null;
+        if (targetException instanceof RuntimeErrorException)
+        {
+            error = ((RuntimeErrorException)targetException).getCause();
+        }
+        else if (targetException instanceof Error)
+        {
+            error = targetException;
+        }
+        if (error == null)
+        {
+            _logger.debug("Exception was thrown on invoking of " + method + " with arguments " + Arrays.toString(args), targetException);
+        }
+        else
+        {
+            _logger.error("Unexpected error occured on invoking of " + method + " with arguments " + Arrays.toString(args), targetException);
+        }
+    }
 
-            boolean oldAccessChecksDisabled = false;
-            if(_managementRightsInferAllAccess)
-            {
-                oldAccessChecksDisabled = SecurityManager.setAccessChecksDisabled(true);
-            }
+    private Object authoriseAndInvoke(Method method, Object[] args) throws IllegalAccessException, InvocationTargetException
+    {
+        String methodName;
+        // Get the component, type and impact, which may be null
+        String type = getType(method, args);
+        String vhost = getVirtualHost(method, args);
+        int impact = getImpact(method, args);
+
+        // Get the security manager for the virtual host (if set)
+        SecurityManager security;
+        if (vhost == null)
+        {
+            security = _appRegistry.getSecurityManager();
+        }
+        else
+        {
+            security = _appRegistry.getVirtualHostRegistry().getVirtualHost(vhost).getSecurityManager();
+        }
 
-            try
+        methodName = getMethodName(method, args);
+        if (isAccessMethod(methodName) || impact == MBeanOperationInfo.INFO)
+        {
+            // Check for read-only method invocation permission
+            if (!security.authoriseMethod(Operation.ACCESS, type, methodName))
             {
-                return doInvokeWrappingWithManagementActor(method, args);
+                throw new SecurityException("Permission denied: Access " + methodName);
             }
-            finally
+        }
+        else
+        {
+            // Check for setting properties permission
+            if (!security.authoriseMethod(Operation.UPDATE, type, methodName))
             {
-                if(_managementRightsInferAllAccess)
-                {
-                    SecurityManager.setAccessChecksDisabled(oldAccessChecksDisabled);
-                }
+                throw new SecurityException("Permission denied: Update " + methodName);
             }
         }
-        catch (InvocationTargetException e)
+
+        boolean oldAccessChecksDisabled = false;
+        if(_managementRightsInferAllAccess)
         {
-            throw e.getTargetException();
+            oldAccessChecksDisabled = SecurityManager.setAccessChecksDisabled(true);
         }
-    }
 
-    private Object doInvokeWrappingWithManagementActor(Method method,
-            Object[] args) throws IllegalAccessException,
-            InvocationTargetException
-    {
         try
         {
-            CurrentActor.set(_logActor);
             return method.invoke(_mbs, args);
         }
         finally
         {
-            CurrentActor.remove();
+            if(_managementRightsInferAllAccess)
+            {
+                SecurityManager.setAccessChecksDisabled(oldAccessChecksDisabled);
+            }
         }
     }
 
@@ -368,14 +392,17 @@ public class MBeanInvocationHandlerImpl implements InvocationHandler, Notificati
             user = splitConnectionId[1];
         }
 
+        // use a separate instance of actor as subject is not set on connect/disconnect
+        // we need to pass principal name explicitly into log actor
+        LogActor logActor = new ManagementActor(_appRegistry.getRootMessageLogger(), user);
         if (JMXConnectionNotification.OPENED.equals(type))
         {
-            _logActor.message(ManagementConsoleMessages.OPEN(user));
+            logActor.message(ManagementConsoleMessages.OPEN(user));
         }
         else if (JMXConnectionNotification.CLOSED.equals(type) ||
                  JMXConnectionNotification.FAILED.equals(type))
         {
-            _logActor.message(ManagementConsoleMessages.CLOSE(user));
+            logActor.message(ManagementConsoleMessages.CLOSE(user));
         }
     }
 }
diff --git a/java/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/ManagementLogActorTest.java b/java/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/ManagementLogActorTest.java
new file mode 100644
index 0000000000..c1df9afc5d
--- /dev/null
+++ b/java/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/ManagementLogActorTest.java
@@ -0,0 +1,170 @@
+/*
+ *
+ * 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.qpid.server.jmx;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.management.JMException;
+import javax.management.MBeanServerConnection;
+import javax.management.ObjectName;
+import javax.management.remote.JMXConnector;
+import javax.management.remote.JMXConnectorFactory;
+import javax.management.remote.JMXServiceURL;
+
+import org.apache.commons.configuration.ConfigurationException;
+import org.apache.commons.configuration.XMLConfiguration;
+import org.apache.qpid.server.configuration.ServerConfiguration;
+import org.apache.qpid.server.configuration.plugins.ConfigurationPlugin;
+import org.apache.qpid.server.logging.actors.CurrentActor;
+import org.apache.qpid.server.registry.ApplicationRegistry;
+import org.apache.qpid.server.security.Result;
+import org.apache.qpid.server.security.SecurityPlugin;
+import org.apache.qpid.server.security.access.ObjectProperties;
+import org.apache.qpid.server.security.access.ObjectType;
+import org.apache.qpid.server.security.access.Operation;
+import org.apache.qpid.server.util.TestApplicationRegistry;
+import org.apache.qpid.test.utils.QpidTestCase;
+
+public class ManagementLogActorTest extends QpidTestCase
+{
+    private ApplicationRegistry _registry;
+    private JMXManagedObjectRegistry _objectRegistry;
+    private int _registryPort;
+    private int _connectorPort;
+    private TestPlugin _plugin;
+
+    @Override
+    public void setUp() throws Exception
+    {
+        super.setUp();
+
+        _registryPort = findFreePort();
+        _connectorPort = getNextAvailable(_registryPort + 1);
+        XMLConfiguration config = new XMLConfiguration();
+        config.addProperty(ServerConfiguration.MGMT_JMXPORT_REGISTRYSERVER, _registryPort + "");
+        config.addProperty(ServerConfiguration.MGMT_JMXPORT_CONNECTORSERVER, _connectorPort + "");
+        _registry = new TestApplicationRegistry(new ServerConfiguration(config));
+        ApplicationRegistry.initialise(_registry);
+
+        _plugin = new TestPlugin();
+        _registry.getSecurityManager().addHostPlugin(_plugin);
+
+        _objectRegistry = new JMXManagedObjectRegistry();
+        new TestMBean(_objectRegistry);
+        _objectRegistry.start();
+    }
+
+    public void tearDown() throws Exception
+    {
+        _objectRegistry.close();
+        ApplicationRegistry.remove();
+        super.tearDown();
+    }
+
+    public void testPrincipalInLogMessage() throws Throwable
+    {
+        Map<String, Object> environment = new HashMap<String, Object>();
+        environment.put(JMXConnector.CREDENTIALS, new String[] { "admin", "admin" });
+        String urlString = "service:jmx:rmi:///jndi/rmi://localhost:" + _registryPort + "/jmxrmi";
+        JMXServiceURL url = new JMXServiceURL(urlString);
+        JMXConnector jmxConnector = JMXConnectorFactory.connect(url, environment);
+        MBeanServerConnection mbsc = jmxConnector.getMBeanServerConnection();
+        ObjectName mbeanObject = new ObjectName("org.apache.qpid:type=TestMBean,name=test");
+
+        String actorLogMessage = (String) mbsc.getAttribute(mbeanObject, "ActorLogMessage");
+
+        assertTrue("Unexpected log principal in security plugin", _plugin.getLogMessage().startsWith("[mng:admin"));
+        assertTrue("Unexpected log principal in MBean", actorLogMessage.startsWith("[mng:admin"));
+    }
+
+    public static class TestMBean extends DefaultManagedObject implements CurrentActorRetriever
+    {
+
+        public TestMBean(ManagedObjectRegistry registry) throws JMException
+        {
+            super(CurrentActorRetriever.class, "TestMBean", registry);
+            register();
+        }
+
+        @Override
+        public String getObjectInstanceName()
+        {
+            return "test";
+        }
+
+        @Override
+        public ManagedObject getParentObject()
+        {
+            return null;
+        }
+
+        @Override
+        public String getActorLogMessage()
+        {
+            return CurrentActor.get().getLogMessage();
+        }
+
+    }
+
+    public static interface CurrentActorRetriever
+    {
+        String getActorLogMessage();
+    }
+
+    public static class TestPlugin implements SecurityPlugin
+    {
+        private String _logMessage;
+
+        @Override
+        public void configure(ConfigurationPlugin config) throws ConfigurationException
+        {
+        }
+
+        @Override
+        public Result getDefault()
+        {
+            return Result.ALLOWED;
+        }
+
+        @Override
+        public Result access(ObjectType objectType, Object instance)
+        {
+            return Result.ALLOWED;
+        }
+
+        @Override
+        public Result authorise(Operation operation, ObjectType objectType, ObjectProperties properties)
+        {
+            // set thread name to work around logic in MangementActor
+            Thread.currentThread().setName("RMI TCP Connection(1)-" + System.currentTimeMillis());
+            _logMessage = CurrentActor.get().getLogMessage();
+            return Result.ALLOWED;
+        }
+
+        public String getLogMessage()
+        {
+            return _logMessage;
+        }
+
+    }
+
+}