From 1fbd88ff462827d324217d45b959ca36664922ca Mon Sep 17 00:00:00 2001 From: nscuro Date: Mon, 26 Feb 2024 22:51:01 +0100 Subject: [PATCH 01/17] Add component property model and resource Supersedes #2717 Co-authored-by: Robert Kesterson Signed-off-by: nscuro --- .../org/dependencytrack/model/Component.java | 13 + .../model/ComponentProperty.java | 149 +++++++ .../persistence/ComponentQueryManager.java | 57 +++ .../persistence/QueryManager.java | 17 + .../v1/ComponentPropertyResource.java | 235 +++++++++++ src/main/resources/META-INF/persistence.xml | 1 + .../model/ComponentPropertyTest.java | 77 ++++ .../v1/ComponentPropertyResourceTest.java | 364 ++++++++++++++++++ 8 files changed, 913 insertions(+) create mode 100644 src/main/java/org/dependencytrack/model/ComponentProperty.java create mode 100644 src/main/java/org/dependencytrack/resources/v1/ComponentPropertyResource.java create mode 100644 src/test/java/org/dependencytrack/model/ComponentPropertyTest.java create mode 100644 src/test/java/org/dependencytrack/resources/v1/ComponentPropertyResourceTest.java diff --git a/src/main/java/org/dependencytrack/model/Component.java b/src/main/java/org/dependencytrack/model/Component.java index 29924065f4..7396382f53 100644 --- a/src/main/java/org/dependencytrack/model/Component.java +++ b/src/main/java/org/dependencytrack/model/Component.java @@ -74,6 +74,7 @@ @Persistent(name = "externalReferences"), @Persistent(name = "parent"), @Persistent(name = "children"), + @Persistent(name = "properties"), @Persistent(name = "vulnerabilities"), }), @FetchGroup(name = "INTERNAL_IDENTIFICATION", members = { @@ -330,6 +331,10 @@ public enum FetchGroup { @Order(extensions = @Extension(vendorName = "datanucleus", key = "list-ordering", value = "id ASC")) private Collection children; + @Persistent(mappedBy = "component", defaultFetchGroup = "false") + @Order(extensions = @Extension(vendorName = "datanucleus", key = "list-ordering", value = "groupName ASC, propertyName ASC")) + private List properties; + @Persistent(table = "COMPONENTS_VULNERABILITIES") @Join(column = "COMPONENT_ID") @Element(column = "VULNERABILITY_ID") @@ -712,6 +717,14 @@ public void setChildren(Collection children) { this.children = children; } + public List getProperties() { + return properties; + } + + public void setProperties(final List properties) { + this.properties = properties; + } + public List getVulnerabilities() { return vulnerabilities; } diff --git a/src/main/java/org/dependencytrack/model/ComponentProperty.java b/src/main/java/org/dependencytrack/model/ComponentProperty.java new file mode 100644 index 0000000000..ccac4048dc --- /dev/null +++ b/src/main/java/org/dependencytrack/model/ComponentProperty.java @@ -0,0 +1,149 @@ +/* + * 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) Steve Springett. All Rights Reserved. + */ +package org.dependencytrack.model; + +import alpine.model.IConfigProperty; +import alpine.server.json.TrimmedStringDeserializer; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; + +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; + +/** + * @since 4.11.0 + */ +@PersistenceCapable(table = "COMPONENT_PROPERTY") +@Unique(name = "COMPONENT_PROPERTY_KEYS_IDX", members = {"component", "groupName", "propertyName"}) +@JsonInclude(JsonInclude.Include.NON_NULL) +public class ComponentProperty implements IConfigProperty, Serializable { + + private static final long serialVersionUID = -7510889645969713080L; + + @PrimaryKey + @Persistent(valueStrategy = IdGeneratorStrategy.NATIVE) + @JsonIgnore + private long id; + + @Persistent + @Column(name = "COMPONENT_ID", allowsNull = "false") + private Component component; + + @Persistent + @Column(name = "GROUPNAME", allowsNull = "false") + @NotBlank + @Size(min = 1, max = 255) + @JsonDeserialize(using = TrimmedStringDeserializer.class) + @Pattern(regexp = "\\P{Cc}+", message = "The groupName must not contain control characters") + private String groupName; + + @Persistent + @Column(name = "PROPERTYNAME", allowsNull = "false") + @NotBlank + @Size(min = 1, max = 255) + @JsonDeserialize(using = TrimmedStringDeserializer.class) + @Pattern(regexp = "\\P{Cc}+", message = "The propertyName must not contain control characters") + private String propertyName; + + @Persistent + @Column(name = "PROPERTYVALUE", length = 1024) + @Size(min = 0, max = 1024) + @JsonDeserialize(using = TrimmedStringDeserializer.class) + @Pattern(regexp = "\\P{Cc}+", message = "The propertyValue must not contain control characters") + private String propertyValue; + + @Persistent + @Column(name = "PROPERTYTYPE", jdbcType = "VARCHAR", allowsNull = "false") + @NotNull + private PropertyType propertyType; + + @Persistent + @Column(name = "DESCRIPTION") + @Size(max = 255) + @JsonDeserialize(using = TrimmedStringDeserializer.class) + @Pattern(regexp = "\\P{Cc}+", message = "The description must not contain control characters") + private String description; + + public long getId() { + return id; + } + + public void setId(final long id) { + this.id = id; + } + + public Component getComponent() { + return component; + } + + public void setComponent(final Component component) { + this.component = component; + } + + public String getGroupName() { + return groupName; + } + + public void setGroupName(final String groupName) { + this.groupName = groupName; + } + + public String getPropertyName() { + return propertyName; + } + + public void setPropertyName(final String propertyName) { + this.propertyName = propertyName; + } + + public String getPropertyValue() { + return propertyValue; + } + + public void setPropertyValue(final String propertyValue) { + this.propertyValue = propertyValue; + } + + public PropertyType getPropertyType() { + return propertyType; + } + + public void setPropertyType(final PropertyType propertyType) { + this.propertyType = propertyType; + } + + public String getDescription() { + return description; + } + + public void setDescription(final String description) { + this.description = description; + } + +} \ No newline at end of file diff --git a/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java b/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java index dec72efdd9..e4e215614d 100644 --- a/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java @@ -21,6 +21,7 @@ import alpine.common.logging.Logger; import alpine.event.framework.Event; import alpine.model.ApiKey; +import alpine.model.IConfigProperty; import alpine.model.Team; import alpine.model.UserPrincipal; import alpine.persistence.PaginatedResult; @@ -31,6 +32,7 @@ import org.dependencytrack.event.IndexEvent; import org.dependencytrack.model.Component; import org.dependencytrack.model.ComponentIdentity; +import org.dependencytrack.model.ComponentProperty; import org.dependencytrack.model.ConfigPropertyConstants; import org.dependencytrack.model.Project; import org.dependencytrack.model.RepositoryMetaComponent; @@ -830,4 +832,59 @@ private void getDirectDependenciesForPathDependencies(Map 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 query = this.pm.newQuery(ComponentProperty.class, "component == :component && groupName == :groupName && propertyName == :propertyName"); + query.setRange(0, 1); + return singleResult(query.execute(component, groupName, propertyName)); + } + + /** + * 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 getComponentProperties(final Component component) { + final Query query = this.pm.newQuery(ComponentProperty.class, "component == :component"); + query.setOrdering("groupName asc, propertyName asc"); + return (List) query.execute(component); + } + + /** + * 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, + final String description) { + final ComponentProperty property = new ComponentProperty(); + property.setComponent(component); + property.setGroupName(groupName); + property.setPropertyName(propertyName); + property.setPropertyValue(propertyValue); + property.setPropertyType(propertyType); + property.setDescription(description); + return persist(property); + } + } diff --git a/src/main/java/org/dependencytrack/persistence/QueryManager.java b/src/main/java/org/dependencytrack/persistence/QueryManager.java index 08680dbe6b..358438c2c0 100644 --- a/src/main/java/org/dependencytrack/persistence/QueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/QueryManager.java @@ -22,6 +22,7 @@ import alpine.event.framework.Event; import alpine.model.ApiKey; import alpine.model.ConfigProperty; +import alpine.model.IConfigProperty; import alpine.model.Team; import alpine.model.UserPrincipal; import alpine.notification.NotificationLevel; @@ -44,6 +45,7 @@ import org.dependencytrack.model.Component; import org.dependencytrack.model.ComponentAnalysisCache; import org.dependencytrack.model.ComponentIdentity; +import org.dependencytrack.model.ComponentProperty; import org.dependencytrack.model.ConfigPropertyConstants; import org.dependencytrack.model.DependencyMetrics; import org.dependencytrack.model.Finding; @@ -557,6 +559,21 @@ public Map getDependencyGraphForComponents(Project project, L return getComponentQueryManager().getDependencyGraphForComponents(project, components); } + public List 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 ComponentProperty createComponentProperty(final Component component, final String groupName, final String propertyName, + final String propertyValue, final IConfigProperty.PropertyType propertyType, + final String description) { + return getComponentQueryManager() + .createComponentProperty(component, groupName, propertyName, propertyValue, propertyType, description); + } + public PaginatedResult getLicenses() { return getLicenseQueryManager().getLicenses(); } diff --git a/src/main/java/org/dependencytrack/resources/v1/ComponentPropertyResource.java b/src/main/java/org/dependencytrack/resources/v1/ComponentPropertyResource.java new file mode 100644 index 0000000000..3a5eaac9e3 --- /dev/null +++ b/src/main/java/org/dependencytrack/resources/v1/ComponentPropertyResource.java @@ -0,0 +1,235 @@ +/* + * 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) Steve Springett. All Rights Reserved. + */ +package org.dependencytrack.resources.v1; + +import alpine.server.auth.PermissionRequired; +import io.swagger.annotations.Api; +import io.swagger.annotations.ApiOperation; +import io.swagger.annotations.ApiParam; +import io.swagger.annotations.ApiResponse; +import io.swagger.annotations.ApiResponses; +import io.swagger.annotations.Authorization; +import org.apache.commons.lang3.StringUtils; +import org.dependencytrack.auth.Permissions; +import org.dependencytrack.model.Component; +import org.dependencytrack.model.ComponentProperty; +import org.dependencytrack.persistence.QueryManager; + +import javax.validation.Validator; +import javax.ws.rs.Consumes; +import javax.ws.rs.DELETE; +import javax.ws.rs.GET; +import javax.ws.rs.POST; +import javax.ws.rs.PUT; +import javax.ws.rs.Path; +import javax.ws.rs.PathParam; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import java.util.List; + +/** + * @since 4.11.0 + */ +@Path("/v1/component/{uuid}/property") +@Api(value = "componentProperty", authorizations = @Authorization(value = "X-Api-Key")) +public class ComponentPropertyResource extends AbstractConfigPropertyResource { + + @GET + @Produces(MediaType.APPLICATION_JSON) + @ApiOperation( + value = "Returns a list of all ComponentProperties for the specified component", + response = ComponentProperty.class, + responseContainer = "List" + ) + @ApiResponses(value = { + @ApiResponse(code = 401, message = "Unauthorized"), + @ApiResponse(code = 403, message = "Access to the specified project is forbidden"), + @ApiResponse(code = 404, message = "The project could not be found") + }) + @PermissionRequired(Permissions.Constants.PORTFOLIO_MANAGEMENT) + public Response getProperties( + @ApiParam(value = "The UUID of the component to retrieve properties for", required = true) + @PathParam("uuid") String uuid) { + try (QueryManager qm = new QueryManager(getAlpineRequest())) { + final Component component = qm.getObjectByUuid(Component.class, uuid); + if (component != null) { + if (qm.hasAccess(super.getPrincipal(), component.getProject())) { + final List properties = qm.getComponentProperties(component); + // Detaches the objects and closes the persistence manager so that if/when encrypted string + // values are replaced by the placeholder, they are not erroneously persisted to the database. + qm.getPersistenceManager().detachCopyAll(properties); + qm.close(); + for (final ComponentProperty property : properties) { + // Replace the value of encrypted strings with the pre-defined placeholder + if (ComponentProperty.PropertyType.ENCRYPTEDSTRING == property.getPropertyType()) { + property.setPropertyValue(ENCRYPTED_PLACEHOLDER); + } + } + return Response.ok(properties).build(); + } else { + return Response.status(Response.Status.FORBIDDEN).entity("Access to the specified project is forbidden").build(); + } + } else { + return Response.status(Response.Status.NOT_FOUND).entity("The component could not be found.").build(); + } + } + } + + @PUT + @Consumes(MediaType.APPLICATION_JSON) + @Produces(MediaType.APPLICATION_JSON) + @ApiOperation( + value = "Creates a new component property", + response = ComponentProperty.class, + code = 201 + ) + @ApiResponses(value = { + @ApiResponse(code = 401, message = "Unauthorized"), + @ApiResponse(code = 403, message = "Access to the specified component is forbidden"), + @ApiResponse(code = 404, message = "The component could not be found"), + @ApiResponse(code = 409, message = "A property with the specified component/group/name combination already exists") + }) + @PermissionRequired(Permissions.Constants.PORTFOLIO_MANAGEMENT) + public Response createProperty( + @ApiParam(value = "The UUID of the component to create a property for", required = true) + @PathParam("uuid") String uuid, + ComponentProperty json) { + final Validator validator = super.getValidator(); + failOnValidationError( + validator.validateProperty(json, "groupName"), + validator.validateProperty(json, "propertyName"), + validator.validateProperty(json, "propertyValue") + ); + try (QueryManager qm = new QueryManager()) { + final Component component = qm.getObjectByUuid(Component.class, uuid); + if (component != null) { + if (qm.hasAccess(super.getPrincipal(), component.getProject())) { + final ComponentProperty existing = qm.getComponentProperty(component, + StringUtils.trimToNull(json.getGroupName()), StringUtils.trimToNull(json.getPropertyName())); + if (existing == null) { + final ComponentProperty property = qm.createComponentProperty(component, + StringUtils.trimToNull(json.getGroupName()), + StringUtils.trimToNull(json.getPropertyName()), + null, // Set value to null - this will be taken care of by updatePropertyValue below + json.getPropertyType(), + StringUtils.trimToNull(json.getDescription())); + updatePropertyValue(qm, json, property); + qm.getPersistenceManager().detachCopy(component); + qm.close(); + if (ComponentProperty.PropertyType.ENCRYPTEDSTRING == property.getPropertyType()) { + property.setPropertyValue(ENCRYPTED_PLACEHOLDER); + } + return Response.status(Response.Status.CREATED).entity(property).build(); + } else { + return Response.status(Response.Status.CONFLICT).entity("A property with the specified component/group/name combination already exists.").build(); + } + } else { + return Response.status(Response.Status.FORBIDDEN).entity("Access to the specified component is forbidden").build(); + } + } else { + return Response.status(Response.Status.NOT_FOUND).entity("The component could not be found.").build(); + } + } + } + + @POST + @Consumes(MediaType.APPLICATION_JSON) + @Produces(MediaType.APPLICATION_JSON) + @ApiOperation( + value = "Updates a project property", + response = ComponentProperty.class + ) + @ApiResponses(value = { + @ApiResponse(code = 401, message = "Unauthorized"), + @ApiResponse(code = 403, message = "Access to the specified project is forbidden"), + @ApiResponse(code = 404, message = "The component could not be found"), + }) + @PermissionRequired(Permissions.Constants.PORTFOLIO_MANAGEMENT) + public Response updateProperty( + @ApiParam(value = "The UUID of the component to create a property for", required = true) + @PathParam("uuid") String uuid, + ComponentProperty json) { + final Validator validator = super.getValidator(); + failOnValidationError( + validator.validateProperty(json, "groupName"), + validator.validateProperty(json, "propertyName"), + validator.validateProperty(json, "propertyValue") + ); + try (QueryManager qm = new QueryManager()) { + final Component component = qm.getObjectByUuid(Component.class, uuid); + if (component != null) { + if (qm.hasAccess(super.getPrincipal(), component.getProject())) { + final ComponentProperty property = qm.getComponentProperty(component, json.getGroupName(), json.getPropertyName()); + if (property != null) { + return updatePropertyValue(qm, json, property); + } else { + return Response.status(Response.Status.NOT_FOUND).entity("A property with the specified component/group/name combination could not be found.").build(); + } + } else { + return Response.status(Response.Status.FORBIDDEN).entity("Access to the specified component is forbidden").build(); + } + } else { + return Response.status(Response.Status.NOT_FOUND).entity("The component could not be found.").build(); + } + } + } + + @DELETE + @Consumes(MediaType.APPLICATION_JSON) + @Produces(MediaType.APPLICATION_JSON) + @ApiOperation( + value = "Deletes a config property", + response = ComponentProperty.class + ) + @ApiResponses(value = { + @ApiResponse(code = 401, message = "Unauthorized"), + @ApiResponse(code = 403, message = "Access to the specified component is forbidden"), + @ApiResponse(code = 404, message = "The component or component property could not be found"), + }) + @PermissionRequired(Permissions.Constants.PORTFOLIO_MANAGEMENT) + public Response deleteProperty( + @ApiParam(value = "The UUID of the component to delete a property from", required = true) + @PathParam("uuid") String uuid, + ComponentProperty json) { + final Validator validator = super.getValidator(); + failOnValidationError( + validator.validateProperty(json, "groupName"), + validator.validateProperty(json, "propertyName") + ); + try (QueryManager qm = new QueryManager()) { + final Component component = qm.getObjectByUuid(Component.class, uuid); + if (component != null) { + if (qm.hasAccess(super.getPrincipal(), component.getProject())) { + final ComponentProperty property = qm.getComponentProperty(component, json.getGroupName(), json.getPropertyName()); + if (property != null) { + qm.delete(property); + return Response.status(Response.Status.NO_CONTENT).build(); + } else { + return Response.status(Response.Status.NOT_FOUND).entity("The component property could not be found.").build(); + } + } else { + return Response.status(Response.Status.FORBIDDEN).entity("Access to the specified component is forbidden").build(); + } + } else { + return Response.status(Response.Status.NOT_FOUND).entity("The component could not be found.").build(); + } + } + } +} \ No newline at end of file diff --git a/src/main/resources/META-INF/persistence.xml b/src/main/resources/META-INF/persistence.xml index 1a2763d086..48f4e91804 100644 --- a/src/main/resources/META-INF/persistence.xml +++ b/src/main/resources/META-INF/persistence.xml @@ -27,6 +27,7 @@ org.dependencytrack.model.Bom org.dependencytrack.model.Component org.dependencytrack.model.ComponentAnalysisCache + org.dependencytrack.model.ComponentProperty org.dependencytrack.model.DependencyMetrics org.dependencytrack.model.FindingAttribution org.dependencytrack.model.License diff --git a/src/test/java/org/dependencytrack/model/ComponentPropertyTest.java b/src/test/java/org/dependencytrack/model/ComponentPropertyTest.java new file mode 100644 index 0000000000..f261d56484 --- /dev/null +++ b/src/test/java/org/dependencytrack/model/ComponentPropertyTest.java @@ -0,0 +1,77 @@ +/* + * 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) Steve Springett. All Rights Reserved. + */ +package org.dependencytrack.model; + +import alpine.model.IConfigProperty; +import org.junit.Assert; +import org.junit.Test; + +public class ComponentPropertyTest { + + @Test + public void testId() { + ComponentProperty property = new ComponentProperty(); + property.setId(111L); + Assert.assertEquals(111L, property.getId()); + } + + @Test + public void testProject() { + Component component = new Component(); + ComponentProperty property = new ComponentProperty(); + property.setComponent(component); + Assert.assertEquals(component, property.getComponent()); + } + + @Test + public void testGroupName() { + ComponentProperty property = new ComponentProperty(); + property.setGroupName("Group Name"); + Assert.assertEquals("Group Name", property.getGroupName()); + } + + @Test + public void testPropertyName() { + ComponentProperty property = new ComponentProperty(); + property.setPropertyName("Property Name"); + Assert.assertEquals("Property Name", property.getPropertyName()); + } + + @Test + public void testPropertyValue() { + ComponentProperty property = new ComponentProperty(); + property.setPropertyValue("Property Value"); + Assert.assertEquals("Property Value", property.getPropertyValue()); + } + + @Test + public void testPropertyType() { + ComponentProperty property = new ComponentProperty(); + property.setPropertyType(IConfigProperty.PropertyType.STRING); + Assert.assertEquals(IConfigProperty.PropertyType.STRING, property.getPropertyType()); + } + + @Test + public void testDescription() { + ComponentProperty property = new ComponentProperty(); + property.setDescription("Property Description"); + Assert.assertEquals("Property Description", property.getDescription()); + } + +} \ No newline at end of file diff --git a/src/test/java/org/dependencytrack/resources/v1/ComponentPropertyResourceTest.java b/src/test/java/org/dependencytrack/resources/v1/ComponentPropertyResourceTest.java new file mode 100644 index 0000000000..1983ea7de6 --- /dev/null +++ b/src/test/java/org/dependencytrack/resources/v1/ComponentPropertyResourceTest.java @@ -0,0 +1,364 @@ +/* + * 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) Steve Springett. All Rights Reserved. + */ +package org.dependencytrack.resources.v1; + +import alpine.model.IConfigProperty.PropertyType; +import alpine.server.filters.ApiFilter; +import alpine.server.filters.AuthenticationFilter; +import org.dependencytrack.ResourceTest; +import org.dependencytrack.model.Component; +import org.dependencytrack.model.ComponentProperty; +import org.dependencytrack.model.Project; +import org.glassfish.jersey.client.ClientProperties; +import org.glassfish.jersey.server.ResourceConfig; +import org.glassfish.jersey.servlet.ServletContainer; +import org.glassfish.jersey.test.DeploymentContext; +import org.glassfish.jersey.test.ServletDeploymentContext; +import org.junit.Test; + +import javax.ws.rs.client.Entity; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import java.util.UUID; + +import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; +import static org.assertj.core.api.Assertions.assertThat; + +public class ComponentPropertyResourceTest extends ResourceTest { + + @Override + protected DeploymentContext configureDeployment() { + return ServletDeploymentContext.forServlet(new ServletContainer( + new ResourceConfig(ComponentPropertyResource.class) + .register(ApiFilter.class) + .register(AuthenticationFilter.class))) + .build(); + } + + @Test + public void getPropertiesTest() { + final var project = new Project(); + project.setName("acme-app"); + qm.persist(project); + + final var component = new Component(); + component.setProject(project); + component.setName("acme-lib"); + qm.persist(component); + + final var propertyA = new ComponentProperty(); + propertyA.setComponent(component); + propertyA.setGroupName("foo-a"); + propertyA.setPropertyName("bar-a"); + propertyA.setPropertyValue("baz-a"); + propertyA.setPropertyType(PropertyType.STRING); + propertyA.setDescription("qux-a"); + qm.persist(propertyA); + + final var propertyB = new ComponentProperty(); + propertyB.setComponent(component); + propertyB.setGroupName("foo-b"); + propertyB.setPropertyName("bar-b"); + propertyB.setPropertyValue("baz-b"); + propertyB.setPropertyType(PropertyType.ENCRYPTEDSTRING); + propertyB.setDescription("qux-b"); + qm.persist(propertyB); + + final Response response = target("%s/%s/property".formatted(V1_COMPONENT, component.getUuid())).request() + .header(X_API_KEY, apiKey) + .get(); + + assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.getHeaderString(TOTAL_COUNT_HEADER)).isNull(); + assertThatJson(getPlainTextBody(response)).isEqualTo(""" + [ + { + "groupName": "foo-a", + "propertyName": "bar-a", + "propertyValue": "baz-a", + "propertyType": "STRING", + "description": "qux-a" + }, + { + "groupName": "foo-b", + "propertyName": "bar-b", + "propertyValue": "HiddenDecryptedPropertyPlaceholder", + "propertyType": "ENCRYPTEDSTRING", + "description": "qux-b" + } + ] + """); + } + + @Test + public void getPropertiesInvalidTest() { + final Response response = target("%s/%s/property".formatted(V1_COMPONENT, UUID.randomUUID())).request() + .header(X_API_KEY, apiKey) + .get(Response.class); + + assertThat(response.getStatus()).isEqualTo(404); + assertThat(response.getHeaderString(TOTAL_COUNT_HEADER)).isNull(); + assertThat(getPlainTextBody(response)).isEqualTo("The component could not be found."); + } + + @Test + public void createPropertyTest() { + final var project = new Project(); + project.setName("acme-app"); + qm.persist(project); + + final var component = new Component(); + component.setProject(project); + component.setName("acme-lib"); + qm.persist(component); + + final Response response = target("%s/%s/property".formatted(V1_COMPONENT, component.getUuid())).request() + .header(X_API_KEY, apiKey) + .put(Entity.entity(""" + { + "groupName": "foo", + "propertyName": "bar", + "propertyValue": "baz", + "propertyType": "STRING", + "description": "qux" + } + """, MediaType.APPLICATION_JSON)); + + assertThat(response.getStatus()).isEqualTo(201); + assertThat(response.getHeaderString(TOTAL_COUNT_HEADER)).isNull(); + assertThatJson(getPlainTextBody(response)).isEqualTo(""" + { + "groupName": "foo", + "propertyName": "bar", + "propertyValue": "baz", + "propertyType": "STRING", + "description": "qux" + } + """); + } + + @Test + public void createPropertyEncryptedTest() { + final var project = new Project(); + project.setName("acme-app"); + qm.persist(project); + + final var component = new Component(); + component.setProject(project); + component.setName("acme-lib"); + qm.persist(component); + + final Response response = target("%s/%s/property".formatted(V1_COMPONENT, component.getUuid())).request() + .header(X_API_KEY, apiKey) + .put(Entity.entity(""" + { + "groupName": "foo", + "propertyName": "bar", + "propertyValue": "baz", + "propertyType": "ENCRYPTEDSTRING", + "description": "qux" + } + """, MediaType.APPLICATION_JSON)); + + assertThat(response.getStatus()).isEqualTo(201); + assertThat(response.getHeaderString(TOTAL_COUNT_HEADER)).isNull(); + assertThatJson(getPlainTextBody(response)).isEqualTo(""" + { + "groupName": "foo", + "propertyName": "bar", + "propertyValue": "HiddenDecryptedPropertyPlaceholder", + "propertyType": "ENCRYPTEDSTRING", + "description": "qux" + } + """); + } + + @Test + public void createPropertyDuplicateTest() { + final var project = new Project(); + project.setName("acme-app"); + qm.persist(project); + + final var component = new Component(); + component.setProject(project); + component.setName("acme-lib"); + qm.persist(component); + + final var property = new ComponentProperty(); + property.setComponent(component); + property.setGroupName("foo"); + property.setPropertyName("bar"); + property.setPropertyValue("baz"); + property.setPropertyType(PropertyType.STRING); + qm.persist(property); + + final Response response = target("%s/%s/property".formatted(V1_COMPONENT, component.getUuid())).request() + .header(X_API_KEY, apiKey) + .put(Entity.entity(""" + { + "groupName": "foo", + "propertyName": "bar", + "propertyValue": "baz", + "propertyType": "STRING", + "description": "qux" + } + """, MediaType.APPLICATION_JSON)); + + assertThat(response.getStatus()).isEqualTo(409); + assertThat(response.getHeaderString(TOTAL_COUNT_HEADER)).isNull(); + assertThat(getPlainTextBody(response)).isEqualTo(""" + A property with the specified component/group/name combination already exists."""); + } + + @Test + public void createPropertyInvalidTest() { + final var project = new Project(); + project.setName("acme-app"); + qm.persist(project); + + final var component = new Component(); + component.setProject(project); + component.setName("acme-lib"); + qm.persist(component); + + final Response response = target("%s/%s/property".formatted(V1_COMPONENT, UUID.randomUUID())).request() + .header(X_API_KEY, apiKey) + .put(Entity.entity(""" + { + "groupName": "foo", + "propertyName": "bar", + "propertyValue": "baz", + "propertyType": "STRING", + "description": "qux" + } + """, MediaType.APPLICATION_JSON)); + + assertThat(response.getStatus()).isEqualTo(404); + assertThat(response.getHeaderString(TOTAL_COUNT_HEADER)).isNull(); + assertThat(getPlainTextBody(response)).isEqualTo("The component could not be found."); + } + + @Test + public void updatePropertyTest() { + final var project = new Project(); + project.setName("acme-app"); + qm.persist(project); + + final var component = new Component(); + component.setProject(project); + component.setName("acme-lib"); + qm.persist(component); + + final var property = new ComponentProperty(); + property.setComponent(component); + property.setGroupName("foo"); + property.setPropertyName("bar"); + property.setPropertyValue("baz"); + property.setPropertyType(PropertyType.STRING); + qm.persist(property); + + final Response response = target("%s/%s/property".formatted(V1_COMPONENT, component.getUuid())).request() + .header(X_API_KEY, apiKey) + .post(Entity.entity(""" + { + "groupName": "foo", + "propertyName": "bar", + "propertyValue": "qux" + } + """, MediaType.APPLICATION_JSON)); + + assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.getHeaderString(TOTAL_COUNT_HEADER)).isNull(); + assertThatJson(getPlainTextBody(response)).isEqualTo(""" + { + "groupName": "foo", + "propertyName": "bar", + "propertyValue": "qux", + "propertyType": "STRING" + } + """); + } + + @Test + public void updatePropertyInvalidTest() { + final var project = new Project(); + project.setName("acme-app"); + qm.persist(project); + + final var component = new Component(); + component.setProject(project); + component.setName("acme-lib"); + qm.persist(component); + + final var property = new ComponentProperty(); + property.setComponent(component); + property.setGroupName("foo"); + property.setPropertyName("bar"); + property.setPropertyValue("baz"); + property.setPropertyType(PropertyType.STRING); + qm.persist(property); + + final Response response = target("%s/%s/property".formatted(V1_COMPONENT, UUID.randomUUID())).request() + .header(X_API_KEY, apiKey) + .post(Entity.entity(""" + { + "groupName": "foo", + "propertyName": "bar", + "propertyValue": "qux" + } + """, MediaType.APPLICATION_JSON)); + + assertThat(response.getStatus()).isEqualTo(404); + assertThat(response.getHeaderString(TOTAL_COUNT_HEADER)).isNull(); + assertThat(getPlainTextBody(response)).isEqualTo("The component could not be found."); + } + + @Test + public void deletePropertyTest() { + final var project = new Project(); + project.setName("acme-app"); + qm.persist(project); + + final var component = new Component(); + component.setProject(project); + component.setName("acme-lib"); + qm.persist(component); + + final var property = new ComponentProperty(); + property.setComponent(component); + property.setGroupName("foo"); + property.setPropertyName("bar"); + property.setPropertyValue("baz"); + property.setPropertyType(PropertyType.STRING); + qm.persist(property); + + final Response response = target("%s/%s/property".formatted(V1_COMPONENT, component.getUuid())).request() + .header(X_API_KEY, apiKey) + .property(ClientProperties.SUPPRESS_HTTP_COMPLIANCE_VALIDATION, true) + .method("DELETE", Entity.entity(""" + { + "groupName": "foo", + "propertyName": "bar" + } + """, MediaType.APPLICATION_JSON)); + + assertThat(response.getStatus()).isEqualTo(204); + assertThat(getPlainTextBody(response)).isEmpty(); + } +} \ No newline at end of file From 0cd433241a82a94b946d363f470d8ffe857a9416 Mon Sep 17 00:00:00 2001 From: nscuro Date: Mon, 26 Feb 2024 23:36:49 +0100 Subject: [PATCH 02/17] Ingest component properties from BOM Signed-off-by: nscuro --- .../parser/cyclonedx/util/ModelConverter.java | 45 +++++++++++++++++++ .../tasks/BomUploadProcessingTaskV2.java | 38 ++++++++++++++++ .../tasks/BomUploadProcessingTaskTest.java | 19 +++++++- src/test/resources/unit/bom-1.xml | 5 +++ 4 files changed, 106 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java b/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java index a413a30e00..20469f1a58 100644 --- a/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java +++ b/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java @@ -19,6 +19,7 @@ package org.dependencytrack.parser.cyclonedx.util; import alpine.common.logging.Logger; +import alpine.model.IConfigProperty.PropertyType; import com.github.packageurl.MalformedPackageURLException; import com.github.packageurl.PackageURL; import org.apache.commons.collections4.CollectionUtils; @@ -38,6 +39,7 @@ import org.dependencytrack.model.Classifier; import org.dependencytrack.model.Component; import org.dependencytrack.model.ComponentIdentity; +import org.dependencytrack.model.ComponentProperty; import org.dependencytrack.model.Cwe; import org.dependencytrack.model.DataClassification; import org.dependencytrack.model.ExternalReference; @@ -168,6 +170,7 @@ public static Component convertComponent(final org.cyclonedx.model.Component cdx component.setCopyright(trimToNull(cdxComponent.getCopyright())); component.setCpe(trimToNull(cdxComponent.getCpe())); component.setExternalReferences(convertExternalReferences(cdxComponent.getExternalReferences())); + component.setProperties(convertToComponentProperties(cdxComponent.getProperties())); if (cdxComponent.getPurl() != null) { try { @@ -255,6 +258,48 @@ public static Component convertComponent(final org.cyclonedx.model.Component cdx return component; } + private static List convertToComponentProperties(final List cdxProperties) { + if (cdxProperties == null || cdxProperties.isEmpty()) { + return Collections.emptyList(); + } + + return cdxProperties.stream() + .map(ModelConverter::convertToComponentProperty) + .filter(Objects::nonNull) + .toList(); + } + + private static ComponentProperty convertToComponentProperty(final org.cyclonedx.model.Property cdxProperty) { + if (cdxProperty == null) { + return null; + } + + final var property = new ComponentProperty(); + property.setPropertyValue(trimToNull(cdxProperty.getValue())); + property.setPropertyType(PropertyType.STRING); + + final String cdxPropertyName = trimToNull(cdxProperty.getName()); + if (cdxPropertyName == null) { + // TODO: What to do here? + // * Generate groupName and propertyName? + // * Log a warning and ignore? + return null; + } + + // Treat property names according to the CycloneDX namespace syntax: + // https://cyclonedx.github.io/cyclonedx-property-taxonomy/ + final int lastSeparatorIndex = cdxPropertyName.lastIndexOf(':'); + if (lastSeparatorIndex < 0) { + property.setGroupName("internal"); + property.setPropertyName(cdxPropertyName); + } else { + property.setGroupName(cdxPropertyName.substring(0, lastSeparatorIndex)); + property.setPropertyName(cdxPropertyName.substring(lastSeparatorIndex + 1)); + } + + return property; + } + public static List convertServices(final List cdxServices) { if (cdxServices == null || cdxServices.isEmpty()) { return Collections.emptyList(); diff --git a/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java b/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java index 09bab0f182..c27a01fe2b 100644 --- a/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java +++ b/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java @@ -41,6 +41,7 @@ import org.dependencytrack.model.Bom; import org.dependencytrack.model.Component; import org.dependencytrack.model.ComponentIdentity; +import org.dependencytrack.model.ComponentProperty; import org.dependencytrack.model.DependencyMetrics; import org.dependencytrack.model.FindingAttribution; import org.dependencytrack.model.License; @@ -74,6 +75,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.concurrent.TimeUnit; @@ -436,12 +438,15 @@ private Map processComponents(final QueryManager q applyIfChanged(persistentComponent, component, Component::getResolvedLicense, persistentComponent::setResolvedLicense); applyIfChanged(persistentComponent, component, Component::getLicense, persistentComponent::setLicense); applyIfChanged(persistentComponent, component, Component::getLicenseUrl, persistentComponent::setLicenseUrl); + applyIfChanged(persistentComponent, component, Component::getLicenseExpression, persistentComponent::setLicenseExpression); applyIfChanged(persistentComponent, component, Component::isInternal, persistentComponent::setInternal); applyIfChanged(persistentComponent, component, Component::getExternalReferences, persistentComponent::setExternalReferences); idsOfComponentsToDelete.remove(persistentComponent.getId()); } + processComponentProperties(qm, persistentComponent, component.getProperties()); + // Update component identities in our Identity->BOMRef map, // as after persisting the components, their identities now include UUIDs. final var newIdentity = new ComponentIdentity(persistentComponent); @@ -463,6 +468,39 @@ private Map processComponents(final QueryManager q return persistentComponents; } + private void processComponentProperties(final QueryManager qm, final Component component, final List properties) { + if (properties == null || properties.isEmpty()) { + // TODO: Should we delete pre-existing properties that no longer exist in the BOM? + return; + } + + if (component.getProperties() == null || component.getProperties().isEmpty()) { + for (final ComponentProperty property : properties) { + property.setComponent(component); + qm.getPersistenceManager().makePersistent(property); + } + + return; + } + + for (final ComponentProperty property : component.getProperties()) { + final Optional optionalPersistentProperty = component.getProperties().stream() + .filter(persistentProperty -> Objects.equals(persistentProperty.getGroupName(), property.getGroupName())) + .filter(persistentProperty -> Objects.equals(persistentProperty.getPropertyName(), property.getPropertyName())) + .findFirst(); + if (optionalPersistentProperty.isEmpty()) { + property.setComponent(component); + qm.getPersistenceManager().makePersistent(property); + continue; + } + + final ComponentProperty persistentProperty = optionalPersistentProperty.get(); + applyIfChanged(persistentProperty, property, ComponentProperty::getPropertyValue, persistentProperty::setPropertyValue); + applyIfChanged(persistentProperty, property, ComponentProperty::getPropertyType, persistentProperty::setPropertyType); + applyIfChanged(persistentProperty, property, ComponentProperty::getDescription, persistentProperty::setDescription); + } + } + private Map processServices(final QueryManager qm, final Project project, final List services, diff --git a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java index 2b3ed2cf83..bf7e5b0192 100644 --- a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java +++ b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java @@ -20,6 +20,7 @@ import alpine.event.framework.Event; import alpine.event.framework.EventService; +import alpine.model.IConfigProperty.PropertyType; import alpine.notification.Notification; import alpine.notification.NotificationLevel; import alpine.notification.NotificationService; @@ -177,7 +178,7 @@ public void informTest() throws Exception { final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), resourceToByteArray("/unit/bom-1.xml")); bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); - assertConditionWithTimeout(() -> NOTIFICATIONS.size() >= 6, Duration.ofSeconds(5)); + awaitBomProcessedNotification(); qm.getPersistenceManager().refresh(project); assertThat(project.getClassifier()).isEqualTo(Classifier.APPLICATION); @@ -237,6 +238,22 @@ public void informTest() throws Exception { assertThat(component.getCpe()).isEqualTo("cpe:/a:example:xmlutil:1.0.0"); assertThat(component.getPurl().canonicalize()).isEqualTo("pkg:maven/com.example/xmlutil@1.0.0?packaging=jar"); assertThat(component.getLicenseUrl()).isEqualTo("https://www.apache.org/licenses/LICENSE-2.0.txt"); + assertThat(component.getProperties()).satisfiesExactly( + property -> { + assertThat(property.getGroupName()).isEqualTo("foo"); + assertThat(property.getPropertyName()).isEqualTo("bar"); + assertThat(property.getPropertyValue()).isEqualTo("baz"); + assertThat(property.getPropertyType()).isEqualTo(PropertyType.STRING); + assertThat(property.getDescription()).isNull(); + }, + property -> { + assertThat(property.getGroupName()).isEqualTo("internal"); + assertThat(property.getPropertyName()).isEqualTo("foo"); + assertThat(property.getPropertyValue()).isEqualTo("bar"); + assertThat(property.getPropertyType()).isEqualTo(PropertyType.STRING); + assertThat(property.getDescription()).isNull(); + } + ); assertThat(qm.getAllVulnerabilities(component)).hasSize(2); assertThat(NOTIFICATIONS).satisfiesExactly( diff --git a/src/test/resources/unit/bom-1.xml b/src/test/resources/unit/bom-1.xml index f647f67487..f64a234bc2 100644 --- a/src/test/resources/unit/bom-1.xml +++ b/src/test/resources/unit/bom-1.xml @@ -86,6 +86,11 @@ cpe:/a:example:xmlutil:1.0.0 pkg:maven/com.example/xmlutil@1.0.0?packaging=jar false + + foo + bar + baz + From 5164bb88701774838edcc3662589782d279060e9 Mon Sep 17 00:00:00 2001 From: nscuro Date: Mon, 26 Feb 2024 23:43:37 +0100 Subject: [PATCH 03/17] Update Trivy IT to use OS component and properties Relates to #3369 Signed-off-by: nscuro --- .../TrivyAnalysisTaskIntegrationTest.java | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/src/test/java/org/dependencytrack/tasks/scanners/TrivyAnalysisTaskIntegrationTest.java b/src/test/java/org/dependencytrack/tasks/scanners/TrivyAnalysisTaskIntegrationTest.java index 90f2662e97..e2261d8a15 100644 --- a/src/test/java/org/dependencytrack/tasks/scanners/TrivyAnalysisTaskIntegrationTest.java +++ b/src/test/java/org/dependencytrack/tasks/scanners/TrivyAnalysisTaskIntegrationTest.java @@ -18,6 +18,7 @@ */ package org.dependencytrack.tasks.scanners; +import alpine.model.IConfigProperty; import alpine.security.crypto.DataEncryption; import org.dependencytrack.PersistenceCapableTest; import org.dependencytrack.event.TrivyAnalysisEvent; @@ -177,19 +178,30 @@ public void testWithUnrecognizedPackageName() { final var project = new Project(); project.setName("acme-app"); qm.persist(project); - - final var componentA = new Component(); - componentA.setProject(project); - componentA.setName("libc6"); - componentA.setVersion("2.35-0ubuntu3.4"); - componentA.setClassifier(Classifier.LIBRARY); - componentA.setPurl("pkg:deb/ubuntu/libc6@2.35-0ubuntu3.4?arch=amd64&distro=ubuntu-22.04"); - qm.persist(componentA); - - final var analysisEvent = new TrivyAnalysisEvent(List.of(componentA)); + + final var osComponent = new Component(); + osComponent.setProject(project); + osComponent.setName("ubuntu"); + osComponent.setVersion("22.04"); + osComponent.setClassifier(Classifier.OPERATING_SYSTEM); + qm.persist(osComponent); + + final var component = new Component(); + component.setProject(project); + component.setName("libc6"); + component.setVersion("2.35-0ubuntu3.4"); + component.setClassifier(Classifier.LIBRARY); + component.setPurl("pkg:deb/ubuntu/libc6@2.35-0ubuntu3.4?arch=amd64&distro=ubuntu-22.04"); + qm.persist(component); + + qm.createComponentProperty(component, "aquasecurity:trivy", "SrcName", "glibc", IConfigProperty.PropertyType.STRING, null); + qm.createComponentProperty(component, "aquasecurity:trivy", "SrcVersion", "2.35", IConfigProperty.PropertyType.STRING, null); + qm.createComponentProperty(component, "aquasecurity:trivy", "SrcRelease", "0ubuntu3.4", IConfigProperty.PropertyType.STRING, null); + + final var analysisEvent = new TrivyAnalysisEvent(List.of(osComponent, component)); new TrivyAnalysisTask().inform(analysisEvent); - assertThat(qm.getAllVulnerabilities(componentA)).isEmpty(); + assertThat(qm.getAllVulnerabilities(component)).isEmpty(); } } From 9bc453c107b56df2e9e12373290917a7d83c4c25 Mon Sep 17 00:00:00 2001 From: nscuro Date: Tue, 27 Feb 2024 09:42:56 +0100 Subject: [PATCH 04/17] Skip property assertions for legacy BOM processing task Signed-off-by: nscuro --- .../tasks/BomUploadProcessingTaskTest.java | 51 +++++++++++-------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java index bf7e5b0192..0989d8a616 100644 --- a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java +++ b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java @@ -96,17 +96,20 @@ public void inform(final Notification notification) { private static final ConcurrentLinkedQueue EVENTS = new ConcurrentLinkedQueue<>(); private static final ConcurrentLinkedQueue NOTIFICATIONS = new ConcurrentLinkedQueue<>(); - @Parameterized.Parameters + @Parameterized.Parameters(name = "{index}: {0}") public static Collection data() { return Arrays.asList(new Object[][]{ - {(Supplier) BomUploadProcessingTask::new}, - {(Supplier) BomUploadProcessingTaskV2::new} + {BomUploadProcessingTask.class.getSimpleName(), (Supplier) BomUploadProcessingTask::new}, + {BomUploadProcessingTaskV2.class.getSimpleName(), (Supplier) BomUploadProcessingTaskV2::new} }); } + private final String bomUploadProcessingTaskName; private final Supplier bomUploadProcessingTaskSupplier; - public BomUploadProcessingTaskTest(final Supplier bomUploadProcessingTaskSupplier) { + public BomUploadProcessingTaskTest(final String bomUploadProcessingTaskName, + final Supplier bomUploadProcessingTaskSupplier) { + this.bomUploadProcessingTaskName = bomUploadProcessingTaskName; this.bomUploadProcessingTaskSupplier = bomUploadProcessingTaskSupplier; } @@ -238,22 +241,26 @@ public void informTest() throws Exception { assertThat(component.getCpe()).isEqualTo("cpe:/a:example:xmlutil:1.0.0"); assertThat(component.getPurl().canonicalize()).isEqualTo("pkg:maven/com.example/xmlutil@1.0.0?packaging=jar"); assertThat(component.getLicenseUrl()).isEqualTo("https://www.apache.org/licenses/LICENSE-2.0.txt"); - assertThat(component.getProperties()).satisfiesExactly( - property -> { - assertThat(property.getGroupName()).isEqualTo("foo"); - assertThat(property.getPropertyName()).isEqualTo("bar"); - assertThat(property.getPropertyValue()).isEqualTo("baz"); - assertThat(property.getPropertyType()).isEqualTo(PropertyType.STRING); - assertThat(property.getDescription()).isNull(); - }, - property -> { - assertThat(property.getGroupName()).isEqualTo("internal"); - assertThat(property.getPropertyName()).isEqualTo("foo"); - assertThat(property.getPropertyValue()).isEqualTo("bar"); - assertThat(property.getPropertyType()).isEqualTo(PropertyType.STRING); - assertThat(property.getDescription()).isNull(); - } - ); + + // TODO: Implement for legacy version of the task. + if (bomUploadProcessingTaskSupplier.get() instanceof BomUploadProcessingTaskV2) { + assertThat(component.getProperties()).satisfiesExactly( + property -> { + assertThat(property.getGroupName()).isEqualTo("foo"); + assertThat(property.getPropertyName()).isEqualTo("bar"); + assertThat(property.getPropertyValue()).isEqualTo("baz"); + assertThat(property.getPropertyType()).isEqualTo(PropertyType.STRING); + assertThat(property.getDescription()).isNull(); + }, + property -> { + assertThat(property.getGroupName()).isEqualTo("internal"); + assertThat(property.getPropertyName()).isEqualTo("foo"); + assertThat(property.getPropertyValue()).isEqualTo("bar"); + assertThat(property.getPropertyType()).isEqualTo(PropertyType.STRING); + assertThat(property.getDescription()).isNull(); + } + ); + } assertThat(qm.getAllVulnerabilities(component)).hasSize(2); assertThat(NOTIFICATIONS).satisfiesExactly( @@ -263,12 +270,12 @@ public void informTest() throws Exception { n -> { assertThat(n.getGroup()).isEqualTo(NotificationGroup.NEW_VULNERABILITY.name()); NewVulnerabilityIdentified nvi = (NewVulnerabilityIdentified) n.getSubject(); - assertThat(nvi.getVulnerabilityAnalysisLevel().equals(VulnerabilityAnalysisLevel.BOM_UPLOAD_ANALYSIS)); + assertThat(nvi.getVulnerabilityAnalysisLevel()).isEqualTo(VulnerabilityAnalysisLevel.BOM_UPLOAD_ANALYSIS); }, n -> { assertThat(n.getGroup()).isEqualTo(NotificationGroup.NEW_VULNERABILITY.name()); NewVulnerabilityIdentified nvi = (NewVulnerabilityIdentified) n.getSubject(); - assertThat(nvi.getVulnerabilityAnalysisLevel().toString().equals(VulnerabilityAnalysisLevel.BOM_UPLOAD_ANALYSIS)); + assertThat(nvi.getVulnerabilityAnalysisLevel()).isEqualTo(VulnerabilityAnalysisLevel.BOM_UPLOAD_ANALYSIS); }, n -> assertThat(n.getGroup()).isEqualTo(NotificationGroup.NEW_VULNERABLE_DEPENDENCY.name()) ); From 57cf065f2d16c5b46bfae420e6a529abbeb59ba6 Mon Sep 17 00:00:00 2001 From: nscuro Date: Sat, 16 Mar 2024 14:02:46 +0100 Subject: [PATCH 05/17] Include project and component properties in CycloneDX exports Signed-off-by: nscuro --- .../parser/cyclonedx/util/ModelConverter.java | 21 ++++++++++- .../resources/v1/BomResourceTest.java | 37 ++++++++++++++++++- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java b/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java index 20469f1a58..4931162af5 100644 --- a/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java +++ b/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java @@ -19,6 +19,7 @@ package org.dependencytrack.parser.cyclonedx.util; import alpine.common.logging.Logger; +import alpine.model.IConfigProperty; import alpine.model.IConfigProperty.PropertyType; import com.github.packageurl.MalformedPackageURLException; import com.github.packageurl.PackageURL; @@ -705,6 +706,7 @@ public static org.cyclonedx.model.Component convert(final QueryManager qm, final cycloneComponent.setCpe(StringUtils.trimToNull(component.getCpe())); cycloneComponent.setAuthor(StringUtils.trimToNull(component.getAuthor())); cycloneComponent.setSupplier(convert(component.getSupplier())); + cycloneComponent.setProperties(convert(component.getProperties())); if (component.getSwidTagId() != null) { final Swid swid = new Swid(); @@ -766,7 +768,7 @@ public static org.cyclonedx.model.Component convert(final QueryManager qm, final } - if (component.getExternalReferences() != null && component.getExternalReferences().size() > 0) { + if (component.getExternalReferences() != null && !component.getExternalReferences().isEmpty()) { List references = new ArrayList<>(); for (ExternalReference ref: component.getExternalReferences()) { org.cyclonedx.model.ExternalReference cdxRef = new org.cyclonedx.model.ExternalReference(); @@ -799,6 +801,22 @@ public static org.cyclonedx.model.Component convert(final QueryManager qm, final return cycloneComponent; } + private static List convert(final Collection dtProperties) { + if (dtProperties == null || dtProperties.isEmpty()) { + return Collections.emptyList(); + } + + final List cdxProperties = new ArrayList<>(); + for (final T dtProperty : dtProperties) { + final var cdxProperty = new org.cyclonedx.model.Property(); + cdxProperty.setName("%s:%s".formatted(dtProperty.getGroupName(), dtProperty.getPropertyName())); + cdxProperty.setValue(dtProperty.getPropertyValue()); + cdxProperties.add(cdxProperty); + } + + return cdxProperties; + } + public static org.cyclonedx.model.Metadata createMetadata(final Project project) { final org.cyclonedx.model.Metadata metadata = new org.cyclonedx.model.Metadata(); final org.cyclonedx.model.Tool tool = new org.cyclonedx.model.Tool(); @@ -849,6 +867,7 @@ public static org.cyclonedx.model.Metadata createMetadata(final Project project) cycloneComponent.setExternalReferences(references); } cycloneComponent.setSupplier(convert(project.getSupplier())); + cycloneComponent.setProperties(convert(project.getProperties())); metadata.setComponent(cycloneComponent); if (project.getMetadata() != null) { diff --git a/src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java b/src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java index 80bb4cc26d..3d4e66518b 100644 --- a/src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java +++ b/src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java @@ -19,6 +19,7 @@ package org.dependencytrack.resources.v1; import alpine.common.util.UuidUtil; +import alpine.model.IConfigProperty; import alpine.server.filters.ApiFilter; import alpine.server.filters.AuthenticationFilter; import com.fasterxml.jackson.core.StreamReadConstraints; @@ -29,10 +30,12 @@ import org.dependencytrack.model.AnalysisState; import org.dependencytrack.model.Classifier; import org.dependencytrack.model.Component; +import org.dependencytrack.model.ComponentProperty; import org.dependencytrack.model.OrganizationalContact; import org.dependencytrack.model.OrganizationalEntity; import org.dependencytrack.model.Project; import org.dependencytrack.model.ProjectMetadata; +import org.dependencytrack.model.ProjectProperty; import org.dependencytrack.model.Severity; import org.dependencytrack.model.Vulnerability; import org.dependencytrack.parser.cyclonedx.CycloneDxValidator; @@ -115,6 +118,7 @@ public void exportProjectAsCycloneDxInventoryTest() { projectManufacturer.setName("projectManufacturer"); final var projectSupplier = new OrganizationalEntity(); projectSupplier.setName("projectSupplier"); + var project = new Project(); project.setName("acme-app"); project.setClassifier(Classifier.APPLICATION); @@ -122,6 +126,14 @@ public void exportProjectAsCycloneDxInventoryTest() { project.setSupplier(projectSupplier); project = qm.createProject(project, null, false); + final var projectProperty = new ProjectProperty(); + projectProperty.setProject(project); + projectProperty.setGroupName("foo"); + projectProperty.setPropertyName("bar"); + projectProperty.setPropertyValue("baz"); + projectProperty.setPropertyType(IConfigProperty.PropertyType.STRING); + qm.persist(projectProperty); + final var bomSupplier = new OrganizationalEntity(); bomSupplier.setName("bomSupplier"); final var bomAuthor = new OrganizationalContact(); @@ -134,6 +146,7 @@ public void exportProjectAsCycloneDxInventoryTest() { final var componentSupplier = new OrganizationalEntity(); componentSupplier.setName("componentSupplier"); + var componentWithoutVuln = new Component(); componentWithoutVuln.setProject(project); componentWithoutVuln.setName("acme-lib-a"); @@ -142,6 +155,14 @@ public void exportProjectAsCycloneDxInventoryTest() { componentWithoutVuln.setDirectDependencies("[]"); componentWithoutVuln = qm.createComponent(componentWithoutVuln, false); + final var componentProperty = new ComponentProperty(); + componentProperty.setComponent(componentWithoutVuln); + componentProperty.setGroupName("foo"); + componentProperty.setPropertyName("bar"); + componentProperty.setPropertyValue("baz"); + componentProperty.setPropertyType(IConfigProperty.PropertyType.STRING); + qm.persist(componentProperty); + var componentWithVuln = new Component(); componentWithVuln.setProject(project); componentWithVuln.setName("acme-lib-b"); @@ -214,7 +235,13 @@ public void exportProjectAsCycloneDxInventoryTest() { "name": "projectSupplier" }, "name": "acme-app", - "version": "SNAPSHOT" + "version": "SNAPSHOT", + "properties": [ + { + "name": "foo:bar", + "value": "baz" + } + ] }, "manufacture": { "name": "projectManufacturer" @@ -238,7 +265,13 @@ public void exportProjectAsCycloneDxInventoryTest() { "name": "componentSupplier" }, "name": "acme-lib-a", - "version": "1.0.0" + "version": "1.0.0", + "properties": [ + { + "name": "foo:bar", + "value": "baz" + } + ] }, { "type": "library", From d812964fe8cc32b3e750f0ac675aa023b4e2e010 Mon Sep 17 00:00:00 2001 From: nscuro Date: Tue, 26 Mar 2024 20:26:55 +0100 Subject: [PATCH 06/17] Apply copyright change Signed-off-by: nscuro --- .../java/org/dependencytrack/model/ComponentProperty.java | 4 +--- .../resources/v1/ComponentPropertyResource.java | 2 +- .../resources/v1/ComponentPropertyResourceTest.java | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/dependencytrack/model/ComponentProperty.java b/src/main/java/org/dependencytrack/model/ComponentProperty.java index ccac4048dc..e56cc41489 100644 --- a/src/main/java/org/dependencytrack/model/ComponentProperty.java +++ b/src/main/java/org/dependencytrack/model/ComponentProperty.java @@ -14,7 +14,7 @@ * limitations under the License. * * SPDX-License-Identifier: Apache-2.0 - * Copyright (c) Steve Springett. All Rights Reserved. + * Copyright (c) OWASP Foundation. All Rights Reserved. */ package org.dependencytrack.model; @@ -29,7 +29,6 @@ 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; @@ -40,7 +39,6 @@ * @since 4.11.0 */ @PersistenceCapable(table = "COMPONENT_PROPERTY") -@Unique(name = "COMPONENT_PROPERTY_KEYS_IDX", members = {"component", "groupName", "propertyName"}) @JsonInclude(JsonInclude.Include.NON_NULL) public class ComponentProperty implements IConfigProperty, Serializable { diff --git a/src/main/java/org/dependencytrack/resources/v1/ComponentPropertyResource.java b/src/main/java/org/dependencytrack/resources/v1/ComponentPropertyResource.java index 3a5eaac9e3..d45ef63da5 100644 --- a/src/main/java/org/dependencytrack/resources/v1/ComponentPropertyResource.java +++ b/src/main/java/org/dependencytrack/resources/v1/ComponentPropertyResource.java @@ -14,7 +14,7 @@ * limitations under the License. * * SPDX-License-Identifier: Apache-2.0 - * Copyright (c) Steve Springett. All Rights Reserved. + * Copyright (c) OWASP Foundation. All Rights Reserved. */ package org.dependencytrack.resources.v1; diff --git a/src/test/java/org/dependencytrack/resources/v1/ComponentPropertyResourceTest.java b/src/test/java/org/dependencytrack/resources/v1/ComponentPropertyResourceTest.java index 1983ea7de6..f680d492c4 100644 --- a/src/test/java/org/dependencytrack/resources/v1/ComponentPropertyResourceTest.java +++ b/src/test/java/org/dependencytrack/resources/v1/ComponentPropertyResourceTest.java @@ -14,7 +14,7 @@ * limitations under the License. * * SPDX-License-Identifier: Apache-2.0 - * Copyright (c) Steve Springett. All Rights Reserved. + * Copyright (c) OWASP Foundation. All Rights Reserved. */ package org.dependencytrack.resources.v1; From d489c2c2a8cd6bd6ef94efaa5595dd38892e50e8 Mon Sep 17 00:00:00 2001 From: nscuro Date: Mon, 1 Apr 2024 17:18:01 +0200 Subject: [PATCH 07/17] Add UUID validation; Add Swagger docs for required permissions Signed-off-by: nscuro --- .../v1/ComponentPropertyResource.java | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/dependencytrack/resources/v1/ComponentPropertyResource.java b/src/main/java/org/dependencytrack/resources/v1/ComponentPropertyResource.java index d45ef63da5..ce33d372ab 100644 --- a/src/main/java/org/dependencytrack/resources/v1/ComponentPropertyResource.java +++ b/src/main/java/org/dependencytrack/resources/v1/ComponentPropertyResource.java @@ -29,6 +29,7 @@ import org.dependencytrack.auth.Permissions; import org.dependencytrack.model.Component; import org.dependencytrack.model.ComponentProperty; +import org.dependencytrack.model.validation.ValidUuid; import org.dependencytrack.persistence.QueryManager; import javax.validation.Validator; @@ -56,17 +57,18 @@ public class ComponentPropertyResource extends AbstractConfigPropertyResource { @ApiOperation( value = "Returns a list of all ComponentProperties for the specified component", response = ComponentProperty.class, - responseContainer = "List" + responseContainer = "List", + notes = "

