From 959f8ac085d5b0d8bf6adffbdc3d141d20e912d9 Mon Sep 17 00:00:00 2001
From: Dustin Jenkins <Dustin.Jenkins@nrc-cnrc.gc.ca>
Date: Wed, 5 Aug 2015 10:21:53 -0700
Subject: [PATCH] Story 1658: Fix for tests and update users.

---
 .../nrc/cadc/ac/server/UserPersistence.java   |  19 ++-
 .../nrc/cadc/ac/server/ldap/LdapGroupDAO.java |  78 ++++-----
 .../nrc/cadc/ac/server/ldap/LdapUserDAO.java  |  23 ++-
 .../ac/server/ldap/LdapUserPersistence.java   |  49 ++++--
 .../ac/server/web/users/CreateUserAction.java |  25 ++-
 .../ac/server/web/users/GetUserAction.java    |  16 +-
 .../ac/server/web/users/ModifyUserAction.java |  55 +++++--
 .../cadc/ac/server/web/users/UsersAction.java |  47 ++++++
 .../server/web/users/UsersActionFactory.java  |  28 +---
 .../cadc/ac/server/ldap/LdapDAOTestImpl.java  |   3 +-
 .../cadc/ac/server/ldap/LdapGroupDAOTest.java |   2 +-
 .../cadc/ac/server/ldap/LdapUserDAOTest.java  |  95 ++++++++---
 .../web/users/ModifyUserActionTest.java       | 152 ++++++++++++++++++
 .../src/ca/nrc/cadc/ac/User.java              |  36 ++++-
 14 files changed, 487 insertions(+), 141 deletions(-)
 create mode 100644 projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/users/ModifyUserActionTest.java

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 1b369f70..720d6d68 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
@@ -94,7 +94,7 @@ public interface UserPersistence<T extends Principal>
     /**
      * Add the new user.
      *
-     * @param user
+     * @param user      The user request to put into the request tree.
      *
      * @return User instance.
      * 
@@ -119,6 +119,21 @@ public interface UserPersistence<T extends Principal>
     User<T> getUser(T userID)
         throws UserNotFoundException, TransientException, 
                AccessControlException;
+
+    /**
+     * Get the user specified by userID whose account is pending approval.
+     *
+     * @param userID The userID.
+     *
+     * @return User instance.
+     *
+     * @throws UserNotFoundException when the user is not found.
+     * @throws TransientException If an temporary, unexpected problem occurred.
+     * @throws AccessControlException If the operation is not permitted.
+     */
+    User<T> getPendingUser(T userID)
+            throws UserNotFoundException, TransientException,
+                   AccessControlException;
     
     /**
      * Attempt to login the specified user.
@@ -139,7 +154,7 @@ public interface UserPersistence<T extends Principal>
     /**
      * Updated the user specified by User.
      *
-     * @param user
+     * @param user      The user instance to modify.
      *
      * @return User instance.
      * 
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 726b5f94..77c635c5 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
@@ -77,32 +77,14 @@ import ca.nrc.cadc.ac.User;
 import ca.nrc.cadc.ac.UserNotFoundException;
 import ca.nrc.cadc.net.TransientException;
 import ca.nrc.cadc.util.StringUtil;
-import com.unboundid.ldap.sdk.AddRequest;
-import com.unboundid.ldap.sdk.Attribute;
-import com.unboundid.ldap.sdk.DN;
-import com.unboundid.ldap.sdk.Filter;
-import com.unboundid.ldap.sdk.LDAPException;
-import com.unboundid.ldap.sdk.LDAPResult;
-import com.unboundid.ldap.sdk.LDAPSearchException;
-import com.unboundid.ldap.sdk.Modification;
-import com.unboundid.ldap.sdk.ModificationType;
-import com.unboundid.ldap.sdk.ModifyRequest;
-import com.unboundid.ldap.sdk.ResultCode;
-import com.unboundid.ldap.sdk.SearchRequest;
-import com.unboundid.ldap.sdk.SearchResult;
-import com.unboundid.ldap.sdk.SearchResultEntry;
-import com.unboundid.ldap.sdk.SearchScope;
+import com.unboundid.ldap.sdk.*;
 import com.unboundid.ldap.sdk.controls.ProxiedAuthorizationV2RequestControl;
 import org.apache.log4j.Logger;
 
 import javax.security.auth.x500.X500Principal;
 import java.security.AccessControlException;
 import java.security.Principal;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
+import java.util.*;
 
 public class LdapGroupDAO<T extends Principal> extends LdapDAO
 {
@@ -323,49 +305,47 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
      * 
      * @throws TransientException If an temporary, unexpected problem occurred.
      */
