QPID-2759: Remove defunct jmxremote.access file user management rights manipulation abilities. 1) Removed remaining jmx access functionality. 2) Removed references to security/jmx/access from all existing configuration files. 3) Made ServerConfiguration#validateConfiguration reject config files that still contain the security/jmx/access element in order to promote good end-user understanding of configuration.

For JMX:
1) setRights now throws UnsupportedOperationException with message.
2) createUser(string,string,bool,bool,bool) throws UnsupportedOperationException iff any of the bool args are true i.e. the user attempts to give a user access JMX rights
3) Deprecated createUser(string,string,bool,bool,bool) in favour of new two arg form createUser(string,string)
4) viewUsers changes to always return admin, read, and write items as false.

Applied patch from Keith Wall <keith.wall@gmail.com>

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

3 files changed, 48 insertions, 398 deletions

diff --git a/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java b/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java
index 43be0611a5..7fc91c817b 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java
@@ -205,7 +205,15 @@ public class ServerConfiguration extends ConfigurationPlugin implements SignalHa
     @Override
     public void validateConfiguration() throws ConfigurationException
     {
-        //Currently doesn't do validation
+        // Support for security.jmx.access was removed when JMX access rights were incorporated into the main ACL.
+        // This ensure that users remove the element from their configuration file.
+        
+        if (getListValue("security.jmx.access").size() > 0)
+        {
+            String message = "Validation error : security/jmx/access is no longer a supported element within the configuration xml." 
+                    + (_configFile == null ? "" : " Configuration file : " + _configFile);
+            throw new ConfigurationException(message);
+        }
     }
 
     /*
@@ -530,11 +538,6 @@ public class ServerConfiguration extends ConfigurationPlugin implements SignalHa
         return getListValue("security.jmx.principal-database");
     }
 
-    public List<String> getManagementAccessList()
-    {
-        return getListValue("security.jmx.access");
-    }
-
     public int getFrameSize()
     {
         return getIntValue("advanced.framesize", DEFAULT_FRAME_SIZE);
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 5cebb7d2d8..e9276e1b0e 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
@@ -184,33 +184,6 @@ public class ConfigurationFilePrincipalDatabaseManager implements PrincipalDatab
             }
 
             _mbean.setPrincipalDatabase(database);
-
-            List<String> jmxaccesslist = config.getManagementAccessList();
-            if (jmxaccesslist.isEmpty())
-            {
-                throw new ConfigurationException("No access control files specified for jmx security");
-            }
-
-            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);
-            }
-
             _mbean.register();
         }
         catch (JMException e)
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/auth/management/AMQUserManagementMBean.java b/java/broker/src/main/java/org/apache/qpid/server/security/auth/management/AMQUserManagementMBean.java
index ee4336055b..a839315bcc 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/auth/management/AMQUserManagementMBean.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/auth/management/AMQUserManagementMBean.java
@@ -20,19 +20,9 @@
  */
 package org.apache.qpid.server.security.auth.management;
 
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
 import java.io.IOException;
-import java.security.AccessControlContext;
-import java.security.AccessController;
 import java.security.Principal;
-import java.util.Enumeration;
 import java.util.List;
-import java.util.Properties;
-import java.util.Random;
-import java.util.Set;
-import java.util.concurrent.locks.ReentrantLock;
 
 import javax.management.JMException;
 import javax.management.openmbean.CompositeData;
@@ -44,17 +34,13 @@ import javax.management.openmbean.SimpleType;
 import javax.management.openmbean.TabularData;
 import javax.management.openmbean.TabularDataSupport;
 import javax.management.openmbean.TabularType;
-import javax.management.remote.JMXPrincipal;
-import javax.security.auth.Subject;
 import javax.security.auth.login.AccountNotFoundException;
 
-import org.apache.commons.configuration.ConfigurationException;
 import org.apache.log4j.Logger;
 import org.apache.qpid.management.common.mbeans.UserManagement;
 import org.apache.qpid.management.common.mbeans.annotations.MBeanDescription;
 import org.apache.qpid.management.common.mbeans.annotations.MBeanOperation;
 import org.apache.qpid.server.management.AMQManagedObject;
-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;
 
@@ -65,22 +51,18 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana
     private static final Logger _logger = Logger.getLogger(AMQUserManagementMBean.class);
 
     private PrincipalDatabase _principalDatabase;
