From 1fd509e184bf0c664b4f45f69adf3b2d97b989b1 Mon Sep 17 00:00:00 2001
From: Brian Major <major.brian@gmail.com>
Date: Fri, 4 Nov 2016 13:24:17 -0700
Subject: [PATCH] issue-10 - Group Identifiers now represented by GroupURI
 objects

---
 cadc-access-control-server/build.gradle       |   4 +-
 .../nrc/cadc/ac/server/ldap/LdapGroupDAO.java | 133 +++++-----
 .../ac/server/ldap/LdapGroupPersistence.java  |  12 +-
 .../nrc/cadc/ac/server/ldap/LdapUserDAO.java  |  52 ++--
 .../web/groups/AddGroupMemberAction.java      |  19 +-
 .../web/groups/AddUserMemberAction.java       |   2 +-
 .../server/web/groups/CreateGroupAction.java  |   4 +-
 .../server/web/groups/DeleteGroupAction.java  |   2 +-
 .../server/web/groups/ModifyGroupAction.java  |   6 +-
 .../web/groups/RemoveGroupMemberAction.java   |  26 +-
 .../web/groups/RemoveUserMemberAction.java    |   2 +-
 .../cadc/ac/server/ldap/LdapGroupDAOTest.java |  39 +--
 .../server/web/ModifyPasswordServletTest.java |   6 +-
 .../ac/server/web/UserLoginServletTest.java   |  44 ++--
 .../web/groups/AddGroupMemberActionTest.java  |  23 +-
 .../web/groups/AddUserMemberActionTest.java   |   7 +-
 .../web/groups/DeleteGroupActionTest.java     |  15 +-
 .../groups/RemoveGroupMemberActionTest.java   |  20 +-
 .../groups/RemoveUserMemberActionTest.java    |   5 +-
 cadc-access-control/build.gradle              |   2 +-
 .../src/main/java/ca/nrc/cadc/ac/Group.java   |  46 ++--
 .../main/java/ca/nrc/cadc/ac/GroupURI.java    | 228 ++++++++++++++++++
 .../java/ca/nrc/cadc/ac/client/GMSClient.java | 132 +++++-----
 .../ca/nrc/cadc/ac/client/GMSClientMain.java  |  67 +++--
 .../nrc/cadc/ac/xml/AbstractReaderWriter.java |  34 ++-
 .../test/java/ca/nrc/cadc/ac/GroupTest.java   |  22 +-
 .../java/ca/nrc/cadc/ac/GroupURITest.java     |  77 ++++++
 .../ca/nrc/cadc/ac/client/GMSClientTest.java  |  66 +++--
 .../ac/json/JsonGroupReaderWriterTest.java    |   9 +-
 .../ac/xml/GroupListReaderWriterTest.java     |   5 +-
 .../cadc/ac/xml/GroupReaderWriterTest.java    |   9 +-
 31 files changed, 758 insertions(+), 360 deletions(-)
 create mode 100644 cadc-access-control/src/main/java/ca/nrc/cadc/ac/GroupURI.java
 create mode 100644 cadc-access-control/src/test/java/ca/nrc/cadc/ac/GroupURITest.java

