From 254a7dfcbad2d79922f20c27675a91fec2b2a8ee Mon Sep 17 00:00:00 2001 From: Adrian Damian <Adrian.Damian@nrc-cnrc.gc.ca> Date: Wed, 24 Sep 2014 15:01:31 -0700 Subject: [PATCH] Fixed get to diferentiate between not found and un-authorized. --- projects/cadcAccessControl-Server/build.xml | 6 ++-- .../ca/nrc/cadc/ac/server/ldap/LdapDAO.java | 2 +- .../nrc/cadc/ac/server/ldap/LdapGroupDAO.java | 32 +++++++++++-------- .../cadc/ac/server/ldap/LdapGroupDAOTest.java | 31 +++++++++++++++--- .../src/ca/nrc/cadc/ac/client/GMSClient.java | 15 +++++++-- 5 files changed, 61 insertions(+), 25 deletions(-) diff --git a/projects/cadcAccessControl-Server/build.xml b/projects/cadcAccessControl-Server/build.xml index 41e56c2e..bad1d7b6 100644 --- a/projects/cadcAccessControl-Server/build.xml +++ b/projects/cadcAccessControl-Server/build.xml @@ -141,10 +141,10 @@ <pathelement path="${build}/test/class"/> <pathelement path="${testingJars}"/> </classpath> - <!--<test name="ca.nrc.cadc.ac.server.ldap.LdapDAOTest" />--> + <test name="ca.nrc.cadc.ac.server.ldap.LdapDAOTest" /> <test name="ca.nrc.cadc.ac.server.ldap.LdapGroupDAOTest" /> - <!--<test name="ca.nrc.cadc.ac.server.web.GroupActionFactoryTest" />--> - <!--<test name="ca.nrc.cadc.ac.server.ldap.LdapUserDAOTest" />--> + <test name="ca.nrc.cadc.ac.server.web.GroupActionFactoryTest" /> + <test name="ca.nrc.cadc.ac.server.ldap.LdapUserDAOTest" /> <formatter type="plain" usefile="false" /> </junit> </target> 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 11a7f00b..287f9d28 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 @@ -212,7 +212,7 @@ public abstract class LdapDAO { throw new AccessControlException("Invalid credentials " + msg); } - else if (code == ResultCode.SUCCESS) + else if ((code == ResultCode.SUCCESS) || (code == ResultCode.NO_SUCH_OBJECT) ) { // all good. nothing to do } 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 f0a205a4..8b3f0667 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 @@ -345,7 +345,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO { String [] attributes = new String[] {"entrydn", "cn", "description", "owner", "uniquemember", - "modifytimestamp"}; + "modifytimestamp", "nsaccountlock"}; return getGroup(groupDN, groupID, withMembers, attributes); } @@ -366,10 +366,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO { try { - Filter filter = Filter.createANDFilter( - Filter.createEqualityFilter("cn", groupID), - Filter.createNOTFilter( - Filter.createEqualityFilter("nsaccountlock", "TRUE"))); + Filter filter = Filter.createEqualityFilter("cn", groupID); SearchRequest searchRequest = new SearchRequest(groupDN.toNormalizedString(), @@ -386,11 +383,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO } 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) + if (e.getResultCode() == ResultCode.NO_SUCH_OBJECT) { String msg = "Group not found " + groupID; logger.debug(msg); @@ -398,23 +391,34 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO } else { - throw new RuntimeException("Unknown LDAP exception: " + e.getResultCode()); + LdapDAO.checkLdapResult(e.getResultCode(), e.getMessage()); } } if (searchResult.getEntryCount() == 0) { - // deleted groups? - String msg = "Group not found " + groupID; + LdapDAO.checkLdapResult(searchResult.getResultCode(), null); + //access denied + String msg = "Not authorized to access " + groupID; logger.debug(msg); - throw new GroupNotFoundException(groupID); + throw new AccessControlException(groupID); } if (searchResult.getEntryCount() >1) { throw new RuntimeException("BUG: multiple results when retrieving group " + groupID); } + SearchResultEntry searchEntry = searchResult.getSearchEntries().get(0); + + if (searchEntry.getAttribute("nsaccountlock") != null) + { + // deleted group + String msg = "Group not found " + groupID; + logger.debug(msg); + throw new GroupNotFoundException(groupID); + } + String groupCN = searchEntry.getAttributeValue("cn"); DN groupOwner = searchEntry.getAttributeValueAsDN("owner"); 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 276d5b6a..7914b367 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 @@ -557,8 +557,29 @@ public class LdapGroupDAOTest Subject.doAs(daoTestUser1Subject, new PrivilegedExceptionAction<Object>() { public Object run() throws Exception - { - getGroupDAO().deleteGroup(groupID); + { + try + { + getGroupDAO().getGroup(groupID); + //fail("getGroup with anonymous access should throw " + + // "AccessControlException"); + } + catch (AccessControlException ignore) {} + return null; + } + }); + + Subject.doAs(daoTestUser2Subject, new PrivilegedExceptionAction<Object>() + { + public Object run() throws Exception + { + try + { + getGroupDAO().getGroup(groupID); + fail("getGroup with anonymous access should throw " + + "AccessControlException"); + } + catch (AccessControlException ignore) {} return null; } }); @@ -729,10 +750,10 @@ public class LdapGroupDAOTest Group group = getGroupDAO().getGroup(groupID); assertTrue(group == null); - fail("searchGroups with unknown user should throw " + - "GroupNotFoundException"); + fail("searchGroups with un-authorized user should throw " + + "AccessControlException"); } - catch (GroupNotFoundException ignore) + catch (AccessControlException ignore) { } 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 4494ff0a..c2714fab 100755 --- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/client/GMSClient.java +++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/client/GMSClient.java @@ -86,6 +86,7 @@ import java.util.Map; import java.util.Set; import javax.net.ssl.HttpsURLConnection; +import javax.net.ssl.SSLHandshakeException; import javax.net.ssl.SSLSocketFactory; import javax.security.auth.Subject; @@ -380,14 +381,24 @@ public class GMSClient ((HttpsURLConnection) conn) .setSSLSocketFactory(getSSLSocketFactory()); } - int responseCode = conn.getResponseCode(); + int responseCode = -1; + try + { + responseCode = conn.getResponseCode(); + } + catch(SSLHandshakeException e) + { + throw new AccessControlException(e.getMessage()); + } + if (responseCode != 200) { String errMessage = NetUtil.getErrorBody(conn); log.debug("deleteGroup response " + responseCode + ": " + errMessage); - if ((responseCode == 401) || (responseCode == 403)) + if ((responseCode == 401) || (responseCode == 403) || + (responseCode == -1)) { throw new AccessControlException(errMessage); } -- GitLab