-    private Properties _accessRights;
-    private File _accessFile;
-
-    private ReentrantLock _accessRightsUpdate = new ReentrantLock();
 
     // Setup for the TabularType
-    static TabularType _userlistDataType; // Datatype for representing User Lists
-    static CompositeType _userDataType; // Composite type for representing User
+    private static final TabularType _userlistDataType; // Datatype for representing User Lists
+    private static final CompositeType _userDataType; // Composite type for representing User
 
     static
     {
         OpenType[] userItemTypes = new OpenType[4]; // User item types.
         userItemTypes[0] = SimpleType.STRING;  // For Username
-        userItemTypes[1] = SimpleType.BOOLEAN; // For Rights - Read
-        userItemTypes[2] = SimpleType.BOOLEAN; // For Rights - Write
-        userItemTypes[3] = SimpleType.BOOLEAN; // For Rights - Admin
+        userItemTypes[1] = SimpleType.BOOLEAN; // For Rights - Read - No longer in use
+        userItemTypes[2] = SimpleType.BOOLEAN; // For Rights - Write - No longer in use
+        userItemTypes[3] = SimpleType.BOOLEAN; // For Rights - Admin - No longer is use
 
         try
         {
@@ -92,12 +74,11 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana
         }
         catch (OpenDataException e)
         {
-            _logger.error("Tabular data setup for viewing users incorrect.");
-            _userlistDataType = null;
+            _logger.error("Tabular data setup for viewing users incorrect.", e);
+            throw new ExceptionInInitializerError("Tabular data setup for viewing users incorrect");
         }
     }
 
