diff --git a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java index e63a3ab59268dd4691a854a683e7693d2f4b604a..da3dcb8040e59e7faebec4a8b6bb62be339daccd 100755 --- a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java +++ b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java @@ -70,7 +70,6 @@ package ca.nrc.cadc.ac.server.ldap; import java.lang.reflect.Field; import java.net.URI; -import java.net.URISyntaxException; import java.security.AccessControlException; import java.util.ArrayList; import java.util.Collection; @@ -753,55 +752,47 @@ public class LdapGroupDAO extends LdapDAO private Group createGroupFromSearchResult(SearchResultEntry result, String[] attributes) throws LDAPException, TransientException { - try + if (result.getAttribute(LDAP_NSACCOUNTLOCK) != null) { - if (result.getAttribute(LDAP_NSACCOUNTLOCK) != null) - { - throw new RuntimeException("BUG: found group with nsaccountlock set: " + - result.getAttributeValue(LDAP_ENTRYDN)); - } + throw new RuntimeException("BUG: found group with nsaccountlock set: " + + result.getAttributeValue(LDAP_ENTRYDN)); + } - String entryDN = result.getAttributeValue(LDAP_ENTRYDN); - String groupName = result.getAttributeValue(LDAP_CN); - LocalAuthority localAuthority = new LocalAuthority(); - URI gmsServiceID = localAuthority.getServiceURI(Standards.GMS_GROUPS_01.toString()); - if (attributes == PUB_GROUP_ATTRS) - { - GroupURI groupID = new GroupURI(gmsServiceID.toString() + "?" + groupName); - return new Group(groupID); - } + String entryDN = result.getAttributeValue(LDAP_ENTRYDN); + String groupName = result.getAttributeValue(LDAP_CN); + LocalAuthority localAuthority = new LocalAuthority(); + URI gmsServiceID = localAuthority.getServiceURI(Standards.GMS_GROUPS_01.toString()); + if (attributes == PUB_GROUP_ATTRS) + { + GroupURI groupID = new GroupURI(gmsServiceID.toString() + "?" + groupName); + return new Group(groupID); + } - String ownerDN = result.getAttributeValue(LDAP_OWNER); - if (ownerDN == null) - { - throw new AccessControlException(groupName); - } - try + String ownerDN = result.getAttributeValue(LDAP_OWNER); + if (ownerDN == null) + { + throw new AccessControlException(groupName); + } + try + { + User owner = userDAO.getUser(new DNPrincipal(ownerDN)); + GroupURI groupID = new GroupURI(gmsServiceID.toString() + "?" + groupName); + Group group = new Group(groupID); + setField(group, owner, LDAP_OWNER); + if (result.hasAttribute(LDAP_DESCRIPTION)) { - User owner = userDAO.getUser(new DNPrincipal(ownerDN)); - GroupURI groupID = new GroupURI(gmsServiceID.toString() + "?" + groupName); - Group group = new Group(groupID); - setField(group, owner, LDAP_OWNER); - if (result.hasAttribute(LDAP_DESCRIPTION)) - { - group.description = result.getAttributeValue(LDAP_DESCRIPTION); - } - if (result.hasAttribute(LDAP_MODIFY_TIMESTAMP)) - { - group.lastModified = result.getAttributeValueAsDate(LDAP_MODIFY_TIMESTAMP); - } - return group; + group.description = result.getAttributeValue(LDAP_DESCRIPTION); } - catch (UserNotFoundException ex) + if (result.hasAttribute(LDAP_MODIFY_TIMESTAMP)) { - throw new RuntimeException("Invalid state: owner does not exist: " + - ownerDN + " group: " + entryDN); + group.lastModified = result.getAttributeValueAsDate(LDAP_MODIFY_TIMESTAMP); } + return group; } - catch (URISyntaxException e) + catch (UserNotFoundException ex) { - logger.error("invalid group URI", e); - throw new IllegalStateException("Invalid group URI", e); + throw new RuntimeException("Invalid state: owner does not exist: " + + ownerDN + " group: " + entryDN); } } diff --git a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java index e656f769736a45d02f94fa1a2a3d53c76e08ac0a..10951087e293e362c51507c2fe915dd6eeffcf27 100755 --- a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java +++ b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java @@ -788,16 +788,8 @@ public class LdapUserDAO extends LdapDAO String[] parts = cn.split("="); if (parts.length == 2 && parts[0].equals("cn")) { - try - { - GroupURI groupID = new GroupURI(gmsServiceURI.toString() + "?" + parts[1]); - return new Group(groupID); - } - catch (URISyntaxException e) - { - logger.error("Illegal Group ID", e); - throw new IllegalStateException("Illegal Group ID", e); - } + GroupURI groupID = new GroupURI(gmsServiceURI.toString() + "?" + parts[1]); + return new Group(groupID); } throw new RuntimeException("BUG: failed to extract group name from " + groupDN .toString()); diff --git a/cadc-access-control/src/main/java/ca/nrc/cadc/ac/GroupURI.java b/cadc-access-control/src/main/java/ca/nrc/cadc/ac/GroupURI.java index dec7e92031502ee59549d5c38498dda29fdaa786..a0c22191d07a0be36eaa7e40db53f6377e0a0f54 100644 --- a/cadc-access-control/src/main/java/ca/nrc/cadc/ac/GroupURI.java +++ b/cadc-access-control/src/main/java/ca/nrc/cadc/ac/GroupURI.java @@ -82,10 +82,8 @@ public class GroupURI { private static Logger log = Logger.getLogger(GroupURI.class); - public static final String SCHEME = "ivo"; - public static final String PATH = "/gms"; - private URI uri; + private String name; /** * Attempts to create a URI using the specified uri. The is expected @@ -104,26 +102,12 @@ public class GroupURI throw new IllegalArgumentException("Null URI"); } - String fragment = uri.getFragment(); - if (fragment != null && fragment.length() > 0) - { - throw new IllegalArgumentException("Fragment not allowed in group URIs"); - } - - try - { - this.uri = new URI(uri.getScheme(), uri.getAuthority(), - uri.getPath(), uri.getQuery(), fragment); - } - catch (URISyntaxException e) - { - throw new IllegalArgumentException("URI malformed: " + uri.toString()); - } + this.uri = uri; // Ensure the scheme is correct - if (uri.getScheme() == null || !uri.getScheme().equalsIgnoreCase(SCHEME)) + if (uri.getScheme() == null) { - throw new IllegalArgumentException("GroupURI scheme must be " + SCHEME); + throw new IllegalArgumentException("GroupURI scheme is required."); } if (uri.getAuthority() == null) @@ -131,20 +115,38 @@ public class GroupURI throw new IllegalArgumentException("Group authority is required."); } - if (uri.getPath() == null || !uri.getPath().equalsIgnoreCase(PATH)) + if (uri.getPath() == null || uri.getPath().length() == 0) { - if (PATH.contains(uri.getAuthority())) + throw new IllegalArgumentException("Missing authority and/or path."); + } + + log.debug("URI: " + uri); + log.debug(" scheme: " + uri.getScheme()); + log.debug(" authority: " + uri.getAuthority()); + log.debug(" path: " + uri.getPath()); + + String fragment = uri.getFragment(); + String query = uri.getQuery(); + if (query == null) + { + if (fragment != null) { - throw new IllegalArgumentException("Missing authority"); + // allow the fragment to define the group name (old style) + name = fragment; + } + else + { + throw new IllegalArgumentException("Group name is required."); } - throw new IllegalArgumentException("GroupURI path must be " + PATH); } - - if (uri.getQuery() == null) + else { - throw new IllegalArgumentException("Group name is required."); + if (fragment != null) + { + throw new IllegalArgumentException("Fragment not allowed in group URIs"); + } + name = query; } - } /** @@ -152,9 +154,8 @@ public class GroupURI * that takes a URI object. */ public GroupURI(String uri) - throws URISyntaxException { - this(new URI(uri)); + this(URI.create(uri)); } @Override @@ -167,7 +168,7 @@ public class GroupURI if (rhs instanceof GroupURI) { GroupURI vu = (GroupURI) rhs; - return uri.equals(vu.uri); + return uri.toString().equals(vu.uri.toString()); } return false; } @@ -199,7 +200,7 @@ public class GroupURI */ public String getName() { - return uri.getQuery(); + return name; } public URI getServiceID() @@ -222,7 +223,7 @@ public class GroupURI @Override public String toString() { - return uri.toString(); + return getServiceID() + "?" + name; } } diff --git a/cadc-access-control/src/main/java/ca/nrc/cadc/ac/xml/AbstractReaderWriter.java b/cadc-access-control/src/main/java/ca/nrc/cadc/ac/xml/AbstractReaderWriter.java index 8a0921c588cb80c74ba49276e4930618a4dcab95..d0095d841b9ac8935562bb02c57a6b4eddef73e8 100644 --- a/cadc-access-control/src/main/java/ca/nrc/cadc/ac/xml/AbstractReaderWriter.java +++ b/cadc-access-control/src/main/java/ca/nrc/cadc/ac/xml/AbstractReaderWriter.java @@ -489,19 +489,7 @@ public abstract class AbstractReaderWriter user = getUser(userElement); } - GroupURI groupURI = null; - try - { - groupURI = new GroupURI(uri); - } - catch (URISyntaxException e) - { - throw new ReaderException("Invalid uri: " + uri + ": " + e.getMessage()); - } - catch (IllegalArgumentException e) - { - throw new ReaderException("Invalid group uri: " + uri + ": " + e.getMessage()); - } + GroupURI groupURI = new GroupURI(uri); Group group = new Group(groupURI); // set owner field diff --git a/cadc-access-control/src/test/java/ca/nrc/cadc/ac/GroupTest.java b/cadc-access-control/src/test/java/ca/nrc/cadc/ac/GroupTest.java index 52b72d041c36ad284cd066148299524193cb429c..a7fd1e8276068bce7084ebcf90dbe7d603ad3fd0 100644 --- a/cadc-access-control/src/test/java/ca/nrc/cadc/ac/GroupTest.java +++ b/cadc-access-control/src/test/java/ca/nrc/cadc/ac/GroupTest.java @@ -165,7 +165,7 @@ public class GroupTest { new Group(new GroupURI("ivo://example.org/New%Test%Group")); } - catch(URISyntaxException e) + catch(IllegalArgumentException e) { thrown = true; } @@ -176,7 +176,7 @@ public class GroupTest { new Group(new GroupURI("ivo://example.org/New\\Test\\Group")); } - catch(URISyntaxException e) + catch(IllegalArgumentException e) { thrown = true; } diff --git a/cadc-access-control/src/test/java/ca/nrc/cadc/ac/GroupURITest.java b/cadc-access-control/src/test/java/ca/nrc/cadc/ac/GroupURITest.java index e1d7e4c75a21713128f379321ed9a3f9a9956079..423f7df9f070bf2d8771c67a31bdb40e3adf28f0 100644 --- a/cadc-access-control/src/test/java/ca/nrc/cadc/ac/GroupURITest.java +++ b/cadc-access-control/src/test/java/ca/nrc/cadc/ac/GroupURITest.java @@ -15,7 +15,7 @@ public class GroupURITest static { - Log4jInit.setLevel("ca.nrc.cadc.ac", Level.INFO); + Log4jInit.setLevel("ca.nrc.cadc.ac", Level.DEBUG); } @Test @@ -23,17 +23,14 @@ public class GroupURITest { try { - // wrong scheme - assertIllegalArgument("iko://cadc.nrc.ca/gms?gname", "scheme"); - - // fragment instead of query - assertIllegalArgument("ivo://cadc.nrc.ca/gms#gname", "fragment"); + // no scheme + assertIllegalArgument("example.org/gms?gname", "scheme"); // no authority assertIllegalArgument("ivo://gms?gname", "authority"); - // extended path in group - assertIllegalArgument("ivo://cadc.nrc.ca/gms/path?gname", "path"); + // no path + assertIllegalArgument("ivo://example.org/gname", "name"); } catch (Throwable t) { @@ -43,7 +40,7 @@ public class GroupURITest } @Test - public void testCorrect() + public void testCorrect1() { try { @@ -53,6 +50,26 @@ public class GroupURITest Assert.assertEquals("/gms", g.getURI().getPath()); Assert.assertEquals("name", g.getName()); Assert.assertEquals("ivo://my.authority/gms", g.getServiceID().toString()); + Assert.assertEquals("ivo://my.authority/gms?name", g.toString()); + } + catch (Throwable t) + { + log.error("Test Failed", t); + } + } + + @Test + public void testCorrect2() + { + try + { + GroupURI g = new GroupURI("ivo://my.authority/gms#name"); + Assert.assertEquals("ivo", g.getURI().getScheme()); + Assert.assertEquals("my.authority", g.getAuthority()); + Assert.assertEquals("/gms", g.getURI().getPath()); + Assert.assertEquals("name", g.getName()); + Assert.assertEquals("ivo://my.authority/gms", g.getServiceID().toString()); + Assert.assertEquals("ivo://my.authority/gms?name", g.toString()); } catch (Throwable t) {