Skip to content

Commit

Permalink
Fix breaking change in PUT /api/v1/project endpoint
Browse files Browse the repository at this point in the history 
With DependencyTrack#4093, the `accessTeams` field became a required request parameter if portfolio ACL is enabled. While it does make sense to enforce team assignment, it is a breaking change in the REST API and as such cannot be done in a minor version release.

Make `accessTeams` optional. Also:

* Enable `accessTeams` to be specified by `name`, not only by `uuid`
* Document this feature in the OpenAPI spec
* Reduce nesting in the `ProjectResource#createProject` method
* Enhance tests to be more explicit and assert response content

Signed-off-by: nscuro <[email protected]>
  • Loading branch information
@nscuro
nscuro committed Sep 28, 2024
1 parent d03cb83 commit 4cd5671
Show file tree
Hide file tree
Showing 3 changed files with 389 additions and 141 deletions.
145 changes: 96 additions & 49 deletions src/main/java/org/dependencytrack/resources/v1/ProjectResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import org.dependencytrack.auth.Permissions;
import org.dependencytrack.event.CloneProjectEvent;
import org.dependencytrack.model.Classifier;
import org.dependencytrack.model.ConfigPropertyConstants;
import org.dependencytrack.model.Project;
import org.dependencytrack.model.Tag;
import org.dependencytrack.model.validation.ValidUuid;
Expand All @@ -65,14 +64,17 @@
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.List;
import java.util.Set;
import java.util.UUID;
import java.util.function.BiConsumer;
import java.util.function.Function;

import static java.util.Objects.requireNonNullElseGet;
import static org.dependencytrack.util.PersistenceUtil.isPersistent;

