From 7f33d51791a0f127f41caa522cb968956272cffc Mon Sep 17 00:00:00 2001 From: Patrick Boos Date: Tue, 23 May 2023 16:19:08 +0200 Subject: [PATCH 1/7] Support fail on request/response violation in webflux --- .../api/model/ValidationResult.java | 3 + .../api/selector/DefaultTrafficSelector.java | 24 ++ .../api/selector/TrafficSelector.java | 8 + .../core/OpenApiRequestValidator.java | 26 +- ...penApiValidationApplicationProperties.java | 2 + .../FallbackLibraryAutoConfiguration.java | 7 +- .../spring-configuration-metadata.json | 10 + ...piValidationApplicationPropertiesTest.java | 8 +- .../filter/OpenApiValidationWebFilter.java | 91 ++++- ...BodyCachingServerHttpRequestDecorator.java | 11 +- ...odyCachingServerHttpResponseDecorator.java | 11 +- .../spring-boot-starter-webflux/build.gradle | 1 + .../filter/OpenApiValidationWebFilter.java | 91 ++++- ...BodyCachingServerHttpRequestDecorator.java | 11 +- ...odyCachingServerHttpResponseDecorator.java | 11 +- .../OpenApiValidationWebFilterTest.java | 340 ++++++++++++++++++ 16 files changed, 631 insertions(+), 24 deletions(-) create mode 100644 openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/model/ValidationResult.java create mode 100644 spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilterTest.java diff --git a/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/model/ValidationResult.java b/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/model/ValidationResult.java new file mode 100644 index 00000000..d8892d90 --- /dev/null +++ b/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/model/ValidationResult.java @@ -0,0 +1,3 @@ +package com.getyourguide.openapi.validation.api.model; + +public enum ValidationResult { VALID, INVALID, NOT_APPLICABLE } diff --git a/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/selector/DefaultTrafficSelector.java b/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/selector/DefaultTrafficSelector.java index 48c67038..a2cbd64e 100644 --- a/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/selector/DefaultTrafficSelector.java +++ b/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/selector/DefaultTrafficSelector.java @@ -11,10 +11,24 @@ public class DefaultTrafficSelector implements TrafficSelector { private final double sampleRate; private final Set excludedPaths; + private final Boolean shouldFailOnRequestViolation; + private final Boolean shouldFailOnResponseViolation; public DefaultTrafficSelector(Double sampleRate, Set excludedPaths) { + this(sampleRate, excludedPaths, null, null); + } + + public DefaultTrafficSelector( + Double sampleRate, + Set excludedPaths, + Boolean shouldFailOnRequestViolation, + Boolean shouldFailOnResponseViolation + ) { this.sampleRate = sampleRate != null ? sampleRate : SAMPLE_RATE_DEFAULT; this.excludedPaths = excludedPaths != null ? excludedPaths : Set.of(); + this.shouldFailOnRequestViolation = shouldFailOnRequestViolation != null ? shouldFailOnRequestViolation : false; + this.shouldFailOnResponseViolation = + shouldFailOnResponseViolation != null ? shouldFailOnResponseViolation : false; } @Override @@ -40,6 +54,16 @@ public boolean canResponseBeValidated(RequestMetaData request, ResponseMetaData && isContentTypeSupported(response.getContentType()); } + @Override + public boolean shouldFailOnRequestViolation(RequestMetaData request) { + return shouldFailOnRequestViolation; + } + + @Override + public boolean shouldFailOnResponseViolation(RequestMetaData request) { + return shouldFailOnResponseViolation; + } + private boolean isExcludedRequest(RequestMetaData request) { return excludedPaths.contains(request.getUri().getPath()); } diff --git a/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/selector/TrafficSelector.java b/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/selector/TrafficSelector.java index 93075d9a..bfce20aa 100644 --- a/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/selector/TrafficSelector.java +++ b/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/selector/TrafficSelector.java @@ -13,4 +13,12 @@ default boolean canRequestBeValidated(RequestMetaData request) { default boolean canResponseBeValidated(RequestMetaData request, ResponseMetaData response) { return true; } + + default boolean shouldFailOnRequestViolation(RequestMetaData request) { + return false; + } + + default boolean shouldFailOnResponseViolation(RequestMetaData request) { + return false; + } } diff --git a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidator.java b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidator.java index 4ec73fa7..e27e529f 100644 --- a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidator.java +++ b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidator.java @@ -3,9 +3,11 @@ import com.atlassian.oai.validator.model.Request; import com.atlassian.oai.validator.model.SimpleRequest; import com.atlassian.oai.validator.model.SimpleResponse; +import com.atlassian.oai.validator.report.ValidationReport; import com.getyourguide.openapi.validation.api.model.Direction; import com.getyourguide.openapi.validation.api.model.RequestMetaData; import com.getyourguide.openapi.validation.api.model.ResponseMetaData; +import com.getyourguide.openapi.validation.api.model.ValidationResult; import com.getyourguide.openapi.validation.api.model.ValidatorConfiguration; import com.getyourguide.openapi.validation.core.validator.OpenApiInteractionValidatorWrapper; import java.nio.charset.StandardCharsets; @@ -39,13 +41,15 @@ public void validateResponseObjectAsync(final RequestMetaData request, ResponseM threadPool.execute(() -> validateResponseObject(request, response, responseBody)); } - private void validateRequestObject(final RequestMetaData request, String requestBody) { + public ValidationResult validateRequestObject(final RequestMetaData request, String requestBody) { try { var simpleRequest = buildSimpleRequest(request, requestBody); var result = validator.validateRequest(simpleRequest); validationReportHandler.handleValidationReport(request, Direction.REQUEST, requestBody, result); + return buildValidationResult(result); } catch (Exception e) { log.error("Could not validate request", e); + return ValidationResult.NOT_APPLICABLE; } } @@ -60,7 +64,11 @@ private static SimpleRequest buildSimpleRequest(RequestMetaData request, String return requestBuilder.build(); } - private void validateResponseObject(final RequestMetaData request, ResponseMetaData response, final String responseBody) { + public ValidationResult validateResponseObject( + final RequestMetaData request, + ResponseMetaData response, + final String responseBody + ) { try { var responseBuilder = new SimpleResponse.Builder(response.getStatusCode()); response.getHeaders().forEach(responseBuilder::withHeader); @@ -75,8 +83,22 @@ private void validateResponseObject(final RequestMetaData request, ResponseMetaD responseBuilder.build() ); validationReportHandler.handleValidationReport(request, Direction.RESPONSE, responseBody, result); + return buildValidationResult(result); } catch (Exception e) { log.error("Could not validate response", e); + return ValidationResult.NOT_APPLICABLE; } } + + private ValidationResult buildValidationResult(ValidationReport validationReport) { + if (validationReport == null) { + return ValidationResult.NOT_APPLICABLE; + } + + if (validationReport.getMessages().isEmpty()) { + return ValidationResult.VALID; + } + + return ValidationResult.INVALID; + } } diff --git a/spring-boot-starter/spring-boot-starter-core/src/main/java/com/getyourguide/openapi/validation/OpenApiValidationApplicationProperties.java b/spring-boot-starter/spring-boot-starter-core/src/main/java/com/getyourguide/openapi/validation/OpenApiValidationApplicationProperties.java index 9f23dccc..d80b4834 100644 --- a/spring-boot-starter/spring-boot-starter-core/src/main/java/com/getyourguide/openapi/validation/OpenApiValidationApplicationProperties.java +++ b/spring-boot-starter/spring-boot-starter-core/src/main/java/com/getyourguide/openapi/validation/OpenApiValidationApplicationProperties.java @@ -27,6 +27,8 @@ public class OpenApiValidationApplicationProperties { private String validationReportMetricName; private String validationReportMetricAdditionalTags; private String excludedPaths; + private Boolean shouldFailOnRequestViolation; + private Boolean shouldFailOnResponseViolation; public List getValidationReportMetricAdditionalTags() { if (validationReportMetricAdditionalTags == null) { diff --git a/spring-boot-starter/spring-boot-starter-core/src/main/java/com/getyourguide/openapi/validation/autoconfigure/FallbackLibraryAutoConfiguration.java b/spring-boot-starter/spring-boot-starter-core/src/main/java/com/getyourguide/openapi/validation/autoconfigure/FallbackLibraryAutoConfiguration.java index ea58cff6..0d364db3 100644 --- a/spring-boot-starter/spring-boot-starter-core/src/main/java/com/getyourguide/openapi/validation/autoconfigure/FallbackLibraryAutoConfiguration.java +++ b/spring-boot-starter/spring-boot-starter-core/src/main/java/com/getyourguide/openapi/validation/autoconfigure/FallbackLibraryAutoConfiguration.java @@ -21,7 +21,12 @@ public class FallbackLibraryAutoConfiguration { @Bean @ConditionalOnMissingBean public TrafficSelector defaultTrafficSelector() { - return new DefaultTrafficSelector(properties.getSampleRate(), properties.getExcludedPathsAsSet()); + return new DefaultTrafficSelector( + properties.getSampleRate(), + properties.getExcludedPathsAsSet(), + properties.getShouldFailOnRequestViolation(), + properties.getShouldFailOnResponseViolation() + ); } } diff --git a/spring-boot-starter/spring-boot-starter-core/src/main/resources/META-INF/spring-configuration-metadata.json b/spring-boot-starter/spring-boot-starter-core/src/main/resources/META-INF/spring-configuration-metadata.json index ed414a1f..ccc71917 100644 --- a/spring-boot-starter/spring-boot-starter-core/src/main/resources/META-INF/spring-configuration-metadata.json +++ b/spring-boot-starter/spring-boot-starter-core/src/main/resources/META-INF/spring-configuration-metadata.json @@ -29,6 +29,16 @@ "name": "openapi.validation.validation-report-metric-additional-tags", "type": "java.lang.String", "description": "Additional tags to be logged with metrics. They should be in the format {KEY}={VALUE},{KEY}={VALUE}. Default is no additional tags." + }, + { + "name": "openapi.validation.should-fail-on-request-violation", + "type": "java.lang.Boolean", + "description": "If set to true the request will fail in case a request violation occurs. Defaults to false." + }, + { + "name": "openapi.validation.should-fail-on-response-violation", + "type": "java.lang.Boolean", + "description": "If set to true the request will fail in case a response violation occurs. Defaults to false." } ] } diff --git a/spring-boot-starter/spring-boot-starter-core/src/test/java/com/getyourguide/openapi/validation/OpenApiValidationApplicationPropertiesTest.java b/spring-boot-starter/spring-boot-starter-core/src/test/java/com/getyourguide/openapi/validation/OpenApiValidationApplicationPropertiesTest.java index 88adf44c..95d27bb1 100644 --- a/spring-boot-starter/spring-boot-starter-core/src/test/java/com/getyourguide/openapi/validation/OpenApiValidationApplicationPropertiesTest.java +++ b/spring-boot-starter/spring-boot-starter-core/src/test/java/com/getyourguide/openapi/validation/OpenApiValidationApplicationPropertiesTest.java @@ -1,6 +1,8 @@ package com.getyourguide.openapi.validation; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import com.getyourguide.openapi.validation.api.metrics.MetricTag; import java.util.List; @@ -24,7 +26,9 @@ void getters() { VALIDATION_REPORT_THROTTLE_WAIT_SECONDS, VALIDATION_REPORT_METRIC_NAME, VALIDATION_REPORT_METRIC_ADDITONAL_TAGS_STRING, - EXCLUDED_PATHS + EXCLUDED_PATHS, + true, + false ); assertEquals(SAMPLE_RATE, loggingConfiguration.getSampleRate()); @@ -37,5 +41,7 @@ void getters() { ); assertEquals(EXCLUDED_PATHS, loggingConfiguration.getExcludedPaths()); assertEquals(Set.of("/_readiness","/_liveness","/_metrics"), loggingConfiguration.getExcludedPathsAsSet()); + assertTrue(loggingConfiguration.getShouldFailOnRequestViolation()); + assertFalse(loggingConfiguration.getShouldFailOnResponseViolation()); } } diff --git a/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java b/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java index f0972792..d2e5f831 100644 --- a/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java +++ b/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java @@ -1,6 +1,7 @@ package com.getyourguide.openapi.validation.filter; import com.getyourguide.openapi.validation.api.model.RequestMetaData; +import com.getyourguide.openapi.validation.api.model.ValidationResult; import com.getyourguide.openapi.validation.api.selector.TrafficSelector; import com.getyourguide.openapi.validation.core.OpenApiRequestValidator; import com.getyourguide.openapi.validation.factory.ReactiveMetaDataFactory; @@ -8,10 +9,13 @@ import com.getyourguide.openapi.validation.filter.decorator.BodyCachingServerHttpResponseDecorator; import com.getyourguide.openapi.validation.filter.decorator.DecoratorBuilder; import lombok.AllArgsConstructor; +import lombok.extern.slf4j.Slf4j; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; +import org.springframework.http.HttpStatus; import org.springframework.lang.NonNull; import org.springframework.stereotype.Component; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebFilter; import org.springframework.web.server.WebFilterChain; @@ -21,6 +25,7 @@ @Component @Order(Ordered.HIGHEST_PRECEDENCE) @AllArgsConstructor +@Slf4j public class OpenApiValidationWebFilter implements WebFilter { private final OpenApiRequestValidator validator; @@ -48,31 +53,101 @@ private Mono decorateWithValidation(ServerWebExchange exchange, WebFilterC var serverWebExchange = exchange.mutate().request(requestDecorated).response(responseDecorated).build(); + var alreadyDidRequestValidation = validateRequestWithFailOnViolation(requestMetaData, requestDecorated); + var alreadyDidResponseValidation = validateResponseWithFailOnViolation(requestMetaData, responseDecorated); + return chain.filter(serverWebExchange) .doFinally(signalType -> { // Note: Errors are not handled here. They could be handled with SignalType.ON_ERROR, but then the response body can't be accessed. // Reason seems to be that those don't use the decorated response that is set here, but use the previous response object. if (signalType == SignalType.ON_COMPLETE) { - validateRequest(requestDecorated, requestMetaData); - validateResponse(responseDecorated, requestMetaData); + if (!alreadyDidRequestValidation) { + validateRequest(requestDecorated, requestMetaData, RunType.ASYNC); + } + if (!alreadyDidResponseValidation) { + validateResponse(responseDecorated, requestMetaData, RunType.ASYNC); + } } }); } - private void validateRequest(BodyCachingServerHttpRequestDecorator request, RequestMetaData requestMetaData) { + /** @return true if validation is done as part of this method */ + private boolean validateRequestWithFailOnViolation( + RequestMetaData requestMetaData, + BodyCachingServerHttpRequestDecorator requestDecorated + ) { + if (!trafficSelector.shouldFailOnRequestViolation(requestMetaData)) { + return false; + } + + if (requestDecorated.getHeaders().containsKey("Content-Type") && requestDecorated.getHeaders().containsKey("Content-Length")) { + requestDecorated.setOnBodyCachedListener(() -> { + var validateRequestResult = validateRequest(requestDecorated, requestMetaData, RunType.SYNC); + throwStatusExceptionOnViolation(validateRequestResult, "Request validation failed"); + }); + } else { + var validateRequestResult = validateRequest(requestDecorated, requestMetaData, RunType.SYNC); + throwStatusExceptionOnViolation(validateRequestResult, "Request validation failed"); + } + return true; + } + + private static void throwStatusExceptionOnViolation(ValidationResult validateRequestResult, String message) { + if (validateRequestResult == ValidationResult.INVALID) { + throw new ResponseStatusException(HttpStatus.valueOf(400), message); + } + } + + /** @return true if validation is done as part of this method */ + private boolean validateResponseWithFailOnViolation( + RequestMetaData requestMetaData, + BodyCachingServerHttpResponseDecorator responseDecorated + ) { + if (!trafficSelector.shouldFailOnResponseViolation(requestMetaData)) { + return false; + } + + responseDecorated.setOnBodyCachedListener(() -> { + var validateResponseResult = validateResponse(responseDecorated, requestMetaData, RunType.SYNC); + throwStatusExceptionOnViolation(validateResponseResult, "Response validation failed"); + }); + return true; + } + + private ValidationResult validateRequest( + BodyCachingServerHttpRequestDecorator request, + RequestMetaData requestMetaData, + RunType runType + ) { if (!trafficSelector.canRequestBeValidated(requestMetaData)) { - return; + return ValidationResult.NOT_APPLICABLE; } - validator.validateRequestObjectAsync(requestMetaData, request.getCachedBody()); + if (runType == RunType.SYNC) { + return validator.validateRequestObject(requestMetaData, request.getCachedBody()); + } else { + validator.validateRequestObjectAsync(requestMetaData, request.getCachedBody()); + return ValidationResult.NOT_APPLICABLE; + } } - private void validateResponse(BodyCachingServerHttpResponseDecorator response, RequestMetaData requestMetaData) { + private ValidationResult validateResponse( + BodyCachingServerHttpResponseDecorator response, + RequestMetaData requestMetaData, + RunType runType + ) { var responseMetaData = metaDataFactory.buildResponseMetaData(response); if (!trafficSelector.canResponseBeValidated(requestMetaData, responseMetaData)) { - return; + return ValidationResult.NOT_APPLICABLE; } - validator.validateResponseObjectAsync(requestMetaData, responseMetaData, response.getCachedBody()); + if (runType == RunType.SYNC) { + return validator.validateResponseObject(requestMetaData, responseMetaData, response.getCachedBody()); + } else { + validator.validateResponseObjectAsync(requestMetaData, responseMetaData, response.getCachedBody()); + return ValidationResult.NOT_APPLICABLE; + } } + + private enum RunType {SYNC, ASYNC} } diff --git a/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/decorator/BodyCachingServerHttpRequestDecorator.java b/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/decorator/BodyCachingServerHttpRequestDecorator.java index 439fd36d..3b25ea01 100644 --- a/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/decorator/BodyCachingServerHttpRequestDecorator.java +++ b/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/decorator/BodyCachingServerHttpRequestDecorator.java @@ -4,6 +4,7 @@ import com.getyourguide.openapi.validation.api.selector.TrafficSelector; import java.nio.charset.StandardCharsets; import lombok.Getter; +import lombok.Setter; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpRequestDecorator; @@ -14,6 +15,9 @@ public class BodyCachingServerHttpRequestDecorator extends ServerHttpRequestDeco private final TrafficSelector trafficSelector; private final RequestMetaData requestMetaData; + @Setter + private Runnable onBodyCachedListener; + @Getter private String cachedBody; @@ -34,6 +38,11 @@ public Flux getBody() { return super.getBody(); } - return super.getBody().doOnNext(dataBuffer -> cachedBody = dataBuffer.toString(StandardCharsets.UTF_8)); + return super.getBody().doOnNext(dataBuffer -> { + cachedBody = dataBuffer.toString(StandardCharsets.UTF_8); + if (onBodyCachedListener != null) { + onBodyCachedListener.run(); + } + }); } } diff --git a/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/decorator/BodyCachingServerHttpResponseDecorator.java b/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/decorator/BodyCachingServerHttpResponseDecorator.java index e52dd3ae..9e48219b 100644 --- a/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/decorator/BodyCachingServerHttpResponseDecorator.java +++ b/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/decorator/BodyCachingServerHttpResponseDecorator.java @@ -5,6 +5,7 @@ import com.getyourguide.openapi.validation.factory.ReactiveMetaDataFactory; import java.nio.charset.StandardCharsets; import lombok.Getter; +import lombok.Setter; import org.reactivestreams.Publisher; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.http.server.reactive.ServerHttpResponse; @@ -18,6 +19,9 @@ public class BodyCachingServerHttpResponseDecorator extends ServerHttpResponseDe private final ReactiveMetaDataFactory metaDataFactory; private final RequestMetaData requestMetaData; + @Setter + private Runnable onBodyCachedListener; + @Getter private String cachedBody; @@ -41,7 +45,12 @@ public Mono writeWith(@NonNull Publisher body) { return super.writeWith(body); } - var buffer = Mono.from(body).doOnNext(dataBuffer -> cachedBody = dataBuffer.toString(StandardCharsets.UTF_8)); + var buffer = Mono.from(body).doOnNext(dataBuffer -> { + cachedBody = dataBuffer.toString(StandardCharsets.UTF_8); + if (onBodyCachedListener != null) { + onBodyCachedListener.run(); + } + }); return super.writeWith(buffer); } } diff --git a/spring-boot-starter/spring-boot-starter-webflux/build.gradle b/spring-boot-starter/spring-boot-starter-webflux/build.gradle index 9e75c735..1329c345 100644 --- a/spring-boot-starter/spring-boot-starter-webflux/build.gradle +++ b/spring-boot-starter/spring-boot-starter-webflux/build.gradle @@ -22,5 +22,6 @@ dependencies { testImplementation 'org.springframework.boot:spring-boot-starter-test' testImplementation 'org.springframework:spring-webflux' + testImplementation 'io.projectreactor:reactor-test' testImplementation 'org.apache.tomcat.embed:tomcat-embed-core' // For jakarta.servlet.ServletContext } diff --git a/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java b/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java index f0972792..5480a6fc 100644 --- a/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java +++ b/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java @@ -1,6 +1,7 @@ package com.getyourguide.openapi.validation.filter; import com.getyourguide.openapi.validation.api.model.RequestMetaData; +import com.getyourguide.openapi.validation.api.model.ValidationResult; import com.getyourguide.openapi.validation.api.selector.TrafficSelector; import com.getyourguide.openapi.validation.core.OpenApiRequestValidator; import com.getyourguide.openapi.validation.factory.ReactiveMetaDataFactory; @@ -8,10 +9,13 @@ import com.getyourguide.openapi.validation.filter.decorator.BodyCachingServerHttpResponseDecorator; import com.getyourguide.openapi.validation.filter.decorator.DecoratorBuilder; import lombok.AllArgsConstructor; +import lombok.extern.slf4j.Slf4j; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; +import org.springframework.http.HttpStatusCode; import org.springframework.lang.NonNull; import org.springframework.stereotype.Component; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebFilter; import org.springframework.web.server.WebFilterChain; @@ -21,6 +25,7 @@ @Component @Order(Ordered.HIGHEST_PRECEDENCE) @AllArgsConstructor +@Slf4j public class OpenApiValidationWebFilter implements WebFilter { private final OpenApiRequestValidator validator; @@ -48,31 +53,101 @@ private Mono decorateWithValidation(ServerWebExchange exchange, WebFilterC var serverWebExchange = exchange.mutate().request(requestDecorated).response(responseDecorated).build(); + var alreadyDidRequestValidation = validateRequestWithFailOnViolation(requestMetaData, requestDecorated); + var alreadyDidResponseValidation = validateResponseWithFailOnViolation(requestMetaData, responseDecorated); + return chain.filter(serverWebExchange) .doFinally(signalType -> { // Note: Errors are not handled here. They could be handled with SignalType.ON_ERROR, but then the response body can't be accessed. // Reason seems to be that those don't use the decorated response that is set here, but use the previous response object. if (signalType == SignalType.ON_COMPLETE) { - validateRequest(requestDecorated, requestMetaData); - validateResponse(responseDecorated, requestMetaData); + if (!alreadyDidRequestValidation) { + validateRequest(requestDecorated, requestMetaData, RunType.ASYNC); + } + if (!alreadyDidResponseValidation) { + validateResponse(responseDecorated, requestMetaData, RunType.ASYNC); + } } }); } - private void validateRequest(BodyCachingServerHttpRequestDecorator request, RequestMetaData requestMetaData) { + /** @return true if validation is done as part of this method */ + private boolean validateRequestWithFailOnViolation( + RequestMetaData requestMetaData, + BodyCachingServerHttpRequestDecorator requestDecorated + ) { + if (!trafficSelector.shouldFailOnRequestViolation(requestMetaData)) { + return false; + } + + if (requestDecorated.getHeaders().containsKey("Content-Type") && requestDecorated.getHeaders().containsKey("Content-Length")) { + requestDecorated.setOnBodyCachedListener(() -> { + var validateRequestResult = validateRequest(requestDecorated, requestMetaData, RunType.SYNC); + throwStatusExceptionOnViolation(validateRequestResult, "Request validation failed"); + }); + } else { + var validateRequestResult = validateRequest(requestDecorated, requestMetaData, RunType.SYNC); + throwStatusExceptionOnViolation(validateRequestResult, "Request validation failed"); + } + return true; + } + + private static void throwStatusExceptionOnViolation(ValidationResult validateRequestResult, String message) { + if (validateRequestResult == ValidationResult.INVALID) { + throw new ResponseStatusException(HttpStatusCode.valueOf(400), message); + } + } + + /** @return true if validation is done as part of this method */ + private boolean validateResponseWithFailOnViolation( + RequestMetaData requestMetaData, + BodyCachingServerHttpResponseDecorator responseDecorated + ) { + if (!trafficSelector.shouldFailOnResponseViolation(requestMetaData)) { + return false; + } + + responseDecorated.setOnBodyCachedListener(() -> { + var validateResponseResult = validateResponse(responseDecorated, requestMetaData, RunType.SYNC); + throwStatusExceptionOnViolation(validateResponseResult, "Response validation failed"); + }); + return true; + } + + private ValidationResult validateRequest( + BodyCachingServerHttpRequestDecorator request, + RequestMetaData requestMetaData, + RunType runType + ) { if (!trafficSelector.canRequestBeValidated(requestMetaData)) { - return; + return ValidationResult.NOT_APPLICABLE; } - validator.validateRequestObjectAsync(requestMetaData, request.getCachedBody()); + if (runType == RunType.SYNC) { + return validator.validateRequestObject(requestMetaData, request.getCachedBody()); + } else { + validator.validateRequestObjectAsync(requestMetaData, request.getCachedBody()); + return ValidationResult.NOT_APPLICABLE; + } } - private void validateResponse(BodyCachingServerHttpResponseDecorator response, RequestMetaData requestMetaData) { + private ValidationResult validateResponse( + BodyCachingServerHttpResponseDecorator response, + RequestMetaData requestMetaData, + RunType runType + ) { var responseMetaData = metaDataFactory.buildResponseMetaData(response); if (!trafficSelector.canResponseBeValidated(requestMetaData, responseMetaData)) { - return; + return ValidationResult.NOT_APPLICABLE; } - validator.validateResponseObjectAsync(requestMetaData, responseMetaData, response.getCachedBody()); + if (runType == RunType.SYNC) { + return validator.validateResponseObject(requestMetaData, responseMetaData, response.getCachedBody()); + } else { + validator.validateResponseObjectAsync(requestMetaData, responseMetaData, response.getCachedBody()); + return ValidationResult.NOT_APPLICABLE; + } } + + private enum RunType {SYNC, ASYNC} } diff --git a/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/decorator/BodyCachingServerHttpRequestDecorator.java b/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/decorator/BodyCachingServerHttpRequestDecorator.java index 439fd36d..3b25ea01 100644 --- a/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/decorator/BodyCachingServerHttpRequestDecorator.java +++ b/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/decorator/BodyCachingServerHttpRequestDecorator.java @@ -4,6 +4,7 @@ import com.getyourguide.openapi.validation.api.selector.TrafficSelector; import java.nio.charset.StandardCharsets; import lombok.Getter; +import lombok.Setter; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpRequestDecorator; @@ -14,6 +15,9 @@ public class BodyCachingServerHttpRequestDecorator extends ServerHttpRequestDeco private final TrafficSelector trafficSelector; private final RequestMetaData requestMetaData; + @Setter + private Runnable onBodyCachedListener; + @Getter private String cachedBody; @@ -34,6 +38,11 @@ public Flux getBody() { return super.getBody(); } - return super.getBody().doOnNext(dataBuffer -> cachedBody = dataBuffer.toString(StandardCharsets.UTF_8)); + return super.getBody().doOnNext(dataBuffer -> { + cachedBody = dataBuffer.toString(StandardCharsets.UTF_8); + if (onBodyCachedListener != null) { + onBodyCachedListener.run(); + } + }); } } diff --git a/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/decorator/BodyCachingServerHttpResponseDecorator.java b/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/decorator/BodyCachingServerHttpResponseDecorator.java index e52dd3ae..9e48219b 100644 --- a/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/decorator/BodyCachingServerHttpResponseDecorator.java +++ b/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/decorator/BodyCachingServerHttpResponseDecorator.java @@ -5,6 +5,7 @@ import com.getyourguide.openapi.validation.factory.ReactiveMetaDataFactory; import java.nio.charset.StandardCharsets; import lombok.Getter; +import lombok.Setter; import org.reactivestreams.Publisher; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.http.server.reactive.ServerHttpResponse; @@ -18,6 +19,9 @@ public class BodyCachingServerHttpResponseDecorator extends ServerHttpResponseDe private final ReactiveMetaDataFactory metaDataFactory; private final RequestMetaData requestMetaData; + @Setter + private Runnable onBodyCachedListener; + @Getter private String cachedBody; @@ -41,7 +45,12 @@ public Mono writeWith(@NonNull Publisher body) { return super.writeWith(body); } - var buffer = Mono.from(body).doOnNext(dataBuffer -> cachedBody = dataBuffer.toString(StandardCharsets.UTF_8)); + var buffer = Mono.from(body).doOnNext(dataBuffer -> { + cachedBody = dataBuffer.toString(StandardCharsets.UTF_8); + if (onBodyCachedListener != null) { + onBodyCachedListener.run(); + } + }); return super.writeWith(buffer); } } diff --git a/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilterTest.java b/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilterTest.java new file mode 100644 index 00000000..a1135be9 --- /dev/null +++ b/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilterTest.java @@ -0,0 +1,340 @@ +package com.getyourguide.openapi.validation.filter; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.getyourguide.openapi.validation.api.model.RequestMetaData; +import com.getyourguide.openapi.validation.api.model.ResponseMetaData; +import com.getyourguide.openapi.validation.api.model.ValidationResult; +import com.getyourguide.openapi.validation.api.selector.TrafficSelector; +import com.getyourguide.openapi.validation.core.OpenApiRequestValidator; +import com.getyourguide.openapi.validation.factory.ReactiveMetaDataFactory; +import com.getyourguide.openapi.validation.filter.decorator.BodyCachingServerHttpRequestDecorator; +import com.getyourguide.openapi.validation.filter.decorator.BodyCachingServerHttpResponseDecorator; +import com.getyourguide.openapi.validation.filter.decorator.DecoratorBuilder; +import java.util.List; +import lombok.Builder; +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpHeaders; +import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.http.server.reactive.ServerHttpResponse; +import org.springframework.web.server.ResponseStatusException; +import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.server.WebFilterChain; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +class OpenApiValidationWebFilterTest { + + public static final String REQUEST_BODY = ""; + public static final String RESPONSE_BODY = ""; + + private final OpenApiRequestValidator validator = mock(); + private final TrafficSelector trafficSelector = mock(); + private final ReactiveMetaDataFactory metaDataFactory = mock(); + private final DecoratorBuilder decoratorBuilder = mock(); + + private final OpenApiValidationWebFilter webFilter = + new OpenApiValidationWebFilter(validator, trafficSelector, metaDataFactory, decoratorBuilder); + + @Test + public void testNormalFlowWithValidation() { + var mockData = mockSetup(MockConfiguration.builder().build()); + + var mono = webFilter.filter(mockData.exchange, mockData.chain); + + StepVerifier.create(mono).expectComplete().verify(); + verifyChainCalled(mockData.chain, mockData.mutatedExchange); + verifyRequestValidatedAsync(mockData); + verifyResponseValidatedAsync(mockData); + } + + @Test + public void testNoValidationIfNotReady() { + var mockData = mockSetup(MockConfiguration.builder().isReady(false).build()); + + var mono = webFilter.filter(mockData.exchange, mockData.chain); + + StepVerifier.create(mono).expectComplete().verify(); + verifyChainCalled(mockData.chain, mockData.exchange); + verifyNoValidation(); + } + + @Test + public void testNoValidationIfNotShouldRequestBeValidated() { + var mockData = mockSetup(MockConfiguration.builder().shouldRequestBeValidated(false).build()); + + var mono = webFilter.filter(mockData.exchange, mockData.chain); + + StepVerifier.create(mono).expectComplete().verify(); + verifyChainCalled(mockData.chain, mockData.exchange); + verifyNoValidation(); + } + + @Test + public void testNoValidationIfNotCanRequestBeValidated() { + var mockData = mockSetup(MockConfiguration.builder().canRequestBeValidated(false).build()); + + var mono = webFilter.filter(mockData.exchange, mockData.chain); + + StepVerifier.create(mono).expectComplete().verify(); + verifyChainCalled(mockData.chain, mockData.mutatedExchange); + verifyNoRequestValidation(); + verifyResponseValidatedAsync(mockData); + } + + @Test + public void testNoValidationIfNotCanResponseBeValidated() { + var mockData = mockSetup(MockConfiguration.builder().canResponseBeValidated(false).build()); + + var mono = webFilter.filter(mockData.exchange, mockData.chain); + + StepVerifier.create(mono).expectComplete().verify(); + verifyChainCalled(mockData.chain, mockData.mutatedExchange); + verifyRequestValidatedAsync(mockData); + verifyNoResponseValidation(); + } + + @Test + public void testShouldFailOnRequestViolationWithoutViolation() { + var mockData = mockSetup(MockConfiguration.builder().shouldFailOnRequestViolation(true).build()); + + var mono = webFilter.filter(mockData.exchange, mockData.chain); + + StepVerifier.create(mono).expectComplete().verify(); + verifyChainCalled(mockData.chain, mockData.mutatedExchange); + verifyRequestValidatedSync(mockData); + verifyResponseValidatedAsync(mockData); + } + + @Test + public void testShouldFailOnReResponseViolationWithoutViolation() { + var mockData = mockSetup(MockConfiguration.builder().shouldFailOnResponseViolation(true).build()); + + var mono = webFilter.filter(mockData.exchange, mockData.chain); + + StepVerifier.create(mono).expectComplete().verify(); + verifyChainCalled(mockData.chain, mockData.mutatedExchange); + verifyRequestValidatedAsync(mockData); + verifyResponseValidatedSync(mockData); + } + + @Test + public void testShouldFailOnRequestViolationWithViolation() { + var mockData = mockSetup(MockConfiguration.builder().shouldFailOnRequestViolation(true).build()); + when(validator.validateRequestObject(eq(mockData.requestMetaData), eq(REQUEST_BODY))) + .thenReturn(ValidationResult.INVALID); + + assertThrows(ResponseStatusException.class, () -> webFilter.filter(mockData.exchange, mockData.chain)); + + verifyChainNotCalled(mockData.chain); + verifyRequestValidatedSync(mockData); + verifyNoResponseValidation(); + } + + @Test + public void testShouldFailOnReResponseViolationWithViolation() { + var mockData = mockSetup(MockConfiguration.builder().shouldFailOnResponseViolation(true).build()); + when( + validator.validateResponseObject( + eq(mockData.requestMetaData), + eq(mockData.responseMetaData), eq(REQUEST_BODY) + ) + ).thenReturn(ValidationResult.INVALID); + + assertThrows(ResponseStatusException.class, () -> webFilter.filter(mockData.exchange, mockData.chain)); + + verifyChainNotCalled(mockData.chain); + verifyNoRequestValidation(); + verifyResponseValidatedSync(mockData); + } + + private void verifyNoValidation() { + verifyNoRequestValidation(); + verifyNoResponseValidation(); + } + + private void verifyNoRequestValidation() { + verify(validator, never()).validateRequestObjectAsync(any(), anyString()); + verify(validator, never()).validateRequestObject(any(), anyString()); + } + + private void verifyNoResponseValidation() { + verify(validator, never()).validateResponseObjectAsync(any(), any(), anyString()); + verify(validator, never()).validateResponseObject(any(), any(), anyString()); + } + + private void verifyRequestValidatedAsync(MockSetupData mockData) { + verify(validator).validateRequestObjectAsync(eq(mockData.requestMetaData), eq(REQUEST_BODY)); + } + + + private void verifyRequestValidatedSync(MockSetupData mockData) { + verify(validator).validateRequestObject(eq(mockData.requestMetaData), eq(REQUEST_BODY)); + } + + private void verifyResponseValidatedAsync(MockSetupData mockData) { + verify(validator).validateResponseObjectAsync( + eq(mockData.requestMetaData), + eq(mockData.responseMetaData), + eq(RESPONSE_BODY) + ); + } + + private void verifyResponseValidatedSync(MockSetupData mockData) { + verify(validator) + .validateResponseObject(eq(mockData.requestMetaData), eq(mockData.responseMetaData), eq(RESPONSE_BODY)); + } + + private void mockTrafficSelectorMethods( + RequestMetaData requestMetaData, + ResponseMetaData responseMetaData, + MockConfiguration configuration + ) { + when(trafficSelector.shouldRequestBeValidated(any())).thenReturn(configuration.shouldRequestBeValidated); + when(trafficSelector.canRequestBeValidated(requestMetaData)).thenReturn(configuration.canRequestBeValidated); + when(trafficSelector.canResponseBeValidated(requestMetaData, responseMetaData)).thenReturn( + configuration.canResponseBeValidated); + when(trafficSelector.shouldFailOnRequestViolation(requestMetaData)).thenReturn( + configuration.shouldFailOnRequestViolation); + when(trafficSelector.shouldFailOnResponseViolation(requestMetaData)).thenReturn( + configuration.shouldFailOnResponseViolation); + } + + private static WebFilterChain mockChain() { + var chain = mock(WebFilterChain.class); + Mono filterMono = Mono.just("").then(); + when(chain.filter(any())).thenReturn(filterMono); + return chain; + } + + private void mockDecoratedRequests( + ServerHttpRequest request, + ServerHttpResponse response, + RequestMetaData requestMetaData, + ResponseMetaData responseMetaData, + MockConfiguration configuration + ) { + var decoratedRequest = mock(BodyCachingServerHttpRequestDecorator.class); + when(decoratorBuilder.buildBodyCachingServerHttpRequestDecorator(request, requestMetaData)) + .thenReturn(decoratedRequest); + when(decoratedRequest.getHeaders()).thenReturn(buildHeadersForBody(configuration.requestBody)); + when(decoratedRequest.getCachedBody()).thenReturn(configuration.requestBody); + if (configuration.requestBody != null) { + doAnswer(invocation -> { + invocation.getArgument(0, Runnable.class).run(); + return null; + }).when(decoratedRequest).setOnBodyCachedListener(any()); + } + + var decoratedResponse = mock(BodyCachingServerHttpResponseDecorator.class); + when(decoratorBuilder.buildBodyCachingServerHttpResponseDecorator(response, requestMetaData)) + .thenReturn(decoratedResponse); + when(decoratedResponse.getHeaders()).thenReturn(buildHeadersForBody(configuration.responseBody)); + when(decoratedResponse.getCachedBody()).thenReturn(configuration.responseBody); + if (configuration.responseBody != null) { + doAnswer(invocation -> { + invocation.getArgument(0, Runnable.class).run(); + return null; + }).when(decoratedResponse).setOnBodyCachedListener(any()); + } + + when(metaDataFactory.buildResponseMetaData(decoratedResponse)).thenReturn(responseMetaData); + } + + private static HttpHeaders buildHeadersForBody(String body) { + var headers = new HttpHeaders(); + if (body != null) { + headers.put(HttpHeaders.CONTENT_TYPE, List.of("application/json")); + headers.put(HttpHeaders.CONTENT_LENGTH, List.of(String.valueOf(body.length()))); + } + return headers; + } + + private static ServerWebExchange mockExchangeMutation(ServerWebExchange exchange) { + var mutatedExchange = mock(ServerWebExchange.class); + var exchangeBuilder = mock(ServerWebExchange.Builder.class); + when(exchange.mutate()).thenReturn(exchangeBuilder); + when(exchangeBuilder.request(any(ServerHttpRequest.class))).thenReturn(exchangeBuilder); + when(exchangeBuilder.response(any(ServerHttpResponse.class))).thenReturn(exchangeBuilder); + when(exchangeBuilder.build()).thenReturn(mutatedExchange); + return mutatedExchange; + } + + private static void verifyChainCalled(WebFilterChain chain, ServerWebExchange mutatedExchange) { + verify(chain).filter(mutatedExchange); + } + + private static void verifyChainNotCalled(WebFilterChain chain) { + verify(chain, never()).filter(any()); + } + + private MockSetupData mockSetup(MockConfiguration configuration) { + var exchange = mock(ServerWebExchange.class); + var request = mock(ServerHttpRequest.class); + when(exchange.getRequest()).thenReturn(request); + var response = mock(ServerHttpResponse.class); + when(exchange.getResponse()).thenReturn(response); + + var requestMetaData = mock(RequestMetaData.class); + when(metaDataFactory.buildRequestMetaData(request)).thenReturn(requestMetaData); + + var responseMetaData = mock(ResponseMetaData.class); + when(metaDataFactory.buildResponseMetaData(response)).thenReturn(responseMetaData); + + mockDecoratedRequests(request, response, requestMetaData, responseMetaData, configuration); + var mutatedExchange = mockExchangeMutation(exchange); + + var chain = mockChain(); + when(validator.isReady()).thenReturn(configuration.isReady); + mockTrafficSelectorMethods(requestMetaData, responseMetaData, configuration); + + return MockSetupData.builder() + .exchange(exchange) + .chain(chain) + .mutatedExchange(mutatedExchange) + .requestMetaData(requestMetaData) + .responseMetaData(responseMetaData) + .build(); + } + + @Builder + private static class MockConfiguration { + @Builder.Default + private boolean isReady = true; + @Builder.Default + private boolean shouldRequestBeValidated = true; + + @Builder.Default + private boolean canRequestBeValidated = true; + @Builder.Default + private boolean canResponseBeValidated = true; + + @Builder.Default + private boolean shouldFailOnRequestViolation = false; + @Builder.Default + private boolean shouldFailOnResponseViolation = false; + + @Builder.Default + private String requestBody = REQUEST_BODY; + @Builder.Default + private String responseBody = RESPONSE_BODY; + } + + @Builder + private record MockSetupData( + ServerWebExchange exchange, + WebFilterChain chain, + ServerWebExchange mutatedExchange, + RequestMetaData requestMetaData, + ResponseMetaData responseMetaData + ) { + } +} From 271da4e43ca6259a6c255170f7b3304dd6ae47ad Mon Sep 17 00:00:00 2001 From: Patrick Boos Date: Tue, 23 May 2023 16:25:30 +0200 Subject: [PATCH 2/7] Update javadoc --- .../filter/OpenApiValidationWebFilter.java | 12 +++++++++--- .../filter/OpenApiValidationWebFilter.java | 12 +++++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java b/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java index d2e5f831..353ea340 100644 --- a/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java +++ b/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java @@ -71,7 +71,10 @@ private Mono decorateWithValidation(ServerWebExchange exchange, WebFilterC }); } - /** @return true if validation is done as part of this method */ + /** + * Validate request and fail on violation if configured to do so. + * @return true if validation is done as part of this method + */ private boolean validateRequestWithFailOnViolation( RequestMetaData requestMetaData, BodyCachingServerHttpRequestDecorator requestDecorated @@ -98,7 +101,10 @@ private static void throwStatusExceptionOnViolation(ValidationResult validateReq } } - /** @return true if validation is done as part of this method */ + /** + * Validate response and fail on violation if configured to do so. + * @return true if validation is done as part of this method + */ private boolean validateResponseWithFailOnViolation( RequestMetaData requestMetaData, BodyCachingServerHttpResponseDecorator responseDecorated @@ -149,5 +155,5 @@ private ValidationResult validateResponse( } } - private enum RunType {SYNC, ASYNC} + private enum RunType { SYNC, ASYNC } } diff --git a/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java b/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java index 5480a6fc..3fd87ca6 100644 --- a/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java +++ b/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java @@ -71,7 +71,10 @@ private Mono decorateWithValidation(ServerWebExchange exchange, WebFilterC }); } - /** @return true if validation is done as part of this method */ + /** + * Validate request and fail on violation if configured to do so. + * @return true if validation is done as part of this method + */ private boolean validateRequestWithFailOnViolation( RequestMetaData requestMetaData, BodyCachingServerHttpRequestDecorator requestDecorated @@ -98,7 +101,10 @@ private static void throwStatusExceptionOnViolation(ValidationResult validateReq } } - /** @return true if validation is done as part of this method */ + /** + * Validate response and fail on violation if configured to do so. + * @return true if validation is done as part of this method + */ private boolean validateResponseWithFailOnViolation( RequestMetaData requestMetaData, BodyCachingServerHttpResponseDecorator responseDecorated @@ -149,5 +155,5 @@ private ValidationResult validateResponse( } } - private enum RunType {SYNC, ASYNC} + private enum RunType { SYNC, ASYNC } } From c6525aa351ace227d145342961fb64be918823c3 Mon Sep 17 00:00:00 2001 From: Patrick Boos Date: Tue, 23 May 2023 16:28:27 +0200 Subject: [PATCH 3/7] Remove unnecessary empty line --- .../validation/filter/OpenApiValidationWebFilterTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilterTest.java b/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilterTest.java index a1135be9..c0b2be3b 100644 --- a/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilterTest.java +++ b/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilterTest.java @@ -175,7 +175,6 @@ private void verifyRequestValidatedAsync(MockSetupData mockData) { verify(validator).validateRequestObjectAsync(eq(mockData.requestMetaData), eq(REQUEST_BODY)); } - private void verifyRequestValidatedSync(MockSetupData mockData) { verify(validator).validateRequestObject(eq(mockData.requestMetaData), eq(REQUEST_BODY)); } From 362373dab108a94e156aa6a471b05fc86e3ddc5f Mon Sep 17 00:00:00 2001 From: Patrick Boos Date: Tue, 23 May 2023 16:54:20 +0200 Subject: [PATCH 4/7] Support fail on request/response violation in web --- .../SpringWebLibraryAutoConfiguration.java | 12 +- .../factory/ContentCachingWrapperFactory.java | 16 + .../filter/OpenApiValidationHttpFilter.java | 85 ++++- .../SpringWebLibraryAutoConfiguration.java | 17 +- .../factory/ContentCachingWrapperFactory.java | 16 + .../filter/OpenApiValidationHttpFilter.java | 85 ++++- .../OpenApiValidationHttpFilterTest.java | 304 ++++++++++++++++++ .../OpenApiValidationWebFilterTest.java | 2 +- 8 files changed, 510 insertions(+), 27 deletions(-) create mode 100644 spring-boot-starter/spring-boot-starter-web-spring2.7/src/main/java/com/getyourguide/openapi/validation/factory/ContentCachingWrapperFactory.java create mode 100644 spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/factory/ContentCachingWrapperFactory.java create mode 100644 spring-boot-starter/spring-boot-starter-web/src/test/java/com/getyourguide/openapi/validation/filter/OpenApiValidationHttpFilterTest.java diff --git a/spring-boot-starter/spring-boot-starter-web-spring2.7/src/main/java/com/getyourguide/openapi/validation/autoconfigure/SpringWebLibraryAutoConfiguration.java b/spring-boot-starter/spring-boot-starter-web-spring2.7/src/main/java/com/getyourguide/openapi/validation/autoconfigure/SpringWebLibraryAutoConfiguration.java index ceff0326..8361d9d6 100644 --- a/spring-boot-starter/spring-boot-starter-web-spring2.7/src/main/java/com/getyourguide/openapi/validation/autoconfigure/SpringWebLibraryAutoConfiguration.java +++ b/spring-boot-starter/spring-boot-starter-web-spring2.7/src/main/java/com/getyourguide/openapi/validation/autoconfigure/SpringWebLibraryAutoConfiguration.java @@ -4,6 +4,7 @@ import com.getyourguide.openapi.validation.api.selector.TrafficSelector; import com.getyourguide.openapi.validation.core.OpenApiRequestValidator; +import com.getyourguide.openapi.validation.factory.ContentCachingWrapperFactory; import com.getyourguide.openapi.validation.factory.ServletMetaDataFactory; import com.getyourguide.openapi.validation.filter.OpenApiValidationHttpFilter; import lombok.AllArgsConstructor; @@ -21,13 +22,20 @@ public ServletMetaDataFactory servletMetaDataFactory() { return new ServletMetaDataFactory(); } + @Bean + @ConditionalOnWebApplication(type = Type.SERVLET) + public ContentCachingWrapperFactory contentCachingWrapperFactory() { + return new ContentCachingWrapperFactory(); + } + @Bean @ConditionalOnWebApplication(type = Type.SERVLET) public OpenApiValidationHttpFilter openApiValidationHttpFilter( OpenApiRequestValidator validator, TrafficSelector trafficSelector, - ServletMetaDataFactory metaDataFactory + ServletMetaDataFactory metaDataFactory, + ContentCachingWrapperFactory contentCachingWrapperFactory ) { - return new OpenApiValidationHttpFilter(validator, trafficSelector, metaDataFactory); + return new OpenApiValidationHttpFilter(validator, trafficSelector, metaDataFactory, contentCachingWrapperFactory); } } diff --git a/spring-boot-starter/spring-boot-starter-web-spring2.7/src/main/java/com/getyourguide/openapi/validation/factory/ContentCachingWrapperFactory.java b/spring-boot-starter/spring-boot-starter-web-spring2.7/src/main/java/com/getyourguide/openapi/validation/factory/ContentCachingWrapperFactory.java new file mode 100644 index 00000000..6474efd7 --- /dev/null +++ b/spring-boot-starter/spring-boot-starter-web-spring2.7/src/main/java/com/getyourguide/openapi/validation/factory/ContentCachingWrapperFactory.java @@ -0,0 +1,16 @@ +package com.getyourguide.openapi.validation.factory; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.springframework.web.util.ContentCachingRequestWrapper; +import org.springframework.web.util.ContentCachingResponseWrapper; + +public class ContentCachingWrapperFactory { + public ContentCachingRequestWrapper buildContentCachingRequestWrapper(HttpServletRequest request) { + return new ContentCachingRequestWrapper(request); + } + + public ContentCachingResponseWrapper buildContentCachingResponseWrapper(HttpServletResponse response) { + return new ContentCachingResponseWrapper(response); + } +} diff --git a/spring-boot-starter/spring-boot-starter-web-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationHttpFilter.java b/spring-boot-starter/spring-boot-starter-web-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationHttpFilter.java index e553acdd..31352c41 100644 --- a/spring-boot-starter/spring-boot-starter-web-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationHttpFilter.java +++ b/spring-boot-starter/spring-boot-starter-web-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationHttpFilter.java @@ -1,8 +1,10 @@ package com.getyourguide.openapi.validation.filter; import com.getyourguide.openapi.validation.api.model.RequestMetaData; +import com.getyourguide.openapi.validation.api.model.ValidationResult; import com.getyourguide.openapi.validation.api.selector.TrafficSelector; import com.getyourguide.openapi.validation.core.OpenApiRequestValidator; +import com.getyourguide.openapi.validation.factory.ContentCachingWrapperFactory; import com.getyourguide.openapi.validation.factory.ServletMetaDataFactory; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -17,7 +19,9 @@ import lombok.extern.slf4j.Slf4j; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; +import org.springframework.http.HttpStatus; import org.springframework.stereotype.Component; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.util.ContentCachingRequestWrapper; import org.springframework.web.util.ContentCachingResponseWrapper; @@ -30,9 +34,11 @@ public class OpenApiValidationHttpFilter extends HttpFilter { private final OpenApiRequestValidator validator; private final TrafficSelector trafficSelector; private final ServletMetaDataFactory metaDataFactory; + private final ContentCachingWrapperFactory contentCachingWrapperFactory; @Override - public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) + throws IOException, ServletException { if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) { super.doFilter(request, response, chain); return; @@ -46,37 +52,94 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha return; } - var requestWrapper = new ContentCachingRequestWrapper(httpServletRequest); - var responseWrapper = new ContentCachingResponseWrapper(httpServletResponse); + var requestWrapper = contentCachingWrapperFactory.buildContentCachingRequestWrapper(httpServletRequest); + var responseWrapper = contentCachingWrapperFactory.buildContentCachingResponseWrapper(httpServletResponse); + + var alreadyDidRequestValidation = validateRequestWithFailOnViolation(requestWrapper, requestMetaData); try { super.doFilter(requestWrapper, responseWrapper, chain); } finally { - validateRequest(requestWrapper, requestMetaData); - validateResponse(responseWrapper, requestMetaData); + if (!alreadyDidRequestValidation) { + validateRequest(requestWrapper, requestMetaData, RunType.ASYNC); + } + + var validateResponseResult = + validateResponse(responseWrapper, requestMetaData, getRunTypeForResponseValidation(requestMetaData)); + throwStatusExceptionOnViolation(validateResponseResult, "Response validation failed"); + responseWrapper.copyBodyToResponse(); // Needs to be done on every call, otherwise there won't be a response body } } - private void validateRequest(ContentCachingRequestWrapper request, RequestMetaData requestMetaData) { + private RunType getRunTypeForResponseValidation(RequestMetaData requestMetaData) { + if (trafficSelector.shouldFailOnResponseViolation(requestMetaData)) { + return RunType.SYNC; + } else { + return RunType.ASYNC; + } + } + + private boolean validateRequestWithFailOnViolation( + ContentCachingRequestWrapper request, + RequestMetaData requestMetaData + ) { + if (!trafficSelector.shouldFailOnRequestViolation(requestMetaData)) { + return false; + } + + var validateRequestResult = validateRequest(request, requestMetaData, RunType.SYNC); + throwStatusExceptionOnViolation(validateRequestResult, "Request validation failed"); + return true; + } + + private ValidationResult validateRequest( + ContentCachingRequestWrapper request, + RequestMetaData requestMetaData, + RunType runType + ) { if (!trafficSelector.canRequestBeValidated(requestMetaData)) { - return; + return ValidationResult.NOT_APPLICABLE; } var requestBody = request.getContentType() != null ? new String(request.getContentAsByteArray(), StandardCharsets.UTF_8) : null; - validator.validateRequestObjectAsync(requestMetaData, requestBody); + + if (runType == RunType.ASYNC) { + validator.validateRequestObjectAsync(requestMetaData, requestBody); + return ValidationResult.NOT_APPLICABLE; + } else { + return validator.validateRequestObject(requestMetaData, requestBody); + } } - private void validateResponse(ContentCachingResponseWrapper response, RequestMetaData requestMetaData) { + private ValidationResult validateResponse( + ContentCachingResponseWrapper response, + RequestMetaData requestMetaData, + RunType runType + ) { var responseMetaData = metaDataFactory.buildResponseMetaData(response); if (!trafficSelector.canResponseBeValidated(requestMetaData, responseMetaData)) { - return; + return ValidationResult.NOT_APPLICABLE; } var responseBody = response.getContentType() != null ? new String(response.getContentAsByteArray(), StandardCharsets.UTF_8) : null; - validator.validateResponseObjectAsync(requestMetaData, responseMetaData, responseBody); + + if (runType == RunType.ASYNC) { + validator.validateResponseObjectAsync(requestMetaData, responseMetaData, responseBody); + return ValidationResult.NOT_APPLICABLE; + } else { + return validator.validateResponseObject(requestMetaData, responseMetaData, responseBody); + } } + + private void throwStatusExceptionOnViolation(ValidationResult validateRequestResult, String message) { + if (validateRequestResult == ValidationResult.INVALID) { + throw new ResponseStatusException(HttpStatus.valueOf(400), message); + } + } + + private enum RunType { SYNC, ASYNC } } diff --git a/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/autoconfigure/SpringWebLibraryAutoConfiguration.java b/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/autoconfigure/SpringWebLibraryAutoConfiguration.java index ceff0326..697ab078 100644 --- a/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/autoconfigure/SpringWebLibraryAutoConfiguration.java +++ b/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/autoconfigure/SpringWebLibraryAutoConfiguration.java @@ -4,6 +4,7 @@ import com.getyourguide.openapi.validation.api.selector.TrafficSelector; import com.getyourguide.openapi.validation.core.OpenApiRequestValidator; +import com.getyourguide.openapi.validation.factory.ContentCachingWrapperFactory; import com.getyourguide.openapi.validation.factory.ServletMetaDataFactory; import com.getyourguide.openapi.validation.filter.OpenApiValidationHttpFilter; import lombok.AllArgsConstructor; @@ -21,13 +22,25 @@ public ServletMetaDataFactory servletMetaDataFactory() { return new ServletMetaDataFactory(); } + @Bean + @ConditionalOnWebApplication(type = Type.SERVLET) + public ContentCachingWrapperFactory contentCachingWrapperFactory() { + return new ContentCachingWrapperFactory(); + } + @Bean @ConditionalOnWebApplication(type = Type.SERVLET) public OpenApiValidationHttpFilter openApiValidationHttpFilter( OpenApiRequestValidator validator, TrafficSelector trafficSelector, - ServletMetaDataFactory metaDataFactory + ServletMetaDataFactory metaDataFactory, + ContentCachingWrapperFactory contentCachingWrapperFactory ) { - return new OpenApiValidationHttpFilter(validator, trafficSelector, metaDataFactory); + return new OpenApiValidationHttpFilter( + validator, + trafficSelector, + metaDataFactory, + contentCachingWrapperFactory + ); } } diff --git a/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/factory/ContentCachingWrapperFactory.java b/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/factory/ContentCachingWrapperFactory.java new file mode 100644 index 00000000..decd6d1f --- /dev/null +++ b/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/factory/ContentCachingWrapperFactory.java @@ -0,0 +1,16 @@ +package com.getyourguide.openapi.validation.factory; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.springframework.web.util.ContentCachingRequestWrapper; +import org.springframework.web.util.ContentCachingResponseWrapper; + +public class ContentCachingWrapperFactory { + public ContentCachingRequestWrapper buildContentCachingRequestWrapper(HttpServletRequest request) { + return new ContentCachingRequestWrapper(request); + } + + public ContentCachingResponseWrapper buildContentCachingResponseWrapper(HttpServletResponse response) { + return new ContentCachingResponseWrapper(response); + } +} diff --git a/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationHttpFilter.java b/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationHttpFilter.java index 34474b7d..656746fc 100644 --- a/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationHttpFilter.java +++ b/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationHttpFilter.java @@ -1,8 +1,10 @@ package com.getyourguide.openapi.validation.filter; import com.getyourguide.openapi.validation.api.model.RequestMetaData; +import com.getyourguide.openapi.validation.api.model.ValidationResult; import com.getyourguide.openapi.validation.api.selector.TrafficSelector; import com.getyourguide.openapi.validation.core.OpenApiRequestValidator; +import com.getyourguide.openapi.validation.factory.ContentCachingWrapperFactory; import com.getyourguide.openapi.validation.factory.ServletMetaDataFactory; import jakarta.servlet.FilterChain; import jakarta.servlet.ServletException; @@ -17,7 +19,9 @@ import lombok.extern.slf4j.Slf4j; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; +import org.springframework.http.HttpStatusCode; import org.springframework.stereotype.Component; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.util.ContentCachingRequestWrapper; import org.springframework.web.util.ContentCachingResponseWrapper; @@ -30,9 +34,11 @@ public class OpenApiValidationHttpFilter extends HttpFilter { private final OpenApiRequestValidator validator; private final TrafficSelector trafficSelector; private final ServletMetaDataFactory metaDataFactory; + private final ContentCachingWrapperFactory contentCachingWrapperFactory; @Override - public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) + throws IOException, ServletException { if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) { super.doFilter(request, response, chain); return; @@ -46,37 +52,94 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha return; } - var requestWrapper = new ContentCachingRequestWrapper(httpServletRequest); - var responseWrapper = new ContentCachingResponseWrapper(httpServletResponse); + var requestWrapper = contentCachingWrapperFactory.buildContentCachingRequestWrapper(httpServletRequest); + var responseWrapper = contentCachingWrapperFactory.buildContentCachingResponseWrapper(httpServletResponse); + + var alreadyDidRequestValidation = validateRequestWithFailOnViolation(requestWrapper, requestMetaData); try { super.doFilter(requestWrapper, responseWrapper, chain); } finally { - validateRequest(requestWrapper, requestMetaData); - validateResponse(responseWrapper, requestMetaData); + if (!alreadyDidRequestValidation) { + validateRequest(requestWrapper, requestMetaData, RunType.ASYNC); + } + + var validateResponseResult = + validateResponse(responseWrapper, requestMetaData, getRunTypeForResponseValidation(requestMetaData)); + throwStatusExceptionOnViolation(validateResponseResult, "Response validation failed"); + responseWrapper.copyBodyToResponse(); // Needs to be done on every call, otherwise there won't be a response body } } - private void validateRequest(ContentCachingRequestWrapper request, RequestMetaData requestMetaData) { + private RunType getRunTypeForResponseValidation(RequestMetaData requestMetaData) { + if (trafficSelector.shouldFailOnResponseViolation(requestMetaData)) { + return RunType.SYNC; + } else { + return RunType.ASYNC; + } + } + + private boolean validateRequestWithFailOnViolation( + ContentCachingRequestWrapper request, + RequestMetaData requestMetaData + ) { + if (!trafficSelector.shouldFailOnRequestViolation(requestMetaData)) { + return false; + } + + var validateRequestResult = validateRequest(request, requestMetaData, RunType.SYNC); + throwStatusExceptionOnViolation(validateRequestResult, "Request validation failed"); + return true; + } + + private ValidationResult validateRequest( + ContentCachingRequestWrapper request, + RequestMetaData requestMetaData, + RunType runType + ) { if (!trafficSelector.canRequestBeValidated(requestMetaData)) { - return; + return ValidationResult.NOT_APPLICABLE; } var requestBody = request.getContentType() != null ? new String(request.getContentAsByteArray(), StandardCharsets.UTF_8) : null; - validator.validateRequestObjectAsync(requestMetaData, requestBody); + + if (runType == RunType.ASYNC) { + validator.validateRequestObjectAsync(requestMetaData, requestBody); + return ValidationResult.NOT_APPLICABLE; + } else { + return validator.validateRequestObject(requestMetaData, requestBody); + } } - private void validateResponse(ContentCachingResponseWrapper response, RequestMetaData requestMetaData) { + private ValidationResult validateResponse( + ContentCachingResponseWrapper response, + RequestMetaData requestMetaData, + RunType runType + ) { var responseMetaData = metaDataFactory.buildResponseMetaData(response); if (!trafficSelector.canResponseBeValidated(requestMetaData, responseMetaData)) { - return; + return ValidationResult.NOT_APPLICABLE; } var responseBody = response.getContentType() != null ? new String(response.getContentAsByteArray(), StandardCharsets.UTF_8) : null; - validator.validateResponseObjectAsync(requestMetaData, responseMetaData, responseBody); + + if (runType == RunType.ASYNC) { + validator.validateResponseObjectAsync(requestMetaData, responseMetaData, responseBody); + return ValidationResult.NOT_APPLICABLE; + } else { + return validator.validateResponseObject(requestMetaData, responseMetaData, responseBody); + } } + + private void throwStatusExceptionOnViolation(ValidationResult validateRequestResult, String message) { + if (validateRequestResult == ValidationResult.INVALID) { + throw new ResponseStatusException(HttpStatusCode.valueOf(400), message); + } + } + + private enum RunType { SYNC, ASYNC } } diff --git a/spring-boot-starter/spring-boot-starter-web/src/test/java/com/getyourguide/openapi/validation/filter/OpenApiValidationHttpFilterTest.java b/spring-boot-starter/spring-boot-starter-web/src/test/java/com/getyourguide/openapi/validation/filter/OpenApiValidationHttpFilterTest.java new file mode 100644 index 00000000..9aed85ef --- /dev/null +++ b/spring-boot-starter/spring-boot-starter-web/src/test/java/com/getyourguide/openapi/validation/filter/OpenApiValidationHttpFilterTest.java @@ -0,0 +1,304 @@ +package com.getyourguide.openapi.validation.filter; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.getyourguide.openapi.validation.api.model.RequestMetaData; +import com.getyourguide.openapi.validation.api.model.ResponseMetaData; +import com.getyourguide.openapi.validation.api.model.ValidationResult; +import com.getyourguide.openapi.validation.api.selector.TrafficSelector; +import com.getyourguide.openapi.validation.core.OpenApiRequestValidator; +import com.getyourguide.openapi.validation.factory.ContentCachingWrapperFactory; +import com.getyourguide.openapi.validation.factory.ServletMetaDataFactory; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import lombok.Builder; +import org.junit.jupiter.api.Test; +import org.springframework.web.server.ResponseStatusException; +import org.springframework.web.util.ContentCachingRequestWrapper; +import org.springframework.web.util.ContentCachingResponseWrapper; + +class OpenApiValidationHttpFilterTest { + + public static final String REQUEST_BODY = ""; + public static final String RESPONSE_BODY = ""; + + private final OpenApiRequestValidator validator = mock(); + private final TrafficSelector trafficSelector = mock(); + private final ServletMetaDataFactory metaDataFactory = mock(); + private final ContentCachingWrapperFactory contentCachingWrapperFactory = mock(); + + private final OpenApiValidationHttpFilter httpFilter = + new OpenApiValidationHttpFilter(validator, trafficSelector, metaDataFactory, contentCachingWrapperFactory); + + @Test + public void testNormalFlowWithValidation() throws ServletException, IOException { + var mockData = mockSetup(MockConfiguration.builder().build()); + + httpFilter.doFilter(mockData.request, mockData.response, mockData.chain); + + verifyChainCalled(mockData.chain, mockData.cachingRequest, mockData.cachingResponse); + verifyRequestValidatedAsync(mockData); + verifyResponseValidatedAsync(mockData); + } + + @Test + public void testNoValidationIfNotReady() throws ServletException, IOException { + var mockData = mockSetup(MockConfiguration.builder().isReady(false).build()); + + httpFilter.doFilter(mockData.request, mockData.response, mockData.chain); + + verifyChainCalled(mockData.chain, mockData.request, mockData.response); + verifyNoValidation(); + } + + @Test + public void testNoValidationIfNotShouldRequestBeValidated() throws ServletException, IOException { + var mockData = mockSetup(MockConfiguration.builder().shouldRequestBeValidated(false).build()); + + httpFilter.doFilter(mockData.request, mockData.response, mockData.chain); + + verifyChainCalled(mockData.chain, mockData.request, mockData.response); + verifyNoValidation(); + } + + @Test + public void testNoValidationIfNotCanRequestBeValidated() throws ServletException, IOException { + var mockData = mockSetup(MockConfiguration.builder().canRequestBeValidated(false).build()); + + httpFilter.doFilter(mockData.request, mockData.response, mockData.chain); + + verifyChainCalled(mockData.chain, mockData.cachingRequest, mockData.cachingResponse); + verifyNoRequestValidation(); + verifyResponseValidatedAsync(mockData); + } + + @Test + public void testNoValidationIfNotCanResponseBeValidated() throws ServletException, IOException { + var mockData = mockSetup(MockConfiguration.builder().canResponseBeValidated(false).build()); + + httpFilter.doFilter(mockData.request, mockData.response, mockData.chain); + + verifyChainCalled(mockData.chain, mockData.cachingRequest, mockData.cachingResponse); + verifyRequestValidatedAsync(mockData); + verifyNoResponseValidation(); + } + + @Test + public void testShouldFailOnRequestViolationWithoutViolation() throws ServletException, IOException { + var mockData = mockSetup(MockConfiguration.builder().shouldFailOnRequestViolation(true).build()); + + httpFilter.doFilter(mockData.request, mockData.response, mockData.chain); + + verifyChainCalled(mockData.chain, mockData.cachingRequest, mockData.cachingResponse); + verifyRequestValidatedSync(mockData); + verifyResponseValidatedAsync(mockData); + } + + @Test + public void testShouldFailOnReResponseViolationWithoutViolation() throws ServletException, IOException { + var mockData = mockSetup(MockConfiguration.builder().shouldFailOnResponseViolation(true).build()); + + httpFilter.doFilter(mockData.request, mockData.response, mockData.chain); + + verifyChainCalled(mockData.chain, mockData.cachingRequest, mockData.cachingResponse); + verifyRequestValidatedAsync(mockData); + verifyResponseValidatedSync(mockData); + } + + @Test + public void testShouldFailOnRequestViolationWithViolation() throws ServletException, IOException { + var mockData = mockSetup(MockConfiguration.builder().shouldFailOnRequestViolation(true).build()); + when(validator.validateRequestObject(eq(mockData.requestMetaData), eq(REQUEST_BODY))) + .thenReturn(ValidationResult.INVALID); + + assertThrows(ResponseStatusException.class, + () -> httpFilter.doFilter(mockData.request, mockData.response, mockData.chain)); + + verifyChainNotCalled(mockData.chain); + verifyRequestValidatedSync(mockData); + verifyNoResponseValidation(); + } + + @Test + public void testShouldFailOnResponseViolationWithViolation() throws ServletException, IOException { + var mockData = mockSetup(MockConfiguration.builder().shouldFailOnResponseViolation(true).build()); + when( + validator.validateResponseObject( + eq(mockData.requestMetaData), + eq(mockData.responseMetaData), eq(REQUEST_BODY) + ) + ).thenReturn(ValidationResult.INVALID); + + assertThrows(ResponseStatusException.class, + () -> httpFilter.doFilter(mockData.request, mockData.response, mockData.chain)); + + verifyChainCalled(mockData.chain, mockData.cachingRequest, mockData.cachingResponse); + verifyRequestValidatedAsync(mockData); + verifyResponseValidatedSync(mockData); + } + + private void verifyNoValidation() { + verifyNoRequestValidation(); + verifyNoResponseValidation(); + } + + private void verifyNoRequestValidation() { + verify(validator, never()).validateRequestObjectAsync(any(), anyString()); + verify(validator, never()).validateRequestObject(any(), anyString()); + } + + private void verifyNoResponseValidation() { + verify(validator, never()).validateResponseObjectAsync(any(), any(), anyString()); + verify(validator, never()).validateResponseObject(any(), any(), anyString()); + } + + private void verifyRequestValidatedAsync(MockSetupData mockData) { + verify(validator).validateRequestObjectAsync(eq(mockData.requestMetaData), eq(REQUEST_BODY)); + } + + private void verifyRequestValidatedSync(MockSetupData mockData) { + verify(validator).validateRequestObject(eq(mockData.requestMetaData), eq(REQUEST_BODY)); + } + + private void verifyResponseValidatedAsync(MockSetupData mockData) { + verify(validator).validateResponseObjectAsync( + eq(mockData.requestMetaData), + eq(mockData.responseMetaData), + eq(RESPONSE_BODY) + ); + } + + private void verifyResponseValidatedSync(MockSetupData mockData) { + verify(validator) + .validateResponseObject(eq(mockData.requestMetaData), eq(mockData.responseMetaData), eq(RESPONSE_BODY)); + } + + private void mockTrafficSelectorMethods( + RequestMetaData requestMetaData, + ResponseMetaData responseMetaData, + MockConfiguration configuration + ) { + when(trafficSelector.shouldRequestBeValidated(any())).thenReturn(configuration.shouldRequestBeValidated); + when(trafficSelector.canRequestBeValidated(requestMetaData)).thenReturn(configuration.canRequestBeValidated); + when(trafficSelector.canResponseBeValidated(requestMetaData, responseMetaData)).thenReturn( + configuration.canResponseBeValidated); + when(trafficSelector.shouldFailOnRequestViolation(requestMetaData)).thenReturn( + configuration.shouldFailOnRequestViolation); + when(trafficSelector.shouldFailOnResponseViolation(requestMetaData)).thenReturn( + configuration.shouldFailOnResponseViolation); + } + + private ContentCachingResponseWrapper mockContentCachingResponse( + HttpServletResponse response, + MockConfiguration configuration + ) { + var cachingResponse = mock(ContentCachingResponseWrapper.class); + when(contentCachingWrapperFactory.buildContentCachingResponseWrapper(response)).thenReturn(cachingResponse); + if (configuration.responseBody != null) { + when(cachingResponse.getContentType()).thenReturn("application/json"); + when(cachingResponse.getContentAsByteArray()) + .thenReturn(configuration.responseBody.getBytes(StandardCharsets.UTF_8)); + } + return cachingResponse; + } + + private ContentCachingRequestWrapper mockContentCachingRequest( + HttpServletRequest request, + MockConfiguration configuration + ) { + var cachingRequest = mock(ContentCachingRequestWrapper.class); + when(contentCachingWrapperFactory.buildContentCachingRequestWrapper(request)).thenReturn(cachingRequest); + if (configuration.responseBody != null) { + when(cachingRequest.getContentType()).thenReturn("application/json"); + when(cachingRequest.getContentAsByteArray()) + .thenReturn(configuration.requestBody.getBytes(StandardCharsets.UTF_8)); + } + return cachingRequest; + } + + private static void verifyChainCalled(FilterChain chain, ServletRequest request, ServletResponse response) + throws ServletException, IOException { + verify(chain).doFilter(request, response); + } + + private static void verifyChainNotCalled(FilterChain chain) throws ServletException, IOException { + verify(chain, never()).doFilter(any(), any()); + } + + private MockSetupData mockSetup(MockConfiguration configuration) { + var request = mock(HttpServletRequest.class); + var response = mock(HttpServletResponse.class); + + var requestMetaData = mock(RequestMetaData.class); + when(metaDataFactory.buildRequestMetaData(request)).thenReturn(requestMetaData); + + var responseMetaData = mock(ResponseMetaData.class); + when(metaDataFactory.buildResponseMetaData(response)).thenReturn(responseMetaData); + + var cachingRequest = mockContentCachingRequest(request, configuration); + var cachingResponse = mockContentCachingResponse(response, configuration); + when(metaDataFactory.buildResponseMetaData(cachingResponse)).thenReturn(responseMetaData); + + var chain = mock(FilterChain.class); + when(validator.isReady()).thenReturn(configuration.isReady); + mockTrafficSelectorMethods(requestMetaData, responseMetaData, configuration); + + return MockSetupData.builder() + .request(request) + .response(response) + .cachingRequest(cachingRequest) + .cachingResponse(cachingResponse) + .chain(chain) + .requestMetaData(requestMetaData) + .responseMetaData(responseMetaData) + .build(); + } + + @Builder + private static class MockConfiguration { + @Builder.Default + private boolean isReady = true; + @Builder.Default + private boolean shouldRequestBeValidated = true; + + @Builder.Default + private boolean canRequestBeValidated = true; + @Builder.Default + private boolean canResponseBeValidated = true; + + @Builder.Default + private boolean shouldFailOnRequestViolation = false; + @Builder.Default + private boolean shouldFailOnResponseViolation = false; + + @Builder.Default + private String requestBody = REQUEST_BODY; + @Builder.Default + private String responseBody = RESPONSE_BODY; + } + + @Builder + private record MockSetupData( + ServletRequest request, + ServletResponse response, + ServletRequest cachingRequest, + ServletResponse cachingResponse, + FilterChain chain, + RequestMetaData requestMetaData, + ResponseMetaData responseMetaData + ) { + } +} diff --git a/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilterTest.java b/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilterTest.java index c0b2be3b..bda37741 100644 --- a/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilterTest.java +++ b/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilterTest.java @@ -140,7 +140,7 @@ public void testShouldFailOnRequestViolationWithViolation() { } @Test - public void testShouldFailOnReResponseViolationWithViolation() { + public void testShouldFailOnResponseViolationWithViolation() { var mockData = mockSetup(MockConfiguration.builder().shouldFailOnResponseViolation(true).build()); when( validator.validateResponseObject( From 5911db19d2e6b94c0eccbb706823a82facdb6ee5 Mon Sep 17 00:00:00 2001 From: Patrick Boos Date: Tue, 23 May 2023 16:56:58 +0200 Subject: [PATCH 5/7] Update README.md --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index 200d465b..0fd77782 100644 --- a/README.md +++ b/README.md @@ -83,6 +83,8 @@ openapi.validation.sample-rate=1.0 # Custom location of specification file within resources or filesystem. openapi.validation.specification-file-path=/tmp/openapi-spec/openapi.json +# If it is within src/main/resources/folder/my-spec.json use +openapi.validation.specification-file-path=folder/my-spec.json # Comma separated list of paths to be excluded from validation. Default is no excluded paths openapi.validation.excluded-paths=/_readiness,/_liveness,/_metrics @@ -98,6 +100,10 @@ openapi.validation.validation-report-metric-name=openapi.violation # Add additional tags to be logged with metrics. They should be in the format {KEY}={VALUE},{KEY}={VALUE} # Default is no additional tags. openapi.validation.validation-report-metric-additional-tags=service=example,team=chk + +# Fail requests on request/response violations. Defaults to false. +openapi.validation.should-fail-on-request-violation=true +openapi.validation.should-fail-on-response-violation=true ``` ### DataDog metrics From 7b7e1daa55f6fd04d0339aac455a33b809718b64 Mon Sep 17 00:00:00 2001 From: Patrick Boos Date: Wed, 24 May 2023 11:05:59 +0200 Subject: [PATCH 6/7] Remove unnecessary `@Slf4j` annotation --- .../openapi/validation/filter/OpenApiValidationWebFilter.java | 2 -- .../openapi/validation/filter/OpenApiValidationWebFilter.java | 2 -- 2 files changed, 4 deletions(-) diff --git a/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java b/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java index 353ea340..f6b95bf6 100644 --- a/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java +++ b/spring-boot-starter/spring-boot-starter-webflux-spring2.7/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java @@ -9,7 +9,6 @@ import com.getyourguide.openapi.validation.filter.decorator.BodyCachingServerHttpResponseDecorator; import com.getyourguide.openapi.validation.filter.decorator.DecoratorBuilder; import lombok.AllArgsConstructor; -import lombok.extern.slf4j.Slf4j; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; import org.springframework.http.HttpStatus; @@ -25,7 +24,6 @@ @Component @Order(Ordered.HIGHEST_PRECEDENCE) @AllArgsConstructor -@Slf4j public class OpenApiValidationWebFilter implements WebFilter { private final OpenApiRequestValidator validator; diff --git a/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java b/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java index 3fd87ca6..f5218dea 100644 --- a/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java +++ b/spring-boot-starter/spring-boot-starter-webflux/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationWebFilter.java @@ -9,7 +9,6 @@ import com.getyourguide.openapi.validation.filter.decorator.BodyCachingServerHttpResponseDecorator; import com.getyourguide.openapi.validation.filter.decorator.DecoratorBuilder; import lombok.AllArgsConstructor; -import lombok.extern.slf4j.Slf4j; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; import org.springframework.http.HttpStatusCode; @@ -25,7 +24,6 @@ @Component @Order(Ordered.HIGHEST_PRECEDENCE) @AllArgsConstructor -@Slf4j public class OpenApiValidationWebFilter implements WebFilter { private final OpenApiRequestValidator validator; From fdc90f5c5c749353566b5912a7924af6eac2022a Mon Sep 17 00:00:00 2001 From: Patrick Boos Date: Wed, 24 May 2023 11:12:53 +0200 Subject: [PATCH 7/7] Remove leftover TODO --- .../getyourguide/openapi/validation/api/log/LoggerExtension.java | 1 - 1 file changed, 1 deletion(-) diff --git a/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/log/LoggerExtension.java b/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/log/LoggerExtension.java index 92af984a..5f891760 100644 --- a/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/log/LoggerExtension.java +++ b/openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/log/LoggerExtension.java @@ -4,7 +4,6 @@ import java.util.Map; import lombok.NonNull; -// TODO CHK-8357 can we get rid of this one? public interface LoggerExtension { Closeable addToLoggingContext(@NonNull Map newTags); }