Requires permission VIEW_PORTFOLIO

" ) @ApiResponses(value = { @ApiResponse(code = 401, message = "Unauthorized"), @ApiResponse(code = 403, message = "Access to the specified project is forbidden"), @ApiResponse(code = 404, message = "The project could not be found") }) - @PermissionRequired(Permissions.Constants.PORTFOLIO_MANAGEMENT) + @PermissionRequired(Permissions.Constants.VIEW_PORTFOLIO) public Response getProperties( - @ApiParam(value = "The UUID of the component to retrieve properties for", required = true) - @PathParam("uuid") String uuid) { + @ApiParam(value = "The UUID of the component to retrieve properties for", format = "uuid", required = true) + @PathParam("uuid") @ValidUuid String uuid) { try (QueryManager qm = new QueryManager(getAlpineRequest())) { final Component component = qm.getObjectByUuid(Component.class, uuid); if (component != null) { @@ -98,7 +100,8 @@ public Response getProperties( @ApiOperation( value = "Creates a new component property", response = ComponentProperty.class, - code = 201 + code = 201, + notes = "

Requires permission PORTFOLIO_MANAGEMENT

" ) @ApiResponses(value = { @ApiResponse(code = 401, message = "Unauthorized"), @@ -108,8 +111,8 @@ public Response getProperties( }) @PermissionRequired(Permissions.Constants.PORTFOLIO_MANAGEMENT) public Response createProperty( - @ApiParam(value = "The UUID of the component to create a property for", required = true) - @PathParam("uuid") String uuid, + @ApiParam(value = "The UUID of the component to create a property for", format = "uuid", required = true) + @PathParam("uuid") @ValidUuid String uuid, ComponentProperty json) { final Validator validator = super.getValidator(); failOnValidationError( @@ -154,7 +157,8 @@ public Response createProperty( @Produces(MediaType.APPLICATION_JSON) @ApiOperation( value = "Updates a project property", - response = ComponentProperty.class + response = ComponentProperty.class, + notes = "

