s1890 - changes to user equality logic (8059e1d5) · Commits · OATS-CADC / ac

cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/groups/CreateGroupAction.java

+7 −1

Original line number	Original line	Diff line number	Diff line
	@@ -69,6 +69,7 @@
	package ca.nrc.cadc.ac.server.web.groups;		package ca.nrc.cadc.ac.server.web.groups;

	import java.io.InputStream;		import java.io.InputStream;
			import java.security.Principal;
	import java.util.ArrayList;		import java.util.ArrayList;
	import java.util.List;		import java.util.List;

	@@ -107,7 +108,12 @@ public class CreateGroupAction extends AbstractGroupAction
	}		}
	for (User usr : group.getUserMembers())		for (User usr : group.getUserMembers())
	{		{
	addedMembers.add(usr.getX500Principal().getName());		Principal p = usr.getHttpPrincipal();
			if (p == null)
			{
			p = usr.getX500Principal();
			}
			addedMembers.add(p.getName());
	}		}
	}		}
	logGroupInfo(group.getID(), null, addedMembers);		logGroupInfo(group.getID(), null, addedMembers);

cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java

+17 −16

Original line number	Original line	Diff line number	Diff line
	@@ -67,23 +67,25 @@

	package ca.nrc.cadc.ac.server.ldap;		package ca.nrc.cadc.ac.server.ldap;

	import ca.nrc.cadc.ac.Group;		import static org.junit.Assert.assertEquals;
	import ca.nrc.cadc.ac.GroupNotFoundException;		import static org.junit.Assert.assertNotNull;
	import ca.nrc.cadc.ac.GroupProperty;		import static org.junit.Assert.assertTrue;
	import ca.nrc.cadc.ac.User;		import static org.junit.Assert.fail;
	import org.apache.log4j.Logger;
	import org.junit.Assert;
	import org.junit.Test;

	import javax.security.auth.Subject;
	import java.security.Principal;		import java.security.Principal;
	import java.security.PrivilegedExceptionAction;		import java.security.PrivilegedExceptionAction;
	import java.util.Collection;		import java.util.Collection;

	import static org.junit.Assert.assertEquals;		import javax.security.auth.Subject;
	import static org.junit.Assert.assertNotNull;
	import static org.junit.Assert.assertTrue;		import org.apache.log4j.Logger;
	import static org.junit.Assert.fail;		import org.junit.Assert;
			import org.junit.Test;

			import ca.nrc.cadc.ac.Group;
			import ca.nrc.cadc.ac.GroupNotFoundException;
			import ca.nrc.cadc.ac.GroupProperty;
			import ca.nrc.cadc.ac.User;

	public class LdapGroupDAOTest extends AbstractLdapDAOTest		public class LdapGroupDAOTest extends AbstractLdapDAOTest
	{		{
	@@ -156,7 +158,7 @@ public class LdapGroupDAOTest extends AbstractLdapDAOTest
	expectGroup.getUserMembers().add(cadcDaoTest2_User);		expectGroup.getUserMembers().add(cadcDaoTest2_User);
	expectGroup.getUserMembers().add(duplicateIdentity);		expectGroup.getUserMembers().add(duplicateIdentity);
	actualGroup = getGroupDAO().modifyGroup(expectGroup);		actualGroup = getGroupDAO().modifyGroup(expectGroup);
	expectGroup.getUserMembers().remove(duplicateIdentity);		//expectGroup.getUserMembers().remove(duplicateIdentity);
	assertGroupsEqual(expectGroup, actualGroup);		assertGroupsEqual(expectGroup, actualGroup);

	expectGroup.getUserMembers().remove(cadcDaoTest2_User);		expectGroup.getUserMembers().remove(cadcDaoTest2_User);
	@@ -205,7 +207,7 @@ public class LdapGroupDAOTest extends AbstractLdapDAOTest
	expectGroup.getUserAdmins().add(cadcDaoTest2_User);		expectGroup.getUserAdmins().add(cadcDaoTest2_User);
	expectGroup.getUserAdmins().add(duplicateIdentity);		expectGroup.getUserAdmins().add(duplicateIdentity);
	actualGroup = getGroupDAO().modifyGroup(expectGroup);		actualGroup = getGroupDAO().modifyGroup(expectGroup);
	expectGroup.getUserAdmins().remove(duplicateIdentity);		//expectGroup.getUserAdmins().remove(duplicateIdentity);
	assertGroupsEqual(expectGroup, actualGroup);		assertGroupsEqual(expectGroup, actualGroup);

	// delete the group		// delete the group
	@@ -443,9 +445,8 @@ public class LdapGroupDAOTest extends AbstractLdapDAOTest
	assertTrue(gr2.getGroupMembers().contains(gr));		assertTrue(gr2.getGroupMembers().contains(gr));
	}		}

			assertEquals(gr1.getUserMembers().size(), gr2.getUserMembers().size());
	assertEquals(gr1.getUserMembers(), gr2.getUserMembers());		assertEquals(gr1.getUserMembers(), gr2.getUserMembers());
	assertEquals(gr1.getUserMembers().size(), gr2.getUserMembers()
	.size());
	for (User user : gr1.getUserMembers())		for (User user : gr1.getUserMembers())
	{		{
	assertTrue(gr2.getUserMembers().contains(user));		assertTrue(gr2.getUserMembers().contains(user));

cadcAccessControl/src/ca/nrc/cadc/ac/Group.java

+18 −17

Original line number	Original line	Diff line number	Diff line
	@@ -71,6 +71,7 @@ package ca.nrc.cadc.ac;
	import java.util.Date;		import java.util.Date;
	import java.util.HashSet;		import java.util.HashSet;
	import java.util.Set;		import java.util.Set;
			import java.util.TreeSet;

	public class Group		public class Group
	{		{
	@@ -82,13 +83,13 @@ public class Group
	protected Set<GroupProperty> properties = new HashSet<GroupProperty>();		protected Set<GroupProperty> properties = new HashSet<GroupProperty>();

	// group's user members		// group's user members
	private Set<User> userMembers = new HashSet<User>();		private Set<User> userMembers = new TreeSet<User>();

	// group's group members		// group's group members
	private Set<Group> groupMembers = new HashSet<Group>();		private Set<Group> groupMembers = new HashSet<Group>();

	// group's user admins		// group's user admins
	private Set<User> userAdmins = new HashSet<User>();		private Set<User> userAdmins = new TreeSet<User>();

	// group's group admins		// group's group admins
	private Set<Group> groupAdmins = new HashSet<Group>();		private Set<Group> groupAdmins = new HashSet<Group>();

cadcAccessControl/src/ca/nrc/cadc/ac/InternalID.java

+36 −0

Original line number	Original line	Diff line number	Diff line
	@@ -94,10 +94,46 @@ public class InternalID
	throw new IllegalArgumentException("uri is null");		throw new IllegalArgumentException("uri is null");
	}		}

			if (uri.getFragment() != null)
			{
			throw new IllegalArgumentException("fragment not allowed");
			}

	this.uri = uri;		this.uri = uri;
	uuid = UUID.fromString(uri.getQuery());		uuid = UUID.fromString(uri.getQuery());
	}		}

			/**
			* Ctor
			* @param uri unique identifier
			* @param id The uuid of the identifier
			*/
			public InternalID(URI uri, UUID id)
			{
			if (uri == null)
			{
			throw new IllegalArgumentException("uri is null");
			}

			if (id == null)
			{
			throw new IllegalArgumentException("id is null");
			}

			if (uri.getQuery() != null)
			{
			throw new IllegalArgumentException("query not allowed in base uri");
			}

			if (uri.getFragment() != null)
			{
			throw new IllegalArgumentException("fragment not allowed");
			}

			this.uri = URI.create(uri.toASCIIString() + "?" + id.toString());
			this.uuid = id;
			}

	public URI getURI()		public URI getURI()
	{		{
	return uri;		return uri;

cadcAccessControl/src/ca/nrc/cadc/ac/User.java

+124 −98

Original line number	Original line	Diff line number	Diff line
	@@ -68,25 +68,23 @@
	*/		*/
	package ca.nrc.cadc.ac;		package ca.nrc.cadc.ac;

	import java.io.PrintWriter;
	import java.security.Principal;		import java.security.Principal;
	import java.util.Comparator;		import java.util.Comparator;
	import java.util.Date;		import java.util.Date;
	import java.util.HashSet;
	import java.util.Set;		import java.util.Set;
			import java.util.SortedSet;
	import java.util.TreeSet;		import java.util.TreeSet;

	import ca.nrc.cadc.auth.HttpPrincipal;

	import javax.security.auth.x500.X500Principal;		import javax.security.auth.x500.X500Principal;

	public class User		import ca.nrc.cadc.auth.AuthenticationUtil;
			import ca.nrc.cadc.auth.HttpPrincipal;

			public class User implements Comparable<User>
	{		{
	// How on God's green earth is this used? Where is it set?
	// jenkinsd 2016.03.24
	private InternalID id;		private InternalID id;

	private Set<Principal> identities = new TreeSet<Principal>(new PrincipalComparator());		private SortedSet<Principal> identities = new TreeSet<Principal>(new PrincipalComparator());

	public PersonalDetails personalDetails;		public PersonalDetails personalDetails;
	public PosixDetails posixDetails;		public PosixDetails posixDetails;
	@@ -120,15 +118,12 @@ public class User
	*/		*/
	public <S extends Principal> Set<S> getIdentities(final Class<S> identityClass)		public <S extends Principal> Set<S> getIdentities(final Class<S> identityClass)
	{		{
	final Set<S> matchedIdentities = new HashSet<S>();		final Set<S> matchedIdentities = new TreeSet<S>(new PrincipalComparator());

	for (final Principal p : identities)		for (final Principal p : identities)
	{		{
	if (p.getClass() == identityClass)		if (identityClass.isAssignableFrom(p.getClass()))
	{		{
	// 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);		matchedIdentities.add((S) p);
	}		}
	}		}
	@@ -146,11 +141,18 @@ public class User
	return null;		return null;
	}		}

			/**
			* @deprecated
			*/
	public X500Principal getX500Principal()		public X500Principal getX500Principal()
	{		{
	final Set<X500Principal> identities =		final Set<X500Principal> identities =
	getIdentities(X500Principal.class);		getIdentities(X500Principal.class);
	return identities.isEmpty() ? null : identities.iterator().next();		if (!identities.isEmpty())
			{
			return identities.iterator().next();
			}
			return null;
	}		}


	@@ -158,90 +160,104 @@ public class User
	* A User is considered consistent if the User's set of identities are a superset		* A User is considered consistent if the User's set of identities are a superset
	* of this Users set of identities.		* of this Users set of identities.
	*		*
	* @param other		* @param superset
	* @return		* @return
	*/		*/
	public boolean isConsistent(final User other)		public boolean isConsistent(final User superset)
	{		{
	if (other == null)		if (superset == null)
	{		{
	return false;		return false;
	}		}

	for (Principal identity: getIdentities())		if (this.identities.isEmpty() \|\| superset.identities.isEmpty())
	{
	boolean found = false;
	for (Principal op: other.getIdentities())
	{
	if (op.equals(identity))
	{
	found = true;
	break;
	}
	}
	if (!found)
	{		{
	return false;		return false;
	}		}
	}
	return true;
	}

	/* (non-Javadoc)		return superset.getIdentities().containsAll(this.getIdentities());
	* @see java.lang.Object#hashCode()
	*/		// // could be improved because both sets are ordered
	@Override		// for (Principal identity: getIdentities())
	public int hashCode()		// {
	{		// boolean found = false;
	int prime = 31;		// for (Principal op: superset.getIdentities())
	int result = 1;		// {
	if (id != null)		// if (AuthenticationUtil.equals(op, identity))
	{		// {
	result = prime * result + id.hashCode();		// found = true;
	}		// break;
	else		// }
	{		// }
	for (Principal principal : getIdentities())		// if (!found)
	{		// {
	result = prime * result + principal.hashCode();		// return false;
	}		// }
	}		// }
	return result;		// return true;
	}		}

			// /* (non-Javadoc)
			// * @see java.lang.Object#hashCode()
			// */
			// @Override
			// public int hashCode()
			// {
			// int prime = 31;
			// int result = 1;
			// if (id != null)
			// {
			// result = prime * result + id.hashCode();
			// }
			// else
			// {
			// for (Principal principal : getIdentities())
			// {
			// result = prime * result + principal.hashCode();
			// }
			// }
			// return result;
			// }

	/* (non-Javadoc)		/* (non-Javadoc)
	* @see java.lang.Object#equals(java.lang.Object)		* @see java.lang.Object#equals(java.lang.Object)
	*/		*/
	@Override		@Override
	public boolean equals(Object obj)		public boolean equals(Object obj)
	{		{
	if (this == obj)		if (obj instanceof User)
	{
	return true;
	}
	if (obj == null)
	{
	return false;
	}
	if (!(obj instanceof User))
	{
	return false;
	}
	User other = (User) obj;
	if (this.id == null && other.id == null)
	{
	return isConsistent(other);
	}
	if ((this.id == null && other.id != null) \|\|
	(this.id != null && other.id == null))
	{
	return false;
	}
	if (id.equals(other.id))
	{		{
	return true;		User user = (User) obj;
			return (this.isConsistent(user) \|\| user.isConsistent(this));
	}		}
	return false;		return false;
			// if (this == obj)
			// {
			// return true;
			// }
			// if (obj == null)
			// {
			// return false;
			// }
			// if (!(obj instanceof User))
			// {
			// return false;
			// }
			// User other = (User) obj;
			// if (this.id == null && other.id == null)
			// {
			// return isConsistent(other);
			// }
			// if ((this.id == null && other.id != null) \|\|
			// (this.id != null && other.id == null))
			// {
			// return false;
			// }
			// if (id.equals(other.id))
			// {
			// return true;
			// }
			// return false;
	}		}

	@Override		@Override
	@@ -263,32 +279,42 @@ public class User
	@Override		@Override
	public int compare(Principal o1, Principal o2)		public int compare(Principal o1, Principal o2)
	{		{
	int ret = -1;		if (o1 == null \|\| o2 == null)
	if (o1 == null && o2 == null)
	{		{
	ret = 0;		throw new IllegalArgumentException("Cannot compare null objects");
	}		}
	else if (o1 == null && o2 != null)
			if (o1 instanceof HttpPrincipal && o2 instanceof HttpPrincipal)
	{		{
	ret = 1;		return 0;
	}		}
	else if (o1 != null && o2 == null)
	{		return AuthenticationUtil.compare(o1, o2);
	ret = -1;
	}		}
	else if (o1 instanceof HttpPrincipal && o2 instanceof HttpPrincipal)
	{
	ret = 0;
	}		}
	else if (o1.getClass() == o2.getClass())
			@Override
			public int compareTo(User other)
	{		{
	if (o1.getName().equals(o2.getName()))		if (other == null)
	{		{
	ret = 0;		throw new IllegalArgumentException("Cannot compare null objects");
	}		}

			if (this.getIdentities().isEmpty() \|\| other.getIdentities().isEmpty())
			{
			throw new IllegalArgumentException("Users need identities for comparison.");
	}		}
	return ret;
			if (this.isConsistent(other) \|\| other.isConsistent(this))
			{
			return 0;
	}		}

			// compare the first pricipals in the order set
			Principal p1 = this.getIdentities().iterator().next();
			Principal p2 = other.getIdentities().iterator().next();
			return AuthenticationUtil.compare(p1, p2);
	}		}

	}		}