/**
* JAX-RS resources for processing projects.
*
Expand Down Expand Up @@ -274,6 +276,13 @@ public Response getProjectsByClassifier(
summary = "Creates a new project",
description = """
<p>If a parent project exists, <code>parent.uuid</code> is required</p>
<p>
When portfolio access control is enabled, one or more teams to grant access
to can be provided via <code>accessTeams</code>. Either <code>uuid</code> or
<code>name</code> of a team must be specified. Only teams which the authenticated
principal is a member of can be assigned. Principals with <strong>ACCESS_MANAGEMENT</strong>
permission can assign <em>any</em> team.
</p>
<p>Requires permission <strong>PORTFOLIO_MANAGEMENT</strong></p>
"""
)
Expand All @@ -283,17 +292,16 @@ public Response getProjectsByClassifier(
description = "The created project",
content = @Content(schema = @Schema(implementation = Project.class))
),
@ApiResponse(responseCode = "400", description = "Bad Request"),
@ApiResponse(responseCode = "401", description = "Unauthorized"),
@ApiResponse(responseCode = "403", description = "You don't have the permission to assign this team to a project."),
@ApiResponse(responseCode = "409", description = """
<ul>
<li>An inactive Parent cannot be selected as parent, or</li>
<li>A project with the specified name already exists</li>
</ul>"""),
@ApiResponse(responseCode = "422", description = "You need to specify at least one team to which the project should belong"),
</ul>""")
})
@PermissionRequired(Permissions.Constants.PORTFOLIO_MANAGEMENT)
public Response createProject(Project jsonProject) {
public Response createProject(final Project jsonProject) {
final Validator validator = super.getValidator();
failOnValidationError(
validator.validateProperty(jsonProject, "author"),
Expand All @@ -312,58 +320,97 @@ public Response createProject(Project jsonProject) {
if (jsonProject.getClassifier() == null) {
jsonProject.setClassifier(Classifier.APPLICATION);
}
try (QueryManager qm = new QueryManager()) {
try (final var qm = new QueryManager()) {
if (qm.doesProjectExist(StringUtils.trimToNull(jsonProject.getName()),
StringUtils.trimToNull(jsonProject.getVersion()))) {
return Response
.status(Response.Status.CONFLICT)
.entity("A project with the specified name already exists.")
.build();
}

if (jsonProject.getParent() != null && jsonProject.getParent().getUuid() != null) {
Project parent = qm.getObjectByUuid(Project.class, jsonProject.getParent().getUuid());
jsonProject.setParent(parent);
jsonProject.setParent(parent);
}
if (!qm.doesProjectExist(StringUtils.trimToNull(jsonProject.getName()),
StringUtils.trimToNull(jsonProject.getVersion()))) {
final List<Team> chosenTeams = jsonProject.getAccessTeams() == null ? new ArrayList<Team>()
: jsonProject.getAccessTeams();
boolean required = qm.isEnabled(ConfigPropertyConstants.ACCESS_MANAGEMENT_ACL_ENABLED);
if (required && chosenTeams.isEmpty()) {
return Response.status(422)
.entity("You need to specify at least one team to which the project should belong").build();

final Principal principal = getPrincipal();

final List<Team> chosenTeams = requireNonNullElseGet(
jsonProject.getAccessTeams(), Collections::emptyList);
jsonProject.setAccessTeams(null);

for (final Team chosenTeam : chosenTeams) {
if (chosenTeam.getUuid() == null && chosenTeam.getName() == null) {
return Response
.status(Response.Status.BAD_REQUEST)
.entity("""
accessTeams must either specify a UUID or a name,\
but the team at index %d has neither.\
""".formatted(chosenTeams.indexOf(chosenTeam)))
.build();
}
Principal principal = getPrincipal();
if (!chosenTeams.isEmpty()) {
List<Team> userTeams = new ArrayList<Team>();
if (principal instanceof final UserPrincipal userPrincipal) {
userTeams = userPrincipal.getTeams();
} else if (principal instanceof final ApiKey apiKey) {
userTeams = apiKey.getTeams();
}

if (!chosenTeams.isEmpty()) {
final List<Team> userTeams;
if (principal instanceof final UserPrincipal userPrincipal) {
userTeams = userPrincipal.getTeams();
} else if (principal instanceof final ApiKey apiKey) {
userTeams = apiKey.getTeams();
} else {
userTeams = Collections.emptyList();
}

final boolean isAdmin = qm.hasAccessManagementPermission(principal);
final List<Team> visibleTeams = isAdmin ? qm.getTeams() : userTeams;
final var visibleTeamByUuid = new HashMap<UUID, Team>(visibleTeams.size());
final var visibleTeamByName = new HashMap<String, Team>(visibleTeams.size());
for (final Team visibleTeam : visibleTeams) {
visibleTeamByUuid.put(visibleTeam.getUuid(), visibleTeam);
visibleTeamByName.put(visibleTeam.getName(), visibleTeam);
}

for (final Team chosenTeam : chosenTeams) {
Team visibleTeam = visibleTeamByUuid.getOrDefault(
chosenTeam.getUuid(),
visibleTeamByName.get(chosenTeam.getName()));

if (visibleTeam == null) {
return Response
.status(Response.Status.BAD_REQUEST)
.entity("""
The team with %s can not be assigned because it does not exist, \
or is not accessible to the authenticated principal.\
""".formatted(chosenTeam.getUuid() != null
? "UUID " + chosenTeam.getUuid()
: "name " + chosenTeam.getName()))
.build();
}
boolean isAdmin = qm.hasAccessManagementPermission(principal);
List<Team> visibleTeams = isAdmin ? qm.getTeams() : userTeams;
List<UUID> visibleUuids = visibleTeams.isEmpty() ? new ArrayList<UUID>()
: visibleTeams.stream().map(Team::getUuid).toList();
jsonProject.setAccessTeams(new ArrayList<Team>());
for (Team choosenTeam : chosenTeams) {
if (!visibleUuids.contains(choosenTeam.getUuid())) {
return isAdmin ? Response.status(404).entity("This team does not exist!").build()
: Response.status(403)
.entity("You don't have the permission to assign this team to a project.")
.build();
}
Team ormTeam = qm.getObjectByUuid(Team.class, choosenTeam.getUuid());
jsonProject.addAccessTeam(ormTeam);

if (!isPersistent(visibleTeam)) {
// Teams sourced from the principal will not be in persistent state
// and need to be attached to the persistence context.
visibleTeam = qm.getObjectById(Team.class, visibleTeam.getId());
}
}

final Project project;
try {
project = qm.createProject(jsonProject, jsonProject.getTags(), true);
} catch (IllegalArgumentException e){
LOGGER.debug(e.getMessage());
return Response.status(Response.Status.CONFLICT).entity("An inactive Parent cannot be selected as parent").build();
jsonProject.addAccessTeam(visibleTeam);
}
qm.updateNewProjectACL(project, principal);
LOGGER.info("Project " + project.toString() + " created by " + super.getPrincipal().getName());
return Response.status(Response.Status.CREATED).entity(project).build();
} else {
return Response.status(Response.Status.CONFLICT).entity("A project with the specified name already exists.").build();
}

final Project project;
try {
project = qm.createProject(jsonProject, jsonProject.getTags(), true);
} catch (IllegalArgumentException e) {
LOGGER.debug("Failed to create project", e);
return Response
.status(Response.Status.CONFLICT)
.entity("An inactive Parent cannot be selected as parent")
.build();
}
qm.updateNewProjectACL(project, principal);
LOGGER.info("Project " + project.toString() + " created by " + super.getPrincipal().getName());
return Response.status(Response.Status.CREATED).entity(project).build();
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/dependencytrack/util/PersistenceUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ public static void assertNonPersistentAll(final Collection<?> objects, final Str
objects.forEach(object -> assertNonPersistent(object, message));
}

private static boolean isPersistent(final Object object) {
public static boolean isPersistent(final Object object) {
final ObjectState objectState = JDOHelper.getObjectState(object);
return objectState == PERSISTENT_CLEAN
|| objectState == PERSISTENT_DIRTY
Expand Down
Loading

0 comments on commit 4cd5671

Please sign in to comment.