QPID-3474: Use JMX notification handback data to ensure that open/close/fail events are logged with username only, rather than a complete list of pricipals.

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

2 files changed, 73 insertions, 13 deletions

diff --git a/java/broker/src/main/java/org/apache/qpid/server/management/JMXManagedObjectRegistry.java b/java/broker/src/main/java/org/apache/qpid/server/management/JMXManagedObjectRegistry.java
index b44964f176..7210b404dd 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/management/JMXManagedObjectRegistry.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/management/JMXManagedObjectRegistry.java
@@ -38,6 +38,8 @@ import java.rmi.server.RMIClientSocketFactory;
 import java.rmi.server.RMIServerSocketFactory;
 import java.rmi.server.UnicastRemoteObject;
 import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 import javax.management.JMException;
 import javax.management.MBeanServer;
@@ -49,11 +51,13 @@ import javax.management.remote.JMXConnectionNotification;
 import javax.management.remote.JMXConnectorServer;
 import javax.management.remote.JMXServiceURL;
 import javax.management.remote.MBeanServerForwarder;
+import javax.management.remote.rmi.RMIConnection;
 import javax.management.remote.rmi.RMIConnectorServer;
 import javax.management.remote.rmi.RMIJRMPServerImpl;
 import javax.management.remote.rmi.RMIServerImpl;
 import javax.rmi.ssl.SslRMIClientSocketFactory;
 import javax.rmi.ssl.SslRMIServerSocketFactory;
+import javax.security.auth.Subject;
 
 import org.apache.commons.configuration.ConfigurationException;
 import org.apache.log4j.Logger;
@@ -63,6 +67,7 @@ import org.apache.qpid.server.logging.messages.ManagementConsoleMessages;
 import org.apache.qpid.server.registry.ApplicationRegistry;
 import org.apache.qpid.server.registry.IApplicationRegistry;
 import org.apache.qpid.server.security.auth.rmi.RMIPasswordAuthenticator;
+import org.apache.qpid.server.security.auth.sasl.UsernamePrincipal;
 
 /**
  * This class starts up an MBeanserver. If out of the box agent has been enabled then there are no 
@@ -223,7 +228,28 @@ public class JMXManagedObjectRegistry implements ManagedObjectRegistry
          * The registry is exported on the defined management port 'port'. We will export the RMIConnectorServer
          * on 'port +1'. Use of these two well-defined ports will ease any navigation through firewall's. 
          */
