QPID-446

JMXManagedObjectRegistry - Split instantiation from starting up. To all the setting of the Access file when loaded later in the startup sequence.
ManagedObjectRegistry - Added Start method
MBeanInvocationHandlerImpl - Updated to allow the setting of the access properties object from the AMQUserManagementMBean
NoopManagedObjectRegistry - implemented no-op start
ConfigurationFileApplicationRegistry - Adjusted to split creation of ManagedObjectRegistry from starting server to allow the setting of access rights.
AMQUserManagementMBean - Implemented reading of access rights file.
Base64MD5PasswordFilePrincipalDatabase - added comment for future Management.
PrincipalDatabaseManager - added initialiseManagement method
ConfigurationFilePrincipalDatabaseManager - implemented general Management initialisation.
PropertiesPrincipalDatabaseManager - no-op implementation




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

10 files changed, 175 insertions, 57 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 66c564c0ab..149a925a33 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
@@ -76,21 +76,27 @@ public class JMXManagedObjectRegistry implements ManagedObjectRegistry
 
         // Retrieve the config parameters
         boolean platformServer = appRegistry.getConfiguration().getBoolean("management.platform-mbeanserver", true);
-        boolean security = appRegistry.getConfiguration().getBoolean("management.security-enabled", true);
-        int port = appRegistry.getConfiguration().getInt("management.jmxport", 8999);
 
         _mbeanServer =
                 platformServer ? ManagementFactory.getPlatformMBeanServer()
                 : MBeanServerFactory.createMBeanServer(ManagedObject.DOMAIN);
+    }
 