diff --git a/cadc-access-control-server/build.gradle b/cadc-access-control-server/build.gradle
index be6ac301..a595894f 100644
--- a/cadc-access-control-server/build.gradle
+++ b/cadc-access-control-server/build.gradle
@@ -13,7 +13,7 @@ repositories {
 sourceCompatibility = 1.7
 group = 'org.opencadc'
 
-version = '1.0.2'
+version = '1.0.3'
 
 dependencies {
     compile 'log4j:log4j:1.2.+'
@@ -22,7 +22,7 @@ dependencies {
     compile 'xerces:xercesImpl:2.+'
     compile 'com.unboundid:unboundid-ldapsdk:2.3.+'
 
-    compile 'org.opencadc:cadc-access-control:1.+'
+    compile 'org.opencadc:cadc-access-control:[1.0.2,)'
     compile 'org.opencadc:cadc-util:1.+'
     compile 'org.opencadc:cadc-log:1.+'
     compile 'org.opencadc:cadc-registry:1.+'
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 d615643e..e63a3ab5 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
@@ -69,6 +69,8 @@
 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;
@@ -79,18 +81,6 @@ import java.util.Set;
 
 import org.apache.log4j.Logger;
 
-import ca.nrc.cadc.ac.ActivatedGroup;
-import ca.nrc.cadc.ac.Group;
-import ca.nrc.cadc.ac.GroupAlreadyExistsException;
-import ca.nrc.cadc.ac.GroupNotFoundException;
-import ca.nrc.cadc.ac.User;
-import ca.nrc.cadc.ac.UserNotFoundException;
-import ca.nrc.cadc.ac.server.GroupDetailSelector;
-import ca.nrc.cadc.auth.DNPrincipal;
-import ca.nrc.cadc.net.TransientException;
-import ca.nrc.cadc.profiler.Profiler;
-import ca.nrc.cadc.util.StringUtil;
-
 import com.unboundid.ldap.sdk.AddRequest;
 import com.unboundid.ldap.sdk.Attribute;
 import com.unboundid.ldap.sdk.DN;
@@ -110,6 +100,21 @@ import com.unboundid.ldap.sdk.SearchResultListener;
 import com.unboundid.ldap.sdk.SearchResultReference;
 import com.unboundid.ldap.sdk.SearchScope;
 
+import ca.nrc.cadc.ac.ActivatedGroup;
+import ca.nrc.cadc.ac.Group;
+import ca.nrc.cadc.ac.GroupAlreadyExistsException;
+import ca.nrc.cadc.ac.GroupNotFoundException;
+import ca.nrc.cadc.ac.GroupURI;
+import ca.nrc.cadc.ac.User;
+import ca.nrc.cadc.ac.UserNotFoundException;
+import ca.nrc.cadc.ac.server.GroupDetailSelector;
+import ca.nrc.cadc.auth.DNPrincipal;
+import ca.nrc.cadc.net.TransientException;
+import ca.nrc.cadc.profiler.Profiler;
+import ca.nrc.cadc.reg.Standards;
+import ca.nrc.cadc.reg.client.LocalAuthority;
+import ca.nrc.cadc.util.StringUtil;
+
 public class LdapGroupDAO extends LdapDAO
 {
     private static final Logger logger = Logger.getLogger(LdapGroupDAO.class);
@@ -193,16 +198,16 @@ public class LdapGroupDAO extends LdapDAO
             else
             {
                 // add group to groups tree
-                LDAPResult result = addGroup(getGroupDN(group.getID()),
-                                             group.getID(), ownerDN,
+                LDAPResult result = addGroup(getGroupDN(group.getID().getName()),
+                                             group.getID().getName(), ownerDN,
                                              group.description,
                                              group.getUserMembers(),
                                              group.getGroupMembers());
                 LdapDAO.checkLdapResult(result.getResultCode());
 
                 // add group to admin groups tree
-                result = addGroup(getAdminGroupDN(group.getID()),
-                                  group.getID(), ownerDN,
+                result = addGroup(getAdminGroupDN(group.getID().getName()),
+                                  group.getID().getName(), ownerDN,
                                   group.description,
                                   group.getUserAdmins(),
                                   group.getGroupAdmins());
@@ -245,7 +250,7 @@ public class LdapGroupDAO extends LdapDAO
         }
         for (Group groupMember : groups)
         {
-            final String groupMemberID = groupMember.getID();
+            final String groupMemberID = groupMember.getID().getName();
             if (!checkGroupExists(groupMemberID))
             {
                 throw new GroupNotFoundException(groupMemberID);
@@ -285,9 +290,9 @@ public class LdapGroupDAO extends LdapDAO
         try
         {
             // check group name exists
-            Filter filter = Filter.createEqualityFilter(LDAP_CN, group.getID());
+            Filter filter = Filter.createEqualityFilter(LDAP_CN, group.getID().getName());
 
-            DN groupDN = getGroupDN(group.getID());
+            DN groupDN = getGroupDN(group.getID().getName());
             SearchRequest searchRequest =
                     new SearchRequest(groupDN.toNormalizedString(), SearchScope.BASE,
                                       filter, new String[]{LDAP_NSACCOUNTLOCK});
@@ -537,7 +542,7 @@ public class LdapGroupDAO extends LdapDAO
             throws GroupNotFoundException, TransientException,
                    AccessControlException, UserNotFoundException
     {
-        String groupID = group.getID();
+        String groupID = group.getID().getName();
         getGroup(getGroupDN(groupID), groupID, PUB_GROUP_ATTRS);//group must exists first
         return modifyGroup(group, false);
     }
@@ -580,11 +585,11 @@ public class LdapGroupDAO extends LdapDAO
             }
             for (Group gr : group.getGroupMembers())
             {
-                if (!checkGroupExists(gr.getID()))
+                if (!checkGroupExists(gr.getID().getName()))
                 {
-                    throw new GroupNotFoundException(gr.getID());
+                    throw new GroupNotFoundException(gr.getID().getName());
                 }
-                DN grDN = getGroupDN(gr.getID());
+                DN grDN = getGroupDN(gr.getID().getName());
                 newMembers.add(grDN.toNormalizedString());
             }
 
@@ -596,11 +601,11 @@ public class LdapGroupDAO extends LdapDAO
             }
             for (Group gr : group.getGroupAdmins())
             {
-                if (!checkGroupExists(gr.getID()))
+                if (!checkGroupExists(gr.getID().getName()))
                 {
-                    throw new GroupNotFoundException(gr.getID());
+                    throw new GroupNotFoundException(gr.getID().getName());
                 }
-                DN grDN = getGroupDN(gr.getID());
+                DN grDN = getGroupDN(gr.getID().getName());
                 newAdmins.add(grDN.toNormalizedString());
             }
 
@@ -610,7 +615,7 @@ public class LdapGroupDAO extends LdapDAO
                                  (String[]) newAdmins.toArray(new String[newAdmins.size()])));
 
             ModifyRequest adminModify =
-                    new ModifyRequest(getAdminGroupDN(group.getID()), adminMods);
+                    new ModifyRequest(getAdminGroupDN(group.getID().getName()), adminMods);
 
             LdapDAO.checkLdapResult(
                 getReadWriteConnection().modify(adminModify).getResultCode());
@@ -621,7 +626,7 @@ public class LdapGroupDAO extends LdapDAO
                                  (String[]) newMembers.toArray(new String[newMembers.size()])));
 
             ModifyRequest modifyRequest =
-                new ModifyRequest(getGroupDN(group.getID()), mods);
+                new ModifyRequest(getGroupDN(group.getID().getName()), mods);
 
             LdapDAO.checkLdapResult(
                 getReadWriteConnection().modify(modifyRequest).getResultCode());
@@ -635,11 +640,11 @@ public class LdapGroupDAO extends LdapDAO
         {
             if (withActivate)
             {
-                return new ActivatedGroup(getGroup(group.getID(), true));
+                return new ActivatedGroup(getGroup(group.getID().getName(), true));
             }
             else
             {
-                return getGroup(group.getID(), true);
+                return getGroup(group.getID().getName(), true);
             }
         }
         catch (GroupNotFoundException e)
@@ -748,43 +753,55 @@ public class LdapGroupDAO extends LdapDAO
     private Group createGroupFromSearchResult(SearchResultEntry result, String[] attributes)
             throws LDAPException, TransientException
     {
-        if (result.getAttribute(LDAP_NSACCOUNTLOCK) != null)
+        try
         {
-            throw new RuntimeException("BUG: found group with nsaccountlock set: " +
-                                        result.getAttributeValue(LDAP_ENTRYDN));
-        }
+            if (result.getAttribute(LDAP_NSACCOUNTLOCK) != null)
+            {
+                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);
-        if (attributes == PUB_GROUP_ATTRS)
-        {
-            return new Group(groupName);
-        }
+            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
-        {
-            User owner = userDAO.getUser(new DNPrincipal(ownerDN));
-            Group group = new Group(groupName);
-            setField(group, owner, LDAP_OWNER);
-            if (result.hasAttribute(LDAP_DESCRIPTION))
+            String ownerDN = result.getAttributeValue(LDAP_OWNER);
+            if (ownerDN == null)
+            {
+                throw new AccessControlException(groupName);
+            }
+            try
             {
-                group.description = result.getAttributeValue(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;
             }
-            if (result.hasAttribute(LDAP_MODIFY_TIMESTAMP))
+            catch (UserNotFoundException ex)
             {
-                group.lastModified = result.getAttributeValueAsDate(LDAP_MODIFY_TIMESTAMP);
+                throw new RuntimeException("Invalid state: owner does not exist: " +
+                                            ownerDN + " group: " + entryDN);
             }
-            return group;
         }
-        catch (UserNotFoundException ex)
+        catch (URISyntaxException e)
         {
-            throw new RuntimeException("Invalid state: owner does not exist: " +
-                                        ownerDN + " group: " + entryDN);
+            logger.error("invalid group URI", e);
+            throw new IllegalStateException("Invalid group URI", e);
         }
     }
 
diff --git a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/ldap/LdapGroupPersistence.java b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/ldap/LdapGroupPersistence.java
index 2871975b..250df4f5 100755
--- a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/ldap/LdapGroupPersistence.java
+++ b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/ldap/LdapGroupPersistence.java
@@ -223,7 +223,7 @@ public class LdapGroupPersistence extends LdapPersistence implements GroupPersis
                AccessControlException, UserNotFoundException
     {
         Subject callerSubject = AuthenticationUtil.getCurrentSubject();
-        boolean allowed = isAdmin(callerSubject, group.getID());
+        boolean allowed = isAdmin(callerSubject, group.getID().getName());
 
         LdapGroupDAO groupDAO = null;
         LdapUserDAO userDAO = null;
@@ -234,7 +234,7 @@ public class LdapGroupPersistence extends LdapPersistence implements GroupPersis
             groupDAO = new LdapGroupDAO(conns, userDAO);
             if (!allowed)
             {
-                Group g = groupDAO.getGroup(group.getID(), false);
+                Group g = groupDAO.getGroup(group.getID().getName(), false);
                 if (isOwner(callerSubject, g))
                     allowed = true;
             }
@@ -287,13 +287,13 @@ public class LdapGroupPersistence extends LdapPersistence implements GroupPersis
                 while ( i.hasNext() )
                 {
                     Group g = i.next();
-                    if (groupID == null || g.getID().equalsIgnoreCase(groupID))
+                    if (groupID == null || g.getID().getName().equalsIgnoreCase(groupID))
                     {
                         if (detailSelector != null && detailSelector.isDetailedSearch(g, role))
                         {
                             try
                             {
-                                Group g2 = groupDAO.getGroup(g.getID(), false);
+                                Group g2 = groupDAO.getGroup(g.getID().getName(), false);
                                 log.debug("role " + role + " loaded: " + g2);
                                 ret.add(g2);
                             }
@@ -340,7 +340,7 @@ public class LdapGroupPersistence extends LdapPersistence implements GroupPersis
         List<Group> groups = getGroupCache(caller, Role.MEMBER);
         for (Group g : groups)
         {
-            if (g.getID().equalsIgnoreCase(groupName))
+            if (g.getID().getName().equalsIgnoreCase(groupName))
                 return true;
         }
         return false;
@@ -351,7 +351,7 @@ public class LdapGroupPersistence extends LdapPersistence implements GroupPersis
         List<Group> groups = getGroupCache(caller, Role.ADMIN);
         for (Group g : groups)
         {
-            if (g.getID().equalsIgnoreCase(groupName))
+            if (g.getID().getName().equalsIgnoreCase(groupName))
                 return true;
         }
         return false;
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 bc9cb5d9..e656f769 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
@@ -86,25 +86,6 @@ import javax.security.auth.x500.X500Principal;
 
 import org.apache.log4j.Logger;
 
-import ca.nrc.cadc.ac.Group;
-import ca.nrc.cadc.ac.InternalID;
-import ca.nrc.cadc.ac.PersonalDetails;
-import ca.nrc.cadc.ac.Role;
-import ca.nrc.cadc.ac.User;
-import ca.nrc.cadc.ac.UserAlreadyExistsException;
-import ca.nrc.cadc.ac.UserNotFoundException;
-import ca.nrc.cadc.ac.UserRequest;
-import ca.nrc.cadc.ac.client.GroupMemberships;
-import ca.nrc.cadc.auth.DNPrincipal;
-import ca.nrc.cadc.auth.HttpPrincipal;
-import ca.nrc.cadc.auth.NumericPrincipal;
-import ca.nrc.cadc.net.TransientException;
-import ca.nrc.cadc.profiler.Profiler;
-import ca.nrc.cadc.reg.Standards;
-import ca.nrc.cadc.reg.client.LocalAuthority;
-import ca.nrc.cadc.util.ObjectUtil;
-import ca.nrc.cadc.util.StringUtil;
-
 import com.unboundid.ldap.sdk.AddRequest;
 import com.unboundid.ldap.sdk.Attribute;
 import com.unboundid.ldap.sdk.BindRequest;
@@ -129,6 +110,26 @@ import com.unboundid.ldap.sdk.SimpleBindRequest;
 import com.unboundid.ldap.sdk.extensions.PasswordModifyExtendedRequest;
 import com.unboundid.ldap.sdk.extensions.PasswordModifyExtendedResult;
 
+import ca.nrc.cadc.ac.Group;
+import ca.nrc.cadc.ac.GroupURI;
+import ca.nrc.cadc.ac.InternalID;
+import ca.nrc.cadc.ac.PersonalDetails;
+import ca.nrc.cadc.ac.Role;
+import ca.nrc.cadc.ac.User;
+import ca.nrc.cadc.ac.UserAlreadyExistsException;
+import ca.nrc.cadc.ac.UserNotFoundException;
+import ca.nrc.cadc.ac.UserRequest;
+import ca.nrc.cadc.ac.client.GroupMemberships;
+import ca.nrc.cadc.auth.DNPrincipal;
+import ca.nrc.cadc.auth.HttpPrincipal;
+import ca.nrc.cadc.auth.NumericPrincipal;
+import ca.nrc.cadc.net.TransientException;
+import ca.nrc.cadc.profiler.Profiler;
+import ca.nrc.cadc.reg.Standards;
+import ca.nrc.cadc.reg.client.LocalAuthority;
+import ca.nrc.cadc.util.ObjectUtil;
+import ca.nrc.cadc.util.StringUtil;
+
 
 /**
  *
@@ -781,11 +782,22 @@ public class LdapUserDAO extends LdapDAO
     // some pretty horrible hacks to avoid querying LDAP for group details...
     private Group createGroupFromDN(DN groupDN)
     {
+        LocalAuthority localAuthority = new LocalAuthority();
+        URI gmsServiceURI = localAuthority.getServiceURI(Standards.GMS_GROUPS_01.toString());
         String cn = groupDN.getRDNString();
         String[] parts = cn.split("=");
         if (parts.length == 2 && parts[0].equals("cn"))
         {
-            return new Group(parts[1]);
+            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);
+            }
         }
         throw new RuntimeException("BUG: failed to extract group name from " + groupDN
                 .toString());
diff --git a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/AddGroupMemberAction.java b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/AddGroupMemberAction.java
index b0d4d536..51af9f53 100755
--- a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/AddGroupMemberAction.java
+++ b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/AddGroupMemberAction.java
@@ -68,12 +68,16 @@
  */
 package ca.nrc.cadc.ac.server.web.groups;
 
-import ca.nrc.cadc.ac.Group;
-import ca.nrc.cadc.ac.GroupAlreadyExistsException;
-import ca.nrc.cadc.ac.server.GroupPersistence;
+import java.net.URI;
 import java.util.ArrayList;
 import java.util.List;
 
+import ca.nrc.cadc.ac.Group;
+import ca.nrc.cadc.ac.GroupAlreadyExistsException;
+import ca.nrc.cadc.ac.GroupURI;
+import ca.nrc.cadc.reg.Standards;
+import ca.nrc.cadc.reg.client.LocalAuthority;
+
 public class AddGroupMemberAction extends AbstractGroupAction
 {
     private final String groupName;
@@ -90,7 +94,10 @@ public class AddGroupMemberAction extends AbstractGroupAction
     public void doAction() throws Exception
     {
         Group group = groupPersistence.getGroup(this.groupName);
-        Group toAdd = new Group(this.groupMemberName);
+        LocalAuthority localAuthority = new LocalAuthority();
+        URI gmsServiceURI = localAuthority.getServiceURI(Standards.GMS_GROUPS_01.toString());
+        GroupURI toAddID = new GroupURI(gmsServiceURI.toString() + "?" + this.groupMemberName);
+        Group toAdd = new Group(toAddID);
         if (!group.getGroupMembers().add(toAdd))
         {
             throw new GroupAlreadyExistsException(this.groupMemberName);
@@ -98,8 +105,8 @@ public class AddGroupMemberAction extends AbstractGroupAction
         groupPersistence.modifyGroup(group);
 
         List<String> addedMembers = new ArrayList<String>();
-        addedMembers.add(toAdd.getID());
-        logGroupInfo(group.getID(), null, addedMembers);
+        addedMembers.add(toAdd.getID().getName());
+        logGroupInfo(group.getID().getName(), null, addedMembers);
     }
 
 }
diff --git a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/AddUserMemberAction.java b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/AddUserMemberAction.java
index e0281883..90bfdfad 100755
--- a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/AddUserMemberAction.java
+++ b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/AddUserMemberAction.java
@@ -113,7 +113,7 @@ public class AddUserMemberAction extends AbstractGroupAction
 
         List<String> addedMembers = new ArrayList<String>();
         addedMembers.add(getUseridForLogging(toAdd));
-        logGroupInfo(group.getID(), null, addedMembers);
+        logGroupInfo(group.getID().getName(), null, addedMembers);
     }
 
 }
diff --git a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/CreateGroupAction.java b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/CreateGroupAction.java
index 851ca7cd..5d811969 100755
--- a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/CreateGroupAction.java
+++ b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/CreateGroupAction.java
@@ -104,7 +104,7 @@ public class CreateGroupAction extends AbstractGroupAction
             addedMembers = new ArrayList<String>();
             for (Group gr : group.getGroupMembers())
             {
-                addedMembers.add(gr.getID());
+                addedMembers.add(gr.getID().getName());
             }
             for (User usr : group.getUserMembers())
             {
@@ -116,7 +116,7 @@ public class CreateGroupAction extends AbstractGroupAction
                 addedMembers.add(p.getName());
             }
         }
-        logGroupInfo(group.getID(), null, addedMembers);
+        logGroupInfo(group.getID().getName(), null, addedMembers);
     }
 
 }
diff --git a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/DeleteGroupAction.java b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/DeleteGroupAction.java
index e75f01a2..ae6c46d6 100755
--- a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/DeleteGroupAction.java
+++ b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/DeleteGroupAction.java
@@ -92,7 +92,7 @@ public class DeleteGroupAction extends AbstractGroupAction
             this.logInfo.deletedMembers = new ArrayList<String>();
             for (Group gr : deletedGroup.getGroupMembers())
             {
-                this.logInfo.deletedMembers.add(gr.getID());
+                this.logInfo.deletedMembers.add(gr.getID().getName());
             }
             for (User usr : deletedGroup.getUserMembers())
             {
diff --git a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/ModifyGroupAction.java b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/ModifyGroupAction.java
index 631e28f5..431bf246 100755
--- a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/ModifyGroupAction.java
+++ b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/ModifyGroupAction.java
@@ -114,7 +114,7 @@ public class ModifyGroupAction extends AbstractGroupAction
         {
             if (!oldGroup.getGroupMembers().remove(gr))
             {
-                addedMembers.add(gr.getID());
+                addedMembers.add(gr.getID().getName());
             }
         }
         if (addedMembers.isEmpty())
@@ -128,13 +128,13 @@ public class ModifyGroupAction extends AbstractGroupAction
         }
         for (Group gr : oldGroup.getGroupMembers())
         {
-            deletedMembers.add(gr.getID());
+            deletedMembers.add(gr.getID().getName());
         }
         if (deletedMembers.isEmpty())
         {
             deletedMembers = null;
         }
-        logGroupInfo(group.getID(), deletedMembers, addedMembers);
+        logGroupInfo(group.getID().getName(), deletedMembers, addedMembers);
         profiler.checkpoint("log GroupInfo");
 
         syncOut.setHeader("Location", request);
diff --git a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/RemoveGroupMemberAction.java b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/RemoveGroupMemberAction.java
index 5336a603..a6542e19 100755
--- a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/RemoveGroupMemberAction.java
+++ b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/RemoveGroupMemberAction.java
@@ -68,14 +68,22 @@
  */
 package ca.nrc.cadc.ac.server.web.groups;
 
-import ca.nrc.cadc.ac.Group;
-import ca.nrc.cadc.ac.GroupNotFoundException;
-
+import java.net.URI;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.apache.log4j.Logger;
+
+import ca.nrc.cadc.ac.Group;
+import ca.nrc.cadc.ac.GroupNotFoundException;
+import ca.nrc.cadc.ac.GroupURI;
+import ca.nrc.cadc.reg.Standards;
+import ca.nrc.cadc.reg.client.LocalAuthority;
+
 public class RemoveGroupMemberAction extends AbstractGroupAction
 {
+    private final static Logger log = Logger.getLogger(RemoveGroupMemberAction.class);
+
     private final String groupName;
     private final String groupMemberName;
 
@@ -89,7 +97,13 @@ public class RemoveGroupMemberAction extends AbstractGroupAction
     public void doAction() throws Exception
     {
         Group group = groupPersistence.getGroup(this.groupName);
-        Group toRemove = new Group(this.groupMemberName);
+        LocalAuthority localAuthority = new LocalAuthority();
+        URI gmsServiceURI = localAuthority.getServiceURI(Standards.GMS_GROUPS_01.toString());
+        GroupURI toRemoveID = new GroupURI(gmsServiceURI.toString() + "?" + this.groupMemberName);
+        Group toRemove = new Group(toRemoveID);
+
+        log.debug("group member count: " + group.getGroupMembers().size());
+        log.debug("contains one to remove: " + group.getGroupMembers().contains(toRemove));
 
         if (!group.getGroupMembers().remove(toRemove))
         {
@@ -98,8 +112,8 @@ public class RemoveGroupMemberAction extends AbstractGroupAction
         groupPersistence.modifyGroup(group);
 
         List<String> deletedMembers = new ArrayList<String>();
-        deletedMembers.add(toRemove.getID());
-        logGroupInfo(group.getID(), deletedMembers, null);
+        deletedMembers.add(toRemove.getID().getName());
+        logGroupInfo(group.getID().getName(), deletedMembers, null);
     }
 
 }
diff --git a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/RemoveUserMemberAction.java b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/RemoveUserMemberAction.java
index b5b6960f..d4439ab1 100755
--- a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/RemoveUserMemberAction.java
+++ b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/groups/RemoveUserMemberAction.java
@@ -110,7 +110,7 @@ public class RemoveUserMemberAction extends AbstractGroupAction
 
         List<String> deletedMembers = new ArrayList<String>();
         deletedMembers.add(getUseridForLogging(user));
-        logGroupInfo(group.getID(), deletedMembers, null);
+        logGroupInfo(group.getID().getName(), deletedMembers, null);
     }
 
     protected UserPersistence getUserPersistence()
diff --git a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java
index 2390292e..cb45ecb3 100644
--- a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java
+++ b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java
@@ -87,6 +87,7 @@ import org.junit.Test;
 import ca.nrc.cadc.ac.Group;
 import ca.nrc.cadc.ac.GroupNotFoundException;
 import ca.nrc.cadc.ac.GroupProperty;
+import ca.nrc.cadc.ac.GroupURI;
 import ca.nrc.cadc.ac.User;
 import ca.nrc.cadc.util.Log4jInit;
 import ca.nrc.cadc.util.PropertiesReader;
@@ -130,17 +131,17 @@ public class LdapGroupDAOTest extends AbstractLdapDAOTest
             {
                 try
                 {
-                    Group expectGroup = new Group(getGroupID());
+                    Group expectGroup = new Group(new GroupURI("ivo://example.org/gms?" + getGroupID()));
                     setField(expectGroup, cadcDaoTest1_AugmentedUser, "owner");
                     getGroupDAO().addGroup(expectGroup);
-                    Group actualGroup = getGroupDAO().getGroup(expectGroup.getID(), true);
+                    Group actualGroup = getGroupDAO().getGroup(expectGroup.getID().getName(), true);
                     log.info("addGroup: " + expectGroup.getID());
                     assertGroupsEqual(expectGroup, actualGroup);
 
-                    Group otherGroup = new Group(getGroupID());
+                    Group otherGroup = new Group(new GroupURI("ivo://example.org/gms?" + getGroupID()));
                     setField(otherGroup, cadcDaoTest1_AugmentedUser, "owner");
                     getGroupDAO().addGroup(otherGroup);
-                    otherGroup = getGroupDAO().getGroup(otherGroup.getID(), true);
+                    otherGroup = getGroupDAO().getGroup(otherGroup.getID().getName(), true);
                     log.info("addGroup: " + otherGroup.getID());
 
                     // modify group fields
@@ -197,10 +198,10 @@ public class LdapGroupDAOTest extends AbstractLdapDAOTest
                     assertGroupsEqual(expectGroup, actualGroup);
 
                     // groupAdmins
-                    Group adminGroup = new Group(getGroupID());
+                    Group adminGroup = new Group(new GroupURI("ivo://example.org/gms?" + getGroupID()));
                     setField(adminGroup, cadcDaoTest1_AugmentedUser, "owner");
                     getGroupDAO().addGroup(adminGroup);
-                    adminGroup = getGroupDAO().getGroup(adminGroup.getID(), true);
+                    adminGroup = getGroupDAO().getGroup(adminGroup.getID().getName(), true);
                     expectGroup.getGroupAdmins().add(adminGroup);
                     actualGroup = getGroupDAO().modifyGroup(expectGroup);
                     assertGroupsEqual(expectGroup, actualGroup);
@@ -220,10 +221,10 @@ public class LdapGroupDAOTest extends AbstractLdapDAOTest
                     assertGroupsEqual(expectGroup, actualGroup);
 
                     // delete the group
-                    getGroupDAO().deleteGroup(expectGroup.getID());
+                    getGroupDAO().deleteGroup(expectGroup.getID().getName());
                     try
                     {
-                        getGroupDAO().getGroup(expectGroup.getID(), false);
+                        getGroupDAO().getGroup(expectGroup.getID().getName(), false);
                         fail("get on deleted group should throw exception");
                     }
                     catch (GroupNotFoundException ignore) {}
@@ -234,7 +235,7 @@ public class LdapGroupDAOTest extends AbstractLdapDAOTest
                     setField(reactGroup, cadcDaoTest1_AugmentedUser, "owner");
                     getGroupDAO().addGroup(reactGroup);
                     log.info("create (reactivate) group: " + expectGroup.getID());
-                    actualGroup = getGroupDAO().getGroup(expectGroup.getID(), true);
+                    actualGroup = getGroupDAO().getGroup(expectGroup.getID().getName(), true);
                     assertTrue(actualGroup.getUserMembers().isEmpty());
                     assertTrue(actualGroup.getGroupMembers().isEmpty());
                     assertTrue(actualGroup.getUserAdmins().isEmpty());
@@ -243,22 +244,22 @@ public class LdapGroupDAOTest extends AbstractLdapDAOTest
 
                     // create another group and make expected group
                     // member of that group. Delete expected group after
-                    Group expectGroup2 = new Group(getGroupID());
+                    Group expectGroup2 = new Group(new GroupURI("ivo://example.org/gms?" + getGroupID()));
                     setField(expectGroup2, cadcDaoTest1_AugmentedUser, "owner");
                     expectGroup2.getGroupAdmins().add(actualGroup);
                     expectGroup2.getGroupMembers().add(actualGroup);
                     getGroupDAO().addGroup(expectGroup2);
-                    Group actualGroup2 = getGroupDAO().getGroup(expectGroup2.getID(), true);
+                    Group actualGroup2 = getGroupDAO().getGroup(expectGroup2.getID().getName(), true);
                     log.debug("addGroup: " + expectGroup2.getID());
                     assertGroupsEqual(expectGroup2, actualGroup2);
 
                     // delete the group
-                    getGroupDAO().deleteGroup(actualGroup.getID());
+                    getGroupDAO().deleteGroup(actualGroup.getID().getName());
 
                     // should not be member of admin of expectGroup2
                     expectGroup2.getGroupAdmins().remove(actualGroup);
                     expectGroup2.getGroupMembers().remove(actualGroup);
-                    actualGroup2 = getGroupDAO().getGroup(expectGroup2.getID(), true);
+                    actualGroup2 = getGroupDAO().getGroup(expectGroup2.getID().getName(), true);
                     log.debug("addGroup: " + expectGroup2.getID());
                     assertGroupsEqual(expectGroup2, actualGroup2);
 
@@ -286,14 +287,14 @@ public class LdapGroupDAOTest extends AbstractLdapDAOTest
             {
                 try
                 {
-                    Group testGroup1 = new Group(testGroup1ID);
+                    Group testGroup1 = new Group(new GroupURI("ivo://example.org/gms?" + testGroup1ID));
                     getGroupDAO().addGroup(testGroup1);
-                    testGroup1 = getGroupDAO().getGroup(testGroup1.getID(), true);
+                    testGroup1 = getGroupDAO().getGroup(testGroup1.getID().getName(), true);
                     log.debug("add group: " + testGroup1ID);
 
-                    Group testGroup2 = new Group(testGroup2ID);
+                    Group testGroup2 = new Group(new GroupURI("ivo://example.org/gms?" + testGroup2ID));
                     getGroupDAO().addGroup(testGroup2);
-                    testGroup2 = getGroupDAO().getGroup(testGroup2.getID(), true);
+                    testGroup2 = getGroupDAO().getGroup(testGroup2.getID().getName(), true);
                     log.debug("add group: " + testGroup2ID);
                     //Thread.sleep(1000); // sleep to let memberof plugin do its work
                 }
@@ -391,7 +392,7 @@ public class LdapGroupDAOTest extends AbstractLdapDAOTest
                 }
                 catch (GroupNotFoundException ignore) {}
 
-                getGroupDAO().addGroup(new Group(groupID));
+                getGroupDAO().addGroup(new Group(new GroupURI("ivo://example.org/gms?" + groupID)));
                 return null;
             }
         });
@@ -409,7 +410,7 @@ public class LdapGroupDAOTest extends AbstractLdapDAOTest
                 //getGroupDAO().addGroup(new Group(groupID, cadcDaoTest1_User));
                 try
                 {
-                    getGroupDAO().modifyGroup(new Group("fooBOGUSASFgomsi"));
+                    getGroupDAO().modifyGroup(new Group(new GroupURI("ivo://example.org/gms?" + "fooBOGUSASFgomsi")));
                     fail("modifyGroup with unknown user should throw " +
                          "GroupNotFoundException");
                 }
diff --git a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/ModifyPasswordServletTest.java b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/ModifyPasswordServletTest.java
index ff87ffcb..afab3654 100644
--- a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/ModifyPasswordServletTest.java
+++ b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/ModifyPasswordServletTest.java
@@ -83,11 +83,13 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.apache.log4j.Level;
+import org.apache.log4j.Logger;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
 import ca.nrc.cadc.ac.server.UserPersistence;
+import ca.nrc.cadc.ac.server.web.groups.GroupActionFactoryTest;
 import ca.nrc.cadc.auth.HttpPrincipal;
 import ca.nrc.cadc.util.Log4jInit;
 import ca.nrc.cadc.util.PropertiesReader;
@@ -97,6 +99,8 @@ import ca.nrc.cadc.util.StringUtil;
 public class ModifyPasswordServletTest
 {
 
+    private final static Logger log = Logger.getLogger(GroupActionFactoryTest.class);
+
     public ModifyPasswordServletTest()
     {
         Log4jInit.setLevel("ca.nrc.cadc.ac", Level.INFO);
@@ -207,7 +211,7 @@ public class ModifyPasswordServletTest
         mockUserPersistence.setPassword(userID, oldPassword, newPassword);
         if (hasInternalServerError)
         {
-            expectLastCall().andThrow(new RuntimeException());
+            expectLastCall().andThrow(new RuntimeException("intentional"));
         }
 
         final Subject subject = new Subject();
diff --git a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/UserLoginServletTest.java b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/UserLoginServletTest.java
index b2927d90..4930f1a6 100644
--- a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/UserLoginServletTest.java
+++ b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/UserLoginServletTest.java
@@ -16,6 +16,7 @@ import org.easymock.EasyMock;
 import org.junit.Test;
 
 import ca.nrc.cadc.ac.Group;
+import ca.nrc.cadc.ac.GroupURI;
 import ca.nrc.cadc.ac.Role;
 import ca.nrc.cadc.ac.server.GroupDetailSelector;
 import ca.nrc.cadc.ac.server.ldap.LdapGroupPersistence;
@@ -77,27 +78,28 @@ public class UserLoginServletTest
             @Override
             protected LdapGroupPersistence getLdapGroupPersistence()
             {
-                proxyGroup = "proxyGroup";
-                nonImpersonGroup = "niGroup";
-                Collection<Group> proxyGroups = new HashSet<Group>();
-                proxyGroups.add(new Group(proxyGroup));
-                Collection<Group> niGroups = new HashSet<Group>();
-                niGroups.add(new Group(nonImpersonGroup));
-                // mock returns a shell instance
-                @SuppressWarnings("unchecked")
-                LdapGroupPersistence mockGp =
-                    (LdapGroupPersistence)EasyMock
-                        .createMock(LdapGroupPersistence.class);
-                mockGp.setDetailSelector(new GroupDetailSelector()
-                {
-                    @Override
-                    public boolean isDetailedSearch(Group g, Role r)
-                    {
-                        return false;
-                    }
-                });
                 try
                 {
+                    proxyGroup = "proxyGroup";
+                    nonImpersonGroup = "niGroup";
+                    Collection<Group> proxyGroups = new HashSet<Group>();
+                    proxyGroups.add(new Group(new GroupURI("ivo://example.com/gms?" + proxyGroup)));
+                    Collection<Group> niGroups = new HashSet<Group>();
+                    niGroups.add(new Group(new GroupURI("ivo://example.com/gms?" + nonImpersonGroup)));
+                    // mock returns a shell instance
+                    @SuppressWarnings("unchecked")
+                    LdapGroupPersistence mockGp =
+                        (LdapGroupPersistence)EasyMock
+                            .createMock(LdapGroupPersistence.class);
+                    mockGp.setDetailSelector(new GroupDetailSelector()
+                    {
+                        @Override
+                        public boolean isDetailedSearch(Group g, Role r)
+                        {
+                            return false;
+                        }
+                    });
+
                     EasyMock.expect(
                             mockGp.getGroups( //new HttpPrincipal("proxyUser"),
                                     Role.MEMBER, proxyGroup)).andReturn(
@@ -115,11 +117,13 @@ public class UserLoginServletTest
                                     Role.MEMBER, nonImpersonGroup)).andReturn(
                             niGroups);
                     replay(mockGp);
+
+                    return mockGp;
                 } catch (Exception e)
                 {
                     throw new RuntimeException(e);
                 }
-                return mockGp;
+
             }
         };
 
diff --git a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/AddGroupMemberActionTest.java b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/AddGroupMemberActionTest.java
index f278f250..6537cc36 100644
--- a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/AddGroupMemberActionTest.java
+++ b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/AddGroupMemberActionTest.java
@@ -73,6 +73,8 @@ import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.replay;
 import static org.junit.Assert.fail;
 
+import java.net.URI;
+
 import org.apache.log4j.Level;
 import org.apache.log4j.Logger;
 import org.easymock.EasyMock;
@@ -81,7 +83,10 @@ import org.junit.Test;
 
 import ca.nrc.cadc.ac.Group;
 import ca.nrc.cadc.ac.GroupAlreadyExistsException;
+import ca.nrc.cadc.ac.GroupURI;
 import ca.nrc.cadc.ac.server.GroupPersistence;
+import ca.nrc.cadc.reg.Standards;
+import ca.nrc.cadc.reg.client.LocalAuthority;
 import ca.nrc.cadc.util.Log4jInit;
 
 /**
@@ -103,13 +108,16 @@ public class AddGroupMemberActionTest
     {
         try
         {
-            Group group = new Group("group");
-            Group member = new Group("member");
+            LocalAuthority localAuthority = new LocalAuthority();
+            URI gmsServiceURI = localAuthority.getServiceURI(Standards.GMS_GROUPS_01.toString());
+
+            Group group = new Group(new GroupURI(gmsServiceURI + "?group"));
+            Group member = new Group(new GroupURI(gmsServiceURI + "?member"));
             group.getGroupMembers().add(member);
 
             final GroupPersistence groupPersistence = createMock(GroupPersistence.class);
             expect(groupPersistence.getGroup("group")).andReturn(group);
-            expect(groupPersistence.getGroup("member")).andReturn(member);
+            //expect(groupPersistence.getGroup("member")).andReturn(member);
             replay(groupPersistence);
 
             AddGroupMemberAction action = new AddGroupMemberAction("group", "member");
@@ -134,9 +142,12 @@ public class AddGroupMemberActionTest
     {
         try
         {
-            Group group = new Group("group");
-            Group member = new Group("member");
-            Group modified = new Group("group");
+            LocalAuthority localAuthority = new LocalAuthority();
+            URI gmsServiceURI = localAuthority.getServiceURI(Standards.GMS_GROUPS_01.toString());
+
+            Group group = new Group(new GroupURI(gmsServiceURI + "?group"));
+            Group member = new Group(new GroupURI(gmsServiceURI + "?member"));
+            Group modified = new Group(new GroupURI(gmsServiceURI + "?group"));
             modified.getGroupMembers().add(member);
 
             final GroupPersistence groupPersistence =
diff --git a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/AddUserMemberActionTest.java b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/AddUserMemberActionTest.java
index b40e0cd3..11e2ad5e 100644
--- a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/AddUserMemberActionTest.java
+++ b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/AddUserMemberActionTest.java
@@ -80,6 +80,7 @@ import org.junit.BeforeClass;
 import org.junit.Test;
 
 import ca.nrc.cadc.ac.Group;
+import ca.nrc.cadc.ac.GroupURI;
 import ca.nrc.cadc.ac.MemberAlreadyExistsException;
 import ca.nrc.cadc.ac.User;
 import ca.nrc.cadc.ac.server.GroupPersistence;
@@ -113,7 +114,7 @@ public class AddUserMemberActionTest
             User user = new User();
             user.getIdentities().add(userPrincipal);
 
-            Group group = new Group("group");
+            Group group = new Group(new GroupURI("ivo://example.org/gms?group"));
             group.getUserMembers().add(user);
 
             final GroupPersistence groupPersistence = EasyMock.createMock(GroupPersistence.class);
@@ -149,8 +150,8 @@ public class AddUserMemberActionTest
             User user = new User();
             user.getIdentities().add(userPrincipal);
 
-            Group group = new Group("group");
-            Group modified = new Group("group");
+            Group group = new Group(new GroupURI("ivo://example.org/gms?group"));
+            Group modified = new Group(new GroupURI("ivo://example.org/gms?group"));
             modified.getUserMembers().add(user);
 
             final GroupPersistence groupPersistence = EasyMock.createMock(GroupPersistence.class);
diff --git a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/DeleteGroupActionTest.java b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/DeleteGroupActionTest.java
index 62abd28a..d90ee169 100644
--- a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/DeleteGroupActionTest.java
+++ b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/DeleteGroupActionTest.java
@@ -68,18 +68,19 @@
  */
 package ca.nrc.cadc.ac.server.web.groups;
 
-import ca.nrc.cadc.ac.Group;
-import ca.nrc.cadc.ac.server.GroupPersistence;
-import ca.nrc.cadc.util.Log4jInit;
-import java.security.Principal;
+import static org.easymock.EasyMock.createMock;
+import static org.junit.Assert.fail;
+
 import org.apache.log4j.Level;
 import org.apache.log4j.Logger;
 import org.easymock.EasyMock;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
-import static org.easymock.EasyMock.createMock;
-import static org.junit.Assert.*;
+import ca.nrc.cadc.ac.Group;
+import ca.nrc.cadc.ac.GroupURI;
+import ca.nrc.cadc.ac.server.GroupPersistence;
+import ca.nrc.cadc.util.Log4jInit;
 
 /**
  *
@@ -100,7 +101,7 @@ public class DeleteGroupActionTest
     {
         try
         {
-            Group group = new Group("group");
+            Group group = new Group(new GroupURI("ivo://example.org/gms?group"));
 
             final GroupPersistence groupPersistence = EasyMock.createMock(GroupPersistence.class);
             EasyMock.expect(groupPersistence.getGroup("group")).andReturn(group);
diff --git a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/RemoveGroupMemberActionTest.java b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/RemoveGroupMemberActionTest.java
index f2bfc058..f0bd60da 100644
--- a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/RemoveGroupMemberActionTest.java
+++ b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/RemoveGroupMemberActionTest.java
@@ -71,6 +71,8 @@ package ca.nrc.cadc.ac.server.web.groups;
 import static org.easymock.EasyMock.createMock;
 import static org.junit.Assert.fail;
 
+import java.net.URI;
+
 import org.apache.log4j.Level;
 import org.apache.log4j.Logger;
 import org.easymock.EasyMock;
@@ -79,7 +81,10 @@ import org.junit.Test;
 
 import ca.nrc.cadc.ac.Group;
 import ca.nrc.cadc.ac.GroupNotFoundException;
+import ca.nrc.cadc.ac.GroupURI;
 import ca.nrc.cadc.ac.server.GroupPersistence;
+import ca.nrc.cadc.reg.Standards;
+import ca.nrc.cadc.reg.client.LocalAuthority;
 import ca.nrc.cadc.util.Log4jInit;
 
 /**
@@ -93,7 +98,7 @@ public class RemoveGroupMemberActionTest
     @BeforeClass
     public static void setUpClass()
     {
-        Log4jInit.setLevel("ca.nrc.cadc.ac", Level.INFO);
+        Log4jInit.setLevel("ca.nrc.cadc.ac", Level.DEBUG);
     }
 
     @Test
@@ -101,8 +106,8 @@ public class RemoveGroupMemberActionTest
     {
         try
         {
-            Group group = new Group("group");
-            Group member = new Group("member");
+            Group group = new Group(new GroupURI("ivo://example.org/gms?group"));
+            Group member = new Group(new GroupURI("ivo://example.org/gms?member"));
 
             final GroupPersistence groupPersistence = EasyMock.createMock(GroupPersistence.class);
             EasyMock.expect(groupPersistence.getGroup("group")).andReturn(group);
@@ -131,11 +136,14 @@ public class RemoveGroupMemberActionTest
     {
         try
         {
-            Group member = new Group("member");
-            Group group = new Group("group");
+            LocalAuthority localAuthority = new LocalAuthority();
+            URI gmsServiceURI = localAuthority.getServiceURI(Standards.GMS_GROUPS_01.toString());
+
+            Group member = new Group(new GroupURI(gmsServiceURI.toString() + "?member"));
+            Group group = new Group(new GroupURI(gmsServiceURI.toString() + "?group"));
             group.getGroupMembers().add(member);
 
-            Group modified = new Group("group");
+            Group modified = new Group(new GroupURI(gmsServiceURI.toString() + "?group"));
             modified.getGroupMembers().add(member);
 
             final GroupPersistence groupPersistence = EasyMock.createMock(GroupPersistence.class);
diff --git a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/RemoveUserMemberActionTest.java b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/RemoveUserMemberActionTest.java
index c41e3437..92d0e3fc 100644
--- a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/RemoveUserMemberActionTest.java
+++ b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/RemoveUserMemberActionTest.java
@@ -85,6 +85,7 @@ import org.junit.BeforeClass;
 import org.junit.Test;
 
 import ca.nrc.cadc.ac.Group;
+import ca.nrc.cadc.ac.GroupURI;
 import ca.nrc.cadc.ac.InternalID;
 import ca.nrc.cadc.ac.MemberNotFoundException;
 import ca.nrc.cadc.ac.User;
@@ -137,7 +138,7 @@ public class RemoveUserMemberActionTest
             Principal x500Principal = AuthenticationUtil.createPrincipal(userID, userIDType);
             user.getIdentities().add(x500Principal);
 
-            Group group = new Group("group");
+            Group group = new Group(new GroupURI("ivo://example.org/gms?group"));
             User member = new User();
             member.getIdentities().add(new X500Principal("cn=bar,c=ca"));
             group.getUserMembers().add(member);
@@ -192,7 +193,7 @@ public class RemoveUserMemberActionTest
             user.getIdentities().add(new X500Principal(userID));
             user.getIdentities().add(new HttpPrincipal("foo"));
 
-            Group group = new Group("group");
+            Group group = new Group(new GroupURI("ivo://example.org/gms?group"));
             group.getUserMembers().add(user);
 
             final GroupPersistence mockGroupPersistence = EasyMock.createMock(GroupPersistence.class);
diff --git a/cadc-access-control/build.gradle b/cadc-access-control/build.gradle
index 31ddc1a7..9a8ec102 100644
--- a/cadc-access-control/build.gradle
+++ b/cadc-access-control/build.gradle
@@ -15,7 +15,7 @@ sourceCompatibility = 1.7
 
 group = 'org.opencadc'
 
-version = '1.0.1'
+version = '1.0.2'
 
 mainClassName = 'ca.nrc.cadc.ac.client.GMSClientMain'
 
diff --git a/cadc-access-control/src/main/java/ca/nrc/cadc/ac/Group.java b/cadc-access-control/src/main/java/ca/nrc/cadc/ac/Group.java
index c775729a..f60e252d 100644
--- a/cadc-access-control/src/main/java/ca/nrc/cadc/ac/Group.java
+++ b/cadc-access-control/src/main/java/ca/nrc/cadc/ac/Group.java
@@ -72,9 +72,13 @@ import java.util.Date;
 import java.util.HashSet;
 import java.util.Set;
 
+import org.apache.log4j.Logger;
+
 public class Group
 {
-    private String groupID;
+    private final static Logger log = Logger.getLogger(Group.class);
+
+    private GroupURI groupID;
 
     private User owner;
 
@@ -101,23 +105,16 @@ public class Group
     }
 
     /**
-     * Ctor.
+     * Group Constructor.
      *
-     * @param groupID   Unique ID for the group. Must be a valid URI fragment
-     *                  component, so it's restricted to alphanumeric
-     *                  and "-", ".","_","~" characters.
+     * @param groupID The URI identifying the group.
      */
-    public Group(String groupID)
+    public Group(GroupURI groupID)
     {
         if (groupID == null)
         {
             throw new IllegalArgumentException("null groupID");
         }
-        if (!groupID.matches("^[a-zA-Z0-9\\-\\.~_]*$"))
-        {
-            throw new IllegalArgumentException("Invalid group ID " + groupID +
-                    ": may not contain space ( ), slash (/), escape (\\), or percent (%)");
-        }
 
         this.groupID = groupID;
     }
@@ -127,7 +124,7 @@ public class Group
      *
      * @return String group ID.
      */
-    public String getID()
+    public GroupURI getID()
     {
         return groupID;
     }
@@ -186,15 +183,6 @@ public class Group
         return groupAdmins;
     }
 
-    /* (non-Javadoc)
-     * @see java.lang.Object#hashCode()
-     */
-    @Override
-    public int hashCode()
-    {
-        return 31 + groupID.toLowerCase().hashCode();
-    }
-
     /* (non-Javadoc)
      * @see java.lang.Object#equals(java.lang.Object)
      */
@@ -214,13 +202,27 @@ public class Group
             return false;
         }
         Group other = (Group) obj;
-        if (!groupID.equalsIgnoreCase(other.groupID))
+
+        if (!groupID.equals(other.groupID))
         {
             return false;
         }
         return true;
     }
 
