From 89ca1ad6409b0cfbd59d8363630cd4d67fc45ad6 Mon Sep 17 00:00:00 2001
From: Jeff Burke <Jeff.Burke@nrc-cnrc.gc.ca>
Date: Wed, 10 Sep 2014 13:08:01 -0700
Subject: [PATCH] s1651: removed test tree from LdapConfig, made owner optional
 in Group and updated tests

---
 projects/cadcAccessControl-Server/build.xml   |   7 +-
 .../nrc/cadc/ac/server/ldap/LdapConfig.java   |  26 +-
 .../ca/nrc/cadc/ac/server/ldap/LdapDAO.java   |   2 +-
 .../nrc/cadc/ac/server/ldap/LdapGroupDAO.java | 325 ++++++------
 .../nrc/cadc/ac/server/ldap/LdapUserDAO.java  |  40 +-
 .../nrc/cadc/ac/server/ldap/LdapDAOTest.java  |  20 +-
 .../cadc/ac/server/ldap/LdapGroupDAOTest.java | 499 ++++++++++++++----
 .../cadc/ac/server/ldap/LdapUserDAOTest.java  |  44 +-
 .../ac/server/web/GroupActionFactoryTest.java |   1 +
 .../src/ca/nrc/cadc/ac/ActivatedGroup.java    |  11 +-
 .../src/ca/nrc/cadc/ac/Group.java             |  25 +-
 .../src/ca/nrc/cadc/ac/GroupReader.java       |  22 +-
 .../src/ca/nrc/cadc/ac/GroupWriter.java       |  11 +-
 .../ca/nrc/cadc/ac/GroupReaderWriterTest.java |  17 +-
 .../test/src/ca/nrc/cadc/ac/GroupTest.java    |  72 +--
 15 files changed, 747 insertions(+), 375 deletions(-)

diff --git a/projects/cadcAccessControl-Server/build.xml b/projects/cadcAccessControl-Server/build.xml
index 61e94bf0..bad1d7b6 100644
--- a/projects/cadcAccessControl-Server/build.xml
+++ b/projects/cadcAccessControl-Server/build.xml
@@ -141,11 +141,10 @@
                 <pathelement path="${build}/test/class"/>
                 <pathelement path="${testingJars}"/>
             </classpath>
-            <!--<test name="ca.nrc.cadc.ac.server.ldap.LdapDAOTest" />-->
-            <!--<test name="ca.nrc.cadc.ac.server.ldap.LdapDAOTestImpl" />-->
+            <test name="ca.nrc.cadc.ac.server.ldap.LdapDAOTest" />
             <test name="ca.nrc.cadc.ac.server.ldap.LdapGroupDAOTest" />
-            <!--<test name="ca.nrc.cadc.ac.server.web.GroupActionFactoryTest" />-->
-            <!--<test name="ca.nrc.cadc.ac.server.ldap.LdapUserDAOTest" />-->
+            <test name="ca.nrc.cadc.ac.server.web.GroupActionFactoryTest" />
+            <test name="ca.nrc.cadc.ac.server.ldap.LdapUserDAOTest" />
             <formatter type="plain" usefile="false" />
         </junit>
     </target>
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapConfig.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapConfig.java
index e0de1381..eadbd826 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapConfig.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapConfig.java
@@ -86,11 +86,9 @@ public class LdapConfig
     public static final String LDAP_PASSWD = "passwd";
     public static final String LDAP_USERS_DN = "usersDn";
     public static final String LDAP_GROUPS_DN = "groupsDn";
-    public static final String LDAP_DELETED_GROUPS_DN = "deletedGroupsDn";
 
     private String usersDN;
     private String groupsDN;
-    private String deletedGroupsDN;
     private String server;
     private int port;
     private String adminUserDN;
@@ -159,21 +157,12 @@ public class LdapConfig
                                        LDAP_GROUPS_DN);
         }
 
-        String ldapDeletedGroupsDn = config.getProperty(LDAP_DELETED_GROUPS_DN);
-        if (!StringUtil.hasText(ldapDeletedGroupsDn))
-        {
-            throw new RuntimeException("failed to read property " + 
-                                       LDAP_DELETED_GROUPS_DN);
-        }
-
         return new LdapConfig(server, Integer.valueOf(port), ldapAdmin, 
-                              ldapPasswd, ldapUsersDn, ldapGroupsDn, 
-                              ldapDeletedGroupsDn);
+                              ldapPasswd, ldapUsersDn, ldapGroupsDn);
     }
 
     public LdapConfig(String server, int port, String adminUserDN, 
-                      String adminPasswd, String usersDN, String groupsDN, 
-                      String deletedGroupsDN)
+                      String adminPasswd, String usersDN, String groupsDN)
     {
         if (!StringUtil.hasText(server))
         {
@@ -205,11 +194,6 @@ public class LdapConfig
             throw new IllegalArgumentException("Illegal groups LDAP DN: " + 
                                                groupsDN);
         }
-        if (!StringUtil.hasText(deletedGroupsDN))
-        {
-            throw new IllegalArgumentException("Illegal groups LDAP DN: " + 
-                                               deletedGroupsDN);
-        }
 
         this.server = server;
         this.port = port;
@@ -217,7 +201,6 @@ public class LdapConfig
         this.adminPasswd = adminPasswd;
         this.usersDN = usersDN;
         this.groupsDN = groupsDN;
-        this.deletedGroupsDN = deletedGroupsDN;
     }
 
     public String getUsersDN()
@@ -230,11 +213,6 @@ public class LdapConfig
         return this.groupsDN;
     }
 
-    public String getDeletedGroupsDN()
-    {
-        return this.deletedGroupsDN;
-    }
-
     public String getServer()
     {
         return this.server;
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapDAO.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapDAO.java
index 643f2962..4252ee66 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapDAO.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapDAO.java
@@ -169,7 +169,7 @@ public abstract class LdapDAO
                 throw new AccessControlException(
                         "No LDAP account when search with rule " + ldapField);
             }
-
+            
             subjDN = ((SearchResultEntry) searchResult.getSearchEntries()
                     .get(0)).getAttributeValueAsDN("entrydn");
         }
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java
index 78382666..e1b8af7e 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java
@@ -96,6 +96,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Date;
 import java.util.List;
+import java.util.logging.Level;
 import javax.security.auth.x500.X500Principal;
 import org.apache.log4j.Logger;
 
@@ -128,23 +129,6 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         this.userPersist = userPersist;
     }
 
-    /**
-     * Get the group with the given Group ID.
-     * 
-     * @param groupID The Group unique ID.
-     * 
-     * @return A Group instance
-     * 
-     * @throws GroupNotFoundException If the group was not found.
-     * @throws TransientException  If an temporary, unexpected problem occurred.
-     */
-    public Group getGroup(String groupID)
-        throws GroupNotFoundException, TransientException,
-               AccessControlException
-    {
-        return getGroup(groupID, true);
-    }
-
     /**
      * Creates the group.
      * 
@@ -161,6 +145,11 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         throws GroupAlreadyExistsException, TransientException,
                UserNotFoundException, AccessControlException
     {
+        if (group.getOwner() == null)
+        {
+            throw new IllegalArgumentException("Group owner must be specified");
+        }
+        
         try
         {
             getGroup(group.getID());
@@ -180,7 +169,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
                     {
                        throw new AccessControlException(
                            "Unable to activate group " + group.getID() + 
-                           " because " + group.getOwner().getUserID().getName() 
+                           " because " + getSubjectDN().toString()
                            + " is not the owner"); 
                     }
                     
@@ -189,10 +178,23 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
                         new Modification(ModificationType.DELETE, 
                                          "nsaccountlock");
                     mods.add(mod);
+                    
+                    Group modifiedGroup = modifyGroup(group, inactiveGroup, 
+                                                      mods);
                     Group activatedGroup = 
-                        modifyGroup(group, inactiveGroup, mods);
-                    return new ActivatedGroup(activatedGroup.getID(),
-                                              activatedGroup.getOwner());
+                            new ActivatedGroup(modifiedGroup.getID(),
+                                               modifiedGroup.getOwner());
+                    activatedGroup.description = modifiedGroup.description;
+                    activatedGroup.publicRead = modifiedGroup.publicRead;
+                    activatedGroup.groupRead = modifiedGroup.groupRead;
+                    activatedGroup.groupWrite = modifiedGroup.groupWrite;
+                    activatedGroup.getProperties()
+                            .addAll(modifiedGroup.getProperties());
+                    activatedGroup.getGroupMembers()
+                            .addAll(modifiedGroup.getGroupMembers());
+                    activatedGroup.getUserMembers()
+                            .addAll(modifiedGroup.getUserMembers());
+                    return activatedGroup;
                 }
                 catch (GroupNotFoundException ignore) {}
                 
@@ -300,110 +302,22 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
     }
 
     /**
-     * Deletes the group.
+     * Get the group with the given Group ID.
      * 
-     * @param groupID The group to delete
+     * @param groupID The Group unique ID.
+     * 
+     * @return A Group instance
      * 
      * @throws GroupNotFoundException If the group was not found.
-     * @throws TransientException If an temporary, unexpected problem occurred.
+     * @throws TransientException  If an temporary, unexpected problem occurred.
      */
