You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ri...@apache.org on 2007/04/12 10:52:24 UTC

svn commit: r527843 - in /incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server: management/ security/access/ security/auth/database/

Author: ritchiem
Date: Thu Apr 12 01:52:19 2007
New Revision: 527843

URL: http://svn.apache.org/viewvc?view=rev&rev=527843
Log:
QPID-446 Update to send userList to JMX Management console.
Currently niave implementation just sending ALL users in one go. If a LDAPPrincipalDatabase was created this could be quite a lot of data a) to send but b) to create in broker Heap.
PrincipalDatabase - javadoc'd and getUsers method, 
 -changed verifyPassword method to take String for username rather than Principal only the Managment Console uses this method and it the MC should be changed to use the Broker SASL modules directly rather than having very similar ones of its own.
 - Removed AccountNotFound exception from createPrincipal as it made no sence
No-op implementation in PlainPasswordFilePrincipalDatabase and PropertiesPrincipalDatabase
Base64MD5PasswordFilePrincipalDatabase changed local User class to implement Principal so current Map can be returned via getUsers
 - Added locking to ensure integrity of files in the face of multiple edits.


Modified:
    incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/management/JMXManagedObjectRegistry.java
    incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/access/AMQUserManagementMBean.java
    incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabase.java
    incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PlainPasswordFilePrincipalDatabase.java
    incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PrincipalDatabase.java
    incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PropertiesPrincipalDatabase.java

