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

Server side validation for restricted values #93

Merged
merged 13 commits into from
Aug 2, 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
2 changes: 2 additions & 0 deletions lists-service/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,5 @@ build/
/src/main/resources/graphql/schema.graphql
/src/main/resources/graphql/.graphqlconfig
/src/main/resources/graphql/graphql.config.yml

.DS_STORE
21 changes: 13 additions & 8 deletions lists-service/pom.xml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.springframework.boot</groupId>
Expand Down Expand Up @@ -35,6 +35,12 @@
</repositories>

<dependencies>

<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-actuator</artifactId>
</dependency>

<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-data-elasticsearch</artifactId>
Expand Down Expand Up @@ -115,11 +121,11 @@
<version>2.0.3</version>
</dependency>
<!-- name matching client has issues with Java 19 -->
<!-- <dependency>-->
<!-- <groupId>au.org.ala.names</groupId>-->
<!-- <artifactId>ala-namematching-client</artifactId>-->
<!-- <version>${ala-namematching-service.version}</version>-->
<!-- </dependency>-->
<!-- <dependency>-->
<!-- <groupId>au.org.ala.names</groupId>-->
<!-- <artifactId>ala-namematching-client</artifactId>-->
<!-- <version>${ala-namematching-service.version}</version>-->
<!-- </dependency>-->
<dependency>
<groupId>com.opencsv</groupId>
<artifactId>opencsv</artifactId>
Expand All @@ -130,7 +136,6 @@
<artifactId>spring-boot-starter-security</artifactId>
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
Expand Down
5 changes: 4 additions & 1 deletion lists-service/src/main/docker/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,7 @@ spring.servlet.multipart.max-request-size=10MB
spring.data.mongodb.uri=mongodb://mongodb:27017/species-lists
spring.data.mongodb.host=mongodb
spring.data.mongodb.port=27017
spring.data.mongodb.database=species-lists
spring.data.mongodb.database=species-lists
constraints.file=/resources/constraints.json
userDetails.url=https://api.test.ala.org.au/userdetails/cognito
userDetails.api.url=https://api.test.ala.org.au/userdetails/cognito
Original file line number Diff line number Diff line change
@@ -1,20 +1,11 @@
package au.org.ala.listsapi.controller;

