Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix project cloning allowing for duplicate versions #2966

Merged
merged 1 commit into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -400,6 +401,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