Modified: incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/management/JMXManagedObjectRegistry.java
URL: http://svn.apache.org/viewvc/incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/management/JMXManagedObjectRegistry.java?view=diff&rev=527843&r1=527842&r2=527843
==============================================================================
--- incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/management/JMXManagedObjectRegistry.java (original)
+++ incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/management/JMXManagedObjectRegistry.java Thu Apr 12 01:52:19 2007
@@ -280,7 +280,7 @@
                 String username = ncb.getDefaultName();
                 try
                 {
-                    authorized = _principalDatabase.verifyPassword(new UsernamePrincipal(username), new String(pcb.getPassword()));
+                    authorized = _principalDatabase.verifyPassword(username, new String(pcb.getPassword()));
                 }
                 catch (AccountNotFoundException e)
                 {

Modified: incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/access/AMQUserManagementMBean.java
URL: http://svn.apache.org/viewvc/incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/access/AMQUserManagementMBean.java?view=diff&rev=527843&r1=527842&r2=527843
==============================================================================
--- incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/access/AMQUserManagementMBean.java (original)
+++ incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/access/AMQUserManagementMBean.java Thu Apr 12 01:52:19 2007
@@ -32,12 +32,24 @@
 
 import javax.management.JMException;
 import javax.management.openmbean.TabularData;
+import javax.management.openmbean.TabularDataSupport;
+import javax.management.openmbean.TabularType;
+import javax.management.openmbean.SimpleType;
+import javax.management.openmbean.CompositeType;
+import javax.management.openmbean.OpenType;
+import javax.management.openmbean.OpenDataException;
+import javax.management.openmbean.CompositeData;
+import javax.management.openmbean.CompositeDataSupport;
 import javax.security.auth.login.AccountNotFoundException;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.FileOutputStream;
 import java.util.Properties;
+import java.util.List;
+import java.util.Enumeration;
+import java.util.concurrent.locks.ReentrantLock;
+import java.security.Principal;
 
 /** MBean class for AMQUserManagementMBean. It implements all the management features exposed for managing users. */
 @MBeanDescription("User Management Interface")
@@ -49,7 +61,41 @@
     private PrincipalDatabase _principalDatabase;
     private String _accessFileName;
     private Properties _accessRights;
-    private File _accessFile;
+    //    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
+    static String[] _userItemNames = {"Username", "Read", "Write", "Admin"};
+
+    static
+    {
+        String[] userItemDesc = {"Broker Login username", "Management Console Read Permission",
+                                 "Management Console Write Permission", "Management Console Admin Permission"};
+
+        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
+        String[] userDataIndex = {_userItemNames[0]};
+
+        try
+        {
+            _userDataType =
+                    new CompositeType("User", "User Data", _userItemNames, userItemDesc, userItemTypes);
+
+            _userlistDataType = new TabularType("Users", "List of users", _userDataType, userDataIndex);
+        }
+        catch (OpenDataException e)
+        {
+            _logger.error("Tabular data setup for viewing users incorrect.");
+            _userlistDataType = null;
+        }
+    }
+
 
     public AMQUserManagementMBean() throws JMException
     {
@@ -66,6 +112,7 @@
     {
         try
         {
+            //delegate password changes to the Principal Database
             return _principalDatabase.updatePassword(new UsernamePrincipal(username), password);
         }
         catch (AccountNotFoundException e)
@@ -83,37 +130,52 @@
 
         if (_accessRights.get(username) == null)
         {
+            // If the user doesn't exist in the user rights file check that they at least have an account.
             if (_principalDatabase.getUser(username) == null)
             {
                 return false;
             }
         }
 
-        if (admin)
-        {
-            _accessRights.put(username, MBeanInvocationHandlerImpl.ADMIN);
-        }
-        else
+        try
         {
-            if (read | write)
+
+            _accessRightsUpdate.lock();
+
+            // Update the access rights
+            if (admin)
+            {
+                _accessRights.put(username, MBeanInvocationHandlerImpl.ADMIN);
+            }
+            else
             {
-                if (read)
+                if (read | write)
                 {
-                    _accessRights.put(username, MBeanInvocationHandlerImpl.READONLY);
+                    if (read)
+                    {
+                        _accessRights.put(username, MBeanInvocationHandlerImpl.READONLY);
+                    }
+                    if (write)
+                    {
+                        _accessRights.put(username, MBeanInvocationHandlerImpl.READWRITE);
+                    }
                 }
-                if (write)
+                else
                 {
-                    _accessRights.put(username, MBeanInvocationHandlerImpl.READWRITE);
+                    _accessRights.remove(username);
                 }
             }
-            else
+
+            saveAccessFile();
+        }
+        finally
+        {
+            if (_accessRightsUpdate.isHeldByCurrentThread())
             {
-                return false;
+                _accessRightsUpdate.unlock();
             }
         }
 
-        saveAccessFile();
-
         return true;
     }
 
@@ -123,18 +185,11 @@
                               @MBeanOperationParameter(name = "write", description = "Administration write")boolean write,
                               @MBeanOperationParameter(name = "admin", description = "Administration rights")boolean admin)
     {
-        try
+        if (_principalDatabase.createPrincipal(new UsernamePrincipal(username), password))
         {
-            if (_principalDatabase.createPrincipal(new UsernamePrincipal(username), password))
-            {
-                _accessRights.put(username, "");
+            _accessRights.put(username, "");
 
-                return setRights(username, read, write, admin);
-            }
-        }
-        catch (AccountNotFoundException e)
-        {
-            _logger.warn("Attempt to delete user (" + username + ") that doesn't exist");
+            return setRights(username, read, write, admin);
         }
 
         return false;
@@ -147,8 +202,20 @@
         {
             if (_principalDatabase.deletePrincipal(new UsernamePrincipal(username)))
             {
-                _accessRights.remove(username);
+                try
+                {
+                    _accessRightsUpdate.lock();
 
+                    _accessRights.remove(username);
+                    saveAccessFile();
+                }
+                finally
+                {
+                    if (_accessRightsUpdate.isHeldByCurrentThread())
+                    {
+                        _accessRightsUpdate.unlock();
+                    }
+                }
                 return true;
             }
         }
@@ -185,10 +252,56 @@
         }
     }
 
+
     @MBeanOperation(name = "viewUsers", description = "All users with access rights to the system.")
     public TabularData viewUsers()
     {
-        return null;       //todo
+        // 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);
+
+        try
+        {
+            // Create the tabular list of message header contents
+            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};
+                CompositeData messageData = new CompositeDataSupport(_userDataType, _userItemNames, itemData);
+                userList.put(messageData);
+            }
+        }
+        catch (OpenDataException e)
+        {
+            _logger.warn("Unable to create user list due to :" + e);
+            return null;
+        }
+
+        return userList;
     }
 
     /*** Broker Methods **/
