From 3f22fd6663ccd37b599330df208cb4253c9e82cb Mon Sep 17 00:00:00 2001
From: Brian Major <major.brian@gmail.com>
Date: Mon, 7 Nov 2016 11:27:34 -0800
Subject: [PATCH] issue-10 - more work towards full group URI use.
---
.../nrc/cadc/ac/server/ldap/LdapGroupDAO.java | 73 ++++++++-----------
.../nrc/cadc/ac/server/ldap/LdapUserDAO.java | 12 +--
.../main/java/ca/nrc/cadc/ac/GroupURI.java | 67 ++++++++---------
.../nrc/cadc/ac/xml/AbstractReaderWriter.java | 14 +---
.../test/java/ca/nrc/cadc/ac/GroupTest.java | 4 +-
.../java/ca/nrc/cadc/ac/GroupURITest.java | 35 ++++++---
6 files changed, 97 insertions(+), 108 deletions(-)
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 e63a3ab5..da3dcb80 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 e656f769..10951087 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 dec7e920..a0c22191 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 8a0921c5..d0095d84 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 52b72d04..a7fd1e82 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 e1d7e4c7..423f7df9 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)
{
--
GitLab