github-advanced-security[bot] commented on code in PR #1263:
URL: https://github.com/apache/syncope/pull/1263#discussion_r2619934774
##########
core/persistence-neo4j/src/main/java/org/apache/syncope/core/persistence/neo4j/entity/user/Neo4jUser.java:
##########
@@ -406,24 +408,30 @@
@Override
public boolean add(final URelationship relationship) {
checkType(relationship, Neo4jURelationship.class);
- return this.relationships.add((Neo4jURelationship) relationship);
+ return relationships.add((Neo4jURelationship) relationship);
}
@Override
- public List<? extends URelationship> getRelationships() {
+ public boolean remove(final
org.apache.syncope.core.persistence.api.entity.Relationship<?, ?> relationship)
{
Review Comment:
## Confusing overloading of methods
Method Neo4jUser.remove(..) could be confused with overloaded method
[remove](1), since dispatch depends on static types.
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2398)
##########
core/persistence-neo4j/src/main/java/org/apache/syncope/core/persistence/neo4j/entity/anyobject/Neo4jAnyObject.java:
##########
@@ -149,7 +150,13 @@
}
@Override
- public List<? extends ARelationship> getRelationships() {
+ public boolean remove(final
org.apache.syncope.core.persistence.api.entity.Relationship<?, ?> relationship)
{
Review Comment:
## Confusing overloading of methods
Method Neo4jAnyObject.remove(..) could be confused with overloaded method
[remove](1), since dispatch depends on static types.
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2397)
##########
core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPAUser.java:
##########
@@ -452,6 +453,13 @@
return this.relationships.add((JPAURelationship) relationship);
}
+ @Override
+ public boolean remove(final Relationship<?, ?> relationship) {
Review Comment:
## Confusing overloading of methods
Method JPAUser.remove(..) could be confused with overloaded method
[remove](1), since dispatch depends on static types.
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2396)
##########
core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/AllowedSchemas.java:
##########
@@ -21,82 +21,61 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
-import java.util.function.Predicate;
+import org.apache.syncope.core.persistence.api.entity.RelationshipType;
import org.apache.syncope.core.persistence.api.entity.Schema;
import org.apache.syncope.core.persistence.api.entity.group.Group;
public class AllowedSchemas<S extends Schema> {
- private final Set<S> forSelf = new HashSet<>();
+ private final Set<S> self = new HashSet<>();
- private final Map<Group, Set<S>> forMemberships = new HashMap<>();
+ private final Map<Group, Set<S>> memberships = new HashMap<>();
- public Set<S> getForSelf() {
- return forSelf;
- }
+ private final Map<RelationshipType, Set<S>> relationshipTypes = new
HashMap<>();
- public Set<S> getForMembership(final Group group) {
- return forMemberships.get(group) == null ? Set.of() :
forMemberships.get(group);
+ public Set<S> self() {
+ return self;
}
- public Map<Group, Set<S>> getForMemberships() {
- return forMemberships;
+ public Set<S> membership(final Group group) {
+ return Optional.ofNullable(memberships.get(group)).orElseGet(Set::of);
}
- public boolean forSelfContains(final S schema) {
- return forSelf.contains(schema);
+ public Map<Group, Set<S>> memberships() {
+ return memberships;
}
- public boolean forSelfContains(final String schema) {
- return forSelf.stream().anyMatch(new KeyMatches(schema));
+ public Set<S> relationshipType(final RelationshipType relationshipType) {
+ return
Optional.ofNullable(relationshipTypes.get(relationshipType)).orElseGet(Set::of);
}
- public boolean forMembershipsContains(final Group group, final S schema) {
- return getForMembership(group).stream().anyMatch(s ->
s.equals(schema));
+ public Map<RelationshipType, Set<S>> relationshipTypes() {
Review Comment:
## Exposing internal representation
relationshipTypes exposes the internal representation stored in field
relationshipTypes. The value may be modified [after this call to
relationshipTypes](1).
relationshipTypes exposes the internal representation stored in field
relationshipTypes. The value may be modified [after this call to
relationshipTypes](2).
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2387)
##########
common/idrepo/lib/src/main/java/org/apache/syncope/common/lib/to/RelationshipTO.java:
##########
@@ -114,6 +144,32 @@
this.end = end;
}
+ @JacksonXmlElementWrapper(localName = "plainAttrs")
+ @JacksonXmlProperty(localName = "plainAttr")
+ @Override
+ public Set<Attr> getPlainAttrs() {
+ return plainAttrs;
+ }
+
+ @JsonIgnore
+ @Override
+ public Optional<Attr> getPlainAttr(final String schema) {
+ return plainAttrs.stream().filter(attr ->
attr.getSchema().equals(schema)).findFirst();
+ }
+
+ @JacksonXmlElementWrapper(localName = "derAttrs")
+ @JacksonXmlProperty(localName = "derAttr")
+ @Override
+ public Set<Attr> getDerAttrs() {
Review Comment:
## Exposing internal representation
getDerAttrs exposes the internal representation stored in field derAttrs.
The value may be modified [after this call to getDerAttrs](1).
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2385)
##########
common/idrepo/lib/src/main/java/org/apache/syncope/common/lib/AnyOperations.java:
##########
@@ -468,6 +447,30 @@
return null;
}
+ private static void relationships(final Set<RelationshipUR> updateReqs,
final RelatableTO relatable) {
+ updateReqs.forEach(relPatch -> {
+ if (relPatch.getType() == null || relPatch.getOtherEndType() ==
null || relPatch.getOtherEndKey() == null) {
+ LOG.warn("Invalid {} specified: {}",
RelationshipUR.class.getName(), relPatch);
Review Comment:
## Use of default toString()
Default toString(): RelationshipUR inherits toString() from Object, and so
is not suitable for printing.
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2390)
##########
common/idrepo/lib/src/main/java/org/apache/syncope/common/lib/request/RelationshipUR.java:
##########
@@ -18,42 +18,99 @@
*/
package org.apache.syncope.common.lib.request;
+import
com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlElementWrapper;
+import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlProperty;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
import org.apache.commons.lang3.builder.EqualsBuilder;
import org.apache.commons.lang3.builder.HashCodeBuilder;
-import org.apache.syncope.common.lib.to.RelationshipTO;
+import org.apache.syncope.common.lib.Attr;
public class RelationshipUR extends AbstractPatch {
private static final long serialVersionUID = 1314175521205206511L;
public static class Builder extends AbstractPatch.Builder<RelationshipUR,
Builder> {
- public Builder(final RelationshipTO relationshipTO) {
+ public Builder(final String type) {
super();
- getInstance().setRelationshipTO(relationshipTO);
+ getInstance().setType(type);
}
@Override
protected RelationshipUR newInstance() {
return new RelationshipUR();
}
+
+ public Builder otherEnd(final String type, final String key) {
+ getInstance().setOtherEndType(type);
+ getInstance().setOtherEndKey(key);
+ return this;
+ }
+
+ public Builder plainAttr(final Attr plainAttr) {
+ getInstance().getPlainAttrs().add(plainAttr);
+ return this;
+ }
+
+ public Builder plainAttrs(final Attr... plainAttrs) {
+ getInstance().getPlainAttrs().addAll(List.of(plainAttrs));
+ return this;
+ }
+
+ public Builder plainAttrs(final Collection<Attr> plainAttrs) {
+ getInstance().getPlainAttrs().addAll(plainAttrs);
+ return this;
+ }
}
- private RelationshipTO relationshipTO;
+ private String type;
+
+ private String otherEndType;
+
+ private String otherEndKey;
+
+ private final Set<Attr> plainAttrs = new HashSet<>();
+
+ public String getType() {
+ return type;
+ }
+
+ public void setType(final String type) {
+ this.type = type;
+ }
+
+ public String getOtherEndType() {
+ return otherEndType;
+ }
+
+ public void setOtherEndType(final String otherEndType) {
+ this.otherEndType = otherEndType;
+ }
+
+ public String getOtherEndKey() {
+ return otherEndKey;
+ }
- public RelationshipTO getRelationshipTO() {
- return relationshipTO;
+ public void setOtherEndKey(final String otherEndKey) {
+ this.otherEndKey = otherEndKey;
}
- public void setRelationshipTO(final RelationshipTO relationshipTO) {
- this.relationshipTO = relationshipTO;
+ @JacksonXmlElementWrapper(localName = "plainAttrs")
+ @JacksonXmlProperty(localName = "plainAttr")
+ public Set<Attr> getPlainAttrs() {
Review Comment:
## Exposing internal representation
getPlainAttrs exposes the internal representation stored in field
plainAttrs. The value may be modified [after this call to getPlainAttrs](1).
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2383)
##########
core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/entity/AnyUtils.java:
##########
@@ -51,7 +52,11 @@
void removeAttr(String key, PlainSchema schema);
- void addRelationship(Relatable<?, ?> relatable, RelationshipType
relationshipType, AnyObject otherEnd);
+ Membership<?> add(Groupable<?, ?, ?> groupable, Group group);
- void removeRelationship(Relatable<?, ?> relatable, RelationshipType
relationshipType, String otherEndKey);
+ void remove(Groupable<?, ?, ?> groupable, Membership<?> membership);
Review Comment:
## Confusing overloading of methods
Method AnyUtils.remove(..) could be confused with overloaded method
[remove](1), since dispatch depends on static types.
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2391)
##########
core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/anyobject/JPAAnyObject.java:
##########
@@ -172,7 +173,14 @@
@Override
public boolean add(final ARelationship relationship) {
checkType(relationship, JPAARelationship.class);
- return this.relationships.add((JPAARelationship) relationship);
+ return relationships.add((JPAARelationship) relationship);
+ }
+
+ @Override
+ public boolean remove(final Relationship<?, ?> relationship) {
Review Comment:
## Confusing overloading of methods
Method JPAAnyObject.remove(..) could be confused with overloaded method
[remove](1), since dispatch depends on static types.
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2395)
##########
core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/RelationshipTypeDataBinderImpl.java:
##########
@@ -60,6 +76,50 @@
}
relationshipType.setDescription(relationshipTypeTO.getDescription());
+
+ // type extensions
+ relationshipTypeTO.getTypeExtensions().
+ forEach(typeExtTO ->
anyTypeDAO.findById(typeExtTO.getAnyType()).ifPresentOrElse(anyType -> {
+
+ RelationshipTypeExtension typeExt =
relationshipType.getTypeExtension(anyType).orElse(null);
+ if (typeExt == null) {
+ typeExt =
entityFactory.newEntity(RelationshipTypeExtension.class);
+ typeExt.setAnyType(anyType);
+ typeExt.setRelationshipType(relationshipType);
+ relationshipType.add(typeExt);
+ }
+
+ // add all classes contained in the TO
+ for (String key : typeExtTO.getAuxClasses()) {
+ AnyTypeClass anyTypeClass =
anyTypeClassDAO.findById(key).orElse(null);
+ if (anyTypeClass == null) {
+ LOG.warn("Ignoring invalid {}: {}",
AnyTypeClass.class.getSimpleName(), key);
+ } else {
+ typeExt.add(anyTypeClass);
+ }
+ }
+ // remove all classes not contained in the TO
+ typeExt.getAuxClasses().
+ removeIf(anyTypeClass ->
!typeExtTO.getAuxClasses().contains(anyTypeClass.getKey()));
+
+ // only consider non-empty type extensions
+ if (typeExt.getAuxClasses().isEmpty()) {
+ relationshipType.getTypeExtensions().remove(typeExt);
+ typeExt.setRelationshipType(null);
+ }
+
+ }, () -> LOG.warn("Ignoring invalid {}: {}",
AnyType.class.getSimpleName(), typeExtTO.getAnyType())));
Review Comment:
## Log Injection
This log entry depends on a [user-provided value](1).
This log entry depends on a [user-provided value](2).
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2382)
##########
core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/entity/Groupable.java:
##########
@@ -27,35 +26,25 @@
extends Any {
/**
- * Returns the plain attribute for this instance, the given schema name
and the given membership -
- * if found, {@code NULL} otherwise.
+ * Returns the plain attribute for this instance, the given schema name
and the given membership.
*
* @param plainSchema plain schema name
* @param membership membership
* @return plain attribute for this instance, the given schema name and
the given membership
*/
Optional<PlainAttr> getPlainAttr(String plainSchema, Membership<?>
membership);
- /**
- * Returns the list of plain attributes for this instance and the given
schema name (including membeship attributes,
- * as opposite to {@link Any#getPlainAttr(java.lang.String)}).
- *
- * @param plainSchema plain schema name
- * @return list of plain attributes for this instance and the given schema
name (including membeship attributes)
- */
- Collection<PlainAttr> getPlainAttrs(String plainSchema);
-
/**
* Returns the list of plain attributes for this instance and the given
membership.
*
* @param membership membership
* @return list of plain attributes for this instance and the given
membership
*/
- Collection<PlainAttr> getPlainAttrs(Membership<?> membership);
+ List<PlainAttr> getPlainAttrs(Membership<?> membership);
Review Comment:
## Confusing overloading of methods
Method Groupable.getPlainAttrs(..) could be confused with overloaded method
[Relatable.getPlainAttrs](1), since dispatch depends on static types.
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2393)
##########
core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/AllowedSchemas.java:
##########
@@ -21,82 +21,61 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
-import java.util.function.Predicate;
+import org.apache.syncope.core.persistence.api.entity.RelationshipType;
import org.apache.syncope.core.persistence.api.entity.Schema;
import org.apache.syncope.core.persistence.api.entity.group.Group;
public class AllowedSchemas<S extends Schema> {
- private final Set<S> forSelf = new HashSet<>();
+ private final Set<S> self = new HashSet<>();
- private final Map<Group, Set<S>> forMemberships = new HashMap<>();
+ private final Map<Group, Set<S>> memberships = new HashMap<>();
- public Set<S> getForSelf() {
- return forSelf;
- }
+ private final Map<RelationshipType, Set<S>> relationshipTypes = new
HashMap<>();
- public Set<S> getForMembership(final Group group) {
- return forMemberships.get(group) == null ? Set.of() :
forMemberships.get(group);
+ public Set<S> self() {
Review Comment:
## Exposing internal representation
self exposes the internal representation stored in field self. The value may
be modified [after this call to self](1).
self exposes the internal representation stored in field self. The value may
be modified [after this call to self](2).
self exposes the internal representation stored in field self. The value may
be modified [after this call to self](3).
self exposes the internal representation stored in field self. The value may
be modified [after this call to self](4).
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2389)
##########
common/idrepo/lib/src/main/java/org/apache/syncope/common/lib/to/RelationshipTypeTO.java:
##########
@@ -66,4 +74,16 @@
public void setRightEndAnyType(final String rightEndAnyType) {
this.rightEndAnyType = rightEndAnyType;
}
+
+ @JsonIgnore
+ public Optional<TypeExtensionTO> getTypeExtension(final String anyType) {
+ return typeExtensions.stream().filter(
+ typeExtension -> anyType != null &&
anyType.equals(typeExtension.getAnyType())).findFirst();
+ }
+
+ @JacksonXmlElementWrapper(localName = "typeExtensions")
+ @JacksonXmlProperty(localName = "typeExtension")
+ public List<TypeExtensionTO> getTypeExtensions() {
Review Comment:
## Exposing internal representation
getTypeExtensions exposes the internal representation stored in field
typeExtensions. The value may be modified [after this call to
getTypeExtensions](1).
getTypeExtensions exposes the internal representation stored in field
typeExtensions. The value may be modified [after this call to
getTypeExtensions](2).
getTypeExtensions exposes the internal representation stored in field
typeExtensions. The value may be modified [after this call to
getTypeExtensions](3).
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2384)
##########
common/idrepo/lib/src/main/java/org/apache/syncope/common/lib/to/RelationshipTO.java:
##########
@@ -114,6 +144,32 @@
this.end = end;
}
+ @JacksonXmlElementWrapper(localName = "plainAttrs")
+ @JacksonXmlProperty(localName = "plainAttr")
+ @Override
+ public Set<Attr> getPlainAttrs() {
Review Comment:
## Exposing internal representation
getPlainAttrs exposes the internal representation stored in field
plainAttrs. The value may be modified [after this call to getPlainAttrs](1).
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2386)
##########
core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/AllowedSchemas.java:
##########
@@ -21,82 +21,61 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
-import java.util.function.Predicate;
+import org.apache.syncope.core.persistence.api.entity.RelationshipType;
import org.apache.syncope.core.persistence.api.entity.Schema;
import org.apache.syncope.core.persistence.api.entity.group.Group;
public class AllowedSchemas<S extends Schema> {
- private final Set<S> forSelf = new HashSet<>();
+ private final Set<S> self = new HashSet<>();
- private final Map<Group, Set<S>> forMemberships = new HashMap<>();
+ private final Map<Group, Set<S>> memberships = new HashMap<>();
- public Set<S> getForSelf() {
- return forSelf;
- }
+ private final Map<RelationshipType, Set<S>> relationshipTypes = new
HashMap<>();
- public Set<S> getForMembership(final Group group) {
- return forMemberships.get(group) == null ? Set.of() :
forMemberships.get(group);
+ public Set<S> self() {
+ return self;
}
- public Map<Group, Set<S>> getForMemberships() {
- return forMemberships;
+ public Set<S> membership(final Group group) {
+ return Optional.ofNullable(memberships.get(group)).orElseGet(Set::of);
}
- public boolean forSelfContains(final S schema) {
- return forSelf.contains(schema);
+ public Map<Group, Set<S>> memberships() {
Review Comment:
## Exposing internal representation
memberships exposes the internal representation stored in field memberships.
The value may be modified [after this call to memberships](1).
memberships exposes the internal representation stored in field memberships.
The value may be modified [after this call to memberships](2).
[Show more
details](https://github.com/apache/syncope/security/code-scanning/2388)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