-    public void deleteGroup(String groupID)
+    public Group getGroup(String groupID)
         throws GroupNotFoundException, TransientException,
                AccessControlException
     {
-        Group group = getGroup(groupID);
-        List<Modification> modifs = new ArrayList<Modification>();
-        modifs.add(new Modification(ModificationType.ADD, "nsaccountlock", "true"));
-        
-        if (group.description != null)
-        {
-            modifs.add(new Modification(ModificationType.DELETE, "description"));
-        }
-        
-        if (group.groupRead != null || 
-            group.groupWrite != null || 
-            group.publicRead)
-        {
-            modifs.add(new Modification(ModificationType.DELETE, "aci"));
-        }
-        
-        if (!group.getGroupMembers().isEmpty() || 
-            !group.getUserMembers().isEmpty())
-        {
-            modifs.add(new Modification(ModificationType.DELETE, "uniquemember"));
-        }
-
-        ModifyRequest modifyRequest = 
-                new ModifyRequest(getGroupDN(group.getID()), modifs);
-        try
-        {
-            modifyRequest.addControl(
-                    new ProxiedAuthorizationV2RequestControl(
-                            "dn:" + getSubjectDN().toNormalizedString()));
-            LDAPResult result = getConnection().modify(modifyRequest);
-        }
-        catch (LDAPException e1)
-        {
-            throw new RuntimeException("LDAP problem", e1);
-        }
-        
-        try
-        {
-            getGroup(group.getID());
-            throw new RuntimeException("BUG: group not deleted " + 
-                                       group.getID());
-        }
-        catch (GroupNotFoundException ignore) {}
-    }
-    
-    /**
-     * Obtain a Collection of Groups that fit the given query.
-     * 
-     * @param userID The userID.
-     * @param role Role of the user, either owner, member, or read/write.
-     * @param groupID The Group ID.
-     * 
-     * @return Collection of Groups
-     *         matching GROUP_READ_ACI.replace(ACTUAL_GROUP_TOKEN,
-     *         readGrDN.toNormalizedString()) the query, or empty
-     *         Collection. Never null.
-     * @throws TransientException  If an temporary, unexpected problem occurred.
-     * @throws UserNotFoundException
-     * @throws GroupNotFoundException
-     */
-    public Collection<Group> searchGroups(T userID, Role role, String groupID)
-        throws TransientException, AccessControlException,
-               GroupNotFoundException, UserNotFoundException
-    {
-        User<T> user = new User<T>(userID);
-        DN userDN;
-        try
-        {   
-            userDN = userPersist.getUserDN(user);
-        }
-        catch (LDAPException e)
-        {
-            // TODO check which LDAP exceptions are transient and which
-            // ones are
-            // access control
-            throw new TransientException("Error getting user", e);
-        }
-        
-        if (role == Role.OWNER)
-        {
-            return getOwnerGroups(user, userDN, groupID);
-        }
-        else if (role == Role.MEMBER)
-        {
-            return getMemberGroups(user, userDN, groupID);
-        }
-        else if (role == Role.RW)
-        {
-            return getRWGroups(user, userDN, groupID);
-        }
-        throw new IllegalArgumentException("Unknown role " + role);
+        return getGroup(groupID, true);
     }
-    
+
     private Group getGroup(String groupID, boolean withMembers)
         throws GroupNotFoundException, TransientException, 
                AccessControlException
@@ -462,17 +376,17 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
                         DN memberDN = new DN(member);
                         if (memberDN.isDescendantOf(config.getUsersDN(), false))
                         {
-                            User<X500Principal> usr;
+                            User<X500Principal> user;
                             try
                             {
-                                usr = userPersist.getMember(memberDN);
+                                user = userPersist.getMember(memberDN);
                             }
                             catch (UserNotFoundException e)
                             {
                                 throw new RuntimeException(
                                     "BUG: group member not found");
                             }
-                            ldapGroup.getUserMembers().add(usr);
+                            ldapGroup.getUserMembers().add(user);
                         }
                         else if (memberDN.isDescendantOf(config.getGroupsDN(),
                                                          false))
@@ -582,7 +496,12 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
             modifs.add(new Modification(ModificationType.DELETE, 
                                         "description"));
         }
-        else if (newGroup.description != null)
+        else if (newGroup.description != null && oldGroup.description == null)
+        {
+            modifs.add(new Modification(ModificationType.ADD, "description", 
+                                        newGroup.description));
+        }
+        else if (newGroup.description != null && oldGroup.description != null)
         {
             modifs.add(new Modification(ModificationType.REPLACE, "description", 
                                         newGroup.description));
@@ -705,54 +624,113 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         }
         catch (GroupNotFoundException e)
         {
-            throw new RuntimeException("BUG: new group not found");
+            throw new RuntimeException("BUG: modified group not found");
         }
     }
 
     /**
-     * Returns a group based on its LDAP DN. The returned group is bared
-     * (contains only group ID, owner and description).
+     * Deletes the group.
      * 
-     * @param groupDN
-     * @return
-     * @throws com.unboundid.ldap.sdk.LDAPException
-     * @throws ca.nrc.cadc.ac.GroupNotFoundException
-     * @throws ca.nrc.cadc.ac.UserNotFoundException
+     * @param groupID The group to delete
+     * 
+     * @throws GroupNotFoundException If the group was not found.
+     * @throws TransientException If an temporary, unexpected problem occurred.
      */
-    protected Group getGroup(DN groupDN)
-        throws LDAPException, GroupNotFoundException, UserNotFoundException
+    public void deleteGroup(String groupID)
+        throws GroupNotFoundException, TransientException,
+               AccessControlException
     {
-        SearchResultEntry searchResult = 
-                getConnection().getEntry(groupDN.toNormalizedString(),
-                                new String[] {"cn", "description", "owner", 
-                                              "modifytimestamp"});
-
-        if (searchResult == null)
+        Group group = getGroup(groupID);
+        List<Modification> modifs = new ArrayList<Modification>();
+        modifs.add(new Modification(ModificationType.ADD, "nsaccountlock", "true"));
+        
+        if (group.description != null)
         {
-            String msg = "Group not found " + groupDN;
-            logger.debug(msg);
-            throw new GroupNotFoundException(groupDN.toNormalizedString());
+            modifs.add(new Modification(ModificationType.DELETE, "description"));
+        }
+        
+        if (group.groupRead != null || 
+            group.groupWrite != null || 
+            group.publicRead)
+        {
+            modifs.add(new Modification(ModificationType.DELETE, "aci"));
+        }
+        
+        if (!group.getGroupMembers().isEmpty() || 
+            !group.getUserMembers().isEmpty())
+        {
+            modifs.add(new Modification(ModificationType.DELETE, "uniquemember"));
         }
 
-        DN ownerDN = searchResult.getAttributeValueAsDN("owner");
-        User<X500Principal> owner = userPersist.getMember(ownerDN);
-        Group group = new Group(searchResult.getAttributeValue("cn"), owner);
-        group.description = searchResult.getAttributeValue("description");
-        group.lastModified = 
-                searchResult.getAttributeValueAsDate("modifytimestamp");
-        return group;
+        ModifyRequest modifyRequest = 
+                new ModifyRequest(getGroupDN(group.getID()), modifs);
+        try
+        {
+            modifyRequest.addControl(
+                    new ProxiedAuthorizationV2RequestControl(
+                            "dn:" + getSubjectDN().toNormalizedString()));
+            LDAPResult result = getConnection().modify(modifyRequest);
+        }
+        catch (LDAPException e1)
+        {
+            throw new RuntimeException("LDAP problem", e1);
+        }
+        
+        try
+        {
+            getGroup(group.getID());
+            throw new RuntimeException("BUG: group not deleted " + 
+                                       group.getID());
+        }
+        catch (GroupNotFoundException ignore) {}
     }