+    /*
+     * @see java.lang.Object#hashCode()
+     */
+    @Override
+    public int hashCode()
+    {
+        if (groupID != null)
+        {
+            return 31 + groupID.getURI().toString().toLowerCase().hashCode();
+        }
+        return 0;
+    }
+
     @Override
     public String 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
new file mode 100644
index 00000000..dec7e920
--- /dev/null
+++ b/cadc-access-control/src/main/java/ca/nrc/cadc/ac/GroupURI.java
@@ -0,0 +1,228 @@
+/*
+************************************************************************
+*******************  CANADIAN ASTRONOMY DATA CENTRE  *******************
+**************  CENTRE CANADIEN DE DONNÉES ASTRONOMIQUES  **************
+*
+*  (c) 2009.                            (c) 2009.
+*  Government of Canada                 Gouvernement du Canada
+*  National Research Council            Conseil national de recherches
+*  Ottawa, Canada, K1A 0R6              Ottawa, Canada, K1A 0R6
+*  All rights reserved                  Tous droits réservés
+*
+*  NRC disclaims any warranties,        Le CNRC dénie toute garantie
+*  expressed, implied, or               énoncée, implicite ou légale,
+*  statutory, of any kind with          de quelque nature que ce
+*  respect to the software,             soit, concernant le logiciel,
+*  including without limitation         y compris sans restriction
+*  any warranty of merchantability      toute garantie de valeur
+*  or fitness for a particular          marchande ou de pertinence
+*  purpose. NRC shall not be            pour un usage particulier.
+*  liable in any event for any          Le CNRC ne pourra en aucun cas
+*  damages, whether direct or           être tenu responsable de tout
+*  indirect, special or general,        dommage, direct ou indirect,
+*  consequential or incidental,         particulier ou général,
+*  arising from the use of the          accessoire ou fortuit, résultant
+*  software.  Neither the name          de l'utilisation du logiciel. Ni
+*  of the National Research             le nom du Conseil National de
+*  Council of Canada nor the            Recherches du Canada ni les noms
+*  names of its contributors may        de ses  participants ne peuvent
+*  be used to endorse or promote        être utilisés pour approuver ou
+*  products derived from this           promouvoir les produits dérivés
+*  software without specific prior      de ce logiciel sans autorisation
+*  written permission.                  préalable et particulière
+*                                       par écrit.
+*
+*  This file is part of the             Ce fichier fait partie du projet
+*  OpenCADC project.                    OpenCADC.
+*
+*  OpenCADC is free software:           OpenCADC est un logiciel libre ;
+*  you can redistribute it and/or       vous pouvez le redistribuer ou le
+*  modify it under the terms of         modifier suivant les termes de
+*  the GNU Affero General Public        la “GNU Affero General Public
+*  License as published by the          License” telle que publiée
+*  Free Software Foundation,            par la Free Software Foundation
+*  either version 3 of the              : soit la version 3 de cette
+*  License, or (at your option)         licence, soit (à votre gré)
+*  any later version.                   toute version ultérieure.
+*
+*  OpenCADC is distributed in the       OpenCADC est distribué
+*  hope that it will be useful,         dans l’espoir qu’il vous
+*  but WITHOUT ANY WARRANTY;            sera utile, mais SANS AUCUNE
+*  without even the implied             GARANTIE : sans même la garantie
+*  warranty of MERCHANTABILITY          implicite de COMMERCIALISABILITÉ
+*  or FITNESS FOR A PARTICULAR          ni d’ADÉQUATION À UN OBJECTIF
+*  PURPOSE.  See the GNU Affero         PARTICULIER. Consultez la Licence
+*  General Public License for           Générale Publique GNU Affero
+*  more details.                        pour plus de détails.
+*
+*  You should have received             Vous devriez avoir reçu une
+*  a copy of the GNU Affero             copie de la Licence Générale
+*  General Public License along         Publique GNU Affero avec
+*  with OpenCADC.  If not, see          OpenCADC ; si ce n’est
+*  <http://www.gnu.org/licenses/>.      pas le cas, consultez :
+*                                       <http://www.gnu.org/licenses/>.
+*
+*  $Revision: 4 $
+*
+************************************************************************
+*/
+
+package ca.nrc.cadc.ac;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import org.apache.log4j.Logger;
+
+/**
+ * Identifier for a group.
+ *
+ */
+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;
+
+    /**
+     * Attempts to create a URI using the specified uri. The is expected
+     * to be in the format:
+     *
+     * ivo://<authority>/gms?<groupName>
+     *
+     * @param uri The URI to use.
+     * @throws IllegalArgumentException if the URI scheme is not vos
+     * @throws NullPointerException if uri is null
+     */
+    public GroupURI(URI uri)
+    {
+        if (uri == null)
+        {
+            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());
+        }
+
+        // Ensure the scheme is correct
+        if (uri.getScheme() == null || !uri.getScheme().equalsIgnoreCase(SCHEME))
+        {
+            throw new IllegalArgumentException("GroupURI scheme must be " + SCHEME);
+        }
+
+        if (uri.getAuthority() == null)
+        {
+            throw new IllegalArgumentException("Group authority is required.");
+        }
+
+        if (uri.getPath() == null || !uri.getPath().equalsIgnoreCase(PATH))
+        {
+            if (PATH.contains(uri.getAuthority()))
+            {
+                throw new IllegalArgumentException("Missing authority");
+            }
+            throw new IllegalArgumentException("GroupURI path must be " + PATH);
+        }
+
+        if (uri.getQuery() == null)
+        {
+            throw new IllegalArgumentException("Group name is required.");
+        }
+
+    }
+
+    /**
+     * Constructs a URI from the string and calls the constructor
+     * that takes a URI object.
+     */
+    public GroupURI(String uri)
+        throws URISyntaxException
+    {
+        this(new URI(uri));
+    }
+
+    @Override
+    public boolean equals(Object rhs)
+    {
+        if (rhs == null)
+            return false;
+        if (this == rhs)
+            return true;
+        if (rhs instanceof GroupURI)
+        {
+            GroupURI vu = (GroupURI) rhs;
+            return uri.equals(vu.uri);
+        }
+        return false;
+    }
+
+    /**
+     * Returns the underlying URI object.
+     *
+     * @return The URI object for this GroupURI.
+     */
+    public URI getURI()
+    {
+        return uri;
+    }
+
+    /**
+     * Returns the decoded authority component of the URI.
+     *
+     * @return authority of the URI, or null if the authority is undefined.
+     */
+    public String getAuthority()
+    {
+        return uri.getAuthority();
+    }
+
+    /**
+     * Returns the decoded fragment component of the URI.
+     *
+     * @return fragment of the URI, or null if the fragment is undefined.
+     */
+    public String getName()
+    {
+        return uri.getQuery();
+    }
+
+    public URI getServiceID()
+    {
+        String serviceID = uri.getScheme() +
+            "://" +
+            uri.getAuthority() +
+            uri.getPath();
+        try
+        {
+            return new URI(serviceID);
+        }
+        catch (URISyntaxException e)
+        {
+            log.error("Could not create service ID", e);
+            throw new IllegalStateException(e);
+        }
+    }
+
+    @Override
+    public String toString()
+    {
+        return uri.toString();
+    }
+
+}
diff --git a/cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/GMSClient.java b/cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/GMSClient.java
index dda5da8a..d51ccb31 100755
--- a/cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/GMSClient.java
+++ b/cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/GMSClient.java
@@ -89,12 +89,12 @@ import javax.net.ssl.HttpsURLConnection;
 import javax.net.ssl.SSLSocketFactory;
 import javax.security.auth.Subject;
 