+
+    public void start()
+    {
         // Check if the "QPID_OPTS" is set to use Out of the Box JMXAgent
         if (areOutOfTheBoxJMXOptionsSet())
         {
             _log.info("JMX: Using the out of the box JMX Agent");
-
             return;
         }
 
+        IApplicationRegistry appRegistry = ApplicationRegistry.getInstance();
+
+        boolean security = appRegistry.getConfiguration().getBoolean("management.security-enabled", true);
+        int port = appRegistry.getConfiguration().getInt("management.jmxport", 8999);
+
         try
         {
             if (security)
@@ -139,7 +145,6 @@ public class JMXManagedObjectRegistry implements ManagedObjectRegistry
                 try
                 {
                     JMXConnectorServer cs = JMXConnectorServerFactory.newJMXConnectorServer(_jmxURL, env, _mbeanServer);
-                    MBeanInvocationHandlerImpl.initialise();
                     MBeanServerForwarder mbsf = MBeanInvocationHandlerImpl.newProxyInstance();
                     cs.setMBeanServerForwarder(mbsf);
                     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 288cc3a7cc..07260d8645 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
@@ -44,21 +44,20 @@ import java.io.IOException;
 import java.io.FileInputStream;
 
 /**
- * This class can be used by the JMXConnectorServer as an InvocationHandler for the mbean operations.
- * This implements the logic for allowing the users to invoke MBean operations and implements the
- * restrictions for readOnly, readWrite and admin users.
+ * This class can be used by the JMXConnectorServer as an InvocationHandler for the mbean operations. This implements
+ * the logic for allowing the users to invoke MBean operations and implements the restrictions for readOnly, readWrite
+ * and admin users.
  */
 public class MBeanInvocationHandlerImpl implements InvocationHandler
 {
     private static final Logger _logger = Logger.getLogger(MBeanInvocationHandlerImpl.class);
-    
+
     private final static String ADMIN = "admin";
-    private final static String READWRITE="readwrite";
+    private final static String READWRITE = "readwrite";
     private final static String READONLY = "readonly";
     private final static String DELEGATE = "JMImplementation:type=MBeanServerDelegate";
-    private static final String DEFAULT_PERMISSIONS_FILE = "etc" + File.separator + "jmxremote.access";
     private MBeanServer mbs;
-    private final static Properties _userRoles = new Properties();
+    private static Properties _userRoles = new Properties();
 
     public static MBeanServerForwarder newProxyInstance()
     {
@@ -119,7 +118,7 @@ public class MBeanInvocationHandlerImpl implements InvocationHandler
         {
             throw new SecurityException("Access denied");
         }
-        
+
         Principal principal = principals.iterator().next();
         String identity = principal.getName();
 
@@ -140,21 +139,9 @@ public class MBeanInvocationHandlerImpl implements InvocationHandler
     }
 
     // Initialises the user roles
-    protected static void initialise() throws AMQException
+    public static void setAccessRights(Properties accessRights)
     {
-        final String QpidHome = System.getProperty("QPID_HOME");
-        String fileName = QpidHome + File.separator + DEFAULT_PERMISSIONS_FILE;
-        try
-        {
-            FileInputStream in = new FileInputStream(fileName);
-            _userRoles.load(in);
-            in.close();
-        }
-        catch (IOException ex)
-        {
-            _logger.error("Error in loading JMX User permissions." + ex.getMessage());
-            //throw new AMQException("Error in loading JMX User permissions", ex);
-        }
+        _userRoles = accessRights;
     }
 
     private boolean isAdmin(String userName)
@@ -200,11 +187,13 @@ public class MBeanInvocationHandlerImpl implements InvocationHandler
         {
             String mbeanMethod = (args.length > 1) ? (String) args[1] : null;
             if (mbeanMethod == null)
+            {
                 return false;
-            
+            }
+
             try
             {
-                MBeanInfo mbeanInfo = mbs.getMBeanInfo((ObjectName)args[0]);
+                MBeanInfo mbeanInfo = mbs.getMBeanInfo((ObjectName) args[0]);
                 if (mbeanInfo != null)
                 {
                     MBeanOperationInfo[] opInfos = mbeanInfo.getOperations();
@@ -219,7 +208,7 @@ public class MBeanInvocationHandlerImpl implements InvocationHandler
             }
             catch (JMException ex)
             {
-                ex.printStackTrace();   
+                ex.printStackTrace();
             }
         }
 
diff --git a/java/broker/src/main/java/org/apache/qpid/server/management/ManagedObjectRegistry.java b/java/broker/src/main/java/org/apache/qpid/server/management/ManagedObjectRegistry.java
index 006a535707..5f9bc9ddad 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/management/ManagedObjectRegistry.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/management/ManagedObjectRegistry.java
@@ -37,6 +37,8 @@ import java.rmi.RemoteException;
  */
 public interface ManagedObjectRegistry
 {
+    void start();
+
     void registerObject(ManagedObject managedObject) throws JMException;
 
     void unregisterObject(ManagedObject managedObject) throws JMException;
diff --git a/java/broker/src/main/java/org/apache/qpid/server/management/NoopManagedObjectRegistry.java b/java/broker/src/main/java/org/apache/qpid/server/management/NoopManagedObjectRegistry.java
index 9e3995a427..b4fbed6948 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/management/NoopManagedObjectRegistry.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/management/NoopManagedObjectRegistry.java
@@ -40,6 +40,11 @@ public class NoopManagedObjectRegistry implements ManagedObjectRegistry
         _log.info("Management is disabled");
     }
 
+    public void start()
+    {
+        //no-op
+    }
+
     public void registerObject(ManagedObject managedObject) throws JMException
     {
     }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/registry/ConfigurationFileApplicationRegistry.java b/java/broker/src/main/java/org/apache/qpid/server/registry/ConfigurationFileApplicationRegistry.java
index 9e942d4d8c..1cca259a8d 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/registry/ConfigurationFileApplicationRegistry.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/registry/ConfigurationFileApplicationRegistry.java
@@ -103,6 +103,8 @@ public class ConfigurationFileApplicationRegistry extends ApplicationRegistry
 
     public void initialise() throws Exception
     {
+        initialiseManagedObjectRegistry();
+
         _virtualHostRegistry = new VirtualHostRegistry();
 
         _accessManager = new AccessManagerImpl("default", _configuration);
@@ -111,8 +113,10 @@ public class ConfigurationFileApplicationRegistry extends ApplicationRegistry
 
         _authenticationManager = new PrincipalDatabaseAuthenticationManager(null, null);
 
-        initialiseManagedObjectRegistry();
-        
+        _databaseManager.initialiseManagement(_configuration);
+
+        _managedObjectRegistry.start();
+
         initialiseVirtualHosts();
 
     }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/access/AMQUserManagementMBean.java b/java/broker/src/main/java/org/apache/qpid/server/security/access/AMQUserManagementMBean.java
index c8b4c92a60..15e3b8681f 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/access/AMQUserManagementMBean.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/access/AMQUserManagementMBean.java
@@ -24,9 +24,11 @@ import org.apache.qpid.server.management.MBeanDescription;
 import org.apache.qpid.server.management.AMQManagedObject;
 import org.apache.qpid.server.management.MBeanOperationParameter;
 import org.apache.qpid.server.management.MBeanOperation;
+import org.apache.qpid.server.management.MBeanInvocationHandlerImpl;
 import org.apache.qpid.server.security.auth.database.PrincipalDatabase;
 import org.apache.qpid.server.security.auth.sasl.UsernamePrincipal;
 import org.apache.log4j.Logger;
+import org.apache.commons.configuration.ConfigurationException;
 
 import javax.management.JMException;
 import javax.management.openmbean.TabularData;
@@ -47,7 +49,7 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana
     private static final Logger _logger = Logger.getLogger(AMQUserManagementMBean.class);
 
     private PrincipalDatabase _principalDatabase;
-    private File _accessFile;
+    private String _accessFile;
 
     Map<String, Principal> _users = new HashMap<String, Principal>();
 
@@ -128,7 +130,15 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana
     {
         try
         {
-            loadAccessFile();
+            try
+            {
+                loadAccessFile();
+            }
+            catch (ConfigurationException e)
+            {
+                _logger.info("Reload failed due to:" + e);
+                return false;
+            }
 
             // Reload successful
             return true;
@@ -144,7 +154,7 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana
     @MBeanOperation(name = "viewUsers", description = "All users with access rights to the system.")
     public TabularData viewUsers()
     {
-        return null;
+        return null;       //todo
     }
 
     /*** Broker Methods **/
@@ -152,9 +162,7 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana
     /**
      * setPrincipalDatabase
      *
-     * @param database
-     *
-     * @throws java.io.IOException If the file cannot be read
+     * @param database set The Database to use for user lookup
      */
     public void setPrincipalDatabase(PrincipalDatabase database)
     {
@@ -166,9 +174,11 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana
      *
      * @param accessFile the file to use for updating.
      *
-     * @throws java.io.IOException If the file cannot be read
+     * @throws java.io.IOException If the file cannot be accessed
+     * @throws org.apache.commons.configuration.ConfigurationException
+     *                             if checks on the file fail.
      */
-    public void setAccessFile(File accessFile) throws IOException
+    public void setAccessFile(String accessFile) throws IOException, ConfigurationException
     {
         _accessFile = accessFile;
 
@@ -182,12 +192,29 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana
         }
     }
 
-    private void loadAccessFile() throws IOException
+    private void loadAccessFile() throws IOException, ConfigurationException
     {
         Properties accessRights = new Properties();
-        accessRights.load(new FileInputStream(_accessFile));
-        processAccessRights(accessRights);
 
+        File access = new File(_accessFile);
+
+        if (!access.exists())
+        {
+            throw new ConfigurationException("'" + _accessFile + "' does not exist");
+        }
+
+        if (!access.canRead())
+        {
+            throw new ConfigurationException("Cannot read '" + _accessFile + "'.");
+        }
+
+        if (!access.canWrite())
+        {
+            _logger.warn("Unable to write to access file '" + _accessFile + "' changes will not be preserved.");
+        }
+
+        accessRights.load(new FileInputStream(access));
+        processAccessRights(accessRights);
     }
 
     /**
@@ -197,6 +224,7 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana
      */
     private void processAccessRights(Properties accessRights)
     {
-        //To change body of created methods use File | Settings | File Templates.
+        _logger.info("Processing Access Rights:" + accessRights);
+        MBeanInvocationHandlerImpl.setAccessRights(accessRights);       
     }
 }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabase.java b/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabase.java
index 1fd86f0bac..687a19e381 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabase.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabase.java
@@ -81,15 +81,16 @@ public class Base64MD5PasswordFilePrincipalDatabase implements PrincipalDatabase
         cram.initialise(this);
         _saslServers.put(cram.getMechanismName(), cram);
 
-        try
-        {
-            _mbean = new AMQUserManagementMBean();
-            _mbean.setPrincipalDatabase(this);
-        }
-        catch (JMException e)
-        {
-            _logger.warn("User management disabled as unable to create MBean:" + e);
-        }
+        //fixme The PDs should setup a PD Mangement MBean
+//        try
+//        {
+//            _mbean = new AMQUserManagementMBean();
+//            _mbean.setPrincipalDatabase(this);
+//        }
+//        catch (JMException e)
+//        {
+//            _logger.warn("User management disabled as unable to create MBean:" + e);
+//        }
     }
 
     public void setPasswordFile(String passwordFile) throws IOException
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/ConfigurationFilePrincipalDatabaseManager.java b/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/ConfigurationFilePrincipalDatabaseManager.java
index 8d2bf6d5ab..2d3f5e5131 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/ConfigurationFilePrincipalDatabaseManager.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/ConfigurationFilePrincipalDatabaseManager.java
@@ -21,6 +21,7 @@
 package org.apache.qpid.server.security.auth.database;
 
 import java.io.FileNotFoundException;
+import java.io.IOException;
 import java.lang.reflect.Method;
 import java.util.HashMap;
 import java.util.List;
@@ -32,9 +33,14 @@ import org.apache.commons.configuration.ConfigurationException;
 import org.apache.log4j.Logger;
 
 import org.apache.qpid.configuration.PropertyUtils;
+import org.apache.qpid.configuration.PropertyException;
 import org.apache.qpid.server.registry.ApplicationRegistry;
 import org.apache.qpid.server.security.auth.database.PrincipalDatabase;
 import org.apache.qpid.server.security.auth.database.PrincipalDatabaseManager;
+import org.apache.qpid.server.security.access.AMQUserManagementMBean;
+import org.apache.qpid.AMQException;
+
+import javax.management.JMException;
 
 public class ConfigurationFilePrincipalDatabaseManager implements PrincipalDatabaseManager
 {
@@ -101,7 +107,7 @@ public class ConfigurationFilePrincipalDatabaseManager implements PrincipalDatab
     }
 
     private void initialisePrincipalDatabase(PrincipalDatabase principalDatabase, Configuration config, int index)
-        throws FileNotFoundException, ConfigurationException
+            throws FileNotFoundException, ConfigurationException
     {
         String baseName = _base + "(" + index + ").attributes.attribute.";
         List<String> argumentNames = config.getList(baseName + "name");
@@ -133,9 +139,9 @@ public class ConfigurationFilePrincipalDatabaseManager implements PrincipalDatab
             if (method == null)
             {
                 throw new ConfigurationException("No method " + methodName + " found in class "
-                    + principalDatabase.getClass()
-                    + " hence unable to configure principal database. The method must be public and "
-                    + "have a single String argument with a void return type");
+                                                 + principalDatabase.getClass()
+                                                 + " hence unable to configure principal database. The method must be public and "
+                                                 + "have a single String argument with a void return type");
             }
 
             try
@@ -146,7 +152,7 @@ public class ConfigurationFilePrincipalDatabaseManager implements PrincipalDatab
             {
                 if (ite instanceof ConfigurationException)
                 {
-                    throw (ConfigurationException) ite;
+                    throw(ConfigurationException) ite;
                 }
                 else
                 {
@@ -160,4 +166,71 @@ public class ConfigurationFilePrincipalDatabaseManager implements PrincipalDatab
     {
         return _databases;
     }
+
+    public void initialiseManagement(Configuration config) throws ConfigurationException
+    {
+        try
+        {
+            AMQUserManagementMBean _mbean = new AMQUserManagementMBean();
+
+            String baseSecurity = "security.jmx";
+            List<String> principalDBs = config.getList(baseSecurity + ".principal-database");
+
+            if (principalDBs.size() == 0)
+            {
+                throw new ConfigurationException("No principal-database specified for jmx security(" + baseSecurity + ".principal-database)");
+            }
+
+            String databaseName = principalDBs.get(0);
+
+            PrincipalDatabase database = getDatabases().get(databaseName);
+
+            if (database == null)
+            {
+                throw new ConfigurationException("Principal-database '" + databaseName + "' not found");
+            }
+
+            _mbean.setPrincipalDatabase(database);
+
+            List<String> jmxaccesslist = config.getList(baseSecurity + ".access");
+
+            if (jmxaccesslist.size() == 0)
+            {
+                throw new ConfigurationException("No access control files specified for jmx security(" + baseSecurity + ".access)");
+            }
+
+            String jmxaccesssFile = null;
+            
+            try
+            {
+                jmxaccesssFile = PropertyUtils.replaceProperties(jmxaccesslist.get(0));
+            }
+            catch (PropertyException e)
+            {
+                throw new ConfigurationException("Unable to parse access control filename '" + jmxaccesssFile + "'");
+            }
+
+            try
+            {
+                _mbean.setAccessFile(jmxaccesssFile);
+            }
+            catch (IOException e)
+            {
+                _logger.warn("Unable to load access file:" + jmxaccesssFile);
+            }
+
+            try
+            {
+                _mbean.register();
+            }
+            catch (AMQException e)
+            {
+                _logger.warn("Unable to register user management MBean");
+            }
+        }
+        catch (JMException e)
+        {
+            _logger.warn("User management disabled as unable to create MBean:" + e);
+        }
+    }
 }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PrincipalDatabaseManager.java b/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PrincipalDatabaseManager.java
index 83f1201bd8..2c553ae76a 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PrincipalDatabaseManager.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PrincipalDatabaseManager.java
@@ -21,10 +21,14 @@
 package org.apache.qpid.server.security.auth.database;
 
 import org.apache.qpid.server.security.auth.database.PrincipalDatabase;
+import org.apache.commons.configuration.Configuration;
+import org.apache.commons.configuration.ConfigurationException;
 
 import java.util.Map;
 
 public interface PrincipalDatabaseManager
 {
     public Map<String, PrincipalDatabase> getDatabases();
+
+    public void initialiseManagement(Configuration config) throws ConfigurationException;
 }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PropertiesPrincipalDatabaseManager.java b/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PropertiesPrincipalDatabaseManager.java
index 89c84e8130..6b86a46bd2 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PropertiesPrincipalDatabaseManager.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PropertiesPrincipalDatabaseManager.java
@@ -20,6 +20,8 @@
  */
 package org.apache.qpid.server.security.auth.database;
 
+import org.apache.commons.configuration.Configuration;
+
 import java.util.Map;
 import java.util.Properties;
 import java.util.HashMap;
@@ -38,4 +40,9 @@ public class PropertiesPrincipalDatabaseManager implements PrincipalDatabaseMana
     {
         return _databases;
     }
+
+    public void initialiseManagement(Configuration config)
+    {
+        //todo
+    }
 }