QPID-1503 : Updates based on code review

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

4 files changed, 21 insertions, 89 deletions

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 86a0b7d961..a2a0be926e 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
@@ -61,7 +61,7 @@ public class Base64MD5PasswordFilePrincipalDatabase implements PrincipalDatabase
 
     AMQUserManagementMBean _mbean;
     public static final String DEFAULT_ENCODING = "utf-8";
-    private Map<String, User> _users = new HashMap<String, User>();
+    private Map<String, HashedUser> _users = new HashMap<String, HashedUser>();
     private ReentrantLock _userUpdate = new ReentrantLock();
 
     public Base64MD5PasswordFilePrincipalDatabase()
@@ -165,7 +165,7 @@ public class Base64MD5PasswordFilePrincipalDatabase implements PrincipalDatabase
 
     public boolean updatePassword(Principal principal, char[] password) throws AccountNotFoundException
     {
-        User user = _users.get(principal.getName());
+        HashedUser user = _users.get(principal.getName());
 
         if (user == null)
         {
@@ -215,7 +215,7 @@ public class Base64MD5PasswordFilePrincipalDatabase implements PrincipalDatabase
             return false;
         }
 
-        User user = new User(principal.getName(), password);
+        HashedUser user = new HashedUser(principal.getName(), password);
 
         try
         {
@@ -245,7 +245,7 @@ public class Base64MD5PasswordFilePrincipalDatabase implements PrincipalDatabase
 
     public boolean deletePrincipal(Principal principal) throws AccountNotFoundException
     {
-        User user = _users.get(principal.getName());
+        HashedUser user = _users.get(principal.getName());
 
         if (user == null)
         {
@@ -309,7 +309,7 @@ public class Base64MD5PasswordFilePrincipalDatabase implements PrincipalDatabase
      */
     private char[] lookupPassword(String name)
     {
-        User user = _users.get(name);
+        HashedUser user = _users.get(name);
         if (user == null)
         {
             return null;
@@ -341,7 +341,7 @@ public class Base64MD5PasswordFilePrincipalDatabase implements PrincipalDatabase
                         continue;
                     }
 
-                    User user = new User(result);
+                    HashedUser user = new HashedUser(result);
                     _logger.info("Created user:" + user);
                     _users.put(user.getName(), user);
                 }
@@ -393,7 +393,7 @@ public class Base64MD5PasswordFilePrincipalDatabase implements PrincipalDatabase
                         continue;
                     }
 
-                    User user = _users.get(result[0]);
+                    HashedUser user = _users.get(result[0]);
 
                     if (user == null)
                     {
@@ -411,7 +411,7 @@ public class Base64MD5PasswordFilePrincipalDatabase implements PrincipalDatabase
                         {
                             try
                             {
-                                byte[] encodedPassword = user.getEncodePassword();
+                                byte[] encodedPassword = user.getEncodedPassword();
 
                                 writer.write((user.getName() + ":").getBytes(DEFAULT_ENCODING));
                                 writer.write(encodedPassword);
@@ -429,14 +429,14 @@ public class Base64MD5PasswordFilePrincipalDatabase implements PrincipalDatabase
                     }
                 }
 
-                for (User user : _users.values())
+                for (HashedUser user : _users.values())
                 {
                     if (user.isModified())
                     {
                         byte[] encodedPassword;
                         try
                         {
-                            encodedPassword = user.getEncodePassword();
+                            encodedPassword = user.getEncodedPassword();
                             writer.write((user.getName() + ":").getBytes(DEFAULT_ENCODING));
                             writer.write(encodedPassword);
                             writer.println();
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/User.java b/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/HashedUser.java
index 9ff9cd25cb..723a1c0cc6 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/User.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/HashedUser.java
@@ -8,9 +8,9 @@ import java.io.UnsupportedEncodingException;
 import java.security.NoSuchAlgorithmException;
 import java.security.Principal;
 
-public class User implements Principal
+public class HashedUser implements Principal
 {
-    private static final Logger _logger = Logger.getLogger(User.class);
+    private static final Logger _logger = Logger.getLogger(HashedUser.class);
 
     String _name;
     char[] _password;
@@ -18,7 +18,7 @@ public class User implements Principal
     private boolean _modified = false;
     private boolean _deleted = false;
 
-    User(String[] data) throws UnsupportedEncodingException
+    HashedUser(String[] data) throws UnsupportedEncodingException
     {
         if (data.length != 2)
         {
@@ -43,7 +43,7 @@ public class User implements Principal
         }
     }
 
-    public User(String name, char[] password)
+    public HashedUser(String name, char[] password)
     {
         _name = name;
         setPassword(password);
@@ -56,14 +56,7 @@ public class User implements Principal
 
     public String toString()
     {
-        if (_logger.isDebugEnabled())
-        {
-            return getName() + ((_encodedPassword == null) ? "" : ":" + new String(_encodedPassword));
-        }
-        else
-        {
-            return _name;
-        }
+        return _name;
     }
 
     char[] getPassword()
@@ -78,7 +71,7 @@ public class User implements Principal
         _encodedPassword = null;
     }
 
-    byte[] getEncodePassword() throws EncoderException, UnsupportedEncodingException, NoSuchAlgorithmException
+    byte[] getEncodedPassword() throws EncoderException, UnsupportedEncodingException, NoSuchAlgorithmException
     {
         if (_encodedPassword == null)
         {
diff --git a/java/broker/src/test/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabaseTest.java b/java/broker/src/test/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabaseTest.java
index 5a74160b95..ededb1cb26 100644
--- a/java/broker/src/test/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabaseTest.java
+++ b/java/broker/src/test/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabaseTest.java
@@ -38,44 +38,19 @@ public class Base64MD5PasswordFilePrincipalDatabaseTest extends TestCase
 {
 
     Base64MD5PasswordFilePrincipalDatabase _database;
-    private String QPID_HOME;
     private static final String TEST_COMMENT = "# Test Comment";
     private String USERNAME = "testUser";
-    private static final String TEST_FILE_NAME = "B64Test.tmp";
 
     public void setUp()
     {
         _database = new Base64MD5PasswordFilePrincipalDatabase();
-
-        QPID_HOME = System.getProperty("QPID_HOME");
-
-        assertNotNull("QPID_HOME not set", QPID_HOME);
-    }
-
-    public void tearDown()
-    {
-        File testFile = new File(QPID_HOME + File.separator + TEST_FILE_NAME);
-        if (testFile.exists())
-        {
-            testFile.delete();
-        }
-
-        testFile = new File(QPID_HOME + File.separator + TEST_FILE_NAME + ".old");
-        if (testFile.exists())
-        {
-            testFile.delete();
-        }
     }
 
     private File createPasswordFile(int commentLines, int users)
     {
         try
         {
-            File testFile = new File(QPID_HOME + File.separator + TEST_FILE_NAME);
-            if (testFile.exists())
-            {
-                testFile.delete();
-            }
+            File testFile = File.createTempFile("Base64MD5PDPDTest","tmp");
             testFile.deleteOnExit();
 
             BufferedWriter writer = new BufferedWriter(new FileWriter(testFile));
diff --git a/java/broker/src/test/java/org/apache/qpid/server/security/auth/database/UserTest.java b/java/broker/src/test/java/org/apache/qpid/server/security/auth/database/HashedUserTest.java
index 99902ef4c5..a7d951cb5b 100644
--- a/java/broker/src/test/java/org/apache/qpid/server/security/auth/database/UserTest.java
+++ b/java/broker/src/test/java/org/apache/qpid/server/security/auth/database/HashedUserTest.java
@@ -29,7 +29,7 @@ import java.io.UnsupportedEncodingException;
 /*
     Note User is mainly tested by Base64MD5PFPDTest this is just to catch the extra methods
  */
-public class UserTest extends TestCase
+public class HashedUserTest extends TestCase
 {
 
     String USERNAME = "username";
@@ -40,7 +40,7 @@ public class UserTest extends TestCase
     {
         try
         {
-            User user = new User(new String[]{USERNAME, PASSWORD, USERNAME});
+            HashedUser user = new HashedUser(new String[]{USERNAME, PASSWORD, USERNAME});
             fail("Error expected");
         }
         catch (IllegalArgumentException e)
@@ -57,7 +57,7 @@ public class UserTest extends TestCase
     {
         try
         {
-            User user = new User(new String[]{USERNAME, HASHED_PASSWORD});
+            HashedUser user = new HashedUser(new String[]{USERNAME, HASHED_PASSWORD});
             assertEquals("Username incorrect", USERNAME, user.getName());
             int index = 0;
 
@@ -65,7 +65,7 @@ public class UserTest extends TestCase
 
             try
             {
-                for (byte c : user.getEncodePassword())
+                for (byte c : user.getEncodedPassword())
                 {
                     assertEquals("Password incorrect", hash[index], (char) c);
                     index++;
@@ -91,41 +91,5 @@ public class UserTest extends TestCase
             fail(e.getMessage());
         }
     }
-
-    public void testToString()
-    {
-
-        User user = new User(USERNAME, PASSWORD.toCharArray());
-
-        // Test logger debug case
-        Logger.getLogger(User.class).setLevel(Level.DEBUG);
-
-        assertEquals("User toString encoding not as expected", USERNAME, user.toString());
-
-        try
-        {
-            char[] hash = HASHED_PASSWORD.toCharArray();
-            int index = 0;
-            for (byte c : user.getEncodePassword())
-            {
-
-                assertEquals("Hash not as expected", hash[index], (char) c);
-                index++;
-            }
-        }
-        catch (Exception e)
-        {
-            fail(e.getMessage());
-        }
-
-        assertEquals("User toString encoding not as expected", USERNAME + ":" + HASHED_PASSWORD,
-                     user.toString());
-
-         Logger.getLogger(User.class).setLevel(Level.INFO);
-
-        // Test normal case
-        assertEquals("User toString encoding not as expected", USERNAME, user.toString());
-    }
-
 }