From a35a40417cde17263ae6ce0bf0e24af03bab7268 Mon Sep 17 00:00:00 2001
From: Jeff Burke <Jeff.Burke@nrc-cnrc.gc.ca>
Date: Tue, 19 Aug 2014 11:19:48 -0700
Subject: [PATCH] Files recovered from jars

---
 .../cadcAccessControl-Server/Dependencies.txt |  15 +
 .../{config => }/PluginFactory.properties     |   1 -
 projects/cadcAccessControl-Server/build.xml   |  61 +--
 .../nrc/cadc/ac/server/GroupPersistence.java  | 102 ++++-
 .../ca/nrc/cadc/ac/server/PluginFactory.java  |  18 +-
 .../nrc/cadc/ac/server/UserPersistence.java   |  12 +-
 .../nrc/cadc/ac/server/ldap/LdapConfig.java   |  70 ++--
 .../ca/nrc/cadc/ac/server/ldap/LdapDAO.java   |  49 +--
 .../nrc/cadc/ac/server/ldap/LdapGroupDAO.java | 368 +++++++++++++-----
 .../ac/server/ldap/LdapGroupPersistence.java  |  68 +++-
 .../nrc/cadc/ac/server/ldap/LdapUserDAO.java  |  65 +++-
 .../cadc/ac/server/web/ACSearchRunner.java    |   5 +
 .../nrc/cadc/ac/server/web/GroupsServlet.java |  12 +
 .../nrc/cadc/ac/server/ldap/LdapDAOTest.java  | 172 ++++++++
 .../cadc/ac/server/ldap/LdapDAOTestImpl.java  |  90 +++++
 .../cadc/ac/server/ldap/LdapGroupDAOTest.java | 239 ++++++++++++
 .../ac/server/web/GroupActionFactoryTest.java | 361 +++++++++++++++++
 .../src/ca/nrc/cadc/ac/AC.java                |  14 +
 .../cadc/ac/GroupAlreadyExistsException.java  |   5 +-
 .../nrc/cadc/ac/GroupNotFoundException.java   |   5 +-
 .../src/ca/nrc/cadc/ac/GroupReader.java       |  44 ++-
 .../src/ca/nrc/cadc/ac/GroupWriter.java       |  50 ++-
 .../src/ca/nrc/cadc/ac/GroupsReader.java      |  34 +-
 .../src/ca/nrc/cadc/ac/GroupsWriter.java      | 117 ++++--
 .../cadc/ac/MemberAlreadyExistsException.java |   4 +
 .../nrc/cadc/ac/MemberNotFoundException.java  |   4 +
 .../src/ca/nrc/cadc/ac/PersonalDetails.java   |   2 +-
 .../src/ca/nrc/cadc/ac/PosixDetails.java      |   9 +-
 .../src/ca/nrc/cadc/ac/ReaderException.java   |  25 ++
 .../ca/nrc/cadc/ac/Role.java}                 |  52 ++-
 .../src/ca/nrc/cadc/ac/User.java              |   2 +-
 .../ca/nrc/cadc/ac/UserNotFoundException.java |   4 +
 .../src/ca/nrc/cadc/ac/UserReader.java        |  32 ++
 .../src/ca/nrc/cadc/ac/UserWriter.java        |  42 ++
 .../src/ca/nrc/cadc/ac/WriterException.java   |  25 ++
 .../src/ca/nrc/cadc/ac/client/GMSClient.java  | 118 +++++-
 36 files changed, 1963 insertions(+), 333 deletions(-)
 create mode 100644 projects/cadcAccessControl-Server/Dependencies.txt
 rename projects/cadcAccessControl-Server/{config => }/PluginFactory.properties (99%)
 create mode 100644 projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapDAOTest.java
 create mode 100644 projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapDAOTestImpl.java
 create mode 100644 projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java
 create mode 100644 projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/GroupActionFactoryTest.java
 rename projects/cadcAccessControl/{test/src/ca/nrc/cadc/ac/ReaderWriterTest.java => src/ca/nrc/cadc/ac/Role.java} (87%)

diff --git a/projects/cadcAccessControl-Server/Dependencies.txt b/projects/cadcAccessControl-Server/Dependencies.txt
new file mode 100644
index 00000000..7ef8710a
--- /dev/null
+++ b/projects/cadcAccessControl-Server/Dependencies.txt
@@ -0,0 +1,15 @@
+JAR files required for the OpenCADC cadcAccessControl-Server project
+====================================================================
+
+Name in build.xml            Versioned Name          Project URL
+-----------------            --------------          -----------
+jdom.jar                     jdom-1.1                http://www.jdom.org
+log4j.jar                    log4j-1.2.15            http://logging.apache.org
+xerces.jar                   xerces-2_9_1            http://xerces.apache.org
+servlet-api.jar              apache-tomcat-5.5.20    http://tomcat.apache.org
+jdom2jar                     jdom-2.0.5              http://www.jdom.org
+cadcRegistryClient.jar                               http://code.google.com/p/opencadc
+cadcUtil.jar                                         http://code.google.com/p/opencadc
+cadcAccessControl.jar                                http://code.google.com/p/opencadc
+cadcUWS.jar                                          http://code.google.com/p/opencadc
+cadcLog.jar                                          http://code.google.com/p/opencadc
\ No newline at end of file
diff --git a/projects/cadcAccessControl-Server/config/PluginFactory.properties b/projects/cadcAccessControl-Server/PluginFactory.properties
similarity index 99%
rename from projects/cadcAccessControl-Server/config/PluginFactory.properties
rename to projects/cadcAccessControl-Server/PluginFactory.properties
index 43f275b1..3c47cbb4 100644
--- a/projects/cadcAccessControl-Server/config/PluginFactory.properties
+++ b/projects/cadcAccessControl-Server/PluginFactory.properties
@@ -1,4 +1,3 @@
-
 ## commented out values are the defaults, shown as examples
 ## to customise behaviour, subclass the specified class and
 ## change the configuration here
diff --git a/projects/cadcAccessControl-Server/build.xml b/projects/cadcAccessControl-Server/build.xml
index f89bb355..9f319e52 100644
--- a/projects/cadcAccessControl-Server/build.xml
+++ b/projects/cadcAccessControl-Server/build.xml
@@ -88,38 +88,49 @@
 
     <property name="project" value="cadcAccessControl-Server" />
 
-    <property name="accessControl"  value="${lib}/cadcAccessControl.jar" />
-    <property name="cadcLog"        value="${lib}/cadcLog.jar" />
-    <property name="cadcUtil"       value="${lib}/cadcUtil.jar" />
-    <property name="cadcUWS"        value="${lib}/cadcUWS.jar" />
+    <property name="cadcAccessControl"   value="${lib}/cadcAccessControl.jar" />
+    <property name="cadcLog"             value="${lib}/cadcLog.jar" />
+    <property name="cadcRegistry"        value="${lib}/cadcRegistryClient.jar" />
+    <property name="cadcUtil"            value="${lib}/cadcUtil.jar" />
+    <property name="cadcUWS"             value="${lib}/cadcUWS.jar" />
     
-    <property name="jdom2"          value="${ext.lib}/jdom2.jar" />
-    <property name="log4j"          value="${ext.lib}/log4j.jar" />
-    <property name="servlet"        value="${ext.lib}/servlet-api.jar" />
-    <property name="unboundid"      value="${ext.lib}/unboundid-ldapsdk-se.jar" />
+    <property name="jdom2"               value="${ext.lib}/jdom2.jar" />
+    <property name="log4j"               value="${ext.lib}/log4j.jar" />
+    <property name="servlet"             value="${ext.lib}/servlet-api.jar" />
+    <property name="unboundid"           value="${ext.lib}/unboundid-ldapsdk-se.jar" />
+    <property name="xerces"              value="${ext.lib}/xerces.jar" />
 
-
-    <property name="jars" value="${accessControl}:${cadcLog}:${cadcUtil}:${cadcUWS}:${jdom2}:${log4j}:${servlet}:${unboundid}" />
+    <property name="jars" value="${cadcAccessControl}:${cadcLog}:${cadcRegistry}:${cadcUtil}:${cadcUWS}:${jdom2}:${log4j}:${servlet}:${unboundid}:${xerces}" />
 
     <target name="build" depends="compile">
         <jar jarfile="${build}/lib/${project}.jar"
-                    basedir="${build}/class"
-                    update="no">
-                <include name="ca/nrc/cadc/**" />
+             basedir="${build}/class"
+             update="no">
+            <include name="ca/nrc/cadc/**" />
         </jar>
     </target>
 
     <!-- JAR files needed to run the test suite -->
-    <property name="xerces"     value="${ext.dev}/xerces.jar" />
-    <property name="asm"        value="${ext.dev}/asm.jar" />
-    <property name="cglib"      value="${ext.dev}/cglib.jar" />
-    <property name="easymock"   value="${ext.dev}/easymock.jar" />
-    <property name="junit"      value="${ext.dev}/junit.jar" />
-    <property name="objenesis"  value="${ext.dev}/objenesis.jar" />
-    
-    <property name="testingJars" value="${build}/class:${jars}:${xerces}:${asm}:${cglib}:${easymock}:${junit}:${objenesis}" />
+    <property name="gson"           value="${ext.lib}/gson.jar" />
+    <property name="easyMock"       value="${ext.dev}/easymock.jar" />
+    <property name="junit"          value="${ext.dev}/junit.jar" />
+    <property name="xmlunit"        value="${ext.dev}/xmlunit.jar" />
+    <property name="xerces"         value="${ext.lib}/xerces.jar" />
+    <property name="cglib"          value="${ext.dev}/cglib.jar" />
+    <property name="objenesis"      value="${ext.dev}/objenesis.jar" />
+    <property name="asm"            value="${ext.dev}/asm.jar" />
+
+    <property name="testingJars"    value="${jars}:${gson}:${easyMock}:${junit}:${xmlunit}:${xerces}:${cglib}:${asm}:${objenesis}" />
 
-    <target name="test" depends="compile-test">
+    <target name="resources">
+        <copy todir="${build}/class">
+            <fileset dir="config">
+                <include name="**.properties" />
+            </fileset>
+        </copy>
+    </target>
+    
+    <target name="test" depends="compile-test,resources">
         <echo message="Running test" />
 
         <!-- Run the junit test suite -->
@@ -130,8 +141,10 @@
                 <pathelement path="${build}/test/class"/>
                 <pathelement path="${testingJars}"/>
             </classpath>
-            <test name="ca.nrc.cadc.ac.UserTest" />
-            <test name="ca.nrc.cadc.ac.GroupTest" />
+            <!--<test name="ca.nrc.cadc.ac.server.ldap.LdapDAOTest" />-->
+            <!--<test name="ca.nrc.cadc.ac.server.ldap.LdapDAOTestImpl" />-->
+            <test name="ca.nrc.cadc.ac.server.ldap.LdapGroupDAOTest" />
+            <!--<test name="ca.nrc.cadc.ac.server.web.GroupActionFactoryTest" />-->
             <formatter type="plain" usefile="false" />
         </junit>
     </target>
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/GroupPersistence.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/GroupPersistence.java
index bd2bd76b..f801d777 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/GroupPersistence.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/GroupPersistence.java
@@ -71,32 +71,108 @@ package ca.nrc.cadc.ac.server;
 import ca.nrc.cadc.ac.Group;
 import ca.nrc.cadc.ac.GroupAlreadyExistsException;
 import ca.nrc.cadc.ac.GroupNotFoundException;
+import ca.nrc.cadc.ac.Role;
 import ca.nrc.cadc.ac.User;
 import ca.nrc.cadc.ac.UserNotFoundException;
 import ca.nrc.cadc.net.TransientException;
 import java.security.AccessControlException;
 import java.security.Principal;
 import java.util.Collection;
