You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@syncope.apache.org by il...@apache.org on 2020/11/05 11:17:04 UTC
[syncope] branch master updated: [SYNCOPE-1598] Raise error in case
of 2+ memberships for same group
This is an automated email from the ASF dual-hosted git repository.
ilgrosso pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/syncope.git
The following commit(s) were added to refs/heads/master by this push:
new 501375b [SYNCOPE-1598] Raise error in case of 2+ memberships for same group
501375b is described below
commit 501375b0f8b2d060e86539ef629d66bcf2e79aa3
Author: Francesco Chicchiriccò <il...@apache.org>
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 | 173 +++++++++++++--------
.../provisioning/java/data/UserDataBinderImpl.java | 46 ++++++
.../apache/syncope/fit/core/MembershipITCase.java | 40 +++++
3 files changed, 195 insertions(+), 64 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 f1126fb..709d4ab 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
- anyObjectCR.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<>();
+ anyObjectCR.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
- anyObjectCR.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<>();
+ anyObjectCR.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, anyObjectCR, anyUtils, scce);
+ fill(anyObject, anyObjectCR, 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,8 +289,10 @@ public class AnyObjectDataBinderImpl extends AbstractAnyDataBinder implements An
propByRes.merge(fill(anyObject, anyObjectUR, anyUtils, scce));
// relationships
+ Set<Pair<String, String>> relationships = new HashSet<>();
anyObjectUR.getRelationships().stream().
- filter(patch -> patch.getRelationshipTO() != null).forEachOrdered((patch) -> {
+ filter(patch -> patch.getRelationshipTO() != null).forEach(patch -> {
+
RelationshipType relationshipType = relationshipTypeDAO.find(patch.getRelationshipTO().getType());
if (relationshipType == null) {
LOG.debug("Ignoring invalid relationship type {}", patch.getRelationshipTO().getType());
@@ -296,7 +317,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);
@@ -337,6 +372,7 @@ public class AnyObjectDataBinderImpl extends AbstractAnyDataBinder implements An
SyncopeClientException invalidValues = SyncopeClientException.build(ClientExceptionType.InvalidValues);
// memberships
+ Set<String> groups = new HashSet<>();
anyObjectUR.getMemberships().stream().filter(patch -> patch.getGroup() != null).forEach(patch -> {
anyObject.getMembership(patch.getGroup()).ifPresent(membership -> {
anyObject.remove(membership);
@@ -361,7 +397,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 fe108f8..2d015d7 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
@@ -268,11 +268,22 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat
user.setRealm(realm);
// relationships
+ Set<Pair<String, String>> relationships = new HashSet<>();
userCR.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());
@@ -296,6 +307,7 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat
});
// memberships
+ Set<String> groups = new HashSet<>();
userCR.getMemberships().forEach(membershipTO -> {
Group group = membershipTO.getGroupKey() == null
? groupDAO.findByName(membershipTO.getGroupName())
@@ -303,7 +315,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);
@@ -451,6 +472,7 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat
propByRes.merge(fill(user, userUR, anyUtils, scce));
// relationships
+ Set<Pair<String, String>> relationships = new HashSet<>();
userUR.getRelationships().stream().filter(patch -> patch.getRelationshipTO() != null).forEach(patch -> {
RelationshipType relationshipType = relationshipTypeDAO.find(patch.getRelationshipTO().getType());
if (relationshipType == null) {
@@ -466,7 +488,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);
@@ -505,6 +541,7 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat
SyncopeClientException invalidValues = SyncopeClientException.build(ClientExceptionType.InvalidValues);
// memberships
+ Set<String> groups = new HashSet<>();
userUR.getMemberships().stream().filter(patch -> patch.getGroup() != null).forEach(patch -> {
user.getMembership(patch.getGroup()).ifPresent(membership -> {
user.remove(membership);
@@ -530,7 +567,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 ad99dca..ecb61a2 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
@@ -34,6 +34,9 @@ import org.apache.syncope.common.lib.request.MembershipUR;
import org.apache.syncope.common.lib.request.UserCR;
import org.apache.syncope.common.lib.request.UserUR;
import org.apache.syncope.common.lib.Attr;
+import org.apache.syncope.common.lib.request.AnyObjectCR;
+import org.apache.syncope.common.lib.request.AnyObjectUR;
+import org.apache.syncope.common.lib.to.AnyObjectTO;
import org.apache.syncope.common.lib.to.ExecTO;
import org.apache.syncope.common.lib.to.GroupTO;
import org.apache.syncope.common.lib.to.ItemTO;
@@ -301,4 +304,41 @@ public class MembershipITCase extends AbstractITCase {
resourceService.delete(newResource.getKey());
}
}
+
+ @Test
+ public void createDoubleMembership() {
+ AnyObjectCR anyObjectCR = AnyObjectITCase.getSample("createDoubleMembership");
+ anyObjectCR.setRealm("/even/two");
+ anyObjectCR.getMemberships().add(new MembershipTO.Builder("034740a9-fa10-453b-af37-dc7897e98fb1").build());
+ anyObjectCR.getMemberships().add(new MembershipTO.Builder("034740a9-fa10-453b-af37-dc7897e98fb1").build());
+
+ try {
+ createAnyObject(anyObjectCR);
+ fail("This should not happen");
+ } catch (SyncopeClientException e) {
+ assertEquals(ClientExceptionType.InvalidMembership, e.getType());
+ }
+ }
+
+ @Test
+ public void updateDoubleMembership() {
+ AnyObjectCR anyObjecCR = AnyObjectITCase.getSample("update");
+ anyObjecCR.setRealm("/even/two");
+ AnyObjectTO anyObjecTO = createAnyObject(anyObjecCR).getEntity();
+ assertNotNull(anyObjecTO.getKey());
+
+ AnyObjectUR req = new AnyObjectUR();
+ req.setKey(anyObjecTO.getKey());
+ req.getMemberships().add(new MembershipUR.Builder("034740a9-fa10-453b-af37-dc7897e98fb1").build());
+ MembershipUR mp = new MembershipUR.Builder("034740a9-fa10-453b-af37-dc7897e98fb1").build();
+ mp.getPlainAttrs().add(attr("any", "useless"));
+ req.getMemberships().add(mp);
+
+ try {
+ updateAnyObject(req).getEntity();
+ fail("This should not happen");
+ } catch (SyncopeClientException e) {
+ assertEquals(ClientExceptionType.InvalidMembership, e.getType());
+ }
+ }
}