-
-    protected DN getGroupDN(String groupID)
+    
+    /**
+     * Obtain a Collection of Groups that fit the given query.
+     * 
+     * @param userID The userID.
+     * @param role Role of the user, either owner, member, or read/write.
+     * @param groupID The Group ID.
+     * 
+     * @return Collection of Groups
+     *         matching GROUP_READ_ACI.replace(ACTUAL_GROUP_TOKEN,
+     *         readGrDN.toNormalizedString()) the query, or empty
+     *         Collection. Never null.
+     * @throws TransientException  If an temporary, unexpected problem occurred.
+     * @throws UserNotFoundException
+     * @throws GroupNotFoundException
+     */
+    public Collection<Group> searchGroups(T userID, Role role, String groupID)
+        throws TransientException, AccessControlException,
+               GroupNotFoundException, UserNotFoundException
     {
+        User<T> user = new User<T>(userID);
+        DN userDN;
         try
-        {
-            return new DN("cn=" + groupID + "," + config.getGroupsDN());
+        {   
+            userDN = userPersist.getUserDN(user);
         }
         catch (LDAPException e)
         {
+            // TODO check which LDAP exceptions are transient and which
+            // ones are
+            // access control
+            throw new TransientException("Error getting user", e);
         }
-        throw new IllegalArgumentException(groupID + " not a valid group ID");
+        
+        if (role == Role.OWNER)
+        {
+            return getOwnerGroups(user, userDN, groupID);
+        }
+        else if (role == Role.MEMBER)
+        {
+            return getMemberGroups(user, userDN, groupID);
+        }
+        else if (role == Role.RW)
+        {
+            return getRWGroups(user, userDN, groupID);
+        }
+        throw new IllegalArgumentException("Unknown role " + role);
     }
     
     protected Collection<Group> getOwnerGroups(User<T> user, DN userDN,
@@ -1020,7 +998,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         
         if (searchResult == null)
         {
-            String msg = "Group not found " + groupID;
+            String msg = "Inactive Group not found " + groupID;
             logger.debug(msg);
             throw new GroupNotFoundException(msg);
         }
@@ -1033,4 +1011,45 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         return new Group(groupCN, owner);
     }
 
+    /**
+     * Returns a group based on its LDAP DN. The returned group is bare
+     * (contains only group ID, description, modifytimestamp).
+     * 
+     * @param groupDN
+     * @return
+     * @throws com.unboundid.ldap.sdk.LDAPException
+     * @throws ca.nrc.cadc.ac.GroupNotFoundException
+     * @throws ca.nrc.cadc.ac.UserNotFoundException
+     */
+    protected Group getGroup(DN groupDN)
+        throws LDAPException, GroupNotFoundException, UserNotFoundException
+    {
+        SearchResultEntry searchResult = 
+                getConnection().getEntry(groupDN.toNormalizedString(),
+                                new String[] {"cn", "description"});
+
+        if (searchResult == null)
+        {
+            String msg = "Group not found " + groupDN;
+            logger.debug(msg);
+            throw new GroupNotFoundException(groupDN.toNormalizedString());
+        }
+
+        Group group = new Group(searchResult.getAttributeValue("cn"));
+        group.description = searchResult.getAttributeValue("description");
+        return group;
+    }
+
+    protected DN getGroupDN(String groupID)
+    {
+        try
+        {
+            return new DN("cn=" + groupID + "," + config.getGroupsDN());
+        }
+        catch (LDAPException e)
+        {
+        }
+        throw new IllegalArgumentException(groupID + " not a valid group ID");
+    }
+
 }
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java
index 92849897..b4243307 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java
@@ -83,6 +83,7 @@ import com.unboundid.ldap.sdk.SearchRequest;
 import com.unboundid.ldap.sdk.SearchResultEntry;
 import com.unboundid.ldap.sdk.SearchScope;
 import com.unboundid.ldap.sdk.controls.ProxiedAuthorizationV1RequestControl;
+import com.unboundid.ldap.sdk.controls.ProxiedAuthorizationV2RequestControl;
 import java.security.AccessControlException;
 import java.security.Principal;
 import java.util.Collection;
@@ -138,7 +139,8 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
                     new String[] {"cn", "entryid", "entrydn", "dn"});
 
             searchRequest.addControl(
-                    new ProxiedAuthorizationV1RequestControl(getSubjectDN()));
+                    new ProxiedAuthorizationV2RequestControl("dn:" + 
+                            getSubjectDN().toNormalizedString()));
 
             searchResult = getConnection().searchForEntry(searchRequest);
         }
@@ -198,7 +200,8 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
                                       filter, new String[] {"memberOf"});
 
             searchRequest.addControl(
-                    new ProxiedAuthorizationV1RequestControl(getSubjectDN()));
+                    new ProxiedAuthorizationV2RequestControl("dn:" + 
+                            getSubjectDN().toNormalizedString()));
 
             SearchResultEntry searchResult = 
                     getConnection().searchForEntry(searchRequest);
@@ -271,7 +274,8 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
                                       filter, new String[] {"cn"});
 
             searchRequest.addControl(
-                    new ProxiedAuthorizationV1RequestControl(getSubjectDN()));
+                    new ProxiedAuthorizationV2RequestControl("dn:" + 
+                            getSubjectDN().toNormalizedString()));
             
             SearchResultEntry searchResults = 
                     getConnection().searchForEntry(searchRequest);
@@ -310,9 +314,10 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
             CompareRequest compareRequest = 
                     new CompareRequest(userDN.toNormalizedString(), 
                                       "memberOf", groupID);
-
+            
             compareRequest.addControl(
-                    new ProxiedAuthorizationV1RequestControl(getSubjectDN()));
+                    new ProxiedAuthorizationV2RequestControl("dn:" + 
+                            getSubjectDN().toNormalizedString()));
             
             CompareResult compareResult = 
                     getConnection().compare(compareRequest);