-import ca.nrc.cadc.reg.Standards;
 import org.apache.log4j.Logger;
 
 import ca.nrc.cadc.ac.Group;
 import ca.nrc.cadc.ac.GroupAlreadyExistsException;
 import ca.nrc.cadc.ac.GroupNotFoundException;
+import ca.nrc.cadc.ac.GroupURI;
 import ca.nrc.cadc.ac.Role;
 import ca.nrc.cadc.ac.UserNotFoundException;
 import ca.nrc.cadc.ac.WriterException;
@@ -113,6 +113,7 @@ import ca.nrc.cadc.net.InputStreamWrapper;
 import ca.nrc.cadc.net.NetUtil;
 import ca.nrc.cadc.net.event.TransferEvent;
 import ca.nrc.cadc.net.event.TransferListener;
+import ca.nrc.cadc.reg.Standards;
 import ca.nrc.cadc.reg.client.RegistryClient;
 
 
@@ -128,20 +129,11 @@ public class GMSClient implements TransferListener
     private SSLSocketFactory sslSocketFactory;
     private SSLSocketFactory mySocketFactory;
 
-    private URI serviceID;
-
     /**
      * Constructor.
-     *
-     * @param serviceID            The service ID.
      */
-    public GMSClient(URI serviceID)
+    public GMSClient()
     {
-        if (serviceID == null)
-            throw new IllegalArgumentException("invalid serviceID: " + serviceID);
-        if (serviceID.getFragment() != null)
-            throw new IllegalArgumentException("invalid serviceID (fragment not allowed): " + serviceID);
-        this.serviceID = serviceID;
     }
 
     public void transferEvent(TransferEvent te)
