From 79dc8a3aef4174a1fa0539c3e02af1f3731b8e57 Mon Sep 17 00:00:00 2001
From: Adrian Damian <Adrian.Damian@nrc-cnrc.gc.ca>
Date: Fri, 19 Sep 2014 12:20:36 -0700
Subject: [PATCH] Fixed unit testing

---
 .../nrc/cadc/ac/server/ldap/LdapGroupDAO.java | 297 +++++++-----------
 .../cadc/ac/server/ldap/LdapGroupDAOTest.java |   9 -
 .../src/ca/nrc/cadc/ac/ActivatedGroup.java    |  19 +-
 3 files changed, 125 insertions(+), 200 deletions(-)

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 10652c10..13580e05 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
@@ -162,10 +162,14 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
                             "Support for groups properties not available");
                 }
                 
-                Group inactiveGroup = getInactiveGroup(group);
-                if (inactiveGroup != null)
+                try
+                {
+                    getInactiveGroup(group);
+                    return reactivateGroup(group);
+                }
+                catch (GroupNotFoundException e)
                 {
-                    return reactiveGroup(group, inactiveGroup);
+                    // ignore
                 }
                 
                 DN ownerDN = userPersist.getUserDN(group.getOwner());
@@ -197,7 +201,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
             {
                 e.printStackTrace();
                 throw new RuntimeException(e);
-            }
+            } 
         }
     }
     
@@ -247,45 +251,55 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
     }
     
     private Group getInactiveGroup(final Group group)
-        throws AccessControlException, UserNotFoundException, LDAPException
+        throws AccessControlException, UserNotFoundException,
+        GroupNotFoundException
     {
-        Group inactiveGroup = 
-                getInactiveGroup(getGroupDN(group.getID()).toNormalizedString(), 
-                                 group.getID());
-
-        if (inactiveGroup == null)
-        {
-            return null;
-        }
-        
-        if (!group.getOwner().equals(inactiveGroup.getOwner()))
+        Group inactiveGroup;
+        try
         {
-            throw new AccessControlException(
-                    "Inactive group not owned be requestor");
-        }
-        
-        Group inactiveAdminGroup = getInactiveGroup(
-                        getAdminGroupDN(group.getID()).toNormalizedString(), 
-                        group.getID());
-        
-        if (inactiveAdminGroup == null)
+            inactiveGroup = getInactiveGroup(getGroupDN(group.getID())
+                    .toNormalizedString(), group.getID());
+
+            if (inactiveGroup == null)
+            {
+                return null;
+            }
+
+            if (!group.getOwner().equals(inactiveGroup.getOwner()))
+            {
+                throw new AccessControlException(
+                        "Inactive group not owned be requestor");
+            }
+
+            Group inactiveAdminGroup = getInactiveGroup(
+                    getAdminGroupDN(group.getID()).toNormalizedString(),
+                    group.getID());
+
+            if (inactiveAdminGroup == null)
+            {
+                throw new RuntimeException(
+                        "BUG: adminGroup not found for group " + group.getID());
+            }
+
+            if (!group.getOwner().equals(inactiveAdminGroup.getOwner()))
+            {
+                throw new RuntimeException(
+                        "Bug: adminGroup owner doesn't match "
+                                + "group owner for group " + group.getID());
+            }
+            return inactiveGroup;
+        } 
+        catch (LDAPException e)
         {
-            throw new RuntimeException("BUG: adminGroup not found for group " +
-                                       group.getID());
+            // TODO Auto-generated catch block
+            throw new RuntimeException("BUG: LDAP Exception: ", e);
         }
         
-        if (!group.getOwner().equals(inactiveAdminGroup.getOwner()))
-        {
-            throw new RuntimeException("Bug: adminGroup owner doesn't match " +
-                                       "group owner for group " + 
-                                       group.getID());
-        }
         
-        return inactiveGroup;
     }
     
     private Group getInactiveGroup(final String groupDN, final String groupID)
