Skip to content

Commit

Permalink
Merge pull request #2966 from nscuro/issue-2958
Browse files Browse the repository at this point in the history
Fix project cloning allowing for duplicate versions
  • Loading branch information
nscuro authored Sep 21, 2023
2 parents 9e247fb + 741de8c commit ec6b704
Show file tree
Hide file tree
Showing 6 changed files with 270 additions and 18 deletions.
7 changes: 7 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
<!-- Dependency Versions -->
<frontend.version>4.8.1</frontend.version>
<lib.alpine.version>${project.parent.version}</lib.alpine.version>
<lib.awaitility.version>4.2.0</lib.awaitility.version>
<lib.cpe-parser.version>2.0.2</lib.cpe-parser.version>
<lib.cvss-calculator.version>1.4.1</lib.cvss-calculator.version>
<lib.owasp-rr-calculator.version>1.0.1</lib.owasp-rr-calculator.version>
Expand Down Expand Up @@ -406,6 +407,12 @@
<version>5.15.0</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.awaitility</groupId>
<artifactId>awaitility</artifactId>
<version>${lib.awaitility.version}</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,15 @@ public Project clone(UUID from, String newVersion, boolean includeTags, boolean
boolean includeACL) {
final Project source = getObjectByUuid(Project.class, from, Project.FetchGroup.ALL.name());
if (source == null) {
LOGGER.warn("Project with UUID %s was supposed to be cloned, but it does not exist anymore".formatted(from));
return null;
}
if (doesProjectExist(source.getName(), newVersion)) {
// Project cloning is an asynchronous process. When receiving the clone request, we already perform
// this check. It is possible though that a project with the new version is created synchronously
// between the clone event being dispatched, and it being processed.
LOGGER.warn("Project %s was supposed to be cloned to version %s, but that version already exists"
.formatted(source, newVersion));
return null;
}
Project project = new Project();
Expand Down Expand Up @@ -654,6 +663,15 @@ public Project clone(UUID from, String newVersion, boolean includeTags, boolean
}
}

if (includeServices) {
final List<ServiceComponent> sourceServices = getAllServiceComponents(source);
if (sourceServices != null) {
for (final ServiceComponent sourceService : sourceServices) {
cloneServiceComponent(sourceService, project, false);
}
}
}

if (includeAuditHistory && includeComponents) {
final List<Analysis> analyses = super.getAnalyses(source);
if (analyses != null) {
Expand Down Expand Up @@ -1122,6 +1140,29 @@ public PaginatedResult getProjectsWithoutDescendantsOf(final String name, final
return result;
}

/**
* Check whether a {@link Project} with a given {@code name} and {@code version} exists.
*
* @param name Name of the {@link Project} to check for
* @param version Version of the {@link Project} to check for
* @return {@code true} when a matching {@link Project} exists, otherwise {@code false}
* @since 4.9.0
*/
@Override
public boolean doesProjectExist(final String name, final String version) {
final Query<Project> query = pm.newQuery(Project.class);
try {
query.setFilter("name == :name && version == :version");
query.setNamedParameters(Map.of(
"name", name,
"version", version
));
query.setResult("count(this)");
return query.executeResultUnique(Long.class) > 0;
} finally {
query.closeAll();
}
}

private static boolean isChildOf(Project project, UUID uuid) {
boolean isChild = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,10 @@ public PaginatedResult getProjects(final Tag tag) {
return getProjectQueryManager().getProjects(tag);
}

public boolean doesProjectExist(final String name, final String version) {
return getProjectQueryManager().doesProjectExist(name, version);
}

public Tag getTagByName(final String name) {
return getProjectQueryManager().getTagByName(name);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ public ServiceComponent cloneServiceComponent(ServiceComponent sourceService, Pr
service.setNotes(sourceService.getNotes());
service.setVulnerabilities(sourceService.getVulnerabilities());
service.setProject(destinationProject);
return createServiceComponent(sourceService, commitIndex);
return createServiceComponent(service, commitIndex);
}

/**
Expand Down
28 changes: 18 additions & 10 deletions src/main/java/org/dependencytrack/resources/v1/ProjectResource.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,7 @@
import org.dependencytrack.model.Tag;
import org.dependencytrack.persistence.QueryManager;
import org.dependencytrack.resources.v1.vo.CloneProjectRequest;
import java.security.Principal;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.function.BiConsumer;
import java.util.function.Function;

import javax.jdo.FetchGroup;
import javax.validation.Validator;
import javax.ws.rs.Consumes;
Expand All @@ -59,6 +54,12 @@
import javax.ws.rs.QueryParam;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import java.security.Principal;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.function.BiConsumer;
import java.util.function.Function;

/**
* JAX-RS resources for processing projects.
Expand Down Expand Up @@ -247,8 +248,8 @@ public Response createProject(Project jsonProject) {
Project parent = qm.getObjectByUuid(Project.class, jsonProject.getParent().getUuid());
jsonProject.setParent(parent);
}
Project project = qm.getProject(StringUtils.trimToNull(jsonProject.getName()), StringUtils.trimToNull(jsonProject.getVersion()));
if (project == null) {
if (!qm.doesProjectExist(StringUtils.trimToNull(jsonProject.getName()), StringUtils.trimToNull(jsonProject.getVersion()))) {
final Project project;
try {
project = qm.createProject(jsonProject, jsonProject.getTags(), true);
} catch (IllegalArgumentException e){
Expand Down Expand Up @@ -369,7 +370,7 @@ public Response patchProject(
modified |= setIfDifferent(jsonProject, project, Project::getName, Project::setName);
modified |= setIfDifferent(jsonProject, project, Project::getVersion, Project::setVersion);
// if either name or version has been changed, verify that this new combination does not already exist
if (modified && qm.getProject(project.getName(), project.getVersion()) != null) {
if (modified && qm.doesProjectExist(project.getName(), project.getVersion())) {
return Response.status(Response.Status.CONFLICT).entity("A project with the specified name and version already exists.").build();
}
modified |= setIfDifferent(jsonProject, project, Project::getAuthor, Project::setAuthor);
Expand Down Expand Up @@ -505,7 +506,14 @@ public Response cloneProject(CloneProjectRequest jsonRequest) {
try (QueryManager qm = new QueryManager()) {
final Project sourceProject = qm.getObjectByUuid(Project.class, jsonRequest.getProject(), Project.FetchGroup.ALL.name());
if (sourceProject != null) {
LOGGER.info("Project " + sourceProject.toString() + " is being cloned by " + super.getPrincipal().getName());
if (!qm.hasAccess(super.getPrincipal(), sourceProject)) {
return Response.status(Response.Status.FORBIDDEN).entity("Access to the specified project is forbidden").build();
}
if (qm.doesProjectExist(sourceProject.getName(), StringUtils.trimToNull(jsonRequest.getVersion()))) {
return Response.status(Response.Status.CONFLICT).entity("A project with the specified name and version already exists.").build();
}

LOGGER.info("Project " + sourceProject + " is being cloned by " + super.getPrincipal().getName());
Event.dispatch(new CloneProjectEvent(jsonRequest));
return Response.ok().build();
} else {
Expand Down
Loading

0 comments on commit ec6b704

Please sign in to comment.