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 70ee0595f868c4805fda36310ff6cbd9b9278a5b..ce7f40f632cc2c85fa49bf8a2e4ac0e06d906655 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 @@ -603,7 +603,8 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO mods.add(new Modification(ModificationType.REPLACE, "description", group.description)); } - List<String> newMembers = new ArrayList<String>(); + + Set<String> newMembers = new HashSet<String>(); for (User<?> member : group.getUserMembers()) { DN memberDN = userPersist.getUserDN(member); @@ -618,7 +619,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO DN grDN = getGroupDN(gr.getID()); newMembers.add(grDN.toNormalizedString()); } - List<String> newAdmins = new ArrayList<String>(); + Set<String> newAdmins = new HashSet<String>(); for (User<?> member : group.getUserAdmins()) { DN memberDN = userPersist.getUserDN(member); 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 7af54bc0ae0a9ec3c1e3621a31038538440a7ae9..6e44f437e27d8f6928306cce7734e33c6ae15b90 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 @@ -68,6 +68,7 @@ package ca.nrc.cadc.ac.server.ldap; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -90,17 +91,20 @@ 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 static org.junit.Assert.assertNotNull; +import ca.nrc.cadc.auth.HttpPrincipal; public class LdapGroupDAOTest extends AbstractLdapDAOTest { private static final Logger log = Logger.getLogger(LdapGroupDAOTest.class); - static String daoTestDN1 = "cn=cadcdaotest1,ou=cadc,o=hia,c=ca"; - static String daoTestDN2 = "cn=cadcdaotest2,ou=cadc,o=hia,c=ca"; - static String daoTestDN3 = "cn=cadcdaotest3,ou=cadc,o=hia,c=ca"; + static String daoTestUid1 = "cadcdaotest1"; + static String daoTestUid2 = "cadcdaotest2"; + static String daoTestUid3 = "cadcdaotest3"; + + static String daoTestDN1 = "cn=" + daoTestUid1 + ",ou=cadc,o=hia,c=ca"; + static String daoTestDN2 = "cn=" + daoTestUid2 + ",ou=cadc,o=hia,c=ca"; + static String daoTestDN3 = "cn=" + daoTestUid3 + ",ou=cadc,o=hia,c=ca"; static String unknownDN = "cn=foo,ou=cadc,o=hia,c=ca"; static X500Principal daoTestPrincipal1; @@ -192,6 +196,18 @@ public class LdapGroupDAOTest extends AbstractLdapDAOTest expectGroup.getUserMembers().add(daoTestUser2); actualGroup = getGroupDAO().modifyGroup(expectGroup); assertGroupsEqual(expectGroup, actualGroup); + + // test adding the same user but with two different + // Principals. The duplicate should be ignored + // the the returned result should contain only + // one entry (the dn one) + User<HttpPrincipal> duplicateIdentity = + new User<HttpPrincipal>(new HttpPrincipal(daoTestUid2)); + expectGroup.getUserMembers().add(daoTestUser2); + expectGroup.getUserMembers().add(duplicateIdentity); + actualGroup = getGroupDAO().modifyGroup(expectGroup); + expectGroup.getUserMembers().remove(duplicateIdentity); + assertGroupsEqual(expectGroup, actualGroup); expectGroup.getUserMembers().remove(daoTestUser2); actualGroup = getGroupDAO().modifyGroup(expectGroup); @@ -222,6 +238,16 @@ public class LdapGroupDAOTest extends AbstractLdapDAOTest actualGroup = getGroupDAO().modifyGroup(expectGroup); assertGroupsEqual(expectGroup, actualGroup); + // test adding the same user admin but with two different + // Principals. The duplicate should be ignored + // the the returned result should contain only + // one entry (the dn one) + expectGroup.getUserAdmins().add(daoTestUser2); + expectGroup.getUserAdmins().add(duplicateIdentity); + actualGroup = getGroupDAO().modifyGroup(expectGroup); + expectGroup.getUserAdmins().remove(duplicateIdentity); + assertGroupsEqual(expectGroup, actualGroup); + // delete the group getGroupDAO().deleteGroup(expectGroup.getID()); try