@@ -228,38 +341,103 @@
 
     private void loadAccessFile() throws IOException, ConfigurationException
     {
-        Properties accessRights = new Properties();
+        try
+        {
+            _accessRightsUpdate.lock();
 
-        _accessFile = new File(_accessFileName);
+            Properties accessRights = new Properties();
 
-        if (!_accessFile.exists())
-        {
-            throw new ConfigurationException("'" + _accessFileName + "' does not exist");
-        }
+            File accessFile = new File(_accessFileName);
+
+            if (!accessFile.exists())
+            {
+                throw new ConfigurationException("'" + _accessFileName + "' does not exist");
+            }
+
+            if (!accessFile.canRead())
+            {
+                throw new ConfigurationException("Cannot read '" + _accessFileName + "'.");
+            }
+
+            if (!accessFile.canWrite())
+            {
+                _logger.warn("Unable to write to access file '" + _accessFileName + "' changes will not be preserved.");
+            }
 
-        if (!_accessFile.canRead())
+            accessRights.load(new FileInputStream(accessFile));
+            checkAccessRights(accessRights);
+            setAccessRights(accessRights);
+        }
+        finally
         {
-            throw new ConfigurationException("Cannot read '" + _accessFileName + "'.");
+            if (_accessRightsUpdate.isHeldByCurrentThread())
+            {
+                _accessRightsUpdate.unlock();
+            }
         }
+    }
+
+    private void checkAccessRights(Properties accessRights)
+    {
+        Enumeration values = accessRights.propertyNames();
 
-        if (!_accessFile.canWrite())
+        while (values.hasMoreElements())
         {
-            _logger.warn("Unable to write to access file '" + _accessFileName + "' changes will not be preserved.");
-        }
+            String user = (String) values.nextElement();
 
-        accessRights.load(new FileInputStream(_accessFileName));
-        processAccessRights(accessRights);
+            if (_principalDatabase.getUser(user) == null)
+            {
+                _logger.warn("Access rights contains user '" + user + "' but there is no authentication data for that user");
+            }
+        }
     }
 
     private void saveAccessFile()
     {
         try
         {
-            _accessRights.store(new FileOutputStream(_accessFile), "");
+            _accessRightsUpdate.lock();
+            try
+            {
+                // remove old temporary file
+                File tmp = new File(_accessFileName + ".tmp");
+                if (tmp.exists())
+                {
+                    tmp.delete();
+                }
+
+                //remove old backup
+                File old = new File(_accessFileName + ".old");
+                if (old.exists())
+                {
+                    old.delete();
+                }
+
+                // Rename current file
+                File rights = new File(_accessFileName);
+                rights.renameTo(old);
+
+                FileOutputStream output = new FileOutputStream(tmp);
+                _accessRights.store(output, "");
+                output.close();
+
+                // Rename new file to main file
+                tmp.renameTo(rights);
+
+                // delete tmp
+                tmp.delete();
+            }
+            catch (IOException e)
+            {
+                _logger.warn("Problem occured saving '" + _accessFileName + "' changes may not be preserved. :" + e);
+            }
         }
-        catch (IOException e)
+        finally
         {
-            _logger.warn("Unable to write to access file '" + _accessFileName + "' changes will not be preserved.");
+            if (_accessRightsUpdate.isHeldByCurrentThread())
+            {
+                _accessRightsUpdate.unlock();
+            }
         }
     }
 
@@ -268,9 +446,9 @@
      *
      * @param accessRights The properties list of access rights to process
      */
-    private void processAccessRights(Properties accessRights)
+    private void setAccessRights(Properties accessRights)
     {
-        _logger.info("Processing Access Rights:" + accessRights);
+        _logger.debug("Setting Access Rights:" + accessRights);
         _accessRights = accessRights;
         MBeanInvocationHandlerImpl.setAccessRights(_accessRights);
     }

