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ò <[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 | 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());
+ }
+ }
}