@@ -337,10 +342,22 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
     User<X500Principal> getMember(DN userDN)
         throws UserNotFoundException, LDAPException
     {
-        SearchResultEntry searchResult = getConnection().getEntry(
-                userDN.toNormalizedString(), 
-                        (String[]) this.attribType.values().toArray(
-                                new String[this.attribType.values().size()]));
+        Filter filter = 
+            Filter.createEqualityFilter("entrydn", 
+                                        userDN.toNormalizedString());
+        
+        SearchRequest searchRequest = 
+                new SearchRequest(this.config.getUsersDN(), SearchScope.SUB, 
+                                  filter, 
+                                  (String[]) this.attribType.values().toArray(
+                                  new String[this.attribType.values().size()]));
+        
+        searchRequest.addControl(
+                    new ProxiedAuthorizationV2RequestControl("dn:" + 
+                            getSubjectDN().toNormalizedString()));
+        
+        SearchResultEntry searchResult = 
+                getConnection().searchForEntry(searchRequest);
 
         if (searchResult == null)
         {
@@ -371,9 +388,10 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
         SearchRequest searchRequest = 
                 new SearchRequest(this.config.getUsersDN(), SearchScope.SUB, 
                                  searchField, new String[] {"entrydn"});
-
+        
         searchRequest.addControl(
-                new ProxiedAuthorizationV1RequestControl(getSubjectDN()));
+                    new ProxiedAuthorizationV2RequestControl("dn:" + 
+                            getSubjectDN().toNormalizedString()));
 
         SearchResultEntry searchResult = 
                 getConnection().searchForEntry(searchRequest);
diff --git a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapDAOTest.java b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapDAOTest.java
index 2a16ce97..da506f72 100644
--- a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapDAOTest.java
+++ b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapDAOTest.java
@@ -68,6 +68,7 @@
 
 package ca.nrc.cadc.ac.server.ldap;
 
+import static ca.nrc.cadc.ac.server.ldap.LdapGroupDAOTest.config;
 import static org.junit.Assert.assertTrue;
 
 import java.security.PrivilegedExceptionAction;
@@ -84,13 +85,14 @@ import com.unboundid.ldap.sdk.LDAPConnection;
 
 public class LdapDAOTest
 {
-    LdapConfig config = new LdapConfig(
-            "mach275.cadc.dao.nrc.ca",
-            389,
-            "uid=webproxy,ou=administrators,ou=topologymanagement,o=netscaperoot",
-            "go4it", "ou=Users,ou=ds,dc=canfar,dc=net",
-            "ou=Groups,ou=ds,dc=canfar,dc=net",
-            "ou=DeletedGroups,ou=ds,dc=canfar,dc=net");
+    static String server = "mach275.cadc.dao.nrc.ca";
+    static int port = 389;
+    static String adminDN = "uid=webproxy,ou=administrators,ou=topologymanagement,o=netscaperoot";
+    static String adminPW = "go4it";
+    static String userBaseDN = "ou=Users,ou=ds,dc=canfartest,dc=net";
+    static String groupBaseDN = "ou=Groups,ou=ds,dc=canfartest,dc=net";
+    
+    LdapConfig config = new LdapConfig(server, port, adminDN, adminPW, userBaseDN, groupBaseDN);
     
     @Test
     public void testLdapBindConnection() throws Exception
@@ -99,7 +101,7 @@ public class LdapDAOTest
         //LdapUserDAO<X500Principal> userDAO = new LdapUserDAO<X500Principal>();
 
         // User authenticated with HttpPrincipal
-        HttpPrincipal httpPrincipal = new HttpPrincipal("cadcauthtest2");
+        HttpPrincipal httpPrincipal = new HttpPrincipal("CadcDaoTest1");
         Subject subject = new Subject();
 
         subject.getPrincipals().add(httpPrincipal);
@@ -125,7 +127,7 @@ public class LdapDAOTest
                
         
         X500Principal subjPrincipal = new X500Principal(
-                "cn=cadc authtest2 10635,ou=cadc,o=hia");
+                "cn=cadcdaotest1,ou=cadc,o=hia,c=ca");
         subject = new Subject();
         subject.getPrincipals().add(subjPrincipal);
         
diff --git a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java
index 47ae5cac..f03f82a9 100644
--- a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java
+++ b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java
@@ -46,12 +46,17 @@ import javax.security.auth.x500.X500Principal;
 import org.junit.Test;
 
 import ca.nrc.cadc.ac.Group;
+import ca.nrc.cadc.ac.GroupAlreadyExistsException;
 import ca.nrc.cadc.ac.GroupNotFoundException;
 import ca.nrc.cadc.ac.GroupProperty;
 import ca.nrc.cadc.ac.Role;
 import ca.nrc.cadc.ac.User;
+import ca.nrc.cadc.ac.UserNotFoundException;
 import ca.nrc.cadc.util.Log4jInit;
+import java.security.AccessControlException;
+import java.security.Principal;
 import java.util.Collection;
+import java.util.Set;
 import org.apache.log4j.Level;
 import org.apache.log4j.Logger;
 import static org.junit.Assert.fail;
@@ -61,10 +66,31 @@ public class LdapGroupDAOTest
 {
     private static final Logger log = Logger.getLogger(LdapGroupDAOTest.class);
     
-    static User<X500Principal> cadctest;
-    static User<X500Principal> authtest1;
-    static User<X500Principal> authtest2;
-    static User<X500Principal> regtest1;
+    static String server = "mach275.cadc.dao.nrc.ca";
+    static int port = 389;
+    static String adminDN = "uid=webproxy,ou=webproxy,ou=topologymanagement,o=netscaperoot";
+    static String adminPW = "go4it";
+//    static String userBaseDN = "ou=Users,ou=ds,dc=canfartest,dc=net";
+//    static String groupBaseDN = "ou=Groups,ou=ds,dc=canfartest,dc=net";
+    static String userBaseDN = "ou=Users,ou=ds,dc=canfar,dc=net";
+    static String groupBaseDN = "ou=Groups,ou=ds,dc=canfar,dc=net";
+    
+    static String daoTestDN1 = "cn=cadcdaotest1,ou=cadc,o=hia,c=ca";
+    static String daoTestDN2 = "cn=cadcdaotest2,ou=cadc,o=hia,c=ca";
+    static String unknownDN = "cn=foo,ou=cadc,o=hia,c=ca";
+    
+    static X500Principal daoTestPrincipal1;
+    static X500Principal daoTestPrincipal2;
+    static X500Principal unknownPrincipal;
+    static X500Principal adminPrincipal;
+    
+    static User<X500Principal> daoTestUser1;
+    static User<X500Principal> daoTestUser2;
+    static User<X500Principal> unknownUser;
+    static User<X500Principal> adminUser;
+    
+    static Subject authSubject;
+    static Subject anonSubject;
     
     static LdapConfig config;
     
@@ -73,21 +99,24 @@ public class LdapGroupDAOTest
         throws Exception
     {
         Log4jInit.setLevel("ca.nrc.cadc.ac", Level.DEBUG);
+        
+        daoTestPrincipal1 = new X500Principal(daoTestDN1);
+        daoTestPrincipal2 = new X500Principal(daoTestDN2);
+        unknownPrincipal = new X500Principal(unknownDN);
+        adminPrincipal = new X500Principal(adminDN);
 
-        cadctest = new User<X500Principal>(
-                new X500Principal("CN=CADCtest_636,OU=CADC,O=HIA,C=CA"));
-        authtest1 = new User<X500Principal>(
-                new X500Principal("cn=cadc authtest1 10627,ou=cadc,o=hia"));
-        authtest2 = new User<X500Principal>(
-                new X500Principal("cn=cadc authtest2 10635,ou=cadc,o=hia"));
-        regtest1 = new User<X500Principal>(
-                new X500Principal("CN=CADC Regtest1 10577,OU=CADC,O=HIA"));
+        daoTestUser1 = new User<X500Principal>(daoTestPrincipal1);
+        daoTestUser2 = new User<X500Principal>(daoTestPrincipal2);
+        unknownUser = new User<X500Principal>(unknownPrincipal);
+        adminUser = new User<X500Principal>(adminPrincipal);
+        
+        authSubject = new Subject();
+        authSubject.getPrincipals().add(daoTestUser1.getUserID());
+        
+        anonSubject = new Subject();
+        anonSubject.getPrincipals().add(unknownUser.getUserID());
     
-        config = new LdapConfig("mach275.cadc.dao.nrc.ca", 389,
-            "uid=webproxy,ou=administrators,ou=topologymanagement,o=netscaperoot",
-            "go4it", "ou=Users,ou=ds,dc=canfar,dc=net",
-            "ou=TestGroups,ou=ds,dc=canfar,dc=net",
-            "ou=DeletedGroups,ou=ds,dc=canfar,dc=net");
+        config = new LdapConfig(server, port, adminDN, adminPW, userBaseDN, groupBaseDN);
     }
 
     LdapGroupDAO<X500Principal> getGroupDAO()
@@ -98,28 +127,25 @@ public class LdapGroupDAOTest
     
     String getGroupID()
     {
-        return "acs-daotest-group1-" + System.currentTimeMillis();
+        return "CadcDaoTestGroup-" + System.currentTimeMillis();
     }
 
     @Test
     public void testOneGroup() throws Exception
     {
-        Subject subject = new Subject();
-        subject.getPrincipals().add(authtest1.getUserID());
-
         // do everything as owner
-        Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
+        Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
         {
             public Object run() throws Exception
             {
                 try
                 {
-                    Group expectGroup = new Group(getGroupID(), authtest1);
+                    Group expectGroup = new Group(getGroupID(), daoTestUser1);
                     Group actualGroup = getGroupDAO().addGroup(expectGroup);
                     log.debug("addGroup: " + expectGroup.getID());
                     assertGroupsEqual(expectGroup, actualGroup);
                     
-                    Group otherGroup = new Group(getGroupID(), authtest1);
+                    Group otherGroup = new Group(getGroupID(), daoTestUser1);
                     otherGroup = getGroupDAO().addGroup(otherGroup);
                     log.debug("addGroup: " + otherGroup.getID());
 
@@ -137,11 +163,19 @@ public class LdapGroupDAOTest
                     expectGroup.groupRead = otherGroup;
                     actualGroup = getGroupDAO().modifyGroup(expectGroup);
                     assertGroupsEqual(expectGroup, actualGroup);
+                    
+                    expectGroup.groupRead = null;
+                    actualGroup = getGroupDAO().modifyGroup(expectGroup);
+                    assertGroupsEqual(expectGroup, actualGroup);
 
                     // groupWrite
                     expectGroup.groupWrite = otherGroup;
                     actualGroup = getGroupDAO().modifyGroup(expectGroup);
                     assertGroupsEqual(expectGroup, actualGroup);
+                    
+                    expectGroup.groupWrite = null;
+                    actualGroup = getGroupDAO().modifyGroup(expectGroup);
+                    assertGroupsEqual(expectGroup, actualGroup);
 
                     // publicRead
                     expectGroup.publicRead = true;
@@ -153,11 +187,11 @@ public class LdapGroupDAOTest
                     assertGroupsEqual(expectGroup, actualGroup);
 
                     // userMembers
-                    expectGroup.getUserMembers().add(authtest2);
+                    expectGroup.getUserMembers().add(daoTestUser2);
                     actualGroup = getGroupDAO().modifyGroup(expectGroup);
                     assertGroupsEqual(expectGroup, actualGroup);
 
-                    expectGroup.getUserMembers().remove(authtest2);
+                    expectGroup.getUserMembers().remove(daoTestUser2);
                     actualGroup = getGroupDAO().modifyGroup(expectGroup);
                     assertGroupsEqual(expectGroup, actualGroup);
                     
@@ -171,6 +205,13 @@ public class LdapGroupDAOTest
                     assertGroupsEqual(expectGroup, actualGroup);
                     
                     // delete the group
+                    expectGroup.description = "Happy testing";
+                    expectGroup.groupRead = otherGroup;
+                    expectGroup.groupWrite = otherGroup;
+                    expectGroup.publicRead = true;
+                    expectGroup.getUserMembers().add(daoTestUser2);
+                    expectGroup.getGroupMembers().add(otherGroup);
+                    
                     getGroupDAO().deleteGroup(expectGroup.getID());
                     try
                     {
@@ -188,6 +229,9 @@ public class LdapGroupDAOTest
                     actualGroup = getGroupDAO().getGroup(expectGroup.getID());
                     assertGroupsEqual(expectGroup, actualGroup);
                     
+                    // delete the group
+                    getGroupDAO().deleteGroup(expectGroup.getID());
+                    
                     return null;
                 }
                 catch (Exception e)
@@ -199,46 +243,41 @@ public class LdapGroupDAOTest
         });
     }
     
-//    @Test
+    // TODO: add test passing in groupID
+    @Test
     public void testSearchOwnerGroups() throws Exception
     {
-        Subject subject = new Subject();
-        subject.getPrincipals().add(authtest1.getUserID());
-
         // do everything as owner
-        Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
+        Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
         {
             public Object run() throws Exception
             {
                 try
                 {
-                    Group expectGroup = new Group(getGroupID(), authtest1);
-                    Group actualGroup = getGroupDAO().addGroup(expectGroup);
-                    assertGroupsEqual(expectGroup, actualGroup);
-                    System.out.println("new group: " + expectGroup.getID());
+                    Group testGroup = new Group(getGroupID(), daoTestUser1);
+                    testGroup = getGroupDAO().addGroup(testGroup);
                     
                     Collection<Group> groups = 
-                        getGroupDAO().searchGroups(authtest1.getUserID(), 
+                        getGroupDAO().searchGroups(daoTestUser1.getUserID(), 
                                                    Role.OWNER, null);
-                    System.out.println("# groups found: " + groups.size());
+
                     boolean found = false;
                     for (Group group : groups)
                     {
-                        System.out.println("found group: " + group.getID());
-                        if (!group.getOwner().equals(authtest1))
+                        if (!group.getOwner().equals(daoTestUser1))
                         {
                             fail("returned group with wrong owner");
                         }
-                        if (group.getID().equals(expectGroup.getID()))
+                        if (group.getID().equals(group.getID()))
                         {
                             found = true;
                         }
                     }
                     if (!found)
                     {
-                        fail("");
+                        fail("Group for owner not found");
                     }
-                    getGroupDAO().deleteGroup(expectGroup.getID());
+                    getGroupDAO().deleteGroup(testGroup.getID());
                 }
                 catch (Exception e)
                 {
@@ -249,40 +288,54 @@ public class LdapGroupDAOTest
         });
     }
     
+    // TODO: add test passing in groupID
 //    @Test
     public void testSearchMemberGroups() throws Exception
     {
-        Subject subject = new Subject();
-        subject.getPrincipals().add(cadctest.getUserID());
-
         // do everything as owner
-        Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
+        Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
         {
             public Object run() throws Exception
             {
                 try
-                {                    
+                {   
+                    Group memberGroup = new Group(getGroupID(), daoTestUser2);
+                    memberGroup = getGroupDAO().addGroup(memberGroup);
+                    log.debug("member group: " + memberGroup.getID());
+                    
+                    Group testGroup = new Group(getGroupID(), daoTestUser1);
+                    testGroup.getGroupMembers().add(memberGroup);
+                    testGroup = getGroupDAO().addGroup(testGroup);
+                    log.debug("test group: " + testGroup.getID());
+                    
                     Collection<Group> groups = 
-                        getGroupDAO().searchGroups(cadctest.getUserID(), 
+                        getGroupDAO().searchGroups(daoTestUser2.getUserID(), 
                                                    Role.MEMBER, null);
-                    System.out.println("# groups found: " + groups.size());
-//                    boolean found = false;
-//                    for (Group group : groups)
-//                    {
-//                        System.out.println("found group: " + group.getID());
-//                        if (!group.getOwner().equals(cadctest))
-//                        {
-//                            fail("returned group with wrong owner");
-//                        }
-//                        if (group.getID().equals(groupID1))
-//                        {
-//                            found = true;
-//                        }
-//                    }
-//                    if (!found)
-//                    {
-//                        fail("");
-//                    }
+                    
+                    log.debug("# groups found: " + groups.size());
+                    boolean found = false;
+                    for (Group group : groups)
+                    {
+                        log.debug("found group: " + group.getID());
+                        if (group.equals(testGroup))
+                        {
+                            log.debug("found test group: " + group.getID());
+                            Set<Group> members = group.getGroupMembers();
+
+                            log.debug("#test group members: " + members.size());
+                            for (Group member : members)
+                            {
+                                if (member.equals(memberGroup))
+                                {
+                                    found = true;
+                                }
+                            }
+                        }
+                    }
+                    if (!found)
+                    {
+                        fail("Group member not found");
+                    }
                 }
                 catch (Exception e)
                 {
@@ -293,40 +346,52 @@ public class LdapGroupDAOTest
         });
     }
     
+    // TODO: add test passing in groupID
 //    @Test
     public void testSearchRWGroups() throws Exception
     {
-        Subject subject = new Subject();
-        subject.getPrincipals().add(authtest1.getUserID());
-
         // do everything as owner
-        Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
+        Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
         {
             public Object run() throws Exception
             {
                 try
-                {                    
+                {             
+                    Group rwGroup = new Group(getGroupID(), daoTestUser2);
+                    rwGroup = getGroupDAO().addGroup(rwGroup);
+                    log.debug("rw group: " + rwGroup.getID());
+                    
+                    Group testGroup = new Group(getGroupID(), daoTestUser1);
+                    testGroup.groupRead = rwGroup;
+                    testGroup.groupWrite = rwGroup;
+                    testGroup = getGroupDAO().addGroup(testGroup);
+                    log.debug("test group: " + testGroup.getID());
+                    
                     Collection<Group> groups = 
-                        getGroupDAO().searchGroups(authtest1.getUserID(), 
+                        getGroupDAO().searchGroups(daoTestUser2.getUserID(), 
                                                    Role.RW, null);
                     System.out.println("# groups found: " + groups.size());
-//                    boolean found = false;
-//                    for (Group group : groups)
-//                    {
-//                        System.out.println("found group: " + group.getID());
-//                        if (!group.getOwner().equals(authtest1))
-//                        {
-//                            fail("returned group with wrong owner");
-//                        }
-//                        if (group.getID().equals(groupID1))
-//                        {
-//                            found = true;
-//                        }
-//                    }
-//                    if (!found)
-//                    {
-//                        fail("");
-//                    }
+                    
+                    boolean found = false;
+                    for (Group group : groups)
+                    {
+                        System.out.println("found group: " + group.getID());
+                        // get the group to get the owner 
+                        // (not returned for RW groups)
+                        group = getGroupDAO().getGroup(group.getID());
+                        if (!group.getOwner().equals(daoTestUser2))
+                        {
+                            fail("returned group with wrong owner");
+                        }
+                        if (group.getID().equals(testGroup.getID()))
+                        {
+                            found = true;
+                        }
+                    }
+                    if (!found)
+                    {
+                        fail("");
+                    }
                 }
                 catch (Exception e)
                 {
@@ -336,6 +401,261 @@ public class LdapGroupDAOTest
             }
         });
     }
+    
+    @Test
+    public void testAddGroupExceptions() throws Exception
+    {
+        Subject.doAs(anonSubject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+                try
+                {                    
+                    getGroupDAO().addGroup(new Group(getGroupID(), daoTestUser1));
+                    fail("addGroup with anonymous access should throw " + 
+                         "AccessControlException");
+                }
+                catch (AccessControlException ignore) {}
+                return null;
+            }
+        });
+        
+        Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+                try
+                {                    
+                    getGroupDAO().addGroup(new Group("foo", unknownUser));
+                    fail("addGroup with unknown user should throw " + 
+                         "UserNotFoundException");
+                }
+                catch (UserNotFoundException ignore) {}
+                
+                Group group = getGroupDAO().addGroup(new Group(getGroupID(), 
+                                                     daoTestUser1));
+                
+                try
+                {
+                    getGroupDAO().addGroup(group);
+                    fail("addGroup with existing group should throw " + 
+                         "GroupAlreadyExistsException");
+                }
+                catch (GroupAlreadyExistsException ignore) {}
+                
+                getGroupDAO().deleteGroup(group.getID());
+                return null;
+            }
+        });
+    }
+    
+    @Test
+    public void testGetGroupExceptions() throws Exception
+    {
+        final String groupID = getGroupID();
+        
+        Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+                try
+                {                    
+                    getGroupDAO().getGroup(groupID);
+                    fail("getGroup with unknown group should throw " + 
+                         "GroupNotFoundException");
+                }
+                catch (GroupNotFoundException ignore) {}
+                
+                getGroupDAO().addGroup(new Group(groupID, daoTestUser1));
+                return null;
+            }
+        });
+
+        Subject.doAs(anonSubject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+                try
+                {                    
+                    getGroupDAO().getGroup(groupID);
+                    fail("getGroup with anonymous access should throw " + 
+                         "AccessControlException");
+                }
+                catch (AccessControlException ignore) {}
+                return null;
+            }
+        });
+        
+        Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {   
+                getGroupDAO().deleteGroup(groupID);
+                return null;
+            }
+        });
+    }
+    
+    @Test
+    public void testModifyGroupExceptions() throws Exception
+    {        
+        final String groupID = getGroupID();
+        
+        Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+                getGroupDAO().addGroup(new Group(groupID, daoTestUser1));
+                
+//                try
+//                {
+//                    getGroupDAO().modifyGroup(new Group(groupID, unknownUser));
+//                    fail("modifyGroup with unknown user should throw " + 
+//                         "UserNotFoundException");
+//                }
+//                catch (UserNotFoundException ignore) {}
+                
+                try
+                {
+                    getGroupDAO().modifyGroup(new Group("foo", daoTestUser1));
+                    fail("modifyGroup with unknown user should throw " + 
+                         "GroupNotFoundException");
+                }
+                catch (GroupNotFoundException ignore) {}
+
+                return null;
+            }
+        });
+
+        Subject.doAs(anonSubject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+                try
+                {                    
+                    getGroupDAO().getGroup(groupID);
+                    fail("getGroup with anonymous access should throw " + 
+                         "AccessControlException");
+                }
+                catch (AccessControlException ignore) {}
+                return null;
+            }
+        });
+        
+        Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {               
+                getGroupDAO().deleteGroup(groupID);
+                return null;
+            }
+        });
+    }
+    
+    @Test
+    public void testDeleteGroupExceptions() throws Exception
+    {
+        final String groupID = getGroupID();
+        
+        Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+                try
+                {                    
+                    getGroupDAO().deleteGroup(groupID);
+                    fail("deleteGroup with unknown group should throw " + 
+                         "GroupNotFoundException");
+                }
+                catch (GroupNotFoundException ignore) {}
+                
+                getGroupDAO().addGroup(new Group(groupID, daoTestUser1));
+                return null;
+            }
+        });
+
+        Subject.doAs(anonSubject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+                try
+                {                    
+                    getGroupDAO().deleteGroup(groupID);
+                    fail("deleteGroup with anonymous access should throw " + 
+                         "AccessControlException");
+                }
+                catch (AccessControlException ignore) {}
+                return null;
+            }
+        });
+        
+        Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {                
+                getGroupDAO().deleteGroup(groupID);
+                return null;
+            }
+        });
+    }
+    
+    @Test
+    public void testSearchGroupsExceptions() throws Exception
+    {        
+        final String groupID = getGroupID();
+        
+        Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+                getGroupDAO().addGroup(new Group(groupID, daoTestUser1));
+                
+                try
+                {
+                    getGroupDAO().searchGroups(unknownPrincipal, Role.OWNER, 
+                                               groupID);
+                    fail("searchGroups with unknown user should throw " + 
+                         "UserNotFoundException");
+                }
+                catch (UserNotFoundException ignore) {}
+                
+                try
+                {
+                    getGroupDAO().searchGroups(daoTestPrincipal1, Role.OWNER, 
+                                               "foo");
+                    fail("searchGroups with unknown user should throw " + 
+                         "GroupNotFoundException");
+                }
+                catch (GroupNotFoundException ignore) {}
+                return null;
+            }
+        });
+
+        Subject.doAs(anonSubject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+                try
+                {                    
+                    getGroupDAO().searchGroups(daoTestPrincipal1, Role.OWNER, 
+                                               groupID);
+                    fail("searchGroups with anonymous access should throw " + 
+                         "AccessControlException");
+                }
+                catch (AccessControlException ignore) {}
+                return null;
+            }
+        });
+        
+        Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {               
+                getGroupDAO().deleteGroup(groupID);
+                return null;
+            }
+        });
+    }
 
     private void assertGroupsEqual(Group gr1, Group gr2)
     {
@@ -371,4 +691,5 @@ public class LdapGroupDAOTest
             assertTrue(gr2.getProperties().contains(prop));
         }
     }
