From fa3332aa75a5a747a48afa71fd6c21e949d59ad2 Mon Sep 17 00:00:00 2001
From: Adrian Damian <Adrian.Damian@nrc-cnrc.gc.ca>
Date: Thu, 25 Sep 2014 10:11:36 -0700
Subject: [PATCH] Added better error messaging
---
.../nrc/cadc/ac/server/ldap/LdapConfig.java | 4 ++-
.../ca/nrc/cadc/ac/server/ldap/LdapDAO.java | 17 ++++------
.../nrc/cadc/ac/server/ldap/LdapGroupDAO.java | 31 +++++++++----------
.../nrc/cadc/ac/server/ldap/LdapUserDAO.java | 12 +++----
.../cadc/ac/server/ldap/LdapGroupDAOTest.java | 4 ++-
5 files changed, 33 insertions(+), 35 deletions(-)
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 9157966d..817b3e84 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
@@ -68,12 +68,14 @@
*/
package ca.nrc.cadc.ac.server.ldap;
-import ca.nrc.cadc.util.StringUtil;
import java.io.IOException;
import java.net.URL;
import java.util.Properties;
+
import org.apache.log4j.Logger;
+import ca.nrc.cadc.util.StringUtil;
+
public class LdapConfig
{
private static final Logger logger = Logger.getLogger(LdapConfig.class);
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 287f9d28..c33961f3 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
@@ -196,21 +196,16 @@ public abstract class LdapDAO
* @param errorMsg
* @throws TransientException
*/
- protected static void checkLdapResult(ResultCode code, String errorMsg)
+ protected static void checkLdapResult(ResultCode code)
throws TransientException
{
- String msg = "";
- if (errorMsg != null)
- {
- msg = "(" + errorMsg + ")";
- }
if (code == ResultCode.INSUFFICIENT_ACCESS_RIGHTS)
{
- throw new AccessControlException("Not authorized " + msg);
+ throw new AccessControlException("Not authorized ");
}
else if (code == ResultCode.INVALID_CREDENTIALS)
{
- throw new AccessControlException("Invalid credentials " + msg);
+ throw new AccessControlException("Invalid credentials ");
}
else if ((code == ResultCode.SUCCESS) || (code == ResultCode.NO_SUCH_OBJECT) )
{
@@ -218,16 +213,16 @@ public abstract class LdapDAO
}
else if (code == ResultCode.PARAM_ERROR)
{
- throw new IllegalArgumentException("Error in Ldap parameters " + msg);
+ throw new IllegalArgumentException("Error in Ldap parameters ");
}
else if (code == ResultCode.BUSY ||
code == ResultCode.CONNECT_ERROR )
{
- throw new TransientException("Connection problems " + msg );
+ throw new TransientException("Connection problems ");
}
else
{
- throw new RuntimeException("Ldap error" + msg);
+ throw new RuntimeException("Ldap error (" + code.getName() + ")");
}
}
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 3223b21b..6b0566c6 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
@@ -173,7 +173,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
group.description,
group.getUserMembers(),
group.getGroupMembers());
- LdapDAO.checkLdapResult(result.getResultCode(), null);
+ LdapDAO.checkLdapResult(result.getResultCode());
// add group to admin groups tree
result = addGroup(getAdminGroupDN(group.getID()),
@@ -181,7 +181,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
group.description,
group.getUserAdmins(),
group.getGroupAdmins());
- LdapDAO.checkLdapResult(result.getResultCode(), null);
+ LdapDAO.checkLdapResult(result.getResultCode());
try
{
@@ -195,8 +195,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
}
catch (LDAPException e)
{
- LdapDAO.checkLdapResult(e.getResultCode(),
- e.getDiagnosticMessage());
+ LdapDAO.checkLdapResult(e.getResultCode());
throw new RuntimeException("Unexpected LDAP exception", e);
}
}
@@ -302,7 +301,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
}
catch (LDAPException e)
{
- LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
+ LdapDAO.checkLdapResult(e.getResultCode());
throw new RuntimeException("Unexpected LDAP exception", e);
}
}
@@ -391,13 +390,13 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
}
else
{
- LdapDAO.checkLdapResult(e.getResultCode(), e.getMessage());
+ LdapDAO.checkLdapResult(e.getResultCode());
}
}
if (searchResult.getEntryCount() == 0)
{
- LdapDAO.checkLdapResult(searchResult.getResultCode(), null);
+ LdapDAO.checkLdapResult(searchResult.getResultCode());
//access denied
String msg = "Not authorized to access " + groupID;
logger.debug(msg);
@@ -485,7 +484,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
}
catch (LDAPException e1)
{
- LdapDAO.checkLdapResult(e1.getResultCode(), e1.getDiagnosticMessage());
+ LdapDAO.checkLdapResult(e1.getResultCode());
throw new GroupNotFoundException("Not found " + groupID);
}
}
@@ -573,7 +572,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
new ProxiedAuthorizationV2RequestControl(
"dn:" + getSubjectDN().toNormalizedString()));
LdapDAO.checkLdapResult(getConnection().
- modify(modifyRequest).getResultCode(), null);
+ modify(modifyRequest).getResultCode());
// modify the group itself now
modifyRequest = new ModifyRequest(getGroupDN(group.getID()), mods);
@@ -582,11 +581,11 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
new ProxiedAuthorizationV2RequestControl(
"dn:" + getSubjectDN().toNormalizedString()));
LdapDAO.checkLdapResult(getConnection().
- modify(modifyRequest).getResultCode(), null);
+ modify(modifyRequest).getResultCode());
}
catch (LDAPException e1)
{
- LdapDAO.checkLdapResult(e1.getResultCode(), e1.getDiagnosticMessage());
+ LdapDAO.checkLdapResult(e1.getResultCode());
}
try
{
@@ -655,11 +654,11 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
new ProxiedAuthorizationV2RequestControl(
"dn:" + getSubjectDN().toNormalizedString()));
LDAPResult result = getConnection().modify(modifyRequest);
- LdapDAO.checkLdapResult(result.getResultCode(), null);
+ LdapDAO.checkLdapResult(result.getResultCode());
}
catch (LDAPException e1)
{
- LdapDAO.checkLdapResult(e1.getResultCode(), e1.getDiagnosticMessage());
+ LdapDAO.checkLdapResult(e1.getResultCode());
}
try
@@ -761,7 +760,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
}
catch (LDAPException e1)
{
- LdapDAO.checkLdapResult(e1.getResultCode(), e1.getDiagnosticMessage());
+ LdapDAO.checkLdapResult(e1.getResultCode());
}
return groupDNs;
}
@@ -851,7 +850,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
}
catch (LDAPException e)
{
- LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
+ LdapDAO.checkLdapResult(e.getResultCode());
}
throw new IllegalArgumentException(groupID + " not a valid group ID");
}
@@ -869,7 +868,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
}
catch (LDAPException e)
{
- LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
+ LdapDAO.checkLdapResult(e.getResultCode());
}
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 88e16fee..c6b9221c 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
@@ -166,7 +166,7 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
}
catch (LDAPException e)
{
- LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
+ LdapDAO.checkLdapResult(e.getResultCode());
}
if (searchResult == null)
@@ -196,7 +196,7 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
* @return Collection of Group instances.
*
* @throws UserNotFoundException when the user is not found.
- * @throws TransientException If an temporary, unexpected problem occurred.
+ * @throws TransientException If an temporary, unexpected problem occurred., e.getMessage(
* @throws AccessControlException If the operation is not permitted.
*/
public Collection<DN> getUserGroups(final T userID, final boolean isAdmin)
@@ -257,7 +257,7 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
}
catch (LDAPException e)
{
- LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
+ LdapDAO.checkLdapResult(e.getResultCode());
}
return groupDNs;
}
@@ -312,7 +312,7 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
}
catch (LDAPException e)
{
- LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
+ LdapDAO.checkLdapResult(e.getResultCode());
}
return false;
}
@@ -347,7 +347,7 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
// }
// catch (LDAPException e)
// {
-// LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
+// LdapDAO.checkLdapResult(e.getResultCode());
// throw new RuntimeException("Unexpected LDAP exception", e);
// }
// }
@@ -423,7 +423,7 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
} catch (LDAPException e)
{
- LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
+ LdapDAO.checkLdapResult(e.getResultCode());
}
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 7914b367..e29da595 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
@@ -292,6 +292,7 @@ public class LdapGroupDAOTest
testGroup2.getUserMembers().add(daoTestUser2);
testGroup2 = getGroupDAO().addGroup(testGroup2);
log.debug("add group: " + testGroup2ID);
+ Thread.sleep(1000); //sleep to let memberof plugin in LDAP do its work
}
catch (Exception e)
{
@@ -392,6 +393,7 @@ public class LdapGroupDAOTest
testGroup2.getUserAdmins().add(daoTestUser2);
testGroup2 = getGroupDAO().addGroup(testGroup2);
log.debug("add group: " + testGroup2ID);
+ Thread.sleep(1000); // sleep to let memberof plugin do its work
}
catch (Exception e)
{
@@ -406,7 +408,7 @@ public class LdapGroupDAOTest
public Object run() throws Exception
{
try
- {
+ {
Collection<Group> groups =
getGroupDAO().getGroups(daoTestUser2.getUserID(),
Role.ADMIN, null);
--
GitLab