Modified: incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabase.java
URL: http://svn.apache.org/viewvc/incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabase.java?view=diff&rev=527843&r1=527842&r2=527843
==============================================================================
--- incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabase.java (original)
+++ incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabase.java Thu Apr 12 01:52:19 2007
@@ -25,26 +25,24 @@
 import org.apache.qpid.server.security.auth.sasl.UsernamePrincipal;
 import org.apache.qpid.server.security.auth.sasl.crammd5.CRAMMD5HashedInitialiser;
 import org.apache.qpid.server.security.access.AMQUserManagementMBean;
-import org.apache.qpid.server.security.Passwd;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.codec.EncoderException;
 
 import javax.security.auth.callback.PasswordCallback;
 import javax.security.auth.login.AccountNotFoundException;
-import javax.management.JMException;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.BufferedReader;
 import java.io.FileReader;
 import java.io.UnsupportedEncodingException;
-import java.io.BufferedWriter;
-import java.io.FileWriter;
 import java.io.PrintStream;
 import java.util.regex.Pattern;
 import java.util.Map;
 import java.util.HashMap;
 import java.util.List;
+import java.util.LinkedList;
+import java.util.concurrent.locks.ReentrantLock;
 import java.security.Principal;
 import java.security.NoSuchAlgorithmException;
 import java.security.MessageDigest;
@@ -69,6 +67,7 @@
     AMQUserManagementMBean _mbean;
     private static final String DEFAULT_ENCODING = "utf-8";
     private Map<String, User> _users = new HashMap<String, User>();
+    private ReentrantLock _userUpdate = new ReentrantLock();
 
     public Base64MD5PasswordFilePrincipalDatabase()
     {
@@ -113,6 +112,14 @@
         loadPasswordFile();
     }
 
+    /**
+     * SASL Callback Mechanism - sets the Password in the PasswordCallback based on the value in the PasswordFile
+     *
+     * @param principal The Principal to set the password for
+     * @param callback  The PasswordCallback to call setPassword on
+     *
+     * @throws AccountNotFoundException If the Principal cannont be found in this Database
+     */
     public void setPassword(Principal principal, PasswordCallback callback) throws AccountNotFoundException
     {
         if (_passwordFile == null)
@@ -136,11 +143,21 @@
         }
     }
 
-    public boolean verifyPassword(Principal principal, String password) throws AccountNotFoundException
+    /**
+     * Used to verify that the presented Password is correct. Currently only used by Management Console
+     *
+     * @param principal The principal to authenticate
+     * @param password  The password to check
+     *
+     * @return true if password is correct
+     *
+     * @throws AccountNotFoundException if the principal cannot be found
+     */
+    public boolean verifyPassword(String principal, String password) throws AccountNotFoundException
     {
         try
         {
-            char[] pwd = lookupPassword(principal.getName());
+            char[] pwd = lookupPassword(principal);
             byte[] passwordBytes = password.getBytes(DEFAULT_ENCODING);
 
             int index = 0;
@@ -173,19 +190,30 @@
 
             char[] passwd = convertPassword(password);
 
-            user.setPassword(passwd);
-
             try
             {
-                savePasswordFile();
+                _userUpdate.lock();
+                user.setPassword(passwd);
+
+                try
+                {
+                    savePasswordFile();
+                }
+                catch (IOException e)
+                {
+                    _logger.error("Unable to save password file, password change for user'"
+                                  + principal + "' will revert at restart");
+                    return false;
+                }
+                return true;
             }
-            catch (IOException e)
+            finally
             {
-                _logger.error("Unable to save password file, password change for user'"
-                              + principal + "' will revert at restart");
-                return false;
+                if (_userUpdate.isHeldByCurrentThread())
+                {
+                    _userUpdate.unlock();
+                }
             }
-            return true;
         }
         catch (UnsupportedEncodingException e)
         {
@@ -209,14 +237,14 @@
         return passwd;
     }
 
-    public boolean createPrincipal(Principal principal, String password) throws AccountNotFoundException
+    public boolean createPrincipal(Principal principal, String password)
     {
         if (_users.get(principal.getName()) != null)
         {
             return false;
         }
 
-        User user = null;
+        User user;
         try
         {
             user = new User(principal.getName(), convertPassword(password));
@@ -227,18 +255,29 @@
             return false;
         }
 
-        _users.put(user.getName(), user);
-
         try
         {
-            savePasswordFile();
-            return true;
+            _userUpdate.lock();
+            _users.put(user.getName(), user);
+
+            try
+            {
+                savePasswordFile();
+                return true;
+            }
+            catch (IOException e)
+            {
+                return false;
+            }
+
         }
-        catch (IOException e)
+        finally
         {
-            return false;
+            if (_userUpdate.isHeldByCurrentThread())
+            {
+                _userUpdate.unlock();
+            }
         }
-
     }
 
     public boolean deletePrincipal(Principal principal) throws AccountNotFoundException
@@ -250,28 +289,45 @@
             throw new AccountNotFoundException(principal.getName());
         }
 