-        final RMIServerImpl rmiConnectorServerStub = new RMIJRMPServerImpl(_jmxPortConnectorServer, csf, ssf, env);
+        final Map<String, String> connectionIdUsernameMap = new ConcurrentHashMap<String, String>();
+        final RMIServerImpl rmiConnectorServerStub = new RMIJRMPServerImpl(_jmxPortConnectorServer, csf, ssf, env)
+        {
+
+            /**
+             * Override makeClient so we can cache the username of the client in a Map keyed by connectionId.
+             * ConnectionId is guaranteed to be unique per client connection, according to the JMS spec.
+             *
+             * MBeanInvocationHandlerImpl#handleNotification is responsible for removing the map entry on receipt
+             * of CLOSE or FAIL notifications.
+             *
+             * @see javax.management.remote.rmi.RMIJRMPServerImpl#makeClient(java.lang.String, javax.security.auth.Subject)
+             */
+            @Override
+            protected RMIConnection makeClient(String connectionId, Subject subject) throws IOException
+            {
+                final RMIConnection makeClient = super.makeClient(connectionId, subject);
+                final UsernamePrincipal usernamePrincipalFromSubject = UsernamePrincipal.getUsernamePrincipalFromSubject(subject);
+                connectionIdUsernameMap.put(connectionId, usernamePrincipalFromSubject.getName());
+                return makeClient;
+            }
+        };
         String localHost;
         try
         {
@@ -295,13 +321,18 @@ public class JMXManagedObjectRegistry implements ManagedObjectRegistry
         MBeanServerForwarder mbsf = MBeanInvocationHandlerImpl.newProxyInstance();
         _cs.setMBeanServerForwarder(mbsf);
 
+
+        // Get the handler that is used by the above MBInvocationHandler Proxy.
+        // which is the MBeanInvocationHandlerImpl and so also a NotificationListener.
+        final NotificationListener invocationHandler = (NotificationListener) Proxy.getInvocationHandler(mbsf);
+
+        // Install a notification listener on OPENED, CLOSED, and FAIL,
+        // passing the map of connection-ids to usernames as hand-back data.
         NotificationFilterSupport filter = new NotificationFilterSupport();
         filter.enableType(JMXConnectionNotification.OPENED);
         filter.enableType(JMXConnectionNotification.CLOSED);
         filter.enableType(JMXConnectionNotification.FAILED);
-        // Get the handler that is used by the above MBInvocationHandler Proxy.
-        // which is the MBeanInvocationHandlerImpl and so also a NotificationListener
-        _cs.addNotificationListener((NotificationListener) Proxy.getInvocationHandler(mbsf), filter, null);
+        _cs.addNotificationListener(invocationHandler, filter, connectionIdUsernameMap);
 
         _cs.start();
 
diff --git a/java/broker/src/main/java/org/apache/qpid/server/management/MBeanInvocationHandlerImpl.java b/java/broker/src/main/java/org/apache/qpid/server/management/MBeanInvocationHandlerImpl.java
index 0dd8c95ef3..562b6404c3 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/management/MBeanInvocationHandlerImpl.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/management/MBeanInvocationHandlerImpl.java
@@ -26,6 +26,7 @@ import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
 import java.security.AccessControlContext;
 import java.security.AccessController;
+import java.util.Map;
 import java.util.Set;
 
 import javax.management.Attribute;
@@ -33,7 +34,6 @@ import javax.management.JMException;
 import javax.management.MBeanInfo;
 import javax.management.MBeanOperationInfo;
 import javax.management.MBeanServer;
-import javax.management.MalformedObjectNameException;
 import javax.management.Notification;
 import javax.management.NotificationListener;
 import javax.management.ObjectName;
@@ -320,23 +320,52 @@ public class MBeanInvocationHandlerImpl implements InvocationHandler, Notificati
         return (methodName.startsWith("query") || methodName.startsWith("get") || methodName.startsWith("is"));
     }
 
-    public void handleNotification(Notification notification, Object handback)
+    /**
+     * Receives notifications from the MBeanServer.
+     */
+    public void handleNotification(final Notification notification, final Object handback)
     {
         assert notification instanceof JMXConnectionNotification;
 
-        // only RMI Connections are serviced here, Local API atta
-        // rmi://169.24.29.116 guest 3
-        String[] connectionData = ((JMXConnectionNotification) notification).getConnectionId().split(" ");
-        String user = connectionData[1];
+        final String connectionId = ((JMXConnectionNotification) notification).getConnectionId();
+        final String type = notification.getType();
 
-        if (notification.getType().equals(JMXConnectionNotification.OPENED))
+        if (_logger.isDebugEnabled())
+        {
+            _logger.debug("Notification connectionId : " + connectionId + " type : " + type
+                    + " Notification handback : " + handback);
+        }
+
+        // Normally JMXManagedObjectRegistry provides a Map as handback data containing a map
+        // between connection id and username.
+        Map<String, String> connectionIdUsernameMap = null;
+        String user = null;
+        if (handback != null && handback instanceof Map)
+        {
+            connectionIdUsernameMap = (Map<String, String>) handback;
+            user = connectionIdUsernameMap.get(connectionId);
+        }
+
+        // If user is still null, fallback to an unordered list of Principals from the connection id.
+        if (user == null)
+        {
+            final String[] splitConnectionId = connectionId.split(" ");
+            user = splitConnectionId[1];
+        }
+
+        if (JMXConnectionNotification.OPENED.equals(type))
         {
             _logActor.message(ManagementConsoleMessages.OPEN(user));
         }
-        else if (notification.getType().equals(JMXConnectionNotification.CLOSED) ||
-                 notification.getType().equals(JMXConnectionNotification.FAILED))
+        else if (JMXConnectionNotification.CLOSED.equals(type) ||
+                 JMXConnectionNotification.FAILED.equals(type))
         {
             _logActor.message(ManagementConsoleMessages.CLOSE(user));
+            // We are responsible for removing the entry from the map
+            if (connectionIdUsernameMap != null)
+            {
+                connectionIdUsernameMap.remove(connectionId);
+            }
         }
     }
 }