-    public Collection<String> getGroupNames()
-        throws TransientException
+    public Collection<String> getGroupNames() throws TransientException
     {
         try
         {
-            Filter filter = Filter.createPresenceFilter("cn");
-            String [] attributes = new String[] {"cn", "nsaccountlock"};
-            
-            SearchRequest searchRequest = 
-                    new SearchRequest(config.getGroupsDN(), 
-                                      SearchScope.SUB, filter, attributes);
-    
-            SearchResult searchResult = null;
-            try
-            {
-                searchResult = getConnection().search(searchRequest);
-            }
-            catch (LDAPSearchException e)
+            final Filter filter = Filter.createPresenceFilter("cn");
+            final String [] attributes = new String[] {"cn", "nsaccountlock"};
+            final List<String> groupNames = new ArrayList<String>();
+            final long begin = System.currentTimeMillis();
+
+            final SearchResult searchResult =
+                    getConnection().search(new SearchResultListener()
             {
-                if (e.getResultCode() == ResultCode.NO_SUCH_OBJECT)
+                @Override
+                public void searchEntryReturned(
+                        final SearchResultEntry searchEntry)
                 {
-                    logger.debug("Could not find groups root", e);
-                    throw new IllegalStateException("Could not find groups root");
+                    groupNames.add(searchEntry.getAttributeValue("cn"));
                 }
-            }
-            
-            LdapDAO.checkLdapResult(searchResult.getResultCode());
-            List<String> groupNames = new ArrayList<String>();
-            for (SearchResultEntry next : searchResult.getSearchEntries())
-            {
-                if (!next.hasAttribute("nsaccountlock"))
+
+                @Override
+                public void searchReferenceReturned(
+                        final SearchResultReference searchReference)
                 {
-                    groupNames.add(next.getAttributeValue("cn"));
+
                 }
-            }
-            
+            }, config.getGroupsDN(), SearchScope.ONE, filter, attributes);
+
+            LdapDAO.checkLdapResult(searchResult.getResultCode());
+            long end = System.currentTimeMillis();
+
+            logger.info("<-- groupNames in " + ((new Long(end).doubleValue()
+                                                 - new Long(begin).doubleValue())
+                                                / 1000.0) + " seconds.");
             return groupNames;
         }
         catch (LDAPException e1)
         {
         	logger.debug("getGroupNames Exception: " + e1, e1);
             LdapDAO.checkLdapResult(e1.getResultCode());
-            throw new IllegalStateException("Unexpected exception: " + e1.getMatchedDN(), e1);
+            throw new IllegalStateException("Unexpected exception: "
+                                            + e1.getMatchedDN(), e1);
         }
         
     }
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 5e0b281b..d3d21770 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
@@ -122,6 +122,7 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
 
     // Returned User attributes
     protected static final String LDAP_OBJECT_CLASS = "objectClass";
+    protected static final String LDAP_INET_USER = "inetuser";
     protected static final String LDAP_INET_ORG_PERSON = "inetOrgPerson";
     protected static final String LDAP_CADC_ACCOUNT = "cadcaccount";
     protected static final String LDAP_NSACCOUNTLOCK = "nsaccountlock";
@@ -352,13 +353,14 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
             // add new user
             List<Attribute> attributes = new ArrayList<Attribute>();
             addAttribute(attributes, LDAP_OBJECT_CLASS, LDAP_INET_ORG_PERSON);
+            addAttribute(attributes, LDAP_OBJECT_CLASS, LDAP_INET_USER);
             addAttribute(attributes, LDAP_OBJECT_CLASS, LDAP_CADC_ACCOUNT);
             addAttribute(attributes, LDAP_COMMON_NAME, user.getUserID()
                 .getName());
             addAttribute(attributes, LDAP_DISTINGUISHED_NAME, userDN
                 .toNormalizedString());
             addAttribute(attributes, LADP_USER_PASSWORD, userRequest
-                .getPassword());
+                    .getPassword());
 
             for (UserDetails details : user.details)
             {
@@ -397,7 +399,7 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
      *
      * @param userID The userID.
      * @return User instance.
-     * @throws UserNotFoundException  when the user is not found.
+     * @throws UserNotFoundException  when the user is not found in the main tree.
      * @throws TransientException     If an temporary, unexpected problem occurred.
      * @throws AccessControlException If the operation is not permitted.
      */
@@ -408,6 +410,23 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
         return getUser(userID, config.getUsersDN());
     }
 