-import java.util.Map;
 
 public abstract interface GroupPersistence<T extends Principal>
 {
-    public abstract Group getGroup(String paramString)
-        throws GroupNotFoundException, TransientException, AccessControlException;
+    /**
+     * Get the group with the given Group ID.
+     *
+     * @param groupID The Group ID.
+     * 
+     * @return A Group instance
+     *
+     * @throws GroupNotFoundException If the group was not found.
+     * @throws TransientException If an temporary, unexpected problem occurred.
+     * @throws AccessControlException If the operation is not permitted.
+     */
+    public abstract Group getGroup(String groupID)
+        throws GroupNotFoundException, TransientException,
+               AccessControlException;
 
-    public abstract Group addGroup(Group paramGroup)
-        throws GroupAlreadyExistsException, TransientException, AccessControlException, UserNotFoundException;
+    /**
+     * Creates the group.
+     *
+     * @param group The group to create
+     * 
+     * @return created group
+     *
+     * @throws GroupAlreadyExistsException If a group with the same ID already
+     *                                     exists.
+     * @throws TransientException If an temporary, unexpected problem occurred.
+     * @throws AccessControlException If the operation is not permitted.
+     * @throws UserNotFoundException If owner or a member not valid user.
+     */
+    public abstract Group addGroup(Group group)
+        throws GroupAlreadyExistsException, TransientException,
+               AccessControlException, UserNotFoundException;
 
-    public abstract void deleteGroup(String paramString)
-        throws GroupNotFoundException, TransientException, AccessControlException;
+    /**
+     * Deletes the group.
+     *
+     * @param groupID The Group ID.
+     *
+     * @throws GroupNotFoundException If the group was not found.
+     * @throws TransientException If an temporary, unexpected problem occurred.
+     * @throws AccessControlException If the operation is not permitted.
+     */
+    public abstract void deleteGroup(String groupID)
+        throws GroupNotFoundException, TransientException,
+               AccessControlException;
 
-    public abstract Group modifyGroup(Group paramGroup)
-        throws GroupNotFoundException, TransientException, AccessControlException, UserNotFoundException;
+    /**
+     * Modify the given group.
+     *
+     * @param group The group to update.
+     * 
+     * @return The newly updated group.
+     * 
+     * @throws GroupNotFoundException If the group was not found.
+     * @throws TransientException If an temporary, unexpected problem occurred.
+     * @throws AccessControlException If the operation is not permitted.
+     * @throws UserNotFoundException If owner or group members not valid users.
+     */
+    public abstract Group modifyGroup(Group group)
+        throws GroupNotFoundException, TransientException,
+               AccessControlException, UserNotFoundException;
 
-    public abstract Collection<Group> getGroups(Map<String, String> paramMap)
-        throws TransientException, AccessControlException;
+    /**
+     * Obtain a Collection of Groups that fit the given query.
+     *
+     * @param user<T> ID of user
+     * @param role Role of the user, either owner, member, or read/write.
+     * 
+     * @return Collection of Groups matching the query, or empty Collection.
+     *         Never null.
+     *
+     * @throws UserNotFoundException If owner or group members not valid users.
+     * @throws TransientException If an temporary, unexpected problem occurred.
+     * @throws AccessControlException If the operation is not permitted.
+     */
+    public abstract Collection<Group> getGroups(User<T> user, Role role)
+        throws UserNotFoundException, TransientException,
+               AccessControlException;
 
-    public abstract boolean isMember(User<T> paramUser, String paramString)
-        throws TransientException, AccessControlException;
+    /**
+     * Check whether the user is a member of the group.
+     *
+     * @param user<T> ID of user
+     * @param groupID ID of group
+     *
+     * @return true or false
+     *
+     * @throws GroupNotFoundException If the group was not found.
+     * @throws TransientException If an temporary, unexpected problem occurred.
+     * @throws AccessControlException If the operation is not permitted.
+     */
+    public abstract boolean isMember(User<T> user, String groupID)
+        throws GroupNotFoundException, TransientException,
+               AccessControlException;
 
 }
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/PluginFactory.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/PluginFactory.java
index 878a021b..e1ce4ab0 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/PluginFactory.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/PluginFactory.java
@@ -91,19 +91,19 @@ public class PluginFactory
     @Override
     public String toString()
     {
-        return getClass().getName() + "[" + this.config.entrySet().size() + "]";
+        return getClass().getName() + "[" + config.entrySet().size() + "]";
     }
 
     private void init()
     {
-        this.config = new Properties();
+        config = new Properties();
         URL url = null;
         try
         {
             url = PluginFactory.class.getClassLoader().getResource(CONFIG);
             if (url != null)
             {
-                this.config.load(url.openStream());
+                config.load(url.openStream());
             }
         }
         catch (Exception ex)
@@ -114,12 +114,12 @@ public class PluginFactory
 
     public <T extends Principal> GroupPersistence<T> getGroupPersistence()
     {
-        GroupPersistence ret = null;
+        GroupPersistence<T> ret = null;
         String name = GroupPersistence.class.getName();
-        String cname = this.config.getProperty(name);
+        String cname = config.getProperty(name);
         if (cname == null)
         {
-            ret = new LdapGroupPersistence();
+            ret = new LdapGroupPersistence<T>();
         }
         else
         {
@@ -138,12 +138,12 @@ public class PluginFactory
 
     public <T extends Principal> UserPersistence<T> getUserPersistence()
     {
-        UserPersistence ret = null;
+        UserPersistence<T> ret = null;
         String name = UserPersistence.class.getName();
-        String cname = this.config.getProperty(name);
+        String cname = config.getProperty(name);
         if (cname == null)
         {
-            ret = new LdapUserPersistence();
+            ret = new LdapUserPersistence<T>();
         }
         else
         {
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/UserPersistence.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/UserPersistence.java
index 31f1419b..9409fc26 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/UserPersistence.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/UserPersistence.java
@@ -76,7 +76,17 @@ import java.security.Principal;
 
 public abstract interface UserPersistence<T extends Principal>
 {
-    public abstract User<T> getUser(T paramT)
+    /**
+     * Get the user specified by userID.
+     *
+     * @param userID The userID.
+     *
+     * @return User instance.
+     * @throws UserNotFoundException when the member is not found.
+     * @throws TransientException If an temporary, unexpected problem occurred.
+     * @throws AccessControlException If the operation is not permitted.
+     */
+    public abstract User<T> getUser(T userID)
         throws UserNotFoundException, TransientException, AccessControlException;
 
 }
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapConfig.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapConfig.java
index 04fb60ec..e0de1381 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapConfig.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapConfig.java
@@ -78,7 +78,8 @@ public class LdapConfig
 {
     private static final Logger logger = Logger.getLogger(LdapConfig.class);
 
-    public static final String CONFIG = LdapConfig.class.getSimpleName() + ".properties";
+    public static final String CONFIG = LdapConfig.class.getSimpleName() + 
+                                        ".properties";
     public static final String LDAP_SERVER = "server";
     public static final String LDAP_PORT = "port";
     public static final String LDAP_ADMIN = "admin";
@@ -86,6 +87,7 @@ public class LdapConfig
     public static final String LDAP_USERS_DN = "usersDn";
     public static final String LDAP_GROUPS_DN = "groupsDn";
     public static final String LDAP_DELETED_GROUPS_DN = "deletedGroupsDn";
+
     private String usersDN;
     private String groupsDN;
     private String deletedGroupsDN;
@@ -110,87 +112,103 @@ public class LdapConfig
             {
                 throw new IOException("File not found");
             }
-
         }
         catch (Exception ex)
         {
-            throw new RuntimeException("failed to read " + CONFIG + " from " + url, ex);
+            throw new RuntimeException("failed to read " + CONFIG + 
+                                       " from " + url, ex);
         }
 
-        String server = config.getProperty("server");
+        String server = config.getProperty(LDAP_SERVER);
         if (!StringUtil.hasText(server))
         {
-            throw new RuntimeException("failed to read server property");
+            throw new RuntimeException("failed to read property " + 
+                                       LDAP_SERVER);
         }
 
-        String port = config.getProperty("port");
+        String port = config.getProperty(LDAP_PORT);
         if (!StringUtil.hasText(port))
         {
-            throw new RuntimeException("failed to read port property");
+            throw new RuntimeException("failed to read property " + LDAP_PORT);
         }
 
-        String ldapAdmin = config.getProperty("admin");
+        String ldapAdmin = config.getProperty(LDAP_ADMIN);
         if (!StringUtil.hasText(ldapAdmin))
         {
-            throw new RuntimeException("failed to read admin property");
+            throw new RuntimeException("failed to read property " + LDAP_ADMIN);
         }
 
-        String ldapPasswd = config.getProperty("passwd");
+        String ldapPasswd = config.getProperty(LDAP_PASSWD);
         if (!StringUtil.hasText(ldapPasswd))
         {
-            throw new RuntimeException("failed to read passwd property");
+            throw new RuntimeException("failed to read property " + 
+                                       LDAP_PASSWD);
         }
 
-        String ldapUsersDn = config.getProperty("usersDn");
+        String ldapUsersDn = config.getProperty(LDAP_USERS_DN);
         if (!StringUtil.hasText(ldapUsersDn))
         {
-            throw new RuntimeException("failed to read usersDn property");
+            throw new RuntimeException("failed to read property " + 
+                                       LDAP_USERS_DN);
         }
 
-        String ldapGroupsDn = config.getProperty("groupsDn");
+        String ldapGroupsDn = config.getProperty(LDAP_GROUPS_DN);
         if (!StringUtil.hasText(ldapGroupsDn))
         {
-            throw new RuntimeException("failed to read groupsDn property");
+            throw new RuntimeException("failed to read property " + 
+                                       LDAP_GROUPS_DN);
         }
 
-        String ldapDeletedGroupsDn = config.getProperty("deletedGroupsDn");
+        String ldapDeletedGroupsDn = config.getProperty(LDAP_DELETED_GROUPS_DN);
         if (!StringUtil.hasText(ldapDeletedGroupsDn))
         {
-            throw new RuntimeException("failed to read deletedGroupsDn property");
+            throw new RuntimeException("failed to read property " + 
+                                       LDAP_DELETED_GROUPS_DN);
         }
 
-        return new LdapConfig(server, Integer.valueOf(port), ldapAdmin, ldapPasswd, ldapUsersDn, ldapGroupsDn, ldapDeletedGroupsDn);
+        return new LdapConfig(server, Integer.valueOf(port), ldapAdmin, 
+                              ldapPasswd, ldapUsersDn, ldapGroupsDn, 
+                              ldapDeletedGroupsDn);
     }
 
-    public LdapConfig(String server, int port, String adminUserDN, String adminPasswd, String usersDN, String groupsDN, String deletedGroupsDN)
+    public LdapConfig(String server, int port, String adminUserDN, 
+                      String adminPasswd, String usersDN, String groupsDN, 
+                      String deletedGroupsDN)
     {
         if (!StringUtil.hasText(server))
         {
-            throw new IllegalArgumentException("Illegal LDAP server name: " + server);
+            throw new IllegalArgumentException("Illegal LDAP server name: " + 
+                                               server);
         }
         if (port < 0)
         {
-            throw new IllegalArgumentException("Illegal LDAP server port: " + port);
+            throw new IllegalArgumentException("Illegal LDAP server port: " + 
+                                               port);
         }
         if (!StringUtil.hasText(adminUserDN))
         {
-            throw new IllegalArgumentException("Illegal Admin DN: " + adminUserDN);
+            throw new IllegalArgumentException("Illegal Admin DN: " + 
+                                               adminUserDN);
         }
         if (!StringUtil.hasText(adminPasswd))
         {
-            throw new IllegalArgumentException("Illegal Admin password: " + adminPasswd);
+            throw new IllegalArgumentException("Illegal Admin password: " + 
+                                               adminPasswd);
         }
         if (!StringUtil.hasText(usersDN))
         {
-            throw new IllegalArgumentException("Illegal users LDAP DN: " + usersDN);
+            throw new IllegalArgumentException("Illegal users LDAP DN: " + 
+                                               usersDN);
         }
         if (!StringUtil.hasText(groupsDN))
         {
-            throw new IllegalArgumentException("Illegal groups LDAP DN: " + groupsDN);
+            throw new IllegalArgumentException("Illegal groups LDAP DN: " + 
+                                               groupsDN);
         }
         if (!StringUtil.hasText(deletedGroupsDN))
         {
-            throw new IllegalArgumentException("Illegal groups LDAP DN: " + deletedGroupsDN);
+            throw new IllegalArgumentException("Illegal groups LDAP DN: " + 
+                                               deletedGroupsDN);
         }
 
         this.server = server;
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapDAO.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapDAO.java
index 0b16851e..643f2962 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapDAO.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapDAO.java
@@ -88,6 +88,7 @@ import javax.security.auth.x500.X500Principal;
 public abstract class LdapDAO
 {
     private LDAPConnection conn;
+    
     LdapConfig config;
     DN subjDN = null;
 
@@ -102,49 +103,52 @@ public abstract class LdapDAO
 
     public void close()
     {
-        if (this.conn != null)
+        if (conn != null)
         {
-            this.conn.close();
+            conn.close();
         }
     }
 
-    protected LDAPConnection getConnection() throws LDAPException, AccessControlException
+    protected LDAPConnection getConnection()
+        throws LDAPException, AccessControlException
     {
-        if (this.conn == null)
+        if (conn == null)
         {
-            this.conn = new LDAPConnection(this.config.getServer(), this.config.getPort());
-            this.conn.bind(this.config.getAdminUserDN(), this.config.getAdminPasswd());
-
-            Subject callerSubject = Subject.getSubject(AccessController.getContext());
+            conn = new LDAPConnection(config.getServer(), config.getPort());
+            conn.bind(config.getAdminUserDN(), config.getAdminPasswd());
 
+            Subject callerSubject = 
+                    Subject.getSubject(AccessController.getContext());
             if (callerSubject == null)
             {
                 throw new AccessControlException("Caller not authenticated.");
             }
+            
             Set<Principal> principals = callerSubject.getPrincipals();
-            if (principals.size() < 1)
+            if (principals.isEmpty())
             {
                 throw new AccessControlException("Caller not authenticated.");
             }
+            
             String ldapField = null;
             for (Principal p : principals)
             {
-                if ((p instanceof HttpPrincipal))
+                if (p instanceof HttpPrincipal)
                 {
                     ldapField = "(uid=" + p.getName() + ")";
                     break;
                 }
-                if ((p instanceof NumericPrincipal))
+                if (p instanceof NumericPrincipal)
                 {
                     ldapField = "(entryid=" + p.getName() + ")";
                     break;
                 }
-                if ((p instanceof X500Principal))
+                if (p instanceof X500Principal)
                 {
                     ldapField = "(distinguishedname=" + p.getName() + ")";
                     break;
                 }
-                if ((p instanceof OpenIdPrincipal))
+                if (p instanceof OpenIdPrincipal)
                 {
                     ldapField = "(openid=" + p.getName() + ")";
                     break;
@@ -156,29 +160,30 @@ public abstract class LdapDAO
                 throw new AccessControlException("Identity of caller unknown.");
             }
 
-            SearchResult searchResult = this.conn.search(this.config.getUsersDN(), SearchScope.ONE, ldapField, new String[]
-                                                     {
-                                                         "entrydn"
-            });
+            SearchResult searchResult = 
+                    conn.search(config.getUsersDN(), SearchScope.ONE, 
+                                ldapField, new String[] {"entrydn"});
 
             if (searchResult.getEntryCount() < 1)
             {
-                throw new AccessControlException("No LDAP account when search with rule " + ldapField);
+                throw new AccessControlException(
+                        "No LDAP account when search with rule " + ldapField);
             }
 
-            this.subjDN = ((SearchResultEntry) searchResult.getSearchEntries().get(0)).getAttributeValueAsDN("entrydn");
+            subjDN = ((SearchResultEntry) searchResult.getSearchEntries()
+                    .get(0)).getAttributeValueAsDN("entrydn");
         }
 
-        return this.conn;
+        return conn;
     }
 
     protected DN getSubjectDN() throws LDAPException
     {
-        if (this.subjDN == null)
+        if (subjDN == null)
         {
             getConnection();
         }
-        return this.subjDN;
+        return subjDN;
     }
 
 }
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java
index 4c0df74c..738710f9 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java
@@ -68,16 +68,18 @@
  */
 package ca.nrc.cadc.ac.server.ldap;
 
+import ca.nrc.cadc.ac.AC;
 import ca.nrc.cadc.ac.Group;
 import ca.nrc.cadc.ac.GroupAlreadyExistsException;
 import ca.nrc.cadc.ac.GroupNotFoundException;
+import ca.nrc.cadc.ac.Role;
 import ca.nrc.cadc.ac.User;
 import ca.nrc.cadc.ac.UserNotFoundException;
 import ca.nrc.cadc.net.TransientException;
 import com.unboundid.ldap.sdk.AddRequest;
 import com.unboundid.ldap.sdk.Attribute;
 import com.unboundid.ldap.sdk.DN;
-import com.unboundid.ldap.sdk.LDAPConnection;
+import com.unboundid.ldap.sdk.Filter;
 import com.unboundid.ldap.sdk.LDAPException;
 import com.unboundid.ldap.sdk.LDAPResult;
 import com.unboundid.ldap.sdk.Modification;
@@ -94,18 +96,25 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Date;
 import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import javax.security.auth.x500.X500Principal;
 import org.apache.log4j.Logger;
 
 public class LdapGroupDAO<T extends Principal> extends LdapDAO
 {
-
     private static final Logger logger = Logger.getLogger(LdapGroupDAO.class);
+    
     private static final String ACTUAL_GROUP_TOKEN = "<ACTUAL_GROUP>";
-    private static final String GROUP_READ_ACI = "(targetattr = \"*\") (version 3.0;acl \"Group Read\";allow (read,compare,search)(groupdn = \"ldap:///<ACTUAL_GROUP>\");)";
-    private static final String GROUP_WRITE_ACI = "(targetattr = \"*\") (version 3.0;acl \"Group Write\";allow (read,compare,search,selfwrite,write,add)(groupdn = \"ldap:///<ACTUAL_GROUP>\");)";
-    private static final String PUB_GROUP_ACI = "(targetattr = \"*\") (version 3.0;acl \"Group Public\";allow (read,compare,search)userdn=\"ldap:///anyone\";)";
+    private static final String GROUP_READ_ACI = "(targetattr = \"*\") " + 
+            "(version 3.0;acl \"Group Read\";allow (read,compare,search)" + 
+            "(groupdn = \"ldap:///<ACTUAL_GROUP>\");)";
+    private static final String GROUP_WRITE_ACI = "(targetattr = \"*\") " + 
+            "(version 3.0;acl \"Group Write\";allow " + 
+            "(read,compare,search,selfwrite,write,add)" + 
+            "(groupdn = \"ldap:///<ACTUAL_GROUP>\");)";
+    private static final String PUB_GROUP_ACI = "(targetattr = \"*\") " + 
+            "(version 3.0;acl \"Group Public\";" + 
+            "allow (read,compare,search)userdn=\"ldap:///anyone\";)";
+    
     private LdapUserDAO<T> userPersist;
 
     public LdapGroupDAO(LdapConfig config, LdapUserDAO<T> userPersist)
@@ -113,20 +122,44 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         super(config);
         if (userPersist == null)
         {
-            throw new IllegalArgumentException("User persistence instance required");
+            throw new IllegalArgumentException(
+                    "User persistence instance required");
         }
-
         this.userPersist = userPersist;
     }
 
+    /**
+     * Get the group with the given Group ID.
+     * 
+     * @param groupID The Group unique ID.
+     * 
+     * @return A Group instance
+     * 
+     * @throws GroupNotFoundException If the group was not found.
+     * @throws TransientException  If an temporary, unexpected problem occurred.
+     */
     public Group getGroup(String groupID)
-        throws GroupNotFoundException, TransientException, AccessControlException
+        throws GroupNotFoundException, TransientException,
+               AccessControlException
     {
         return getGroup(groupID, true);
     }
 
+    /**
+     * Creates the group.
+     * 
+     * @param group The group to create
+     * 
+     * @return created group
+     * 
+     * @throws GroupAlreadyExistsException If a group with the same ID already 
+     *                                     exists.
+     * @throws TransientException If an temporary, unexpected problem occurred.
+     * @throws UserNotFoundException If owner or a member not valid user.
+     */
     public Group addGroup(Group group)
-        throws GroupAlreadyExistsException, TransientException, UserNotFoundException
+        throws GroupAlreadyExistsException, TransientException,
+               UserNotFoundException, AccessControlException
     {
         try
         {
@@ -137,41 +170,51 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         {
             try
             {
-                if (group.getProperties().size() > 0)
+                if (!group.getProperties().isEmpty())
                 {
-                    throw new UnsupportedOperationException("Support for groups properties not available");
+                    throw new UnsupportedOperationException(
+                            "Support for groups properties not available");
                 }
 
-                DN ownerDN = this.userPersist.getUserDN(group.getOwner());
+                DN ownerDN = userPersist.getUserDN(group.getOwner());
                 String groupWriteAci = null;
                 String groupReadAci = null;
                 if (group.groupWrite != null)
                 {
                     DN groupWrite = getGroupDN(group.groupWrite.getID());
-                    groupWriteAci = "(targetattr = \"*\") (version 3.0;acl \"Group Write\";allow (read,compare,search,selfwrite,write,add)(groupdn = \"ldap:///<ACTUAL_GROUP>\");)".replace("<ACTUAL_GROUP>", groupWrite.toNormalizedString());
+                    groupWriteAci = GROUP_WRITE_ACI.replace(
+                            ACTUAL_GROUP_TOKEN, 
+                            groupWrite.toNormalizedString());
                 }
 
                 if (group.groupRead != null)
                 {
                     DN groupRead = getGroupDN(group.groupRead.getID());
-                    groupReadAci = "(targetattr = \"*\") (version 3.0;acl \"Group Read\";allow (read,compare,search)(groupdn = \"ldap:///<ACTUAL_GROUP>\");)".replace("<ACTUAL_GROUP>", groupRead.toNormalizedString());
+                    groupReadAci = GROUP_READ_ACI.replace(
+                            ACTUAL_GROUP_TOKEN, 
+                            groupRead.toNormalizedString());
                 }
 
-                List attributes = new ArrayList();
-                attributes.add(new Attribute("objectClass", "groupofuniquenames"));
+                // add new group
+                List<Attribute> attributes = new ArrayList<Attribute>();
+                attributes.add(new Attribute("objectClass", 
+                                             "groupofuniquenames"));
 
                 attributes.add(new Attribute("cn", group.getID()));
                 if (group.description != null)
                 {
-                    attributes.add(new Attribute("description", group.description));
+                    attributes.add(new Attribute("description", 
+                                                 group.description));
                 }
 
-                attributes.add(new Attribute("owner", ownerDN.toNormalizedString()));
+                attributes.add(new Attribute("owner", 
+                                             ownerDN.toNormalizedString()));
 
-                List acis = new ArrayList();
+                // acis
+                List<String> acis = new ArrayList<String>();
                 if (group.publicRead)
                 {
-                    acis.add("(targetattr = \"*\") (version 3.0;acl \"Group Public\";allow (read,compare,search)userdn=\"ldap:///anyone\";)");
+                    acis.add(PUB_GROUP_ACI);
                 }
                 if (groupWriteAci != null)
                 {
@@ -182,12 +225,13 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
                     acis.add(groupReadAci);
                 }
 
-                if (acis.size() > 0)
+                if (!acis.isEmpty())
                 {
-                    attributes.add(new Attribute("aci", (String[]) acis.toArray(new String[acis.size()])));
+                    attributes.add(new Attribute("aci", 
+                            (String[]) acis.toArray(new String[acis.size()])));
                 }
 
-                List members = new ArrayList();
+                List<String> members = new ArrayList<String>();
                 for (User member : group.getUserMembers())
                 {
                     DN memberDN = this.userPersist.getUserDN(member);
@@ -198,14 +242,18 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
                     DN grDN = getGroupDN(gr.getID());
                     members.add(grDN.toNormalizedString());
                 }
-                if (members.size() > 0)
+                if (!members.isEmpty())
                 {
-                    attributes.add(new Attribute("uniquemember", (String[]) members.toArray(new String[members.size()])));
+                    attributes.add(new Attribute("uniquemember", 
+                        (String[]) members.toArray(new String[members.size()])));
                 }
 
-                AddRequest addRequest = new AddRequest(getGroupDN(group.getID()), attributes);
+                AddRequest addRequest = 
+                        new AddRequest(getGroupDN(group.getID()), attributes);
 
-                addRequest.addControl(new ProxiedAuthorizationV2RequestControl("dn:" + getSubjectDN().toNormalizedString()));
+                addRequest.addControl(
+                        new ProxiedAuthorizationV2RequestControl(
+                                "dn:" + getSubjectDN().toNormalizedString()));
 
                 LDAPResult result = getConnection().add(addRequest);
                 try
@@ -224,17 +272,29 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         }
     }
 
+    /**
+     * Deletes the group.
+     * 
+     * @param groupID The group to delete
+     * 
+     * @throws GroupNotFoundException If the group was not found.
+     * @throws TransientException If an temporary, unexpected problem occurred.
+     */
     public void deleteGroup(String groupID)
-        throws GroupNotFoundException, TransientException
+        throws GroupNotFoundException, TransientException,
+               AccessControlException
     {
         Group group = getGroup(groupID, false);
-
         DN groupDN = getGroupDN(group.getID());
         try
         {
-            ModifyDNRequest modifyDNRequest = new ModifyDNRequest(group.getID(), group.getID(), true, this.config.getDeletedGroupsDN());
+            ModifyDNRequest modifyDNRequest = new ModifyDNRequest(
+                    group.getID(), group.getID(), 
+                    true, config.getDeletedGroupsDN());
 
-            modifyDNRequest.addControl(new ProxiedAuthorizationV2RequestControl("dn:" + getSubjectDN().toNormalizedString()));
+            modifyDNRequest.addControl(
+                    new ProxiedAuthorizationV2RequestControl("dn:" + 
+                            getSubjectDN().toNormalizedString()));
 
             LDAPResult result = getConnection().modifyDN(modifyDNRequest);
         }
@@ -242,36 +302,96 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         {
             throw new RuntimeException("LDAP problem", e1);
         }
+        
         try
         {
             getGroup(group.getID());
-            throw new RuntimeException("BUG: group not deleted " + group.getID());
+            throw new RuntimeException("BUG: group not deleted " + 
+                                       group.getID());
+        }
+        catch (GroupNotFoundException ignore) {}
+    }
+
+    /**
+     * Obtain a Collection of Groups that fit the given query.
+     * 
+     * @param user<T> ID of user
+     * @param role Role of the user, either owner, member, or read/write.
+     * 
+     * @return Collection of Groups
+     *         matching GROUP_READ_ACI.replace(ACTUAL_GROUP_TOKEN,
+     *         readGrDN.toNormalizedString()) the query, or empty
+     *         Collection. Never null.
+     * @throws TransientException  If an temporary, unexpected problem occurred.
+     * @throws ca.nrc.cadc.ac.UserNotFoundException
+     */
+    public Collection<Group> getGroups(User<T> user, Role role)
+        throws TransientException, AccessControlException,
+               UserNotFoundException
+    {
+        try
+        {
+            Filter filter;
+            switch (role)
+            {
+                case AC.ID_TYPE_X500:
+                
+                    
+                    
+            }
+            
+            SearchRequest searchRequest =  new SearchRequest(
+                    config.getGroupsDN(), SearchScope.SUB, 
+                    "(cn=" + groupID + ")", new String[] {"entrydn", "entryid", 
+                    "cn", "description", "owner", "uniquemember", "aci", 
+                    "modifytimestamp"});
         }
-        catch (GroupNotFoundException ignore)
+        catch (LDAPException e1)
         {
+            // TODO check which LDAP exceptions are transient and which
+            // ones are
+            // access control
+            throw new TransientException("Error getting the group", e1);
         }
     }
 
-    public Collection<Group> getGroups(Map<String, String> criteria)
-        throws TransientException, AccessControlException
+    public boolean isMember(User<T> member, String groupID)
+        throws UserNotFoundException, TransientException,
+               AccessControlException
     {
-        return null;
+        return false;
+//        try
+//        {
+//            
+//           
+//        }
+//        catch (LDAPException e1)
+//        {
+//            // TODO check which LDAP exceptions are transient and which
+//            // ones are
+//            // access control
+//            throw new TransientException("Error getting the group", e1);
+//        }
     }
-
+    
     private Group getGroup(String groupID, boolean withMembers)
-        throws GroupNotFoundException, TransientException, AccessControlException
+        throws GroupNotFoundException, TransientException, 
+               AccessControlException
     {
         try
         {
-            SearchRequest searchRequest = 
-                new SearchRequest(
-                    this.config.getGroupsDN(), SearchScope.SUB, "(cn=" + groupID + ")", 
-                    new String[] {"entrydn", "entryid", "cn", "description", "owner", "uniquemember", "aci", "modifytimestamp" });
-
-            searchRequest.addControl(new ProxiedAuthorizationV2RequestControl("dn:" + getSubjectDN().toNormalizedString()));
-
-            SearchResultEntry group = getConnection().searchForEntry(searchRequest);
-
+            SearchRequest searchRequest =  new SearchRequest(
+                    config.getGroupsDN(), SearchScope.SUB, 
+                    "(cn=" + groupID + ")", new String[] {"entrydn", "entryid", 
+                    "cn", "description", "owner", "uniquemember", "aci", 
+                    "modifytimestamp"});
+
+            searchRequest.addControl(
+                    new ProxiedAuthorizationV2RequestControl("dn:" + 
+                            getSubjectDN().toNormalizedString()));
+
+            SearchResultEntry group = 
+                    getConnection().searchForEntry(searchRequest);
             if (group == null)
             {
                 String msg = "Group not found " + groupID;
@@ -281,81 +401,87 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
             String groupCN = group.getAttributeValue("cn");
             DN groupOwner = group.getAttributeValueAsDN("owner");
             Long grID = group.getAttributeValueAsLong("entryid");
-            Date lastModified = group.getAttributeValueAsDate("modifytimestamp");
-            User owner;
+            Date lastModified = 
+                group.getAttributeValueAsDate("modifytimestamp");
+            
+            User<X500Principal> owner;
             try
             {
-                owner = this.userPersist.getMember(groupOwner);
+                owner = userPersist.getMember(groupOwner);
             }
             catch (UserNotFoundException e)
             {
                 throw new RuntimeException("BUG: group owner not found");
             }
+            
             Group ldapGroup = new Group(groupCN, owner);
-
             ldapGroup.description = group.getAttributeValue("description");
-
             ldapGroup.lastModified = lastModified;
 
             if (withMembers)
             {
                 if (group.getAttributeValues("uniquemember") != null)
                 {
-                    for (String member : group.getAttributeValues("uniquemember"))
+                    for (String member : group
+                            .getAttributeValues("uniquemember"))
                     {
                         DN memberDN = new DN(member);
-                        if (memberDN.isDescendantOf(this.config.getUsersDN(), false))
+                        if (memberDN.isDescendantOf(config.getUsersDN(), false))
                         {
-                            User usr;
+                            User<X500Principal> usr;
                             try
                             {
-                                usr = this.userPersist.getMember(memberDN);
+                                usr = userPersist.getMember(memberDN);
                             }
                             catch (UserNotFoundException e)
                             {
-                                throw new RuntimeException("BUG: group member not found");
+                                throw new RuntimeException(
+                                    "BUG: group member not found");
                             }
-
                             ldapGroup.getUserMembers().add(usr);
                         }
-                        else if (memberDN.isDescendantOf(this.config.getGroupsDN(), false))
+                        else if (memberDN.isDescendantOf(config.getGroupsDN(),
+                                                         false))
                         {
                             Group memberGroup = getGroup(memberDN);
                             ldapGroup.getGroupMembers().add(memberGroup);
                         }
                         else
                         {
-                            throw new RuntimeException("BUG: unknown member DN type: " + memberDN);
+                            throw new RuntimeException(
+                                "BUG: unknown member DN type: " + memberDN);
                         }
-
                     }
-
                 }
 
+                // TODO not sure this is going to fly...
                 if (group.getAttributeValues("aci") != null)
                 {
                     for (String aci : group.getAttributeValues("aci"))
                     {
                         if (aci.contains("Group Read"))
                         {
-                            String grRead = aci.substring(aci.indexOf("ldap:///"));
-
-                            grRead = grRead.substring(grRead.indexOf("cn"), grRead.lastIndexOf('"'));
+                            // TODO it's gotta be a better way to do this.
+                            String grRead = aci.substring(
+                                    aci.indexOf("ldap:///"));
+                            grRead = grRead.substring(grRead.indexOf("cn"),
+                                                      grRead.lastIndexOf('"'));
 
                             Group groupRead = getGroup(new DN(grRead));
                             ldapGroup.groupRead = groupRead;
                         }
                         else if (aci.contains("Group Write"))
                         {
-                            String grWrite = aci.substring(aci.indexOf("ldap:///"));
-
-                            grWrite = grWrite.substring(grWrite.indexOf("cn"), grWrite.lastIndexOf('"'));
+                            // TODO it's gotta be a better way to do this.
+                            String grWrite = aci.substring(
+                                    aci.indexOf("ldap:///"));
+                            grWrite = grWrite.substring(grWrite.indexOf("cn"), 
+                                                    grWrite.lastIndexOf('"'));
 
                             Group groupWrite = getGroup(new DN(grWrite));
-
                             ldapGroup.groupWrite = groupWrite;
                         }
-                        else if (aci.equals("(targetattr = \"*\") (version 3.0;acl \"Group Public\";allow (read,compare,search)userdn=\"ldap:///anyone\";)"))
+                        else if (aci.equals(PUB_GROUP_ACI))
                         {
                             ldapGroup.publicRead = true;
                         }
@@ -367,6 +493,9 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         }
         catch (LDAPException e1)
         {
+            // TODO check which LDAP exceptions are transient and which
+            // ones are
+            // access control
             throw new TransientException("Error getting the group", e1);
         }
         catch (UserNotFoundException e2)
@@ -376,68 +505,78 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
     }
 
     public Group modifyGroup(Group group)
-        throws GroupNotFoundException, TransientException, AccessControlException, UserNotFoundException
+        throws GroupNotFoundException, TransientException,
+               AccessControlException, UserNotFoundException
     {
+        // check if group exists
         Group oldGroup = getGroup(group.getID());
-        if (group.getProperties().size() > 0)
+        if (!group.getProperties().isEmpty())
         {
-            throw new UnsupportedOperationException("Support for groups properties not available");
+            throw new UnsupportedOperationException(
+                    "Support for groups properties not available");
         }
 
-        List modifs = new ArrayList();
+        List<Modification> modifs = new ArrayList<Modification>();
         if (group.description == null)
         {
-            modifs.add(new Modification(ModificationType.DELETE, "description"));
+            modifs.add(new Modification(ModificationType.DELETE, 
+                                        "description"));
         }
         else
         {
-            modifs.add(new Modification(ModificationType.REPLACE, "description", group.description));
+            modifs.add(new Modification(ModificationType.REPLACE, "description", 
+                                        group.description));
         }
 
-        List acis = new ArrayList();
+        List<String> acis = new ArrayList<String>();
         if (group.groupRead != null)
         {
             if (group.groupRead.equals(group))
             {
-                throw new IllegalArgumentException("cyclical reference from groupRead to group");
+                throw new IllegalArgumentException(
+                        "cyclical reference from groupRead to group");
             }
 
             DN readGrDN = getGroupDN(group.groupRead.getID());
-            acis.add("(targetattr = \"*\") (version 3.0;acl \"Group Read\";allow (read,compare,search)(groupdn = \"ldap:///<ACTUAL_GROUP>\");)".replace("<ACTUAL_GROUP>", readGrDN.toNormalizedString()));
+            acis.add(GROUP_READ_ACI.replace(ACTUAL_GROUP_TOKEN, 
+                                            readGrDN.toNormalizedString()));
         }
 
         if (group.groupWrite != null)
         {
             if (group.groupWrite.equals(group))
             {
-                throw new IllegalArgumentException("cyclical reference from groupWrite to group");
+                throw new IllegalArgumentException(
+                        "cyclical reference from groupWrite to group");
             }
 
             DN writeGrDN = getGroupDN(group.groupWrite.getID());
-            acis.add("(targetattr = \"*\") (version 3.0;acl \"Group Write\";allow (read,compare,search,selfwrite,write,add)(groupdn = \"ldap:///<ACTUAL_GROUP>\");)".replace("<ACTUAL_GROUP>", writeGrDN.toNormalizedString()));
+            acis.add(GROUP_WRITE_ACI.replace(ACTUAL_GROUP_TOKEN, 
+                                             writeGrDN.toNormalizedString()));
         }
 
         if (group.publicRead)
         {
-            acis.add("(targetattr = \"*\") (version 3.0;acl \"Group Public\";allow (read,compare,search)userdn=\"ldap:///anyone\";)");
+            acis.add(PUB_GROUP_ACI);
         }
-        modifs.add(new Modification(ModificationType.REPLACE, "aci", (String[]) acis.toArray(new String[acis.size()])));
+        modifs.add(new Modification(ModificationType.REPLACE, "aci", (String[]) 
+                                    acis.toArray(new String[acis.size()])));
 
-        List newMembers = new ArrayList();
-        for (User member : group.getUserMembers())
+        List<String> newMembers = new ArrayList<String>();
+        for (User<?> member : group.getUserMembers())
         {
             if (!oldGroup.getUserMembers().remove(member))
             {
                 DN memberDN;
                 try
                 {
-                    memberDN = this.userPersist.getUserDN(member);
+                    memberDN = userPersist.getUserDN(member);
                 }
                 catch (LDAPException e)
                 {
-                    throw new UserNotFoundException("User not found " + member.getUserID());
+                    throw new UserNotFoundException(
+                            "User not found " + member.getUserID());
                 }
-
                 newMembers.add(memberDN.toNormalizedString());
             }
         }
@@ -445,7 +584,8 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         {
             if (gr.equals(group))
             {
-                throw new IllegalArgumentException("cyclical reference from group member to group");
+                throw new IllegalArgumentException(
+                        "cyclical reference from group member to group");
             }
 
             if (!oldGroup.getGroupMembers().remove(gr))
@@ -454,12 +594,13 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
                 newMembers.add(grDN.toNormalizedString());
             }
         }
-        if (newMembers.size() > 0)
+        if (!newMembers.isEmpty())
         {
-            modifs.add(new Modification(ModificationType.ADD, "uniquemember", (String[]) newMembers.toArray(new String[newMembers.size()])));
+            modifs.add(new Modification(ModificationType.ADD, "uniquemember", 
+                (String[]) newMembers.toArray(new String[newMembers.size()])));
         }
 
-        List delMembers = new ArrayList();
+        List<String> delMembers = new ArrayList<String>();
         for (User member : oldGroup.getUserMembers())
         {
             DN memberDN;
@@ -469,9 +610,9 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
             }
             catch (LDAPException e)
             {
-                throw new UserNotFoundException("User not found " + member.getUserID());
+                throw new UserNotFoundException(
+                        "User not found " + member.getUserID());
             }
-
             delMembers.add(memberDN.toNormalizedString());
         }
         for (Group gr : oldGroup.getGroupMembers())
@@ -479,15 +620,19 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
             DN grDN = getGroupDN(gr.getID());
             delMembers.add(grDN.toNormalizedString());
         }
-        if (delMembers.size() > 0)
+        if (!delMembers.isEmpty())
         {
-            modifs.add(new Modification(ModificationType.DELETE, "uniquemember", (String[]) delMembers.toArray(new String[delMembers.size()])));
+            modifs.add(new Modification(ModificationType.DELETE, "uniquemember",
+                (String[]) delMembers.toArray(new String[delMembers.size()])));
         }
 
-        ModifyRequest modifyRequest = new ModifyRequest(getGroupDN(group.getID()), modifs);
+        ModifyRequest modifyRequest = 
+                new ModifyRequest(getGroupDN(group.getID()), modifs);
         try
         {
-            modifyRequest.addControl(new ProxiedAuthorizationV2RequestControl("dn:" + getSubjectDN().toNormalizedString()));
+            modifyRequest.addControl(
+                    new ProxiedAuthorizationV2RequestControl(
+                            "dn:" + getSubjectDN().toNormalizedString()));
             LDAPResult result = getConnection().modify(modifyRequest);
         }
         catch (LDAPException e1)
@@ -500,19 +645,28 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         }
         catch (GroupNotFoundException e)
         {
+            throw new RuntimeException("BUG: new group not found");
         }
-        throw new RuntimeException("BUG: new group not found");
     }
 
+    /**
+     * Returns a group based on its LDAP DN. The returned group is bared
+     * (contains only group ID, owner and description).
+     * 
+     * @param groupDN
+     * @return
+     * @throws com.unboundid.ldap.sdk.LDAPException
+     * @throws ca.nrc.cadc.ac.GroupNotFoundException
+     * @throws ca.nrc.cadc.ac.UserNotFoundException
+     */
     protected Group getGroup(DN groupDN)
         throws LDAPException, GroupNotFoundException, UserNotFoundException
     {
         SearchResultEntry searchResult = null;
 
-        searchResult = getConnection().getEntry(groupDN.toNormalizedString(), new String[]
-                                            {
-                                                "cn", "description", "owner"
-        });
+        searchResult = 
+                getConnection().getEntry(groupDN.toNormalizedString(),
+                                new String[] {"cn", "description", "owner"});
 
         if (searchResult == null)
         {
@@ -522,7 +676,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
         }
 
         DN ownerDN = searchResult.getAttributeValueAsDN("owner");
-        User owner = this.userPersist.getMember(ownerDN);
+        User<X500Principal> owner = userPersist.getMember(ownerDN);
         Group group = new Group(searchResult.getAttributeValue("cn"), owner);
 
         return group;
@@ -532,7 +686,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
     {
         try
         {
-            return new DN("cn=" + groupID + "," + this.config.getGroupsDN());
+            return new DN("cn=" + groupID + "," + config.getGroupsDN());
         }
         catch (LDAPException e)
         {
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupPersistence.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupPersistence.java
index e8463e61..2704962f 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupPersistence.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupPersistence.java
@@ -71,6 +71,7 @@ package ca.nrc.cadc.ac.server.ldap;
 import ca.nrc.cadc.ac.Group;
 import ca.nrc.cadc.ac.GroupAlreadyExistsException;
 import ca.nrc.cadc.ac.GroupNotFoundException;
+import ca.nrc.cadc.ac.Role;
 import ca.nrc.cadc.ac.User;
 import ca.nrc.cadc.ac.UserNotFoundException;
 import ca.nrc.cadc.ac.server.GroupPersistence;
@@ -78,27 +79,28 @@ import ca.nrc.cadc.net.TransientException;
 import java.security.AccessControlException;
 import java.security.Principal;
 import java.util.Collection;
-import java.util.Map;
 import org.apache.log4j.Logger;
 
 public class LdapGroupPersistence<T extends Principal>
     implements GroupPersistence<T>
 {
-    private static final Logger logger = Logger.getLogger(LdapGroupPersistence.class);
-    private LdapConfig config;
+    private static final Logger log = 
+            Logger.getLogger(LdapGroupPersistence.class);
+    private final LdapConfig config;
 
     public LdapGroupPersistence()
     {
-        this.config = LdapConfig.getLdapConfig();
+        config = LdapConfig.getLdapConfig();
     }
 
     public Group getGroup(String groupName)
-        throws GroupNotFoundException, TransientException, AccessControlException
+        throws GroupNotFoundException, TransientException,
+               AccessControlException
     {
         LdapGroupDAO groupDAO = null;
         try
         {
-            groupDAO = new LdapGroupDAO(this.config, new LdapUserDAO(this.config));
+            groupDAO = new LdapGroupDAO(config, new LdapUserDAO(config));
             Group ret = groupDAO.getGroup(groupName);
             return ret;
         }
@@ -112,12 +114,13 @@ public class LdapGroupPersistence<T extends Principal>
     }
 
     public Group addGroup(Group group)
-        throws GroupAlreadyExistsException, TransientException, AccessControlException, UserNotFoundException
+        throws GroupAlreadyExistsException, TransientException, 
+               AccessControlException, UserNotFoundException
     {
         LdapGroupDAO groupDAO = null;
         try
         {
-            groupDAO = new LdapGroupDAO(this.config, new LdapUserDAO(this.config));
+            groupDAO = new LdapGroupDAO(config, new LdapUserDAO(config));
             Group ret = groupDAO.addGroup(group);
             return ret;
         }
@@ -131,12 +134,13 @@ public class LdapGroupPersistence<T extends Principal>
     }
 
     public void deleteGroup(String groupName)
-        throws GroupNotFoundException, TransientException, AccessControlException
+        throws GroupNotFoundException, TransientException,
+               AccessControlException
     {
         LdapGroupDAO groupDAO = null;
         try
         {
-            groupDAO = new LdapGroupDAO(this.config, new LdapUserDAO(this.config));
+            groupDAO = new LdapGroupDAO(config, new LdapUserDAO(config));
             groupDAO.deleteGroup(groupName);
         }
         finally
@@ -149,12 +153,13 @@ public class LdapGroupPersistence<T extends Principal>
     }
 
     public Group modifyGroup(Group group)
-        throws GroupNotFoundException, TransientException, AccessControlException, UserNotFoundException
+        throws GroupNotFoundException, TransientException,
+               AccessControlException, UserNotFoundException
     {
         LdapGroupDAO groupDAO = null;
         try
         {
-            groupDAO = new LdapGroupDAO(this.config, new LdapUserDAO(this.config));
+            groupDAO = new LdapGroupDAO(config, new LdapUserDAO(config));
             Group ret = groupDAO.modifyGroup(group);
             return ret;
         }
@@ -167,16 +172,43 @@ public class LdapGroupPersistence<T extends Principal>
         }
     }
 
-    public Collection<Group> getGroups(Map<String, String> criteria)
-        throws TransientException, AccessControlException
+    public Collection<Group> getGroups(User<T> user, Role role)
+        throws UserNotFoundException, TransientException, AccessControlException
     {
-        throw new UnsupportedOperationException("To be implemented");
+        LdapGroupDAO groupDAO = null;
+        try
+        {
+            groupDAO = new LdapGroupDAO(config, new LdapUserDAO(config));
+            Collection<Group> ret = groupDAO.getGroups(user, role);
+            return ret;
+        }
+        finally
+        {
+            if (groupDAO != null)
+            {
+                groupDAO.close();
+            }
+        }
     }
 
-    public boolean isMember(User<T> member, String groupName)
-        throws TransientException, AccessControlException
+    public boolean isMember(User<T> member, String groupID)
+        throws GroupNotFoundException, TransientException,
+               AccessControlException
     {
-        throw new UnsupportedOperationException("To be implemented");
+        LdapGroupDAO groupDAO = null;
+        try
+        {
+            groupDAO = new LdapGroupDAO(config, new LdapUserDAO(config));
+            boolean ret = groupDAO.isMember(member, groupID);
+            return ret;
+        }
+        finally
+        {
+            if (groupDAO != null)
+            {
+                groupDAO.close();
+            }
+        }
     }
 
 }
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java
index 2fb52ecd..645b5fda 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java
@@ -93,7 +93,8 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
 {
     private static final Logger logger = Logger.getLogger(LdapUserDAO.class);
 
-    private Map<Class<?>, String> attribType = new HashMap();
+    // Map of identity type to LDAP attribute
+    private Map<Class<?>, String> attribType = new HashMap<Class<?>, String>();
 
     public LdapUserDAO(LdapConfig config)
     {
@@ -103,13 +104,22 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
         this.attribType.put(NumericPrincipal.class, "entryid");
     }
 
+    /**
+     * Get the user specified by userID.
+     * 
+     * @param userID The unique userID.
+     * @return User instance.
+     * @throws UserNotFoundException when the user is not found.
+     * @throws TransientException If an temporary, unexpected problem occurred.
+     */
     public User<T> getUser(T userID)
         throws UserNotFoundException, TransientException, AccessControlException
     {
-        String searchField = (String) this.attribType.get(userID.getClass());
+        String searchField = (String) attribType.get(userID.getClass());
         if (searchField == null)
         {
-            throw new IllegalArgumentException("Unsupported principal type " + userID.getClass());
+            throw new IllegalArgumentException(
+                    "Unsupported principal type " + userID.getClass());
         }
 
         searchField = "(" + searchField + "=" + userID.getName() + ")";
@@ -117,12 +127,12 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
         SearchResultEntry searchResult = null;
         try
         {
-            SearchRequest searchRequest = new SearchRequest(this.config.getUsersDN(), SearchScope.SUB, searchField, new String[]
-                                                        {
-                                                            "cn", "entryid", "entrydn", "dn"
-            });
+            SearchRequest searchRequest = new SearchRequest(config.getUsersDN(), 
+                    SearchScope.SUB, searchField, 
+                    new String[] {"cn", "entryid", "entrydn", "dn"});
 
-            searchRequest.addControl(new ProxiedAuthorizationV1RequestControl(getSubjectDN()));
+            searchRequest.addControl(
+                    new ProxiedAuthorizationV1RequestControl(getSubjectDN()));
 
             searchResult = getConnection().searchForEntry(searchRequest);
         }
@@ -137,18 +147,31 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
             logger.debug(msg);
             throw new UserNotFoundException(msg);
         }
-        User user = new User(userID);
-        user.getIdentities().add(new HttpPrincipal(searchResult.getAttributeValue("cn")));
+        User<T> user = new User<T>(userID);
+        user.getIdentities().add(
+                new HttpPrincipal(searchResult.getAttributeValue("cn")));
 
-        user.getIdentities().add(new NumericPrincipal(searchResult.getAttributeValueAsInteger("entryid")));
+        user.getIdentities().add(
+                new NumericPrincipal(
+                        searchResult.getAttributeValueAsInteger("entryid")));
 
         return user;
     }
 
+    /**
+     * Returns a member user identified by the X500Principal only.
+     * @param userDN
+     * @return
+     * @throws UserNotFoundException
+     * @throws LDAPException
+     */
     User<X500Principal> getMember(DN userDN)
         throws UserNotFoundException, LDAPException
     {
-        SearchResultEntry searchResult = getConnection().getEntry(userDN.toNormalizedString(), (String[]) this.attribType.values().toArray(new String[this.attribType.values().size()]));
+        SearchResultEntry searchResult = getConnection().getEntry(
+                userDN.toNormalizedString(), 
+                        (String[]) this.attribType.values().toArray(
+                                new String[this.attribType.values().size()]));
 
         if (searchResult == null)
         {
@@ -156,7 +179,9 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
             logger.debug(msg);
             throw new UserNotFoundException(msg);
         }
-        User user = new User(new X500Principal(searchResult.getAttributeValue((String) this.attribType.get(X500Principal.class))));
+        User<X500Principal> user = new User<X500Principal>(
+                new X500Principal(searchResult.getAttributeValue(
+                        (String) attribType.get(X500Principal.class))));
 
         return user;
     }
@@ -164,21 +189,25 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
     DN getUserDN(User<? extends Principal> user)
         throws LDAPException, UserNotFoundException
     {
-        String searchField = (String) this.attribType.get(user.getUserID().getClass());
+        String searchField = (String) attribType.get(user.getUserID().getClass());
         if (searchField == null)
         {
-            throw new IllegalArgumentException("Unsupported principal type " + user.getUserID().getClass());
+            throw new IllegalArgumentException(
+                "Unsupported principal type " + user.getUserID().getClass());
         }
 
-        searchField = "(" + searchField + "=" + user.getUserID().getName() + ")";
+        searchField = "(" + searchField + "=" + 
+                      user.getUserID().getName() + ")";
 
         SearchRequest searchRequest = 
                 new SearchRequest(this.config.getUsersDN(), SearchScope.SUB, 
                                  searchField, new String[] {"entrydn"});
 
-        searchRequest.addControl(new ProxiedAuthorizationV1RequestControl(getSubjectDN()));
+        searchRequest.addControl(
+                new ProxiedAuthorizationV1RequestControl(getSubjectDN()));
 
-        SearchResultEntry searchResult = getConnection().searchForEntry(searchRequest);
+        SearchResultEntry searchResult = 
+                getConnection().searchForEntry(searchRequest);
 
         if (searchResult == null)
         {
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/ACSearchRunner.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/ACSearchRunner.java
index 56d9d77d..0c85a4d5 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/ACSearchRunner.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/ACSearchRunner.java
@@ -80,23 +80,28 @@ public class ACSearchRunner
     private SyncOutput syncOut;
     private Job job;
 
+    @Override
     public void setJobUpdater(JobUpdater jobUpdater)
     {
         this.jobUpdater = jobUpdater;
     }
 
+    @Override
     public void setJob(Job job)
     {
         this.job = job;
     }
 
+    @Override
     public void setSyncOutput(SyncOutput syncOut)
     {
         this.syncOut = syncOut;
     }
 
+    @Override
     public void run()
     {
+        // TODO Run the search query against GroupPersistence
     }
 
 }
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/GroupsServlet.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/GroupsServlet.java
index bd0713ee..0f2e7a8b 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/GroupsServlet.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/GroupsServlet.java
@@ -81,6 +81,9 @@ public class GroupsServlet extends HttpServlet
 {
     private static final Logger log = Logger.getLogger(GroupsServlet.class);
 
+    /**
+     * Create a GroupAction and run the action safely.
+     */
     private void doAction(HttpServletRequest request, HttpServletResponse response)
         throws IOException
     {
@@ -90,6 +93,15 @@ public class GroupsServlet extends HttpServlet
         {
             log.info(logInfo.start());
 
+            // Note: For this servlet, one does not want the subject to be
+            // augmented with all user principals, only the one in which
+            // they used to connect to the service.  This is accomplished
+            // by ensuring that there is no authenticator implementation
+            // available in the classpath with the name:
+            //   ca.nrc.cadc.auth.AuthenticatorImpl.class
+            // See cadcUtil method ca.nrc.cadc.auth.AuthenticationUtil#getAuthenticator()
+            // for more information.
+            
             Subject subject = AuthenticationUtil.getSubject(request);
             logInfo.setSubject(subject);
             GroupsAction action = GroupsActionFactory.getGroupsAction(request, logInfo);
diff --git a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapDAOTest.java b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapDAOTest.java
new file mode 100644
index 00000000..2a16ce97
--- /dev/null
+++ b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapDAOTest.java
@@ -0,0 +1,172 @@
+/**
+ ************************************************************************
+ *******************  CANADIAN ASTRONOMY DATA CENTRE  *******************
+ **************  CENTRE CANADIEN DE DONNÉES ASTRONOMIQUES  **************
+ *
+ *  (c) 2014.                            (c) 2014.
+ *  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/>.
+ *
+ ************************************************************************
+ */
+
+
+package ca.nrc.cadc.ac.server.ldap;
+
+import static org.junit.Assert.assertTrue;
+
+import java.security.PrivilegedExceptionAction;
+
+import javax.security.auth.Subject;
+import javax.security.auth.x500.X500Principal;
+
+import org.junit.Test;
+
+import ca.nrc.cadc.auth.HttpPrincipal;
+import ca.nrc.cadc.auth.NumericPrincipal;
+
+import com.unboundid.ldap.sdk.LDAPConnection;
+
+public class LdapDAOTest
+{
+    LdapConfig config = new LdapConfig(
+            "mach275.cadc.dao.nrc.ca",
+            389,
+            "uid=webproxy,ou=administrators,ou=topologymanagement,o=netscaperoot",
+            "go4it", "ou=Users,ou=ds,dc=canfar,dc=net",
+            "ou=Groups,ou=ds,dc=canfar,dc=net",
+            "ou=DeletedGroups,ou=ds,dc=canfar,dc=net");
+    
+    @Test
+    public void testLdapBindConnection() throws Exception
+    {
+        //TODO use a test user to test with. To be done when addUser available.
+        //LdapUserDAO<X500Principal> userDAO = new LdapUserDAO<X500Principal>();
+
+        // User authenticated with HttpPrincipal
+        HttpPrincipal httpPrincipal = new HttpPrincipal("cadcauthtest2");
+        Subject subject = new Subject();
+
+        subject.getPrincipals().add(httpPrincipal);
+        
+        final LdapDAOTestImpl ldapDao = new LdapDAOTestImpl(config);
+
+        Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+                try
+                {
+                    LDAPConnection ldapCon = ldapDao.getConnection();
+                    assertTrue(ldapCon.isConnected());
+                    return null;
+                }
+                catch (Exception e)
+                {
+                    throw new Exception("Problems", e);
+                }
+            }
+        });
+               
+        
+        X500Principal subjPrincipal = new X500Principal(
+                "cn=cadc authtest2 10635,ou=cadc,o=hia");
+        subject = new Subject();
+        subject.getPrincipals().add(subjPrincipal);
+        
+        Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+                try
+                {
+                    LDAPConnection ldapCon = ldapDao.getConnection();
+                    assertTrue(ldapCon.isConnected());
+                    return null;
+                }
+                catch (Exception e)
+                {
+                    throw new Exception("Problems", e);
+                }
+            }
+        });
+        
+        
+        NumericPrincipal numPrincipal = new NumericPrincipal(1866);       
+        subject.getPrincipals().add(numPrincipal);
+
+        Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+                try
+                {
+
+                    LDAPConnection ldapCon = ldapDao.getConnection();
+                    assertTrue(ldapCon.isConnected());
+                    return null;
+                }
+                catch (Exception e)
+                {
+                    throw new Exception("Problems", e);
+                }
+            }
+        });
+
+    }
+}
diff --git a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapDAOTestImpl.java b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapDAOTestImpl.java
new file mode 100644
index 00000000..cf3c8d2e
--- /dev/null
+++ b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapDAOTestImpl.java
@@ -0,0 +1,90 @@
+/**
+ ************************************************************************
+ *******************  CANADIAN ASTRONOMY DATA CENTRE  *******************
+ **************  CENTRE CANADIEN DE DONNÉES ASTRONOMIQUES  **************
+ *
+ *  (c) 2014.                            (c) 2014.
+ *  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/>.
+ *
+ ************************************************************************
+ */
+
+
+
+package ca.nrc.cadc.ac.server.ldap;
+
+import java.security.AccessControlException;
+
+import com.unboundid.ldap.sdk.LDAPConnection;
+import com.unboundid.ldap.sdk.LDAPException;
+
+public class LdapDAOTestImpl extends LdapDAO
+{
+    public LdapDAOTestImpl(LdapConfig config)
+    {
+        super(config);
+    }
+    
+    @Override
+    public LDAPConnection getConnection() throws LDAPException,
+    AccessControlException
+    {
+        return super.getConnection();
+    }
+}
diff --git a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java
new file mode 100644
index 00000000..7ac6defc
--- /dev/null
+++ b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java
@@ -0,0 +1,239 @@
+/*
+ ************************************************************************
+ ****  C A N A D I A N   A S T R O N O M Y   D A T A   C E N T R E  *****
+ *
+ * (c) 2014.                            (c) 2014.
+ * National Research Council            Conseil national de recherches
+ * Ottawa, Canada, K1A 0R6              Ottawa, Canada, K1A 0R6
+ * All rights reserved                  Tous droits reserves
+ *
+ * NRC disclaims any warranties         Le CNRC denie toute garantie
+ * expressed, implied, or statu-        enoncee, implicite ou legale,
+ * tory, of any kind with respect       de quelque nature que se soit,
+ * to the software, including           concernant le logiciel, y com-
+ * without limitation any war-          pris sans restriction toute
+ * ranty of merchantability or          garantie de valeur marchande
+ * fitness for a particular pur-        ou de pertinence pour un usage
+ * pose.  NRC shall not be liable       particulier.  Le CNRC ne
+ * in any event for any damages,        pourra en aucun cas etre tenu
+ * whether direct or indirect,          responsable de tout dommage,
+ * special or general, consequen-       direct ou indirect, particul-
+ * tial or incidental, arising          ier ou general, accessoire ou
+ * from the use of the software.        fortuit, resultant de l'utili-
+ *                                      sation du logiciel.
+ *
+ *
+ * @author adriand
+ * 
+ * @version $Revision: $
+ * 
+ * 
+ ****  C A N A D I A N   A S T R O N O M Y   D A T A   C E N T R E  *****
+ ************************************************************************
+ */
+
+package ca.nrc.cadc.ac.server.ldap;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.security.PrivilegedExceptionAction;
+
+import javax.security.auth.Subject;
+import javax.security.auth.x500.X500Principal;
+
+import org.junit.Test;
+
+import ca.nrc.cadc.ac.Group;
+import ca.nrc.cadc.ac.GroupProperty;
+import ca.nrc.cadc.ac.User;
+
+public class LdapGroupDAOTest
+{
+    final String groupID1 = "acs-daotest-group1-" + System.currentTimeMillis();
+    final String groupID2 = "acs-daotest-group2-" + System.currentTimeMillis();
+    
+    LdapConfig config = new LdapConfig(
+            "199.116.235.122",
+//            "mach275.cadc.dao.nrc.ca",
+            389,
+            "uid=webproxy,ou=administrators,ou=topologymanagement,o=netscaperoot",
+            "go4it", "ou=Users,ou=ds,dc=canfar,dc=net",
+            "ou=TestGroups,ou=ds,dc=canfar,dc=net",
+            "ou=DeletedGroups,ou=ds,dc=canfar,dc=net");
+
+    LdapGroupDAO<X500Principal> getGroupDAO()
+    {
+        return new LdapGroupDAO<X500Principal>(config,
+                new LdapUserDAO<X500Principal>(config));
+    }
+
+    @Test
+    public void testOneGroup() throws Exception
+    {
+
+        final User<X500Principal> owner = new User<X500Principal>(
+                new X500Principal("cn=cadc authtest1 10627,ou=cadc,o=hia"));
+        final User<X500Principal> authtest2 = new User<X500Principal>(
+                new X500Principal("CN=cadc authtest2 10635,OU=cadc,O=hia"));
+        final User<X500Principal> regtest1 = new User<X500Principal>(
+                new X500Principal("CN=CADC Regtest1 10577,OU=CADC,O=HIA"));
+        
+        Subject subject = new Subject();
+        subject.getPrincipals().add(owner.getUserID());
+
+        // do everything as owner
+        Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+                try
+                {
+                    Group expectGroup = new Group(groupID1, owner);
+                    Group actualGroup = getGroupDAO().addGroup(expectGroup);
+                    assertGroupsEqual(expectGroup, actualGroup);
+                    
+//                    Group otherGroup = new Group(groupID2, authtest2);
+//                    otherGroup = getGroupDAO().addGroup(otherGroup);
+
+                    // modify group fields
+                    // description
+                    expectGroup.description = "Happy testing";
+                    actualGroup = getGroupDAO().modifyGroup(expectGroup);
+                    assertGroupsEqual(expectGroup, actualGroup);
+
+//                    // groupRead
+//                    expectGroup.groupRead = otherGroup;
+//                    actualGroup = getGroupDAO().modifyGroup(expectGroup);
+//                    assertGroupsEqual(expectGroup, actualGroup);
+//
+//                    // groupWrite
+//                    expectGroup.groupWrite = otherGroup;
+//                    actualGroup = getGroupDAO().modifyGroup(expectGroup);
+//                    assertGroupsEqual(expectGroup, actualGroup);
+
+                    // publicRead
+                    expectGroup.publicRead = true;
+                    actualGroup = getGroupDAO().modifyGroup(expectGroup);
+                    assertGroupsEqual(expectGroup, actualGroup);
+
+//                    // userMembers
+//                    expectGroup.getUserMembers().add(authtest2);
+//                    actualGroup = getGroupDAO().modifyGroup(expectGroup);
+//                    assertGroupsEqual(expectGroup, actualGroup);
+//
+//                    // groupMembers
+//                    expectGroup.getGroupMembers().add(otherGroup);
+//                    actualGroup = getGroupDAO().modifyGroup(expectGroup);
+//                    assertGroupsEqual(expectGroup, actualGroup);
+                    return null;
+                }
+                catch (Exception e)
+                {
+                    throw new Exception("Problems", e);
+                }
+            }
+        });
+    }
+
+//    @Test
+    public void testMultipleGroups() throws Exception
+    {
+
+        final User<X500Principal> owner = new User<X500Principal>(
+                new X500Principal("cn=cadc authtest1 10627,ou=cadc,o=hia"));
+        final User<X500Principal> authtest2 = new User<X500Principal>(
+                new X500Principal("cn=cadc authtest2 10635,ou=cadc,o=hia"));
+
+        Subject subject = new Subject();
+        subject.getPrincipals().add(owner.getUserID());
+
+        // do everything as owner
+        Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+                try
+                {
+                    Group expectGroup = new Group(groupID1, owner);
+                    Group actualGroup = getGroupDAO().addGroup(expectGroup);
+                    assertGroupsEqual(expectGroup, actualGroup);
+                    
+                    Group otherGroup = new Group(groupID2, authtest2);
+                    otherGroup = getGroupDAO().addGroup(otherGroup);
+
+                    // modify group fields
+                    // description
+                    expectGroup.description = "Happy testing";
+                    actualGroup = getGroupDAO().modifyGroup(expectGroup);
+                    assertGroupsEqual(expectGroup, actualGroup);
+
+                    // groupRead
+                    expectGroup.groupRead = otherGroup;
+                    actualGroup = getGroupDAO().modifyGroup(expectGroup);
+                    assertGroupsEqual(expectGroup, actualGroup);
+
+                    // groupWrite
+                    expectGroup.groupWrite = otherGroup;
+                    actualGroup = getGroupDAO().modifyGroup(expectGroup);
+                    assertGroupsEqual(expectGroup, actualGroup);
+
+                    // publicRead
+                    expectGroup.publicRead = true;
+                    actualGroup = getGroupDAO().modifyGroup(expectGroup);
+                    assertGroupsEqual(expectGroup, actualGroup);
+
+                    // userMembers
+                    expectGroup.getUserMembers().add(authtest2);
+                    actualGroup = getGroupDAO().modifyGroup(expectGroup);
+                    assertGroupsEqual(expectGroup, actualGroup);
+
+                    // groupMembers
+                    expectGroup.getGroupMembers().add(otherGroup);
+                    actualGroup = getGroupDAO().modifyGroup(expectGroup);
+                    assertGroupsEqual(expectGroup, actualGroup);
+                    return null;
+                }
+                catch (Exception e)
+                {
+                    throw new Exception("Problems", e);
+                }
+            }
+        });
+    }
+
+    private void assertGroupsEqual(Group gr1, Group gr2)
+    {
+        if ((gr1 == null) && (gr2 == null))
+        {
+            return;
+        }
+        assertEquals(gr1, gr2);
+        assertEquals(gr1.getID(), gr2.getID());
+        assertEquals(gr1.description, gr2.description);
+        assertEquals(gr1.getOwner(), gr2.getOwner());
+        assertEquals(gr1.getGroupMembers(), gr2.getGroupMembers());
+        assertEquals(gr1.getGroupMembers().size(), gr2.getGroupMembers()
+                .size());
+        for (Group gr : gr1.getGroupMembers())
+        {
+            assertTrue(gr2.getGroupMembers().contains(gr));
+        }
+        assertEquals(gr1.getUserMembers(), gr2.getUserMembers());
+        assertEquals(gr1.getUserMembers().size(), gr2.getUserMembers()
+                .size());
+        for (User<?> user : gr1.getUserMembers())
+        {
+            assertTrue(gr2.getUserMembers().contains(user));
+        }
+        assertEquals(gr1.publicRead, gr2.publicRead);
+        assertEquals(gr1.groupRead, gr2.groupRead);
+        assertEquals(gr1.groupWrite, gr2.groupWrite);
+        assertEquals(gr1.groupWrite, gr2.groupWrite);
+        assertEquals(gr1.getProperties(), gr2.getProperties());
+        for (GroupProperty prop : gr1.getProperties())
+        {
+            assertTrue(gr2.getProperties().contains(prop));
+        }
+    }
+}
diff --git a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/GroupActionFactoryTest.java b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/GroupActionFactoryTest.java
new file mode 100644
index 00000000..110306d1
--- /dev/null
+++ b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/GroupActionFactoryTest.java
@@ -0,0 +1,361 @@
+/**
+ ************************************************************************
+ *******************  CANADIAN ASTRONOMY DATA CENTRE  *******************
+ **************  CENTRE CANADIEN DE DONNÉES ASTRONOMIQUES  **************
+ *
+ *  (c) 2014.                            (c) 2014.
+ *  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/>.
+ *
+ ************************************************************************
+ */
+
+package ca.nrc.cadc.ac.server.web;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.apache.log4j.Level;
+import org.apache.log4j.Logger;
+import org.easymock.EasyMock;
+import org.junit.Assert;
+import org.junit.Test;
+
+import ca.nrc.cadc.util.Log4jInit;
+
+public class GroupActionFactoryTest
+{
+    private final static Logger log = Logger.getLogger(GroupActionFactoryTest.class);
+
+    public GroupActionFactoryTest()
+    {
+        Log4jInit.setLevel("ca.nrc.cadc.ac", Level.INFO);
+    }
+
+    @Test
+    public void testCreateAddGroupMemberAction()
+    {
+        try
+        {
+            HttpServletRequest request = EasyMock.createMock(HttpServletRequest.class);
+            EasyMock.expect(request.getPathInfo()).andReturn("groupName/groupMembers/groupToAdd");
+            EasyMock.expect(request.getMethod()).andReturn("PUT");
+            EasyMock.replay(request);
+            GroupsAction action = GroupsActionFactory.getGroupsAction(request, null);
+            EasyMock.verify(request);
+            Assert.assertTrue("Wrong action", action instanceof AddGroupMemberAction);
+        }
+        catch (Throwable t)
+        {
+            log.error(t.getMessage(), t);
+            Assert.fail("unexpected error: " + t.getMessage());
+        }
+    }
+
+    @Test
+    public void testCreateAddUserMemberAction()
+    {
+        try
+        {
+            HttpServletRequest request = EasyMock.createMock(HttpServletRequest.class);
+            EasyMock.expect(request.getPathInfo()).andReturn("groupName/userMembers/userToAdd");
+            EasyMock.expect(request.getParameter("idType")).andReturn("x509");
+            EasyMock.expect(request.getMethod()).andReturn("PUT");
+            EasyMock.replay(request);
+            GroupsAction action = GroupsActionFactory.getGroupsAction(request, null);
+            EasyMock.verify(request);
+            Assert.assertTrue("Wrong action", action instanceof AddUserMemberAction);
+        }
+        catch (Throwable t)
+        {
+            log.error(t.getMessage(), t);
+            Assert.fail("unexpected error: " + t.getMessage());
+        }
+    }
+
+    @Test
+    public void testCreateCreateGroupAction()
+    {
+        try
+        {
+            HttpServletRequest request = EasyMock.createMock(HttpServletRequest.class);
+            EasyMock.expect(request.getPathInfo()).andReturn("");
+            EasyMock.expect(request.getMethod()).andReturn("PUT");
+            EasyMock.expect(request.getInputStream()).andReturn(null);
+            EasyMock.replay(request);
+            GroupsAction action = GroupsActionFactory.getGroupsAction(request, null);
+            EasyMock.verify(request);
+            Assert.assertTrue("Wrong action", action instanceof CreateGroupAction);
+        }
+        catch (Throwable t)
+        {
+            log.error(t.getMessage(), t);
+            Assert.fail("unexpected error: " + t.getMessage());
+        }
+    }
+
+    @Test
+    public void testCreateDeleteGroupAction()
+    {
+        try
+        {
+            HttpServletRequest request = EasyMock.createMock(HttpServletRequest.class);
+            EasyMock.expect(request.getPathInfo()).andReturn("groupName");
+            EasyMock.expect(request.getMethod()).andReturn("DELETE");
+            EasyMock.replay(request);
+            GroupsAction action = GroupsActionFactory.getGroupsAction(request, null);
+            EasyMock.verify(request);
+            Assert.assertTrue("Wrong action", action instanceof DeleteGroupAction);
+        }
+        catch (Throwable t)
+        {
+            log.error(t.getMessage(), t);
+            Assert.fail("unexpected error: " + t.getMessage());
+        }
+    }
+
+    @Test
+    public void testCreateGetGroupAction()
+    {
+        try
+        {
+            HttpServletRequest request = EasyMock.createMock(HttpServletRequest.class);
+            EasyMock.expect(request.getPathInfo()).andReturn("groupName");
+            EasyMock.expect(request.getMethod()).andReturn("GET");
+            EasyMock.replay(request);
+            GroupsAction action = GroupsActionFactory.getGroupsAction(request, null);
+            EasyMock.verify(request);
+            Assert.assertTrue("Wrong action", action instanceof GetGroupAction);
+        }
+        catch (Throwable t)
+        {
+            log.error(t.getMessage(), t);
+            Assert.fail("unexpected error: " + t.getMessage());
+        }
+    }
+
+    @Test
+    public void testCreateListGroupsAction()
+    {
+        try
+        {
+            HttpServletRequest request = EasyMock.createMock(HttpServletRequest.class);
+            EasyMock.expect(request.getPathInfo()).andReturn("");
+            EasyMock.expect(request.getMethod()).andReturn("GET");
+            EasyMock.replay(request);
+            GroupsAction action = GroupsActionFactory.getGroupsAction(request, null);
+            EasyMock.verify(request);
+            Assert.assertTrue("Wrong action", action instanceof ListGroupsAction);
+        }
+        catch (Throwable t)
+        {
+            log.error(t.getMessage(), t);
+            Assert.fail("unexpected error: " + t.getMessage());
+        }
+    }
+
+    @Test
+    public void testCreateModifyGroupAction()
+    {
+        try
+        {
+            HttpServletRequest request = EasyMock.createMock(HttpServletRequest.class);
+            EasyMock.expect(request.getPathInfo()).andReturn("groupName");
+            EasyMock.expect(request.getMethod()).andReturn("POST");
+            EasyMock.expect(request.getInputStream()).andReturn(null);
+            EasyMock.replay(request);
+            GroupsAction action = GroupsActionFactory.getGroupsAction(request, null);
+            EasyMock.verify(request);
+            Assert.assertTrue("Wrong action", action instanceof ModifyGroupAction);
+        }
+        catch (Throwable t)
+        {
+            log.error(t.getMessage(), t);
+            Assert.fail("unexpected error: " + t.getMessage());
+        }
+    }
+
+    @Test
+    public void testCreateRemoveGroupMemberAction()
+    {
+        try
+        {
+            HttpServletRequest request = EasyMock.createMock(HttpServletRequest.class);
+            EasyMock.expect(request.getPathInfo()).andReturn("groupName/groupMembers/groupToRenove");
+            EasyMock.expect(request.getMethod()).andReturn("DELETE");
+            EasyMock.replay(request);
+            GroupsAction action = GroupsActionFactory.getGroupsAction(request, null);
+            EasyMock.verify(request);
+            Assert.assertTrue("Wrong action", action instanceof RemoveGroupMemberAction);
+        }
+        catch (Throwable t)
+        {
+            log.error(t.getMessage(), t);
+            Assert.fail("unexpected error: " + t.getMessage());
+        }
+    }
+
+    @Test
+    public void testCreateRemoveUserMemberAction()
+    {
+        try
+        {
+            HttpServletRequest request = EasyMock.createMock(HttpServletRequest.class);
+            EasyMock.expect(request.getPathInfo()).andReturn("groupName/userMembers/userToRemove");
+            EasyMock.expect(request.getMethod()).andReturn("DELETE");
+            EasyMock.expect(request.getParameter("idType")).andReturn("x509");
+            EasyMock.replay(request);
+            GroupsAction action = GroupsActionFactory.getGroupsAction(request, null);
+            EasyMock.verify(request);
+            Assert.assertTrue("Wrong action", action instanceof RemoveUserMemberAction);
+        }
+        catch (Throwable t)
+        {
+            log.error(t.getMessage(), t);
+            Assert.fail("unexpected error: " + t.getMessage());
+        }
+    }
+
+    @Test
+    public void testBadRequests()
+    {
+        try
+        {
+            TestRequest[] testRequests =
+            {
+                new TestRequest("", "POST"),
+                new TestRequest("", "DELETE"),
+                new TestRequest("", "HEAD"),
+                new TestRequest("groupName/groupMembers", "GET"),
+                new TestRequest("groupName/groupMembers", "POST"),
+                new TestRequest("groupName/groupMembers", "PUT"),
+                new TestRequest("groupName/groupMembers", "DELETE"),
+                new TestRequest("groupName/groupMembers", "HEAD"),
+                new TestRequest("groupName/misspelled/id", "GET"),
+                new TestRequest("groupName/groupMembers/groupMemberName", "GET"),
+                new TestRequest("groupName/groupMembers/groupMemberName", "POST"),
+                new TestRequest("groupName/groupMembers/groupMemberName", "HEAD"),
+                new TestRequest("groupName/userMembers/userMemberName", "GET"),
+                new TestRequest("groupName/userMembers/userMemberName", "POST"),
+                new TestRequest("groupName/userMembers/userMemberName", "HEAD"),
+                new TestRequest("groupName/groupMembers/groupMemberName/tooManySegments", "GET"),
+                new TestRequest("groupName/groupMembers/groupMemberName/tooManySegments", "POST"),
+                new TestRequest("groupName/groupMembers/groupMemberName/tooManySegments", "PUT"),
+                new TestRequest("groupName/groupMembers/groupMemberName/tooManySegments", "HEAD"),
+                new TestRequest("groupName/groupMembers/groupMemberName/tooManySegments", "DELETE"),
+            };
+
+            for (TestRequest testRequest : testRequests)
+            {
+
+                log.debug("Testing: " + testRequest);
+
+                HttpServletRequest request = EasyMock.createMock(HttpServletRequest.class);
+                EasyMock.expect(request.getPathInfo()).andReturn(testRequest.path);
+                EasyMock.expect(request.getMethod()).andReturn(testRequest.method);
+                if (testRequest.paramName != null)
+                {
+                    EasyMock.expect(request.getParameter(testRequest.paramName)).andReturn(testRequest.paramValue);
+                }
+                EasyMock.replay(request);
+                try
+                {
+                    GroupsActionFactory.getGroupsAction(request, null);
+                    Assert.fail("Should have been a bad request: " + testRequest.method + " on " + testRequest.path);
+                }
+                catch (IllegalArgumentException e)
+                {
+                    // expected
+                }
+                EasyMock.verify(request);
+            }
+        }
+        catch (Throwable t)
+        {
+            log.error(t.getMessage(), t);
+            Assert.fail("unexpected error: " + t.getMessage());
+        }
+    }
+
+    private class TestRequest
+    {
+        public String path;
+        public String method;
+        public String paramName;
+        public String paramValue;
+
+        public TestRequest(String path, String method)
+        {
+            this(path, method, null, null);
+        }
+        public TestRequest(String path, String method, String paramName, String paramValue)
+        {
+            this.path = path;
+            this.method = method;
+            this.paramName = paramName;
+            this.paramValue = paramValue;
+        }
+        @Override
+        public String toString()
+        {
+            return method + " on path " + path +
+                ((paramName == null) ? "" : "?" + paramName + "=" + paramValue);
+        }
+
+    }
+
+}
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/AC.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/AC.java
index 677d7b89..782a0529 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/AC.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/AC.java
@@ -68,14 +68,28 @@
  */
 package ca.nrc.cadc.ac;
 
