This is an automated email from the ASF dual-hosted git repository.
ilgrosso pushed a commit to branch 2_1_X
in repository https://gitbox.apache.org/repos/asf/syncope.git
The following commit(s) were added to refs/heads/2_1_X by this push:
new 9d02943 [SYNCOPE-1598] Raise error in case of 2+ memberships for same
group
9d02943 is described below
commit 9d02943ed8676916584f8a40ef384526c1b8f72f
Author: Francesco Chicchiriccò <[email protected]>
AuthorDate: Thu Nov 5 11:44:53 2020 +0100
[SYNCOPE-1598] Raise error in case of 2+ memberships for same group
---
.../java/data/AnyObjectDataBinderImpl.java | 170 +++++++++++++--------
.../provisioning/java/data/UserDataBinderImpl.java | 46 ++++++
.../apache/syncope/fit/core/MembershipITCase.java | 42 +++++
3 files changed, 195 insertions(+), 63 deletions(-)
diff --git
a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AnyObjectDataBinderImpl.java
b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AnyObjectDataBinderImpl.java
index bf058df..f0547fb 100644
---
a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AnyObjectDataBinderImpl.java
+++
b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AnyObjectDataBinderImpl.java
@@ -162,77 +162,96 @@ public class AnyObjectDataBinderImpl extends
AbstractAnyDataBinder implements An
}
anyObject.setRealm(realm);
- AnyUtils anyUtils =
anyUtilsFactory.getInstance(AnyTypeKind.ANY_OBJECT);
- if (anyObject.getRealm() != null) {
- // relationships
- anyObjectTO.getRelationships().forEach(relationshipTO -> {
- if (StringUtils.isBlank(relationshipTO.getOtherEndType())
- ||
AnyTypeKind.USER.name().equals(relationshipTO.getOtherEndType())
- ||
AnyTypeKind.GROUP.name().equals(relationshipTO.getOtherEndType())) {
-
- SyncopeClientException invalidAnyType =
-
SyncopeClientException.build(ClientExceptionType.InvalidAnyType);
-
invalidAnyType.getElements().add(AnyType.class.getSimpleName()
- + " not allowed for relationship: " +
relationshipTO.getOtherEndType());
- scce.addException(invalidAnyType);
- } else {
- AnyObject otherEnd =
anyObjectDAO.find(relationshipTO.getOtherEndKey());
- if (otherEnd == null) {
- LOG.debug("Ignoring invalid anyObject " +
relationshipTO.getOtherEndKey());
- } else if
(anyObject.getRealm().getFullPath().startsWith(otherEnd.getRealm().getFullPath()))
{
- RelationshipType relationshipType =
relationshipTypeDAO.find(relationshipTO.getType());
- if (relationshipType == null) {
- LOG.debug("Ignoring invalid relationship type {}",
relationshipTO.getType());
- } else {
- ARelationship relationship =
entityFactory.newEntity(ARelationship.class);
- relationship.setType(relationshipType);
- relationship.setRightEnd(otherEnd);
- relationship.setLeftEnd(anyObject);
-
- anyObject.add(relationship);
- }
+ // relationships
+ Set<Pair<String, String>> relationships = new HashSet<>();
+ anyObjectTO.getRelationships().forEach(relationshipTO -> {
+ if (StringUtils.isBlank(relationshipTO.getOtherEndType())
+ ||
AnyTypeKind.USER.name().equals(relationshipTO.getOtherEndType())
+ ||
AnyTypeKind.GROUP.name().equals(relationshipTO.getOtherEndType())) {
+
+ SyncopeClientException invalidAnyType =
+
SyncopeClientException.build(ClientExceptionType.InvalidAnyType);
+ invalidAnyType.getElements().add(AnyType.class.getSimpleName()
+ + " not allowed for relationship: " +
relationshipTO.getOtherEndType());
+ scce.addException(invalidAnyType);
+ } else {
+ AnyObject otherEnd =
anyObjectDAO.find(relationshipTO.getOtherEndKey());
+ if (otherEnd == null) {
+ LOG.debug("Ignoring invalid anyObject " +
relationshipTO.getOtherEndKey());
+ } else if (relationships.contains(Pair.of(otherEnd.getKey(),
relationshipTO.getType()))) {
+ LOG.error("{} was already in relationship {} with {}",
+ otherEnd, relationshipTO.getType(), anyObject);
+
+ SyncopeClientException assigned =
+
SyncopeClientException.build(ClientExceptionType.InvalidRelationship);
+ assigned.getElements().add(otherEnd.getType().getKey() + "
" + otherEnd.getName()
+ + " in relationship " + relationshipTO.getType());
+ scce.addException(assigned);
+ } else if
(anyObject.getRealm().getFullPath().startsWith(otherEnd.getRealm().getFullPath()))
{
+ relationships.add(Pair.of(otherEnd.getKey(),
relationshipTO.getType()));
+
+ RelationshipType relationshipType =
relationshipTypeDAO.find(relationshipTO.getType());
+ if (relationshipType == null) {
+ LOG.debug("Ignoring invalid relationship type {}",
relationshipTO.getType());
} else {
- LOG.error("{} cannot be related to {}", otherEnd,
anyObject);
+ ARelationship relationship =
entityFactory.newEntity(ARelationship.class);
+ relationship.setType(relationshipType);
+ relationship.setRightEnd(otherEnd);
+ relationship.setLeftEnd(anyObject);
- SyncopeClientException unrelatable =
-
SyncopeClientException.build(ClientExceptionType.InvalidRelationship);
-
unrelatable.getElements().add(otherEnd.getType().getKey() + " " +
otherEnd.getName()
- + " cannot be related");
- scce.addException(unrelatable);
+ anyObject.add(relationship);
}
- }
- });
-
- // memberships
- anyObjectTO.getMemberships().forEach(membershipTO -> {
- Group group = membershipTO.getGroupKey() == null
- ? groupDAO.findByName(membershipTO.getGroupName())
- : groupDAO.find(membershipTO.getGroupKey());
- if (group == null) {
- LOG.debug("Ignoring invalid group "
- + membershipTO.getGroupKey() + " / " +
membershipTO.getGroupName());
- } else if
(anyObject.getRealm().getFullPath().startsWith(group.getRealm().getFullPath()))
{
- AMembership membership =
entityFactory.newEntity(AMembership.class);
- membership.setRightEnd(group);
- membership.setLeftEnd(anyObject);
-
- anyObject.add(membership);
-
- // membership attributes
- fill(anyObject, membership, membershipTO, anyUtils, scce);
} else {
- LOG.error("{} cannot be assigned to {}", group, anyObject);
+ LOG.error("{} cannot be related to {}", otherEnd,
anyObject);
- SyncopeClientException unassignable =
-
SyncopeClientException.build(ClientExceptionType.InvalidMembership);
- unassignable.getElements().add("Group " + group.getName()
+ " cannot be assigned");
- scce.addException(unassignable);
+ SyncopeClientException unrelatable =
+
SyncopeClientException.build(ClientExceptionType.InvalidRelationship);
+ unrelatable.getElements().add(otherEnd.getType().getKey()
+ " " + otherEnd.getName()
+ + " cannot be related");
+ scce.addException(unrelatable);
}
- });
- }
+ }
+ });
+
+ // memberships
+ Set<String> groups = new HashSet<>();
+ anyObjectTO.getMemberships().forEach(membershipTO -> {
+ Group group = membershipTO.getGroupKey() == null
+ ? groupDAO.findByName(membershipTO.getGroupName())
+ : groupDAO.find(membershipTO.getGroupKey());
+ if (group == null) {
+ LOG.debug("Ignoring invalid group "
+ + membershipTO.getGroupKey() + " / " +
membershipTO.getGroupName());
+ } else if (groups.contains(group.getKey())) {
+ LOG.error("{} was already assigned to {}", group, anyObject);
+
+ SyncopeClientException assigned =
+
SyncopeClientException.build(ClientExceptionType.InvalidMembership);
+ assigned.getElements().add("Group " + group.getName() + " was
already assigned");
+ scce.addException(assigned);
+ } else if
(anyObject.getRealm().getFullPath().startsWith(group.getRealm().getFullPath()))
{
+ groups.add(group.getKey());
+
+ AMembership membership =
entityFactory.newEntity(AMembership.class);
+ membership.setRightEnd(group);
+ membership.setLeftEnd(anyObject);
+
+ anyObject.add(membership);
+
+ // membership attributes
+ fill(anyObject, membership, membershipTO,
anyUtilsFactory.getInstance(AnyTypeKind.ANY_OBJECT), scce);
+ } else {
+ LOG.error("{} cannot be assigned to {}", group, anyObject);
+
+ SyncopeClientException unassignable =
+
SyncopeClientException.build(ClientExceptionType.InvalidMembership);
+ unassignable.getElements().add("Group " + group.getName() + "
cannot be assigned");
+ scce.addException(unassignable);
+ }
+ });
// attributes and resources
- fill(anyObject, anyObjectTO, anyUtils, scce);
+ fill(anyObject, anyObjectTO,
anyUtilsFactory.getInstance(AnyTypeKind.ANY_OBJECT), scce);
// Throw composite exception if there is at least one element set in
the composing exceptions
if (scce.hasExceptions()) {
@@ -270,6 +289,7 @@ public class AnyObjectDataBinderImpl extends
AbstractAnyDataBinder implements An
propByRes.merge(fill(anyObject, anyObjectPatch, anyUtils, scce));
// relationships
+ Set<Pair<String, String>> relationships = new HashSet<>();
anyObjectPatch.getRelationships().stream().
filter(patch -> patch.getRelationshipTO() !=
null).forEachOrdered((patch) -> {
RelationshipType relationshipType =
relationshipTypeDAO.find(patch.getRelationshipTO().getType());
@@ -296,7 +316,21 @@ public class AnyObjectDataBinderImpl extends
AbstractAnyDataBinder implements An
AnyObject otherEnd =
anyObjectDAO.find(patch.getRelationshipTO().getOtherEndKey());
if (otherEnd == null) {
LOG.debug("Ignoring invalid any object {}",
patch.getRelationshipTO().getOtherEndKey());
+ } else if (relationships.contains(
+ Pair.of(otherEnd.getKey(),
patch.getRelationshipTO().getType()))) {
+
+ LOG.error("{} was already in relationship {} with
{}",
+ anyObject,
patch.getRelationshipTO().getType(), otherEnd);
+
+ SyncopeClientException assigned =
+
SyncopeClientException.build(ClientExceptionType.InvalidRelationship);
+ assigned.getElements().add("AnyObject was already
in relationship "
+ + patch.getRelationshipTO().getType() + "
with "
+ + otherEnd.getType().getKey() + " " +
otherEnd.getName());
+ scce.addException(assigned);
} else if
(anyObject.getRealm().getFullPath().startsWith(otherEnd.getRealm().getFullPath()))
{
+ relationships.add(Pair.of(otherEnd.getKey(),
patch.getRelationshipTO().getType()));
+
ARelationship newRelationship =
entityFactory.newEntity(ARelationship.class);
newRelationship.setType(relationshipType);
newRelationship.setRightEnd(otherEnd);
@@ -339,6 +373,7 @@ public class AnyObjectDataBinderImpl extends
AbstractAnyDataBinder implements An
SyncopeClientException invalidValues =
SyncopeClientException.build(ClientExceptionType.InvalidValues);
// memberships
+ Set<String> groups = new HashSet<>();
anyObjectPatch.getMemberships().stream().filter(patch ->
patch.getGroup() != null).forEach(patch -> {
anyObject.getMembership(patch.getGroup()).ifPresent(membership -> {
anyObject.remove(membership);
@@ -363,7 +398,16 @@ public class AnyObjectDataBinderImpl extends
AbstractAnyDataBinder implements An
Group group = groupDAO.find(patch.getGroup());
if (group == null) {
LOG.debug("Ignoring invalid group {}", patch.getGroup());
+ } else if (groups.contains(group.getKey())) {
+ LOG.error("Multiple patches for group {} of {} were
found", group, anyObject);
+
+ SyncopeClientException assigned =
+
SyncopeClientException.build(ClientExceptionType.InvalidMembership);
+ assigned.getElements().add("Multiple patches for group " +
group.getName() + " were found");
+ scce.addException(assigned);
} else if
(anyObject.getRealm().getFullPath().startsWith(group.getRealm().getFullPath()))
{
+ groups.add(group.getKey());
+
AMembership newMembership =
entityFactory.newEntity(AMembership.class);
newMembership.setRightEnd(group);
newMembership.setLeftEnd(anyObject);
diff --git
a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/UserDataBinderImpl.java
b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/UserDataBinderImpl.java
index 771c3df..58e8a93 100644
---
a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/UserDataBinderImpl.java
+++
b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/UserDataBinderImpl.java
@@ -267,11 +267,22 @@ public class UserDataBinderImpl extends
AbstractAnyDataBinder implements UserDat
user.setRealm(realm);
// relationships
+ Set<Pair<String, String>> relationships = new HashSet<>();
userTO.getRelationships().forEach(relationshipTO -> {
AnyObject otherEnd =
anyObjectDAO.find(relationshipTO.getOtherEndKey());
if (otherEnd == null) {
LOG.debug("Ignoring invalid anyObject " +
relationshipTO.getOtherEndKey());
+ } else if (relationships.contains(Pair.of(otherEnd.getKey(),
relationshipTO.getType()))) {
+ LOG.error("{} was already in relationship {} with {}",
otherEnd, relationshipTO.getType(), user);
+
+ SyncopeClientException assigned =
+
SyncopeClientException.build(ClientExceptionType.InvalidRelationship);
+ assigned.getElements().add(otherEnd.getType().getKey() + " " +
otherEnd.getName()
+ + " in relationship " + relationshipTO.getType());
+ scce.addException(assigned);
} else if
(user.getRealm().getFullPath().startsWith(otherEnd.getRealm().getFullPath())) {
+ relationships.add(Pair.of(otherEnd.getKey(),
relationshipTO.getType()));
+
RelationshipType relationshipType =
relationshipTypeDAO.find(relationshipTO.getType());
if (relationshipType == null) {
LOG.debug("Ignoring invalid relationship type {}",
relationshipTO.getType());
@@ -295,6 +306,7 @@ public class UserDataBinderImpl extends
AbstractAnyDataBinder implements UserDat
});
// memberships
+ Set<String> groups = new HashSet<>();
userTO.getMemberships().forEach(membershipTO -> {
Group group = membershipTO.getGroupKey() == null
? groupDAO.findByName(membershipTO.getGroupName())
@@ -302,7 +314,16 @@ public class UserDataBinderImpl extends
AbstractAnyDataBinder implements UserDat
if (group == null) {
LOG.debug("Ignoring invalid group {}",
membershipTO.getGroupKey() + " / " +
membershipTO.getGroupName());
+ } else if (groups.contains(group.getKey())) {
+ LOG.error("{} was already assigned to {}", group, user);
+
+ SyncopeClientException assigned =
+
SyncopeClientException.build(ClientExceptionType.InvalidMembership);
+ assigned.getElements().add("Group " + group.getName() + " was
already assigned");
+ scce.addException(assigned);
} else if
(user.getRealm().getFullPath().startsWith(group.getRealm().getFullPath())) {
+ groups.add(group.getKey());
+
UMembership membership =
entityFactory.newEntity(UMembership.class);
membership.setRightEnd(group);
membership.setLeftEnd(user);
@@ -450,6 +471,7 @@ public class UserDataBinderImpl extends
AbstractAnyDataBinder implements UserDat
propByRes.merge(fill(user, userPatch, anyUtils, scce));
// relationships
+ Set<Pair<String, String>> relationships = new HashSet<>();
userPatch.getRelationships().stream().filter(patch ->
patch.getRelationshipTO() != null).forEach(patch -> {
RelationshipType relationshipType =
relationshipTypeDAO.find(patch.getRelationshipTO().getType());
if (relationshipType == null) {
@@ -465,7 +487,21 @@ public class UserDataBinderImpl extends
AbstractAnyDataBinder implements UserDat
AnyObject otherEnd =
anyObjectDAO.find(patch.getRelationshipTO().getOtherEndKey());
if (otherEnd == null) {
LOG.debug("Ignoring invalid any object {}",
patch.getRelationshipTO().getOtherEndKey());
+ } else if (relationships.contains(
+ Pair.of(otherEnd.getKey(),
patch.getRelationshipTO().getType()))) {
+
+ LOG.error("{} was already in relationship {} with {}",
+ user, patch.getRelationshipTO().getType(),
otherEnd);
+
+ SyncopeClientException assigned =
+
SyncopeClientException.build(ClientExceptionType.InvalidRelationship);
+ assigned.getElements().add("User was already in
relationship "
+ + patch.getRelationshipTO().getType() + " with
"
+ + otherEnd.getType().getKey() + " " +
otherEnd.getName());
+ scce.addException(assigned);
} else if
(user.getRealm().getFullPath().startsWith(otherEnd.getRealm().getFullPath())) {
+ relationships.add(Pair.of(otherEnd.getKey(),
patch.getRelationshipTO().getType()));
+
URelationship newRelationship =
entityFactory.newEntity(URelationship.class);
newRelationship.setType(relationshipType);
newRelationship.setRightEnd(otherEnd);
@@ -507,6 +543,7 @@ public class UserDataBinderImpl extends
AbstractAnyDataBinder implements UserDat
SyncopeClientException invalidValues =
SyncopeClientException.build(ClientExceptionType.InvalidValues);
// memberships
+ Set<String> groups = new HashSet<>();
userPatch.getMemberships().stream().filter(patch -> patch.getGroup()
!= null).forEach(patch -> {
user.getMembership(patch.getGroup()).ifPresent(membership -> {
user.remove(membership);
@@ -532,7 +569,16 @@ public class UserDataBinderImpl extends
AbstractAnyDataBinder implements UserDat
Group group = groupDAO.find(patch.getGroup());
if (group == null) {
LOG.debug("Ignoring invalid group {}", patch.getGroup());
+ } else if (groups.contains(group.getKey())) {
+ LOG.error("Multiple patches for group {} of {} were
found", group, user);
+
+ SyncopeClientException assigned =
+
SyncopeClientException.build(ClientExceptionType.InvalidMembership);
+ assigned.getElements().add("Multiple patches for group " +
group.getName() + " were found");
+ scce.addException(assigned);
} else if
(user.getRealm().getFullPath().startsWith(group.getRealm().getFullPath())) {
+ groups.add(group.getKey());
+
UMembership newMembership =
entityFactory.newEntity(UMembership.class);
newMembership.setRightEnd(group);
newMembership.setLeftEnd(user);
diff --git
a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MembershipITCase.java
b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MembershipITCase.java
index 5e64d99..3066915 100644
---
a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MembershipITCase.java
+++
b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MembershipITCase.java
@@ -28,10 +28,12 @@ import javax.sql.DataSource;
import javax.ws.rs.core.Response;
import org.apache.syncope.client.lib.SyncopeClient;
import org.apache.syncope.common.lib.SyncopeClientException;
+import org.apache.syncope.common.lib.patch.AnyObjectPatch;
import org.apache.syncope.common.lib.patch.AttrPatch;
import org.apache.syncope.common.lib.patch.DeassociationPatch;
import org.apache.syncope.common.lib.patch.MembershipPatch;
import org.apache.syncope.common.lib.patch.UserPatch;
+import org.apache.syncope.common.lib.to.AnyObjectTO;
import org.apache.syncope.common.lib.to.AttrTO;
import org.apache.syncope.common.lib.to.ExecTO;
import org.apache.syncope.common.lib.to.GroupTO;
@@ -306,4 +308,44 @@ public class MembershipITCase extends AbstractITCase {
resourceService.delete(newResource.getKey());
}
}
+
+ @Test
+ public void createDoubleMembership() {
+ AnyObjectTO anyObjectTO =
AnyObjectITCase.getSampleTO("createDoubleMembership");
+ anyObjectTO.setRealm("/even/two");
+ anyObjectTO.getMemberships().add(
+ new
MembershipTO.Builder().group("034740a9-fa10-453b-af37-dc7897e98fb1").build());
+ anyObjectTO.getMemberships().add(
+ new
MembershipTO.Builder().group("034740a9-fa10-453b-af37-dc7897e98fb1").build());
+
+ try {
+ createAnyObject(anyObjectTO);
+ fail("This should not happen");
+ } catch (SyncopeClientException e) {
+ assertEquals(ClientExceptionType.InvalidMembership, e.getType());
+ }
+ }
+
+ @Test
+ public void updateDoubleMembership() {
+ AnyObjectTO anyObjectTO = AnyObjectITCase.getSampleTO("update");
+ anyObjectTO.setRealm("/even/two");
+ anyObjectTO = createAnyObject(anyObjectTO).getEntity();
+ assertNotNull(anyObjectTO.getKey());
+
+ AnyObjectPatch anyObjectPatch = new AnyObjectPatch();
+ anyObjectPatch.setKey(anyObjectTO.getKey());
+ anyObjectPatch.getMemberships().add(
+ new
MembershipPatch.Builder().group("034740a9-fa10-453b-af37-dc7897e98fb1").build());
+ MembershipPatch mp = new
MembershipPatch.Builder().group("034740a9-fa10-453b-af37-dc7897e98fb1").build();
+ mp.getPlainAttrs().add(attrTO("any", "useless"));
+ anyObjectPatch.getMemberships().add(mp);
+
+ try {
+ updateAnyObject(anyObjectPatch).getEntity();
+ fail("This should not happen");
+ } catch (SyncopeClientException e) {
+ assertEquals(ClientExceptionType.InvalidMembership, e.getType());
+ }
+ }
}
