diff --git a/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java b/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java
index e2f4371fe2818f086db178aca0b5b95370ba4494..9fb16743fbb52a67faf47f47c66b117914b921b1 100755
--- a/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java
+++ b/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java
@@ -815,7 +815,7 @@ public class LdapUserDAO extends LdapDAO
logger.debug("search filter: " + filter);
final String[] attributes = new String[]
- { LDAP_UID, LDAP_FIRST_NAME, LDAP_LAST_NAME };
+ { LDAP_USER_NAME, LDAP_FIRST_NAME, LDAP_LAST_NAME };
final SearchRequest searchRequest =
new SearchRequest(usersDN, SearchScope.ONE, filter, attributes);
@@ -831,10 +831,10 @@ public class LdapUserDAO extends LdapDAO
next.getAttributeValue(LDAP_FIRST_NAME);
final String lastName =
next.getAttributeValue(LDAP_LAST_NAME).trim();
- final String uid = next.getAttributeValue(LDAP_UID);
+ final String username = next.getAttributeValue(LDAP_USER_NAME);
User user = new User();
- user.getIdentities().add(new HttpPrincipal(uid));
+ user.getIdentities().add(new HttpPrincipal(username));
// Only add Personal Details if it is relevant.
if (StringUtil.hasLength(firstName) &&
diff --git a/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/groups/CreateGroupAction.java b/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/groups/CreateGroupAction.java
index 37c3e1335499e464880c68141f041b724df29e89..d58b5b791a82f3864527ee49834c537e419c3212 100755
--- a/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/groups/CreateGroupAction.java
+++ b/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/groups/CreateGroupAction.java
@@ -107,7 +107,7 @@ public class CreateGroupAction extends AbstractGroupAction
}
for (User usr : group.getUserMembers())
{
- addedMembers.add(usr.getHttpPrincipal().getName());
+ addedMembers.add(usr.getX500Principal().getName());
}
}
logGroupInfo(group.getID(), null, addedMembers);
diff --git a/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/groups/RemoveUserMemberAction.java b/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/groups/RemoveUserMemberAction.java
index 1bda22da92a2d9b4dbb52c744c9e3678b6ecf2c6..8abb5a7a6a2d2ed80b4a436baa8285947e28f3ed 100755
--- a/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/groups/RemoveUserMemberAction.java
+++ b/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/groups/RemoveUserMemberAction.java
@@ -78,6 +78,7 @@ import ca.nrc.cadc.ac.User;
import ca.nrc.cadc.ac.server.PluginFactory;
import ca.nrc.cadc.ac.server.UserPersistence;
import ca.nrc.cadc.auth.AuthenticationUtil;
+import ca.nrc.cadc.util.ObjectUtil;
public class RemoveUserMemberAction extends AbstractGroupAction
{
@@ -99,7 +100,12 @@ public class RemoveUserMemberAction extends AbstractGroupAction
Group group = groupPersistence.getGroup(this.groupName);
Principal userPrincipal = AuthenticationUtil.createPrincipal(this.userID, this.userIDType);
- User toRemove = getUserPersistence().getUser(userPrincipal);
+
+ User user = getUserPersistence().getAugmentedUser(userPrincipal);
+ User toRemove = new User();
+ ObjectUtil.setField(toRemove, user.getID(), "id");
+ toRemove.getIdentities().addAll(user.getIdentities());
+
if (!group.getUserMembers().remove(toRemove))
{
throw new MemberNotFoundException();
diff --git a/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/groups/RemoveUserMemberActionTest.java b/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/groups/RemoveUserMemberActionTest.java
index 042cd981939921e4025f4c1657be61777ad05edb..d12fc441d495414ac8b4fa84b6df56af7750392f 100644
--- a/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/groups/RemoveUserMemberActionTest.java
+++ b/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/groups/RemoveUserMemberActionTest.java
@@ -71,11 +71,16 @@ package ca.nrc.cadc.ac.server.web.groups;
import static org.easymock.EasyMock.createMock;
import static org.junit.Assert.fail;
+import java.net.URI;
import java.security.Principal;
+import java.util.UUID;
import javax.security.auth.x500.X500Principal;
+import ca.nrc.cadc.ac.AC;
+import ca.nrc.cadc.ac.InternalID;
import ca.nrc.cadc.auth.HttpPrincipal;
+import ca.nrc.cadc.util.ObjectUtil;
import org.apache.log4j.Level;
import org.apache.log4j.Logger;
import org.easymock.EasyMock;
@@ -111,11 +116,14 @@ public class RemoveUserMemberActionTest
{
try
{
+ User user = new User();
+ InternalID internalID = new InternalID(new URI(AC.USER_URI + "?" + UUID.randomUUID()));
+ ObjectUtil.setField(user, internalID, "id");
+
String userID = "cn=foo,c=ca";
String userIDType = IdentityType.X500.getValue();
- Principal userPrincipal = AuthenticationUtil.createPrincipal(userID, userIDType);
- User user = new User();
- user.getIdentities().add(userPrincipal);
+ Principal x500Principal = AuthenticationUtil.createPrincipal(userID, userIDType);
+ user.getIdentities().add(x500Principal);
Group group = new Group("group");
User member = new User();
@@ -126,7 +134,7 @@ public class RemoveUserMemberActionTest
EasyMock.expect(mockGroupPersistence.getGroup("group")).andReturn(group);
final UserPersistence mockUserPersistence = EasyMock.createMock(UserPersistence.class);
- EasyMock.expect(mockUserPersistence.getUser(userPrincipal)).andReturn(user);
+ EasyMock.expect(mockUserPersistence.getAugmentedUser(x500Principal)).andReturn(user);
EasyMock.replay(mockGroupPersistence, mockUserPersistence);
@@ -160,10 +168,13 @@ public class RemoveUserMemberActionTest
{
try
{
+ User user = new User();
+ InternalID internalID = new InternalID(new URI(AC.USER_URI + "?" + UUID.randomUUID()));
+ ObjectUtil.setField(user, internalID, "id");
+
String userID = "cn=foo,c=ca";
String userIDType = IdentityType.X500.getValue();
Principal userPrincipal = AuthenticationUtil.createPrincipal(userID, userIDType);
- User user = new User();
user.getIdentities().add(new X500Principal(userID));
user.getIdentities().add(new HttpPrincipal("foo"));
@@ -176,7 +187,7 @@ public class RemoveUserMemberActionTest
EasyMock.expectLastCall();
final UserPersistence mockUserPersistence = EasyMock.createMock(UserPersistence.class);
- EasyMock.expect(mockUserPersistence.getUser(userPrincipal)).andReturn(user);
+ EasyMock.expect(mockUserPersistence.getAugmentedUser(userPrincipal)).andReturn(user);
EasyMock.replay(mockGroupPersistence, mockUserPersistence);
diff --git a/cadcAccessControl/src/ca/nrc/cadc/ac/User.java b/cadcAccessControl/src/ca/nrc/cadc/ac/User.java
index 19a1b2cd6e8fdaa05652ada5013ffcd2bcfb6568..827f3df6c1f3e36c64fed54d8567ba9835c83aa1 100644
--- a/cadcAccessControl/src/ca/nrc/cadc/ac/User.java
+++ b/cadcAccessControl/src/ca/nrc/cadc/ac/User.java
@@ -68,6 +68,7 @@
*/
package ca.nrc.cadc.ac;
+import java.io.PrintWriter;
import java.security.Principal;
import java.util.Comparator;
import java.util.Date;
@@ -77,8 +78,12 @@ import java.util.TreeSet;
import ca.nrc.cadc.auth.HttpPrincipal;
+import javax.security.auth.x500.X500Principal;
+
public class User
{
+ // How on God's green earth is this used? Where is it set?
+ // jenkinsd 2016.03.24
private InternalID id;
private Set<Principal> identities = new TreeSet<Principal>(new PrincipalComparator());
@@ -141,6 +146,14 @@ public class User
return null;
}
+ public X500Principal getX500Principal()
+ {
+ final Set<X500Principal> identities =
+ getIdentities(X500Principal.class);
+ return identities.isEmpty() ? null : identities.iterator().next();
+ }
+
+
/**
* A User is considered consistent if the User's set of identities are a superset
* of this Users set of identities.
diff --git a/cadcAccessControl/src/ca/nrc/cadc/ac/client/UserClient.java b/cadcAccessControl/src/ca/nrc/cadc/ac/client/UserClient.java
index 5d4bbc919b42ea7d6265b5acabb404ac0bc45895..389ceab4c6b7ce8177b644659f3a3dd9df78b687 100644
--- a/cadcAccessControl/src/ca/nrc/cadc/ac/client/UserClient.java
+++ b/cadcAccessControl/src/ca/nrc/cadc/ac/client/UserClient.java
@@ -124,8 +124,8 @@ public class UserClient
/**
* Constructor.
*
- * @param baseURL The URL of the supporting access control web service
- * obtained from the registry.
+ * @param serviceURI The URI of the supporting access control web service
+ * obtained from the registry.
*/
public UserClient(URI serviceURI)
throws IllegalArgumentException
@@ -136,7 +136,7 @@ public class UserClient
public UserClient(URI serviceURI, RegistryClient registryClient)
{
if (serviceURI == null)
- throw new IllegalArgumentException("invalid serviceURI: " + serviceURI);
+ throw new IllegalArgumentException("Service URI cannot be null.");
if (serviceURI.getFragment() != null)
throw new IllegalArgumentException("invalid serviceURI (fragment not allowed): " + serviceURI);
@@ -211,7 +211,9 @@ public class UserClient
{
URL usersURL = registryClient.getServiceURL(usersURI, "https");
final List<User> webUsers = new ArrayList<User>();
- HttpDownload httpDownload = new HttpDownload(usersURL, new JsonUserListInputStreamWrapper(webUsers));
+ HttpDownload httpDownload =
+ new HttpDownload(usersURL,
+ new JsonUserListInputStreamWrapper(webUsers));
httpDownload.setRequestProperty("Accept", "application/json");
httpDownload.run();
diff --git a/cadcAccessControl/test/src/ca/nrc/cadc/ac/client/UserClientTest.java b/cadcAccessControl/test/src/ca/nrc/cadc/ac/client/UserClientTest.java
index 205d68ad80646b0e3610a8f397eacb5cdc6d5bd8..067d5ab6b8f24d430ff5b063b41b3c483c8e0822 100644
--- a/cadcAccessControl/test/src/ca/nrc/cadc/ac/client/UserClientTest.java
+++ b/cadcAccessControl/test/src/ca/nrc/cadc/ac/client/UserClientTest.java
@@ -108,16 +108,32 @@ public class UserClientTest
try
{
new UserClient(null);
- Assert.fail("Null base URL should throw an illegalArgumentException.");
+ Assert.fail("Null service URI should throw an illegalArgumentException.");
}
catch (IllegalArgumentException iae)
{
- Assert.assertTrue(iae.getMessage().contains("invalid serviceURI"));
+ Assert.assertTrue(iae.getMessage().contains("cannot be null"));
}
catch (Throwable t)
{
Assert.fail("Unexpected exception: " + t.getMessage());
}
+
+ // case 2: serviceURI with a fragment
+ try
+ {
+ URI uri = new URI("http://foo.com/bar?test#fragment");
+ new UserClient(uri);
+ Assert.fail("Service URI containing a fragment should throw an illegalArgumentException.");
+ }
+ catch (IllegalArgumentException iae)
+ {
+ Assert.assertTrue(iae.getMessage().contains("fragment not allowed"));
+ }
+ catch (Throwable t)
+ {
+ Assert.fail("Unexpected exception: " + t.getMessage());
+ }
}
@Test