-        throws UserNotFoundException, LDAPException
+        throws UserNotFoundException, LDAPException, GroupNotFoundException
     {
         Filter filter = Filter.createANDFilter(
                 Filter.createEqualityFilter("cn", groupID),
@@ -306,7 +320,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         {
             String msg = "Inactive Group not found " + groupID;
             logger.debug(msg);
-            return null;
+            throw new GroupNotFoundException(msg);
         }
 
         String groupCN = searchResult.getAttributeValue("cn");
@@ -317,35 +331,12 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         return new Group(groupCN, owner);
     }
     
-    private Group reactiveGroup(final Group newGroup, final Group inactiveGroup)
-        throws UserNotFoundException, LDAPException, TransientException
+    private Group reactivateGroup(final Group group)
+        throws UserNotFoundException, LDAPException, TransientException, AccessControlException, GroupNotFoundException
     {
-        Group group = reactiveGroup(getGroupDN(newGroup.getID()), newGroup, 
-                                    inactiveGroup);
-        Group adminGroup = reactiveGroup(getGroupDN(newGroup.getID()), newGroup, 
-                                         inactiveGroup);
-        return group;
+        return modifyGroup(group, true);
     }
-    
-    private Group reactiveGroup(final DN groupDN, final Group newGroup, 
-                                final Group inactiveGroup)
-        throws UserNotFoundException, LDAPException, TransientException
-    {
-        List<Modification> mods = new ArrayList<Modification>();
-        mods.add(new Modification(ModificationType.DELETE, "nsaccountlock"));
 
-        Group modifiedGroup = modifyGroup(groupDN, newGroup, inactiveGroup, 
-                                          mods);
-        Group activatedGroup = new ActivatedGroup(modifiedGroup.getID(),
-                                                  modifiedGroup.getOwner());
-        activatedGroup.description = modifiedGroup.description;
-        activatedGroup.getProperties().addAll(modifiedGroup.getProperties());
-        activatedGroup.getGroupMembers().addAll(modifiedGroup.getGroupMembers());
-        activatedGroup.getUserMembers().addAll(modifiedGroup.getUserMembers());
-        activatedGroup.getGroupAdmins().addAll(modifiedGroup.getGroupAdmins());
-        activatedGroup.getUserAdmins().addAll(modifiedGroup.getUserAdmins());
-        return activatedGroup;
-    }
 
     /**
      * Get the group with the given Group ID.
@@ -504,7 +495,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
     /**
      * Modify the given group.
      *
-     * @param group The group to update.
+     * @param group The group to update. It must be an existing group
      * 
      * @return The newly updated group.
      * 
@@ -517,171 +508,108 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         throws GroupNotFoundException, TransientException,
                AccessControlException, UserNotFoundException
     {
-        DN groupDN = getGroupDN(group.getID());
-        Group oldGroup = getGroup(groupDN, group.getID(), true);
-        Group newGroup = modifyGroup(groupDN, group, oldGroup, null);
-        
-        DN adminGroupDN = getAdminGroupDN(group.getID());
-        Group oldAdminGroup = getGroup(adminGroupDN, group.getID(), true);
-        Group newAdminGroup = modifyGroup(adminGroupDN, group, oldAdminGroup, 
-                                          null);
-        
-        newGroup.getGroupAdmins().addAll(newAdminGroup.getGroupAdmins());
-        newGroup.getUserAdmins().addAll(newAdminGroup.getUserAdmins());
-        
-        return newGroup;
+        return modifyGroup(group, false); 
     }
     
-    private Group modifyGroup(final DN groupDN, final Group newGroup, 
-                              final Group oldGroup,
-                              final List<Modification> modifications)
+    private Group modifyGroup(final Group group,
+                              boolean withActivate)
         throws UserNotFoundException, TransientException,
-               AccessControlException
+               AccessControlException, GroupNotFoundException
     {
-        if (!newGroup.getProperties().isEmpty())
+        if (!group.getProperties().isEmpty())
         {
             throw new UnsupportedOperationException(
                     "Support for groups properties not available");
         }
-
-        List<Modification> mods = new ArrayList<Modification>();
-        if (modifications != null)
-        {
-            mods.addAll(modifications);
-        }
-
-        if (newGroup.description == null && oldGroup.description != null)
-        {
-            mods.add(new Modification(ModificationType.DELETE, "description"));
-        }
-        else if (newGroup.description != null && oldGroup.description == null)
+        
+        // check if group exists
+        if (withActivate)
         {
-            mods.add(new Modification(ModificationType.ADD, "description", 
-                                        newGroup.description));
+            getInactiveGroup(group);
         }
-        else if (newGroup.description != null && oldGroup.description != null)
+        else
         {
-            mods.add(new Modification(ModificationType.REPLACE, "description", 
-                                        newGroup.description));
+            getGroup(group.getID());
         }
 
-        List<String> newMembers = new ArrayList<String>();
-        for (User<?> member : newGroup.getUserMembers())
+        List<Modification> mods = new ArrayList<Modification>();
+        List<Modification> adminMods = new ArrayList<Modification>();
+        if (withActivate)
         {
-            if (!oldGroup.getUserMembers().remove(member))
-            {
-                DN memberDN;
-                try
-                {
-                    memberDN = userPersist.getUserDN(member);
-                }
-                catch (LDAPException e)
-                {
-                    throw new UserNotFoundException(
-                            "User not found " + member.getUserID());
-                }
-                newMembers.add(memberDN.toNormalizedString());
-            }
+            mods.add(new Modification(ModificationType.DELETE, "nsaccountlock"));
+            adminMods.add(new Modification(ModificationType.DELETE, "nsaccountlock"));
         }
-        for (Group gr : newGroup.getGroupMembers())
-        {
-            if (gr.equals(newGroup))
-            {
-                throw new IllegalArgumentException(
-                        "cyclical reference from group member to group");
-            }
 
-            if (!oldGroup.getGroupMembers().remove(gr))
-            {
-                DN grDN = getGroupDN(gr.getID());
-                newMembers.add(grDN.toNormalizedString());
-            }
-        }
-        for (User<?> member : newGroup.getUserAdmins())
-        {
-            if (!oldGroup.getUserAdmins().remove(member))
-            {
-                DN memberDN;
-                try
-                {
-                    memberDN = userPersist.getUserDN(member);
-                }
-                catch (LDAPException e)
-                {
-                    throw new UserNotFoundException(
-                            "User not found " + member.getUserID());
-                }
-                newMembers.add(memberDN.toNormalizedString());
-            }
-        }
-        for (Group gr : newGroup.getGroupAdmins())
+        if (group.description == null)
         {
-            if (gr.equals(newGroup))
-            {
-                throw new IllegalArgumentException(
-                        "cyclical reference from group member to group");
-            }
-
-            if (!oldGroup.getGroupAdmins().remove(gr))
-            {
-                DN grDN = getGroupDN(gr.getID());
-                newMembers.add(grDN.toNormalizedString());
-            }
+            mods.add(new Modification(ModificationType.REPLACE, "description"));
         }
-        
-        if (!newMembers.isEmpty())
+        else
         {
-            mods.add(new Modification(ModificationType.ADD, "uniquemember", 
-                (String[]) newMembers.toArray(new String[newMembers.size()])));
+            mods.add(new Modification(ModificationType.REPLACE, "description", group.description));
         }
 
-        List<String> delMembers = new ArrayList<String>();
-        for (User<?> member : oldGroup.getUserMembers())
+        List<String> newMembers = new ArrayList<String>();
+        for (User<?> member : group.getUserMembers())
         {
             DN memberDN;
             try
             {
-                memberDN = this.userPersist.getUserDN(member);
-            }
+                memberDN = userPersist.getUserDN(member);
+            } 
             catch (LDAPException e)
             {
-                throw new UserNotFoundException(
-                        "User not found " + member.getUserID());
+                throw new UserNotFoundException("User not found "
+                        + member.getUserID());
             }
-            delMembers.add(memberDN.toNormalizedString());
+            newMembers.add(memberDN.toNormalizedString());
         }
-        for (Group gr : oldGroup.getGroupMembers())
+        for (Group gr : group.getGroupMembers())
         {
-            DN grDN = getGroupDN(gr.getID());
-            delMembers.add(grDN.toNormalizedString());
+                DN grDN = getGroupDN(gr.getID());
+                newMembers.add(grDN.toNormalizedString());
         }
-        for (User<?> member : oldGroup.getUserAdmins())
+        List<String> newAdmins = new ArrayList<String>();
+        for (User<?> member : group.getUserAdmins())
         {
             DN memberDN;
             try
             {
-                memberDN = this.userPersist.getUserDN(member);
+                memberDN = userPersist.getUserDN(member);
             }
             catch (LDAPException e)
             {
                 throw new UserNotFoundException(
                         "User not found " + member.getUserID());
             }
-            delMembers.add(memberDN.toNormalizedString());
+            newAdmins.add(memberDN.toNormalizedString());
         }
-        for (Group gr : oldGroup.getGroupAdmins())
+        for (Group gr : group.getGroupAdmins())
         {
             DN grDN = getGroupDN(gr.getID());
-            delMembers.add(grDN.toNormalizedString());
+            newMembers.add(grDN.toNormalizedString());
         }
+
+        mods.add(new Modification(ModificationType.REPLACE, "uniquemember", 
+                (String[]) newMembers.toArray(new String[newMembers.size()])));
+        adminMods.add(new Modification(ModificationType.REPLACE, "uniquemember", 
+                (String[]) newAdmins.toArray(new String[newAdmins.size()])));
         
-        if (!delMembers.isEmpty())
+        // modify admin group first
+        ModifyRequest modifyRequest = new ModifyRequest(getAdminGroupDN(group.getID()), adminMods);
+        try
         {
-            mods.add(new Modification(ModificationType.DELETE, "uniquemember",
-                (String[]) delMembers.toArray(new String[delMembers.size()])));
+            modifyRequest.addControl(
+                    new ProxiedAuthorizationV2RequestControl(
+                            "dn:" + getSubjectDN().toNormalizedString()));
+            LDAPResult result = getConnection().modify(modifyRequest);
         }
-
-        ModifyRequest modifyRequest = new ModifyRequest(groupDN, mods);
+        catch (LDAPException e1)
+        {
+            throw new RuntimeException("LDAP problem", e1);
+        }
+        // modify the group itself now
+        modifyRequest = new ModifyRequest(getGroupDN(group.getID()), mods);
         try
         {
             modifyRequest.addControl(
@@ -695,12 +623,19 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         }
         try
         {
-            return getGroup(newGroup.getID());
+            if (withActivate)
+            {
+                return new ActivatedGroup(getGroup(group.getID()));
+            }
+            else
+            {
+                return getGroup(group.getID());
+            }
         }
         catch (GroupNotFoundException e)
         {
             throw new RuntimeException(
-                    "BUG: modified group not found (" + groupDN + ")");
+                    "BUG: modified group not found (" + group.getID() + ")");
         }
     }
 
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 1039a5d3..922432fb 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
@@ -457,15 +457,6 @@ public class LdapGroupDAOTest
             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));
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/ActivatedGroup.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/ActivatedGroup.java
index 189088c9..22e445ed 100644
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/ActivatedGroup.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/ActivatedGroup.java
@@ -68,19 +68,18 @@
  */
 package ca.nrc.cadc.ac;
 
-import java.security.Principal;
 
 public class ActivatedGroup extends Group
 {
-
-    public ActivatedGroup(String groupID)
-    {
-        super(groupID);
-    }
-    
-    public ActivatedGroup(String groupID, User<? extends Principal> owner)
+    public ActivatedGroup(Group group)
     {
-        super(groupID, owner);
+        super(group.getID(), group.getOwner());
+        this.description = group.description;
+        this.properties = group.getProperties();
+        this.lastModified = group.lastModified;
+        this.getUserMembers().addAll(group.getUserMembers());
+        this.getGroupMembers().addAll(group.getGroupMembers());
+        this.getUserAdmins().addAll(group.getUserAdmins());
+        this.getGroupAdmins().addAll(group.getGroupAdmins());
     }
-
 }
-- 
GitLab