-        user.delete();
-
         try
         {
-            savePasswordFile();
+            _userUpdate.lock();
+            user.delete();
+
+            try
+            {
+                savePasswordFile();
+            }
+            catch (IOException e)
+            {
+                _logger.warn("Unable to remove user '" + user.getName() + "' from password file.");
+                return false;
+            }
+
+            _users.remove(user.getName());
         }
-        catch (IOException e)
+        finally
         {
-            _logger.warn("Unable to remove user '" + user.getName() + "' from password file.");
-            return false;
+            if (_userUpdate.isHeldByCurrentThread())
+            {
+                _userUpdate.unlock();
+            }
         }
 
-        _users.remove(user.getName());
-
         return true;
     }
 
+
     public Map<String, AuthenticationProviderInitialiser> getMechanisms()
     {
         return _saslServers;
     }
 
+    public List<Principal> getUsers()
+    {
+        return new LinkedList<Principal>(_users.values());
+    }
+
     public Principal getUser(String username)
     {
         if (_users.containsKey(username))
@@ -305,141 +361,164 @@
 
     private void loadPasswordFile() throws IOException
     {
-        _users.clear();
-
-        BufferedReader reader = null;
         try
         {
-            reader = new BufferedReader(new FileReader(_passwordFile));
-            String line;
+            _userUpdate.lock();
+            _users.clear();
 
-            while ((line = reader.readLine()) != null)
+            BufferedReader reader = null;
+            try
             {
-                String[] result = _regexp.split(line);
-                if (result == null || result.length < 2 || result[0].startsWith("#"))
+                reader = new BufferedReader(new FileReader(_passwordFile));
+                String line;
+
+                while ((line = reader.readLine()) != null)
                 {
-                    continue;
-                }
+                    String[] result = _regexp.split(line);
+                    if (result == null || result.length < 2 || result[0].startsWith("#"))
+                    {
+                        continue;
+                    }
 
-                User user = new User(result);
-                _logger.info("Created user:" + user);
-                _users.put(user.getName(), user);
+                    User user = new User(result);
+                    _logger.info("Created user:" + user);
+                    _users.put(user.getName(), user);
+                }
+            }
+            finally
+            {
+                if (reader != null)
+                {
+                    reader.close();
+                }
             }
         }
         finally
         {
-            if (reader != null)
+            if (_userUpdate.isHeldByCurrentThread())
             {
-                reader.close();
+                _userUpdate.unlock();
             }
         }
     }
 
     private void savePasswordFile() throws IOException
     {
-        BufferedReader reader = null;
-        PrintStream writer = null;
-        File tmp = new File(_passwordFile.getAbsolutePath() + ".tmp");
-        if (tmp.exists())
-        {
-            tmp.delete();
-        }
         try
         {
-            writer = new PrintStream(tmp);
-            reader = new BufferedReader(new FileReader(_passwordFile));
-            String line;
+            _userUpdate.lock();
 
-            while ((line = reader.readLine()) != null)
+            BufferedReader reader = null;
+            PrintStream writer = null;
+            File tmp = new File(_passwordFile.getAbsolutePath() + ".tmp");
+            if (tmp.exists())
             {
-                String[] result = _regexp.split(line);
-                if (result == null || result.length < 2 || result[0].startsWith("#"))
+                tmp.delete();
+            }
+            try
+            {
+                writer = new PrintStream(tmp);
+                reader = new BufferedReader(new FileReader(_passwordFile));
+                String line;
+
+                while ((line = reader.readLine()) != null)
                 {
-                    writer.write(line.getBytes(DEFAULT_ENCODING));
-                    continue;
-                }
+                    String[] result = _regexp.split(line);
+                    if (result == null || result.length < 2 || result[0].startsWith("#"))
+                    {
+                        writer.write(line.getBytes(DEFAULT_ENCODING));
+                        continue;
+                    }
 
-                User user = _users.get(result[0]);
+                    User user = _users.get(result[0]);
 
-                if (user == null)
-                {
-                    writer.write(line.getBytes(DEFAULT_ENCODING));
-                    writer.println();
-                }
-                else if (!user.isDeleted())
-                {
-                    if (!user.isModified())
+                    if (user == null)
                     {
                         writer.write(line.getBytes(DEFAULT_ENCODING));
                         writer.println();
                     }
-                    else
+                    else if (!user.isDeleted())
                     {
-                        try
+                        if (!user.isModified())
                         {
-                            byte[] encodedPassword = user.getEncodePassword();
+                            writer.write(line.getBytes(DEFAULT_ENCODING));
+                            writer.println();
+                        }
+                        else
+                        {
+                            try
+                            {
+                                byte[] encodedPassword = user.getEncodePassword();
+
+                                writer.write((user.getName() + ":").getBytes(DEFAULT_ENCODING));
+                                writer.write(encodedPassword);
+                                writer.println();
+
+                                user.saved();
+                            }
+                            catch (Exception e)
+                            {
+                                _logger.warn("Unable to encode new password reverting to old password.");
+                                writer.write(line.getBytes(DEFAULT_ENCODING));
+                                writer.println();
+                            }
+                        }
+                    }
+                }
 
+                for (User user : _users.values())
+                {
+                    if (user.isModified())
+                    {
+                        byte[] encodedPassword;
+                        try
+                        {
+                            encodedPassword = user.getEncodePassword();
                             writer.write((user.getName() + ":").getBytes(DEFAULT_ENCODING));
                             writer.write(encodedPassword);
                             writer.println();
-
                             user.saved();
                         }
                         catch (Exception e)
                         {
-                            _logger.warn("Unable to encode new password reverting to old password.");
-                            writer.write(line.getBytes(DEFAULT_ENCODING));
-                            writer.println();
+                            _logger.warn("Unable to get Encoded password for user'" + user.getName() + "' password not saved");
                         }
                     }
                 }
             }
-
-            for (User user : _users.values())
+            finally
             {
-                if (user.isModified())
+                if (reader != null)
                 {
-                    byte[] encodedPassword;
-                    try
-                    {
-                        encodedPassword = user.getEncodePassword();
-                        writer.write((user.getName() + ":").getBytes(DEFAULT_ENCODING));
-                        writer.write(encodedPassword);
-                        writer.println();
-                        user.saved();
-                    }
-                    catch (Exception e)
-                    {
-                        _logger.warn("Unable to get Encoded password for user'" + user.getName() + "' password not saved");
-                    }
+                    reader.close();
                 }
+
+                if (writer != null)
+                {
+                    writer.close();
+                }
+
+                // Swap temp file to main password file.
+                File old = new File(_passwordFile.getAbsoluteFile() + ".old");
+                if (old.exists())
+                {
+                    old.delete();
+                }
+                _passwordFile.renameTo(old);
+                tmp.renameTo(_passwordFile);
+                tmp.delete();
             }
         }
         finally
         {
-            if (reader != null)
-            {
-                reader.close();
-            }
-
-            if (writer != null)
+            if (_userUpdate.isHeldByCurrentThread())
             {
-                writer.close();
+                _userUpdate.unlock();
             }
-
-            // Swap temp file to main password file.
-            File old = new File(_passwordFile.getAbsoluteFile() + ".old");
-            if (old.exists())
-            {
-                old.delete();
-            }
-            _passwordFile.renameTo(old);
-            tmp.renameTo(_passwordFile);
-            tmp.delete();
         }
     }
 
