QPID-5611 : Ensure the appropriate principals are available at the time of all event logging

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

5 files changed, 46 insertions, 34 deletions

diff --git a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
index b20685985f..7b0a48cac1 100644
--- a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
+++ b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
@@ -222,11 +222,7 @@ public class MBeanInvocationHandlerImpl implements InvocationHandler
         {
             try
             {
-                Subject subject = Subject.getSubject(AccessController.getContext());
-                subject = new Subject(false, subject.getPrincipals(), subject.getPublicCredentials(), subject.getPrivateCredentials());
-                subject.getPrincipals().addAll(SecurityManager.SYSTEM.getPrincipals());
-
-                return Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
+                return Subject.doAs(SecurityManager.getSubjectWithAddedSystemRights(), new PrivilegedExceptionAction<Object>()
                 {
                     @Override
                     public Object run() throws IllegalAccessException, InvocationTargetException
diff --git a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/ManagementLogonLogoffReporter.java b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/ManagementLogonLogoffReporter.java
index f99fe89f7b..c1792f0227 100644
--- a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/ManagementLogonLogoffReporter.java
+++ b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/ManagementLogonLogoffReporter.java
@@ -38,7 +38,6 @@ import org.apache.qpid.server.security.auth.jmx.JMXConnectionPrincipal;
 import java.rmi.server.RemoteServer;
 import java.rmi.server.ServerNotActiveException;
 import java.security.PrivilegedAction;
-import java.util.Collections;
 
 public class ManagementLogonLogoffReporter implements  NotificationListener, NotificationFilter
 {
@@ -63,31 +62,45 @@ public class ManagementLogonLogoffReporter implements  NotificationListener, Not
             LOGGER.debug("Notification connectionId : " + connectionId + " type : " + type);
         }
 
-        String user = _usernameAccessor.getUsernameForConnectionId(connectionId);
+        Subject subject = _usernameAccessor.getSubjectConnectionId(connectionId);
+        if(subject == null)
+        {
+            subject = new Subject();
+        }
+        AuthenticatedPrincipal authenticatedPrincipal =
+                AuthenticatedPrincipal.getOptionalAuthenticatedPrincipalFromSubject(subject);
 
-        // If user is still null, fallback to an unordered list of Principals from the connection id.
-        if (user == null)
+        String user;
+
+        if(authenticatedPrincipal != null)
         {
+            user = authenticatedPrincipal.getName();
+        }
+        else
+        {
+            // If user is still null, fallback to an unordered list of Principals from the connection id.
             final String[] splitConnectionId = connectionId.split(" ");
             user = splitConnectionId[1];
         }
-        Subject originalSubject = new Subject(false, Collections.singleton(new AuthenticatedPrincipal(user)), Collections.emptySet(), Collections.emptySet());
-        Subject subject;
 
-        try
-        {
-            String clientHost = RemoteServer.getClientHost();
-            subject = new Subject(false,
-                                  originalSubject.getPrincipals(),
-                                  originalSubject.getPublicCredentials(),
-                                  originalSubject.getPrivateCredentials());
-            subject.getPrincipals().add(new JMXConnectionPrincipal(clientHost));
-            subject.setReadOnly();
-        }
-        catch(ServerNotActiveException e)
+
+        if(subject.getPrincipals(JMXConnectionPrincipal.class).isEmpty())
         {
-            subject = originalSubject;
+            try
+            {
+                String clientHost = RemoteServer.getClientHost();
+                subject = new Subject(false,
+                                      subject.getPrincipals(),
+                                      subject.getPublicCredentials(),
+                                      subject.getPrivateCredentials());
+                subject.getPrincipals().add(new JMXConnectionPrincipal(clientHost));
+                subject.setReadOnly();
+            }
+            catch(ServerNotActiveException e)
+            {
+            }
         }
+
         final String username = user;
         Subject.doAs(subject, new PrivilegedAction<Object>()
         {
diff --git a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/UsernameAccessor.java b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/UsernameAccessor.java
index 0cbb0d2687..18ab02ece1 100644
--- a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/UsernameAccessor.java
+++ b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/UsernameAccessor.java
@@ -19,8 +19,10 @@
  */
 package org.apache.qpid.server.jmx;
 
+import javax.security.auth.Subject;
+
 public interface UsernameAccessor
 {
-    public String getUsernameForConnectionId(String connectionId);
+    public Subject getSubjectConnectionId(String connectionId);
 
 }
diff --git a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/UsernameCachingRMIJRMPServer.java b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/UsernameCachingRMIJRMPServer.java
index 838e9e5664..4caa14014a 100644
--- a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/UsernameCachingRMIJRMPServer.java
+++ b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/UsernameCachingRMIJRMPServer.java
@@ -37,11 +37,9 @@ import javax.management.remote.rmi.RMIConnection;
 import javax.management.remote.rmi.RMIJRMPServerImpl;
 import javax.security.auth.Subject;
 
-import org.apache.qpid.server.security.auth.AuthenticatedPrincipal;
-
 /**
  * An implementation of RMIJRMPServerImpl that caches the usernames of users as they log-on
- * and makes the same available via {@link UsernameAccessor#getUsernameForConnectionId(String)}.
+ * and makes the same available via {@link UsernameAccessor#getSubjectConnectionId(String)}.
  *
  * Caller is responsible for installing this object as a {@link NotificationListener} of the
  * {@link JMXConnectorServer} so the cache entries are removed as the clients disconnect.
@@ -50,7 +48,7 @@ import org.apache.qpid.server.security.auth.AuthenticatedPrincipal;
 public class UsernameCachingRMIJRMPServer extends RMIJRMPServerImpl implements NotificationListener, NotificationFilter, UsernameAccessor
 {
     // ConnectionId is guaranteed to be unique per client connection, according to the JMX spec.
-    private final Map<String, String> _connectionIdUsernameMap = new ConcurrentHashMap<String, String>();
+    private final Map<String, Subject> _connectionIdUsernameMap = new ConcurrentHashMap<String, Subject>();
 
     UsernameCachingRMIJRMPServer(int port, RMIClientSocketFactory csf, RMIServerSocketFactory ssf,
             Map<String, ?> env) throws IOException
@@ -62,13 +60,12 @@ public class UsernameCachingRMIJRMPServer extends RMIJRMPServerImpl implements N
     protected RMIConnection makeClient(String connectionId, Subject subject) throws IOException
     {
         final RMIConnection makeClient = super.makeClient(connectionId, subject);
-        final AuthenticatedPrincipal authenticatedPrincipalFromSubject = AuthenticatedPrincipal.getAuthenticatedPrincipalFromSubject(subject);
-        _connectionIdUsernameMap.put(connectionId, authenticatedPrincipalFromSubject.getName());
+        _connectionIdUsernameMap.put(connectionId, subject);
         return makeClient;
     }
 
     @Override
-    public String getUsernameForConnectionId(String connectionId)
+    public Subject getSubjectConnectionId(String connectionId)
     {
         return _connectionIdUsernameMap.get(connectionId);
     }
diff --git a/qpid/java/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/ManagementLogonLogoffReporterTest.java b/qpid/java/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/ManagementLogonLogoffReporterTest.java
index 0027815142..be5fe1eeca 100644
--- a/qpid/java/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/ManagementLogonLogoffReporterTest.java
+++ b/qpid/java/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/ManagementLogonLogoffReporterTest.java
@@ -31,18 +31,22 @@ import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
 
 import javax.management.remote.JMXConnectionNotification;
+import javax.security.auth.Subject;
 
 import org.apache.qpid.server.logging.EventLogger;
 import org.apache.qpid.server.logging.LogMessage;
 import org.apache.qpid.server.logging.MessageLogger;
 
 import junit.framework.TestCase;
+import org.apache.qpid.server.security.auth.AuthenticatedPrincipal;
 import org.mockito.ArgumentMatcher;
 
+import java.util.Collections;
+
 public class ManagementLogonLogoffReporterTest extends TestCase
 {
     private static final String TEST_JMX_UNIQUE_CONNECTION_ID = "jmxconnectionid1 jmxuser,group";
-    private static final String TEST_USER = "jmxuser";
+    private static final Subject TEST_USER = new Subject(false, Collections.singleton(new AuthenticatedPrincipal("jmxuser")), Collections.emptySet(), Collections.emptySet());
 
     private ManagementLogonLogoffReporter _reporter;
     private UsernameAccessor _usernameAccessor;
@@ -62,7 +66,7 @@ public class ManagementLogonLogoffReporterTest extends TestCase
 
     public void testOpenedNotification()
     {
-        when(_usernameAccessor.getUsernameForConnectionId(TEST_JMX_UNIQUE_CONNECTION_ID)).thenReturn(TEST_USER);
+        when(_usernameAccessor.getSubjectConnectionId(TEST_JMX_UNIQUE_CONNECTION_ID)).thenReturn(TEST_USER);
         JMXConnectionNotification openNotification = createMockNotification(TEST_JMX_UNIQUE_CONNECTION_ID, OPENED);
 
         _reporter.handleNotification(openNotification, null);
@@ -86,7 +90,7 @@ public class ManagementLogonLogoffReporterTest extends TestCase
 
     public void testClosedNotification()
     {
-        when(_usernameAccessor.getUsernameForConnectionId(TEST_JMX_UNIQUE_CONNECTION_ID)).thenReturn(TEST_USER);
+        when(_usernameAccessor.getSubjectConnectionId(TEST_JMX_UNIQUE_CONNECTION_ID)).thenReturn(TEST_USER);
         JMXConnectionNotification closeNotification = createMockNotification(TEST_JMX_UNIQUE_CONNECTION_ID, CLOSED);
 
         _reporter.handleNotification(closeNotification, null);