From b33e29c206482ac6776c682c598518653774ccec Mon Sep 17 00:00:00 2001
From: Adrian Damian <Adrian.Damian@nrc-cnrc.gc.ca>
Date: Mon, 22 Sep 2014 16:37:45 -0700
Subject: [PATCH] Re-factored and fixed unit test
---
.../nrc/cadc/ac/server/GroupPersistence.java | 8 +-
.../ca/nrc/cadc/ac/server/PluginFactory.java | 8 +-
.../ca/nrc/cadc/ac/server/ldap/LdapDAO.java | 75 +++-
.../nrc/cadc/ac/server/ldap/LdapGroupDAO.java | 353 ++++++++----------
.../nrc/cadc/ac/server/ldap/LdapUserDAO.java | 49 ++-
.../cadc/ac/server/ldap/LdapGroupDAOTest.java | 63 +++-
6 files changed, 289 insertions(+), 267 deletions(-)
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/GroupPersistence.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/GroupPersistence.java
index 3c55bae0..01701154 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/GroupPersistence.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/GroupPersistence.java
@@ -68,16 +68,16 @@
*/
package ca.nrc.cadc.ac.server;
+import java.security.AccessControlException;
+import java.security.Principal;
+import java.util.Collection;
+
import ca.nrc.cadc.ac.Group;
import ca.nrc.cadc.ac.GroupAlreadyExistsException;
import ca.nrc.cadc.ac.GroupNotFoundException;
-import ca.nrc.cadc.ac.IdentityType;
import ca.nrc.cadc.ac.Role;
import ca.nrc.cadc.ac.UserNotFoundException;
import ca.nrc.cadc.net.TransientException;
-import java.security.AccessControlException;
-import java.security.Principal;
-import java.util.Collection;
public abstract interface GroupPersistence<T extends Principal>
{
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/PluginFactory.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/PluginFactory.java
index e1ce4ab0..06d92772 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/PluginFactory.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/PluginFactory.java
@@ -125,8 +125,8 @@ public class PluginFactory
{
try
{
- Class c = Class.forName(cname);
- ret = (GroupPersistence) c.newInstance();
+ Class<?> c = Class.forName(cname);
+ ret = (GroupPersistence<T>) c.newInstance();
}
catch (Exception ex)
{
@@ -149,8 +149,8 @@ public class PluginFactory
{
try
{
- Class c = Class.forName(cname);
- ret = (UserPersistence) c.newInstance();
+ Class<?> c = Class.forName(cname);
+ ret = (UserPersistence<T>) c.newInstance();
}
catch (Exception ex)
{
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 4252ee66..11a7f00b 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
@@ -68,22 +68,26 @@
*/
package ca.nrc.cadc.ac.server.ldap;
+import java.security.AccessControlException;
+import java.security.AccessController;
+import java.security.Principal;
+import java.util.Set;
+
+import javax.security.auth.Subject;
+import javax.security.auth.x500.X500Principal;
+
import ca.nrc.cadc.auth.HttpPrincipal;
import ca.nrc.cadc.auth.NumericPrincipal;
import ca.nrc.cadc.auth.OpenIdPrincipal;
+import ca.nrc.cadc.net.TransientException;
+
import com.unboundid.ldap.sdk.DN;
import com.unboundid.ldap.sdk.LDAPConnection;
import com.unboundid.ldap.sdk.LDAPException;
+import com.unboundid.ldap.sdk.ResultCode;
import com.unboundid.ldap.sdk.SearchResult;
import com.unboundid.ldap.sdk.SearchResultEntry;
import com.unboundid.ldap.sdk.SearchScope;
-import java.security.AccessControlException;
-import java.security.AccessController;
-import java.security.Principal;
-import java.util.List;
-import java.util.Set;
-import javax.security.auth.Subject;
-import javax.security.auth.x500.X500Principal;
public abstract class LdapDAO
{
@@ -116,7 +120,15 @@ public abstract class LdapDAO
{
conn = new LDAPConnection(config.getServer(), config.getPort());
conn.bind(config.getAdminUserDN(), config.getAdminPasswd());
+ }
+ return conn;
+ }
+
+ protected DN getSubjectDN() throws LDAPException
+ {
+ if (subjDN == null)
+ {
Subject callerSubject =
Subject.getSubject(AccessController.getContext());
if (callerSubject == null)
@@ -161,7 +173,7 @@ public abstract class LdapDAO
}
SearchResult searchResult =
- conn.search(config.getUsersDN(), SearchScope.ONE,
+ getConnection().search(config.getUsersDN(), SearchScope.ONE,
ldapField, new String[] {"entrydn"});
if (searchResult.getEntryCount() < 1)
@@ -173,17 +185,50 @@ public abstract class LdapDAO
subjDN = ((SearchResultEntry) searchResult.getSearchEntries()
.get(0)).getAttributeValueAsDN("entrydn");
}
-
- return conn;
+ return subjDN;
}
-
- protected DN getSubjectDN() throws LDAPException
+
+ /**
+ * Checks the Ldap result code, and if the result is not SUCCESS,
+ * throws an appropriate exception. This is the place to decide on
+ * mapping between ldap errors and exception types
+ * @param code
+ * @param errorMsg
+ * @throws TransientException
+ */
+ protected static void checkLdapResult(ResultCode code, String errorMsg)
+ throws TransientException
{
- if (subjDN == null)
+ String msg = "";
+ if (errorMsg != null)
{
- getConnection();
+ msg = "(" + errorMsg + ")";
+ }
+ if (code == ResultCode.INSUFFICIENT_ACCESS_RIGHTS)
+ {
+ throw new AccessControlException("Not authorized " + msg);
+ }
+ else if (code == ResultCode.INVALID_CREDENTIALS)
+ {
+ throw new AccessControlException("Invalid credentials " + msg);
+ }
+ else if (code == ResultCode.SUCCESS)
+ {
+ // all good. nothing to do
+ }
+ else if (code == ResultCode.PARAM_ERROR)
+ {
+ throw new IllegalArgumentException("Error in Ldap parameters " + msg);
+ }
+ else if (code == ResultCode.BUSY ||
+ code == ResultCode.CONNECT_ERROR )
+ {
+ throw new TransientException("Connection problems " + msg );
+ }
+ else
+ {
+ throw new RuntimeException("Ldap error" + msg);
}
- return subjDN;
}
}
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 13580e05..1a7b5f85 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
@@ -94,9 +94,11 @@ import com.unboundid.ldap.sdk.DN;
import com.unboundid.ldap.sdk.Filter;
import com.unboundid.ldap.sdk.LDAPException;
import com.unboundid.ldap.sdk.LDAPResult;
+import com.unboundid.ldap.sdk.LDAPSearchException;
import com.unboundid.ldap.sdk.Modification;
import com.unboundid.ldap.sdk.ModificationType;
import com.unboundid.ldap.sdk.ModifyRequest;
+import com.unboundid.ldap.sdk.ResultCode;
import com.unboundid.ldap.sdk.SearchRequest;
import com.unboundid.ldap.sdk.SearchResult;
import com.unboundid.ldap.sdk.SearchResultEntry;
@@ -122,7 +124,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
}
/**
- * Creates the group.
+ * Persists a group.
*
* @param group The group to create
*
@@ -142,6 +144,12 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
throw new IllegalArgumentException("Group owner must be specified");
}
+ if (!group.getProperties().isEmpty())
+ {
+ throw new UnsupportedOperationException(
+ "Support for groups properties not available");
+ }
+
if (!isCreatorOwner(group.getOwner()))
{
throw new AccessControlException("Group owner must be creator");
@@ -149,28 +157,13 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
try
{
- getGroup(group.getID());
- throw new GroupAlreadyExistsException(group.getID());
- }
- catch (GroupNotFoundException ex)
- {
- try
- {
- if (!group.getProperties().isEmpty())
- {
- throw new UnsupportedOperationException(
- "Support for groups properties not available");
- }
-
- try
- {
- getInactiveGroup(group);
- return reactivateGroup(group);
- }
- catch (GroupNotFoundException e)
- {
- // ignore
- }
+ Group newGroup = reactivateGroup(group);
+ if ( newGroup != null)
+ {
+ return newGroup;
+ }
+ else
+ {
DN ownerDN = userPersist.getUserDN(group.getOwner());
@@ -180,13 +173,15 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
group.description,
group.getUserMembers(),
group.getGroupMembers());
+ LdapDAO.checkLdapResult(result.getResultCode(), null);
// add group to admin groups tree
result = addGroup(getAdminGroupDN(group.getID()),
group.getID(), ownerDN,
group.description,
- group.getUserMembers(),
- group.getGroupMembers());
+ group.getUserAdmins(),
+ group.getGroupAdmins());
+ LdapDAO.checkLdapResult(result.getResultCode(), null);
try
{
@@ -197,19 +192,20 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
throw new RuntimeException("BUG: new group not found");
}
}
- catch (LDAPException e)
- {
- e.printStackTrace();
- throw new RuntimeException(e);
- }
}
+ catch (LDAPException e)
+ {
+ LdapDAO.checkLdapResult(e.getResultCode(),
+ e.getDiagnosticMessage());
+ return null; //TODO
+ }
}
private LDAPResult addGroup(final DN groupDN, final String groupID,
final DN ownerDN, final String description,
final Set<User<? extends Principal>> users,
final Set<Group> groups)
- throws UserNotFoundException, LDAPException
+ throws UserNotFoundException, LDAPException, TransientException
{
// add new group
List<Attribute> attributes = new ArrayList<Attribute>();
@@ -242,7 +238,6 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
}
AddRequest addRequest = new AddRequest(groupDN, attributes);
-
addRequest.addControl(
new ProxiedAuthorizationV2RequestControl(
"dn:" + getSubjectDN().toNormalizedString()));
@@ -250,91 +245,66 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
return getConnection().add(addRequest);
}
- private Group getInactiveGroup(final Group group)
+
+ /**
+ * Checks whether group name available for the user or already in use.
+ * @param group
+ * @return activated group or null if group does not exists
+ * @throws AccessControlException
+ * @throws UserNotFoundException
+ * @throws GroupNotFoundException
+ * @throws TransientException
+ * @throws GroupAlreadyExistsException
+ */
+ private Group reactivateGroup(final Group group)
throws AccessControlException, UserNotFoundException,
- GroupNotFoundException
+ TransientException, GroupAlreadyExistsException
{
- Group inactiveGroup;
try
{
- inactiveGroup = getInactiveGroup(getGroupDN(group.getID())
- .toNormalizedString(), group.getID());
+ // check group name exists
+ Filter filter = Filter.createEqualityFilter("cn", group.getID());
- if (inactiveGroup == null)
+ SearchRequest searchRequest =
+ new SearchRequest(
+ getGroupDN(group.getID())
+ .toNormalizedString(), SearchScope.SUB, filter,
+ new String[] {"nsaccountlock"});
+
+ searchRequest.addControl(
+ new ProxiedAuthorizationV2RequestControl("dn:" +
+ getSubjectDN().toNormalizedString()));
+
+ SearchResultEntry searchResult =
+ getConnection().searchForEntry(searchRequest);
+
+ if (searchResult == null)
{
return null;
}
- if (!group.getOwner().equals(inactiveGroup.getOwner()))
+ if (searchResult.getAttributeValue("nsaccountlock") == null)
{
- throw new AccessControlException(
- "Inactive group not owned be requestor");
+ throw new
+ GroupAlreadyExistsException("Group already exists " + group.getID());
}
-
- Group inactiveAdminGroup = getInactiveGroup(
- getAdminGroupDN(group.getID()).toNormalizedString(),
- group.getID());
-
- if (inactiveAdminGroup == null)
+
+ // activate group
+ try
{
- throw new RuntimeException(
- "BUG: adminGroup not found for group " + group.getID());
- }
-
- if (!group.getOwner().equals(inactiveAdminGroup.getOwner()))
+ return modifyGroup(group, true);
+ }
+ catch (GroupNotFoundException e)
{
throw new RuntimeException(
- "Bug: adminGroup owner doesn't match "
- + "group owner for group " + group.getID());
- }
- return inactiveGroup;
+ "BUG: group to modify does not exist" + group.getID());
+ }
}
catch (LDAPException e)
{
- // TODO Auto-generated catch block
- throw new RuntimeException("BUG: LDAP Exception: ", e);
+ LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
}
-
-
- }
-
- private Group getInactiveGroup(final String groupDN, final String groupID)
- throws UserNotFoundException, LDAPException, GroupNotFoundException
- {
- Filter filter = Filter.createANDFilter(
- Filter.createEqualityFilter("cn", groupID),
- Filter.createEqualityFilter("nsaccountlock", "true"));
-
- SearchRequest searchRequest =
- new SearchRequest(groupDN, SearchScope.SUB, filter,
- new String[] {"cn", "owner"});
-
- searchRequest.addControl(
- new ProxiedAuthorizationV2RequestControl("dn:" +
- getSubjectDN().toNormalizedString()));
-
- SearchResultEntry searchResult =
- getConnection().searchForEntry(searchRequest);
-
- if (searchResult == null)
- {
- String msg = "Inactive Group not found " + groupID;
- logger.debug(msg);
- throw new GroupNotFoundException(msg);
- }
-
- String groupCN = searchResult.getAttributeValue("cn");
- DN groupOwner = searchResult.getAttributeValueAsDN("owner");
-
- User<X500Principal> owner = userPersist.getMember(groupOwner);
-
- return new Group(groupCN, owner);
- }
-
- private Group reactivateGroup(final Group group)
- throws UserNotFoundException, LDAPException, TransientException, AccessControlException, GroupNotFoundException
- {
- return modifyGroup(group, true);
+ return null;
}
@@ -410,18 +380,45 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
new ProxiedAuthorizationV2RequestControl("dn:" +
getSubjectDN().toNormalizedString()));
- SearchResultEntry searchResult =
- getConnection().searchForEntry(searchRequest);
+ SearchResult searchResult = null;
+ try
+ {
+ searchResult =
+ getConnection().search(searchRequest);
+ }
+ catch (LDAPSearchException e)
+ {
+ if (e.getResultCode() == ResultCode.AUTHORIZATION_DENIED)
+ {
+ throw new AccessControlException("Unauthorized to access group " + groupID);
+ }
+ else if (e.getResultCode() == ResultCode.NO_SUCH_OBJECT)
+ {
+ String msg = "Group not found " + groupID;
+ logger.debug(msg);
+ throw new GroupNotFoundException(groupID);
+ }
+ else
+ {
+ throw new RuntimeException("Unknown LDAP exception: " + e.getResultCode());
+ }
+ }
- if (searchResult == null)
+ if (searchResult.getEntryCount() == 0)
{
+ // deleted groups?
String msg = "Group not found " + groupID;
logger.debug(msg);
throw new GroupNotFoundException(groupID);
}
- String groupCN = searchResult.getAttributeValue("cn");
- DN groupOwner = searchResult.getAttributeValueAsDN("owner");
+ if (searchResult.getEntryCount() >1)
+ {
+ throw new RuntimeException("BUG: multiple results when retrieving group " + groupID);
+ }
+ SearchResultEntry searchEntry = searchResult.getSearchEntries().get(0);
+ String groupCN = searchEntry.getAttributeValue("cn");
+ DN groupOwner = searchEntry.getAttributeValueAsDN("owner");
User<X500Principal> owner;
try
@@ -434,22 +431,22 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
}
Group ldapGroup = new Group(groupCN, owner);
- if (searchResult.hasAttribute("description"))
+ if (searchEntry.hasAttribute("description"))
{
ldapGroup.description =
- searchResult.getAttributeValue("description");
+ searchEntry.getAttributeValue("description");
}
- if (searchResult.hasAttribute("modifytimestamp"))
+ if (searchEntry.hasAttribute("modifytimestamp"))
{
ldapGroup.lastModified =
- searchResult.getAttributeValueAsDate("modifytimestamp");
+ searchEntry.getAttributeValueAsDate("modifytimestamp");
}
if (withMembers)
{
- if (searchResult.getAttributeValues("uniquemember") != null)
+ if (searchEntry.getAttributeValues("uniquemember") != null)
{
- for (String member : searchResult
+ for (String member : searchEntry
.getAttributeValues("uniquemember"))
{
DN memberDN = new DN(member);
@@ -485,10 +482,8 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
}
catch (LDAPException e1)
{
- // TODO check which LDAP exceptions are transient and which
- // ones are
- // access control
- throw new TransientException("Error getting the group", e1);
+ LdapDAO.checkLdapResult(e1.getResultCode(), e1.getDiagnosticMessage());
+ throw new GroupNotFoundException("Not found " + groupID);
}
}
@@ -508,6 +503,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
throws GroupNotFoundException, TransientException,
AccessControlException, UserNotFoundException
{
+ getGroup(group.getID()); //group must exists first
return modifyGroup(group, false);
}
@@ -521,16 +517,6 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
throw new UnsupportedOperationException(
"Support for groups properties not available");
}
-
- // check if group exists
- if (withActivate)
- {
- getInactiveGroup(group);
- }
- else
- {
- getGroup(group.getID());
- }
List<Modification> mods = new ArrayList<Modification>();
List<Modification> adminMods = new ArrayList<Modification>();
@@ -552,16 +538,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
List<String> newMembers = new ArrayList<String>();
for (User<?> member : group.getUserMembers())
{
- DN memberDN;
- try
- {
- memberDN = userPersist.getUserDN(member);
- }
- catch (LDAPException e)
- {
- throw new UserNotFoundException("User not found "
- + member.getUserID());
- }
+ DN memberDN = userPersist.getUserDN(member);
newMembers.add(memberDN.toNormalizedString());
}
for (Group gr : group.getGroupMembers())
@@ -572,16 +549,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
List<String> newAdmins = new ArrayList<String>();
for (User<?> member : group.getUserAdmins())
{
- DN memberDN;
- try
- {
- memberDN = userPersist.getUserDN(member);
- }
- catch (LDAPException e)
- {
- throw new UserNotFoundException(
- "User not found " + member.getUserID());
- }
+ DN memberDN = userPersist.getUserDN(member);
newAdmins.add(memberDN.toNormalizedString());
}
for (Group gr : group.getGroupAdmins())
@@ -602,24 +570,21 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
modifyRequest.addControl(
new ProxiedAuthorizationV2RequestControl(
"dn:" + getSubjectDN().toNormalizedString()));
- LDAPResult result = getConnection().modify(modifyRequest);
- }
- catch (LDAPException e1)
- {
- throw new RuntimeException("LDAP problem", e1);
- }
- // modify the group itself now
- modifyRequest = new ModifyRequest(getGroupDN(group.getID()), mods);
- try
- {
+ LdapDAO.checkLdapResult(getConnection().
+ modify(modifyRequest).getResultCode(), null);
+
+ // modify the group itself now
+ modifyRequest = new ModifyRequest(getGroupDN(group.getID()), mods);
+
modifyRequest.addControl(
new ProxiedAuthorizationV2RequestControl(
"dn:" + getSubjectDN().toNormalizedString()));
- LDAPResult result = getConnection().modify(modifyRequest);
+ LdapDAO.checkLdapResult(getConnection().
+ modify(modifyRequest).getResultCode(), null);
}
catch (LDAPException e1)
{
- throw new RuntimeException("LDAP problem", e1);
+ LdapDAO.checkLdapResult(e1.getResultCode(), e1.getDiagnosticMessage());
}
try
{
@@ -688,10 +653,11 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
new ProxiedAuthorizationV2RequestControl(
"dn:" + getSubjectDN().toNormalizedString()));
LDAPResult result = getConnection().modify(modifyRequest);
+ LdapDAO.checkLdapResult(result.getResultCode(), null);
}
catch (LDAPException e1)
{
- throw new RuntimeException("LDAP problem", e1);
+ LdapDAO.checkLdapResult(e1.getResultCode(), e1.getDiagnosticMessage());
}
try
@@ -723,18 +689,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
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);
- }
+ DN userDN = userPersist.getUserDN(user);
if (role == Role.OWNER)
{
@@ -756,6 +711,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
throws TransientException, AccessControlException,
GroupNotFoundException, UserNotFoundException
{
+ Collection<Group> groups = new ArrayList<Group>();
try
{
Filter filter = Filter.createEqualityFilter("owner",
@@ -775,7 +731,6 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
new ProxiedAuthorizationV2RequestControl("dn:" +
getSubjectDN().toNormalizedString()));
- Collection<Group> groups = new ArrayList<Group>();
SearchResult results = getConnection().search(searchRequest);
for (SearchResultEntry result : results.getSearchEntries())
{
@@ -791,22 +746,17 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
}
catch (IllegalArgumentException ignore) { }
}
-
- return groups;
}
catch (LDAPException e1)
{
- // TODO check which LDAP exceptions are transient and which
- // ones are
- // access control
- throw new TransientException("Error getting groups", e1);
+ LdapDAO.checkLdapResult(e1.getResultCode(), e1.getDiagnosticMessage());
}
+ return groups;
}
- protected Collection<Group> getMemberGroups(User<T> user, DN userDN,
- String groupID)
- throws TransientException, AccessControlException,
- GroupNotFoundException, UserNotFoundException
+ protected Collection<Group> getMemberGroups(User<T> user, DN userDN,
+ String groupID) throws TransientException, AccessControlException,
+ GroupNotFoundException, UserNotFoundException
{
if (groupID != null)
{
@@ -816,38 +766,37 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
groups.add(getGroup(groupID, false));
}
return groups;
- }
+ }
else
{
+ Collection<Group> groups = userPersist.getUserGroups(user
+ .getUserID());
try
{
- Collection<Group> groups =
- userPersist.getUserGroups(user.getUserID());
-
List<Filter> filters = new ArrayList<Filter>();
for (Group group : groups)
{
- filters.add(Filter.createEqualityFilter("cn",
- group.getID()));
+ filters.add(Filter.createEqualityFilter("cn", group.getID()));
}
Filter filter = Filter.createORFilter(filters);
- SearchRequest searchRequest = new SearchRequest(
- config.getAdminGroupsDN(), SearchScope.SUB, filter,
- "cn");
-
+ SearchRequest searchRequest = new SearchRequest(
+ config.getAdminGroupsDN(), SearchScope.SUB, filter,
+ "cn");
+
SearchResult results = getConnection().search(searchRequest);
for (SearchResultEntry result : results.getSearchEntries())
{
String groupName = result.getAttributeValue("cn");
-
}
- return groups;
- }
+
+ }
catch (LDAPException e)
{
- throw new TransientException(e.getDiagnosticMessage());
+ LdapDAO.checkLdapResult(e.getResultCode(),
+ e.getDiagnosticMessage());
}
+ return groups;
}
}
@@ -856,6 +805,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
throws TransientException, AccessControlException,
GroupNotFoundException, UserNotFoundException
{
+ Collection<Group> groups = new ArrayList<Group>();
try
{
Collection<Group> queryGroups = new ArrayList<Group>();
@@ -898,7 +848,6 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
// }
}
- Collection<Group> groups = new ArrayList<Group>();
if (filters.isEmpty())
{
return groups;
@@ -932,15 +881,13 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
}
catch (IllegalArgumentException ignore) { }
}
- return groups;
+
}
catch (LDAPException e)
{
- // TODO check which LDAP exceptions are transient and which
- // ones are
- // access control
- throw new TransientException("Error getting groups", e);
+ LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
}
+ return groups;
}
// protected Collection<Group> getRWGroups2(User<T> user, DN userDN,
@@ -1058,7 +1005,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
* @param groupID
* @return
*/
- protected DN getGroupDN(final String groupID)
+ protected DN getGroupDN(final String groupID) throws TransientException
{
try
{
@@ -1066,6 +1013,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
}
catch (LDAPException e)
{
+ LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
}
throw new IllegalArgumentException(groupID + " not a valid group ID");
}
@@ -1075,7 +1023,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
* @param groupID
* @return
*/
- protected DN getAdminGroupDN(final String groupID)
+ protected DN getAdminGroupDN(final String groupID) throws TransientException
{
try
{
@@ -1083,6 +1031,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
}
catch (LDAPException e)
{
+ LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
}
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 bb11f40a..8c21c14e 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
@@ -84,7 +84,6 @@ import ca.nrc.cadc.ac.PersonalDetails;
import ca.nrc.cadc.ac.User;
import ca.nrc.cadc.ac.UserNotFoundException;
import ca.nrc.cadc.auth.HttpPrincipal;
-import ca.nrc.cadc.auth.NumericPrincipal;
import ca.nrc.cadc.net.TransientException;
import com.unboundid.ldap.sdk.CompareRequest;
@@ -167,7 +166,7 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
}
catch (LDAPException e)
{
- e.printStackTrace();
+ LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
}
if (searchResult == null)
@@ -202,6 +201,7 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
public Collection<Group> getUserGroups(T userID)
throws UserNotFoundException, TransientException, AccessControlException
{
+ Collection<Group> groups = new HashSet<Group>();
try
{
String searchField = (String) userLdapAttrib.get(userID.getClass());
@@ -227,8 +227,7 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
SearchResultEntry searchResult =
getConnection().searchForEntry(searchRequest);
-
- Collection<Group> groups = new HashSet<Group>();
+
if (searchResult != null)
{
String[] members =
@@ -248,16 +247,13 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
catch (IllegalArgumentException ignore) { }
}
}
- }
- return groups;
+ }
}
catch (LDAPException e)
{
- // TODO check which LDAP exceptions are transient and which
- // ones are
- // access control
- throw new TransientException("Error getting user groups", e);
+ LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
}
+ return groups;
}
/**
@@ -310,11 +306,9 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
}
catch (LDAPException e1)
{
- // TODO check which LDAP exceptions are transient and which
- // ones are
- // access control
- throw new TransientException("Error getting the user", e1);
+ LdapDAO.checkLdapResult(e1.getResultCode(), e1.getDiagnosticMessage());
}
+ return false;
}
public boolean isMember(T userID, String groupID)
@@ -347,11 +341,9 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
}
catch (LDAPException e)
{
- // TODO check which LDAP exceptions are transient and which
- // ones are
- // access control
- throw new TransientException("Error getting the user", e);
+ LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
}
+ return false;
}
/**
@@ -401,7 +393,7 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
DN getUserDN(User<? extends Principal> user)
- throws LDAPException, UserNotFoundException
+ throws UserNotFoundException, TransientException
{
String searchField = (String) userLdapAttrib.get(user.getUserID().getClass());
if (searchField == null)
@@ -413,17 +405,22 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
searchField = "(" + searchField + "=" +
user.getUserID().getName() + ")";
- SearchRequest searchRequest =
- new SearchRequest(this.config.getUsersDN(), SearchScope.SUB,
- searchField, new String[] {"entrydn"});
+ SearchResultEntry searchResult = null;
+ try
+ {
+ SearchRequest searchRequest = new SearchRequest(this.config.getUsersDN(), SearchScope.SUB,
+ searchField, new String[] {"entrydn"});
-// searchRequest.addControl(
-// new ProxiedAuthorizationV2RequestControl("dn:" +
-// getSubjectDN().toNormalizedString()));
- SearchResultEntry searchResult =
+ searchResult =
getConnection().searchForEntry(searchRequest);
+ } catch (LDAPException e)
+ {
+ LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
+ }
+
+
if (searchResult == null)
{
String msg = "User not found " + user.getUserID().toString();
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 922432fb..677a4dad 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
@@ -89,7 +89,8 @@ public class LdapGroupDAOTest
static User<X500Principal> unknownUser;
static User<X500Principal> adminUser;
- static Subject authSubject;
+ static Subject authSubject1;
+ static Subject authSubject2;
static Subject anonSubject;
static LdapConfig config;
@@ -110,8 +111,11 @@ public class LdapGroupDAOTest
unknownUser = new User<X500Principal>(unknownPrincipal);
adminUser = new User<X500Principal>(adminPrincipal);
- authSubject = new Subject();
- authSubject.getPrincipals().add(daoTestUser1.getUserID());
+ authSubject1 = new Subject();
+ authSubject1.getPrincipals().add(daoTestUser1.getUserID());
+
+ authSubject2 = new Subject();
+ authSubject2.getPrincipals().add(daoTestUser2.getUserID());
anonSubject = new Subject();
anonSubject.getPrincipals().add(unknownUser.getUserID());
@@ -134,7 +138,7 @@ public class LdapGroupDAOTest
public void testOneGroup() throws Exception
{
// do everything as owner
- Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+ Subject.doAs(authSubject1, new PrivilegedExceptionAction<Object>()
{
public Object run() throws Exception
{
@@ -218,7 +222,7 @@ public class LdapGroupDAOTest
public void testSearchOwnerGroups() throws Exception
{
// do everything as owner
- Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+ Subject.doAs(authSubject1, new PrivilegedExceptionAction<Object>()
{
public Object run() throws Exception
{
@@ -263,7 +267,7 @@ public class LdapGroupDAOTest
public void testSearchMemberGroups() throws Exception
{
// do everything as owner
- Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+ Subject.doAs(authSubject1, new PrivilegedExceptionAction<Object>()
{
public Object run() throws Exception
{
@@ -310,7 +314,7 @@ public class LdapGroupDAOTest
public void testSearchRWGroups() throws Exception
{
// do everything as owner
- Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+ Subject.doAs(authSubject1, new PrivilegedExceptionAction<Object>()
{
public Object run() throws Exception
{
@@ -371,7 +375,7 @@ public class LdapGroupDAOTest
}
});
- Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+ Subject.doAs(authSubject1, new PrivilegedExceptionAction<Object>()
{
public Object run() throws Exception
{
@@ -405,7 +409,7 @@ public class LdapGroupDAOTest
{
final String groupID = getGroupID();
- Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+ Subject.doAs(authSubject1, new PrivilegedExceptionAction<Object>()
{
public Object run() throws Exception
{
@@ -437,7 +441,7 @@ public class LdapGroupDAOTest
}
});
- Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+ Subject.doAs(authSubject1, new PrivilegedExceptionAction<Object>()
{
public Object run() throws Exception
{
@@ -452,7 +456,7 @@ public class LdapGroupDAOTest
{
final String groupID = getGroupID();
- Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+ Subject.doAs(authSubject1, new PrivilegedExceptionAction<Object>()
{
public Object run() throws Exception
{
@@ -484,7 +488,7 @@ public class LdapGroupDAOTest
}
});
- Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+ Subject.doAs(authSubject1, new PrivilegedExceptionAction<Object>()
{
public Object run() throws Exception
{
@@ -499,7 +503,7 @@ public class LdapGroupDAOTest
{
final String groupID = getGroupID();
- Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+ Subject.doAs(authSubject1, new PrivilegedExceptionAction<Object>()
{
public Object run() throws Exception
{
@@ -531,7 +535,7 @@ public class LdapGroupDAOTest
}
});
- Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+ Subject.doAs(authSubject1, new PrivilegedExceptionAction<Object>()
{
public Object run() throws Exception
{
@@ -546,7 +550,7 @@ public class LdapGroupDAOTest
{
final String groupID = getGroupID();
- Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+ Subject.doAs(authSubject1, new PrivilegedExceptionAction<Object>()
{
public Object run() throws Exception
{
@@ -589,7 +593,7 @@ public class LdapGroupDAOTest
}
});
- Subject.doAs(authSubject, new PrivilegedExceptionAction<Object>()
+ Subject.doAs(authSubject1, new PrivilegedExceptionAction<Object>()
{
public Object run() throws Exception
{
@@ -597,6 +601,33 @@ public class LdapGroupDAOTest
return null;
}
});
+
+
+ // change the user
+
+ Subject.doAs(authSubject2, new PrivilegedExceptionAction<Object>()
+ {
+
+ public Object run() throws Exception
+ {
+
+
+ try
+ {
+ Group group = getGroupDAO().getGroup(groupID);
+ assertTrue(group == null);
+
+ fail("searchGroups with unknown user should throw " +
+ "GroupNotFoundException");
+ }
+ catch (GroupNotFoundException ignore)
+ {
+
+ }
+
+ return null;
+ }
+ });
}
private void assertGroupsEqual(Group gr1, Group gr2)
--
GitLab