Merged 5.2.N (5.2.2) to HEAD (5.2)

134777 rmunteanu: REPO-1884: Don't allow adding a sub-group with memberType:"PERSON"
      - Added validation and tests


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@137358 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261
This commit is contained in:
Andrei Rebegea
2017-06-14 16:59:16 +00:00
parent c691e82867
commit 33c862b455
2 changed files with 49 additions and 8 deletions

View File

@@ -623,6 +623,12 @@ public class GroupsImpl implements Groups
throw new EntityNotFoundException("Group member with id " + groupMember.getId() + " does not exists"); throw new EntityNotFoundException("Group member with id " + groupMember.getId() + " does not exists");
} }
AuthorityType existingAuthorityType = AuthorityType.getAuthorityType(groupMember.getId());
if (existingAuthorityType != authorityType)
{
throw new IllegalArgumentException("Incorrect group member type, " + existingAuthorityType + " exists with the given id");
}
authorityService.addAuthority(groupId, groupMember.getId()); authorityService.addAuthority(groupId, groupMember.getId());
String authority = authorityService.getName(authorityType, groupMember.getId()); String authority = authorityService.getName(authorityType, groupMember.getId());
@@ -728,7 +734,7 @@ public class GroupsImpl implements Groups
throw new EntityNotFoundException(groupId); throw new EntityNotFoundException(groupId);
} }
} }
private void validateGroup(Group group, boolean isUpdate) private void validateGroup(Group group, boolean isUpdate)
{ {
if (group == null) if (group == null)

View File

@@ -956,14 +956,14 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
people.create(personAlice); people.create(personAlice);
} }
GroupMember personMember = new GroupMember();
personMember.setId(personAlice.getId());
personMember.setMemberType(MEMBER_TYPE_PERSON);
// +ve tests // +ve tests
// Create a group membership (for a existing person and a sub-group) // Create a group membership (for a existing person and a sub-group)
// within a group groupId // within a group groupId
{ {
GroupMember personMember = new GroupMember();
personMember.setId(personAlice.getId());
personMember.setMemberType(MEMBER_TYPE_PERSON);
// Add person as groupB member // Add person as groupB member
groupsProxy.createGroupMember(groupB.getId(), personMember, HttpServletResponse.SC_CREATED); groupsProxy.createGroupMember(groupB.getId(), personMember, HttpServletResponse.SC_CREATED);
// Add group as groupB sub-group // Add group as groupB sub-group
@@ -986,6 +986,13 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
assertFalse("Group was expected to be sub-group.", subGroup.getIsRoot()); assertFalse("Group was expected to be sub-group.", subGroup.getIsRoot());
} }
// -ve tests
// Id clashes with an existing group member
{
//Add a group member that has been already added
groupsProxy.createGroupMember(groupB.getId(), groupMemberA, HttpServletResponse.SC_CONFLICT);
}
// Person or group with given id does not exists // Person or group with given id does not exists
{ {
GroupMember invalidIdGroupMember = new GroupMember(); GroupMember invalidIdGroupMember = new GroupMember();
@@ -1014,12 +1021,40 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
groupsProxy.createGroupMember(groupA.getId(), invalidGroupMember, HttpServletResponse.SC_BAD_REQUEST); groupsProxy.createGroupMember(groupA.getId(), invalidGroupMember, HttpServletResponse.SC_BAD_REQUEST);
} }
// -ve tests // Validation tests
// Add group with non-admin user {
// Add group as groupB sub-group with member id null
personMember.setId(null);
groupsProxy.createGroupMember(groupB.getId(), personMember, HttpServletResponse.SC_BAD_REQUEST);
// Add group as groupB sub-group with member display name null
personMember.setDisplayName(null);
groupsProxy.createGroupMember(groupB.getId(), personMember, HttpServletResponse.SC_BAD_REQUEST);
// Add group as groupB sub-group with member type null
personMember.setMemberType(null);
groupsProxy.createGroupMember(groupB.getId(), personMember, HttpServletResponse.SC_BAD_REQUEST);
}
// Add group member with a different type from the existing one
{
// Add person as groupB member with member type GROUP
personMember.setMemberType(MEMBER_TYPE_GROUP);
groupsProxy.createGroupMember(groupB.getId(), personMember, HttpServletResponse.SC_BAD_REQUEST);
// Add group as groupB sub-group with member type PERSON
groupMemberA.setMemberType(MEMBER_TYPE_PERSON);
groupsProxy.createGroupMember(groupB.getId(), groupMemberA, HttpServletResponse.SC_BAD_REQUEST);
}
// User does not have admin permission to create a group membership
{ {
setRequestContext(user1); setRequestContext(user1);
groupsProxy.createGroupMember(groupA.getId(), groupMemberA, HttpServletResponse.SC_FORBIDDEN); groupsProxy.createGroupMember(groupB.getId(), groupMemberB, HttpServletResponse.SC_FORBIDDEN);
} }
//Authentication failed
{
setRequestContext(networkOne.getId(), GUID.generate(), "password");
groupsProxy.createGroupMember(groupB.getId(), groupMemberB, HttpServletResponse.SC_UNAUTHORIZED);
}
} }
finally finally
{ {