From b5f945d69ce7f9682fa967baa40b9aa269080d4f Mon Sep 17 00:00:00 2001 From: Elena Kubantseva Date: Tue, 27 Aug 2024 20:01:11 +0200 Subject: [PATCH] fix: Add missing encoded characters filter for services (#3701) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fixed encoded slashes filter and added missing encoded characters filter Signed-off-by: Elena Kubantseva * Added and fixed some tests Signed-off-by: Elena Kubantseva * Added description to the filter Signed-off-by: Elena Kubantseva * Addressed review comments Signed-off-by: Elena Kubantseva * changed default value for enableUrlEncodedCharacters Signed-off-by: Elena Kubantseva * removed unneeded annotations Signed-off-by: Elena Kubantseva * removed public from tests Signed-off-by: Elena Kubantseva --------- Signed-off-by: Elena Kubantseva Co-authored-by: Pavel Jareš <58428711+pj892031@users.noreply.github.com> --- .../constants/EurekaMetadataDefinition.java | 1 + .../gateway/config/UrlTomcatCustomizer.java | 29 +++++ ...bstractEncodedCharactersFilterFactory.java | 80 +++++++++++++ .../ForbidEncodedCharactersFilterFactory.java | 41 +++++++ .../ForbidEncodedSlashesFilterFactory.java | 69 ++--------- .../apiml/gateway/service/RouteLocator.java | 16 ++- .../config/UrlTomcatCustomizerTest.java | 31 +++++ ...bidEncodedCharactersFilterFactoryTest.java | 110 ++++++++++++++++++ ...ForbidEncodedSlashesFilterFactoryTest.java | 35 +++--- .../gateway/service/RouteLocatorTest.java | 49 +++++++- .../config/NewSecurityConfiguration.java | 9 -- 11 files changed, 380 insertions(+), 90 deletions(-) create mode 100644 gateway-service/src/main/java/org/zowe/apiml/gateway/config/UrlTomcatCustomizer.java create mode 100644 gateway-service/src/main/java/org/zowe/apiml/gateway/filters/AbstractEncodedCharactersFilterFactory.java create mode 100644 gateway-service/src/main/java/org/zowe/apiml/gateway/filters/ForbidEncodedCharactersFilterFactory.java create mode 100644 gateway-service/src/test/java/org/zowe/apiml/gateway/config/UrlTomcatCustomizerTest.java create mode 100644 gateway-service/src/test/java/org/zowe/apiml/gateway/filters/ForbidEncodedCharactersFilterFactoryTest.java diff --git a/common-service-core/src/main/java/org/zowe/apiml/constants/EurekaMetadataDefinition.java b/common-service-core/src/main/java/org/zowe/apiml/constants/EurekaMetadataDefinition.java index 486327cf4e..e2459369d6 100644 --- a/common-service-core/src/main/java/org/zowe/apiml/constants/EurekaMetadataDefinition.java +++ b/common-service-core/src/main/java/org/zowe/apiml/constants/EurekaMetadataDefinition.java @@ -33,6 +33,7 @@ private EurekaMetadataDefinition() { public static final String SERVICE_DESCRIPTION = "apiml.service.description"; public static final String SERVICE_EXTERNAL_URL = "apiml.service.externalUrl"; public static final String SERVICE_SUPPORTING_CLIENT_CERT_FORWARDING = "apiml.service.supportClientCertForwarding"; + public static final String ENABLE_URL_ENCODED_CHARACTERS = "apiml.enableUrlEncodedCharacters"; public static final String APIML_ID = "apiml.service.apimlId"; public static final String API_INFO = "apiml.apiInfo"; diff --git a/gateway-service/src/main/java/org/zowe/apiml/gateway/config/UrlTomcatCustomizer.java b/gateway-service/src/main/java/org/zowe/apiml/gateway/config/UrlTomcatCustomizer.java new file mode 100644 index 0000000000..17f43ef14a --- /dev/null +++ b/gateway-service/src/main/java/org/zowe/apiml/gateway/config/UrlTomcatCustomizer.java @@ -0,0 +1,29 @@ +/* + * This program and the accompanying materials are made available under the terms of the + * Eclipse Public License v2.0 which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-v20.html + * + * SPDX-License-Identifier: EPL-2.0 + * + * Copyright Contributors to the Zowe Project. + */ + +package org.zowe.apiml.gateway.config; + +import lombok.extern.slf4j.Slf4j; +import org.apache.catalina.connector.Connector; +import org.apache.tomcat.util.buf.EncodedSolidusHandling; +import org.springframework.boot.web.embedded.tomcat.TomcatConnectorCustomizer; +import org.springframework.stereotype.Component; + +@Slf4j +@Component +public class UrlTomcatCustomizer implements TomcatConnectorCustomizer { + + @Override + public void customize(Connector connector) { + connector.setAllowBackslash(true); + connector.setEncodedSolidusHandling(EncodedSolidusHandling.PASS_THROUGH.getValue()); + } + +} diff --git a/gateway-service/src/main/java/org/zowe/apiml/gateway/filters/AbstractEncodedCharactersFilterFactory.java b/gateway-service/src/main/java/org/zowe/apiml/gateway/filters/AbstractEncodedCharactersFilterFactory.java new file mode 100644 index 0000000000..16069b6900 --- /dev/null +++ b/gateway-service/src/main/java/org/zowe/apiml/gateway/filters/AbstractEncodedCharactersFilterFactory.java @@ -0,0 +1,80 @@ +/* + * This program and the accompanying materials are made available under the terms of the + * Eclipse Public License v2.0 which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-v20.html + * + * SPDX-License-Identifier: EPL-2.0 + * + * Copyright Contributors to the Zowe Project. + */ + +package org.zowe.apiml.gateway.filters; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import lombok.AccessLevel; +import lombok.RequiredArgsConstructor; +import org.springframework.cloud.gateway.filter.GatewayFilter; +import org.springframework.cloud.gateway.filter.factory.AbstractGatewayFilterFactory; +import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.http.HttpHeaders; +import org.springframework.http.codec.ServerCodecConfigurer; +import org.springframework.web.server.adapter.DefaultServerWebExchange; +import org.springframework.web.server.i18n.LocaleContextResolver; +import org.springframework.web.server.session.DefaultWebSessionManager; +import org.springframework.web.server.session.WebSessionManager; +import org.zowe.apiml.message.core.Message; +import org.zowe.apiml.message.core.MessageService; +import org.zowe.apiml.message.log.ApimlLogger; +import org.zowe.apiml.product.logging.annotations.InjectApimlLogger; +import reactor.core.publisher.Flux; + +import static org.apache.hc.core5.http.HttpStatus.SC_BAD_REQUEST; +import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; + +@RequiredArgsConstructor(access = AccessLevel.PROTECTED) +public abstract class AbstractEncodedCharactersFilterFactory extends AbstractGatewayFilterFactory { + + private final MessageService messageService; + private final ObjectMapper mapper; + private final LocaleContextResolver localeContextResolver; + private final String messageKey; + private final WebSessionManager sessionManager = new DefaultWebSessionManager(); + private final ServerCodecConfigurer serverCodecConfigurer = ServerCodecConfigurer.create(); + + @InjectApimlLogger + private final ApimlLogger apimlLog = ApimlLogger.empty(); + + protected abstract boolean shouldFilter(String uri); + + /** + * Filters requests by checking for encoded characters in the URI. + * If encoded characters are not allowed and found, returns a BAD_REQUEST response. + * Otherwise, proceeds with the filter chain. + * + * @return GatewayFilter + */ + @Override + public GatewayFilter apply(Object routeId) { + return ((exchange, chain) -> { + String uri = exchange.getRequest().getURI().toString(); + + if (!shouldFilter(uri)) { + return chain.filter(exchange); + } + + var serverWebExchange = new DefaultServerWebExchange(exchange.getRequest(), exchange.getResponse(), sessionManager, serverCodecConfigurer, localeContextResolver); + serverWebExchange.getResponse().setRawStatusCode(SC_BAD_REQUEST); + serverWebExchange.getResponse().getHeaders().add(HttpHeaders.CONTENT_TYPE, APPLICATION_JSON_VALUE); + + Message message = messageService.createMessage(messageKey, uri); + try { + DataBuffer buffer = serverWebExchange.getResponse().bufferFactory().wrap(mapper.writeValueAsBytes(message.mapToView())); + return serverWebExchange.getResponse().writeWith(Flux.just(buffer)); + } catch (JsonProcessingException e) { + apimlLog.log("org.zowe.apiml.security.errorWritingResponse", e.getMessage()); + throw new RuntimeException(e); + } + }); + } +} diff --git a/gateway-service/src/main/java/org/zowe/apiml/gateway/filters/ForbidEncodedCharactersFilterFactory.java b/gateway-service/src/main/java/org/zowe/apiml/gateway/filters/ForbidEncodedCharactersFilterFactory.java new file mode 100644 index 0000000000..d0d77eaa25 --- /dev/null +++ b/gateway-service/src/main/java/org/zowe/apiml/gateway/filters/ForbidEncodedCharactersFilterFactory.java @@ -0,0 +1,41 @@ +/* + * This program and the accompanying materials are made available under the terms of the + * Eclipse Public License v2.0 which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-v20.html + * + * SPDX-License-Identifier: EPL-2.0 + * + * Copyright Contributors to the Zowe Project. + */ + +package org.zowe.apiml.gateway.filters; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.commons.lang3.StringUtils; +import org.springframework.stereotype.Component; +import org.springframework.web.server.i18n.LocaleContextResolver; +import org.zowe.apiml.message.core.MessageService; + +/** + * This filter should run on all requests for services, which do not have enabled encoded characters in URL + *

+ * Special characters encoding is enabled on Tomcat so this filter takes over responsibility + * for filtering them. + * Encoded characters in URL are allowed by default. + */ + +@Component +public class ForbidEncodedCharactersFilterFactory extends AbstractEncodedCharactersFilterFactory { + + private static final char[] PROHIBITED_CHARACTERS = {'%', ';', '\\'}; + + public ForbidEncodedCharactersFilterFactory(MessageService messageService, ObjectMapper mapper, LocaleContextResolver localeContextResolver) { + super(messageService, mapper, localeContextResolver, "org.zowe.apiml.gateway.requestContainEncodedCharacter"); + } + + @Override + protected boolean shouldFilter(String uri) { + return StringUtils.containsAny(uri, PROHIBITED_CHARACTERS); + } + +} diff --git a/gateway-service/src/main/java/org/zowe/apiml/gateway/filters/ForbidEncodedSlashesFilterFactory.java b/gateway-service/src/main/java/org/zowe/apiml/gateway/filters/ForbidEncodedSlashesFilterFactory.java index fb3e27effd..5280022a80 100644 --- a/gateway-service/src/main/java/org/zowe/apiml/gateway/filters/ForbidEncodedSlashesFilterFactory.java +++ b/gateway-service/src/main/java/org/zowe/apiml/gateway/filters/ForbidEncodedSlashesFilterFactory.java @@ -10,79 +10,30 @@ package org.zowe.apiml.gateway.filters; -import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; -import lombok.RequiredArgsConstructor; -import org.springframework.cloud.gateway.filter.GatewayFilter; -import org.springframework.cloud.gateway.filter.factory.AbstractGatewayFilterFactory; -import org.springframework.core.io.buffer.DataBuffer; -import org.springframework.http.HttpHeaders; -import org.springframework.http.codec.ServerCodecConfigurer; +import org.apache.commons.lang3.StringUtils; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.stereotype.Component; -import org.springframework.web.server.adapter.DefaultServerWebExchange; import org.springframework.web.server.i18n.LocaleContextResolver; -import org.springframework.web.server.session.DefaultWebSessionManager; -import org.springframework.web.server.session.WebSessionManager; -import org.zowe.apiml.message.core.Message; import org.zowe.apiml.message.core.MessageService; -import org.zowe.apiml.message.log.ApimlLogger; -import org.zowe.apiml.product.logging.annotations.InjectApimlLogger; -import reactor.core.publisher.Flux; - -import java.net.URLDecoder; -import java.nio.charset.StandardCharsets; - -import static org.apache.hc.core5.http.HttpStatus.SC_BAD_REQUEST; -import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; /** * This filter checks if encoded slashes in the URI are allowed based on configuration. * If not allowed and encoded slashes are present, it returns a BAD_REQUEST response. */ @Component -@RequiredArgsConstructor -public class ForbidEncodedSlashesFilterFactory extends AbstractGatewayFilterFactory { +@ConditionalOnProperty(name = "apiml.service.allowEncodedSlashes", havingValue = "false", matchIfMissing = true) +public class ForbidEncodedSlashesFilterFactory extends AbstractEncodedCharactersFilterFactory { - private final MessageService messageService; - private final ObjectMapper mapper; + private static final String ENCODED_SLASH = "%2f"; - private final LocaleContextResolver localeContextResolver; - private final WebSessionManager sessionManager = new DefaultWebSessionManager(); - private final ServerCodecConfigurer serverCodecConfigurer = ServerCodecConfigurer.create(); - - @InjectApimlLogger - private final ApimlLogger apimlLog = ApimlLogger.empty(); + public ForbidEncodedSlashesFilterFactory(MessageService messageService, ObjectMapper mapper, LocaleContextResolver localeContextResolver) { + super(messageService, mapper, localeContextResolver, "org.zowe.apiml.gateway.requestContainEncodedSlash"); + } - /** - * Filters requests to check for encoded slashes in the URI. - * If encoded slashes are not allowed and found, returns a BAD_REQUEST response. - * Otherwise, proceeds with the filter chain. - * - * @return Allowed Encoded slashes filter. - */ @Override - public GatewayFilter apply(Object routeId) { - return ((exchange, chain) -> { - String uri = exchange.getRequest().getURI().toString(); - String decodedUri = URLDecoder.decode(uri, StandardCharsets.UTF_8); - - if (uri.equals(decodedUri)) { - return chain.filter(exchange); - } - - var serverWebExchange = new DefaultServerWebExchange(exchange.getRequest(), exchange.getResponse(), sessionManager, serverCodecConfigurer, localeContextResolver); - serverWebExchange.getResponse().setRawStatusCode(SC_BAD_REQUEST); - serverWebExchange.getResponse().getHeaders().add(HttpHeaders.CONTENT_TYPE, APPLICATION_JSON_VALUE); - - Message message = messageService.createMessage("org.zowe.apiml.gateway.requestContainEncodedSlash", uri); - try { - DataBuffer buffer = serverWebExchange.getResponse().bufferFactory().wrap(mapper.writeValueAsBytes(message.mapToView())); - return serverWebExchange.getResponse().writeWith(Flux.just(buffer)); - } catch (JsonProcessingException e) { - apimlLog.log("org.zowe.apiml.security.errorWritingResponse", e.getMessage()); - throw new RuntimeException(e); - } - }); + protected boolean shouldFilter(String uri) { + return StringUtils.containsIgnoreCase(uri, ENCODED_SLASH); } } diff --git a/gateway-service/src/main/java/org/zowe/apiml/gateway/service/RouteLocator.java b/gateway-service/src/main/java/org/zowe/apiml/gateway/service/RouteLocator.java index c9fd1113cf..4e5f8ce216 100644 --- a/gateway-service/src/main/java/org/zowe/apiml/gateway/service/RouteLocator.java +++ b/gateway-service/src/main/java/org/zowe/apiml/gateway/service/RouteLocator.java @@ -32,12 +32,18 @@ import org.zowe.apiml.util.StringUtils; import reactor.core.publisher.Flux; -import java.util.*; +import java.util.Comparator; +import java.util.EnumMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import java.util.stream.Stream; import static org.zowe.apiml.constants.EurekaMetadataDefinition.APIML_ID; +import static org.zowe.apiml.constants.EurekaMetadataDefinition.ENABLE_URL_ENCODED_CHARACTERS; import static org.zowe.apiml.constants.EurekaMetadataDefinition.SERVICE_SUPPORTING_CLIENT_CERT_FORWARDING; @Service @@ -133,6 +139,14 @@ List getPostRoutingFilters(ServiceInstance serviceInstance) { forwardClientCertFilter.setName("ForwardClientCertFilterFactory"); serviceRelated.add(forwardClientCertFilter); } + //Allow encoded characters by default + if (!Optional.ofNullable(serviceInstance.getMetadata().get(ENABLE_URL_ENCODED_CHARACTERS)) + .map(Boolean::parseBoolean) + .orElse(true)) { + FilterDefinition forbidEncodedCharactersFilter = new FilterDefinition(); + forbidEncodedCharactersFilter.setName("ForbidEncodedCharactersFilterFactory"); + serviceRelated.add(forbidEncodedCharactersFilter); + } FilterDefinition pageRedirectionFilter = new FilterDefinition(); pageRedirectionFilter.setName("PageRedirectionFilterFactory"); diff --git a/gateway-service/src/test/java/org/zowe/apiml/gateway/config/UrlTomcatCustomizerTest.java b/gateway-service/src/test/java/org/zowe/apiml/gateway/config/UrlTomcatCustomizerTest.java new file mode 100644 index 0000000000..c4a17a3474 --- /dev/null +++ b/gateway-service/src/test/java/org/zowe/apiml/gateway/config/UrlTomcatCustomizerTest.java @@ -0,0 +1,31 @@ +/* + * This program and the accompanying materials are made available under the terms of the + * Eclipse Public License v2.0 which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-v20.html + * + * SPDX-License-Identifier: EPL-2.0 + * + * Copyright Contributors to the Zowe Project. + */ + +package org.zowe.apiml.gateway.config; + +import org.apache.catalina.connector.Connector; +import org.apache.tomcat.util.buf.EncodedSolidusHandling; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class UrlTomcatCustomizerTest { + + private final Connector connector = new Connector(); + + @Test + void givenConnector_whenCustomize_thenCustomized() { + UrlTomcatCustomizer urlTomcatCustomizer = new UrlTomcatCustomizer(); + urlTomcatCustomizer.customize(connector); + assertTrue(connector.getAllowBackslash()); + assertEquals(EncodedSolidusHandling.PASS_THROUGH.getValue(), connector.getEncodedSolidusHandling()); + } +} diff --git a/gateway-service/src/test/java/org/zowe/apiml/gateway/filters/ForbidEncodedCharactersFilterFactoryTest.java b/gateway-service/src/test/java/org/zowe/apiml/gateway/filters/ForbidEncodedCharactersFilterFactoryTest.java new file mode 100644 index 0000000000..d15b79d86f --- /dev/null +++ b/gateway-service/src/test/java/org/zowe/apiml/gateway/filters/ForbidEncodedCharactersFilterFactoryTest.java @@ -0,0 +1,110 @@ +/* + * This program and the accompanying materials are made available under the terms of the + * Eclipse Public License v2.0 which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-v20.html + * + * SPDX-License-Identifier: EPL-2.0 + * + * Copyright Contributors to the Zowe Project. + */ + +package org.zowe.apiml.gateway.filters; + +import com.fasterxml.jackson.core.JsonGenerationException; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpMethod; +import org.springframework.mock.http.server.reactive.MockServerHttpRequest; +import org.springframework.mock.web.server.MockServerWebExchange; +import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.web.server.i18n.LocaleContextResolver; +import org.zowe.apiml.message.api.ApiMessageView; +import org.zowe.apiml.message.core.MessageService; +import org.zowe.apiml.message.yaml.YamlMessageService; +import reactor.core.publisher.Mono; + +import java.net.URI; +import java.net.URISyntaxException; + +import static org.apache.hc.core5.http.HttpStatus.SC_BAD_REQUEST; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; + +class ForbidEncodedCharactersFilterFactoryTest { + + private static final String ENCODED_REQUEST_URI = "/api/v1/encoded;ch%25rs"; + private static final String ENCODED_REQUEST_URI_WITH_BACKSLASH = "/api/v1/enc\\oded;ch%25rs"; + private static final String NORMAL_REQUEST_URI = "/api/v1/normal"; + private final ObjectMapper objectMapperError = spy(new ObjectMapper()); + private ForbidEncodedCharactersFilterFactory filter; + + @BeforeEach + public void setUp() { + MessageService messageService = new YamlMessageService("/gateway-log-messages.yml"); + filter = new ForbidEncodedCharactersFilterFactory( + messageService, objectMapperError, mock(LocaleContextResolver.class) + ); + } + + @Nested + class Responses { + @Test + void givenNormalRequestUri_whenFilterApply_thenSuccess() { + MockServerHttpRequest request = MockServerHttpRequest + .get(NORMAL_REQUEST_URI) + .build(); + MockServerWebExchange exchange = MockServerWebExchange.from(request); + + var response = filter.apply("").filter(exchange, e -> { + exchange.getResponse().setRawStatusCode(200); + return Mono.empty(); + }); + response.block(); + assertTrue(exchange.getResponse().getStatusCode().is2xxSuccessful()); + } + + @Test + void givenRequestUriWithEncodedCharacters_whenFilterApply_thenReturnBadRequest() throws JsonProcessingException, URISyntaxException { + // A little hack to test request URI with backslashes, otherwise parser in URI.class will throw an exception + URI uri = new URI(ENCODED_REQUEST_URI); // creating URI without backslashes + ReflectionTestUtils.setField(uri, "path", ENCODED_REQUEST_URI_WITH_BACKSLASH); // resetting the path with backslashes + MockServerHttpRequest request = MockServerHttpRequest + .method(HttpMethod.GET, uri) + .build(); + MockServerWebExchange exchange = MockServerWebExchange.from(request); + + var response = filter.apply("").filter(exchange, e -> Mono.empty()); + response.block(); + assertEquals(SC_BAD_REQUEST, exchange.getResponse().getStatusCode().value()); + String body = exchange.getResponse().getBodyAsString().block(); + var message = objectMapperError.readValue(body, ApiMessageView.class); + assertEquals("org.zowe.apiml.gateway.requestContainEncodedCharacter", message.getMessages().get(0).getMessageKey()); + } + } + + @Nested + class Errors { + + @Test + void givenRequestUriWithEncodedCharacters_whenJsonProcessorThrowsAnException_thenThrownRuntimeException() throws JsonProcessingException, URISyntaxException { + MockServerHttpRequest request = MockServerHttpRequest + .method(HttpMethod.GET, new URI(ENCODED_REQUEST_URI)) + .build(); + MockServerWebExchange exchange = MockServerWebExchange.from(request); + + doThrow(new JsonGenerationException("error")).when(objectMapperError).writeValueAsBytes(any()); + RuntimeException er = assertThrows(RuntimeException.class, () -> filter.apply("").filter(exchange, e -> Mono.empty())); + assertEquals("com.fasterxml.jackson.core.JsonGenerationException: error", er.getMessage()); + } + + } + +} diff --git a/gateway-service/src/test/java/org/zowe/apiml/gateway/filters/ForbidEncodedSlashesFilterFactoryTest.java b/gateway-service/src/test/java/org/zowe/apiml/gateway/filters/ForbidEncodedSlashesFilterFactoryTest.java index 933840a73c..7a05911213 100644 --- a/gateway-service/src/test/java/org/zowe/apiml/gateway/filters/ForbidEncodedSlashesFilterFactoryTest.java +++ b/gateway-service/src/test/java/org/zowe/apiml/gateway/filters/ForbidEncodedSlashesFilterFactoryTest.java @@ -17,6 +17,7 @@ import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.http.HttpMethod; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import org.springframework.mock.web.server.MockServerWebExchange; import org.springframework.test.context.TestPropertySource; @@ -26,8 +27,13 @@ import org.zowe.apiml.message.core.MessageService; import reactor.core.publisher.Mono; +import java.net.URI; +import java.net.URISyntaxException; + import static org.apache.hc.core5.http.HttpStatus.SC_BAD_REQUEST; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.*; @SpringBootTest @@ -35,23 +41,23 @@ class ForbidEncodedSlashesFilterFactoryTest { private static final String ENCODED_REQUEST_URI = "/api/v1/encoded%2fslash"; private static final String NORMAL_REQUEST_URI = "/api/v1/normal"; - - private final ObjectMapper objectMapper = new ObjectMapper(); + private final ObjectMapper objectMapperError = spy(new ObjectMapper()); @Nested - @TestPropertySource(properties = "apiml.service.allowEncodedSlashes=true") + @TestPropertySource(properties = "apiml.service.allowEncodedSlashes=false") class Responses { @Autowired - ForbidEncodedSlashesFilterFactory gatewayFiler; + ForbidEncodedSlashesFilterFactory filter; @Test - void whenUrlDoesNotContainEncodedCharacters() { + void givenNormalRequestUri_whenFilterApply_thenSuccess() { MockServerHttpRequest request = MockServerHttpRequest .get(NORMAL_REQUEST_URI) .build(); MockServerWebExchange exchange = MockServerWebExchange.from(request); - var response = gatewayFiler.apply("").filter(exchange, exchange2 -> { + + var response = filter.apply("").filter(exchange, e -> { exchange.getResponse().setRawStatusCode(200); return Mono.empty(); }); @@ -60,16 +66,17 @@ void whenUrlDoesNotContainEncodedCharacters() { } @Test - void whenUrlContainsEncodedCharacters() throws JsonProcessingException { + void givenRequestUriWithEncodedSlashes_whenFilterApply_thenReturnBadRequest() throws JsonProcessingException, URISyntaxException { MockServerHttpRequest request = MockServerHttpRequest - .get(ENCODED_REQUEST_URI) + .method(HttpMethod.GET, new URI(ENCODED_REQUEST_URI)) .build(); MockServerWebExchange exchange = MockServerWebExchange.from(request); - var response = gatewayFiler.apply("").filter(exchange, exchange2 -> Mono.empty()); + + var response = filter.apply("").filter(exchange, e -> Mono.empty()); response.block(); assertEquals(SC_BAD_REQUEST, exchange.getResponse().getStatusCode().value()); String body = exchange.getResponse().getBodyAsString().block(); - var message = objectMapper.readValue(body, ApiMessageView.class); + var message = objectMapperError.readValue(body, ApiMessageView.class); assertEquals("org.zowe.apiml.gateway.requestContainEncodedSlash", message.getMessages().get(0).getMessageKey()); } @@ -79,21 +86,19 @@ void whenUrlContainsEncodedCharacters() throws JsonProcessingException { class Errors { @Test - void whenJsonProcessorThrowsAnException() throws JsonProcessingException { + void givenRequestUriWithEncodedCharacters_whenJsonProcessorThrowsAnException_thenThrownRuntimeException() throws JsonProcessingException, URISyntaxException { MessageService messageService = mock(MessageService.class); doReturn(mock(Message.class)).when(messageService).createMessage(any(), (Object[]) any()); - ObjectMapper objectMapperError = spy(objectMapper); ForbidEncodedSlashesFilterFactory filter = new ForbidEncodedSlashesFilterFactory( messageService, objectMapperError, mock(LocaleContextResolver.class) ); MockServerHttpRequest request = MockServerHttpRequest - .get(ENCODED_REQUEST_URI) + .method(HttpMethod.GET, new URI(ENCODED_REQUEST_URI)) .build(); MockServerWebExchange exchange = MockServerWebExchange.from(request); doThrow(new JsonGenerationException("error")).when(objectMapperError).writeValueAsBytes(any()); - RuntimeException er = assertThrows(RuntimeException.class, () -> filter.apply("").filter(exchange, e -> Mono.empty())); assertEquals("com.fasterxml.jackson.core.JsonGenerationException: error", er.getMessage()); } diff --git a/gateway-service/src/test/java/org/zowe/apiml/gateway/service/RouteLocatorTest.java b/gateway-service/src/test/java/org/zowe/apiml/gateway/service/RouteLocatorTest.java index 30579389bd..1534b038be 100644 --- a/gateway-service/src/test/java/org/zowe/apiml/gateway/service/RouteLocatorTest.java +++ b/gateway-service/src/test/java/org/zowe/apiml/gateway/service/RouteLocatorTest.java @@ -38,6 +38,7 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; import static org.zowe.apiml.constants.EurekaMetadataDefinition.APIML_ID; +import static org.zowe.apiml.constants.EurekaMetadataDefinition.ENABLE_URL_ENCODED_CHARACTERS; import static org.zowe.apiml.constants.EurekaMetadataDefinition.SERVICE_SUPPORTING_CLIENT_CERT_FORWARDING; class RouteLocatorTest { @@ -267,16 +268,21 @@ class PostRoutingFilterDefinition { private final List COMMON_FILTERS = Collections.singletonList(mock(FilterDefinition.class)); private final RouteLocator routeLocator = new RouteLocator(null, null, null, COMMON_FILTERS, Collections.emptyList(), null); - private ServiceInstance createServiceInstance(Boolean forwardingEnabled) { + private ServiceInstance createServiceInstance(Boolean forwardingEnabled, Boolean encodedCharactersEnabled) { Map metadata = new HashMap<>(); if (forwardingEnabled != null) { metadata.put(SERVICE_SUPPORTING_CLIENT_CERT_FORWARDING, String.valueOf(forwardingEnabled)); } + if (encodedCharactersEnabled != null) { + metadata.put(ENABLE_URL_ENCODED_CHARACTERS, String.valueOf(encodedCharactersEnabled)); + } ServiceInstance serviceInstance = mock(ServiceInstance.class); doReturn(metadata).when(serviceInstance).getMetadata(); return serviceInstance; } + + @Nested class EnabledForwarding { @@ -287,16 +293,16 @@ void enableForwarding() { @Test void givenServiceAllowingCertForwarding_whenGetPostRoutingFilters_thenAddClientCertFilterFactory() { - ServiceInstance serviceInstance = createServiceInstance(Boolean.TRUE); + ServiceInstance serviceInstance = createServiceInstance(Boolean.TRUE, null); List filterDefinitions = routeLocator.getPostRoutingFilters(serviceInstance); - assertEquals(3, filterDefinitions.size()); // common filters + pageredirectionfilter + assertEquals(3, filterDefinitions.size()); // common filters + PageRedirectionFilterFactory assertEquals("ForwardClientCertFilterFactory", filterDefinitions.get(1).getName()); } @Test void givenServiceNotAllowingCertForwarding_whenGetPostRoutingFilters_thenReturnJustCommon() { - ServiceInstance serviceInstance = createServiceInstance(Boolean.FALSE); + ServiceInstance serviceInstance = createServiceInstance(Boolean.FALSE, null); List filterDefinitions = routeLocator.getPostRoutingFilters(serviceInstance); assertTrue(filterDefinitions.containsAll(COMMON_FILTERS), "Not all common filters are defined"); @@ -307,7 +313,7 @@ void givenServiceNotAllowingCertForwarding_whenGetPostRoutingFilters_thenReturnJ @Test void givenServiceWithoutCertForwardingConfig_whenGetPostRoutingFilters_thenReturnJustCommon() { - ServiceInstance serviceInstance = createServiceInstance(null); + ServiceInstance serviceInstance = createServiceInstance(null, null); List filterDefinitions = routeLocator.getPostRoutingFilters(serviceInstance); assertTrue(filterDefinitions.containsAll(COMMON_FILTERS), "Not all common filters are defined"); @@ -327,7 +333,7 @@ void disableForwarding() { @Test void givenAnyService_whenGetPostRoutingFilters_thenReturnJustCommon() { - ServiceInstance serviceInstance = createServiceInstance(Boolean.TRUE); + ServiceInstance serviceInstance = createServiceInstance(Boolean.TRUE, null); List filterDefinitions = routeLocator.getPostRoutingFilters(serviceInstance); assertTrue(filterDefinitions.containsAll(COMMON_FILTERS), "Not all common filters are defined"); @@ -337,6 +343,37 @@ void givenAnyService_whenGetPostRoutingFilters_thenReturnJustCommon() { } + @Nested + class EncodedCharacters { + + @Test + void givenServiceAllowingEncodedCharacters_whenGetPostRoutingFilters_thenReturnJustCommon() { + ServiceInstance serviceInstance = createServiceInstance(null, Boolean.TRUE); + List filterDefinitions = routeLocator.getPostRoutingFilters(serviceInstance); + assertTrue(filterDefinitions.containsAll(COMMON_FILTERS), "Not all common filters are defined"); + assertEquals(2, filterDefinitions.size()); + assertTrue(filterDefinitions.stream().noneMatch(filter -> "ForbidEncodedCharactersFilterFactory".equals(filter.getName()))); + } + + @Test + void givenServiceNotAllowingEncodedCharacters_whenGetPostRoutingFilters_thenAddEncodedCharacterFilterFactory() { + ServiceInstance serviceInstance = createServiceInstance(null, Boolean.FALSE); + List filterDefinitions = routeLocator.getPostRoutingFilters(serviceInstance); + assertEquals(3, filterDefinitions.size()); + assertEquals("ForbidEncodedCharactersFilterFactory", filterDefinitions.get(1).getName()); + } + + @Test + void givenServiceWithoutAllowingEncodedCharacters_whenGetPostRoutingFilters_thenAddEncodedCharacterFilterFactory() { + ServiceInstance serviceInstance = createServiceInstance(null, null); + List filterDefinitions = routeLocator.getPostRoutingFilters(serviceInstance); + assertTrue(filterDefinitions.containsAll(COMMON_FILTERS), "Not all common filters are defined"); + assertEquals(2, filterDefinitions.size()); + assertTrue(filterDefinitions.stream().noneMatch(filter -> "ForbidEncodedCharactersFilterFactory".equals(filter.getName()))); + } + + } + } } diff --git a/zaas-service/src/main/java/org/zowe/apiml/zaas/security/config/NewSecurityConfiguration.java b/zaas-service/src/main/java/org/zowe/apiml/zaas/security/config/NewSecurityConfiguration.java index 24cda4970f..c3bce5def3 100644 --- a/zaas-service/src/main/java/org/zowe/apiml/zaas/security/config/NewSecurityConfiguration.java +++ b/zaas-service/src/main/java/org/zowe/apiml/zaas/security/config/NewSecurityConfiguration.java @@ -37,7 +37,6 @@ import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.authentication.logout.HttpStatusReturningLogoutSuccessHandler; import org.springframework.security.web.authentication.logout.LogoutHandler; -import org.springframework.security.web.firewall.StrictHttpFirewall; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.zowe.apiml.filter.AttlsFilter; import org.zowe.apiml.filter.SecureConnectionFilter; @@ -596,15 +595,7 @@ class DefaultSecurity { // Web security only needs to be configured once, putting it to multiple filter chains causes multiple evaluations of the same rules @Bean public WebSecurityCustomizer webSecurityCustomizer() { - StrictHttpFirewall firewall = new StrictHttpFirewall(); - firewall.setAllowUrlEncodedSlash(true); - firewall.setAllowBackSlash(true); - firewall.setAllowUrlEncodedPercent(true); - firewall.setAllowUrlEncodedPeriod(true); - firewall.setAllowSemicolon(true); - return web -> { - web.httpFirewall(firewall); if (!isHealthEndpointProtected) { web.ignoring().requestMatchers("/application/health"); }