Skip to content

Closes #2635: Make owner-is-null=owner-is-null not valid, allow use case "owner-is-null=true" #2655

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

Merged
merged 1 commit into from
Aug 8, 2024
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
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
package pro.taskana.common.rest.util;

import com.fasterxml.jackson.annotation.JsonProperty;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.servlet.http.HttpServletRequest;
import pro.taskana.common.api.exceptions.InvalidArgumentException;

public class QueryParamsValidator {

Expand All @@ -35,21 +34,57 @@ public static void validateParams(HttpServletRequest request, Class<?>... filter
if (!providedParams.isEmpty()) {
throw new IllegalArgumentException("Unknown request parameters found: " + providedParams);
}
checkExactParam(request, "owner-is-null");
}

public static void checkExactParam(HttpServletRequest request, String queryParameter) {
String queryString = request.getQueryString();
boolean containParam = queryString != null && queryString.contains(queryParameter);
if (containParam) {
Pattern pattern = Pattern.compile("\\b" + queryParameter + "(&|$)");
Matcher matcher = pattern.matcher(queryString);

boolean hasExactParam = matcher.find();
if (!hasExactParam) {
throw new InvalidArgumentException(
"It is prohibited to use the param " + queryParameter + " with values.");
}
public static boolean hasQueryParameterValues(HttpServletRequest request, String queryParameter) {

Map<String, String[]> queryParametersMap = request.getParameterMap();

if (queryParametersMap.isEmpty()) {
return false;
}

String[] queryParameterValues = queryParametersMap.get(queryParameter);

if (queryParameterValues == null) {
return false;
}

boolean hasQueryParameterNotEmptyValues =
Arrays.stream(queryParameterValues).anyMatch(value -> !value.isBlank());

/* Workaround to manage the case "query-param=".
It should be safe enough to use because we have checked all other possibilities before. */
boolean hasQueryParameterEmptyValues = request.getQueryString().contains(queryParameter + "=");

return hasQueryParameterNotEmptyValues || hasQueryParameterEmptyValues;
}

public static boolean hasQueryParameterValuesOrIsNotTrue(
HttpServletRequest request, String queryParameter) {

Map<String, String[]> queryParametersMap = request.getParameterMap();

if (queryParametersMap.isEmpty()) {
return false;
}

String[] queryParameterValues = queryParametersMap.get(queryParameter);

if (queryParameterValues == null) {
return false;
}

boolean hasQueryParameterProhibitedValues =
Arrays.stream(queryParameterValues)
.anyMatch(value -> !value.isBlank() && !Boolean.parseBoolean(value));

/* Workaround to manage the case "query-param=".
It should be safe enough to use because we have checked all other possibilities before. */
boolean hasQueryParameterEmptyValues =
Arrays.stream(queryParameterValues).allMatch(String::isBlank)
&& request.getQueryString().contains(queryParameter + "=");

return hasQueryParameterProhibitedValues || hasQueryParameterEmptyValues;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,12 @@ public ResponseEntity<TaskRepresentationModel> createTask(
NotAuthorizedOnWorkbasketException {

if (!taskRepresentationModel.getAttachments().stream()
.filter(att -> Objects.nonNull(att.getTaskId()))
.filter(att -> !att.getTaskId().equals(taskRepresentationModel.getTaskId()))
.collect(Collectors.toList()).isEmpty()) {
.filter(att -> Objects.nonNull(att.getTaskId()))
.filter(att -> !att.getTaskId().equals(taskRepresentationModel.getTaskId()))
.collect(Collectors.toList())
.isEmpty()) {
throw new InvalidArgumentException(
"An attachments' taskId must be empty or equal to the id of the task it belongs to");
"An attachments' taskId must be empty or equal to the id of the task it belongs to");
}

Task fromResource = taskRepresentationModelAssembler.toEntityModel(taskRepresentationModel);
Expand All @@ -148,6 +149,7 @@ public ResponseEntity<TaskRepresentationModel> createTask(
* @param sortParameter the sort parameters
* @param pagingParameter the paging parameters
* @return the Tasks with the given filter, sort and paging options.
* @throws InvalidArgumentException if the query parameter "owner-is-null" has values
*/
@GetMapping(path = RestEndpoints.URL_TASKS)
@Transactional(readOnly = true, rollbackFor = Exception.class)
Expand All @@ -167,6 +169,12 @@ public ResponseEntity<TaskSummaryPagedRepresentationModel> getTasks(
TaskQueryGroupByParameter.class,
QuerySortParameter.class,
QueryPagingParameter.class);

if (QueryParamsValidator.hasQueryParameterValuesOrIsNotTrue(request, "owner-is-null")) {
throw new InvalidArgumentException(
"It is prohibited to use the param owner-is-null with values.");
}

TaskQuery query = taskService.createTaskQuery();

filterParameter.apply(query);
Expand Down Expand Up @@ -653,11 +661,12 @@ public ResponseEntity<TaskRepresentationModel> updateTask(
}

if (!taskRepresentationModel.getAttachments().stream()
.filter(att -> Objects.nonNull(att.getTaskId()))
.filter(att -> !att.getTaskId().equals(taskRepresentationModel.getTaskId()))
.collect(Collectors.toList()).isEmpty()) {
.filter(att -> Objects.nonNull(att.getTaskId()))
.filter(att -> !att.getTaskId().equals(taskRepresentationModel.getTaskId()))
.collect(Collectors.toList())
.isEmpty()) {
throw new InvalidArgumentException(
"An attachments' taskId must be empty or equal to the id of the task it belongs to");
"An attachments' taskId must be empty or equal to the id of the task it belongs to");
}

Task task = taskRepresentationModelAssembler.toEntityModel(taskRepresentationModel);
Expand Down
Loading
Loading