Skip to content

Commit

Permalink
Handle duplicate component properties
Browse files Browse the repository at this point in the history
This extends the identity of a `ComponentProperty` to also include its value. As a consequence, encrypted values will not be supported.

In order to support duplicate `groupName` / `propertyValue` pairs, the `ComponentProperty` class now has a separate `uuid` field in order to still be able to address individual properties via REST API (e.g. for deletion operations).

It is no longer possible to update a `ComponentProperty` via REST API.

Uniqueness of properties is now enforced across `groupName`, `propertyName`, *and* `propertyValue`.

Signed-off-by: nscuro <nscuro@protonmail.com>
  • Loading branch information
nscuro committed Apr 14, 2024
1 parent 8afc52f commit 912217e
Show file tree
Hide file tree
Showing 11 changed files with 275 additions and 218 deletions.
2 changes: 2 additions & 0 deletions docs/_posts/2024-xx-xx-v4.11.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ environment variable `BOM_VALIDATION_ENABLED` to `false`.
* Add attribution notice to NVD documentation - [apiserver/#3490]
* Bump CWE dictionary to v4.13 - [apiserver/#3491]
* Align retry configuration and behavior across analyzers - [apiserver/#3494]
* Add support for component properties - [apiserver/#3499]
* Add auto-generated changelog to GitHub releases - [apiserver/#3502]
* Bump SPDX license list to v3.23 - [apiserver/#3508]
* Validate uploaded BOMs against CycloneDX schema prior to processing them - [apiserver/#3522]
Expand Down Expand Up @@ -181,6 +182,7 @@ Special thanks to everyone who contributed code to implement enhancements and fi
[apiserver/#3490]: https://github.com/DependencyTrack/dependency-track/pull/3490
[apiserver/#3491]: https://github.com/DependencyTrack/dependency-track/pull/3491
[apiserver/#3494]: https://github.com/DependencyTrack/dependency-track/pull/3494
[apiserver/#3499]: https://github.com/DependencyTrack/dependency-track/pull/3499
[apiserver/#3502]: https://github.com/DependencyTrack/dependency-track/pull/3502
[apiserver/#3508]: https://github.com/DependencyTrack/dependency-track/pull/3508
[apiserver/#3511]: https://github.com/DependencyTrack/dependency-track/pull/3511
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/dependencytrack/model/Component.java
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ public enum FetchGroup {
private Collection<Component> children;

@Persistent(mappedBy = "component", defaultFetchGroup = "false")
@Order(extensions = @Extension(vendorName = "datanucleus", key = "list-ordering", value = "groupName ASC, propertyName ASC"))
@Order(extensions = @Extension(vendorName = "datanucleus", key = "list-ordering", value = "groupName ASC, propertyName ASC, id ASC"))
private List<ComponentProperty> properties;

@Persistent(table = "COMPONENTS_VULNERABILITIES")
Expand Down
33 changes: 33 additions & 0 deletions src/main/java/org/dependencytrack/model/ComponentProperty.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,20 @@
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.google.common.base.MoreObjects;
import org.dependencytrack.model.validation.EnumValue;

import javax.jdo.annotations.Column;
import javax.jdo.annotations.IdGeneratorStrategy;
import javax.jdo.annotations.PersistenceCapable;
import javax.jdo.annotations.Persistent;
import javax.jdo.annotations.PrimaryKey;
import javax.jdo.annotations.Unique;
import javax.validation.constraints.NotBlank;
import javax.validation.constraints.NotNull;
import javax.validation.constraints.Pattern;
import javax.validation.constraints.Size;
import java.io.Serializable;
import java.util.UUID;

/**
* @since 4.11.0
Expand All @@ -45,6 +48,14 @@ public class ComponentProperty implements IConfigProperty, Serializable {

private static final long serialVersionUID = -7510889645969713080L;

public record Identity(String group, String name, String value) {

public Identity(final ComponentProperty property) {
this(property.getGroupName(), property.getPropertyName(), property.getPropertyValue());
}

}

@PrimaryKey
@Persistent(valueStrategy = IdGeneratorStrategy.NATIVE)
@JsonIgnore
Expand Down Expand Up @@ -79,6 +90,13 @@ public class ComponentProperty implements IConfigProperty, Serializable {
@Persistent
@Column(name = "PROPERTYTYPE", jdbcType = "VARCHAR", allowsNull = "false")
@NotNull
// NB: Encrypted values are disallowed because it complicates identity management.
// Because duplicate groupName/propertyName combinations are allowed, the value
// is critical to determine property uniqueness. We'd need to decrypt encrypted
// values prior to uniqueness checks. We'd also open the door for attackers to
// guess the encrypted value. As of now, there is no known use-case for encrypted
// properties on the component level.
@EnumValue(disallowed = "ENCRYPTEDSTRING", message = "Encrypted component property values are not supported")
private PropertyType propertyType;

@Persistent
Expand All @@ -88,6 +106,12 @@ public class ComponentProperty implements IConfigProperty, Serializable {
@Pattern(regexp = "\\P{Cc}+", message = "The description must not contain control characters")
private String description;

@Persistent(customValueStrategy = "uuid")
@Unique(name = "COMPONENT_PROPERTY_UUID_IDX")
@Column(name = "UUID", jdbcType = "VARCHAR", length = 36, allowsNull = "false")
@NotNull
private UUID uuid;

public long getId() {
return id;
}
Expand Down Expand Up @@ -144,6 +168,14 @@ public void setDescription(final String description) {
this.description = description;
}

public UUID getUuid() {
return uuid;
}

public void setUuid(final UUID uuid) {
this.uuid = uuid;
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this)
Expand All @@ -154,6 +186,7 @@ public String toString() {
.add("propertyValue", propertyValue)
.add("propertyType", propertyType)
.add("description", description)
.add("uuid", uuid)
.omitNullValues()
.toString();
}
Expand Down
43 changes: 43 additions & 0 deletions src/main/java/org/dependencytrack/model/validation/EnumValue.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* This file is part of Dependency-Track.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
* Copyright (c) OWASP Foundation. All Rights Reserved.
*/
package org.dependencytrack.model.validation;

import javax.validation.Constraint;
import javax.validation.Payload;
import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* @since 4.11.0
*/
@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.FIELD, ElementType.PARAMETER})
@Constraint(validatedBy = EnumValueValidator.class)
public @interface EnumValue {

String[] disallowed() default {};
String message() default "";
Class<?>[] groups() default {};
Class<? extends Payload>[] payload() default {};

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* This file is part of Dependency-Track.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
* Copyright (c) OWASP Foundation. All Rights Reserved.
*/
package org.dependencytrack.model.validation;

import javax.validation.ConstraintValidator;
import javax.validation.ConstraintValidatorContext;
import java.util.Arrays;
import java.util.Set;

/**
* @since 4.11.0
*/
public class EnumValueValidator implements ConstraintValidator<EnumValue, Enum<?>> {

private Set<String> disallowedValues;

@Override
public void initialize(final EnumValue constraintAnnotation) {
disallowedValues = Set.copyOf(Arrays.asList(constraintAnnotation.disallowed()));
}

@Override
public boolean isValid(final Enum<?> value, final ConstraintValidatorContext context) {
if (value == null) {
return true;
}

return !disallowedValues.contains(value.name());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -833,49 +833,35 @@ private void getDirectDependenciesForPathDependencies(Map<String, Component> dep
dependencyGraph.putAll(addToDependencyGraph);
}

/**
* Returns a ComponentProperty with the specified groupName and propertyName.
*
* @param component the component the property belongs to
* @param groupName the group name of the config property
* @param propertyName the name of the property
* @return a ComponentProperty object
*/
@Override
public ComponentProperty getComponentProperty(final Component component, final String groupName, final String propertyName) {
final Query<ComponentProperty> query = this.pm.newQuery(ComponentProperty.class, "component == :component && groupName == :groupName && propertyName == :propertyName");
query.setRange(0, 1);
return singleResult(query.execute(component, groupName, propertyName));
public List<ComponentProperty> getComponentProperties(final Component component, final String groupName, final String propertyName) {
final Query<ComponentProperty> query = pm.newQuery(ComponentProperty.class);
query.setFilter("component == :component && groupName == :groupName && propertyName == :propertyName");
query.setParameters(component, groupName, propertyName);
try {
return List.copyOf(query.executeList());
} finally {
query.closeAll();
}
}

/**
* Returns a List of ProjectProperty's for the specified project.
*
* @param component the project the property belongs to
* @return a List ProjectProperty objects
*/
@Override
@SuppressWarnings("unchecked")
public List<ComponentProperty> getComponentProperties(final Component component) {
final Query<ComponentProperty> query = this.pm.newQuery(ComponentProperty.class, "component == :component");
query.setOrdering("groupName asc, propertyName asc");
return (List<ComponentProperty>) query.execute(component);
final Query<ComponentProperty> query = pm.newQuery(ComponentProperty.class, "component == :component");
query.setOrdering("groupName ASC, propertyName ASC, id ASC");
try {
return List.copyOf(query.executeList());
} finally {
query.closeAll();
}
}

/**
* Creates a key/value pair (ComponentProperty) for the specified Project.
*
* @param component the Component to create the property for
* @param groupName the group name of the property
* @param propertyName the name of the property
* @param propertyValue the value of the property
* @param propertyType the type of property
* @param description a description of the property
* @return the created ComponentProperty object
*/
@Override
public ComponentProperty createComponentProperty(final Component component, final String groupName, final String propertyName,
final String propertyValue, final IConfigProperty.PropertyType propertyType,
public ComponentProperty createComponentProperty(final Component component,
final String groupName,
final String propertyName,
final String propertyValue,
final IConfigProperty.PropertyType propertyType,
final String description) {
final ComponentProperty property = new ComponentProperty();
property.setComponent(component);
Expand All @@ -887,4 +873,15 @@ public ComponentProperty createComponentProperty(final Component component, fina
return persist(property);
}

@Override
public long deleteComponentPropertyByUuid(final Component component, final UUID uuid) {
final Query<ComponentProperty> query = pm.newQuery(ComponentProperty.class);
query.setFilter("component == :component && uuid == :uuid");
try {
return query.deletePersistentAll(component, uuid);
} finally {
query.closeAll();
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -563,8 +563,8 @@ public List<ComponentProperty> getComponentProperties(final Component component)
return getComponentQueryManager().getComponentProperties(component);
}

public ComponentProperty getComponentProperty(final Component component, final String groupName, final String propertyName) {
return getComponentQueryManager().getComponentProperty(component, groupName, propertyName);
public List<ComponentProperty> getComponentProperties(final Component component, final String groupName, final String propertyName) {
return getComponentQueryManager().getComponentProperties(component, groupName, propertyName);
}

public ComponentProperty createComponentProperty(final Component component, final String groupName, final String propertyName,
Expand All @@ -574,6 +574,10 @@ public ComponentProperty createComponentProperty(final Component component, fina
.createComponentProperty(component, groupName, propertyName, propertyValue, propertyType, description);
}

public long deleteComponentPropertyByUuid(final Component component, final UUID uuid) {
return getComponentQueryManager().deleteComponentPropertyByUuid(component, uuid);
}

public PaginatedResult getLicenses() {
return getLicenseQueryManager().getLicenses();
}
Expand Down
Loading

0 comments on commit 912217e

Please sign in to comment.