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..6a7ec0cfa244665a668eb5573b8a3ca023e146b9 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
@@ -535,8 +535,16 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
else if (memberDN.isDescendantOf(config.getGroupsDN(),
false))
{
- ldapGroup.getGroupMembers().add(new Group(
- memberDN.getRDNString().replace("cn=", "")));
+ try
+ {
+ ldapGroup.getGroupMembers().
+ add(new Group(getGroupID(memberDN)));
+ }
+ catch(GroupNotFoundException e)
+ {
+ // ignore as we are not cleaning up
+ // deleted groups from the group members
+ }
}
else
{
@@ -603,7 +611,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 +627,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);
@@ -908,13 +917,14 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
}
/**
- * Returns a group based on its LDAP DN. The returned group is bare
- * (contains only group ID, description, modifytimestamp).
+ * Returns a group based on its LDAP DN. The returned group does not contain
+ * members or admins
*
* @param groupDN
* @return
* @throws com.unboundid.ldap.sdk.LDAPException
- * @throws ca.nrc.cadc.ac.GroupNotFoundException
+ * @throws ca.nrc.cadc.ac.GroupNotFoundException - if group does not exist,
+ * it's deleted or caller has no access to it.
*/
protected Group getGroup(final DN groupDN)
throws LDAPException, GroupNotFoundException, UserNotFoundException
@@ -956,6 +966,53 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
return group;
}
+ /**
+ * Returns a group ID corresponding to a DN. Although the groupID can be
+ * deduced from the group DN, this method checks if the group exists and
+ * it's active and throws an exception if any of those conditions are not
+ * met.
+ *
+ * @param groupDN
+ * @return
+ * @throws com.unboundid.ldap.sdk.LDAPException
+ * @throws ca.nrc.cadc.ac.GroupNotFoundException - Group not found or not
+ * active
+ */
+ protected String getGroupID(final DN groupDN)
+ throws LDAPException, GroupNotFoundException
+ {
+ Filter filter = Filter.createEqualityFilter("entrydn",
+ groupDN.toNormalizedString());
+
+ SearchRequest searchRequest = new SearchRequest(
+ config.getGroupsDN(), SearchScope.SUB, filter,
+ "cn", "nsaccountlock");
+
+ searchRequest.addControl(
+ new ProxiedAuthorizationV2RequestControl("dn:" +
+ getSubjectDN().toNormalizedString()));
+
+ SearchResultEntry searchResult =
+ getConnection().searchForEntry(searchRequest);
+
+ if (searchResult == null)
+ {
+ String msg = "Group not found " + groupDN;
+ logger.debug(msg);
+ throw new GroupNotFoundException(groupDN.toNormalizedString());
+ }
+
+ if (searchResult.getAttribute("nsaccountlock") != null)
+ {
+ // deleted group
+ String msg = "Group not found " + groupDN;
+ logger.debug(msg);
+ throw new GroupNotFoundException(groupDN.toNormalizedString());
+ }
+
+ return searchResult.getAttributeValue("cn");
+ }
+
/**
*
* @param groupID
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/AddGroupMemberAction.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/AddGroupMemberAction.java
index 9577061cca259dcc76f5e4dae527685aafc7164d..c842e9708c09ea603ebc77c21173802586bd1bea 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/AddGroupMemberAction.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/AddGroupMemberAction.java
@@ -73,7 +73,6 @@ import ca.nrc.cadc.ac.GroupAlreadyExistsException;
import ca.nrc.cadc.ac.server.GroupPersistence;
import java.util.ArrayList;
import java.util.List;
-import java.util.Set;
public class AddGroupMemberAction extends GroupsAction
{
@@ -93,7 +92,7 @@ public class AddGroupMemberAction extends GroupsAction
{
GroupPersistence groupPersistence = getGroupPersistence();
Group group = groupPersistence.getGroup(this.groupName);
- Group toAdd = groupPersistence.getGroup(this.groupMemberName);
+ Group toAdd = new Group(this.groupMemberName);
if (!group.getGroupMembers().add(toAdd))
{
throw new GroupAlreadyExistsException(this.groupMemberName);
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/AddUserMemberAction.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/AddUserMemberAction.java
index d8a84b2650cf2b3d4cd191b5fbf7f4a54014235c..7c6873401e1aa28c94b36e0980ec70bedf72bea5 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/AddUserMemberAction.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/AddUserMemberAction.java
@@ -99,14 +99,14 @@ public class AddUserMemberAction extends GroupsAction
throws Exception
{
GroupPersistence groupPersistence = getGroupPersistence();
- UserPersistence userPersistence = getUserPersistence();
Group group = groupPersistence.getGroup(this.groupName);
Principal userPrincipal = AuthenticationUtil.createPrincipal(this.userID, this.userIDType);
- User toAdd = userPersistence.getUser(userPrincipal);
+ User<Principal> toAdd = new User(userPrincipal);
if (!group.getUserMembers().add(toAdd))
{
throw new MemberAlreadyExistsException();
}
+
groupPersistence.modifyGroup(group);
List<String> addedMembers = new ArrayList<String>();
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/GroupsAction.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/GroupsAction.java
index 02f64926a9982e7ae88b89bed6c054087cb473a9..42c0ac3cf8138d192991ae40b80da515016fd1dc 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/GroupsAction.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/GroupsAction.java
@@ -132,56 +132,56 @@ public abstract class GroupsAction
}
catch (AccessControlException e)
{
- log.debug(e);
+ log.debug(e.getMessage(), e);
String message = "Permission Denied";
this.logInfo.setMessage(message);
sendError(403, message);
}
catch (IllegalArgumentException e)
{
- log.debug(e);
+ log.debug(e.getMessage(), e);
String message = e.getMessage();
this.logInfo.setMessage(message);
sendError(400, message);
}
catch (MemberNotFoundException e)
{
- log.debug(e);
+ log.debug(e.getMessage(), e);
String message = "Member not found: " + e.getMessage();
this.logInfo.setMessage(message);
sendError(404, message);
}
catch (GroupNotFoundException e)
{
- log.debug(e);
+ log.debug(e.getMessage(), e);
String message = "Group not found: " + e.getMessage();
this.logInfo.setMessage(message);
sendError(404, message);
}
catch (UserNotFoundException e)
{
- log.debug(e);
+ log.debug(e.getMessage(), e);
String message = "User not found: " + e.getMessage();
this.logInfo.setMessage(message);
sendError(404, message);
}
catch (MemberAlreadyExistsException e)
{
- log.debug(e);
+ log.debug(e.getMessage(), e);
String message = "Member already exists: " + e.getMessage();
this.logInfo.setMessage(message);
sendError(409, message);
}
catch (GroupAlreadyExistsException e)
{
- log.debug(e);
+ log.debug(e.getMessage(), e);
String message = "Group already exists: " + e.getMessage();
this.logInfo.setMessage(message);
sendError(409, message);
}
catch (UnsupportedOperationException e)
{
- log.debug(e);
+ log.debug(e.getMessage(), e);
this.logInfo.setMessage("Not yet implemented.");
sendError(501);
}
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/RemoveGroupMemberAction.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/RemoveGroupMemberAction.java
index 39e148916a204e9369340fe50a5b0eabc22c69bf..269984b17e2d626ed58551a7c5ad130349405cc4 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/RemoveGroupMemberAction.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/RemoveGroupMemberAction.java
@@ -92,7 +92,7 @@ public class RemoveGroupMemberAction extends GroupsAction
{
GroupPersistence groupPersistence = getGroupPersistence();
Group group = groupPersistence.getGroup(this.groupName);
- Group toRemove = groupPersistence.getGroup(this.groupMemberName);
+ Group toRemove = new Group(this.groupMemberName);
if (!group.getGroupMembers().remove(toRemove))
{
throw new GroupNotFoundException(this.groupMemberName);
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/RemoveUserMemberAction.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/RemoveUserMemberAction.java
index c9612f663c5009d88c9d00d17fa82974a266a6bb..9b4d34f73c8af063650ac3ecf5245ba2c7058b5b 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/RemoveUserMemberAction.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/RemoveUserMemberAction.java
@@ -97,10 +97,9 @@ public class RemoveUserMemberAction extends GroupsAction
throws Exception
{
GroupPersistence groupPersistence = getGroupPersistence();
- UserPersistence userPersistence = getUserPersistence();
Group group = groupPersistence.getGroup(this.groupName);
Principal userPrincipal = AuthenticationUtil.createPrincipal(this.userID, this.userIDType);
- User toRemove = userPersistence.getUser(userPrincipal);
+ User<Principal> toRemove = new User(userPrincipal);
if (!group.getUserMembers().remove(toRemove))
{
throw new MemberNotFoundException();
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..bad3d32ac6ec804357f8bae67f88707fdca8d619 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
@@ -240,8 +266,24 @@ public class LdapGroupDAOTest extends AbstractLdapDAOTest
actualGroup = getGroupDAO().getGroup(expectGroup.getID());
assertGroupsEqual(expectGroup, actualGroup);
+ // create another group and make expected group
+ // member of that group. Delete expected group after
+ Group expectGroup2 = new Group(getGroupID(), daoTestUser1);
+ expectGroup2.getGroupAdmins().add(expectGroup);
+ expectGroup2.getGroupMembers().add(expectGroup);
+ Group actualGroup2 = getGroupDAO().addGroup(expectGroup2);
+ log.debug("addGroup: " + expectGroup2.getID());
+ assertGroupsEqual(expectGroup2, actualGroup2);
+
// delete the group
getGroupDAO().deleteGroup(expectGroup.getID());
+ // now expectGroup should not be member of admin of
+ // expectGroup2
+ expectGroup2.getGroupAdmins().remove(expectGroup);
+ expectGroup2.getGroupMembers().remove(expectGroup);
+ actualGroup2 = getGroupDAO().getGroup(expectGroup2.getID());
+ log.debug("addGroup: " + expectGroup2.getID());
+ assertGroupsEqual(expectGroup2, actualGroup2);
return null;
}
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/client/GMSClient.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/client/GMSClient.java
index 7609ee7326133d0d2975059fdd7666c0e3435f0c..cb0ec4f65ed7605c44f0731fe0246200dccaf6cb 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/client/GMSClient.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/client/GMSClient.java
@@ -375,7 +375,7 @@ public class GMSClient
* @throws AccessControlException If unauthorized to perform this operation.
* @throws java.io.IOException
*/
- public void updateGroup(Group group)
+ public Group updateGroup(Group group)
throws IllegalArgumentException, GroupNotFoundException, UserNotFoundException,
AccessControlException, IOException
{
@@ -388,13 +388,12 @@ public class GMSClient
StringBuilder groupXML = new StringBuilder();
GroupWriter.write(group, groupXML);
log.debug("updateGroup: " + groupXML);
-
+
HttpPost transfer = new HttpPost(updateGroupURL, groupXML.toString(),
- "application/xml", false);
-
+ "application/xml", true);
transfer.setSSLSocketFactory(getSSLSocketFactory());
transfer.run();
-
+
Throwable error = transfer.getThrowable();
if (error != null)
{
@@ -418,6 +417,18 @@ public class GMSClient
}
throw new IOException(error);
}
+
+ try
+ {
+ String retXML = transfer.getResponseBody();
+ log.debug("getGroup returned: " + retXML);
+ return GroupReader.read(retXML);
+ }
+ catch (Exception bug)
+ {
+ log.error("Unexpected exception", bug);
+ throw new RuntimeException(bug);
+ }
}
/**