import au.org.ala.listsapi.model.Classification;
import au.org.ala.listsapi.model.Facet;
import au.org.ala.listsapi.model.FacetCount;
import au.org.ala.listsapi.model.Filter;
import au.org.ala.listsapi.model.Image;
import au.org.ala.listsapi.model.InputSpeciesListItem;
import au.org.ala.listsapi.model.KeyValue;
import au.org.ala.listsapi.model.Release;
import au.org.ala.listsapi.model.SpeciesList;
import au.org.ala.listsapi.model.SpeciesListIndex;
import au.org.ala.listsapi.model.SpeciesListItem;
import au.org.ala.listsapi.model.*;
import au.org.ala.listsapi.repo.ReleaseMongoRepository;
import au.org.ala.listsapi.repo.SpeciesListIndexElasticRepository;
import au.org.ala.listsapi.repo.SpeciesListItemMongoRepository;
import au.org.ala.listsapi.repo.SpeciesListMongoRepository;
import au.org.ala.listsapi.service.ValidationService;
import au.org.ala.listsapi.service.TaxonService;
import co.elastic.clients.elasticsearch._types.FieldSort;
import co.elastic.clients.elasticsearch._types.SortOrder;
Expand All @@ -25,6 +16,8 @@
import co.elastic.clients.elasticsearch._types.query_dsl.Operator;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import graphql.*;
import graphql.schema.DataFetchingEnvironment;
import io.swagger.v3.oas.annotations.tags.Tag;
import java.net.URL;
import java.security.Principal;
Expand All @@ -51,8 +44,10 @@
import org.springframework.data.elasticsearch.core.mapping.IndexCoordinates;
import org.springframework.data.elasticsearch.core.query.Query;
import org.springframework.graphql.data.method.annotation.Argument;
import org.springframework.graphql.data.method.annotation.GraphQlExceptionHandler;
import org.springframework.graphql.data.method.annotation.QueryMapping;
import org.springframework.graphql.data.method.annotation.SchemaMapping;
import org.springframework.lang.NonNull;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.stereotype.Controller;
Expand Down Expand Up @@ -105,6 +100,7 @@ public class GraphQLController {
@Autowired private SpeciesListItemMongoRepository speciesListItemMongoRepository;

@Autowired protected TaxonService taxonService;
@Autowired protected ValidationService validationService;
@Autowired protected AuthUtils authUtils;

public static SpeciesListItem convert(SpeciesListIndex index) {
Expand Down Expand Up @@ -273,6 +269,17 @@ static void addFilter(Filter filter, BoolQuery.Builder bq) {
}
}

@GraphQlExceptionHandler
public GraphQLError handle(@NonNull Throwable ex, @NonNull DataFetchingEnvironment environment){
return GraphQLError
.newError()
.errorType(ErrorType.ValidationError)
.message(ex.getMessage())
.path(environment.getExecutionStepInfo().getPath())
.location(environment.getField().getSourceLocation())
.build();
}

@QueryMapping
public Page<Release> listReleases(
@Argument String speciesListID, @Argument Integer page, @Argument Integer size) {
Expand Down Expand Up @@ -664,7 +671,7 @@ public SpeciesList updateMetadata(
@Argument Boolean isSDS,
@Argument Boolean isBIE,
@Argument List<String> tags,
@AuthenticationPrincipal Principal principal) {
@AuthenticationPrincipal Principal principal) throws Exception {
Optional<SpeciesList> speciesList = speciesListMongoRepository.findById(id);
if (speciesList.isEmpty()) {
return null;
Expand All @@ -674,6 +681,15 @@ public SpeciesList updateMetadata(
boolean reindexRequired = false;

SpeciesList toUpdate = speciesList.get();

// check that the supplied list type, region and license is valid
if (
!validationService.isValueValid(ConstraintType.lists, listType) ||
!validationService.isValueValid(ConstraintType.licenses, licence)
) {
throw new Exception("Updated list contains invalid properties for a controlled value (list type, license)");
}

if (title != null && !title.equalsIgnoreCase(toUpdate.getTitle())
|| listType != null && !listType.equalsIgnoreCase(toUpdate.getListType())
|| isPrivate != null && !isPrivate.equals(toUpdate.getIsPrivate())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@

import au.org.ala.listsapi.model.*;
import au.org.ala.listsapi.repo.SpeciesListMongoRepository;
import au.org.ala.listsapi.service.MigrateService;
import au.org.ala.listsapi.service.ReleaseService;
import au.org.ala.listsapi.service.TaxonService;
import au.org.ala.listsapi.service.UploadService;
import au.org.ala.listsapi.service.*;
import au.org.ala.ws.security.profile.AlaUserProfile;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.enums.SecuritySchemeType;
Expand Down Expand Up @@ -57,6 +54,7 @@ public class IngressController {
@Autowired protected ReleaseService releaseService;
@Autowired protected UploadService uploadService;
@Autowired protected MigrateService migrateService;
@Autowired protected ValidationService validationService;

@Autowired protected AuthUtils authUtils;

Expand Down Expand Up @@ -258,6 +256,11 @@ public ResponseEntity<Object> ingest(
return ResponseEntity.badRequest().body("User not found");
}

// check that the supplied list type, region and license is valid
if (!validationService.isListValid(speciesList)) {
return ResponseEntity.badRequest().body("Supplied list contains invalid properties for a controlled value (list type, license, region)");
}

logger.info("Ingestion started...");
File tempFile = new File(tempDir + "/" + fileName);
if (!tempFile.exists()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package au.org.ala.listsapi.controller;

import au.org.ala.listsapi.model.ConstraintType;
import au.org.ala.listsapi.service.ValidationService;
import io.swagger.v3.oas.annotations.Operation;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.CrossOrigin;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;

@CrossOrigin(origins = "*", maxAge = 3600)
@org.springframework.web.bind.annotation.RestController
public class UtilsController {
@Autowired protected ValidationService validationService;

@Operation(tags = "REST", summary = "Get all constraint lists")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The openapi annotations could be more detailed. I'll leave this as optional as the same can be said for the other services.

@GetMapping("/constraints")
public ResponseEntity<Object> constraints() {
try {
return new ResponseEntity<>(
validationService.getConstraintMap(),
HttpStatus.OK
);
} catch (Exception e) {
return ResponseEntity.badRequest().body(e.getMessage());
}
}

@Operation(tags = "REST", summary = "Get constraints for list types, licenses, countries")
@GetMapping("/constraints/{type:lists|licenses|countries}")
public ResponseEntity<Object> constraintsForType(
@PathVariable("type") ConstraintType constraintType) {
try {
return new ResponseEntity<>(
validationService.getConstraintList(constraintType),
HttpStatus.OK
);
} catch (Exception e) {
return ResponseEntity.badRequest().body(e.getMessage());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package au.org.ala.listsapi.model;

import lombok.AllArgsConstructor;
import lombok.Data;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.experimental.SuperBuilder;

@Getter
@NoArgsConstructor
@Data
@SuperBuilder
@AllArgsConstructor
public class ConstraintListItem {
private String value;
private String label;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package au.org.ala.listsapi.model;

public enum ConstraintType {
lists,
licenses,
countries,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package au.org.ala.listsapi.model;

import lombok.AllArgsConstructor;
import lombok.Data;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.experimental.SuperBuilder;

@Getter
@NoArgsConstructor
@Data
@SuperBuilder
@AllArgsConstructor
public class Location {
private String isoCode;
private String name;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
package au.org.ala.listsapi.service;

import au.org.ala.listsapi.model.*;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import jakarta.annotation.PostConstruct;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Service;

import java.io.File;
import java.io.IOException;
import java.net.ProxySelector;
import java.net.URI;
import java.net.http.HttpClient;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

@Service
public class ValidationService {

public static final Logger log = LoggerFactory.getLogger(ValidationService.class);

@Value("${constraints.file}")
jack-brinkman marked this conversation as resolved.
Show resolved Hide resolved
private String constraintsFile;

@Value("${userDetails.api.url}")
jack-brinkman marked this conversation as resolved.
Show resolved Hide resolved
private String userDetailsUrl;

private Map<String, List<ConstraintListItem>> constraints = null;

private List<ConstraintListItem> getConstraintsByKey(ConstraintType constraintType) {
return constraints.get(constraintType.name());
}

private void setConstraintsByKey(ConstraintType constraintType, List<ConstraintListItem> items) {
constraints.put(constraintType.name(), items);
}

@PostConstruct
private void init() throws IOException {
ObjectMapper objectMapper = new ObjectMapper();
File json = new File(constraintsFile);

constraints = objectMapper.readValue(json, new TypeReference<HashMap<String, List<ConstraintListItem>>>() {});

try {
List<Location> userdetailsCountries = fetchJson(userDetailsUrl + "/ws/registration/countries.json", new TypeReference<>() {});
List<ConstraintListItem> countries = getConstraintsByKey(ConstraintType.countries);

// Map the countries list into UI constraints
userdetailsCountries.forEach(e -> {
var constraint = new ConstraintListItem();
constraint.setLabel(e.getName());
constraint.setValue(e.getIsoCode());

countries.add(constraint);
});

setConstraintsByKey(ConstraintType.countries, countries);
} catch (Exception ex) {
log.error("Error loading country constraints from userdetails", ex);
}
}
private <T> T fetchJson(String uri, TypeReference<T> type) throws Exception {
HttpRequest httpRequest =
HttpRequest.newBuilder(new URI(uri))
.GET()
.build();

HttpResponse<String> response =
HttpClient.newBuilder()
.proxy(ProxySelector.getDefault())
.build()
.send(httpRequest, HttpResponse.BodyHandlers.ofString());

ObjectMapper objectMapper = new ObjectMapper();
return objectMapper.readValue(response.body(), type);
}

// Returns the constraint list as a map of List<ConstraintListItem> to correctly output JSON
public Map<String, List<ConstraintListItem>> getConstraintMap() {
return constraints;
}

public boolean isValueValid(ConstraintType constraintType, String value) {
List<ConstraintListItem> list = getConstraintsByKey(constraintType);

return list.stream()
.filter(elm -> elm.getValue().equals(value))
.findAny().orElse(null) != null;
}

public boolean isListValid(InputSpeciesList speciesList) {
// check that the supplied list type, region and license is valid
return (
isValueValid(ConstraintType.lists, speciesList.getListType()) &&
isValueValid(ConstraintType.licenses, speciesList.getLicence())
);
}

public List<ConstraintListItem> getConstraintList(ConstraintType constraintType) throws Exception {
List<ConstraintListItem> list = getConstraintsByKey(constraintType);

if (list == null) {
throw new Exception("Could not find corresponding constraint list for '" + constraintType + "' type!");
}

return list;
}
}
Loading