-    private class User
+    private class User implements Principal
     {
         String _name;
         char[] _password;
@@ -478,14 +557,21 @@
             setPassword(password);
         }
 
-        String getName()
+        public String getName()
         {
             return _name;
         }
 
         public String toString()
         {
-            return getName() + ((_encodedPassword == null) ? "" : ":" + _encodedPassword);
+            if (_logger.isDebugEnabled())
+            {
+                return getName() + ((_encodedPassword == null) ? "" : ":" + new String(_encodedPassword));
+            }
+            else
+            {
+                return _name;
+            }
         }
 
         char[] getPassword()
@@ -547,7 +633,7 @@
                 md.update(b);
             }
 
-            return  md.digest();
+            return md.digest();
         }
     }
 }

Modified: incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PlainPasswordFilePrincipalDatabase.java
URL: http://svn.apache.org/viewvc/incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PlainPasswordFilePrincipalDatabase.java?view=diff&rev=527843&r1=527842&r2=527843
==============================================================================
--- incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PlainPasswordFilePrincipalDatabase.java (original)
+++ incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PlainPasswordFilePrincipalDatabase.java Thu Apr 12 01:52:19 2007
@@ -21,7 +21,6 @@
 package org.apache.qpid.server.security.auth.database;
 
 import org.apache.log4j.Logger;
