Skip to content

Commit

Permalink
Implement the changes suggested by @nscuro
Browse files Browse the repository at this point in the history
Signed-off-by: Mikael Carneholm <[email protected]>
  • Loading branch information
mikael-carneholm-2-wcar committed Dec 2, 2024
1 parent 54cce6b commit beba9d2
Show file tree
Hide file tree
Showing 8 changed files with 463 additions and 173 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* 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.exception;

import java.util.Map;

public class ProjectOperationException extends IllegalStateException {

private final Map<String, String> errorByUUID;

private ProjectOperationException(final String message, final Map<String, String> errorByUUID) {
super(message);
this.errorByUUID = errorByUUID;
}

public static ProjectOperationException forDeletion(final Map<String, String> errorByUUID) {
return new ProjectOperationException("One or more projects could not be deleted", errorByUUID);
}

public Map<String, String> getErrorByUUID() {
return errorByUUID;
}

}
390 changes: 269 additions & 121 deletions src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ public void recursivelyDelete(final Project project, final boolean commitIndex)
getProjectQueryManager().recursivelyDelete(project, commitIndex);
}

public void deleteProjectsByUUIDs(List<UUID> uuids) {
public void deleteProjectsByUUIDs(Collection<UUID> uuids) {
getProjectQueryManager().deleteProjectsByUUIDs(uuids);
}

Expand Down
49 changes: 12 additions & 37 deletions src/main/java/org/dependencytrack/resources/v1/ProjectResource.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import io.swagger.v3.oas.annotations.responses.ApiResponses;
import io.swagger.v3.oas.annotations.security.SecurityRequirement;
import io.swagger.v3.oas.annotations.security.SecurityRequirements;
import jakarta.validation.constraints.Size;
import org.apache.commons.lang3.StringUtils;
import org.dependencytrack.auth.Permissions;
import org.dependencytrack.event.CloneProjectEvent;
Expand All @@ -46,6 +47,8 @@
import org.dependencytrack.model.validation.ValidUuid;
import org.dependencytrack.persistence.QueryManager;
import org.dependencytrack.resources.v1.openapi.PaginatedApi;
import org.dependencytrack.resources.v1.problems.ProblemDetails;
import org.dependencytrack.resources.v1.problems.ProjectOperationProblemDetails;
import org.dependencytrack.resources.v1.vo.BomUploadResponse;
import org.dependencytrack.resources.v1.vo.CloneProjectRequest;

Expand All @@ -66,10 +69,8 @@
import jakarta.ws.rs.core.Response;
import javax.jdo.FetchGroup;
import java.security.Principal;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -816,44 +817,18 @@ public Response deleteProject(
)
@ApiResponses(value = {
@ApiResponse(responseCode = "204", description = "Projects removed successfully"),
@ApiResponse(responseCode = "207", description = "Access is forbidden to the projects listed in the response"),
@ApiResponse(responseCode = "401", description = "Unauthorized")
@ApiResponse(
responseCode = "400",
description = "Operation failed",
content = @Content(schema = @Schema(implementation = ProjectOperationProblemDetails.class), mediaType = ProblemDetails.MEDIA_TYPE_JSON)
)
})
@PermissionRequired(Permissions.Constants.PORTFOLIO_MANAGEMENT)
public Response deleteProjects(List<UUID> uuids) {
List<UUID> inaccessibleProjects = new ArrayList<>();
try (QueryManager qm = new QueryManager()) {
for (Iterator<UUID> it = uuids.iterator(); it.hasNext();) {
UUID uuid = it.next();
final Project project = qm.getObjectByUuid(Project.class, uuid, Project.FetchGroup.ALL.name());
if (project != null) {
if (!qm.hasAccess(super.getPrincipal(), project)) {
LOGGER.warn(super.getPrincipal().getName() + " lacks access to project " + project
+ ", delete request denied"
);
inaccessibleProjects.add(uuid);
it.remove();
}
} else {
LOGGER.warn("No project found by UUID " + uuid);
it.remove();
}
}

if (uuids.isEmpty()) {
return Response.status(Response.Status.FORBIDDEN)
.entity(inaccessibleProjects)
.build();
}

qm.deleteProjectsByUUIDs(uuids);

if (inaccessibleProjects.isEmpty()) {
return Response.status(Response.Status.NO_CONTENT).build();
} else {
return Response.status(207).entity(inaccessibleProjects).build();
}
public Response deleteProjects(@Size(min = 1, max = 1000) final Set<UUID> uuids) {
try (final var qm = new QueryManager(getAlpineRequest())) {
qm.deleteProjectsByUUIDs(uuids);
}
return Response.status(Response.Status.NO_CONTENT).build();
}

@PUT
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* 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.resources.v1.exception;

import org.dependencytrack.exception.ProjectOperationException;
import org.dependencytrack.resources.v1.problems.ProjectOperationProblemDetails;

import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.ext.ExceptionMapper;
import jakarta.ws.rs.ext.Provider;

@Provider
public class ProjectOperationExceptionMapper implements ExceptionMapper<ProjectOperationException> {

@Override
public Response toResponse(final ProjectOperationException exception) {
final var problemDetails = new ProjectOperationProblemDetails();
problemDetails.setStatus(400);
problemDetails.setTitle("Project operation failed");
problemDetails.setDetail(exception.getMessage());
problemDetails.setErrors(exception.getErrorByUUID());
return problemDetails.toResponse();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
description = "An RFC 9457 problem object",
subTypes = {
InvalidBomProblemDetails.class,
TagOperationProblemDetails.class
TagOperationProblemDetails.class,
ProjectOperationProblemDetails.class
}
)
@JsonInclude(JsonInclude.Include.NON_NULL)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* 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.resources.v1.problems;

import io.swagger.v3.oas.annotations.media.Schema;

import java.util.Map;

public class ProjectOperationProblemDetails extends ProblemDetails {

@Schema(description = "Errors encountered during the operation, grouped by project UUID")
private Map<String, String> errors;

public Map<String, String> getErrors() {
return errors;
}

public void setErrors(final Map<String, String> errors) {
this.errors = errors;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import alpine.server.auth.JsonWebToken;
import alpine.server.filters.ApiFilter;
import alpine.server.filters.AuthenticationFilter;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.cyclonedx.model.ExternalReference.Type;
import org.dependencytrack.JerseyTestRule;
import org.dependencytrack.ResourceTest;
Expand All @@ -47,8 +49,10 @@
import org.dependencytrack.model.ServiceComponent;
import org.dependencytrack.model.Tag;
import org.dependencytrack.model.Vulnerability;
import org.dependencytrack.resources.v1.exception.ProjectOperationExceptionMapper;
import org.dependencytrack.tasks.CloneProjectTask;
import org.dependencytrack.tasks.scanners.AnalyzerIdentity;
import org.glassfish.jersey.client.ClientProperties;
import org.glassfish.jersey.client.HttpUrlConnectorProvider;
import org.glassfish.jersey.server.ResourceConfig;
import org.hamcrest.CoreMatchers;
Expand Down Expand Up @@ -88,7 +92,8 @@ public class ProjectResourceTest extends ResourceTest {
public static JerseyTestRule jersey = new JerseyTestRule(
new ResourceConfig(ProjectResource.class)
.register(ApiFilter.class)
.register(AuthenticationFilter.class));
.register(AuthenticationFilter.class)
.register(ProjectOperationExceptionMapper.class));

@After
@Override
Expand Down Expand Up @@ -1249,7 +1254,7 @@ List<UUID> createProjects(int size, boolean accessible) {
}

@Test
public void batchDeleteProjectsTest() {
public void batchDeleteProjectsTest() throws JsonProcessingException {
// Enable portfolio access control.
qm.createConfigProperty(
ConfigPropertyConstants.ACCESS_MANAGEMENT_ACL_ENABLED.getGroupName(),
Expand All @@ -1269,19 +1274,50 @@ public void batchDeleteProjectsTest() {
.post(Entity.json(uuidsOfAccessibleProjects));
Assert.assertEquals(204, response.getStatus(), 0);

// Try to delete them again (they should now be gone)
response = jersey.target(V1_PROJECT + "/batchDelete")
.request()
.header(X_API_KEY, apiKey)
.property(ClientProperties.SUPPRESS_HTTP_COMPLIANCE_VALIDATION, true)
.post(Entity.json(uuidsOfAccessibleProjects));
Assert.assertEquals(400, response.getStatus(), 0);
assertThat(response.getHeaderString("Content-Type")).isEqualTo("application/problem+json");
Map<String, String> expectedErrors = uuidsOfAccessibleProjects.stream()
.collect(Collectors.toMap(UUID::toString, uuid -> "Project not found"));
ObjectMapper objectMapper = new ObjectMapper();
String expectedErrorsJson = objectMapper.writeValueAsString(expectedErrors);
assertThatJson(
getPlainTextBody(response)
).isEqualTo("""
{
"status": 400,
"title": "Project operation failed",
"detail": "One or more projects could not be deleted",
"errors": %s
}
""".formatted(expectedErrorsJson)
);

// Delete only inaccessible projects
response = jersey.target(V1_PROJECT + "/batchDelete")
.request()
.header(X_API_KEY, apiKey)
.property(ClientProperties.SUPPRESS_HTTP_COMPLIANCE_VALIDATION, true)
.post(Entity.json(uuidsOfInaccessibleProjects));
Assert.assertEquals(403, response.getStatus(), 0);
JsonArray jsonResponse = parseJsonArray(response);
JsonArrayBuilder jsonArrayBuilder = Json.createArrayBuilder();
for (UUID uuid: uuidsOfInaccessibleProjects) {
jsonArrayBuilder.add(uuid.toString());
}
JsonArray uuidsOfInaccessibleProjectsAsJson = jsonArrayBuilder.build();
Assert.assertEquals("", uuidsOfInaccessibleProjectsAsJson, jsonResponse);
assertThat(response.getStatus()).isEqualTo(400);
assertThat(response.getHeaderString("Content-Type")).isEqualTo("application/problem+json");
assertThatJson(
getPlainTextBody(response)
).isEqualTo("""
{
"status": 400,
"title": "Project operation failed",
"detail": "One or more projects could not be deleted",
"errors": {
"%": "Access denied to project"
}
}
""".replaceAll("%", uuidsOfInaccessibleProjects.getFirst().toString()));

// Delete mixed accessible + inaccessible projects
List<UUID> uuidsOfMixedProjects = new ArrayList<>();
Expand All @@ -1292,9 +1328,20 @@ public void batchDeleteProjectsTest() {
.request()
.header(X_API_KEY, apiKey)
.post(Entity.json(uuidsOfMixedProjects));
Assert.assertEquals(207, response.getStatus(), 0);
jsonResponse = parseJsonArray(response);
Assert.assertEquals("", uuidsOfInaccessibleProjectsAsJson, jsonResponse);
Assert.assertEquals(400, response.getStatus(), 0);
assertThat(response.getHeaderString("Content-Type")).isEqualTo("application/problem+json");
expectedErrors = uuidsOfInaccessibleProjects.stream()
.collect(Collectors.toMap(UUID::toString, uuid -> "Access denied to project"));
expectedErrorsJson = objectMapper.writeValueAsString(expectedErrors);
assertThatJson(getPlainTextBody(response)).isEqualTo("""
{
"status": 400,
"title": "Project operation failed",
"detail": "One or more projects could not be deleted",
"errors": %s
}
""".formatted(expectedErrorsJson));

}

@Test
Expand Down

0 comments on commit beba9d2

Please sign in to comment.