+    /**
+     * Obtain a user who is awaiting approval.
+     *
+     * @param userID        The user ID of the pending user.
+     * @return              A User instance awaiting approval.
+     *
+     * @throws UserNotFoundException  when the user is not found in the user request tree.
+     * @throws TransientException     If an temporary, unexpected problem occurred.
+     * @throws AccessControlException If the operation is not permitted.
+     */
+    public User<T> getPendingUser(final T userID)
+            throws UserNotFoundException, TransientException,
+                   AccessControlException
+    {
+        return getUser(userID, config.getUserRequestsDN());
+    }
+
 
     /**
      * Get the user specified by userID.
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserPersistence.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserPersistence.java
index 05aedbac..f01c6f2b 100755
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserPersistence.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserPersistence.java
@@ -134,8 +134,7 @@ public class LdapUserPersistence<T extends Principal>
         try
         {
             userDAO = new LdapUserDAO<T>(this.config);
-            User<T> ret = userDAO.addUser(user);
-            return ret;
+            return userDAO.addUser(user);
         }
         finally
         {
@@ -164,8 +163,36 @@ public class LdapUserPersistence<T extends Principal>
         try
         {
             userDAO = new LdapUserDAO<T>(this.config);
-            User<T> ret = userDAO.getUser(userID);
-            return ret;
+            return userDAO.getUser(userID);
+        }
+        finally
+        {
+            if (userDAO != null)
+            {
+                userDAO.close();
+            }
+        }
+    }
+
+    /**
+    * Get the user specified by userID whose account is pending approval.
+    *
+    * @param userID The userID.
+    * @return User instance.
+    * @throws UserNotFoundException  when the user is not found.
+    * @throws TransientException     If an temporary, unexpected problem occurred.
+    * @throws AccessControlException If the operation is not permitted.
+    */
+    @Override
+    public User<T> getPendingUser(final T userID) throws UserNotFoundException,
+                                                         TransientException,
+                                                         AccessControlException
+    {
+        LdapUserDAO<T> userDAO = null;
+        try
+        {
+            userDAO = new LdapUserDAO<T>(this.config);
+            return userDAO.getPendingUser(userID);
         }
         finally
         {
@@ -204,11 +231,11 @@ public class LdapUserPersistence<T extends Principal>
             }
         }
     }
-       
+
     /**
      * Updated the user specified by User.
      *
-     * @param user
+     * @param user          The user to update.
      *
      * @return User instance.
      * 
@@ -224,8 +251,7 @@ public class LdapUserPersistence<T extends Principal>
         try
         {
             userDAO = new LdapUserDAO<T>(this.config);
-            User<T> ret = userDAO.modifyUser(user);
-            return ret;
+            return userDAO.modifyUser(user);
         }
         finally
         {
@@ -284,8 +310,7 @@ public class LdapUserPersistence<T extends Principal>
         try
         {
             userDAO = new LdapUserDAO<T>(this.config);
-            Collection<DN> ret = userDAO.getUserGroups(userID, isAdmin);
-            return ret;
+            return userDAO.getUserGroups(userID, isAdmin);
         }
         finally
         {
@@ -316,8 +341,7 @@ public class LdapUserPersistence<T extends Principal>
         try
         {
             userDAO = new LdapUserDAO<T>(this.config);
-            boolean ret = userDAO.isMember(userID, groupID);
-            return ret;
+            return userDAO.isMember(userID, groupID);
         }
         finally
         {
@@ -327,5 +351,4 @@ public class LdapUserPersistence<T extends Principal>
             }
         }
     }
-
 }
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/CreateUserAction.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/CreateUserAction.java
index d53ad283..96ec4f62 100644
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/CreateUserAction.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/CreateUserAction.java
@@ -68,6 +68,7 @@
  */
 package ca.nrc.cadc.ac.server.web.users;
 
+import java.io.IOException;
 import java.io.InputStream;
 
 import ca.nrc.cadc.ac.ReaderException;
@@ -75,23 +76,27 @@ import ca.nrc.cadc.ac.User;
 import ca.nrc.cadc.ac.UserAlreadyExistsException;
 import ca.nrc.cadc.ac.UserRequest;
 import ca.nrc.cadc.ac.server.UserPersistence;
+import ca.nrc.cadc.auth.HttpPrincipal;
 
 import javax.servlet.http.HttpServletResponse;
 import java.security.Principal;