-import org.apache.qpid.server.security.auth.database.PrincipalDatabase;
 import org.apache.qpid.server.security.auth.sasl.AuthenticationProviderInitialiser;
 import org.apache.qpid.server.security.auth.sasl.UsernamePrincipal;
 import org.apache.qpid.server.security.auth.sasl.amqplain.AmqPlainInitialiser;
@@ -39,6 +38,7 @@
 import java.util.regex.Pattern;
 import java.util.Map;
 import java.util.HashMap;
+import java.util.List;
 import java.security.Principal;
 
 /**
@@ -121,11 +121,11 @@
         }
     }
 
-    public boolean verifyPassword(Principal principal, String password) throws AccountNotFoundException
+    public boolean verifyPassword(String principal, String password) throws AccountNotFoundException
     {
         try
         {
-            char[] pwd = lookupPassword(principal.getName());
+            char[] pwd = lookupPassword(principal);
 
             return compareCharArray(pwd, convertPassword(password));
         }
@@ -156,7 +156,7 @@
         return false; // updates denied
     }
 
-    public boolean createPrincipal(Principal principal, String password) throws AccountNotFoundException
+    public boolean createPrincipal(Principal principal, String password)
     {
         return false; // updates denied
     }
@@ -171,6 +171,11 @@
         return _saslServers;
     }
 
+    public List<Principal> getUsers()
+    {
+        return null; //todo
+    }
+
     public Principal getUser(String username)
     {
         try
@@ -208,11 +213,11 @@
      * Looks up the password for a specified user in the password file. Note this code is <b>not</b> secure since it
      * creates strings of passwords. It should be modified to create only char arrays which get nulled out.
      *
-     * @param name
+     * @param name the name of the principal to lookup
      *
-     * @return
+     * @return char[] of the password
      *
-     * @throws java.io.IOException
+     * @throws java.io.IOException whilst accessing the file
      */
     private char[] lookupPassword(String name) throws IOException
     {

Modified: incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PrincipalDatabase.java
URL: http://svn.apache.org/viewvc/incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PrincipalDatabase.java?view=diff&rev=527843&r1=527842&r2=527843
==============================================================================
--- incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PrincipalDatabase.java (original)
+++ incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PrincipalDatabase.java Thu Apr 12 01:52:19 2007
@@ -26,6 +26,7 @@
 import java.io.UnsupportedEncodingException;
 import java.security.Principal;
 import java.util.Map;
+import java.util.List;
 
 import javax.security.auth.callback.PasswordCallback;
 import javax.security.auth.login.AccountNotFoundException;
@@ -47,31 +48,53 @@
     void setPassword(Principal principal, PasswordCallback callback)
             throws IOException, AccountNotFoundException;
 
-    /**
-     * Set the password for a given principal in the specified callback. This is used for certain SASL providers. The
-     * user database implementation should look up the password in any way it chooses and set it in the callback by
-     * calling its setPassword method.
-     *
-     * @param principal the principal
-     * @param password  the password to be verified
-     *
-     * @return true if the account is verified.
-     *
-     * @throws AccountNotFoundException if the account for specified principal could not be found
+     /**
+     * Used to verify that the presented Password is correct. Currently only used by Management Console
+     * @param principal The principal to authenticate
+     * @param password The password to check
+     * @return true if password is correct
+     * @throws AccountNotFoundException if the principal cannot be found
      */
-    boolean verifyPassword(Principal principal, String password)
+    boolean verifyPassword(String principal, String password)
             throws AccountNotFoundException;
 
+    /**
+     * Update(Change) the password for the given principal
+     * @param principal Who's password is to be changed
+     * @param password The new password to use
+     * @return True if change was successful
+     * @throws AccountNotFoundException If the given principal doesn't exist in the Database
+     */
     boolean updatePassword(Principal principal, String password)
             throws AccountNotFoundException;
 
-    boolean createPrincipal(Principal principal, String password)
-            throws AccountNotFoundException;
+    /**
+     * Create a new principal in the database
+     * @param principal The principal to create
+     * @param password The password to set for the principal
+     * @return True on a successful creation
+     */
+    boolean createPrincipal(Principal principal, String password);
 
+    /**
+     * Delete a principal
+     * @param principal The principal to delete
+     * @return True on a successful creation
+     * @throws AccountNotFoundException If the given principal doesn't exist in the Database
+     */
     boolean deletePrincipal(Principal principal)
             throws AccountNotFoundException;
 
+    /**
+     * Get the principal from the database with the given username
+     * @param username of the principal to lookup
+     * @return The Principal object for the given username or null if not found.
+     */
+    Principal getUser(String username);
+
+
     public Map<String, AuthenticationProviderInitialiser> getMechanisms();
 
-    Principal getUser(String username);
+
+    List<Principal> getUsers();
 }