+/**
+ * Holder of commonly used consts in GMS
+ */
 public class AC
 {
+    // Denotes a description given to a group
     public static final String PROPERTY_GROUP_DESCRIPTION = "ivo://ivoa.net/gms#description";
+    
+    // Denotes the DN of a group owner
     public static final String PROPERTY_OWNER_DN = "ivo://ivoa.net/gms#owner_dn";
+    
+    // Denotes the DN of a user
     public static final String PROPERTY_USER_DN = "ivo://ivoa.net/gms#user_dn";
+    
+    // Denotes a group readable by public
     public static final String PROPERTY_PUBLIC = "ivo://ivoa.net/gms#public";
+    
     public static final String GMS_SERVICE_URI = "ivo://cadc.nrc.ca/gms";
+    
+    // Group URI attribute once the group name is appended
     public static final String GROUP_URI = "ivo://cadc.nrc.ca/gms#";
+    
     public static final String ID_TYPE_X500 = "X500";
     public static final String ID_TYPE_OPENID = "OpenID";
     public static final String ID_TYPE_USERNAME = "HTTP";
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupAlreadyExistsException.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupAlreadyExistsException.java
index a73d2340..7e3e0961 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupAlreadyExistsException.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupAlreadyExistsException.java
@@ -70,7 +70,10 @@ package ca.nrc.cadc.ac;
 
 public class GroupAlreadyExistsException extends Exception
 {
-
+    /**
+     * Thrown when there is a group conflict.
+     *
+     */
     public GroupAlreadyExistsException(String message)
     {
         super(message);
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupNotFoundException.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupNotFoundException.java
index 521ae17b..a03e5364 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupNotFoundException.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupNotFoundException.java
@@ -70,7 +70,10 @@ package ca.nrc.cadc.ac;
 
 public class GroupNotFoundException extends Exception
 {
-
+    /**
+     * Thrown when a group cannot be found.
+     *
+     */
     public GroupNotFoundException(String message)
     {
         super(message);
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupReader.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupReader.java
index 95ba98a4..a72a6c58 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupReader.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupReader.java
@@ -89,6 +89,15 @@ import org.jdom2.JDOMException;
 public class GroupReader
 {
 
+    /**
+     * Construct a Group from an XML String source.
+     * 
+     * @param xml String of the XML.
+     * @return Group Group.
+     * @throws ca.nrc.cadc.ac.ReaderException
+     * @throws java.io.IOException
+     * @throws java.net.URISyntaxException
+     */
     public static Group read(String xml)
         throws ReaderException, IOException, URISyntaxException
     {
@@ -99,6 +108,15 @@ public class GroupReader
         return read(new StringReader(xml));
     }
 
+    /**
+     * Construct a Group from a InputStream.
+     * 
+     * @param in InputStream.
+     * @return Group Group.
+     * @throws ca.nrc.cadc.ac.ReaderException
+     * @throws java.io.IOException
+     * @throws java.net.URISyntaxException
+     */
     public static Group read(InputStream in)
         throws ReaderException, IOException, URISyntaxException
     {
@@ -118,6 +136,15 @@ public class GroupReader
         return read(reader);
     }
 
+    /**
+     * Construct a Group from a Reader.
+     * 
+     * @param reader Reader.
+     * @return Group Group.
+     * @throws ca.nrc.cadc.ac.ReaderException
+     * @throws java.io.IOException
+     * @throws java.net.URISyntaxException
+     */
     public static Group read(Reader reader)
         throws ReaderException, IOException, URISyntaxException
     {
@@ -160,14 +187,16 @@ public class GroupReader
             throw new ReaderException(error);
         }
 
-        int index = uri.indexOf("ivo://cadc.nrc.ca/gms#");
+        // Group groupID
+        int index = uri.indexOf(AC.GROUP_URI);
         if (index == -1)
         {
             String error = "group uri attribute malformed: " + uri;
             throw new ReaderException(error);
         }
-        String groupID = uri.substring("ivo://cadc.nrc.ca/gms#".length());
+        String groupID = uri.substring(AC.GROUP_URI.length());
 
+        // Group owner
         Element ownerElement = groupElement.getChild("owner");
         if (ownerElement == null)
         {
@@ -175,6 +204,7 @@ public class GroupReader
             throw new ReaderException(error);
         }
 
+        // Owner user
         Element userElement = ownerElement.getChild("user");
         if (userElement == null)
         {
@@ -185,18 +215,20 @@ public class GroupReader
 
         Group group = new Group(groupID, user);
 
+        // description
         Element descriptionElement = groupElement.getChild("description");
         if (descriptionElement != null)
         {
             group.description = descriptionElement.getText();
         }
 
+        // lastModified
         Element lastModifiedElement = groupElement.getChild("lastModified");
         if (lastModifiedElement != null)
         {
             try
             {
-                DateFormat df = DateUtil.getDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS", DateUtil.UTC);
+                DateFormat df = DateUtil.getDateFormat(DateUtil.IVOA_DATE_FORMAT, DateUtil.UTC);
                 group.lastModified = df.parse(lastModifiedElement.getText());
             }
             catch (ParseException e)
@@ -208,12 +240,14 @@ public class GroupReader
 
         }
 
+        // publicRead
         Element publicReadElement = groupElement.getChild("publicRead");
         if (publicReadElement != null)
         {
             group.publicRead = Boolean.valueOf(publicReadElement.getText());
         }
 
+        // properties
         Element propertiesElement = groupElement.getChild("properties");
         if (propertiesElement != null)
         {
@@ -225,6 +259,7 @@ public class GroupReader
 
         }
 
+        // groupMembers
         Element groupMembersElement = groupElement.getChild("groupMembers");
         if (groupMembersElement != null)
         {
@@ -236,6 +271,7 @@ public class GroupReader
 
         }
 
+        // groupRead
         Element groupReadElement = groupElement.getChild("groupRead");
         if (groupReadElement != null)
         {
@@ -247,6 +283,7 @@ public class GroupReader
 
         }
 
+        // groupWrite
         Element groupWriteElement = groupElement.getChild("groupWrite");
         if (groupWriteElement != null)
         {
@@ -258,6 +295,7 @@ public class GroupReader
 
         }
 
+        // userMembers
         Element userMembersElement = groupElement.getChild("userMembers");
         if (userMembersElement != null)
         {
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupWriter.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupWriter.java
index bf4eee6d..17c8a87b 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupWriter.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupWriter.java
@@ -87,12 +87,27 @@ import org.jdom2.output.XMLOutputter;
 
 public class GroupWriter
 {
+    /**
+     * Write a Group to a StringBuilder.
+     * @param group
+     * @param builder
+     * @throws java.io.IOException
+     * @throws ca.nrc.cadc.ac.WriterException
+     */
     public static void write(Group group, StringBuilder builder)
         throws IOException, WriterException
     {
         write(group, new StringBuilderWriter(builder));
     }
 
+    /**
+     * Write a Group to an OutputStream.
+     * 
+     * @param group Group to write.
+     * @param out OutputStream to write to.
+     * @throws IOException if the writer fails to write.
+     * @throws ca.nrc.cadc.ac.WriterException
+     */
     public static void write(Group group, OutputStream out)
         throws IOException, WriterException
     {
@@ -108,6 +123,14 @@ public class GroupWriter
         write(group, new BufferedWriter(outWriter));
     }
 
+    /**
+     * Write a Group to a Writer.
+     * 
+     * @param group Group to write.
+     * @param writer  Writer to write to.
+     * @throws IOException if the writer fails to write.
+     * @throws ca.nrc.cadc.ac.WriterException
+     */
     public static void write(Group group, Writer writer)
         throws IOException, WriterException
     {
@@ -119,6 +142,12 @@ public class GroupWriter
         write(getGroupElement(group), writer);
     }
 
+    /**
+     * 
+     * @param group
+     * @return 
+     * @throws ca.nrc.cadc.ac.WriterException 
+     */
     public static Element getGroupElement(Group group)
         throws WriterException
     {
@@ -128,10 +157,12 @@ public class GroupWriter
     public static Element getGroupElement(Group group, boolean deepCopy)
         throws WriterException
     {
+        // Create the root group element.
         Element groupElement = new Element("group");
-        String groupURI = "ivo://cadc.nrc.ca/gms#" + group.getID();
+        String groupURI = AC.GROUP_URI + group.getID();
         groupElement.setAttribute(new Attribute("uri", groupURI));
 
+        // Group owner
         Element ownerElement = new Element("owner");
         Element userElement = UserWriter.getUserElement(group.getOwner());
         ownerElement.addContent(userElement);
@@ -139,6 +170,7 @@ public class GroupWriter
 
         if (deepCopy)
         {
+            // Group description
             if (group.description != null)
             {
                 Element descriptionElement = new Element("description");
@@ -146,18 +178,21 @@ public class GroupWriter
                 groupElement.addContent(descriptionElement);
             }
 
+            // Group publicRead
             Element publicReadElement = new Element("publicRead");
             publicReadElement.setText(String.valueOf(group.publicRead));
             groupElement.addContent(publicReadElement);
 
+            // lastModified
             if (group.lastModified != null)
             {
                 Element lastModifiedElement = new Element("lastModified");
-                DateFormat df = DateUtil.getDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS", DateUtil.UTC);
+                DateFormat df = DateUtil.getDateFormat(DateUtil.IVOA_DATE_FORMAT, DateUtil.UTC);
                 lastModifiedElement.setText(df.format(group.lastModified));
                 groupElement.addContent(lastModifiedElement);
             }
 
+            // Group properties
             if (!group.getProperties().isEmpty())
             {
                 Element propertiesElement = new Element("properties");
@@ -168,6 +203,7 @@ public class GroupWriter
                 groupElement.addContent(propertiesElement);
             }
 
+            // Group groupMembers.
             if ((group.getGroupMembers() != null) && (!group.getGroupMembers().isEmpty()))
             {
                 Element groupMembersElement = new Element("groupMembers");
@@ -178,6 +214,7 @@ public class GroupWriter
                 groupElement.addContent(groupMembersElement);
             }
 
+            // Group groupRead.
             if (group.groupRead != null)
             {
                 Element groupReadElement = new Element("groupRead");
@@ -185,6 +222,7 @@ public class GroupWriter
                 groupElement.addContent(groupReadElement);
             }
 
+            // Group groupWrite.
             if (group.groupWrite != null)
             {
                 Element groupWriteElement = new Element("groupWrite");
@@ -192,6 +230,7 @@ public class GroupWriter
                 groupElement.addContent(groupWriteElement);
             }
 
+            // Group userMembers
             if ((group.getUserMembers() != null) && (!group.getUserMembers().isEmpty()))
             {
                 Element userMembersElement = new Element("userMembers");
@@ -206,6 +245,13 @@ public class GroupWriter
         return groupElement;
     }
 
+    /**
+     * Write to root Element to a writer.
+     * 
+     * @param root Root Element to write.
+     * @param writer Writer to write to.
+     * @throws IOException if the writer fails to write.
+     */
     private static void write(Element root, Writer writer)
         throws IOException
     {
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupsReader.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupsReader.java
index 923d2b53..656bdecc 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupsReader.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupsReader.java
@@ -84,9 +84,17 @@ import org.jdom2.JDOMException;
 
 public class GroupsReader
 {
-
+    /**
+     * Construct a list of Group's from an XML String source.
+     * 
+     * @param xml String of the XML.
+     * @return Groups List of Group.
+     * @throws ca.nrc.cadc.ac.ReaderException
+     * @throws java.io.IOException
+     * @throws java.net.URISyntaxException
+     */
     public static List<Group> read(String xml)
-            throws ReaderException, IOException, URISyntaxException
+        throws ReaderException, IOException, URISyntaxException
     {
         if (xml == null)
         {
@@ -95,8 +103,17 @@ public class GroupsReader
         return read(new StringReader(xml));
     }
 
+    /**
+     * Construct a list of Group's from a InputStream.
+     * 
+     * @param in InputStream.
+     * @return Groups List of Group.
+     * @throws ca.nrc.cadc.ac.ReaderException
+     * @throws java.io.IOException
+     * @throws java.net.URISyntaxException
+     */
     public static List<Group> read(InputStream in)
-            throws ReaderException, IOException, URISyntaxException
+        throws ReaderException, IOException, URISyntaxException
     {
         if (in == null)
         {
@@ -114,8 +131,17 @@ public class GroupsReader
         return read(reader);
     }
 
+    /**
+     * Construct a List of Group's from a Reader.
+     * 
+     * @param reader Reader.
+     * @return Groups List of Group.
+     * @throws ca.nrc.cadc.ac.ReaderException
+     * @throws java.io.IOException
+     * @throws java.net.URISyntaxException
+     */
     public static List<Group> read(Reader reader)
-            throws ReaderException, IOException, URISyntaxException
+        throws ReaderException, IOException, URISyntaxException
     {
         if (reader == null)
         {
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupsWriter.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupsWriter.java
index 44a0d093..f98b38e1 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupsWriter.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/GroupsWriter.java
@@ -15,56 +15,93 @@ import org.jdom2.output.XMLOutputter;
 
 public class GroupsWriter
 {
-  public static void write(List<Group> groups, StringBuilder builder)
-    throws IOException, WriterException
-  {
-    write(groups, new StringBuilderWriter(builder));
-  }
-
-  public static void write(List<Group> groups, OutputStream out)
-    throws IOException, WriterException
-  {
-    OutputStreamWriter outWriter;
-    try
+    /**
+     * Write a List of Group's to a StringBuilder.
+     * @param groups List of Group's to write.
+     * @param builder
+     * @throws java.io.IOException
+     * @throws ca.nrc.cadc.ac.WriterException
+     */
+    public static void write(List<Group> groups, StringBuilder builder)
+        throws IOException, WriterException
     {
-      outWriter = new OutputStreamWriter(out, "UTF-8");
+        write(groups, new StringBuilderWriter(builder));
     }
-    catch (UnsupportedEncodingException e)
+
+    /**
+     * Write a List of Group's to an OutputStream.
+     * 
+     * @param groups List of Group's to write.
+     * @param out OutputStream to write to.
+     * @throws IOException if the writer fails to write.
+     * @throws ca.nrc.cadc.ac.WriterException
+     */
+    public static void write(List<Group> groups, OutputStream out)
+        throws IOException, WriterException
     {
-      throw new RuntimeException("UTF-8 encoding not supported", e);
+        OutputStreamWriter outWriter;
+        try
+        {
+        outWriter = new OutputStreamWriter(out, "UTF-8");
+        }
+        catch (UnsupportedEncodingException e)
+        {
+        throw new RuntimeException("UTF-8 encoding not supported", e);
+        }
+        write(groups, new BufferedWriter(outWriter));
     }
-    write(groups, new BufferedWriter(outWriter));
-  }
 
-  public static void write(List<Group> groups, Writer writer)
-    throws IOException, WriterException
-  {
-    if (groups == null)
+    /**
+     * Write a List of Group's to a Writer.
+     * 
+     * @param groups List of Group's to write.
+     * @param writer  Writer to write to.
+     * @throws IOException if the writer fails to write.
+     * @throws ca.nrc.cadc.ac.WriterException
+     */
+    public static void write(List<Group> groups, Writer writer)
+        throws IOException, WriterException
     {
-      throw new WriterException("null groups");
+        if (groups == null)
+        {
+        throw new WriterException("null groups");
+        }
+
+        write(getGroupsElement(groups), writer);
     }
 
-    write(getGroupsElement(groups), writer);
-  }
+    /**
+     * 
+     * @param groups List of Group's to write.
+     * @return Element of list of Group's.
+     * @throws ca.nrc.cadc.ac.WriterException 
+     */
+    public static Element getGroupsElement(List<Group> groups)
+        throws WriterException
+    {
+        Element groupsElement = new Element("groups");
+
+        for (Group group : groups)
+        {
+            groupsElement.addContent(GroupWriter.getGroupElement(group));
+        }
 
-  public static Element getGroupsElement(List<Group> groups)
-    throws WriterException
-  {
-    Element groupsElement = new Element("groups");
+        return groupsElement;
+    }
 
-    for (Group group : groups)
+    /**
+     * Write to root Element to a writer.
+     * 
+     * @param root Root Element to write.
+     * @param writer Writer to write to.
+     * @throws IOException if the writer fails to write.
+     */
+    private static void write(Element root, Writer writer)
+        throws IOException
     {
-      groupsElement.addContent(GroupWriter.getGroupElement(group));
+        XMLOutputter outputter = new XMLOutputter();
+        outputter.setFormat(Format.getPrettyFormat());
+        outputter.output(new Document(root), writer);
     }
-
-    return groupsElement;
-  }
-
-  private static void write(Element root, Writer writer)
-    throws IOException
-  {
-    XMLOutputter outputter = new XMLOutputter();
-    outputter.setFormat(Format.getPrettyFormat());
-    outputter.output(new Document(root), writer);
-  }
+    
 }
\ No newline at end of file
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/MemberAlreadyExistsException.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/MemberAlreadyExistsException.java
index bacc1393..e1b6d383 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/MemberAlreadyExistsException.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/MemberAlreadyExistsException.java
@@ -68,6 +68,10 @@
  */
 package ca.nrc.cadc.ac;
 
+/**
+ * Thrown when there is a member conflict.
+ *
+ */
 public class MemberAlreadyExistsException extends Exception
 {
 }
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/MemberNotFoundException.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/MemberNotFoundException.java
index 3ff660a7..64e99998 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/MemberNotFoundException.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/MemberNotFoundException.java
@@ -68,6 +68,10 @@
  */
 package ca.nrc.cadc.ac;
 
+/**
+ * Thrown when a member could not be found.
+ *
+ */
 public class MemberNotFoundException extends Exception
 {
 }
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/PersonalDetails.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/PersonalDetails.java
index 2ff3a7e2..60d9a813 100644
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/PersonalDetails.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/PersonalDetails.java
@@ -144,7 +144,7 @@ public class PersonalDetails implements UserDetails
     }
 
     /* (non-Javadoc)
-     * 
+     * @see ca.nrc.cadc.auth.model.UserDetails#hashCode()
      */
     @Override
     public int hashCode()
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/PosixDetails.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/PosixDetails.java
index 838ad9d2..af282d6d 100644
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/PosixDetails.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/PosixDetails.java
@@ -104,12 +104,9 @@ public class PosixDetails implements UserDetails
 
     /**
      * 
-     * @param uid
-     *            posix uid
-     * @param gid
-     *            posix gid
-     * @param homeDirectory
-     *            home directory
+     * @param uid posix uid
+     * @param gid posix gid
+     * @param homeDirectory home directory
      */
     public PosixDetails(long uid, long gid, String homeDirectory)
     {
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/ReaderException.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/ReaderException.java
index dae488c9..397d1e62 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/ReaderException.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/ReaderException.java
@@ -70,13 +70,38 @@ package ca.nrc.cadc.ac;
 
 import java.io.IOException;
 
+/**
+ * Class for all Exceptions that occur during reading.
+ */
 public class ReaderException extends IOException
 {
+    /**
+     * Constructs a new exception with the specified detail message.  The
+     * cause is not initialized, and may subsequently be initialized by
+     * a call to {@link #initCause}.
+     *
+     * @param message the detail message. The detail message is saved for
+     *                later retrieval by the {@link #getMessage()} method.
+     */
     public ReaderException(String message)
     {
         super(message);
     }
 
+    /**
+     * Constructs a new exception with the specified detail message and
+     * cause.  <p>Note that the detail message associated with
+     * <code>cause</code> is <i>not</i> automatically incorporated in
+     * this exception's detail message.
+     *
+     * @param message the detail message (which is saved for later retrieval
+     *                by the {@link #getMessage()} method).
+     * @param cause   the cause (which is saved for later retrieval by the
+     *                {@link #getCause()} method).  (A <tt>null</tt> value is
+     *                permitted, and indicates that the cause is nonexistent or
+     *                unknown.)
+     * @since 1.4
+     */
     public ReaderException(String message, Throwable cause)
     {
         super(message, cause);
diff --git a/projects/cadcAccessControl/test/src/ca/nrc/cadc/ac/ReaderWriterTest.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/Role.java
similarity index 87%
rename from projects/cadcAccessControl/test/src/ca/nrc/cadc/ac/ReaderWriterTest.java
rename to projects/cadcAccessControl/src/ca/nrc/cadc/ac/Role.java
index 68c325e8..ef9e04b4 100644
--- a/projects/cadcAccessControl/test/src/ca/nrc/cadc/ac/ReaderWriterTest.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/Role.java
@@ -68,47 +68,45 @@
  */
 package ca.nrc.cadc.ac;
 
-import org.junit.After;
-import org.junit.AfterClass;
-import org.junit.Before;
-import org.junit.BeforeClass;
-import org.junit.Test;
-import static org.junit.Assert.*;
-
 /**
  *
  * @author jburke
  */
-public class ReaderWriterTest
+public enum Role
 {
+    OWNER("owner"),
+    MEMBER("member"),
+    RW("rw");
     
-    public ReaderWriterTest()
+    private final String value;
+
+    private Role(String value)
     {
+        this.value = value;
     }
-    
-    @BeforeClass
-    public static void setUpClass()
+
+    public static Role toValue(String s)
     {
+        for (Role role : values())
+            if (role.value.equals(s))
+                return role;
+        throw new IllegalArgumentException("invalid value: " + s);
     }
-    
-    @AfterClass
-    public static void tearDownClass()
-    {
+
+    public String getValue()
+    { 
+        return value;
     }
-    
-    @Before
-    public void setUp()
+
+    public int checksum()
     {
+        return value.hashCode();
     }
     
-    @After
-    public void tearDown()
+    @Override
+    public String toString()
     {
+        return this.getClass().getSimpleName() + "[" + value + "]";
     }
-
-    // TODO add test methods here.
-    // The methods must be annotated with annotation @Test. For example:
-    //
-    // @Test
-    // public void hello() {}
+    
 }
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/User.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/User.java
index b14c4251..a6e74d85 100644
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/User.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/User.java
@@ -80,7 +80,7 @@ public class User<T extends Principal>
 
     public Set<UserDetails> details = new HashSet<UserDetails>();
 
-    public User(T userID)
+    public User(final T userID)
     {
         if (userID == null)
         {
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/UserNotFoundException.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/UserNotFoundException.java
index 46cf3715..9b9123c7 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/UserNotFoundException.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/UserNotFoundException.java
@@ -68,6 +68,10 @@
  */
 package ca.nrc.cadc.ac;
 
+/**
+ * Thrown when a user could not be found.
+ *
+ */
 public class UserNotFoundException extends Exception
 {
     public UserNotFoundException(String message)
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/UserReader.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/UserReader.java
index 2f6ada0b..a28883a3 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/UserReader.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/UserReader.java
@@ -84,6 +84,15 @@ import org.jdom2.JDOMException;
 
 public class UserReader
 {
+    /**
+     * Construct a User from an XML String source.
+     * 
+     * @param xml String of the XML.
+     * @return User User.
+     * @throws ca.nrc.cadc.ac.ReaderException
+     * @throws java.io.IOException
+     * @throws java.net.URISyntaxException
+     */
     public static User<? extends Principal> read(String xml)
         throws ReaderException, IOException, URISyntaxException
     {
@@ -94,6 +103,15 @@ public class UserReader
         return read(new StringReader(xml));
     }
 
+    /**
+     * Construct a User from a InputStream.
+     * 
+     * @param in InputStream.
+     * @return User User.
+     * @throws ca.nrc.cadc.ac.ReaderException
+     * @throws java.io.IOException
+     * @throws java.net.URISyntaxException
+     */
     public static User<? extends Principal> read(InputStream in)
         throws ReaderException, IOException, URISyntaxException
     {
@@ -113,6 +131,14 @@ public class UserReader
         return read(reader);
     }
 
+    /**
+     * Construct a User from a Reader.
+     * 
+     * @param reader Reader.
+     * @return User User.
+     * @throws ca.nrc.cadc.ac.ReaderException
+     * @throws java.io.IOException
+     */
     public static User<? extends Principal> read(Reader reader)
         throws ReaderException, IOException
     {
@@ -121,6 +147,7 @@ public class UserReader
             throw new IllegalArgumentException("reader must not be null");
         }
 
+        // Create a JDOM Document from the XML
         Document document;
         try
         {
@@ -132,6 +159,7 @@ public class UserReader
             throw new ReaderException(error, jde);
         }
 
+        // Root element and namespace of the Document
         Element root = document.getRootElement();
 
         return parseUser(root);
@@ -140,6 +168,7 @@ public class UserReader
     protected static User<? extends Principal> parseUser(Element userElement)
         throws ReaderException
     {
+        // userID element of the User element
         Element userIDElement = userElement.getChild("userID");
         if (userIDElement == null)
         {
@@ -147,6 +176,7 @@ public class UserReader
             throw new ReaderException(error);
         }
 
+        // identity element of the userID element
         Element userIDIdentityElement = userIDElement.getChild("identity");
         if (userIDIdentityElement == null)
         {
@@ -158,6 +188,7 @@ public class UserReader
 
         User<Principal> user = new User<Principal>(userID);
 
+        // identities
         Element identitiesElement = userElement.getChild("identities");
         if (identitiesElement != null)
         {
@@ -169,6 +200,7 @@ public class UserReader
 
         }
 
+        // details
         Element detailsElement = userElement.getChild("details");
         if (detailsElement != null)
         {
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/UserWriter.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/UserWriter.java
index 4fae569d..832e41a6 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/UserWriter.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/UserWriter.java
@@ -84,12 +84,28 @@ import org.jdom2.output.XMLOutputter;
 
 public class UserWriter
 {
+    /**
+     * Write a User to a StringBuilder.
+     * 
+     * @param user User to write.
+     * @param builder StringBuilder to write to.
+     * @throws java.io.IOException if the writer fails to write.
+     * @throws ca.nrc.cadc.ac.WriterException
+     */
     public static void write(User<? extends Principal> user, StringBuilder builder)
         throws IOException, WriterException
     {
         write(user, new StringBuilderWriter(builder));
     }
 
+    /**
+     * Write a User to an OutputStream.
+     *
+     * @param user User to write.
+     * @param out OutputStream to write to.
+     * @throws IOException if the writer fails to write.
+     * @throws ca.nrc.cadc.ac.WriterException
+     */
     public static void write(User<? extends Principal> user, OutputStream out)
         throws IOException, WriterException
     {                
@@ -105,6 +121,14 @@ public class UserWriter
         write(user, new BufferedWriter(outWriter));
     }
 
+    /**
+     * Write a User to a Writer.
+     *
+     * @param user User to write.
+     * @param writer Writer to write to.
+     * @throws IOException if the writer fails to write.
+     * @throws ca.nrc.cadc.ac.WriterException
+     */
     public static void write(User<? extends Principal> user, Writer writer)
         throws IOException, WriterException
     {
@@ -116,15 +140,25 @@ public class UserWriter
         write(getUserElement(user), writer);
     }
 
+    /**
+     * Build the member Element of a User.
+     *
+     * @param user User.
+     * @return member Element.
+     * @throws ca.nrc.cadc.ac.WriterException
+     */
     public static Element getUserElement(User<? extends Principal> user)
         throws WriterException
     {
+        // Create the user Element.
         Element userElement = new Element("user");
 
+        // userID element
         Element userIDElement = new Element("userID");
         userIDElement.addContent(IdentityWriter.write(user.getUserID()));
         userElement.addContent(userIDElement);
 
+        // identities
         Set<Principal> identities = user.getIdentities();
         if (!identities.isEmpty())
         {
@@ -136,6 +170,7 @@ public class UserWriter
             userElement.addContent(identitiesElement);
         }
 
+        // details
         if (!user.details.isEmpty())
         {
             Element detailsElement = new Element("details");
@@ -150,6 +185,13 @@ public class UserWriter
         return userElement;
     }
 
+    /**
+     * Write to root Element to a writer.
+     *
+     * @param root Root Element to write.
+     * @param writer Writer to write to.
+     * @throws IOException if the writer fails to write.
+     */
     private static void write(Element root, Writer writer)
         throws IOException
     {
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/WriterException.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/WriterException.java
index 23a7ca89..3bf5a2b8 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/WriterException.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/WriterException.java
@@ -70,13 +70,38 @@ package ca.nrc.cadc.ac;
 
 import java.io.IOException;
 
+/**
+ * Base exception for all Writer class exceptions.
+ */
 public class WriterException extends IOException
 {
+    /**
+     * Constructs a new exception with the specified detail message.  The
+     * cause is not initialized, and may subsequently be initialized by
+     * a call to {@link #initCause}.
+     *
+     * @param message the detail message. The detail message is saved for
+     *                later retrieval by the {@link #getMessage()} method.
+     */
     public WriterException(String message)
     {
         super(message);
     }
 
+    /**
+     * Constructs a new exception with the specified detail message and
+     * cause.  <p>Note that the detail message associated with
+     * <code>cause</code> is <i>not</i> automatically incorporated in
+     * this exception's detail message.
+     *
+     * @param message the detail message (which is saved for later retrieval
+     *                by the {@link #getMessage()} method).
+     * @param cause   the cause (which is saved for later retrieval by the
+     *                {@link #getCause()} method).  (A <tt>null</tt> value is
+     *                permitted, and indicates that the cause is nonexistent or
+     *                unknown.)
+     * @since 1.4
+     */
     public WriterException(String message, Throwable cause)
     {
         super(message, cause);
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/client/GMSClient.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/client/GMSClient.java
index 31072689..0ef0393d 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/client/GMSClient.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/client/GMSClient.java
@@ -101,12 +101,22 @@ import javax.net.ssl.SSLSocketFactory;
 import javax.security.auth.Subject;
 import org.apache.log4j.Logger;
 
+/**
+ * Client class for communicating with the access control web service.
+ */
 public class GMSClient
 {
     private static final Logger log = Logger.getLogger(GMSClient.class);
+    
+    // socket factory to use when connecting
     public SSLSocketFactory sslSocketFactory;
+    
     private String baseURL;
 
+    /**
+     *
+     * @param baseURL
+     */
     public GMSClient(String baseURL)
         throws IllegalArgumentException
     {
@@ -137,18 +147,29 @@ public class GMSClient
         }
     }
 
-    public void setSSLSocketFactory(SSLSocketFactory sslSocketFactory)
-    {
-        this.sslSocketFactory = sslSocketFactory;
-    }
-
+    /**
+     * Get a list of groups.
+     *
+     * @return The list of groups.
+     */
     public List<Group> getGroups()
     {
         throw new UnsupportedOperationException("Not yet implemented");
     }
 
+    /**
+     * Create a new group
+     *
+     * @param group The group to create
+     * @return The newly created group will all the information.
+     * @throws GroupAlreadyExistsException If a group with the same name already exists.
+     * @throws AccessControlException If unauthorized to perform this operation.
+     * @throws UserNotFoundException
+     * @throws IOException
+     */
     public Group createGroup(Group group)
-        throws GroupAlreadyExistsException, AccessControlException, UserNotFoundException, IOException
+        throws GroupAlreadyExistsException, AccessControlException, 
+               UserNotFoundException, IOException
     {
         URL createGroupURL = new URL(this.baseURL + "/groups");
         log.debug("createGroupURL request to " + createGroupURL.toString());
@@ -201,6 +222,15 @@ public class GMSClient
         }
     }
 
+    /**
+     * Get the group object.
+     *
+     * @param groupName Identifies the group to get.
+     * @return The group.
+     * @throws GroupNotFoundException If the group was not found.
+     * @throws AccessControlException If unauthorized to perform this operation.
+     * @throws java.io.IOException
+     */
     public Group getGroup(String groupName)
         throws GroupNotFoundException, AccessControlException, IOException
     {
@@ -244,8 +274,19 @@ public class GMSClient
         }
     }
 
+    /**
+     * Update a group.
+     *
+     * @param group The update group object.
+     * @return The group after update.
+     * @throws IllegalArgumentException If cyclical membership is detected.
+     * @throws GroupNotFoundException If the group was not found.
+     * @throws AccessControlException If unauthorized to perform this operation.
+     * @throws java.io.IOException
+     */
     public Group updateGroup(Group group)
-        throws IllegalArgumentException, GroupNotFoundException, AccessControlException, IOException
+        throws IllegalArgumentException, GroupNotFoundException,
+               AccessControlException, IOException
     {
         URL updateGroupURL = new URL(this.baseURL + "/groups/" + group.getID());
         log.debug("updateGroup request to " + updateGroupURL.toString());
@@ -291,6 +332,14 @@ public class GMSClient
         }
     }
 
+    /**
+     * Delete the group.
+     *
+     * @param groupName Identifies the group to delete.
+     * @throws GroupNotFoundException If the group was not found.
+     * @throws AccessControlException If unauthorized to perform this operation.
+     * @throws java.io.IOException
+     */
     public void deleteGroup(String groupName)
         throws GroupNotFoundException, AccessControlException, IOException
     {
@@ -326,8 +375,19 @@ public class GMSClient
         }
     }
 
+    /**
+     * Add a group as a member of another group.
+     *
+     * @param targetGroupName The group in which to add the group member.
+     * @param groupMemberName The group member to add.
+     * @throws IllegalArgumentException If cyclical membership is detected.
+     * @throws GroupNotFoundException If the group was not found.
+     * @throws AccessControlException If unauthorized to perform this operation.
+     * @throws java.io.IOException
+     */
     public void addGroupMember(String targetGroupName, String groupMemberName)
-        throws IllegalArgumentException, GroupNotFoundException, AccessControlException, IOException
+        throws IllegalArgumentException, GroupNotFoundException,
+               AccessControlException, IOException
     {
         URL addGroupMemberURL = new URL(this.baseURL + "/groups/" + targetGroupName + "/groupMembers/" + groupMemberName);
         log.debug("addGroupMember request to " + addGroupMemberURL.toString());
@@ -362,6 +422,15 @@ public class GMSClient
         }
     }
 
+    /**
+     * Add a user as a member of a group.
+     *
+     * @param targetGroupName The group in which to add the group member.
+     * @param userID The user to add.
+     * @throws GroupNotFoundException If the group was not found.
+     * @throws java.io.IOException
+     * @throws AccessControlException If unauthorized to perform this operation.
+     */
     public void addUserMember(String targetGroupName, Principal userID)
         throws GroupNotFoundException, AccessControlException, IOException
     {
@@ -401,6 +470,15 @@ public class GMSClient
         }
     }
 
+    /**
+     * Remove a group as a member of another group.
+     *
+     * @param targetGroupName 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, String groupMemberName)
         throws GroupNotFoundException, AccessControlException, IOException
     {
@@ -437,6 +515,15 @@ public class GMSClient
         }
     }
 
+    /**
+     * Remove a user as a member of a group.
+     *
+     * @param targetGroupName The group from which to remove the group member.
+     * @param userID The user to remove.
+     * @throws GroupNotFoundException If the group was not found.
+     * @throws java.io.IOException
+     * @throws AccessControlException If unauthorized to perform this operation.
+     */
     public void removeUserMember(String targetGroupName, Principal userID)
         throws GroupNotFoundException, AccessControlException, IOException
     {
@@ -481,6 +568,17 @@ public class GMSClient
         throw new UnsupportedOperationException();
     }
 
+    /**
+     * @param sslSocketFactory the sslSocketFactory to set
+     */
+    public void setSSLSocketFactory(SSLSocketFactory sslSocketFactory)
+    {
+        this.sslSocketFactory = sslSocketFactory;
+    }
+    
+    /**
+     * @return the sslSocketFactory
+     */
     private SSLSocketFactory getSSLSocketFactory()
     {
         if (this.sslSocketFactory == null)
@@ -521,6 +619,10 @@ public class GMSClient
         }
     }
 
+    /**
+     * Class used to hold list of groups in which
+     * a user is a member.
+     */
     protected class GroupCredentials
     {
         Collection<Group> groupMemberships = new ArrayList<Group>();
-- 
GitLab