+import java.util.Set;
 
 
 public class CreateUserAction extends UsersAction
 {
     private final InputStream inputStream;
 
-    CreateUserAction(UserLogInfo logInfo, InputStream inputStream)
+
+    CreateUserAction(final UserLogInfo logInfo,
+                     final InputStream inputStream)
     {
         super(logInfo);
         this.inputStream = inputStream;
     }
 
-    public Object run()
-        throws Exception
+
+    public Object run() throws Exception
     {
         try
         {
@@ -101,8 +106,20 @@ public class CreateUserAction extends UsersAction
                     readUserRequest(this.inputStream);
             final User<Principal> newUser =
                     userPersistence.addUser(userRequest);
+            final Set<HttpPrincipal> httpPrincipals =
+                    newUser.getIdentities(HttpPrincipal.class);
+
+            if (httpPrincipals.isEmpty())
+            {
+                throw new IOException("No Web Identity found (HttpPrincipal)");
+            }
+            else
+            {
+                response.setStatus(HttpServletResponse.SC_CREATED);
+                redirectGet(httpPrincipals.toArray(
+                        new HttpPrincipal[1])[0].getName());
+            }
 
-            writeUser(newUser);
             logUserInfo(newUser.getUserID().getName());
         }
         catch (UserAlreadyExistsException e)
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/GetUserAction.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/GetUserAction.java
index d4ee1d0e..0d87074e 100644
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/GetUserAction.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/GetUserAction.java
@@ -68,6 +68,7 @@
  */package ca.nrc.cadc.ac.server.web.users;
 
 import ca.nrc.cadc.ac.User;
+import ca.nrc.cadc.ac.UserNotFoundException;
 import ca.nrc.cadc.ac.server.UserPersistence;
 
 import java.security.Principal;
@@ -85,8 +86,19 @@ public class GetUserAction extends UsersAction
 
     public Object run() throws Exception
     {
-        UserPersistence<Principal> userPersistence = getUserPersistence();
-        User<Principal> user = userPersistence.getUser(userID);
+        final UserPersistence<Principal> userPersistence = getUserPersistence();
+
+        User<Principal> user;
+
+        try
+        {
+            user = userPersistence.getUser(userID);
+        }
+        catch (UserNotFoundException e)
+        {
+            user = userPersistence.getPendingUser(userID);
+        }
+
         writeUser(user);
         return null;
     }
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/ModifyUserAction.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/ModifyUserAction.java
index c98c2968..4bcdffcb 100644
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/ModifyUserAction.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/ModifyUserAction.java
@@ -68,41 +68,64 @@
  */
 package ca.nrc.cadc.ac.server.web.users;
 
-import java.io.InputStream;
 import ca.nrc.cadc.ac.User;
 import ca.nrc.cadc.ac.server.UserPersistence;
-import ca.nrc.cadc.ac.xml.UserReader;
+import ca.nrc.cadc.auth.HttpPrincipal;
 
+import java.io.IOException;
+import java.io.InputStream;
 import java.security.Principal;
+import java.util.Set;
+
 
 public class ModifyUserAction extends UsersAction
 {
-    private final Principal userID;
-    private final String request;
     private final InputStream inputStream;
 
-    ModifyUserAction(UserLogInfo logInfo, Principal userID,
-                      final String request, InputStream inputStream)
+
+    ModifyUserAction(final UserLogInfo logInfo,
+                     final InputStream inputStream)
     {
         super(logInfo);
-        this.userID = userID;
-        this.request = request;
+
         this.inputStream = inputStream;
     }
 
-    public Object run()
-        throws Exception
+
+    public Object run() throws Exception
     {
-        UserPersistence userPersistence = getUserPersistence();
-        User<? extends Principal> user = UserReader.read(this.inputStream);
-        User<? extends Principal> oldUser = userPersistence.getUser(userID);
-        userPersistence.modifyUser(user);
+        final User<Principal> user = readUser(this.inputStream);
+        final User<Principal> modifiedUser = modifyUser(user);
+        logUserInfo(modifiedUser.getUserID().getName());
 
-        logUserInfo(user.getUserID().getName());
+        final Set<HttpPrincipal> httpPrincipals =
+                modifiedUser.getIdentities(HttpPrincipal.class);
 
-        this.response.sendRedirect(request);
+        if (httpPrincipals.isEmpty())
+        {
+            throw new IOException("No Web Identity found (HttpPrincipal)");
+        }
+        else
+        {
+            redirectGet(httpPrincipals.toArray(
+                    new HttpPrincipal[1])[0].getName());
+        }
 
         return null;
     }
 
+    /**
+     * Perform the modification at the persistence level.
+     *
+     * @param user          The user to modify.
+     * @param <T>           The ID (Principal) type.
+     * @return              The modified User.
+     * @throws Exception    Any problems during update.
+     */
+    <T extends Principal> User<T> modifyUser(final User<T> user)
+            throws Exception
+    {
+        final UserPersistence<T> userPersistence = getUserPersistence();
+        return userPersistence.modifyUser(user);
+    }
 }
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UsersAction.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UsersAction.java
index df6880b9..39ca1313 100644
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UsersAction.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UsersAction.java
@@ -99,6 +99,9 @@ public abstract class UsersAction
     protected HttpServletResponse response;
     protected String acceptedContentType = DEFAULT_CONTENT_TYPE;
 
+    private String redirectURLPrefix;
+
+
     UsersAction(UserLogInfo logInfo)
     {
         this.logInfo = logInfo;
@@ -250,6 +253,38 @@ public abstract class UsersAction
         return userRequest;
     }
 
+    /**
+     * Read the user from the given stream of marshalled data.
+     *
+     * @param inputStream       The stream to read in.
+     * @return                  User instance, never null.
+     *
+     * @throws IOException      Any errors in reading the stream.
+     */
+    protected final User<Principal> readUser(final InputStream inputStream)
+            throws IOException
+    {
+        response.setContentType(acceptedContentType);
+        final User<Principal> user;
+
+        if (acceptedContentType.equals(DEFAULT_CONTENT_TYPE))
+        {
+            user = ca.nrc.cadc.ac.xml.UserReader.read(inputStream);
+        }
+        else if (acceptedContentType.equals(JSON_CONTENT_TYPE))
+        {
+            user = ca.nrc.cadc.ac.json.UserReader.read(inputStream);
+        }
+        else
+        {
+            // Should never happen.
+            throw new IOException("Unknown content being asked for: "
+                                  + acceptedContentType);
+        }
+
+        return user;
+    }
+
     /**
      * Write a user to the response's writer.
      *
@@ -292,4 +327,16 @@ public abstract class UsersAction
             ca.nrc.cadc.ac.json.UsersWriter.write(users, writer);
         }
     }
+
+    protected void setRedirectURLPrefix(final String redirectURLPrefix)
+    {
+        this.redirectURLPrefix = redirectURLPrefix;
+    }
+
+    void redirectGet(final String userID) throws Exception
+    {
+        final String redirectURL = this.redirectURLPrefix + "/" + userID
+                                   + "?idType=HTTP";
+        response.setHeader("Location", redirectURL);
+    }
 }
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UsersActionFactory.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UsersActionFactory.java
index 37ddf363..9333357c 100644
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UsersActionFactory.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UsersActionFactory.java
@@ -77,7 +77,7 @@ import ca.nrc.cadc.auth.OpenIdPrincipal;
 import ca.nrc.cadc.util.StringUtil;
 
 import java.io.IOException;
-import java.net.URL;
+import java.io.InputStream;
 import java.security.Principal;
 import javax.security.auth.x500.X500Principal;
 import javax.servlet.http.HttpServletRequest;
@@ -128,10 +128,10 @@ public class UsersActionFactory
             }
             else if (method.equals("PUT"))
             {
-                action = new CreateUserAction(logInfo, request
-                        .getInputStream());
+                action = new CreateUserAction(logInfo,
+                                              request.getInputStream());
+                action.setRedirectURLPrefix(request.getRequestURL().toString());
             }
-
         }
         else
         {
@@ -147,25 +147,9 @@ public class UsersActionFactory
             }
             else if (method.equals("POST"))
             {
-                final URL requestURL = new URL(request.getRequestURL()
-                                                       .toString());
-                final StringBuilder sb = new StringBuilder();
-                sb.append(requestURL.getProtocol());
-                sb.append("://");
-                sb.append(requestURL.getHost());
-                if (requestURL.getPort() > 0)
-                {
-                    sb.append(":");
-                    sb.append(requestURL.getPort());
-                }
-                sb.append(request.getContextPath());
-                sb.append(request.getServletPath());
-                sb.append("/");
-                sb.append(path);
-
-                action = new ModifyUserAction(logInfo, user.getUserID(), sb
-                        .toString(),
+                action = new ModifyUserAction(logInfo,
                                               request.getInputStream());
+                action.setRedirectURLPrefix(request.getRequestURL().toString());
             }
         }
 
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
index 7d3be479..2dd02014 100644
--- 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
@@ -70,13 +70,14 @@ package ca.nrc.cadc.ac.server.ldap;
 
 import java.security.AccessControlException;
 
+import ca.nrc.cadc.net.TransientException;
 import com.unboundid.ldap.sdk.LDAPConnection;
 import com.unboundid.ldap.sdk.LDAPException;
 
 
 public class LdapDAOTestImpl extends LdapDAO
 {
-    public LdapDAOTestImpl(LdapConfig config)
+    public LdapDAOTestImpl(LdapConfig config) throws TransientException
     {
         super(config);
     }
diff --git a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java
index 2f9bcdf2..b5f7b7fc 100644
--- a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java
+++ b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java
@@ -150,7 +150,7 @@ public class LdapGroupDAOTest extends AbstractLdapDAOTest
         anonSubject.getPrincipals().add(unknownUser.getUserID());
     }
 
-    LdapGroupDAO<X500Principal> getGroupDAO()
+    LdapGroupDAO<X500Principal> getGroupDAO() throws Exception
     {
         return new LdapGroupDAO<X500Principal>(config,
                 new LdapUserDAO<X500Principal>(config));
diff --git a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAOTest.java b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAOTest.java
index 0e592c80..48d6a142 100644
--- a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAOTest.java
+++ b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAOTest.java
@@ -73,6 +73,7 @@ import ca.nrc.cadc.ac.User;
 import ca.nrc.cadc.ac.UserDetails;
 import ca.nrc.cadc.ac.UserRequest;
 import ca.nrc.cadc.auth.HttpPrincipal;
+import ca.nrc.cadc.net.TransientException;
 import ca.nrc.cadc.util.Log4jInit;
 import com.unboundid.ldap.sdk.DN;
 import org.apache.log4j.Level;
@@ -92,7 +93,7 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
-public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
+public class LdapUserDAOTest extends AbstractLdapDAOTest
 {
     private static final Logger log = Logger.getLogger(LdapUserDAOTest.class);
 
@@ -100,8 +101,10 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
 
     static String testUserDN;
     static User<X500Principal> testUser;
+    static User<HttpPrincipal> testPendingUser;
     static LdapConfig config;
 
+
     @BeforeClass
     public static void setUpBeforeClass()
             throws Exception
@@ -111,6 +114,12 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
         // get the configuration of the development server from and config files...
         config = getLdapConfig();
 
+        testPendingUser =
+                new User<HttpPrincipal>(new HttpPrincipal("CADCtestRequest"));
+        testPendingUser.details.add(new PersonalDetails("CADCtest", "Request"));
+        testPendingUser.getIdentities().add(
+                new HttpPrincipal("CADCtestRequest"));
+
         testUser = new User<X500Principal>(new X500Principal(testUserX509DN));
         testUser.details.add(new PersonalDetails("CADC", "DAOTest1"));
         testUser.getIdentities().add(new HttpPrincipal("CadcDaoTest1"));
@@ -118,12 +127,12 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
         testUserDN = "uid=cadcdaotest1," + config.getUsersDN();
     }
 
-    LdapUserDAO getUserDAO()
+    <T extends Principal> LdapUserDAO<T> getUserDAO() throws Exception
     {
-        return new LdapUserDAO(config);
+        return new LdapUserDAO<T>(config);
     }
 
-    String getUserID()
+    String createUserID()
     {
         return "CadcDaoTestUser-" + System.currentTimeMillis();
     }
@@ -134,8 +143,9 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
     @Test
     public void testAddUser() throws Exception
     {
-        final User<HttpPrincipal> expected = new User<HttpPrincipal>(new HttpPrincipal(getUserID()));
-        expected.getIdentities().add(new HttpPrincipal(getUserID()));
+        final User<HttpPrincipal> expected =
+                new User<HttpPrincipal>(new HttpPrincipal(createUserID()));
+        expected.getIdentities().add(new HttpPrincipal(createUserID()));
         expected.details.add(new PersonalDetails("foo", "bar"));
 
         final UserRequest<HttpPrincipal> userRequest =
@@ -144,10 +154,39 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
         Subject subject = new Subject();
         subject.getPrincipals().add(testUser.getUserID());
 
-        User<HttpPrincipal> actual = getUserDAO().addUser(userRequest);
+        final LdapUserDAO<HttpPrincipal> userDAO = getUserDAO();
+        User<HttpPrincipal> actual = userDAO.addUser(userRequest);
         check(expected, actual);
     }
 
+    @Test
+    public void testGetPendingUser() throws Exception
+    {
+        final Subject subject = new Subject();
+        subject.getPrincipals().add(testUser.getUserID());
+
+        // do everything as owner
+        Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+                try
+                {
+                    final LdapUserDAO<HttpPrincipal> userDAO = getUserDAO();
+                    final User<HttpPrincipal> actual =
+                            userDAO.getPendingUser(testPendingUser.getUserID());
+                    check(testPendingUser, actual);
+
+                    return null;
+                }
+                catch (Exception e)
+                {
+                    throw new Exception("Problems", e);
+                }
+            }
+        });
+    }
+
     /**
      * Test of getUser method, of class LdapUserDAO.
      */
@@ -160,12 +199,13 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
         // do everything as owner
         Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
         {
-            public Object run()
-                throws Exception
+            public Object run() throws Exception
             {
                 try
                 {
-                    User<X500Principal> actual = getUserDAO().getUser(testUser.getUserID());
+                    final LdapUserDAO<X500Principal> userDAO = getUserDAO();
+                    final User<X500Principal> actual =
+                            userDAO.getUser(testUser.getUserID());
                     check(testUser, actual);
 
                     return null;
@@ -176,7 +216,6 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
                 }
             }
         });
-
     }
 
     /**
@@ -375,7 +414,7 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
 
         // Create a test user with a known password
         final User<HttpPrincipal> testUser2;
-        final String username = getUserID();
+        final String username = createUserID();
         final String password = "foo";
         final String newPassword = "bar";
 
@@ -383,7 +422,8 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
         testUser2 = new User<HttpPrincipal>(principal);
         testUser2.getIdentities().add(principal);
         testUser2.details.add(new PersonalDetails("firstName", "lastName"));
-        final UserRequest userRequest = new UserRequest(testUser2, password);
+        final UserRequest<HttpPrincipal> userRequest =
+                new UserRequest<HttpPrincipal>(testUser2, password);
 
         // add the user
         Subject subject = new Subject();
@@ -395,7 +435,8 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
             {
                 try
                 {
-                    return getUserDAO().newUser(userRequest);
+                    final LdapUserDAO<HttpPrincipal> userDAO = getUserDAO();
+                    return userDAO.newUser(userRequest);
                 }
                 catch (Exception e)
                 {
@@ -431,7 +472,8 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
             {
                 try
                 {
-                    getUserDAO().setPassword(testUser2, password, newPassword);
+                    final LdapUserDAO<HttpPrincipal> userDAO = getUserDAO();
+                    userDAO.setPassword(testUser2, password, newPassword);
                     fail("should throw exception if subject and user are not the same");
                 }
                 catch (Exception ignore){}
@@ -448,7 +490,8 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
             {
                 try
                 {
-                    getUserDAO().setPassword(testUser2, password, newPassword);
+                    final LdapUserDAO<HttpPrincipal> userDAO = getUserDAO();
+                    userDAO.setPassword(testUser2, password, newPassword);
                 }
                 catch (Exception e)
                 {
@@ -484,14 +527,15 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
     {
         // Create a test user
         final User<HttpPrincipal> testUser2;
-        final String username = getUserID();
+        final String username = createUserID();
         final String password = "foo";
 
         HttpPrincipal principal = new HttpPrincipal(username);
         testUser2 = new User<HttpPrincipal>(principal);
         testUser2.getIdentities().add(principal);
         testUser2.details.add(new PersonalDetails("firstName", "lastName"));
-        final UserRequest userRequest = new UserRequest(testUser2, password);
+        final UserRequest<HttpPrincipal> userRequest =
+                new UserRequest<HttpPrincipal>(testUser2, password);
 
         // add the user
         Subject subject = new Subject();
@@ -503,7 +547,8 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
             {
                 try
                 {
-                    return getUserDAO().newUser(userRequest);
+                    final LdapUserDAO<HttpPrincipal> userDAO = getUserDAO();
+                    return userDAO.newUser(userRequest);
                 }
                 catch (Exception e)
                 {
@@ -536,7 +581,8 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
             {
                 try
                 {
-                    getUserDAO().modifyUser(testUser2);
+                    final LdapUserDAO<HttpPrincipal> userDAO = getUserDAO();
+                    userDAO.modifyUser(testUser2);
                     fail("should throw exception if subject and user are not the same");
                 }
                 catch (Exception ignore)
@@ -556,7 +602,8 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
             {
                 try
                 {
-                    return getUserDAO().modifyUser(testUser2);
+                    final LdapUserDAO<HttpPrincipal> userDAO = getUserDAO();
+                    return userDAO.modifyUser(testUser2);
                 }
                 catch (Exception e)
                 {
@@ -570,12 +617,14 @@ public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest
         check(testUser2, updatedUser);
     }
 
-    private static void check(final User<? extends Principal> user1, final User<? extends Principal> user2)
+    private static void check(final User<? extends Principal> user1,
+                              final User<? extends Principal> user2)
     {
         assertEquals(user1, user2);
         assertEquals(user1.details, user2.details);
         assertEquals(user1.details.size(), user2.details.size());
-        assertEquals(user1.getIdentities(), user2.getIdentities());
+        assertEquals("Identities don't match.", user1.getIdentities(),
+                     user2.getIdentities());
         for (UserDetails d1 : user1.details)
         {
             assertTrue(user2.details.contains(d1));
diff --git a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/users/ModifyUserActionTest.java b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/users/ModifyUserActionTest.java
new file mode 100644
index 00000000..dde4aaa3
--- /dev/null
+++ b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/users/ModifyUserActionTest.java
@@ -0,0 +1,152 @@
+/*
+ ************************************************************************
+ *******************  CANADIAN ASTRONOMY DATA CENTRE  *******************
+ **************  CENTRE CANADIEN DE DONNÉES ASTRONOMIQUES  **************
+ *
+ *  (c) 2015.                            (c) 2015.
+ *  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.users;
+
+
+import java.io.*;
+import java.security.Principal;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import ca.nrc.cadc.ac.PersonalDetails;
+import ca.nrc.cadc.ac.User;
+import ca.nrc.cadc.ac.server.UserPersistence;
+import ca.nrc.cadc.auth.HttpPrincipal;
+import org.junit.Test;
+import static org.junit.Assert.*;
+import static org.easymock.EasyMock.*;
+
+
+public class ModifyUserActionTest
+{
+    @Test
+    public void run() throws Exception
+    {
+        final byte[] input =
+                "{\"user\":{\"userID\":{\"identity\":{\"name\":\"CADCtest\",\"type\":\"HTTP\"}},\"details\":[{\"userDetails\":{\"lastName\":\"Test\",\"firstName\":\"CADC\",\"email\":\"CADC.Test@nrc-cnrc.gc.ca\",\"type\":\"personalDetails\"}}],\"identities\":[{\"identity\":{\"name\":\"CADCtest\",\"type\":\"HTTP\"}}]}}"
+                        .getBytes();
+        final InputStream inputStream = new ByteArrayInputStream(input);
+
+        // Should match the JSON above, without the e-mail modification.
+        final User<Principal> userObject =
+                new User<Principal>(new HttpPrincipal("CADCtest"));
+        final PersonalDetails personalDetail =
+                new PersonalDetails("CADC", "Test");
+        personalDetail.email = "CADC.Test@nrc-cnrc.gc.ca";
+        userObject.details.add(personalDetail);
+
+        final HttpServletRequest mockRequest =
+                createMock(HttpServletRequest.class);
+        final HttpServletResponse mockResponse =
+                createMock(HttpServletResponse.class);
+
+        final String requestURL = "http://mysite.com:8080/noentry/authorize";
+
+        @SuppressWarnings("unchecked")
+        final UserPersistence<Principal> mockUserPersistence =
+                createMock(UserPersistence.class);
+
+        expect(mockUserPersistence.modifyUser(userObject)).andReturn(
+                userObject).once();
+
+        expect(mockRequest.getMethod()).andReturn("POST").once();
+        expect(mockRequest.getRemoteAddr()).andReturn(requestURL).
+                once();
+
+        mockResponse.sendRedirect(requestURL);
+        expectLastCall().once();
+
+        mockResponse.setContentType("application/json");
+        expectLastCall().once();
+
+        replay(mockRequest, mockResponse, mockUserPersistence);
+
+        final UserLogInfo userLogInfo = new UserLogInfo(mockRequest);
+        final ModifyUserAction testSubject = new ModifyUserAction(userLogInfo,
+                                                                  inputStream)
+        {
+            @Override
+            @SuppressWarnings("unchecked")
+            UserPersistence<Principal> getUserPersistence()
+            {
+                return mockUserPersistence;
+            }
+        };
+
+        testSubject.setAcceptedContentType("application/json");
+        testSubject.response = mockResponse;
+
+        testSubject.run();
+
+        assertEquals("Wrong username logged.", "CADCtest",
+                     userLogInfo.userName);
+        verify(mockRequest, mockResponse, mockUserPersistence);
+    }
+}
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/User.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/User.java
index b7ad0bcc..97a63b6b 100644
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/User.java
+++ b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/User.java
@@ -134,18 +134,15 @@ public class User<T extends Principal>
         {
             return false;
         }
-        User other = (User) obj;
+
+        final User other = (User) obj;
         if (userID instanceof X500Principal)
         {
             return AuthenticationUtil.equals(userID, other.userID);
         }
         else
         {
-            if (!userID.equals(other.userID))
-            {
-                return false;
-            }
-            return true;
+            return userID.equals(other.userID);
         }
     }
 
@@ -173,4 +170,31 @@ public class User<T extends Principal>
 
         return matchedDetails;
     }
+
+    /**
+     * Obtain a set of identities whose type match the given one.
+     *
+     * @param identityClass     The class to search on.
+     * @param <S>               The Principal type.
+     * @return                  Set of matched identities, or empty Set.
+     *                          Never null.
+     */
+    public <S extends Principal> Set<S> getIdentities(
+            final Class<S> identityClass)
+    {
+        final Set<S> matchedIdentities = new HashSet<S>();
+
+        for (final Principal p : identities)
+        {
+            if (p.getClass() == identityClass)
+            {
+                // This casting shouldn't happen, but it's the only way to
+                // do this without a lot of work.
+                // jenkinsd 2014.09.26
+                matchedIdentities.add((S) p);
+            }
+        }
+
+        return matchedIdentities;
+    }
 }
-- 
GitLab