+    
 }
diff --git a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAOTest.java b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAOTest.java
index 8526a90a..84ed2d88 100644
--- a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAOTest.java
+++ b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAOTest.java
@@ -92,9 +92,18 @@ public class LdapUserDAOTest
 {
     private static final Logger log = Logger.getLogger(LdapUserDAOTest.class);
     
-    static final String cadcTestDN = "CN=CADCtest_636,OU=CADC,O=HIA,C=CA";
+    static String server = "mach275.cadc.dao.nrc.ca";
+    static int port = 389;
+    static String adminDN = "uid=webproxy,ou=administrators,ou=topologymanagement,o=netscaperoot";
+    static String adminPW = "go4it";
+    static String userBaseDN = "ou=Users,ou=ds,dc=canfartest,dc=net";
+    static String groupBaseDN = "ou=Groups,ou=ds,dc=canfartest,dc=net";
+//    static String userBaseDN = "ou=Users,ou=ds,dc=canfar,dc=net";
+//    static String groupBaseDN = "ou=Groups,ou=ds,dc=canfar,dc=net";
     
-    static User<X500Principal> cadcTest;
+    static final String testUserDN = "cn=cadcdaotest1,ou=cadc,o=hia,c=ca";
+    
+    static User<X500Principal> testUser;
     static LdapConfig config;
     
     @BeforeClass
@@ -103,14 +112,9 @@ public class LdapUserDAOTest
     {
         Log4jInit.setLevel("ca.nrc.cadc.ac", Level.DEBUG);
         
-        cadcTest = new User<X500Principal>(new X500Principal(cadcTestDN));
+        testUser = new User<X500Principal>(new X500Principal(testUserDN));
     
-        config = new LdapConfig("mach275.cadc.dao.nrc.ca", 389,
-            "uid=webproxy,ou=administrators,ou=topologymanagement,o=netscaperoot",
-            "go4it", 
-            "ou=Users,ou=ds,dc=canfar,dc=net",
-            "ou=TestGroups,ou=ds,dc=canfar,dc=net",
-            "ou=DeletedGroups,ou=ds,dc=canfar,dc=net");
+        config = new LdapConfig(server, port, adminDN, adminPW, userBaseDN, groupBaseDN);
     }
 
     LdapUserDAO<X500Principal> getUserDAO()
@@ -121,11 +125,11 @@ public class LdapUserDAOTest
     /**
      * Test of getUser method, of class LdapUserDAO.
      */
-//    @Test
+    @Test
     public void testGetUser() throws Exception
     {
         Subject subject = new Subject();
-        subject.getPrincipals().add(cadcTest.getUserID());
+        subject.getPrincipals().add(testUser.getUserID());
 
         // do everything as owner
         Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
@@ -134,8 +138,8 @@ public class LdapUserDAOTest
             {
                 try
                 {
-                    User actual = getUserDAO().getUser(cadcTest.getUserID());
-                    assertEquals(cadcTest, actual);
+                    User actual = getUserDAO().getUser(testUser.getUserID());
+                    assertEquals(testUser, actual);
                     
                     return null;
                 }
@@ -154,7 +158,7 @@ public class LdapUserDAOTest
     public void testGetUserGroups() throws Exception
     {
         Subject subject = new Subject();
-        subject.getPrincipals().add(cadcTest.getUserID());
+        subject.getPrincipals().add(testUser.getUserID());
 
         // do everything as owner
         Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
@@ -163,7 +167,7 @@ public class LdapUserDAOTest
             {
                 try
                 {            
-                    Collection<Group> groups = getUserDAO().getUserGroups(cadcTest.getUserID());
+                    Collection<Group> groups = getUserDAO().getUserGroups(testUser.getUserID());
                     assertNotNull(groups);
                     assertTrue(!groups.isEmpty());
                     for (Group group : groups)
@@ -182,11 +186,11 @@ public class LdapUserDAOTest
     /**
      * Test of getUserGroups method, of class LdapUserDAO.
      */
-    @Test
+//    @Test
     public void testIsMember() throws Exception
     {
         Subject subject = new Subject();
-        subject.getPrincipals().add(cadcTest.getUserID());
+        subject.getPrincipals().add(testUser.getUserID());
 
         // do everything as owner
         Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
@@ -195,11 +199,11 @@ public class LdapUserDAOTest
             {
                 try
                 {   
-                    boolean isMember = getUserDAO().isMember(cadcTest.getUserID(), "foo");
+                    boolean isMember = getUserDAO().isMember(testUser.getUserID(), "foo");
                     assertFalse(isMember);
                     
-                    String groupID = "cn=cadcsw,cn=groups,ou=ds,dc=canfar,dc=net";
-                    isMember = getUserDAO().isMember(cadcTest.getUserID(), groupID);
+                    String groupID = "cn=cadcdaotestgroup,cn=groups,ou=ds,dc=canfartest,dc=net";
+                    isMember = getUserDAO().isMember(testUser.getUserID(), groupID);
                     assertTrue(isMember);
                     
                     return null;
diff --git a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/GroupActionFactoryTest.java b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/GroupActionFactoryTest.java
index 110306d1..22df7faa 100644
--- a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/GroupActionFactoryTest.java
+++ b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/GroupActionFactoryTest.java
@@ -216,6 +216,7 @@ public class GroupActionFactoryTest
             HttpServletRequest request = EasyMock.createMock(HttpServletRequest.class);
             EasyMock.expect(request.getPathInfo()).andReturn("groupName");
             EasyMock.expect(request.getMethod()).andReturn("POST");
+            EasyMock.expect(request.getRequestURI()).andReturn(null);
             EasyMock.expect(request.getInputStream()).andReturn(null);
             EasyMock.replay(request);
             GroupsAction action = GroupsActionFactory.getGroupsAction(request, null);
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/ActivatedGroup.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/ActivatedGroup.java
index 6720c643..189088c9 100644
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/ActivatedGroup.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/ActivatedGroup.java
@@ -70,16 +70,17 @@ package ca.nrc.cadc.ac;
 
 import java.security.Principal;
 
-/**
- *
- * @author jburke
- */
 public class ActivatedGroup extends Group
 {
 
+    public ActivatedGroup(String groupID)
+    {
+        super(groupID);
+    }
+    
     public ActivatedGroup(String groupID, User<? extends Principal> owner)
     {
         super(groupID, owner);
     }
-    
+
 }
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/Group.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/Group.java
index 38432d05..3bb88cd4 100644
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/Group.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/Group.java
@@ -109,15 +109,26 @@ public class Group
      * Note: this class does not enforce any access control rules
      */
     public boolean publicRead = false;
+    
+    /**
+     * Ctor.
+     * 
+     * @param groupID   Unique ID for the group. Must be a valid URI fragment 
+     *                  component, so it's restricted to alphanumeric 
+     *                  and "-", ".","_","~" characters.
+     */
+    public Group(String groupID)
+    {
+        this(groupID, null);
+    }
 
     /**
      * Ctor.
      * 
-     * @param groupID
-     *            Unique ID for the group. Must be a valid URI fragment component,
-     *            so it's restricted to alphanumeric and "-", ".","_","~" characters.
-     * @param owner
-     *            Owner/Creator of the group.
+     * @param groupID   Unique ID for the group. Must be a valid URI fragment 
+     *                  component, so it's restricted to alphanumeric 
+     *                  and "-", ".","_","~" characters.
+     * @param owner     Owner/Creator of the group.
      */
     public Group(String groupID, User<? extends Principal> owner)
     {
@@ -133,10 +144,6 @@ public class Group
         }
 
         this.groupID = groupID;
-        if (owner == null)
-        {
-            throw new IllegalArgumentException("Null owner");
-        }
         this.owner = owner;
     }
 
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupReader.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupReader.java
index a72a6c58..7e51c1dd 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupReader.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupReader.java
@@ -197,21 +197,19 @@ public class GroupReader
         String groupID = uri.substring(AC.GROUP_URI.length());
 
         // Group owner
+        User<? extends Principal> user = null;
         Element ownerElement = groupElement.getChild("owner");
-        if (ownerElement == null)
+        if (ownerElement != null)
         {
-            String error = "group missing required owner element";
-            throw new ReaderException(error);
-        }
-
-        // Owner user
-        Element userElement = ownerElement.getChild("user");
-        if (userElement == null)
-        {
-            String error = "owner missing required user element";
-            throw new ReaderException(error);
+            // Owner user
+            Element userElement = ownerElement.getChild("user");
+            if (userElement == null)
+            {
+                String error = "owner missing required user element";
+                throw new ReaderException(error);
+            }
+            user = UserReader.parseUser(userElement);
         }
-        User<? extends Principal> user = UserReader.parseUser(userElement);
 
         Group group = new Group(groupID, user);
 
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupWriter.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupWriter.java
index 17c8a87b..ff3ae502 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupWriter.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupWriter.java
@@ -163,10 +163,13 @@ public class GroupWriter
         groupElement.setAttribute(new Attribute("uri", groupURI));
 
         // Group owner
-        Element ownerElement = new Element("owner");
-        Element userElement = UserWriter.getUserElement(group.getOwner());
-        ownerElement.addContent(userElement);
-        groupElement.addContent(ownerElement);
+        if (group.getOwner() != null)
+        {
+            Element ownerElement = new Element("owner");
+            Element userElement = UserWriter.getUserElement(group.getOwner());
+            ownerElement.addContent(userElement);
+            groupElement.addContent(ownerElement);
+        }
 
         if (deepCopy)
         {
diff --git a/projects/cadcAccessControl/test/src/ca/nrc/cadc/ac/GroupReaderWriterTest.java b/projects/cadcAccessControl/test/src/ca/nrc/cadc/ac/GroupReaderWriterTest.java
index e3a239e6..2ea98693 100644
--- a/projects/cadcAccessControl/test/src/ca/nrc/cadc/ac/GroupReaderWriterTest.java
+++ b/projects/cadcAccessControl/test/src/ca/nrc/cadc/ac/GroupReaderWriterTest.java
@@ -131,7 +131,22 @@ public class GroupReaderWriterTest
     }
      
     @Test
-    public void testReadWrite()
+    public void testMinimalReadWrite()
+        throws Exception
+    {
+        Group expected = new Group("groupID", null);
+                
+        StringBuilder xml = new StringBuilder();
+        GroupWriter.write(expected, xml);
+        assertFalse(xml.toString().isEmpty());
+        
+        Group actual = GroupReader.read(xml.toString());
+        assertNotNull(actual);
+        assertEquals(expected, actual);
+    }
+    
+    @Test
+    public void testMaximalReadWrite()
         throws Exception
     {
         Group expected = new Group("groupID", new User<Principal>(new HttpPrincipal("foo")));
diff --git a/projects/cadcAccessControl/test/src/ca/nrc/cadc/ac/GroupTest.java b/projects/cadcAccessControl/test/src/ca/nrc/cadc/ac/GroupTest.java
index 586a0170..0cb97835 100644
--- a/projects/cadcAccessControl/test/src/ca/nrc/cadc/ac/GroupTest.java
+++ b/projects/cadcAccessControl/test/src/ca/nrc/cadc/ac/GroupTest.java
@@ -81,55 +81,60 @@ public class GroupTest
     @Test
     public void simpleGroupTest() throws Exception
     {
+        Group group1 = new Group("TestGroup");
+        Group group2 = group1;
+        assertEquals(group1.hashCode(), group2.hashCode());
+        assertEquals(group1, group2);
+        assertTrue(group1 == group2);
         
         User<HttpPrincipal> owner = new User<HttpPrincipal>(new HttpPrincipal("owner"));
-        Group group1 = new Group("TestGroup", owner);
+        Group group3 = new Group("TestGroup", owner);
         User<HttpPrincipal> user = new User<HttpPrincipal>(new HttpPrincipal("user"));
         
-        group1.getUserMembers().add(user);
-        assertEquals(1, group1.getUserMembers().size());
+        group3.getUserMembers().add(user);
+        assertEquals(1, group3.getUserMembers().size());
 
-        Group group2 = group1;
-        assertEquals(group1.hashCode(), group2.hashCode());
-        assertEquals(group1, group2);
-        assertTrue(group1 == group2);
+        Group group4 = group3;
+        assertEquals(group3.hashCode(), group4.hashCode());
+        assertEquals(group3, group4);
+        assertTrue(group3 == group4);
         
-        group2 = new Group("TestGroup", owner);
-        assertEquals(group1.hashCode(), group2.hashCode());
-        assertEquals(group1,group2);
+        group4 = new Group("TestGroup", owner);
+        assertEquals(group3.hashCode(), group4.hashCode());
+        assertEquals(group3,group4);
         
-        group2.getUserMembers().add(user);
-        assertEquals(group1.hashCode(), group2.hashCode());
-        assertEquals(group1,group2);
+        group4.getUserMembers().add(user);
+        assertEquals(group3.hashCode(), group4.hashCode());
+        assertEquals(group3,group4);
         
-        group1.getGroupMembers().add(group2);
-        assertEquals(group1.hashCode(), group2.hashCode());
-        assertEquals(group1,group2);
+        group3.getGroupMembers().add(group4);
+        assertEquals(group3.hashCode(), group4.hashCode());
+        assertEquals(group3,group4);
         
-        group1.description = "Test group";
-        assertEquals(group1.hashCode(), group2.hashCode());
-        assertEquals(group1,group2);
+        group3.description = "Test group";
+        assertEquals(group3.hashCode(), group4.hashCode());
+        assertEquals(group3,group4);
         
         // group read and write equality tests     
-        group1.groupRead = group2;
-        assertEquals(group1.hashCode(), group2.hashCode());
-        assertEquals(group1,group2);
+        group3.groupRead = group4;
+        assertEquals(group3.hashCode(), group4.hashCode());
+        assertEquals(group3,group4);
         
         // group write equality tests
-        group1.groupWrite = group2;
-        assertEquals(group1.hashCode(), group2.hashCode());
-        assertEquals(group1,group2);
+        group3.groupWrite = group4;
+        assertEquals(group3.hashCode(), group4.hashCode());
+        assertEquals(group3,group4);
 
-        group1.publicRead = true;
-        assertEquals(group1.hashCode(), group2.hashCode());
-        assertEquals(group1,group2);
+        group3.publicRead = true;
+        assertEquals(group3.hashCode(), group4.hashCode());
+        assertEquals(group3,group4);
         
-        group2 = new Group("NewTestGroup-._~.", owner);
-        assertFalse(group1.hashCode() == group2.hashCode());
-        assertFalse(group1.equals(group2));
+        group4 = new Group("NewTestGroup-._~.", owner);
+        assertFalse(group3.hashCode() == group4.hashCode());
+        assertFalse(group3.equals(group4));
         
         // test toString
-        System.out.println(group1);
+        System.out.println(group3);
     }
     
     @Test
@@ -151,10 +156,11 @@ public class GroupTest
         try
         {
             new Group("NewTestGroup", null);
+            thrown = true;
         }
         catch(IllegalArgumentException e)
         {
-            thrown = true;
+            fail("Owner can be null");
         }
         assertTrue(thrown);
         
-- 
GitLab