Requires permission PORTFOLIO_MANAGEMENT

" ) @ApiResponses(value = { @ApiResponse(code = 401, message = "Unauthorized"), @@ -163,8 +167,8 @@ public Response createProperty( }) @PermissionRequired(Permissions.Constants.PORTFOLIO_MANAGEMENT) public Response updateProperty( - @ApiParam(value = "The UUID of the component to create a property for", required = true) - @PathParam("uuid") String uuid, + @ApiParam(value = "The UUID of the component to create a property for", format = "uuid", required = true) + @PathParam("uuid") @ValidUuid String uuid, ComponentProperty json) { final Validator validator = super.getValidator(); failOnValidationError( @@ -196,7 +200,8 @@ public Response updateProperty( @Produces(MediaType.APPLICATION_JSON) @ApiOperation( value = "Deletes a config property", - response = ComponentProperty.class + response = ComponentProperty.class, + notes = "

Requires permission PORTFOLIO_MANAGEMENT

" ) @ApiResponses(value = { @ApiResponse(code = 401, message = "Unauthorized"), @@ -205,8 +210,8 @@ public Response updateProperty( }) @PermissionRequired(Permissions.Constants.PORTFOLIO_MANAGEMENT) public Response deleteProperty( - @ApiParam(value = "The UUID of the component to delete a property from", required = true) - @PathParam("uuid") String uuid, + @ApiParam(value = "The UUID of the component to delete a property from", format = "uuid", required = true) + @PathParam("uuid") @ValidUuid String uuid, ComponentProperty json) { final Validator validator = super.getValidator(); failOnValidationError( From 59e8e2244e7ba0fa5523b4ed59ffa5b1f24a18fb Mon Sep 17 00:00:00 2001 From: nscuro Date: Mon, 8 Apr 2024 23:49:20 +0200 Subject: [PATCH 08/17] Make `groupName` optional for component properties This is primarily to handle CycloneDX properties which do not use namespaces. Signed-off-by: nscuro --- .../model/ComponentProperty.java | 20 +++- .../parser/cyclonedx/util/ModelConverter.java | 26 ++++-- .../tasks/BomUploadProcessingTaskV2.java | 12 ++- .../tasks/BomUploadProcessingTaskTest.java | 93 ++++++++++++++++++- 4 files changed, 134 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/dependencytrack/model/ComponentProperty.java b/src/main/java/org/dependencytrack/model/ComponentProperty.java index e56cc41489..0337c5d568 100644 --- a/src/main/java/org/dependencytrack/model/ComponentProperty.java +++ b/src/main/java/org/dependencytrack/model/ComponentProperty.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.google.common.base.MoreObjects; import javax.jdo.annotations.Column; import javax.jdo.annotations.IdGeneratorStrategy; @@ -54,8 +55,7 @@ public class ComponentProperty implements IConfigProperty, Serializable { private Component component; @Persistent - @Column(name = "GROUPNAME", allowsNull = "false") - @NotBlank + @Column(name = "GROUPNAME") @Size(min = 1, max = 255) @JsonDeserialize(using = TrimmedStringDeserializer.class) @Pattern(regexp = "\\P{Cc}+", message = "The groupName must not contain control characters") @@ -71,7 +71,7 @@ public class ComponentProperty implements IConfigProperty, Serializable { @Persistent @Column(name = "PROPERTYVALUE", length = 1024) - @Size(min = 0, max = 1024) + @Size(max = 1024) @JsonDeserialize(using = TrimmedStringDeserializer.class) @Pattern(regexp = "\\P{Cc}+", message = "The propertyValue must not contain control characters") private String propertyValue; @@ -144,4 +144,18 @@ public void setDescription(final String description) { this.description = description; } + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("id", id) + .add("component", component) + .add("groupName", groupName) + .add("propertyName", propertyName) + .add("propertyValue", propertyValue) + .add("propertyType", propertyType) + .add("description", description) + .omitNullValues() + .toString(); + } + } \ No newline at end of file diff --git a/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java b/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java index 4931162af5..1028ca846b 100644 --- a/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java +++ b/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java @@ -281,21 +281,17 @@ private static ComponentProperty convertToComponentProperty(final org.cyclonedx. final String cdxPropertyName = trimToNull(cdxProperty.getName()); if (cdxPropertyName == null) { - // TODO: What to do here? - // * Generate groupName and propertyName? - // * Log a warning and ignore? return null; } // Treat property names according to the CycloneDX namespace syntax: - // https://cyclonedx.github.io/cyclonedx-property-taxonomy/ - final int lastSeparatorIndex = cdxPropertyName.lastIndexOf(':'); - if (lastSeparatorIndex < 0) { - property.setGroupName("internal"); + // https://cyclonedx.github.io/cyclonedx-property-taxonomy/ + final int firstSeparatorIndex = cdxPropertyName.indexOf(':'); + if (firstSeparatorIndex < 0) { property.setPropertyName(cdxPropertyName); } else { - property.setGroupName(cdxPropertyName.substring(0, lastSeparatorIndex)); - property.setPropertyName(cdxPropertyName.substring(lastSeparatorIndex + 1)); + property.setGroupName(cdxPropertyName.substring(0, firstSeparatorIndex)); + property.setPropertyName(cdxPropertyName.substring(firstSeparatorIndex + 1)); } return property; @@ -808,8 +804,18 @@ private static List co final List cdxProperties = new ArrayList<>(); for (final T dtProperty : dtProperties) { + if (dtProperty.getPropertyType() == PropertyType.ENCRYPTEDSTRING) { + // We treat encrypted properties as internal. + // They shall not be leaked when exporting. + continue; + } + final var cdxProperty = new org.cyclonedx.model.Property(); - cdxProperty.setName("%s:%s".formatted(dtProperty.getGroupName(), dtProperty.getPropertyName())); + if (dtProperty.getGroupName() == null) { + cdxProperty.setName(dtProperty.getPropertyName()); + } else { + cdxProperty.setName("%s:%s".formatted(dtProperty.getGroupName(), dtProperty.getPropertyName())); + } cdxProperty.setValue(dtProperty.getPropertyValue()); cdxProperties.add(cdxProperty); } diff --git a/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java b/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java index c27a01fe2b..0ec410a125 100644 --- a/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java +++ b/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java @@ -470,7 +470,15 @@ private Map processComponents(final QueryManager q private void processComponentProperties(final QueryManager qm, final Component component, final List properties) { if (properties == null || properties.isEmpty()) { - // TODO: Should we delete pre-existing properties that no longer exist in the BOM? + // TODO: We currently remove all existing properties that are no longer included in the BOM. + // This is to stay consistent with the BOM being the source of truth. However, this may feel + // counter-intuitive to some users, who might expect their manual changes to persist. + // If we want to support that, we need a way to track which properties were added and / or + // modified manually. + if (component.getProperties() != null) { + qm.getPersistenceManager().deletePersistentAll(component.getProperties()); + } + return; } @@ -483,7 +491,7 @@ private void processComponentProperties(final QueryManager qm, final Component c return; } - for (final ComponentProperty property : component.getProperties()) { + for (final ComponentProperty property : properties) { final Optional optionalPersistentProperty = component.getProperties().stream() .filter(persistentProperty -> Objects.equals(persistentProperty.getGroupName(), property.getGroupName())) .filter(persistentProperty -> Objects.equals(persistentProperty.getPropertyName(), property.getPropertyName())) diff --git a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java index 0989d8a616..72e46ec2cd 100644 --- a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java +++ b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java @@ -35,6 +35,7 @@ import org.dependencytrack.model.Bom; import org.dependencytrack.model.Classifier; import org.dependencytrack.model.Component; +import org.dependencytrack.model.ComponentProperty; import org.dependencytrack.model.ConfigPropertyConstants; import org.dependencytrack.model.License; import org.dependencytrack.model.Project; @@ -244,7 +245,7 @@ public void informTest() throws Exception { // TODO: Implement for legacy version of the task. if (bomUploadProcessingTaskSupplier.get() instanceof BomUploadProcessingTaskV2) { - assertThat(component.getProperties()).satisfiesExactly( + assertThat(component.getProperties()).satisfiesExactlyInAnyOrder( property -> { assertThat(property.getGroupName()).isEqualTo("foo"); assertThat(property.getPropertyName()).isEqualTo("bar"); @@ -253,7 +254,7 @@ public void informTest() throws Exception { assertThat(property.getDescription()).isNull(); }, property -> { - assertThat(property.getGroupName()).isEqualTo("internal"); + assertThat(property.getGroupName()).isNull(); assertThat(property.getPropertyName()).isEqualTo("foo"); assertThat(property.getPropertyValue()).isEqualTo("bar"); assertThat(property.getPropertyType()).isEqualTo(PropertyType.STRING); @@ -917,6 +918,94 @@ public void informWithBomContainingServiceTest() throws Exception { assertThat(qm.getAllServiceComponents(project)).isNotEmpty(); } + @Test + public void informWithExistingComponentPropertiesAndBomWithoutComponentProperties() { + final var project = new Project(); + project.setName("acme-app"); + qm.persist(project); + + final var component = new Component(); + component.setProject(project); + component.setName("acme-lib"); + component.setClassifier(Classifier.LIBRARY); + qm.persist(component); + + final var componentProperty = new ComponentProperty(); + componentProperty.setComponent(component); + componentProperty.setPropertyName("foo"); + componentProperty.setPropertyValue("bar"); + componentProperty.setPropertyType(PropertyType.STRING); + qm.persist(componentProperty); + + final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), """ + { + "bomFormat": "CycloneDX", + "specVersion": "1.4", + "serialNumber": "urn:uuid:3e671687-395b-41f5-a30f-a58921a69b79", + "version": 1, + "components": [ + { + "type": "library", + "name": "acme-lib" + } + ] + } + """.getBytes()); + bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); + awaitBomProcessedNotification(); + + qm.getPersistenceManager().refresh(component); + assertThat(component.getProperties()).isEmpty(); + } + + @Test + public void informWithExistingComponentPropertiesAndBomWithComponentProperties() { + final var project = new Project(); + project.setName("acme-app"); + qm.persist(project); + + final var component = new Component(); + component.setProject(project); + component.setName("acme-lib"); + component.setClassifier(Classifier.LIBRARY); + qm.persist(component); + + final var componentProperty = new ComponentProperty(); + componentProperty.setComponent(component); + componentProperty.setPropertyName("foo"); + componentProperty.setPropertyValue("bar"); + componentProperty.setPropertyType(PropertyType.STRING); + qm.persist(componentProperty); + + final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), """ + { + "bomFormat": "CycloneDX", + "specVersion": "1.4", + "serialNumber": "urn:uuid:3e671687-395b-41f5-a30f-a58921a69b79", + "version": 1, + "components": [ + { + "type": "library", + "name": "acme-lib", + "properties": [ + { + "name": "foo", + "value": "baz" + } + ] + } + ] + } + """.getBytes()); + bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); + awaitBomProcessedNotification(); + + qm.getPersistenceManager().refresh(componentProperty); + assertThat(componentProperty.getGroupName()).isNull(); + assertThat(componentProperty.getPropertyName()).isEqualTo("foo"); + assertThat(componentProperty.getPropertyValue()).isEqualTo("baz"); + } + @Test // https://github.com/DependencyTrack/dependency-track/issues/1905 public void informIssue1905Test() throws Exception { // Known to now work with old task implementation. From 70b02e96a3e5a79086c307c128a98c8f824e162e Mon Sep 17 00:00:00 2001 From: nscuro Date: Sun, 14 Apr 2024 18:13:25 +0200 Subject: [PATCH 09/17] Handle duplicate component properties 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 --- docs/_posts/2024-xx-xx-v4.11.0.md | 2 + .../org/dependencytrack/model/Component.java | 2 +- .../model/ComponentProperty.java | 33 ++++ .../model/validation/EnumValue.java | 43 ++++++ .../model/validation/EnumValueValidator.java | 47 ++++++ .../persistence/ComponentQueryManager.java | 67 ++++---- .../persistence/QueryManager.java | 8 +- .../v1/ComponentPropertyResource.java | 81 ++-------- .../tasks/BomUploadProcessingTaskV2.java | 39 +++-- .../v1/ComponentPropertyResourceTest.java | 145 +++++++----------- .../tasks/BomUploadProcessingTaskTest.java | 26 +++- 11 files changed, 275 insertions(+), 218 deletions(-) create mode 100644 src/main/java/org/dependencytrack/model/validation/EnumValue.java create mode 100644 src/main/java/org/dependencytrack/model/validation/EnumValueValidator.java diff --git a/docs/_posts/2024-xx-xx-v4.11.0.md b/docs/_posts/2024-xx-xx-v4.11.0.md index bd45f4ae99..a2d9deb450 100644 --- a/docs/_posts/2024-xx-xx-v4.11.0.md +++ b/docs/_posts/2024-xx-xx-v4.11.0.md @@ -55,6 +55,7 @@ Community input and contributions are explicitly requested. The chart repository * 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] @@ -209,6 +210,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 diff --git a/src/main/java/org/dependencytrack/model/Component.java b/src/main/java/org/dependencytrack/model/Component.java index 7396382f53..cd4f92327a 100644 --- a/src/main/java/org/dependencytrack/model/Component.java +++ b/src/main/java/org/dependencytrack/model/Component.java @@ -332,7 +332,7 @@ public enum FetchGroup { private Collection 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 properties; @Persistent(table = "COMPONENTS_VULNERABILITIES") diff --git a/src/main/java/org/dependencytrack/model/ComponentProperty.java b/src/main/java/org/dependencytrack/model/ComponentProperty.java index 0337c5d568..15ed1a123e 100644 --- a/src/main/java/org/dependencytrack/model/ComponentProperty.java +++ b/src/main/java/org/dependencytrack/model/ComponentProperty.java @@ -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 @@ -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 @@ -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 @@ -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; } @@ -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) @@ -154,6 +186,7 @@ public String toString() { .add("propertyValue", propertyValue) .add("propertyType", propertyType) .add("description", description) + .add("uuid", uuid) .omitNullValues() .toString(); } diff --git a/src/main/java/org/dependencytrack/model/validation/EnumValue.java b/src/main/java/org/dependencytrack/model/validation/EnumValue.java new file mode 100644 index 0000000000..5e3a37a454 --- /dev/null +++ b/src/main/java/org/dependencytrack/model/validation/EnumValue.java @@ -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[] payload() default {}; + +} diff --git a/src/main/java/org/dependencytrack/model/validation/EnumValueValidator.java b/src/main/java/org/dependencytrack/model/validation/EnumValueValidator.java new file mode 100644 index 0000000000..25e4d466a3 --- /dev/null +++ b/src/main/java/org/dependencytrack/model/validation/EnumValueValidator.java @@ -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> { + + private Set 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()); + } + +} diff --git a/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java b/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java index e4e215614d..e37b2f62b1 100644 --- a/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java @@ -833,49 +833,35 @@ private void getDirectDependenciesForPathDependencies(Map 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 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 getComponentProperties(final Component component, final String groupName, final String propertyName) { + final Query 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 getComponentProperties(final Component component) { - final Query query = this.pm.newQuery(ComponentProperty.class, "component == :component"); - query.setOrdering("groupName asc, propertyName asc"); - return (List) query.execute(component); + final Query 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); @@ -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 query = pm.newQuery(ComponentProperty.class); + query.setFilter("component == :component && uuid == :uuid"); + try { + return query.deletePersistentAll(component, uuid); + } finally { + query.closeAll(); + } + } + } diff --git a/src/main/java/org/dependencytrack/persistence/QueryManager.java b/src/main/java/org/dependencytrack/persistence/QueryManager.java index 358438c2c0..7fc1516a4b 100644 --- a/src/main/java/org/dependencytrack/persistence/QueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/QueryManager.java @@ -563,8 +563,8 @@ public List 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 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, @@ -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(); } diff --git a/src/main/java/org/dependencytrack/resources/v1/ComponentPropertyResource.java b/src/main/java/org/dependencytrack/resources/v1/ComponentPropertyResource.java index ce33d372ab..9a90a8c27c 100644 --- a/src/main/java/org/dependencytrack/resources/v1/ComponentPropertyResource.java +++ b/src/main/java/org/dependencytrack/resources/v1/ComponentPropertyResource.java @@ -36,7 +36,6 @@ import javax.ws.rs.Consumes; import javax.ws.rs.DELETE; import javax.ws.rs.GET; -import javax.ws.rs.POST; import javax.ws.rs.PUT; import javax.ws.rs.Path; import javax.ws.rs.PathParam; @@ -44,6 +43,7 @@ import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import java.util.List; +import java.util.UUID; /** * @since 4.11.0 @@ -118,15 +118,20 @@ public Response createProperty( failOnValidationError( validator.validateProperty(json, "groupName"), validator.validateProperty(json, "propertyName"), - validator.validateProperty(json, "propertyValue") + validator.validateProperty(json, "propertyValue"), + validator.validateProperty(json, "propertyType") ); try (QueryManager qm = new QueryManager()) { final Component component = qm.getObjectByUuid(Component.class, uuid); if (component != null) { if (qm.hasAccess(super.getPrincipal(), component.getProject())) { - final ComponentProperty existing = qm.getComponentProperty(component, + final List existingProperties = qm.getComponentProperties(component, StringUtils.trimToNull(json.getGroupName()), StringUtils.trimToNull(json.getPropertyName())); - if (existing == null) { + final var jsonPropertyIdentity = new ComponentProperty.Identity(json); + final boolean isDuplicate = existingProperties.stream() + .map(ComponentProperty.Identity::new) + .anyMatch(jsonPropertyIdentity::equals); + if (existingProperties.isEmpty() || !isDuplicate) { final ComponentProperty property = qm.createComponentProperty(component, StringUtils.trimToNull(json.getGroupName()), StringUtils.trimToNull(json.getPropertyName()), @@ -134,57 +139,9 @@ public Response createProperty( json.getPropertyType(), StringUtils.trimToNull(json.getDescription())); updatePropertyValue(qm, json, property); - qm.getPersistenceManager().detachCopy(component); - qm.close(); - if (ComponentProperty.PropertyType.ENCRYPTEDSTRING == property.getPropertyType()) { - property.setPropertyValue(ENCRYPTED_PLACEHOLDER); - } return Response.status(Response.Status.CREATED).entity(property).build(); } else { - return Response.status(Response.Status.CONFLICT).entity("A property with the specified component/group/name combination already exists.").build(); - } - } else { - return Response.status(Response.Status.FORBIDDEN).entity("Access to the specified component is forbidden").build(); - } - } else { - return Response.status(Response.Status.NOT_FOUND).entity("The component could not be found.").build(); - } - } - } - - @POST - @Consumes(MediaType.APPLICATION_JSON) - @Produces(MediaType.APPLICATION_JSON) - @ApiOperation( - value = "Updates a project property", - response = ComponentProperty.class, - notes = "

Requires permission PORTFOLIO_MANAGEMENT

" - ) - @ApiResponses(value = { - @ApiResponse(code = 401, message = "Unauthorized"), - @ApiResponse(code = 403, message = "Access to the specified project is forbidden"), - @ApiResponse(code = 404, message = "The component could not be found"), - }) - @PermissionRequired(Permissions.Constants.PORTFOLIO_MANAGEMENT) - public Response updateProperty( - @ApiParam(value = "The UUID of the component to create a property for", format = "uuid", required = true) - @PathParam("uuid") @ValidUuid String uuid, - ComponentProperty json) { - final Validator validator = super.getValidator(); - failOnValidationError( - validator.validateProperty(json, "groupName"), - validator.validateProperty(json, "propertyName"), - validator.validateProperty(json, "propertyValue") - ); - try (QueryManager qm = new QueryManager()) { - final Component component = qm.getObjectByUuid(Component.class, uuid); - if (component != null) { - if (qm.hasAccess(super.getPrincipal(), component.getProject())) { - final ComponentProperty property = qm.getComponentProperty(component, json.getGroupName(), json.getPropertyName()); - if (property != null) { - return updatePropertyValue(qm, json, property); - } else { - return Response.status(Response.Status.NOT_FOUND).entity("A property with the specified component/group/name combination could not be found.").build(); + return Response.status(Response.Status.CONFLICT).entity("A property with the specified component/group/name/value combination already exists.").build(); } } else { return Response.status(Response.Status.FORBIDDEN).entity("Access to the specified component is forbidden").build(); @@ -196,6 +153,7 @@ public Response updateProperty( } @DELETE + @Path("/{propertyUuid}") @Consumes(MediaType.APPLICATION_JSON) @Produces(MediaType.APPLICATION_JSON) @ApiOperation( @@ -211,20 +169,15 @@ public Response updateProperty( @PermissionRequired(Permissions.Constants.PORTFOLIO_MANAGEMENT) public Response deleteProperty( @ApiParam(value = "The UUID of the component to delete a property from", format = "uuid", required = true) - @PathParam("uuid") @ValidUuid String uuid, - ComponentProperty json) { - final Validator validator = super.getValidator(); - failOnValidationError( - validator.validateProperty(json, "groupName"), - validator.validateProperty(json, "propertyName") - ); + @PathParam("uuid") @ValidUuid final String componentUuid, + @ApiParam(value = "The UUID of the component property to delete", format = "uuid", required = true) + @PathParam("propertyUuid") @ValidUuid final String propertyUuid) { try (QueryManager qm = new QueryManager()) { - final Component component = qm.getObjectByUuid(Component.class, uuid); + final Component component = qm.getObjectByUuid(Component.class, componentUuid); if (component != null) { if (qm.hasAccess(super.getPrincipal(), component.getProject())) { - final ComponentProperty property = qm.getComponentProperty(component, json.getGroupName(), json.getPropertyName()); - if (property != null) { - qm.delete(property); + final long propertiesDeleted = qm.deleteComponentPropertyByUuid(component, UUID.fromString(propertyUuid)); + if (propertiesDeleted > 0) { return Response.status(Response.Status.NO_CONTENT).build(); } else { return Response.status(Response.Status.NOT_FOUND).entity("The component property could not be found.").build(); diff --git a/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java b/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java index 0ec410a125..e4027341c0 100644 --- a/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java +++ b/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java @@ -75,11 +75,12 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.concurrent.TimeUnit; +import java.util.function.Function; import java.util.function.Predicate; +import java.util.stream.Collectors; import static org.apache.commons.lang3.StringUtils.isNotBlank; import static org.apache.commons.lang3.StringUtils.trim; @@ -491,21 +492,27 @@ private void processComponentProperties(final QueryManager qm, final Component c return; } - for (final ComponentProperty property : properties) { - final Optional optionalPersistentProperty = component.getProperties().stream() - .filter(persistentProperty -> Objects.equals(persistentProperty.getGroupName(), property.getGroupName())) - .filter(persistentProperty -> Objects.equals(persistentProperty.getPropertyName(), property.getPropertyName())) - .findFirst(); - if (optionalPersistentProperty.isEmpty()) { - property.setComponent(component); - qm.getPersistenceManager().makePersistent(property); - continue; - } + // Group properties by group, name, and value. Because CycloneDX supports duplicate + // property names, uniqueness can only be determined by also considering the value. + final var existingPropertiesByIdentity = component.getProperties().stream() + .collect(Collectors.toMap(ComponentProperty.Identity::new, Function.identity())); + final var incomingPropertiesByIdentity = properties.stream() + .collect(Collectors.toMap(ComponentProperty.Identity::new, Function.identity())); - final ComponentProperty persistentProperty = optionalPersistentProperty.get(); - applyIfChanged(persistentProperty, property, ComponentProperty::getPropertyValue, persistentProperty::setPropertyValue); - applyIfChanged(persistentProperty, property, ComponentProperty::getPropertyType, persistentProperty::setPropertyType); - applyIfChanged(persistentProperty, property, ComponentProperty::getDescription, persistentProperty::setDescription); + final var propertyIdentities = new HashSet(); + propertyIdentities.addAll(existingPropertiesByIdentity.keySet()); + propertyIdentities.addAll(incomingPropertiesByIdentity.keySet()); + + for (final ComponentProperty.Identity identity : propertyIdentities) { + final ComponentProperty existingProperty = existingPropertiesByIdentity.get(identity); + final ComponentProperty incomingProperty = incomingPropertiesByIdentity.get(identity); + + if (existingProperty == null) { + incomingProperty.setComponent(component); + qm.getPersistenceManager().makePersistent(incomingProperty); + } else if (incomingProperty == null) { + qm.getPersistenceManager().deletePersistent(existingProperty); + } } } @@ -719,7 +726,7 @@ private static void resolveAndApplyLicense(final QueryManager qm, if (isNotBlank(licenseCandidate.getName())) { final License resolvedLicense = licenseCache.computeIfAbsent(licenseCandidate.getName(), - licenseName -> resolveLicense(qm, licenseName)); + licenseName -> resolveLicense(qm, licenseName)); if (resolvedLicense != License.UNRESOLVED) { component.setResolvedLicense(resolvedLicense); component.setLicenseUrl(trimToNull(licenseCandidate.getUrl())); diff --git a/src/test/java/org/dependencytrack/resources/v1/ComponentPropertyResourceTest.java b/src/test/java/org/dependencytrack/resources/v1/ComponentPropertyResourceTest.java index f680d492c4..9756832a42 100644 --- a/src/test/java/org/dependencytrack/resources/v1/ComponentPropertyResourceTest.java +++ b/src/test/java/org/dependencytrack/resources/v1/ComponentPropertyResourceTest.java @@ -25,7 +25,6 @@ import org.dependencytrack.model.Component; import org.dependencytrack.model.ComponentProperty; import org.dependencytrack.model.Project; -import org.glassfish.jersey.client.ClientProperties; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.servlet.ServletContainer; import org.glassfish.jersey.test.DeploymentContext; @@ -39,6 +38,7 @@ import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; +import static org.hamcrest.CoreMatchers.equalTo; public class ComponentPropertyResourceTest extends ResourceTest { @@ -76,7 +76,7 @@ public void getPropertiesTest() { propertyB.setGroupName("foo-b"); propertyB.setPropertyName("bar-b"); propertyB.setPropertyValue("baz-b"); - propertyB.setPropertyType(PropertyType.ENCRYPTEDSTRING); + propertyB.setPropertyType(PropertyType.STRING); propertyB.setDescription("qux-b"); qm.persist(propertyB); @@ -86,24 +86,29 @@ public void getPropertiesTest() { assertThat(response.getStatus()).isEqualTo(200); assertThat(response.getHeaderString(TOTAL_COUNT_HEADER)).isNull(); - assertThatJson(getPlainTextBody(response)).isEqualTo(""" - [ - { - "groupName": "foo-a", - "propertyName": "bar-a", - "propertyValue": "baz-a", - "propertyType": "STRING", - "description": "qux-a" - }, - { - "groupName": "foo-b", - "propertyName": "bar-b", - "propertyValue": "HiddenDecryptedPropertyPlaceholder", - "propertyType": "ENCRYPTEDSTRING", - "description": "qux-b" - } - ] - """); + assertThatJson(getPlainTextBody(response)) + .withMatcher("property-a-uuid", equalTo(propertyA.getUuid().toString())) + .withMatcher("property-b-uuid", equalTo(propertyB.getUuid().toString())) + .isEqualTo(""" + [ + { + "groupName": "foo-a", + "propertyName": "bar-a", + "propertyValue": "baz-a", + "propertyType": "STRING", + "description": "qux-a", + "uuid": "${json-unit.matches:property-a-uuid}" + }, + { + "groupName": "foo-b", + "propertyName": "bar-b", + "propertyValue": "baz-b", + "propertyType": "STRING", + "description": "qux-b", + "uuid": "${json-unit.matches:property-b-uuid}" + } + ] + """); } @Test @@ -148,13 +153,14 @@ public void createPropertyTest() { "propertyName": "bar", "propertyValue": "baz", "propertyType": "STRING", - "description": "qux" + "description": "qux", + "uuid": "${json-unit.any-string}" } """); } @Test - public void createPropertyEncryptedTest() { + public void createPropertyWithoutGroupTest() { final var project = new Project(); project.setName("acme-app"); qm.persist(project); @@ -168,10 +174,9 @@ public void createPropertyEncryptedTest() { .header(X_API_KEY, apiKey) .put(Entity.entity(""" { - "groupName": "foo", "propertyName": "bar", "propertyValue": "baz", - "propertyType": "ENCRYPTEDSTRING", + "propertyType": "STRING", "description": "qux" } """, MediaType.APPLICATION_JSON)); @@ -180,11 +185,11 @@ public void createPropertyEncryptedTest() { assertThat(response.getHeaderString(TOTAL_COUNT_HEADER)).isNull(); assertThatJson(getPlainTextBody(response)).isEqualTo(""" { - "groupName": "foo", "propertyName": "bar", - "propertyValue": "HiddenDecryptedPropertyPlaceholder", - "propertyType": "ENCRYPTEDSTRING", - "description": "qux" + "propertyValue": "baz", + "propertyType": "STRING", + "description": "qux", + "uuid": "${json-unit.any-string}" } """); } @@ -223,11 +228,11 @@ public void createPropertyDuplicateTest() { assertThat(response.getStatus()).isEqualTo(409); assertThat(response.getHeaderString(TOTAL_COUNT_HEADER)).isNull(); assertThat(getPlainTextBody(response)).isEqualTo(""" - A property with the specified component/group/name combination already exists."""); + A property with the specified component/group/name/value combination already exists."""); } @Test - public void createPropertyInvalidTest() { + public void createPropertyDisallowedPropertyTypeTest() { final var project = new Project(); project.setName("acme-app"); qm.persist(project); @@ -237,66 +242,34 @@ public void createPropertyInvalidTest() { component.setName("acme-lib"); qm.persist(component); - final Response response = target("%s/%s/property".formatted(V1_COMPONENT, UUID.randomUUID())).request() + final Response response = target("%s/%s/property".formatted(V1_COMPONENT, component.getUuid())).request() .header(X_API_KEY, apiKey) .put(Entity.entity(""" { "groupName": "foo", "propertyName": "bar", "propertyValue": "baz", - "propertyType": "STRING", + "propertyType": "ENCRYPTEDSTRING", "description": "qux" } """, MediaType.APPLICATION_JSON)); - assertThat(response.getStatus()).isEqualTo(404); - assertThat(response.getHeaderString(TOTAL_COUNT_HEADER)).isNull(); - assertThat(getPlainTextBody(response)).isEqualTo("The component could not be found."); - } - - @Test - public void updatePropertyTest() { - final var project = new Project(); - project.setName("acme-app"); - qm.persist(project); - - final var component = new Component(); - component.setProject(project); - component.setName("acme-lib"); - qm.persist(component); - - final var property = new ComponentProperty(); - property.setComponent(component); - property.setGroupName("foo"); - property.setPropertyName("bar"); - property.setPropertyValue("baz"); - property.setPropertyType(PropertyType.STRING); - qm.persist(property); - - final Response response = target("%s/%s/property".formatted(V1_COMPONENT, component.getUuid())).request() - .header(X_API_KEY, apiKey) - .post(Entity.entity(""" - { - "groupName": "foo", - "propertyName": "bar", - "propertyValue": "qux" - } - """, MediaType.APPLICATION_JSON)); - - assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.getStatus()).isEqualTo(400); assertThat(response.getHeaderString(TOTAL_COUNT_HEADER)).isNull(); assertThatJson(getPlainTextBody(response)).isEqualTo(""" - { - "groupName": "foo", - "propertyName": "bar", - "propertyValue": "qux", - "propertyType": "STRING" - } + [ + { + "message": "Encrypted component property values are not supported", + "messageTemplate": "Encrypted component property values are not supported", + "path": "propertyType", + "invalidValue":"ENCRYPTEDSTRING" + } + ] """); } @Test - public void updatePropertyInvalidTest() { + public void createPropertyComponentNotFoundTest() { final var project = new Project(); project.setName("acme-app"); qm.persist(project); @@ -306,21 +279,15 @@ public void updatePropertyInvalidTest() { component.setName("acme-lib"); qm.persist(component); - final var property = new ComponentProperty(); - property.setComponent(component); - property.setGroupName("foo"); - property.setPropertyName("bar"); - property.setPropertyValue("baz"); - property.setPropertyType(PropertyType.STRING); - qm.persist(property); - final Response response = target("%s/%s/property".formatted(V1_COMPONENT, UUID.randomUUID())).request() .header(X_API_KEY, apiKey) - .post(Entity.entity(""" + .put(Entity.entity(""" { "groupName": "foo", "propertyName": "bar", - "propertyValue": "qux" + "propertyValue": "baz", + "propertyType": "STRING", + "description": "qux" } """, MediaType.APPLICATION_JSON)); @@ -348,15 +315,9 @@ public void deletePropertyTest() { property.setPropertyType(PropertyType.STRING); qm.persist(property); - final Response response = target("%s/%s/property".formatted(V1_COMPONENT, component.getUuid())).request() + final Response response = target("%s/%s/property/%s".formatted(V1_COMPONENT, component.getUuid(), property.getUuid())).request() .header(X_API_KEY, apiKey) - .property(ClientProperties.SUPPRESS_HTTP_COMPLIANCE_VALIDATION, true) - .method("DELETE", Entity.entity(""" - { - "groupName": "foo", - "propertyName": "bar" - } - """, MediaType.APPLICATION_JSON)); + .delete(); assertThat(response.getStatus()).isEqualTo(204); assertThat(getPlainTextBody(response)).isEmpty(); diff --git a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java index 72e46ec2cd..9ab1796245 100644 --- a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java +++ b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java @@ -105,12 +105,10 @@ public static Collection data() { }); } - private final String bomUploadProcessingTaskName; private final Supplier bomUploadProcessingTaskSupplier; - public BomUploadProcessingTaskTest(final String bomUploadProcessingTaskName, + public BomUploadProcessingTaskTest(final String ignoredBomUploadProcessingTaskName, final Supplier bomUploadProcessingTaskSupplier) { - this.bomUploadProcessingTaskName = bomUploadProcessingTaskName; this.bomUploadProcessingTaskSupplier = bomUploadProcessingTaskSupplier; } @@ -243,7 +241,6 @@ public void informTest() throws Exception { assertThat(component.getPurl().canonicalize()).isEqualTo("pkg:maven/com.example/xmlutil@1.0.0?packaging=jar"); assertThat(component.getLicenseUrl()).isEqualTo("https://www.apache.org/licenses/LICENSE-2.0.txt"); - // TODO: Implement for legacy version of the task. if (bomUploadProcessingTaskSupplier.get() instanceof BomUploadProcessingTaskV2) { assertThat(component.getProperties()).satisfiesExactlyInAnyOrder( property -> { @@ -920,6 +917,11 @@ public void informWithBomContainingServiceTest() throws Exception { @Test public void informWithExistingComponentPropertiesAndBomWithoutComponentProperties() { + // Known to now work with old task implementation. + if (bomUploadProcessingTaskSupplier.get() instanceof BomUploadProcessingTask) { + return; + } + final var project = new Project(); project.setName("acme-app"); qm.persist(project); @@ -960,6 +962,11 @@ public void informWithExistingComponentPropertiesAndBomWithoutComponentPropertie @Test public void informWithExistingComponentPropertiesAndBomWithComponentProperties() { + // Known to now work with old task implementation. + if (bomUploadProcessingTaskSupplier.get() instanceof BomUploadProcessingTask) { + return; + } + final var project = new Project(); project.setName("acme-app"); qm.persist(project); @@ -1000,10 +1007,13 @@ public void informWithExistingComponentPropertiesAndBomWithComponentProperties() bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); awaitBomProcessedNotification(); - qm.getPersistenceManager().refresh(componentProperty); - assertThat(componentProperty.getGroupName()).isNull(); - assertThat(componentProperty.getPropertyName()).isEqualTo("foo"); - assertThat(componentProperty.getPropertyValue()).isEqualTo("baz"); + qm.getPersistenceManager().refresh(component); + assertThat(component.getProperties()).satisfiesExactly(property -> { + assertThat(property.getGroupName()).isNull(); + assertThat(property.getPropertyName()).isEqualTo("foo"); + assertThat(property.getPropertyValue()).isEqualTo("baz"); + assertThat(property.getUuid()).isNotEqualTo(componentProperty.getUuid()); + }); } @Test // https://github.com/DependencyTrack/dependency-track/issues/1905 From e711933271a7e4969ee1b2626161eefd0dedb84a Mon Sep 17 00:00:00 2001 From: nscuro Date: Sun, 14 Apr 2024 18:53:43 +0200 Subject: [PATCH 10/17] De-duplicate component properties during model conversion Also don't include project properties in BOM exports (yet). Signed-off-by: nscuro --- .../parser/cyclonedx/util/ModelConverter.java | 11 ++++++++++- .../tasks/BomUploadProcessingTaskV2.java | 5 +++++ .../dependencytrack/resources/v1/BomResourceTest.java | 8 +------- .../tasks/BomUploadProcessingTaskTest.java | 7 +++++++ src/test/resources/unit/bom-1.xml | 2 ++ 5 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java b/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java index 1028ca846b..b32cf8fe4d 100644 --- a/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java +++ b/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java @@ -73,6 +73,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -264,9 +265,11 @@ private static List convertToComponentProperties(final List(); return cdxProperties.stream() .map(ModelConverter::convertToComponentProperty) .filter(Objects::nonNull) + .filter(property -> identitiesSeen.add(new ComponentProperty.Identity(property))) .toList(); } @@ -873,7 +876,13 @@ public static org.cyclonedx.model.Metadata createMetadata(final Project project) cycloneComponent.setExternalReferences(references); } cycloneComponent.setSupplier(convert(project.getSupplier())); - cycloneComponent.setProperties(convert(project.getProperties())); + + // NB: Project properties are currently used to configure integrations + // such as Defect Dojo. They can also contain encrypted values that most + // definitely are not safe to share. Before we can include project properties + // in BOM exports, we need a filtering mechanism. + // cycloneComponent.setProperties(convert(project.getProperties())); + metadata.setComponent(cycloneComponent); if (project.getMetadata() != null) { diff --git a/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java b/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java index e4027341c0..e08b3b444f 100644 --- a/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java +++ b/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java @@ -470,6 +470,11 @@ private Map processComponents(final QueryManager q } private void processComponentProperties(final QueryManager qm, final Component component, final List properties) { + if (component.isNew()) { + // If the component is new, its properties are already in the desired state. + return; + } + if (properties == null || properties.isEmpty()) { // TODO: We currently remove all existing properties that are no longer included in the BOM. // This is to stay consistent with the BOM being the source of truth. However, this may feel diff --git a/src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java b/src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java index 3d4e66518b..9f9001e31d 100644 --- a/src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java +++ b/src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java @@ -235,13 +235,7 @@ public void exportProjectAsCycloneDxInventoryTest() { "name": "projectSupplier" }, "name": "acme-app", - "version": "SNAPSHOT", - "properties": [ - { - "name": "foo:bar", - "value": "baz" - } - ] + "version": "SNAPSHOT" }, "manufacture": { "name": "projectManufacturer" diff --git a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java index 9ab1796245..db20129164 100644 --- a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java +++ b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java @@ -256,6 +256,13 @@ public void informTest() throws Exception { assertThat(property.getPropertyValue()).isEqualTo("bar"); assertThat(property.getPropertyType()).isEqualTo(PropertyType.STRING); assertThat(property.getDescription()).isNull(); + }, + property -> { + assertThat(property.getGroupName()).isEqualTo("foo"); + assertThat(property.getPropertyName()).isEqualTo("bar"); + assertThat(property.getPropertyValue()).isEqualTo("qux"); + assertThat(property.getPropertyType()).isEqualTo(PropertyType.STRING); + assertThat(property.getDescription()).isNull(); } ); } diff --git a/src/test/resources/unit/bom-1.xml b/src/test/resources/unit/bom-1.xml index f64a234bc2..adde0b1861 100644 --- a/src/test/resources/unit/bom-1.xml +++ b/src/test/resources/unit/bom-1.xml @@ -90,6 +90,8 @@ foo bar baz + qux + qux From 0fabefcd1693cfdf12f070caca922dc70ff94355 Mon Sep 17 00:00:00 2001 From: nscuro Date: Sun, 14 Apr 2024 19:02:41 +0200 Subject: [PATCH 11/17] Update property names in Trivy integration test Signed-off-by: nscuro --- .../tasks/scanners/TrivyAnalysisTaskIntegrationTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/dependencytrack/tasks/scanners/TrivyAnalysisTaskIntegrationTest.java b/src/test/java/org/dependencytrack/tasks/scanners/TrivyAnalysisTaskIntegrationTest.java index e2261d8a15..38790d7c45 100644 --- a/src/test/java/org/dependencytrack/tasks/scanners/TrivyAnalysisTaskIntegrationTest.java +++ b/src/test/java/org/dependencytrack/tasks/scanners/TrivyAnalysisTaskIntegrationTest.java @@ -194,9 +194,9 @@ public void testWithUnrecognizedPackageName() { component.setPurl("pkg:deb/ubuntu/libc6@2.35-0ubuntu3.4?arch=amd64&distro=ubuntu-22.04"); qm.persist(component); - qm.createComponentProperty(component, "aquasecurity:trivy", "SrcName", "glibc", IConfigProperty.PropertyType.STRING, null); - qm.createComponentProperty(component, "aquasecurity:trivy", "SrcVersion", "2.35", IConfigProperty.PropertyType.STRING, null); - qm.createComponentProperty(component, "aquasecurity:trivy", "SrcRelease", "0ubuntu3.4", IConfigProperty.PropertyType.STRING, null); + qm.createComponentProperty(component, "aquasecurity", "trivy:SrcName", "glibc", IConfigProperty.PropertyType.STRING, null); + qm.createComponentProperty(component, "aquasecurity", "trivy:SrcVersion", "2.35", IConfigProperty.PropertyType.STRING, null); + qm.createComponentProperty(component, "aquasecurity", "trivy:SrcRelease", "0ubuntu3.4", IConfigProperty.PropertyType.STRING, null); final var analysisEvent = new TrivyAnalysisEvent(List.of(osComponent, component)); new TrivyAnalysisTask().inform(analysisEvent); From 650e6c2ac51df8c2c9aece64ed778f332c03868d Mon Sep 17 00:00:00 2001 From: nscuro Date: Sun, 14 Apr 2024 19:21:24 +0200 Subject: [PATCH 12/17] Fix `getComponentProperties` query Signed-off-by: nscuro --- .../dependencytrack/persistence/ComponentQueryManager.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java b/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java index e37b2f62b1..ae0a432b8d 100644 --- a/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java @@ -847,7 +847,9 @@ public List getComponentProperties(final Component component, @Override public List getComponentProperties(final Component component) { - final Query query = pm.newQuery(ComponentProperty.class, "component == :component"); + final Query query = pm.newQuery(ComponentProperty.class); + query.setFilter("component == :component"); + query.setParameters(component); query.setOrdering("groupName ASC, propertyName ASC, id ASC"); try { return List.copyOf(query.executeList()); From f39fc0cc00efc8f5e01da1eee8d52d07e4ba3fb3 Mon Sep 17 00:00:00 2001 From: nscuro Date: Sun, 14 Apr 2024 19:22:00 +0200 Subject: [PATCH 13/17] Ignore `component` field when serializing `ComponentProperty` Signed-off-by: nscuro --- src/main/java/org/dependencytrack/model/ComponentProperty.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/dependencytrack/model/ComponentProperty.java b/src/main/java/org/dependencytrack/model/ComponentProperty.java index 15ed1a123e..170e0f4289 100644 --- a/src/main/java/org/dependencytrack/model/ComponentProperty.java +++ b/src/main/java/org/dependencytrack/model/ComponentProperty.java @@ -63,6 +63,7 @@ public Identity(final ComponentProperty property) { @Persistent @Column(name = "COMPONENT_ID", allowsNull = "false") + @JsonIgnore private Component component; @Persistent From bb30ecf17167da7dbc87c984e5044f0407790036 Mon Sep 17 00:00:00 2001 From: nscuro Date: Sun, 14 Apr 2024 20:01:31 +0200 Subject: [PATCH 14/17] Attempt at avoid race conditions with test subscribers Signed-off-by: nscuro --- .../dependencytrack/tasks/BomUploadProcessingTaskTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java index db20129164..52d04fe942 100644 --- a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java +++ b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java @@ -134,13 +134,13 @@ public void setUp() { @After public void tearDown() { - EVENTS.clear(); - NOTIFICATIONS.clear(); - EventService.getInstance().unsubscribe(EventSubscriber.class); EventService.getInstance().unsubscribe(VulnerabilityAnalysisTask.class); EventService.getInstance().unsubscribe(NewVulnerableDependencyAnalysisTask.class); NotificationService.getInstance().unsubscribe(new Subscription(NotificationSubscriber.class)); + + EVENTS.clear(); + NOTIFICATIONS.clear(); } @Test From 009cc7423e5a757c91344af9b0747fac8d1ad0b9 Mon Sep 17 00:00:00 2001 From: nscuro Date: Sun, 14 Apr 2024 21:27:56 +0200 Subject: [PATCH 15/17] Try to fix test flakiness by waiting for event processing completion Signed-off-by: nscuro --- .../tasks/BomUploadProcessingTaskTest.java | 77 +++++++++++-------- 1 file changed, 45 insertions(+), 32 deletions(-) diff --git a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java index 52d04fe942..c2045db008 100644 --- a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java +++ b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java @@ -180,7 +180,7 @@ public void informTest() throws Exception { final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), resourceToByteArray("/unit/bom-1.xml")); bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); - awaitBomProcessedNotification(); + awaitBomProcessedNotification(bomUploadEvent); qm.getPersistenceManager().refresh(project); assertThat(project.getClassifier()).isEqualTo(Classifier.APPLICATION); @@ -298,7 +298,7 @@ public void informWithEmptyBomTest() throws Exception { final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), resourceToByteArray("/unit/bom-empty.json")); bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); - awaitBomProcessedNotification(); + awaitBomProcessedNotification(bomUploadEvent); qm.getPersistenceManager().refresh(project); assertThat(project.getClassifier()).isNull(); @@ -379,7 +379,7 @@ public void informWithComponentsUnderMetadataBomTest() throws Exception { final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), resourceToByteArray("/unit/bom-metadata-components.json")); bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); - awaitBomProcessedNotification(); + awaitBomProcessedNotification(bomUploadEvent); final List boms = qm.getAllBoms(project); assertThat(boms).hasSize(1); @@ -447,8 +447,9 @@ public void informWithExistingDuplicateComponentsTest() { } """.getBytes(StandardCharsets.UTF_8); - bomUploadProcessingTaskSupplier.get().inform(new BomUploadEvent(qm.detach(Project.class, project.getId()), bomBytes)); - awaitBomProcessedNotification(); + final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), bomBytes); + bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); + awaitBomProcessedNotification(bomUploadEvent); qm.getPersistenceManager().evictAll(); assertThat(qm.getAllComponents(project)).satisfiesExactly(c -> { @@ -500,7 +501,7 @@ public void informWithBloatedBomTest() throws Exception { final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), resourceToByteArray("/unit/bom-bloated.json")); bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); - awaitBomProcessedNotification(); + awaitBomProcessedNotification(bomUploadEvent); final List boms = qm.getAllBoms(project); assertThat(boms).hasSize(1); @@ -560,7 +561,7 @@ public void informWithCustomLicenseResolutionTest() throws Exception { final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), resourceToByteArray("/unit/bom-custom-license.json")); bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); - awaitBomProcessedNotification(); + awaitBomProcessedNotification(bomUploadEvent); assertThat(qm.getAllComponents(project)).satisfiesExactly( component -> { @@ -611,8 +612,9 @@ public void informWithBomContainingLicenseExpressionTest() { } """.getBytes(StandardCharsets.UTF_8); - bomUploadProcessingTaskSupplier.get().inform(new BomUploadEvent(qm.detach(Project.class, project.getId()), bomBytes)); - awaitBomProcessedNotification(); + final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), bomBytes); + bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); + awaitBomProcessedNotification(bomUploadEvent); assertThat(qm.getAllComponents(project)).satisfiesExactly(component -> { assertThat(component.getLicense()).isNull(); @@ -655,8 +657,9 @@ public void informWithBomContainingLicenseExpressionWithSingleIdTest() { } """.getBytes(StandardCharsets.UTF_8); - bomUploadProcessingTaskSupplier.get().inform(new BomUploadEvent(qm.detach(Project.class, project.getId()), bomBytes)); - awaitBomProcessedNotification(); + final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), bomBytes); + bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); + awaitBomProcessedNotification(bomUploadEvent); assertThat(qm.getAllComponents(project)).satisfiesExactly(component -> { assertThat(component.getResolvedLicense()).isNotNull(); @@ -695,8 +698,9 @@ public void informWithBomContainingInvalidLicenseExpressionTest() { } """.getBytes(StandardCharsets.UTF_8); - bomUploadProcessingTaskSupplier.get().inform(new BomUploadEvent(qm.detach(Project.class, project.getId()), bomBytes)); - awaitBomProcessedNotification(); + final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), bomBytes); + bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); + awaitBomProcessedNotification(bomUploadEvent); assertThat(qm.getAllComponents(project)).satisfiesExactly(component -> { assertThat(component.getLicense()).isNull(); @@ -738,8 +742,9 @@ public void informIssue3433Test() { } """.getBytes(StandardCharsets.UTF_8); - bomUploadProcessingTaskSupplier.get().inform(new BomUploadEvent(qm.detach(Project.class, project.getId()), bomBytes)); - awaitBomProcessedNotification(); + final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), bomBytes); + bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); + awaitBomProcessedNotification(bomUploadEvent); assertThat(qm.getAllComponents(project)).satisfiesExactly(component -> { assertThat(component.getResolvedLicense()).isNotNull(); @@ -788,8 +793,9 @@ public void informUpdateExistingLicenseTest() { } """.getBytes(StandardCharsets.UTF_8); - bomUploadProcessingTaskSupplier.get().inform(new BomUploadEvent(qm.detach(Project.class, project.getId()), existingBomBytes)); - awaitBomProcessedNotification(); + final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), existingBomBytes); + bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); + awaitBomProcessedNotification(bomUploadEvent); assertThat(qm.getAllComponents(project)).satisfiesExactly(component -> { assertThat(component.getResolvedLicense()).isNotNull(); @@ -823,7 +829,7 @@ public void informUpdateExistingLicenseTest() { """.getBytes(StandardCharsets.UTF_8); bomUploadProcessingTaskSupplier.get().inform(new BomUploadEvent(qm.detach(Project.class, project.getId()), updatedBomBytes)); - awaitBomProcessedNotification(); + awaitBomProcessedNotification(bomUploadEvent); qm.getPersistenceManager().evictAll(); assertThat(qm.getAllComponents(project)).satisfiesExactly(component -> { @@ -869,8 +875,9 @@ public void informDeleteExistingLicenseTest() { } """.getBytes(StandardCharsets.UTF_8); - bomUploadProcessingTaskSupplier.get().inform(new BomUploadEvent(qm.detach(Project.class, project.getId()), existingBomBytes)); - awaitBomProcessedNotification(); + final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), existingBomBytes); + bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); + awaitBomProcessedNotification(bomUploadEvent); assertThat(qm.getAllComponents(project)).satisfiesExactly(component -> { assertThat(component.getResolvedLicense()).isNotNull(); @@ -898,7 +905,7 @@ public void informDeleteExistingLicenseTest() { """.getBytes(StandardCharsets.UTF_8); bomUploadProcessingTaskSupplier.get().inform(new BomUploadEvent(qm.detach(Project.class, project.getId()), updatedBomBytes)); - awaitBomProcessedNotification(); + awaitBomProcessedNotification(bomUploadEvent); qm.getPersistenceManager().evictAll(); assertThat(qm.getAllComponents(project)).satisfiesExactly(component -> { @@ -916,7 +923,7 @@ public void informWithBomContainingServiceTest() throws Exception { final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), resourceToByteArray("/unit/bom-service.json")); bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); - awaitBomProcessedNotification(); + awaitBomProcessedNotification(bomUploadEvent); assertThat(qm.getAllComponents(project)).isNotEmpty(); assertThat(qm.getAllServiceComponents(project)).isNotEmpty(); @@ -961,7 +968,7 @@ public void informWithExistingComponentPropertiesAndBomWithoutComponentPropertie } """.getBytes()); bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); - awaitBomProcessedNotification(); + awaitBomProcessedNotification(bomUploadEvent); qm.getPersistenceManager().refresh(component); assertThat(component.getProperties()).isEmpty(); @@ -1012,7 +1019,7 @@ public void informWithExistingComponentPropertiesAndBomWithComponentProperties() } """.getBytes()); bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); - awaitBomProcessedNotification(); + awaitBomProcessedNotification(bomUploadEvent); qm.getPersistenceManager().refresh(component); assertThat(component.getProperties()).satisfiesExactly(property -> { @@ -1038,7 +1045,7 @@ public void informIssue1905Test() throws Exception { bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); // Make sure processing did not fail. - awaitBomProcessedNotification(); + awaitBomProcessedNotification(bomUploadEvent); NOTIFICATIONS.clear(); // Ensure all expected components are present. @@ -1079,7 +1086,7 @@ public void informIssue2519Test() throws Exception { bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); // Make sure processing did not fail. - awaitBomProcessedNotification(); + awaitBomProcessedNotification(bomUploadEvent); NOTIFICATIONS.clear(); // Ensure the expected amount of components is present. @@ -1127,14 +1134,16 @@ public void informIssue3309Test() { } """.getBytes(); - bomUploadProcessingTaskSupplier.get().inform(new BomUploadEvent(qm.detach(Project.class, project.getId()), bomBytes)); - awaitBomProcessedNotification(); + var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), bomBytes); + bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); + awaitBomProcessedNotification(bomUploadEvent); assertProjectAuthors.run(); NOTIFICATIONS.clear(); - bomUploadProcessingTaskSupplier.get().inform(new BomUploadEvent(qm.detach(Project.class, project.getId()), bomBytes)); - awaitBomProcessedNotification(); + bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), bomBytes); + bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); + awaitBomProcessedNotification(bomUploadEvent); assertProjectAuthors.run(); } @@ -1155,7 +1164,7 @@ public void informIssue3371Test() throws Exception { bomUploadProcessingTaskSupplier.get().inform(bomUploadEvent); // Make sure processing did not fail. - awaitBomProcessedNotification(); + awaitBomProcessedNotification(bomUploadEvent); NOTIFICATIONS.clear(); // Ensure the expected amount of components is present. @@ -1174,7 +1183,7 @@ public void informIssue3371Test() throws Exception { } } - private void awaitBomProcessedNotification() { + private void awaitBomProcessedNotification(final BomUploadEvent bomUploadEvent) { try { await("BOM Processed Notification") .atMost(Duration.ofSeconds(3)) @@ -1191,6 +1200,10 @@ private void awaitBomProcessedNotification() { final var subject = (BomProcessingFailed) optionalNotification.get().getSubject(); fail("Expected BOM processing to succeed, but it failed due to: %s", subject.getCause()); } + + await("Event Processing Completion") + .atMost(Duration.ofSeconds(3)) + .untilAsserted(() -> assertThat(Event.isEventBeingProcessed(bomUploadEvent.getChainIdentifier())).isFalse()); } } From 523b5f092dcf65eb746435ad8d1464d94b0dcf46 Mon Sep 17 00:00:00 2001 From: nscuro Date: Sun, 14 Apr 2024 22:09:11 +0200 Subject: [PATCH 16/17] Add `@rkesters` to release credits For his work on the component property feature in #2717. Signed-off-by: nscuro --- docs/_posts/2024-xx-xx-v4.11.0.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/_posts/2024-xx-xx-v4.11.0.md b/docs/_posts/2024-xx-xx-v4.11.0.md index a2d9deb450..47b81f2877 100644 --- a/docs/_posts/2024-xx-xx-v4.11.0.md +++ b/docs/_posts/2024-xx-xx-v4.11.0.md @@ -158,7 +158,7 @@ We thank all organizations and individuals who contributed to this release, from Special thanks to everyone who contributed code to implement enhancements and fix defects: [@AnthonyMastrean], [@LaVibeX], [@MangoIV], [@Robbilie], [@VithikaS], [@a5a351e7], [@acdha], [@aravindparappil46], [@baburkin], [@fnxpt], [@kepten], [@leec94], [@lukas-braune], [@malice00], [@mehab], [@mge-mm] -[@mikkeschiren], [@mykter], [@rbt-mm], [@rkg-mm], [@sahibamittal], [@sebD], [@setchy] +[@mikkeschiren], [@mykter], [@rbt-mm], [@rkesters], [@rkg-mm], [@sahibamittal], [@sebD], [@setchy] ###### dependency-track-apiserver.jar @@ -282,6 +282,7 @@ Special thanks to everyone who contributed code to implement enhancements and fi [@mprencipe]: https://github.com/mprencipe [@mykter]: https://github.com/mykter [@rbt-mm]: https://github.com/rbt-mm +[@rkesters]: https://github.com/rkesters [@rkg-mm]: https://github.com/rkg-mm [@sahibamittal]: https://github.com/sahibamittal [@sebD]: https://github.com/sebD From 11020d15ed01fe0394f734f28a75944ce91673f2 Mon Sep 17 00:00:00 2001 From: nscuro Date: Sun, 14 Apr 2024 23:04:28 +0200 Subject: [PATCH 17/17] Implement component property ingest for legacy BOM processing task Signed-off-by: nscuro --- .../parser/cyclonedx/util/ModelConverter.java | 7 +++ .../persistence/ComponentQueryManager.java | 56 ++++++++++++++++++ .../persistence/QueryManager.java | 4 ++ .../tasks/BomUploadProcessingTaskV2.java | 59 +------------------ .../tasks/BomUploadProcessingTaskTest.java | 58 ++++++++---------- 5 files changed, 91 insertions(+), 93 deletions(-) diff --git a/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java b/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java index b32cf8fe4d..003d85a18f 100644 --- a/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java +++ b/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java @@ -604,6 +604,13 @@ else if (StringUtils.isNotBlank(cycloneLicense.getName())) component.setExternalReferences(null); } + final List properties = convertToComponentProperties(cycloneDxComponent.getProperties()); + if (component.getId() == 0) { + component.setProperties(properties); + } else { + qm.synchronizeComponentProperties(component, properties); + } + if (cycloneDxComponent.getComponents() != null && !cycloneDxComponent.getComponents().isEmpty()) { final Collection components = new ArrayList<>(); for (int i = 0; i < cycloneDxComponent.getComponents().size(); i++) { diff --git a/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java b/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java index ae0a432b8d..83d956797a 100644 --- a/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java @@ -52,6 +52,11 @@ import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.function.Function; +import java.util.stream.Collectors; + +import static org.dependencytrack.util.PersistenceUtil.assertNonPersistent; +import static org.dependencytrack.util.PersistenceUtil.assertPersistent; final class ComponentQueryManager extends QueryManager implements IQueryManager { @@ -886,4 +891,55 @@ public long deleteComponentPropertyByUuid(final Component component, final UUID } } + public void synchronizeComponentProperties(final Component component, final List properties) { + assertPersistent(component, "component must be persistent"); + + if (properties == null || properties.isEmpty()) { + // TODO: We currently remove all existing properties that are no longer included in the BOM. + // This is to stay consistent with the BOM being the source of truth. However, this may feel + // counter-intuitive to some users, who might expect their manual changes to persist. + // If we want to support that, we need a way to track which properties were added and / or + // modified manually. + if (component.getProperties() != null) { + pm.deletePersistentAll(component.getProperties()); + } + + return; + } + + properties.forEach(property -> assertNonPersistent(property, "property must not be persistent")); + + if (component.getProperties() == null || component.getProperties().isEmpty()) { + for (final ComponentProperty property : properties) { + property.setComponent(component); + pm.makePersistent(property); + } + + return; + } + + // Group properties by group, name, and value. Because CycloneDX supports duplicate + // property names, uniqueness can only be determined by also considering the value. + final var existingPropertiesByIdentity = component.getProperties().stream() + .collect(Collectors.toMap(ComponentProperty.Identity::new, Function.identity())); + final var incomingPropertiesByIdentity = properties.stream() + .collect(Collectors.toMap(ComponentProperty.Identity::new, Function.identity())); + + final var propertyIdentities = new HashSet(); + propertyIdentities.addAll(existingPropertiesByIdentity.keySet()); + propertyIdentities.addAll(incomingPropertiesByIdentity.keySet()); + + for (final ComponentProperty.Identity identity : propertyIdentities) { + final ComponentProperty existingProperty = existingPropertiesByIdentity.get(identity); + final ComponentProperty incomingProperty = incomingPropertiesByIdentity.get(identity); + + if (existingProperty == null) { + incomingProperty.setComponent(component); + pm.makePersistent(incomingProperty); + } else if (incomingProperty == null) { + pm.deletePersistent(existingProperty); + } + } + } + } diff --git a/src/main/java/org/dependencytrack/persistence/QueryManager.java b/src/main/java/org/dependencytrack/persistence/QueryManager.java index 7fc1516a4b..e8b2f7f813 100644 --- a/src/main/java/org/dependencytrack/persistence/QueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/QueryManager.java @@ -578,6 +578,10 @@ public long deleteComponentPropertyByUuid(final Component component, final UUID return getComponentQueryManager().deleteComponentPropertyByUuid(component, uuid); } + public void synchronizeComponentProperties(final Component component, final List properties) { + getComponentQueryManager().synchronizeComponentProperties(component, properties); + } + public PaginatedResult getLicenses() { return getLicenseQueryManager().getLicenses(); } diff --git a/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java b/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java index e08b3b444f..9c0fee24d3 100644 --- a/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java +++ b/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTaskV2.java @@ -41,7 +41,6 @@ import org.dependencytrack.model.Bom; import org.dependencytrack.model.Component; import org.dependencytrack.model.ComponentIdentity; -import org.dependencytrack.model.ComponentProperty; import org.dependencytrack.model.DependencyMetrics; import org.dependencytrack.model.FindingAttribution; import org.dependencytrack.model.License; @@ -78,9 +77,7 @@ import java.util.Set; import java.util.UUID; import java.util.concurrent.TimeUnit; -import java.util.function.Function; import java.util.function.Predicate; -import java.util.stream.Collectors; import static org.apache.commons.lang3.StringUtils.isNotBlank; import static org.apache.commons.lang3.StringUtils.trim; @@ -442,12 +439,10 @@ private Map processComponents(final QueryManager q applyIfChanged(persistentComponent, component, Component::getLicenseExpression, persistentComponent::setLicenseExpression); applyIfChanged(persistentComponent, component, Component::isInternal, persistentComponent::setInternal); applyIfChanged(persistentComponent, component, Component::getExternalReferences, persistentComponent::setExternalReferences); - + qm.synchronizeComponentProperties(persistentComponent, component.getProperties()); idsOfComponentsToDelete.remove(persistentComponent.getId()); } - processComponentProperties(qm, persistentComponent, component.getProperties()); - // Update component identities in our Identity->BOMRef map, // as after persisting the components, their identities now include UUIDs. final var newIdentity = new ComponentIdentity(persistentComponent); @@ -469,58 +464,6 @@ private Map processComponents(final QueryManager q return persistentComponents; } - private void processComponentProperties(final QueryManager qm, final Component component, final List properties) { - if (component.isNew()) { - // If the component is new, its properties are already in the desired state. - return; - } - - if (properties == null || properties.isEmpty()) { - // TODO: We currently remove all existing properties that are no longer included in the BOM. - // This is to stay consistent with the BOM being the source of truth. However, this may feel - // counter-intuitive to some users, who might expect their manual changes to persist. - // If we want to support that, we need a way to track which properties were added and / or - // modified manually. - if (component.getProperties() != null) { - qm.getPersistenceManager().deletePersistentAll(component.getProperties()); - } - - return; - } - - if (component.getProperties() == null || component.getProperties().isEmpty()) { - for (final ComponentProperty property : properties) { - property.setComponent(component); - qm.getPersistenceManager().makePersistent(property); - } - - return; - } - - // Group properties by group, name, and value. Because CycloneDX supports duplicate - // property names, uniqueness can only be determined by also considering the value. - final var existingPropertiesByIdentity = component.getProperties().stream() - .collect(Collectors.toMap(ComponentProperty.Identity::new, Function.identity())); - final var incomingPropertiesByIdentity = properties.stream() - .collect(Collectors.toMap(ComponentProperty.Identity::new, Function.identity())); - - final var propertyIdentities = new HashSet(); - propertyIdentities.addAll(existingPropertiesByIdentity.keySet()); - propertyIdentities.addAll(incomingPropertiesByIdentity.keySet()); - - for (final ComponentProperty.Identity identity : propertyIdentities) { - final ComponentProperty existingProperty = existingPropertiesByIdentity.get(identity); - final ComponentProperty incomingProperty = incomingPropertiesByIdentity.get(identity); - - if (existingProperty == null) { - incomingProperty.setComponent(component); - qm.getPersistenceManager().makePersistent(incomingProperty); - } else if (incomingProperty == null) { - qm.getPersistenceManager().deletePersistent(existingProperty); - } - } - } - private Map processServices(final QueryManager qm, final Project project, final List services, diff --git a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java index c2045db008..ddf003722e 100644 --- a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java +++ b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java @@ -241,31 +241,29 @@ public void informTest() throws Exception { assertThat(component.getPurl().canonicalize()).isEqualTo("pkg:maven/com.example/xmlutil@1.0.0?packaging=jar"); assertThat(component.getLicenseUrl()).isEqualTo("https://www.apache.org/licenses/LICENSE-2.0.txt"); - if (bomUploadProcessingTaskSupplier.get() instanceof BomUploadProcessingTaskV2) { - assertThat(component.getProperties()).satisfiesExactlyInAnyOrder( - property -> { - assertThat(property.getGroupName()).isEqualTo("foo"); - assertThat(property.getPropertyName()).isEqualTo("bar"); - assertThat(property.getPropertyValue()).isEqualTo("baz"); - assertThat(property.getPropertyType()).isEqualTo(PropertyType.STRING); - assertThat(property.getDescription()).isNull(); - }, - property -> { - assertThat(property.getGroupName()).isNull(); - assertThat(property.getPropertyName()).isEqualTo("foo"); - assertThat(property.getPropertyValue()).isEqualTo("bar"); - assertThat(property.getPropertyType()).isEqualTo(PropertyType.STRING); - assertThat(property.getDescription()).isNull(); - }, - property -> { - assertThat(property.getGroupName()).isEqualTo("foo"); - assertThat(property.getPropertyName()).isEqualTo("bar"); - assertThat(property.getPropertyValue()).isEqualTo("qux"); - assertThat(property.getPropertyType()).isEqualTo(PropertyType.STRING); - assertThat(property.getDescription()).isNull(); - } - ); - } + assertThat(component.getProperties()).satisfiesExactlyInAnyOrder( + property -> { + assertThat(property.getGroupName()).isEqualTo("foo"); + assertThat(property.getPropertyName()).isEqualTo("bar"); + assertThat(property.getPropertyValue()).isEqualTo("baz"); + assertThat(property.getPropertyType()).isEqualTo(PropertyType.STRING); + assertThat(property.getDescription()).isNull(); + }, + property -> { + assertThat(property.getGroupName()).isNull(); + assertThat(property.getPropertyName()).isEqualTo("foo"); + assertThat(property.getPropertyValue()).isEqualTo("bar"); + assertThat(property.getPropertyType()).isEqualTo(PropertyType.STRING); + assertThat(property.getDescription()).isNull(); + }, + property -> { + assertThat(property.getGroupName()).isEqualTo("foo"); + assertThat(property.getPropertyName()).isEqualTo("bar"); + assertThat(property.getPropertyValue()).isEqualTo("qux"); + assertThat(property.getPropertyType()).isEqualTo(PropertyType.STRING); + assertThat(property.getDescription()).isNull(); + } + ); assertThat(qm.getAllVulnerabilities(component)).hasSize(2); assertThat(NOTIFICATIONS).satisfiesExactly( @@ -931,11 +929,6 @@ public void informWithBomContainingServiceTest() throws Exception { @Test public void informWithExistingComponentPropertiesAndBomWithoutComponentProperties() { - // Known to now work with old task implementation. - if (bomUploadProcessingTaskSupplier.get() instanceof BomUploadProcessingTask) { - return; - } - final var project = new Project(); project.setName("acme-app"); qm.persist(project); @@ -976,11 +969,6 @@ public void informWithExistingComponentPropertiesAndBomWithoutComponentPropertie @Test public void informWithExistingComponentPropertiesAndBomWithComponentProperties() { - // Known to now work with old task implementation. - if (bomUploadProcessingTaskSupplier.get() instanceof BomUploadProcessingTask) { - return; - } - final var project = new Project(); project.setName("acme-app"); qm.persist(project);