-
     public AMQUserManagementMBean() throws JMException
     {
         super(UserManagement.class, UserManagement.TYPE);
@@ -122,141 +103,45 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana
         }
         catch (AccountNotFoundException e)
         {
-            _logger.warn("Attempt to set password of non-existant user'" + username + "'");
+            _logger.warn("Attempt to set password of non-existent user'" + username + "'");
             return false;
         }
     }
 
     public boolean setRights(String username, boolean read, boolean write, boolean admin)
     {
-
-        Object oldRights = null;
-        if ((oldRights =_accessRights.get(username)) == null)
-        {
-            // If the user doesn't exist in the access rights file check that they at least have an account.
-            if (_principalDatabase.getUser(username) == null)
-            {
-                return false;
-            }
-        }
-
-        try
-        {
-            _accessRightsUpdate.lock();
-
-            // Update the access rights
-            if (admin)
-            {
-                _accessRights.put(username, MBeanInvocationHandlerImpl.ADMIN);
-            }
-            else
-            {
-                if (read | write)
-                {
-                    if (read)
-                    {
-                        _accessRights.put(username, MBeanInvocationHandlerImpl.READONLY);
-                    }
-                    if (write)
-                    {
-                        _accessRights.put(username, MBeanInvocationHandlerImpl.READWRITE);
-                    }
-                }
-                else
-                {
-                    _accessRights.remove(username);
-                }
-            }
-            
-            //save the rights file
-            try
-            {
-                saveAccessFile();
-            }
-            catch (IOException e)
-            {
-                _logger.warn("Problem occured saving '" + _accessFile + "', the access right changes will not be preserved: " + e);
-
-                //the rights file was not successfully saved, restore user rights to previous value
-                _logger.warn("Reverting attempted rights update for user'" + username + "'");
-                if (oldRights != null)
-                {
-                    _accessRights.put(username, oldRights);
-                }
-                else
-                {
-                    _accessRights.remove(username);
-                }
-
-                return false;
-            }
-        }
-        finally
+        throw new UnsupportedOperationException("Support for setting access rights no longer supported.");
+    }
+    
+    public boolean createUser(String username, String password)
+    {
+        if (_principalDatabase.createPrincipal(new UsernamePrincipal(username), password.toCharArray()))
         {
-            _accessRightsUpdate.unlock();
+            return true;
         }
 
-        return true;
+        return false;
     }
     
     public boolean createUser(String username, String password, boolean read, boolean write, boolean admin)
     {
-        return createUser(username, password.toCharArray(), read, write, admin);
+        if (read || write || admin)
+        {
+            throw new UnsupportedOperationException("Support for setting access rights to true no longer supported.");
+        }
+        return createUser(username, password);
     }
 
     public boolean createUser(String username, char[] password, boolean read, boolean write, boolean admin)
     {
-        if (_principalDatabase.createPrincipal(new UsernamePrincipal(username), password))
-        {
-            if (!setRights(username, read, write, admin))
-            {
-                //unable to set rights for user, remove account
-                try
-                {
-                    _principalDatabase.deletePrincipal(new UsernamePrincipal(username));
-                }
-                catch (AccountNotFoundException e)
-                {
-                    //ignore
-                }
-                return false;
-            }
-            else
-            {
-                return true;
-            }
-        }
-
-        return false;
+        return createUser(username, new String(password), read, write, admin);
     }
 
     public boolean deleteUser(String username)
     {
         try
         {
-            if (_principalDatabase.deletePrincipal(new UsernamePrincipal(username)))
-            {
-                try
-                {
-                    _accessRightsUpdate.lock();
-
-                    _accessRights.remove(username);
-                    
-                    try
-                    {
-                        saveAccessFile();
-                    }
-                    catch (IOException e)
-                    {
-                        _logger.warn("Problem occured saving '" + _accessFile + "', the access right changes will not be preserved: " + e);
-                        return false;
-                    }
-                }
-                finally
-                {
-                    _accessRightsUpdate.unlock();
-                }
-            }
+            _principalDatabase.deletePrincipal(new UsernamePrincipal(username));
         }
         catch (AccountNotFoundException e)
         {
@@ -269,38 +154,23 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana
 
     public boolean reloadData()
     {
-            try
-            {
-                loadAccessFile();
-                _principalDatabase.reload();
-            }
-            catch (ConfigurationException e)
-            {
-                _logger.warn("Reload failed due to:" + e);
-                return false;
-            }
-            catch (IOException e)
-            {
-                _logger.warn("Reload failed due to:" + e);
-                return false;
-            }
-            // Reload successful
-            return true;
+        try
+        {
+            _principalDatabase.reload();
+        }
+        catch (IOException e)
+        {
+            _logger.warn("Reload failed due to:", e);
+            return false;
+        }
+        // Reload successful
+        return true;
     }
 
 
-    @MBeanOperation(name = "viewUsers", description = "All users with access rights to the system.")
+    @MBeanOperation(name = "viewUsers", description = "All users that are currently available to the system.")
     public TabularData viewUsers()
     {
-        // Table of users
-        // Username(string), Access rights Read,Write,Admin(bool,bool,bool)
-
-        if (_userlistDataType == null)
-        {
-            _logger.warn("TabluarData not setup correctly");
-            return null;
-        }
-
         List<Principal> users = _principalDatabase.getUsers();
 
         TabularDataSupport userList = new TabularDataSupport(_userlistDataType);
@@ -311,29 +181,16 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana
             for (Principal user : users)
             {
                 // Create header attributes list
-
-                String rights = (String) _accessRights.get(user.getName());
-
-                Boolean read = false;
-                Boolean write = false;
-                Boolean admin = false;
-
-                if (rights != null)
-                {
-                    read = rights.equals(MBeanInvocationHandlerImpl.READONLY)
-                           || rights.equals(MBeanInvocationHandlerImpl.READWRITE);
-                    write = rights.equals(MBeanInvocationHandlerImpl.READWRITE);
-                    admin = rights.equals(MBeanInvocationHandlerImpl.ADMIN);
-                }
-
-                Object[] itemData = {user.getName(), read, write, admin};
+                
+                // Read,Write,Admin items are depcreated and we return always false.
+                Object[] itemData = {user.getName(), false, false, false};
                 CompositeData messageData = new CompositeDataSupport(_userDataType, COMPOSITE_ITEM_NAMES.toArray(new String[COMPOSITE_ITEM_NAMES.size()]), itemData);
                 userList.put(messageData);
             }
         }
         catch (OpenDataException e)
         {
-            _logger.warn("Unable to create user list due to :" + e);
+            _logger.warn("Unable to create user list due to :", e);
             return null;
         }
 
@@ -351,187 +208,4 @@ public class AMQUserManagementMBean extends AMQManagedObject implements UserMana
     {
         _principalDatabase = database;
     }
-
-    /**
-     * setAccessFile
-     *
-     * @param accessFile the file to use for updating.
-     *
-     * @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(String accessFile) throws IOException, ConfigurationException
-    {
-        if (accessFile != null)
-        {
-            _accessFile = new File(accessFile);
-            if (!_accessFile.exists())
-            {
-                throw new ConfigurationException("'" + _accessFile + "' does not exist");
-            }
-
-            if (!_accessFile.canRead())
-            {
-                throw new ConfigurationException("Cannot read '" + _accessFile + "'.");
-            }
-
-            if (!_accessFile.canWrite())
-            {
-                _logger.warn("Unable to write to access rights file '" + _accessFile + "', changes will not be preserved.");
-            }
-
-            loadAccessFile();
-        }
-        else
-        {
-            _logger.warn("Access rights file specified is null. Access rights not changed.");
-        }
-    }
-
-    private void loadAccessFile() throws IOException, ConfigurationException
-    {
-        if(_accessFile == null)
-        {
-            _logger.error("No jmx access rights file has been specified.");
-            return;
-        }
-        
-        if(_accessFile.exists())
-        {
-            try
-            {
-                _accessRightsUpdate.lock();
-
-                Properties accessRights = new Properties();
-                FileInputStream inStream = new FileInputStream(_accessFile);
-                try
-                {
-                    accessRights.load(inStream);
-                }
-                finally
-                {
-                    inStream.close();
-                }
-
-                checkAccessRights(accessRights);
-                setAccessRights(accessRights);
-            }
-            finally
-            {
-                _accessRightsUpdate.unlock();
-            }
-        }
-        else
-        {
-            _logger.error("Specified jmxaccess rights file '" + _accessFile + "' does not exist.");
-        }
-    }
-
-    private void checkAccessRights(Properties accessRights)
-    {
-        Enumeration values = accessRights.propertyNames();
-
-        while (values.hasMoreElements())
-        {
-            String user = (String) values.nextElement();
-
-            if (_principalDatabase.getUser(user) == null)
-            {
-                _logger.warn("Access rights contains user '" + user + "' but there is no authentication data for that user");
-            }
-        }
-    }
-
-    private void saveAccessFile() throws IOException
-    {
-        try
-        {
-            _accessRightsUpdate.lock();
-
-            // Create temporary file
-            Random r = new Random();
-            File tmp;
-            do
-            {
-                tmp = new File(_accessFile.getPath() + r.nextInt() + ".tmp");
-            }
-            while(tmp.exists());
-            
-            tmp.deleteOnExit();
-
-            FileOutputStream output = new FileOutputStream(tmp);
-            _accessRights.store(output, "Generated by AMQUserManagementMBean Console : Last edited by user:" + getCurrentJMXUser());
-            output.close();
-
-            // Swap temp file to main rights file.
-            File old = new File(_accessFile.getAbsoluteFile() + ".old");
-            if (old.exists())
-            {
-                old.delete();
-            }
-            
-            if(!_accessFile.renameTo(old))
-            {
-                //unable to rename the existing file to the backup name 
-                _logger.error("Could not backup the existing management rights file");
-                throw new IOException("Could not backup the existing management rights file");
-            }
-
-            if(!tmp.renameTo(_accessFile))
-            {
-                //failed to rename the new file to the required filename
-                
-                if(!old.renameTo(_accessFile))
-                {
-                    //unable to return the backup to required filename
-                    _logger.error("Could not rename the new management rights file into place, and unable to restore original file");
-                    throw new IOException("Could not rename the new management rights file into place, and unable to restore original file");
-                }
-                
-                _logger.error("Could not rename the new management rights file into place");
-                throw new IOException("Could not rename the new management rights file into place");
-            }
-        }
-        finally
-        {
-            _accessRightsUpdate.unlock();
-        }
-        
-    }
-
-    private String getCurrentJMXUser()
-    {
-        AccessControlContext acc = AccessController.getContext();
-        
-        Subject subject = Subject.getSubject(acc);
-        if (subject == null)
-        {
-            return "Unknown user, authentication Subject was null";
-        }
-
-        // Retrieve JMXPrincipal from Subject
-        Set<JMXPrincipal> principals = subject.getPrincipals(JMXPrincipal.class);
-        if (principals == null || principals.isEmpty())
-        {
-            return "Unknown user principals were null";
-        }
-
-        Principal principal = principals.iterator().next();
-        return principal.getName();
-    }
-
-    /**
-     * user=read user=write user=readwrite user=admin
-     *
-     * @param accessRights The properties list of access rights to process
-     */
-    private void setAccessRights(Properties accessRights)
-    {
-        _logger.debug("Setting Access Rights:" + accessRights);
-        _accessRights = accessRights;
-		
-		// TODO check where this is used
-        // MBeanInvocationHandlerImpl.setAccessRights(_accessRights);
-    }
 }