Modified: incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PropertiesPrincipalDatabase.java
URL: http://svn.apache.org/viewvc/incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PropertiesPrincipalDatabase.java?view=diff&rev=527843&r1=527842&r2=527843
==============================================================================
--- incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PropertiesPrincipalDatabase.java (original)
+++ incubator/qpid/branches/M2/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PropertiesPrincipalDatabase.java Thu Apr 12 01:52:19 2007
@@ -30,6 +30,7 @@
 import java.util.Properties;
 import java.util.Map;
 import java.util.HashMap;
+import java.util.List;
 import java.security.Principal;
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
@@ -78,9 +79,9 @@
         }
     }
 
-    public boolean verifyPassword(Principal principal, String password) throws AccountNotFoundException
+    public boolean verifyPassword(String principal, String password) throws AccountNotFoundException
     {
-        char[] pwd = _users.getProperty(principal.getName()).toCharArray();
+        char[] pwd = _users.getProperty(principal).toCharArray();
 
         try
         {
@@ -97,7 +98,7 @@
         return false; // updates denied
     }
 
-    public boolean createPrincipal(Principal principal, String password) throws AccountNotFoundException
+    public boolean createPrincipal(Principal principal, String password)
     {
         return false; // updates denied
     }
@@ -143,6 +144,11 @@
     public Map<String, AuthenticationProviderInitialiser> getMechanisms()
     {
         return _saslServers;
+    }
+
+    public List<Principal> getUsers()
+    {
+        return null; //todo
     }
 
     public Principal getUser(String username)