@@ -181,8 +173,9 @@ public class GMSClient implements TransferListener
         throws GroupAlreadyExistsException, AccessControlException,
                UserNotFoundException, WriterException, IOException
     {
+
         URL createGroupURL = getRegistryClient()
-            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
+            .getServiceURL(group.getID().getServiceID(), Standards.GMS_GROUPS_01, AuthMethod.CERT);
         log.debug("createGroupURL request to " + createGroupURL.toString());
 
         // reset the state of the cache
@@ -250,12 +243,13 @@ public class GMSClient implements TransferListener
      * @throws AccessControlException If unauthorized to perform this operation.
      * @throws java.io.IOException
      */
-    public Group getGroup(String groupName)
+    public Group getGroup(GroupURI groupID)
         throws GroupNotFoundException, AccessControlException, IOException
     {
+
         URL groupsURL = getRegistryClient()
-            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
-        URL getGroupURL = new URL(groupsURL.toExternalForm() + "/" + groupName);
+            .getServiceURL(groupID.getServiceID(), Standards.GMS_GROUPS_01, AuthMethod.CERT);
+        URL getGroupURL = new URL(groupsURL.toExternalForm() + "/" + groupID.getName());
         log.debug("getGroup request to " + getGroupURL.toString());
 
         ByteArrayOutputStream out = new ByteArrayOutputStream();
@@ -306,11 +300,11 @@ public class GMSClient implements TransferListener
      * @throws AccessControlException If unauthorized to perform this operation.
      * @throws java.io.IOException
      */
-    public List<String> getGroupNames()
+    public List<String> getGroupNames(URI serviceID)
         throws AccessControlException, IOException
     {
         URL getGroupNamesURL = getRegistryClient()
-            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
+            .getServiceURL(serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
 
         log.debug("getGroupNames request to " + getGroupNamesURL.toString());
 
@@ -388,8 +382,8 @@ public class GMSClient implements TransferListener
                AccessControlException, WriterException, IOException
     {
         URL groupsURL = getRegistryClient()
-            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
-        URL updateGroupURL = new URL(groupsURL.toExternalForm() + "/" + group.getID());
+            .getServiceURL(group.getID().getServiceID(), Standards.GMS_GROUPS_01, AuthMethod.CERT);
+        URL updateGroupURL = new URL(groupsURL.toExternalForm() + "/" + group.getID().getName());
         log.debug("updateGroup request to " + updateGroupURL.toString());
 
         // reset the state of the cache
@@ -453,12 +447,12 @@ public class GMSClient implements TransferListener
      * @throws AccessControlException If unauthorized to perform this operation.
      * @throws java.io.IOException
      */
-    public void deleteGroup(String groupName)
+    public void deleteGroup(GroupURI groupID)
         throws GroupNotFoundException, AccessControlException, IOException
     {
         URL groupsURL = getRegistryClient()
-            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
-        URL deleteGroupURL = new URL(groupsURL.toExternalForm() + "/" + groupName);
+            .getServiceURL(groupID.getServiceID(), Standards.GMS_GROUPS_01, AuthMethod.CERT);
+        URL deleteGroupURL = new URL(groupsURL.toExternalForm() + "/" + groupID.getName());
         log.debug("deleteGroup request to " + deleteGroupURL.toString());
 
         // reset the state of the cache
@@ -519,14 +513,14 @@ public class GMSClient implements TransferListener
      * @throws AccessControlException If unauthorized to perform this operation.
      * @throws java.io.IOException
      */
-    public void addGroupMember(String targetGroupName, String groupMemberName)
+    public void addGroupMember(GroupURI targetGroup, String groupMemberName)
         throws IllegalArgumentException, GroupNotFoundException,
                AccessControlException, IOException
     {
 
-        String path = "/" + targetGroupName + "/groupMembers/" + groupMemberName;
+        String path = "/" + targetGroup.getName() + "/groupMembers/" + groupMemberName;
         URL groupsURL = getRegistryClient()
-            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
+            .getServiceURL(targetGroup.getServiceID(), Standards.GMS_GROUPS_01, AuthMethod.CERT);
         URL addGroupMemberURL = new URL(groupsURL.toExternalForm() + path);
         log.debug("addGroupMember request to " + addGroupMemberURL.toString());
 
@@ -566,28 +560,28 @@ public class GMSClient implements TransferListener
     /**
      * Add a user as a member of a group.
      *
-     * @param targetGroupName The group in which to add the group member.
+     * @param targetGroup The group in which to add the group member.
      * @param userID The user to add.
      * @throws GroupNotFoundException If the group was not found.
      * @throws UserNotFoundException If the member was not found.
      * @throws java.io.IOException
      * @throws AccessControlException If unauthorized to perform this operation.
      */
-    public void addUserMember(String targetGroupName, Principal userID)
+    public void addUserMember(GroupURI targetGroup, Principal userID)
         throws GroupNotFoundException, UserNotFoundException, AccessControlException, IOException
     {
-        if (targetGroupName == null)
-            throw new IllegalArgumentException("targetGroupName required");
+        if (targetGroup == null)
+            throw new IllegalArgumentException("targetGroup required");
 
         if (userID == null)
             throw new IllegalArgumentException("userID required");
 
-        log.debug("addUserMember: " + targetGroupName + " + " + userID.getName());
+        log.debug("addUserMember: " + targetGroup + " + " + userID.getName());
 
         String userIDType = AuthenticationUtil.getPrincipalType(userID);
-        String path = "/" + targetGroupName + "/userMembers/" + NetUtil.encode(userID.getName()) + "?idType=" + userIDType;
+        String path = "/" + targetGroup.getName() + "/userMembers/" + NetUtil.encode(userID.getName()) + "?idType=" + userIDType;
         URL groupsURL = getRegistryClient()
-            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
+            .getServiceURL(targetGroup.getServiceID(), Standards.GMS_GROUPS_01, AuthMethod.CERT);
         URL addUserMemberURL = new URL(groupsURL.toExternalForm() + path);
 
         log.debug("addUserMember request to " + addUserMemberURL.toString());
@@ -631,20 +625,20 @@ public class GMSClient implements TransferListener
     /**
      * Remove a group as a member of another group.
      *
-     * @param targetGroupName The group from which to remove the group member.
+     * @param targetGroup The group from which to remove the group member.
      * @param groupMemberName The group member to remove.
      * @throws GroupNotFoundException If the group was not found.
      * @throws java.io.IOException
      * @throws AccessControlException If unauthorized to perform this operation.
      */
-    public void removeGroupMember(String targetGroupName,
+    public void removeGroupMember(GroupURI targetGroup,
                                   String groupMemberName)
         throws GroupNotFoundException, AccessControlException, IOException
     {
 
-        String path = "/" + targetGroupName + "/groupMembers/" + groupMemberName;
+        String path = "/" + targetGroup.getName() + "/groupMembers/" + groupMemberName;
         URL groupsURL = getRegistryClient()
-            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
+            .getServiceURL(targetGroup.getServiceID(), Standards.GMS_GROUPS_01, AuthMethod.CERT);
         URL removeGroupMemberURL = new URL(groupsURL.toExternalForm() + path);
         log.debug("removeGroupMember request to " +
                   removeGroupMemberURL.toString());
@@ -705,15 +699,15 @@ public class GMSClient implements TransferListener
      * @throws java.io.IOException
      * @throws AccessControlException If unauthorized to perform this operation.
      */
-    public void removeUserMember(String targetGroupName, Principal userID)
+    public void removeUserMember(GroupURI targetGroup, Principal userID)
         throws GroupNotFoundException, UserNotFoundException, AccessControlException, IOException
     {
         String userIDType = AuthenticationUtil.getPrincipalType(userID);
 
-        log.debug("removeUserMember: " + targetGroupName + " - " + userID.getName() + " type: " + userIDType);
-        String path = "/" + targetGroupName + "/userMembers/" + NetUtil.encode(userID.getName()) + "?idType=" + userIDType;
+        log.debug("removeUserMember: " + targetGroup + " - " + userID.getName() + " type: " + userIDType);
+        String path = "/" + targetGroup.getName() + "/userMembers/" + NetUtil.encode(userID.getName()) + "?idType=" + userIDType;
         URL groupsURL = getRegistryClient()
-            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
+            .getServiceURL(targetGroup.getServiceID(), Standards.GMS_GROUPS_01, AuthMethod.CERT);
         URL removeUserMemberURL = new URL(groupsURL.toExternalForm() + path);
 
         log.debug("removeUserMember: " + removeUserMemberURL.toString());
@@ -789,14 +783,14 @@ public class GMSClient implements TransferListener
      * @throws ca.nrc.cadc.ac.UserNotFoundException
      * @throws java.io.IOException
      */
-    public List<Group> getMemberships(Role role)
+    public List<Group> getMemberships(URI serviceID, Role role)
         throws UserNotFoundException, AccessControlException, IOException
     {
-        return getMemberships(null, role);
+        return getMemberships(serviceID, null, role);
     }
 
 
-    private List<Group> getMemberships(Principal ignore, Role role)
+    private List<Group> getMemberships(URI serviceID, Principal ignore, Role role)
         throws UserNotFoundException, AccessControlException, IOException
     {
         if (role == null)
@@ -807,7 +801,7 @@ public class GMSClient implements TransferListener
         Principal userID = getCurrentUserID();
         if (userID != null)
         {
-            List<Group> cachedGroups = getCachedGroups(userID, role, true);
+            List<Group> cachedGroups = getCachedGroups(serviceID, userID, role, true);
             if (cachedGroups != null)
             {
                 return cachedGroups;
@@ -825,7 +819,7 @@ public class GMSClient implements TransferListener
         searchGroupPath.append("&ROLE=").append(NetUtil.encode(roleString));
 
         URL searchURL = getRegistryClient()
-            .getServiceURL(this.serviceID, Standards.GMS_SEARCH_01, AuthMethod.CERT);
+            .getServiceURL(serviceID, Standards.GMS_SEARCH_01, AuthMethod.CERT);
         URL getMembershipsURL = new URL(searchURL.toExternalForm() + searchGroupPath.toString());
 
         log.debug("getMemberships request to " + getMembershipsURL.toString());
@@ -863,7 +857,7 @@ public class GMSClient implements TransferListener
             log.debug("getMemberships returned: " + groupsXML);
             GroupListReader groupListReader = new GroupListReader();
             List<Group> groups = groupListReader.read(groupsXML);
-            setCachedGroups(userID, groups, role);
+            setCachedGroups(serviceID, userID, groups, role);
             return groups;
         }
         catch (Exception bug)
@@ -880,17 +874,17 @@ public class GMSClient implements TransferListener
      *
      * This call is identical to getMemberShip(userID, groupName, Role.MEMBER)
      *
-     * @param groupName Identifies the group.
+     * @param groupID Identifies the group.
      * @return The group or null of the user is not a member.
      * @throws UserNotFoundException If the user does not exist.
      * @throws AccessControlException If not allowed to peform the search.
      * @throws IllegalArgumentException If a parameter is null.
      * @throws IOException If an unknown error occured.
      */
-    public Group getMembership(String groupName)
+    public Group getMembership(GroupURI groupID)
         throws UserNotFoundException, AccessControlException, IOException
     {
-        return getMembership(groupName, Role.MEMBER);
+        return getMembership(groupID, Role.MEMBER);
     }
 
     /**
@@ -906,10 +900,10 @@ public class GMSClient implements TransferListener
      * @throws IllegalArgumentException If a parameter is null.
      * @throws IOException If an unknown error occured.
      */
-    public Group getMembership(String groupName, Role role)
+    public Group getMembership(GroupURI groupID, Role role)
         throws UserNotFoundException, AccessControlException, IOException
     {
-        if (groupName == null || role == null)
+        if (groupID == null || role == null)
         {
             throw new IllegalArgumentException("groupName and role are required.");
         }
@@ -917,7 +911,7 @@ public class GMSClient implements TransferListener
         Principal userID = getCurrentUserID();
         if (userID != null)
         {
-            Group cachedGroup = getCachedGroup(userID, groupName, role);
+            Group cachedGroup = getCachedGroup(userID, groupID, role);
             if (cachedGroup != null)
             {
                 return cachedGroup;
@@ -933,10 +927,10 @@ public class GMSClient implements TransferListener
         //searchGroupURL.append("ID=").append(NetUtil.encode(id));
         //searchGroupURL.append("&IDTYPE=").append(NetUtil.encode(idType));
         searchGroupPath.append("&ROLE=").append(NetUtil.encode(roleString));
-        searchGroupPath.append("&GROUPID=").append(NetUtil.encode(groupName));
+        searchGroupPath.append("&GROUPID=").append(NetUtil.encode(groupID.getName()));
 
         URL searchURL = getRegistryClient()
-            .getServiceURL(this.serviceID, Standards.GMS_SEARCH_01, AuthMethod.CERT);
+            .getServiceURL(groupID.getServiceID(), Standards.GMS_SEARCH_01, AuthMethod.CERT);
         URL getMembershipURL = new URL(searchURL.toExternalForm() + searchGroupPath.toString());
 
         log.debug("getMembership request to " + getMembershipURL.toString());
@@ -985,7 +979,7 @@ public class GMSClient implements TransferListener
                 return ret;
             }
             throw new IllegalStateException(
-                    "Duplicate membership for " + userID + " in group " + groupName);
+                    "Duplicate membership for " + userID + " in group " + groupID);
         }
         catch (Exception bug)
         {
@@ -1003,10 +997,10 @@ public class GMSClient implements TransferListener
      * @throws AccessControlException
      * @throws IOException
      */
-    public boolean isMember(String groupName)
+    public boolean isMember(GroupURI groupID)
         throws UserNotFoundException, AccessControlException, IOException
     {
-        return isMember(groupName, Role.MEMBER);
+        return isMember(groupID, Role.MEMBER);
     }
 
     /**
@@ -1018,16 +1012,16 @@ public class GMSClient implements TransferListener
      * @throws AccessControlException
      * @throws IOException
      */
-    public boolean isMember(String groupName, Role role)
+    public boolean isMember(GroupURI groupID, Role role)
         throws UserNotFoundException, AccessControlException, IOException
     {
-        return isMember(getCurrentUserID(), groupName, role);
+        return isMember(getCurrentUserID(), groupID, role);
     }
 
-    private boolean isMember(Principal userID, String groupName, Role role)
+    private boolean isMember(Principal userID, GroupURI groupID, Role role)
         throws UserNotFoundException, AccessControlException, IOException
     {
-        Group group = getMembership(groupName, role);
+        Group group = getMembership(groupID, role);
         return group != null;
     }
 
@@ -1083,7 +1077,7 @@ public class GMSClient implements TransferListener
         }
     }
 
-    protected GroupMemberships getGroupCache(Principal userID)
+    protected GroupMemberships getGroupCache(URI serviceID, Principal userID)
     {
         AccessControlContext acContext = AccessController.getContext();
         Subject subject = Subject.getSubject(acContext);
@@ -1113,9 +1107,9 @@ public class GMSClient implements TransferListener
         return null; // no cache
     }
 
-    protected Group getCachedGroup(Principal userID, String groupID, Role role)
+    protected Group getCachedGroup(Principal userID, GroupURI groupID, Role role)
     {
-        List<Group> groups = getCachedGroups(userID, role, false);
+        List<Group> groups = getCachedGroups(groupID.getServiceID(), userID, role, false);
         if (groups == null)
             return null; // no cache
         for (Group g : groups)
@@ -1125,9 +1119,9 @@ public class GMSClient implements TransferListener
         }
         return null;
     }
-    protected List<Group> getCachedGroups(Principal userID, Role role, boolean complete)
+    protected List<Group> getCachedGroups(URI serviceID, Principal userID, Role role, boolean complete)
     {
-        GroupMemberships mems = getGroupCache(userID);
+        GroupMemberships mems = getGroupCache(serviceID, userID);
         if (mems == null)
             return null; // no cache
 
@@ -1141,16 +1135,16 @@ public class GMSClient implements TransferListener
 
     protected void addCachedGroup(Principal userID, Group group, Role role)
     {
-        GroupMemberships mems = getGroupCache(userID);
+        GroupMemberships mems = getGroupCache(group.getID().getServiceID(), userID);
         if (mems == null)
             return; // no cache
 
         mems.add(group, role);
     }
 
-    protected void setCachedGroups(Principal userID, List<Group> groups, Role role)
+    protected void setCachedGroups(URI serviceID, Principal userID, List<Group> groups, Role role)
     {
-        GroupMemberships mems = getGroupCache(userID);
+        GroupMemberships mems = getGroupCache(serviceID, userID);
         if (mems == null)
             return; // no cache
 
diff --git a/cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/GMSClientMain.java b/cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/GMSClientMain.java
index dafea221..14e8db6e 100644
--- a/cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/GMSClientMain.java
+++ b/cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/GMSClientMain.java
@@ -69,26 +69,26 @@
 
 package ca.nrc.cadc.ac.client;
 
-import ca.nrc.cadc.ac.Group;
-import ca.nrc.cadc.ac.User;
-import java.net.URI;
+import java.security.AccessControlContext;
+import java.security.AccessController;
+import java.security.Principal;
 import java.security.PrivilegedAction;
+import java.util.Iterator;
+import java.util.Set;
 
 import javax.security.auth.Subject;
+import javax.security.auth.x500.X500Principal;
 
 import org.apache.log4j.Level;
 import org.apache.log4j.Logger;
 
+import ca.nrc.cadc.ac.Group;
+import ca.nrc.cadc.ac.GroupURI;
+import ca.nrc.cadc.ac.User;
 import ca.nrc.cadc.auth.CertCmdArgUtil;
 import ca.nrc.cadc.auth.HttpPrincipal;
 import ca.nrc.cadc.util.ArgumentMap;
 import ca.nrc.cadc.util.Log4jInit;
-import java.security.AccessControlContext;
-import java.security.AccessController;
-import java.security.Principal;
-import java.util.Iterator;
-import java.util.Set;
-import javax.security.auth.x500.X500Principal;
 
 /**
  * Prototype main class for the GMSClient.  Currently
@@ -123,7 +123,7 @@ public class GMSClientMain implements PrivilegedAction<Object>
 
     private GMSClientMain()
     {
-        client = new GMSClient(URI.create("ivo://cadc.nrc.ca/canfargms"));
+        client = new GMSClient();
     }
 
     public static void main(String[] args)
@@ -168,22 +168,22 @@ public class GMSClientMain implements PrivilegedAction<Object>
     {
         if (argMap.isSet(ARG_ADD_MEMBER))
             return ARG_ADD_MEMBER;
-        
+
         if (argMap.isSet(ARG_CREATE_GROUP))
             return ARG_CREATE_GROUP;
-        
+
         if (argMap.isSet(ARG_GET_GROUP))
             return ARG_GET_GROUP;
-        
+
         if (argMap.isSet(ARG_DELETE_GROUP))
             return ARG_DELETE_GROUP;
-        
+
         if (argMap.isSet(ARG_DEL_MEMBER))
             return ARG_DEL_MEMBER;
-        
+
         if (argMap.isSet(ARG_ADD_ADMIN))
             return ARG_ADD_ADMIN;
-        
+
         if (argMap.isSet(ARG_DEL_ADMIN))
             return ARG_DEL_ADMIN;
 
@@ -210,6 +210,8 @@ public class GMSClientMain implements PrivilegedAction<Object>
         {
             String command = getCommand();
 
+            GroupURI groupID = null;
+
             if (command.equals(ARG_ADD_MEMBER))
             {
                 String group = argMap.getValue(ARG_GROUP);
@@ -221,7 +223,7 @@ public class GMSClientMain implements PrivilegedAction<Object>
                 if (userID == null)
                     throw new IllegalArgumentException("No userid specified");
 
-                client.addUserMember(group, new HttpPrincipal(userID));
+                client.addUserMember(new GroupURI(group), new HttpPrincipal(userID));
             }
             else if (command.equals(ARG_DEL_MEMBER))
             {
@@ -233,7 +235,7 @@ public class GMSClientMain implements PrivilegedAction<Object>
                 if (member == null)
                     throw new IllegalArgumentException("No user specified");
 
-                client.removeUserMember(group, new HttpPrincipal(member));
+                client.removeUserMember(new GroupURI(group), new HttpPrincipal(member));
             }
             else if (command.equals(ARG_ADD_ADMIN))
             {
@@ -246,8 +248,8 @@ public class GMSClientMain implements PrivilegedAction<Object>
                 if (userID == null)
                     throw new IllegalArgumentException("No userid specified");
                 HttpPrincipal hp = new HttpPrincipal(userID);
-                
-                Group cur = client.getGroup(group);
+
+                Group cur = client.getGroup(new GroupURI(group));
                 boolean update = true;
                 for (User admin : cur.getUserAdmins())
                 {
@@ -286,8 +288,8 @@ public class GMSClientMain implements PrivilegedAction<Object>
                 if (userID == null)
                     throw new IllegalArgumentException("No user specified");
                 HttpPrincipal hp = new HttpPrincipal(userID);
-                
-                Group cur = client.getGroup(group);
+
+                Group cur = client.getGroup(new GroupURI(group));
                 boolean update = false;
                 Iterator<User> iter = cur.getUserAdmins().iterator();
                 while (iter.hasNext())
@@ -308,7 +310,7 @@ public class GMSClientMain implements PrivilegedAction<Object>
                     }
                 }
                 if (update)
-                {   
+                {
                     client.updateGroup(cur);
                     log.info("admin removed: " + userID);
                 }
@@ -321,12 +323,25 @@ public class GMSClientMain implements PrivilegedAction<Object>
                 if (group == null)
                     throw new IllegalArgumentException("No group specified");
 
+                GroupURI groupURI = null;
+                try
+                {
+                    groupURI = new GroupURI(group);
+                }
+                catch (Exception e)
+                {
+                    String message = "Invalid group URI format '" +
+                        group + "': " + e.getMessage();
+                    log.debug(message, e);
+                    throw new IllegalArgumentException(message);
+                }
+
                 AccessControlContext accessControlContext = AccessController.getContext();
                 Subject subject = Subject.getSubject(accessControlContext);
                 Set<X500Principal> principals = subject.getPrincipals(X500Principal.class);
                 X500Principal p = principals.iterator().next();
 
-                Group g = new Group(group);
+                Group g = new Group(groupURI);
 
                 User member = new User();
                 member.getIdentities().add(p);
@@ -339,7 +354,7 @@ public class GMSClientMain implements PrivilegedAction<Object>
                 if (group == null)
                     throw new IllegalArgumentException("No group specified");
 
-                Group g = client.getGroup(group);
+                Group g = client.getGroup(new GroupURI(group));
                 System.out.println("found: " + g.getID());
                 System.out.println("\t" + g.description);
                 System.out.println("owner: " + g.getOwner());
@@ -363,7 +378,7 @@ public class GMSClientMain implements PrivilegedAction<Object>
                 if (group == null)
                     throw new IllegalArgumentException("No group specified");
 
-                client.deleteGroup(group);
+                client.deleteGroup(new GroupURI(group));
             }
 
             return null;
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 6d8f0058..8a0921c5 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
@@ -91,6 +91,7 @@ import org.jdom2.output.XMLOutputter;
 
 import ca.nrc.cadc.ac.Group;
 import ca.nrc.cadc.ac.GroupProperty;
+import ca.nrc.cadc.ac.GroupURI;
 import ca.nrc.cadc.ac.InternalID;
 import ca.nrc.cadc.ac.PersonalDetails;
 import ca.nrc.cadc.ac.PosixDetails;
@@ -104,8 +105,6 @@ import ca.nrc.cadc.auth.IdentityType;
 import ca.nrc.cadc.auth.NumericPrincipal;
 import ca.nrc.cadc.auth.OpenIdPrincipal;
 import ca.nrc.cadc.date.DateUtil;
-import ca.nrc.cadc.reg.Standards;
-import ca.nrc.cadc.reg.client.LocalAuthority;
 
 /**
  * AbstractReaderWriter TODO describe class
@@ -152,13 +151,8 @@ public abstract class AbstractReaderWriter
     public static final String USER_MEMBERS = "userMembers";
     public static final String USER_REQUEST = "userRequest";
 
-    private String gmsServiceURI;
-
     public AbstractReaderWriter()
     {
-        LocalAuthority localAuthority = new LocalAuthority();
-        URI serviceURI = localAuthority.getServiceURI(Standards.GMS_GROUPS_01.toString());
-        gmsServiceURI = serviceURI.toString();
     }
 
     /**
@@ -480,15 +474,6 @@ public abstract class AbstractReaderWriter
             throw new ReaderException(error);
         }
 
-        // Group groupID
-        int index = uri.indexOf(gmsServiceURI);
-        if (index == -1)
-        {
-            String error = "group uri attribute malformed: " + uri;
-            throw new ReaderException(error);
-        }
-        String groupID = uri.substring(gmsServiceURI.length() + 1);
-
         // Group owner
         User user = null;
         Element ownerElement = element.getChild(OWNER);
@@ -504,7 +489,20 @@ public abstract class AbstractReaderWriter
             user = getUser(userElement);
         }
 
-        Group group = new Group(groupID);
+        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());
+        }
+        Group group = new Group(groupURI);
 
         // set owner field
         setField(group, user, OWNER);
@@ -934,7 +932,7 @@ public abstract class AbstractReaderWriter
 
         // Create the root group element.
         Element groupElement = new Element(GROUP);
-        String groupURI = gmsServiceURI + "#" + group.getID();
+        String groupURI = group.getID().toString();
         groupElement.setAttribute(new Attribute(URI, groupURI));
 
         // Group owner
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 3d92e767..52b72d04 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
@@ -72,6 +72,8 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import java.net.URISyntaxException;
+
 import org.apache.log4j.Logger;
 import org.junit.Test;
 
@@ -84,12 +86,12 @@ public class GroupTest
     @Test
     public void simpleGroupTest() throws Exception
     {
-        Group group1 = new Group("TestGroup");
+        Group group1 = new Group(new GroupURI("ivo://example.org/gms?TestGroup"));
         Group group2 = group1;
         assertEquals(group1.hashCode(), group2.hashCode());
         assertEquals(group1, group2);
 
-        Group group3 = new Group("TestGroup");
+        Group group3 = new Group(new GroupURI("ivo://example.org/gms?TestGroup"));
         User user = new User();
         user.getIdentities().add(new HttpPrincipal("foo"));
 
@@ -100,7 +102,7 @@ public class GroupTest
         assertEquals(group3.hashCode(), group4.hashCode());
         assertEquals(group3, group4);
 
-        group4 = new Group("TestGroup");
+        group4 = new Group(new GroupURI("ivo://example.org/gms?TestGroup"));
         assertEquals(group3.hashCode(), group4.hashCode());
         assertEquals(group3,group4);
 
@@ -124,7 +126,7 @@ public class GroupTest
         assertEquals(group3.hashCode(), group4.hashCode());
         assertEquals(group3,group4);
 
-        group4 = new Group("NewTestGroup-._~.");
+        group4 = new Group(new GroupURI("ivo://example.org/gms?NewTestGroup-._~."));
         assertFalse(group3.hashCode() == group4.hashCode());
         assertFalse(group3.equals(group4));
 
@@ -133,7 +135,7 @@ public class GroupTest
     }
 
     @Test
-    public void exceptionTests()
+    public void exceptionTests() throws URISyntaxException
     {
         boolean thrown = false;
         try
@@ -150,7 +152,7 @@ public class GroupTest
         thrown = false;
         try
         {
-            new Group("New/Test/Group");
+            new Group(new GroupURI("ivo://example.org/New/Test/Group"));
         }
         catch(IllegalArgumentException e)
         {
@@ -161,9 +163,9 @@ public class GroupTest
         thrown = false;
         try
         {
-            new Group("New%Test%Group");
+            new Group(new GroupURI("ivo://example.org/New%Test%Group"));
         }
-        catch(IllegalArgumentException e)
+        catch(URISyntaxException e)
         {
             thrown = true;
         }
@@ -172,9 +174,9 @@ public class GroupTest
         thrown = false;
         try
         {
-            new Group("New\\Test\\Group");
+            new Group(new GroupURI("ivo://example.org/New\\Test\\Group"));
         }
-        catch(IllegalArgumentException e)
+        catch(URISyntaxException 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
new file mode 100644
index 00000000..e1d7e4c7
--- /dev/null
+++ b/cadc-access-control/src/test/java/ca/nrc/cadc/ac/GroupURITest.java
@@ -0,0 +1,77 @@
+package ca.nrc.cadc.ac;
+
+import java.net.URISyntaxException;
+
+import org.apache.log4j.Level;
+import org.apache.log4j.Logger;
+import org.junit.Assert;
+import org.junit.Test;
+
+import ca.nrc.cadc.util.Log4jInit;
+
+public class GroupURITest
+{
+    private static Logger log = Logger.getLogger(GroupURITest.class);
+
+    static
+    {
+        Log4jInit.setLevel("ca.nrc.cadc.ac", Level.INFO);
+    }
+
+    @Test
+    public void testMalformed()
+    {
+        try
+        {
+            // wrong scheme
+            assertIllegalArgument("iko://cadc.nrc.ca/gms?gname", "scheme");
+
+            // fragment instead of query
+            assertIllegalArgument("ivo://cadc.nrc.ca/gms#gname", "fragment");
+
+            // no authority
+            assertIllegalArgument("ivo://gms?gname", "authority");
+
+            // extended path in group
+            assertIllegalArgument("ivo://cadc.nrc.ca/gms/path?gname", "path");
+        }
+        catch (Throwable t)
+        {
+            log.error("Test Failed", t);
+            Assert.fail();
+        }
+    }
+
+    @Test
+    public void testCorrect()
+    {
+        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());
+        }
+        catch (Throwable t)
+        {
+            log.error("Test Failed", t);
+        }
+    }
+
+    private void assertIllegalArgument(String uri, String message) throws URISyntaxException
+    {
+        try
+        {
+            new GroupURI(uri);
+            Assert.fail("Expected Illegal argument for URI " + uri);
+        }
+        catch (IllegalArgumentException e)
+        {
+            // expected
+            log.debug("Checking if message '" + e.getMessage() + "' contains '" + message + "'");
+            Assert.assertTrue(e.getMessage().toLowerCase().contains(message));
+        }
+    }
+}
diff --git a/cadc-access-control/src/test/java/ca/nrc/cadc/ac/client/GMSClientTest.java b/cadc-access-control/src/test/java/ca/nrc/cadc/ac/client/GMSClientTest.java
index e8e9b659..cfdc3a18 100644
--- a/cadc-access-control/src/test/java/ca/nrc/cadc/ac/client/GMSClientTest.java
+++ b/cadc-access-control/src/test/java/ca/nrc/cadc/ac/client/GMSClientTest.java
@@ -81,15 +81,16 @@ import java.util.List;
 
 import javax.security.auth.Subject;
 
-import ca.nrc.cadc.auth.AuthMethod;
-import ca.nrc.cadc.reg.Standards;
 import org.apache.log4j.Level;
 import org.junit.Assert;
 import org.junit.Test;
 
 import ca.nrc.cadc.ac.Group;
+import ca.nrc.cadc.ac.GroupURI;
 import ca.nrc.cadc.ac.Role;
+import ca.nrc.cadc.auth.AuthMethod;
 import ca.nrc.cadc.auth.HttpPrincipal;
+import ca.nrc.cadc.reg.Standards;
 import ca.nrc.cadc.reg.client.RegistryClient;
 import ca.nrc.cadc.util.Log4jInit;
 
@@ -114,13 +115,11 @@ public class GMSClientTest
         final RegistryClient mockRegistryClient =
                 createMock(RegistryClient.class);
 
-        final URI serviceID = URI.create("ivo://mysite.com/users");
-
-        expect(mockRegistryClient.getServiceURL(serviceID, Standards.UMS_USERS_01, AuthMethod.CERT))
-            .andReturn(new URL("http://mysite.com/users"));
+//        expect(mockRegistryClient.getServiceURL(serviceID, Standards.UMS_USERS_01, AuthMethod.CERT))
+//            .andReturn(new URL("http://mysite.com/users"));
 
         replay(mockRegistryClient);
-        GMSClient client = new GMSClient(serviceID)
+        GMSClient client = new GMSClient()
         {
             @Override
             protected RegistryClient getRegistryClient()
@@ -150,15 +149,15 @@ public class GMSClientTest
         final HttpPrincipal test1UserID = new HttpPrincipal("test");
         subject.getPrincipals().add(test1UserID);
 
-        final URI serviceID = URI.create("ivo://mysite.com/users");
+        final URI serviceID = URI.create("ivo://example.org/gms");
         final RegistryClient mockRegistryClient =
                 createMock(RegistryClient.class);
 
         expect(mockRegistryClient.getServiceURL(serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT ))
-            .andReturn(new URL("http://mysite.com/users"));
+            .andReturn(new URL("http://example.org/gms"));
 
         replay(mockRegistryClient);
-        final GMSClient client = new GMSClient(serviceID)
+        final GMSClient client = new GMSClient()
         {
             @Override
             protected RegistryClient getRegistryClient()
@@ -174,43 +173,42 @@ public class GMSClientTest
             {
 
                 List<Group> initial = client
-                        .getCachedGroups(test1UserID, Role.MEMBER, true);
+                        .getCachedGroups(serviceID, test1UserID, Role.MEMBER, true);
                 Assert.assertNull("Cache should be null", initial);
 
                 // add single group as isMember might do
-                Group group0 = new Group("0");
+                GroupURI group0uri = new GroupURI("ivo://example.org/gms?0");
+                Group group0 = new Group(group0uri);
                 client.addCachedGroup(test1UserID, group0, Role.MEMBER);
-                List<Group> actual = client
-                        .getCachedGroups(test1UserID, Role.MEMBER, true);
+                List<Group> actual = client.getCachedGroups(serviceID, test1UserID, Role.MEMBER, true);
                 Assert.assertNull("Cache should be null", actual);
 
-                Group g = client
-                        .getCachedGroup(test1UserID, "0", Role.MEMBER);
+                Group g = client.getCachedGroup(test1UserID, group0uri, Role.MEMBER);
                 Assert.assertNotNull("cached group from incomplete cache", g);
 
                 // add all groups like getMemberships might do
                 List<Group> expected = new ArrayList<Group>();
-                Group group1 = new Group("1");
-                Group group2 = new Group("2");
+                Group group1 = new Group(new GroupURI("ivo://example.org/gms?1"));
+                Group group2 = new Group(new GroupURI("ivo://example.org/gms?2"));
                 expected.add(group0);
                 expected.add(group1);
                 expected.add(group2);
 
-                client.setCachedGroups(test1UserID, expected, Role.MEMBER);
+                client.setCachedGroups(serviceID, test1UserID, expected, Role.MEMBER);
 
                 actual = client
-                        .getCachedGroups(test1UserID, Role.MEMBER, true);
+                        .getCachedGroups(serviceID, test1UserID, Role.MEMBER, true);
                 Assert.assertEquals("Wrong cached groups", expected, actual);
 
                 // check against another role
                 actual = client
-                        .getCachedGroups(test1UserID, Role.OWNER, true);
+                        .getCachedGroups(serviceID, test1UserID, Role.OWNER, true);
                 Assert.assertNull("Cache should be null", actual);
 
                 // check against another userid
                 final HttpPrincipal anotherUserID = new HttpPrincipal("anotheruser");
                 actual = client
-                        .getCachedGroups(anotherUserID, Role.MEMBER, true);
+                        .getCachedGroups(serviceID, anotherUserID, Role.MEMBER, true);
                 Assert.assertNull("Cache should be null", actual);
 
                 return null;
@@ -230,30 +228,30 @@ public class GMSClientTest
             {
 
                 List<Group> initial = client
-                        .getCachedGroups(test2UserID, Role.MEMBER, true);
+                        .getCachedGroups(serviceID, test2UserID, Role.MEMBER, true);
                 Assert.assertNull("Cache should be null", initial);
 
                 List<Group> expected = new ArrayList<Group>();
-                Group group1 = new Group("1");
-                Group group2 = new Group("2");
+                Group group1 = new Group(new GroupURI("ivo://example.org/gms?1"));
+                Group group2 = new Group(new GroupURI("ivo://example.org/gms?2"));
                 expected.add(group1);
                 expected.add(group2);
 
-                client.setCachedGroups(test2UserID, expected, Role.MEMBER);
+                client.setCachedGroups(serviceID, test2UserID, expected, Role.MEMBER);
 
                 List<Group> actual = client
-                        .getCachedGroups(test2UserID, Role.MEMBER, true);
+                        .getCachedGroups(serviceID, test2UserID, Role.MEMBER, true);
                 Assert.assertEquals("Wrong cached groups", expected, actual);
 
                 // check against another role
                 actual = client
-                        .getCachedGroups(test2UserID, Role.OWNER, true);
+                        .getCachedGroups(serviceID, test2UserID, Role.OWNER, true);
                 Assert.assertNull("Cache should be null", actual);
 
                 // check against another userid
                 final HttpPrincipal anotherUserID = new HttpPrincipal("anotheruser");
                 actual = client
-                        .getCachedGroups(anotherUserID, Role.MEMBER, true);
+                        .getCachedGroups(serviceID, anotherUserID, Role.MEMBER, true);
                 Assert.assertNull("Cache should be null", actual);
 
                 return null;
@@ -263,19 +261,19 @@ public class GMSClientTest
         // do the same without a subject
 
         List<Group> initial = client
-                .getCachedGroups(test1UserID, Role.MEMBER, true);
+                .getCachedGroups(serviceID, test1UserID, Role.MEMBER, true);
         Assert.assertNull("Cache should be null", initial);
 
         List<Group> newgroups = new ArrayList<Group>();
-        Group group1 = new Group("1");
-        Group group2 = new Group("2");
+        Group group1 = new Group(new GroupURI("ivo://example.org/gms?1"));
+        Group group2 = new Group(new GroupURI("ivo://example.org/gms?2"));
         newgroups.add(group1);
         newgroups.add(group2);
 
-        client.setCachedGroups(test1UserID, newgroups, Role.MEMBER);
+        client.setCachedGroups(serviceID, test1UserID, newgroups, Role.MEMBER);
 
         List<Group> actual = client
-                .getCachedGroups(test1UserID, Role.MEMBER, true);
+                .getCachedGroups(serviceID, test1UserID, Role.MEMBER, true);
         Assert.assertNull("Cache should still be null", actual);
     }
 
diff --git a/cadc-access-control/src/test/java/ca/nrc/cadc/ac/json/JsonGroupReaderWriterTest.java b/cadc-access-control/src/test/java/ca/nrc/cadc/ac/json/JsonGroupReaderWriterTest.java
index 50b73124..eab7b8d5 100644
--- a/cadc-access-control/src/test/java/ca/nrc/cadc/ac/json/JsonGroupReaderWriterTest.java
+++ b/cadc-access-control/src/test/java/ca/nrc/cadc/ac/json/JsonGroupReaderWriterTest.java
@@ -95,6 +95,7 @@ import org.junit.Test;
 
 import ca.nrc.cadc.ac.Group;
 import ca.nrc.cadc.ac.GroupProperty;
+import ca.nrc.cadc.ac.GroupURI;
 import ca.nrc.cadc.ac.InternalID;
 import ca.nrc.cadc.ac.PersonalDetails;
 import ca.nrc.cadc.ac.PosixDetails;
@@ -186,7 +187,7 @@ public class JsonGroupReaderWriterTest
     public void testMinimalReadWrite()
             throws Exception
     {
-        Group expected = new Group("groupID");
+        Group expected = new Group(new GroupURI("ivo://example.org/gms?groupID"));
 
         StringBuilder json = new StringBuilder();
         JsonGroupWriter writer = new JsonGroupWriter();
@@ -220,7 +221,7 @@ public class JsonGroupReaderWriterTest
         PosixDetails posixDetails = new PosixDetails("foo", 123L, 456L, "/dev/null");
         owner.posixDetails = posixDetails;
 
-        Group expected = new Group("groupID");
+        Group expected = new Group(new GroupURI("ivo://example.org/gms?groupID"));
 
 
         expected.description = "description";
@@ -229,12 +230,12 @@ public class JsonGroupReaderWriterTest
         expected.getProperties().add(new GroupProperty("key2", "value2", true));
         expected.getProperties().add(new GroupProperty("key3", "value3", true));
 
-        Group groupMember = new Group("member");
+        Group groupMember = new Group(new GroupURI("ivo://example.org/gms?member"));
         User userMember = new User();
         userMember.getIdentities().add(new HttpPrincipal("foo"));
         URI memberUri = new URI("ivo://cadc.nrc.ca/user?" + UUID.randomUUID());
         TestUtil.setField(userMember, new InternalID(memberUri), AbstractReaderWriter.ID);
-        Group groupAdmin = new Group("admin");
+        Group groupAdmin = new Group(new GroupURI("ivo://example.org/gms?admin"));
         User userAdmin = new User();
         userAdmin.getIdentities().add(new HttpPrincipal("bar"));
         URI adminUri = new URI("ivo://cadc.nrc.ca/user?" + UUID.randomUUID());
diff --git a/cadc-access-control/src/test/java/ca/nrc/cadc/ac/xml/GroupListReaderWriterTest.java b/cadc-access-control/src/test/java/ca/nrc/cadc/ac/xml/GroupListReaderWriterTest.java
index cefbb2a2..8020102e 100644
--- a/cadc-access-control/src/test/java/ca/nrc/cadc/ac/xml/GroupListReaderWriterTest.java
+++ b/cadc-access-control/src/test/java/ca/nrc/cadc/ac/xml/GroupListReaderWriterTest.java
@@ -85,6 +85,7 @@ import org.junit.BeforeClass;
 import org.junit.Test;
 
 import ca.nrc.cadc.ac.Group;
+import ca.nrc.cadc.ac.GroupURI;
 import ca.nrc.cadc.ac.WriterException;
 import ca.nrc.cadc.util.PropertiesReader;
 
@@ -158,8 +159,8 @@ public class GroupListReaderWriterTest
         throws Exception
     {
         List<Group> expected = new ArrayList<Group>();
-        expected.add(new Group("group1"));
-        expected.add(new Group("group2"));
+        expected.add(new Group(new GroupURI("ivo://example.org/gms?group1")));
+        expected.add(new Group(new GroupURI("ivo://example.org/gms?group2")));
 
         StringBuilder xml = new StringBuilder();
         GroupListWriter groupListWriter = new GroupListWriter();
diff --git a/cadc-access-control/src/test/java/ca/nrc/cadc/ac/xml/GroupReaderWriterTest.java b/cadc-access-control/src/test/java/ca/nrc/cadc/ac/xml/GroupReaderWriterTest.java
index 17343df0..434d2d17 100644
--- a/cadc-access-control/src/test/java/ca/nrc/cadc/ac/xml/GroupReaderWriterTest.java
+++ b/cadc-access-control/src/test/java/ca/nrc/cadc/ac/xml/GroupReaderWriterTest.java
@@ -91,6 +91,7 @@ import org.junit.Test;
 
 import ca.nrc.cadc.ac.Group;
 import ca.nrc.cadc.ac.GroupProperty;
+import ca.nrc.cadc.ac.GroupURI;
 import ca.nrc.cadc.ac.InternalID;
 import ca.nrc.cadc.ac.PersonalDetails;
 import ca.nrc.cadc.ac.PosixDetails;
@@ -169,7 +170,7 @@ public class GroupReaderWriterTest
     public void testMinimalReadWrite()
         throws Exception
     {
-        Group expected = new Group("groupID");
+        Group expected = new Group(new GroupURI("ivo://example.org/gms?groupID"));
 
         StringBuilder xml = new StringBuilder();
         GroupWriter groupWriter = new GroupWriter();
@@ -197,20 +198,20 @@ public class GroupReaderWriterTest
         owner.personalDetails.country = "country";
         owner.posixDetails = new PosixDetails("foo", 123L, 456L, "/dev/null");
 
-        Group expected = new Group("groupID");
+        Group expected = new Group(new GroupURI("ivo://example.org/gms?groupID"));
         setGroupOwner(expected, owner);
         expected.description = "description";
         expected.lastModified = new Date();
         expected.getProperties().add(new GroupProperty("key1", "value1", true));
         expected.getProperties().add(new GroupProperty("key2", "value2", false));
 
-        Group groupMember = new Group("member");
+        Group groupMember = new Group(new GroupURI("ivo://example.org/gms?member"));
         User userMember = new User();
         userMember.getIdentities().add(new HttpPrincipal("foo"));
         URI memberUri = new URI("ivo://cadc.nrc.ca/user?" + UUID.randomUUID());
         TestUtil.setField(userMember, new InternalID(memberUri), AbstractReaderWriter.ID);
 
-        Group groupAdmin = new Group("admin");
+        Group groupAdmin = new Group(new GroupURI("ivo://example.org/gms?admin"));
         User userAdmin = new User();
         userAdmin.getIdentities().add(new HttpPrincipal("bar"));
         URI adminUri = new URI("ivo://cadc.nrc.ca/user?" + UUID.randomUUID());
-- 
GitLab