From 70f07bd83852b4c7a3b2141fa71a3e808240c3b8 Mon Sep 17 00:00:00 2001 From: Patrick Boos Date: Fri, 24 Nov 2023 17:03:15 +0100 Subject: [PATCH] bugfix: do not report violation on blocked request (#66) --- ...OpenApiRequestValidationConfiguration.java | 1 + .../core/OpenApiRequestValidator.java | 9 +- ...penApiValidationApplicationProperties.java | 1 + .../factory/ContentCachingWrapperFactory.java | 18 ++-- ...MultiReadContentCachingRequestWrapper.java | 64 +++++++++++ .../filter/OpenApiValidationInterceptor.java | 19 +++- .../validation/filter/BaseFilterTest.java | 29 +++-- .../FailOnViolationIntegrationTest.java | 94 ++++++++++++++++ .../OpenApiValidationIntegrationTest.java | 2 - .../filter/OpenApiValidationWebFilter.java | 54 +++++----- ...BodyCachingServerHttpRequestDecorator.java | 29 +++-- .../OpenApiValidationWebFilterTest.java | 11 +- .../FailOnViolationIntegrationTest.java | 101 ++++++++++++++++++ .../OpenApiValidationIntegrationTest.java | 2 - 14 files changed, 364 insertions(+), 70 deletions(-) create mode 100644 spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/filter/MultiReadContentCachingRequestWrapper.java create mode 100644 spring-boot-starter/spring-boot-starter-web/src/test/java/com/getyourguide/openapi/validation/integration/FailOnViolationIntegrationTest.java create mode 100644 spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/integration/FailOnViolationIntegrationTest.java diff --git a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidationConfiguration.java b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidationConfiguration.java index 0c04f46..e37fd67 100644 --- a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidationConfiguration.java +++ b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidationConfiguration.java @@ -8,4 +8,5 @@ public class OpenApiRequestValidationConfiguration { private double sampleRate; private int validationReportThrottleWaitSeconds; + private boolean shouldFailOnRequestViolation; } 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 a3422ac..79f4c48 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 @@ -23,6 +23,7 @@ public class OpenApiRequestValidator { private final ThreadPoolExecutor threadPoolExecutor; private final OpenApiInteractionValidatorWrapper validator; private final ValidationReportHandler validationReportHandler; + private final OpenApiRequestValidationConfiguration configuration; public OpenApiRequestValidator( ThreadPoolExecutor threadPoolExecutor, @@ -34,6 +35,7 @@ public OpenApiRequestValidator( this.threadPoolExecutor = threadPoolExecutor; this.validator = validator; this.validationReportHandler = validationReportHandler; + this.configuration = configuration; metricsReporter.reportStartup( validator != null, @@ -74,7 +76,12 @@ public ValidationResult validateRequestObject( try { var simpleRequest = buildSimpleRequest(request, requestBody); var result = validator.validateRequest(simpleRequest); - validationReportHandler.handleValidationReport(request, response, Direction.REQUEST, requestBody, result); + // TODO this should not be done here, but currently the only way to do it -> Refactor this so that logging + // is actually done in the interceptor/filter where logging can easily be skipped then. + if (!configuration.isShouldFailOnRequestViolation()) { + validationReportHandler + .handleValidationReport(request, response, Direction.REQUEST, requestBody, result); + } return buildValidationResult(result); } catch (Exception e) { log.error("Could not validate request", e); 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 6e0f891..97663d3 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 @@ -88,6 +88,7 @@ public OpenApiRequestValidationConfiguration toOpenApiRequestValidationConfigura return OpenApiRequestValidationConfiguration.builder() .sampleRate(getSampleRate()) .validationReportThrottleWaitSeconds(getValidationReportThrottleWaitSeconds()) + .shouldFailOnRequestViolation(getShouldFailOnRequestViolation() != null && getShouldFailOnRequestViolation()) .build(); } } 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 index c4f705d..3c56450 100644 --- 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 @@ -1,19 +1,19 @@ package com.getyourguide.openapi.validation.factory; +import com.getyourguide.openapi.validation.filter.MultiReadContentCachingRequestWrapper; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import javax.annotation.Nullable; -import org.springframework.web.util.ContentCachingRequestWrapper; import org.springframework.web.util.ContentCachingResponseWrapper; import org.springframework.web.util.WebUtils; public class ContentCachingWrapperFactory { - public ContentCachingRequestWrapper buildContentCachingRequestWrapper(HttpServletRequest request) { - if (request instanceof ContentCachingRequestWrapper) { - return (ContentCachingRequestWrapper) request; + public MultiReadContentCachingRequestWrapper buildContentCachingRequestWrapper(HttpServletRequest request) { + if (request instanceof MultiReadContentCachingRequestWrapper) { + return (MultiReadContentCachingRequestWrapper) request; } - return new ContentCachingRequestWrapper(request); + return new MultiReadContentCachingRequestWrapper(request); } public ContentCachingResponseWrapper buildContentCachingResponseWrapper(HttpServletResponse response) { @@ -26,12 +26,12 @@ public ContentCachingResponseWrapper buildContentCachingResponseWrapper(HttpServ } @Nullable - public ContentCachingResponseWrapper getCachingResponse(final HttpServletResponse response) { - return WebUtils.getNativeResponse(response, ContentCachingResponseWrapper.class); + public MultiReadContentCachingRequestWrapper getCachingRequest(HttpServletRequest request) { + return request instanceof MultiReadContentCachingRequestWrapper ? (MultiReadContentCachingRequestWrapper) request : null; } @Nullable - public ContentCachingRequestWrapper getCachingRequest(HttpServletRequest request) { - return request instanceof ContentCachingRequestWrapper ? (ContentCachingRequestWrapper) request : null; + public ContentCachingResponseWrapper getCachingResponse(final HttpServletResponse response) { + return WebUtils.getNativeResponse(response, ContentCachingResponseWrapper.class); } } diff --git a/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/filter/MultiReadContentCachingRequestWrapper.java b/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/filter/MultiReadContentCachingRequestWrapper.java new file mode 100644 index 0000000..b9ba01b --- /dev/null +++ b/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/filter/MultiReadContentCachingRequestWrapper.java @@ -0,0 +1,64 @@ +package com.getyourguide.openapi.validation.filter; + +import jakarta.servlet.ReadListener; +import jakarta.servlet.ServletInputStream; +import jakarta.servlet.http.HttpServletRequest; +import java.io.BufferedReader; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStreamReader; +import org.springframework.web.util.ContentCachingRequestWrapper; + +public class MultiReadContentCachingRequestWrapper extends ContentCachingRequestWrapper { + + public MultiReadContentCachingRequestWrapper(HttpServletRequest request) { + super(request); + } + + public MultiReadContentCachingRequestWrapper(HttpServletRequest request, int contentCacheLimit) { + super(request, contentCacheLimit); + } + + @Override + public ServletInputStream getInputStream() throws IOException { + var inputStream = super.getInputStream(); + if (inputStream.isFinished()) { + return new CachedServletInputStream(getContentAsByteArray()); + } + + return inputStream; + } + + @Override + public BufferedReader getReader() throws IOException { + return new BufferedReader(new InputStreamReader(getInputStream())); + } + + private static class CachedServletInputStream extends ServletInputStream { + private final ByteArrayInputStream buffer; + + public CachedServletInputStream(byte[] contents) { + this.buffer = new ByteArrayInputStream(contents); + } + + @Override + public int read() throws IOException { + return buffer.read(); + } + + @Override + public boolean isFinished() { + return buffer.available() == 0; + } + + @Override + public boolean isReady() { + return true; + } + + @Override + public void setReadListener(ReadListener listener) { + throw new UnsupportedOperationException("Not implemented"); + } + } +} diff --git a/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationInterceptor.java b/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationInterceptor.java index 58a5789..411d21f 100644 --- a/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationInterceptor.java +++ b/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationInterceptor.java @@ -9,15 +9,16 @@ import com.getyourguide.openapi.validation.factory.ServletMetaDataFactory; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import java.io.IOException; import java.nio.charset.StandardCharsets; import javax.annotation.Nullable; import lombok.AllArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.http.HttpStatusCode; +import org.springframework.util.StreamUtils; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.servlet.AsyncHandlerInterceptor; import org.springframework.web.servlet.ModelAndView; -import org.springframework.web.util.ContentCachingRequestWrapper; import org.springframework.web.util.ContentCachingResponseWrapper; @Slf4j @@ -114,6 +115,8 @@ private void validateResponse( ); // Note: validateResponseResult will always be null on ASYNC if (validateResponseResult == ValidationResult.INVALID) { + response.reset(); + response.setStatus(500); throw new ResponseStatusException(HttpStatusCode.valueOf(500), "Response validation failed"); } } @@ -126,7 +129,7 @@ private static RequestMetaData getRequestMetaData(HttpServletRequest request) { } private ValidationResult validateRequest( - ContentCachingRequestWrapper request, + MultiReadContentCachingRequestWrapper request, RequestMetaData requestMetaData, @Nullable ResponseMetaData responseMetaData, RunType runType @@ -137,9 +140,7 @@ private ValidationResult validateRequest( return ValidationResult.NOT_APPLICABLE; } - var requestBody = request.getContentType() != null - ? new String(request.getContentAsByteArray(), StandardCharsets.UTF_8) - : null; + var requestBody = request.getContentType() != null ? readBodyCatchingException(request) : null; if (runType == RunType.ASYNC) { validator.validateRequestObjectAsync(requestMetaData, responseMetaData, requestBody); @@ -149,6 +150,14 @@ private ValidationResult validateRequest( } } + private static String readBodyCatchingException(MultiReadContentCachingRequestWrapper request) { + try { + return StreamUtils.copyToString(request.getInputStream(), StandardCharsets.UTF_8); + } catch (IOException e) { + return null; + } + } + private ValidationResult validateResponse( HttpServletRequest request, ContentCachingResponseWrapper response, diff --git a/spring-boot-starter/spring-boot-starter-web/src/test/java/com/getyourguide/openapi/validation/filter/BaseFilterTest.java b/spring-boot-starter/spring-boot-starter-web/src/test/java/com/getyourguide/openapi/validation/filter/BaseFilterTest.java index d293af6..364b47c 100644 --- a/spring-boot-starter/spring-boot-starter-web/src/test/java/com/getyourguide/openapi/validation/filter/BaseFilterTest.java +++ b/spring-boot-starter/spring-boot-starter-web/src/test/java/com/getyourguide/openapi/validation/filter/BaseFilterTest.java @@ -16,11 +16,13 @@ import jakarta.servlet.ServletResponse; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import java.io.ByteArrayInputStream; +import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.HashMap; import lombok.Builder; import org.mockito.Mockito; -import org.springframework.web.util.ContentCachingRequestWrapper; +import org.springframework.mock.web.DelegatingServletInputStream; import org.springframework.web.util.ContentCachingResponseWrapper; public class BaseFilterTest { @@ -48,15 +50,12 @@ private static void mockRequestAttributes(ServletRequest request, HashMap violations, String rule) { return violations.stream() 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 6834c82..aa2b260 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 @@ -52,19 +52,19 @@ 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) + final var alreadyDidValidation = new AlreadyDidValidation(); + alreadyDidValidation.response = validateResponseWithFailOnViolation(requestMetaData, responseDecorated); + return optionalValidateRequestWithFailOnViolation(requestMetaData, requestDecorated, alreadyDidValidation) + .flatMap(dataBuffer -> 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) { var responseMetaData = metaDataFactory.buildResponseMetaData(responseDecorated); - if (!alreadyDidRequestValidation) { + if (!alreadyDidValidation.request) { validateRequest(requestDecorated, requestMetaData, responseMetaData, RunType.ASYNC); } - if (!alreadyDidResponseValidation) { + if (!alreadyDidValidation.response) { validateResponse( requestMetaData, responseMetaData, responseDecorated.getCachedBody(), RunType.ASYNC); } @@ -72,34 +72,29 @@ private Mono decorateWithValidation(ServerWebExchange exchange, WebFilterC }); } - /** - * 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( + private Mono optionalValidateRequestWithFailOnViolation( RequestMetaData requestMetaData, - BodyCachingServerHttpRequestDecorator requestDecorated + BodyCachingServerHttpRequestDecorator request, + AlreadyDidValidation alreadyDidValidation ) { - if (!trafficSelector.shouldFailOnRequestViolation(requestMetaData)) { - return false; + if (!trafficSelector.shouldFailOnRequestViolation(requestMetaData) + || !request.getHeaders().containsKey("Content-Type") + || !request.getHeaders().containsKey("Content-Length")) { + return Mono.just(alreadyDidValidation); } - if (requestDecorated.getHeaders().containsKey("Content-Type") && requestDecorated.getHeaders().containsKey("Content-Length")) { - requestDecorated.setOnBodyCachedListener(() -> { - var validateRequestResult = validateRequest(requestDecorated, requestMetaData, null, RunType.SYNC); - throwStatusExceptionOnViolation(validateRequestResult, "Request validation failed"); + return request.consumeRequestBody() + .map((optionalRequestBody) -> { + var validateRequestResult = validateRequest(request, requestMetaData, null, RunType.SYNC); + throwStatusExceptionOnViolation(validateRequestResult, 400, "Request validation failed"); + alreadyDidValidation.request = true; + return alreadyDidValidation; }); - } else { - var validateRequestResult = validateRequest(requestDecorated, requestMetaData, null, RunType.SYNC); - throwStatusExceptionOnViolation(validateRequestResult, "Request validation failed"); - } - return true; } - private static void throwStatusExceptionOnViolation(ValidationResult validateRequestResult, String message) { + private static void throwStatusExceptionOnViolation(ValidationResult validateRequestResult, int statusCode, String message) { if (validateRequestResult == ValidationResult.INVALID) { - throw new ResponseStatusException(HttpStatus.valueOf(400), message); + throw new ResponseStatusException(HttpStatus.valueOf(statusCode), message); } } @@ -123,7 +118,7 @@ private boolean validateResponseWithFailOnViolation( responseDecorated.getCachedBody(), RunType.SYNC ); - throwStatusExceptionOnViolation(validateResponseResult, "Response validation failed"); + throwStatusExceptionOnViolation(validateResponseResult, 500, "Response validation failed"); }); return true; } @@ -165,4 +160,9 @@ private ValidationResult validateResponse( } private enum RunType { SYNC, ASYNC } + + private static class AlreadyDidValidation { + private boolean request = false; + private boolean response = false; + } } 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 c883a4f..67bf914 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 @@ -3,24 +3,24 @@ import com.getyourguide.openapi.validation.api.model.RequestMetaData; import com.getyourguide.openapi.validation.api.selector.TrafficSelector; import java.nio.charset.StandardCharsets; +import java.util.stream.Collectors; import lombok.Getter; -import lombok.Setter; import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpRequestDecorator; import org.springframework.lang.NonNull; import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; import reactor.core.publisher.SignalType; public class BodyCachingServerHttpRequestDecorator extends ServerHttpRequestDecorator { private final TrafficSelector trafficSelector; private final RequestMetaData requestMetaData; - @Setter - private Runnable onBodyCachedListener; - @Getter private String cachedBody; + private boolean bodyCached = false; public BodyCachingServerHttpRequestDecorator( ServerHttpRequest delegate, @@ -32,9 +32,26 @@ public BodyCachingServerHttpRequestDecorator( this.requestMetaData = requestMetaData; } + public Mono consumeRequestBody() { + return Mono.from(getBody().collectList()) + .map(dataBuffers -> + dataBuffers.stream() + .map(dataBuffer -> dataBuffer.toString(StandardCharsets.UTF_8)) + .collect(Collectors.joining("")) + ); + } + @Override @NonNull public Flux getBody() { + if (bodyCached) { + var bufferFactory = new DefaultDataBufferFactory(); + var bytes = cachedBody.getBytes(StandardCharsets.UTF_8); + var buffer = bufferFactory.allocateBuffer(bytes.length); + buffer.write(bytes); + return Flux.just(buffer); + } + if (!trafficSelector.canRequestBeValidated(requestMetaData)) { return super.getBody(); } @@ -47,8 +64,8 @@ public Flux getBody() { cachedBody += dataBuffer.toString(StandardCharsets.UTF_8); }) .doFinally(signalType -> { - if (signalType == SignalType.ON_COMPLETE && onBodyCachedListener != null) { - onBodyCachedListener.run(); + if (signalType == SignalType.ON_COMPLETE) { + bodyCached = true; } }); } 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 0a6a411..a6572d7 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 @@ -132,7 +132,9 @@ public void testShouldFailOnRequestViolationWithViolation() { when(validator.validateRequestObject(eq(mockData.requestMetaData), any(), eq(REQUEST_BODY))) .thenReturn(ValidationResult.INVALID); - assertThrows(ResponseStatusException.class, () -> webFilter.filter(mockData.exchange, mockData.chain)); + StepVerifier.create(webFilter.filter(mockData.exchange, mockData.chain)) + .expectErrorMatches(throwable -> throwable instanceof ResponseStatusException) + .verify(); verifyChainNotCalled(mockData.chain); verifyRequestValidatedSync(mockData); @@ -226,12 +228,7 @@ private void mockDecoratedRequests( .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()); - } + when(decoratedRequest.consumeRequestBody()).thenReturn(Mono.just(configuration.requestBody)); var decoratedResponse = mock(BodyCachingServerHttpResponseDecorator.class); when(decoratorBuilder.buildBodyCachingServerHttpResponseDecorator(response, requestMetaData)) diff --git a/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/integration/FailOnViolationIntegrationTest.java b/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/integration/FailOnViolationIntegrationTest.java new file mode 100644 index 0000000..602236b --- /dev/null +++ b/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/integration/FailOnViolationIntegrationTest.java @@ -0,0 +1,101 @@ +package com.getyourguide.openapi.validation.integration; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +import com.getyourguide.openapi.validation.integration.controller.DefaultRestController; +import com.getyourguide.openapi.validation.test.TestViolationLogger; +import java.util.Optional; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.SpyBean; +import org.springframework.http.MediaType; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.test.web.reactive.server.WebTestClient; + +@SpringBootTest(properties = { + "openapi.validation.should-fail-on-request-violation=true", + "openapi.validation.should-fail-on-response-violation=true", +}) +@AutoConfigureMockMvc +@ExtendWith(SpringExtension.class) +public class FailOnViolationIntegrationTest { + + @Autowired + private WebTestClient webTestClient; + + @Autowired + private TestViolationLogger openApiViolationLogger; + + @SpyBean + private DefaultRestController defaultRestController; + + @BeforeEach + public void setup() { + openApiViolationLogger.clearViolations(); + } + + @Test + public void whenValidRequestThenReturnSuccessfully() throws Exception { + webTestClient + .post().uri("/test") + .accept(MediaType.APPLICATION_JSON) + .contentType(MediaType.APPLICATION_JSON) + .bodyValue("{ \"value\": \"testing\", \"responseStatusCode\": 200 }") + .exchange() + .expectStatus().isOk() + .expectBody().jsonPath("$.value").isEqualTo("testing"); + Thread.sleep(100); + + assertEquals(0, openApiViolationLogger.getViolations().size()); + verify(defaultRestController).postTest(any(), any()); + } + + @Test + public void whenInvalidRequestThenReturn400AndNoViolationLogged() throws Exception { + webTestClient + .post().uri("/test") + .accept(MediaType.APPLICATION_JSON) + .contentType(MediaType.APPLICATION_JSON) + .bodyValue("{ \"value\": 1 }") + .exchange() + .expectStatus().is4xxClientError() + .expectBody() + .jsonPath("$.status").isEqualTo(400) + .jsonPath("$.path").isEqualTo("/test") + .jsonPath("$.error").isEqualTo("Bad Request"); + Thread.sleep(100); + + assertEquals(0, openApiViolationLogger.getViolations().size()); + verify(defaultRestController, never()).postTest(any(), any()); + + // TODO check that something else gets logged? + } + + @Test + public void whenInvalidResponseThenReturn500AndViolationLogged() throws Exception { + webTestClient + .get().uri("/test?value=invalid-response-value!") + .accept(MediaType.APPLICATION_JSON) + .exchange() + .expectStatus().is5xxServerError() + .expectBody() + .jsonPath("$.status").isEqualTo(500) + .jsonPath("$.path").isEqualTo("/test") + .jsonPath("$.error").isEqualTo("Internal Server Error"); + Thread.sleep(100); + + assertEquals(1, openApiViolationLogger.getViolations().size()); + var violation = openApiViolationLogger.getViolations().get(0); + assertEquals("validation.response.body.schema.pattern", violation.getRule()); + assertEquals(Optional.of(200), violation.getResponseStatus()); + assertEquals(Optional.of("/value"), violation.getInstance()); + verify(defaultRestController).getTest(any(), any(), any(), any()); + } +} diff --git a/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/integration/OpenApiValidationIntegrationTest.java b/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/integration/OpenApiValidationIntegrationTest.java index 7faf80d..3a7afe4 100644 --- a/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/integration/OpenApiValidationIntegrationTest.java +++ b/spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/integration/OpenApiValidationIntegrationTest.java @@ -134,8 +134,6 @@ public void whenTestOptionsCallThenShouldNotValidate() throws Exception { assertEquals(0, openApiViolationLogger.getViolations().size()); } - // TODO Add test that fails on request violation immediately (maybe needs separate test class & setup) should not log violation - @Nullable private OpenApiViolation getViolationByRule(List violations, String rule) { return violations.stream()