BREAKING CHANGE: do not allow simple requests for login

apiml-security-common/src/main/java/org/zowe/apiml/security/common/filter/ContentTypeFilter.java

@@ -0,0 +1,60 @@
+/*
+ * 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.security.common.filter;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import lombok.NonNull;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.InvalidMediaTypeException;
+import org.springframework.http.MediaType;
+import org.springframework.web.filter.OncePerRequestFilter;
+
+import java.io.IOException;
+
+/**
+ * Rejects requests that carry a body with HTTP 415 unless they declare a {@code Content-Type}
+ * equals to {@code application/json}. Bodyless requests (e.g. cert or Basic-Auth login,
+ * logout) pass through unchecked, since they have nothing to be misinterpreted as JSON.
+ * <p>
+ * Body presence is determined from the {@code Content-Length}/{@code Transfer-Encoding} headers
+ * rather than by reading the request body, since the body stream is only safely readable once
+ * downstream (e.g. by {@code LoginFilter}).
+ */
+public class ContentTypeFilter extends OncePerRequestFilter {
+
+    @Override
+    protected void doFilterInternal(@NonNull HttpServletRequest request, @NonNull HttpServletResponse response, @NonNull FilterChain filterChain) throws ServletException, IOException {
+        boolean hasBody = request.getContentLengthLong() > 0 || request.getHeader(HttpHeaders.TRANSFER_ENCODING) != null;
+        if (hasBody && !hasJsonContentType(request)) {
+            response.setStatus(HttpStatus.UNSUPPORTED_MEDIA_TYPE.value());
+            return;
+        }
+        filterChain.doFilter(request, response);
+    }
+
+    private boolean hasJsonContentType(HttpServletRequest request) {
+        String contentType = request.getContentType();
+        if (contentType == null) {
+            return false;
+        }
+        try {
+           MediaType mediaType = MediaType.parseMediaType(contentType);
+            return MediaType.APPLICATION_JSON.equalsTypeAndSubtype(mediaType);
+        } catch (InvalidMediaTypeException e) {
+            return false;
+        }
+    }
+
+}

apiml-security-common/src/test/java/org/zowe/apiml/security/common/filter/ContentTypeFilterTest.java

@@ -0,0 +1,107 @@
+/*
+ * 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.security.common.filter;
+
+import jakarta.servlet.ServletException;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
+import org.springframework.mock.web.MockFilterChain;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+class ContentTypeFilterTest {
+
+    private ContentTypeFilter filter;
+    private MockHttpServletResponse response;
+    private MockFilterChain chain;
+
+    @BeforeEach
+    void setUp() {
+        filter = new ContentTypeFilter();
+        response = new MockHttpServletResponse();
+        chain = new MockFilterChain();
+    }
+
+    @Nested
+    class WhenRequestHasBody {
+
+        @Test
+        void givenJsonContentType_thenRequestProceeds() throws ServletException, IOException {
+            var request = new MockHttpServletRequest("POST", "/auth/login");
+            request.setContentType(MediaType.APPLICATION_JSON_VALUE);
+            request.setContent("{}".getBytes(StandardCharsets.UTF_8));
+
+            filter.doFilter(request, response, chain);
+
+            assertEquals(request, chain.getRequest());
+        }
+
+        @Test
+        void givenNoContentType_thenRejectedWithUnsupportedMediaType() throws ServletException, IOException {
+            var request = new MockHttpServletRequest("POST", "/auth/login");
+            request.setContent("{}".getBytes(StandardCharsets.UTF_8));
+
+            filter.doFilter(request, response, chain);
+
+            assertEquals(HttpStatus.UNSUPPORTED_MEDIA_TYPE.value(), response.getStatus());
+            assertNull(chain.getRequest());
+        }
+
+        @Test
+        void givenNonJsonContentType_thenRejectedWithUnsupportedMediaType() throws ServletException, IOException {
+            var request = new MockHttpServletRequest("POST", "/auth/login");
+            request.setContentType(MediaType.APPLICATION_FORM_URLENCODED_VALUE);
+            request.setContent("username=a&password=b".getBytes(StandardCharsets.UTF_8));
+
+            filter.doFilter(request, response, chain);
+
+            assertEquals(HttpStatus.UNSUPPORTED_MEDIA_TYPE.value(), response.getStatus());
+            assertNull(chain.getRequest());
+        }
+
+        @Test
+        void givenChunkedTransferEncodingAndNoContentType_thenRejectedWithUnsupportedMediaType() throws ServletException, IOException {
+            var request = new MockHttpServletRequest("POST", "/auth/login");
+            request.addHeader(HttpHeaders.TRANSFER_ENCODING, "chunked");
+
+            filter.doFilter(request, response, chain);
+
+            assertEquals(HttpStatus.UNSUPPORTED_MEDIA_TYPE.value(), response.getStatus());
+            assertNull(chain.getRequest());
+        }
+
+    }
+
+    @Nested
+    class WhenRequestHasNoBody {
+
+        @Test
+        void givenNoContentTypeAndNoBody_thenRequestProceeds() throws ServletException, IOException {
+            var request = new MockHttpServletRequest("POST", "/auth/logout");
+
+            filter.doFilter(request, response, chain);
+
+            assertEquals(request, chain.getRequest());
+        }
+
+    }
+
+}

apiml/src/main/java/org/zowe/apiml/WebSecurityConfig.java

@@ -24,11 +24,7 @@
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
-import org.springframework.security.authentication.BadCredentialsException;
-import org.springframework.security.authentication.ProviderManager;
-import org.springframework.security.authentication.ReactiveAuthenticationManager;
-import org.springframework.security.authentication.ReactiveAuthenticationManagerAdapter;
-import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
+import org.springframework.security.authentication.*;
 import org.springframework.security.config.annotation.method.configuration.EnableReactiveMethodSecurity;
 import org.springframework.security.config.annotation.web.reactive.EnableWebFluxSecurity;
 import org.springframework.security.config.web.server.SecurityWebFiltersOrder;
@@ -44,12 +40,7 @@
 import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher;
 import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher.MatchResult;
 import org.zowe.apiml.constants.ApimlConstants;
-import org.zowe.apiml.filter.BasicLoginFilter;
-import org.zowe.apiml.filter.CachedBodyFilter;
-import org.zowe.apiml.filter.LogoutHandler;
-import org.zowe.apiml.filter.OIDCAuthFilter;
-import org.zowe.apiml.filter.QueryWebFilter;
-import org.zowe.apiml.filter.X509AuthFilter;
+import org.zowe.apiml.filter.*;
 import org.zowe.apiml.gateway.filters.security.AuthExceptionHandlerReactive;
 import org.zowe.apiml.gateway.filters.security.TokenAuthFilter;
 import org.zowe.apiml.handler.FailedAuthenticationWebHandler;
@@ -73,9 +64,7 @@
 import java.util.List;
 import java.util.Set;
 
-import static org.springframework.http.HttpMethod.DELETE;
-import static org.springframework.http.HttpMethod.GET;
-import static org.springframework.http.HttpMethod.POST;
+import static org.springframework.http.HttpMethod.*;
 import static org.springframework.security.web.server.util.matcher.ServerWebExchangeMatchers.pathMatchers;
 import static org.zowe.apiml.gateway.services.ServicesInfoController.SERVICES_FULL_URL;
 import static org.zowe.apiml.gateway.services.ServicesInfoController.SERVICES_SHORT_URL;
@@ -436,6 +425,7 @@ SecurityWebFilterChain loginAndLogoutSecurityWebFilterChain(ServerHttpSecurity h
                 .logoutSuccessHandler(new HttpStatusReturningServerLogoutSuccessHandler(HttpStatus.NO_CONTENT)))
             .httpBasic(ServerHttpSecurity.HttpBasicSpec::disable)
             .addFilterAfter(new CachedBodyFilter(), SecurityWebFiltersOrder.FIRST)
+            .addFilterAfter(new ContentTypeFilter(), SecurityWebFiltersOrder.FIRST)
             .addFilterAfter(new CategorizeCertsWebFilter(publicKeyCertificatesBase64, certificateValidator), SecurityWebFiltersOrder.FIRST)
             .addFilterAfter(new BasicLoginFilter(compoundAuthProvider, failedAuthenticationWebHandler), SecurityWebFiltersOrder.AUTHENTICATION)
             .addFilterAfter(new X509AuthFilter(reactiveX509provider), SecurityWebFiltersOrder.AUTHENTICATION);

apiml/src/main/java/org/zowe/apiml/filter/CachedBodyFilter.java

@@ -39,25 +39,23 @@ public class CachedBodyFilter implements WebFilter {
     @Override
     public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
         return DataBufferUtils.join(exchange.getRequest().getBody())
-            .flatMap(dataBuffer -> {
+            .map(dataBuffer -> {
                 var bytes = new byte[dataBuffer.readableByteCount()];
                 dataBuffer.read(bytes);
                 DataBufferUtils.release(dataBuffer);
                 exchange.getAttributes().put(CACHED_BODY_ATTR, new String(bytes, StandardCharsets.UTF_8));
+                return bytes;
+            })
+            .defaultIfEmpty(new byte[0])
+            .flatMap(bytes -> {
                 var decoratedRequest = new ServerHttpRequestDecorator(exchange.getRequest()) {
                     @Override
                     public Flux<DataBuffer> getBody() {
                         return Flux.just(exchange.getResponse().bufferFactory().wrap(bytes));
                     }
                 };
                 return chain.filter(exchange.mutate().request(decoratedRequest).build());
-            })
-            .switchIfEmpty(chain.filter(exchange.mutate().request(new ServerHttpRequestDecorator(exchange.getRequest()) {
-                @Override
-                    public Flux<DataBuffer> getBody() {
-                        return Flux.just(exchange.getResponse().bufferFactory().wrap(new byte[]{}));
-                    }
-            }).build()));
+            });
     }
 
 }

apiml/src/main/java/org/zowe/apiml/filter/ContentTypeFilter.java

@@ -0,0 +1,50 @@
+/*
+ * 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.filter;
+
+import org.springframework.core.Ordered;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
+import org.springframework.web.server.ServerWebExchange;
+import org.springframework.web.server.WebFilter;
+import org.springframework.web.server.WebFilterChain;
+import reactor.core.publisher.Mono;
+
+/**
+ * Rejects requests that carry a body with HTTP 415 unless they declare a {@code Content-Type}
+ * equals to {@code application/json}. Bodyless requests (e.g. cert or Basic-Auth login,
+ * logout) pass through unchecked, since they have nothing to be misinterpreted as JSON.
+ * <p>
+ * Must run after {@link CachedBodyFilter}: whether a body was actually sent is determined from
+ * {@link CachedBodyFilter#CACHED_BODY_ATTR} rather than the {@code Content-Length} header, which
+ * is absent for chunked/proxied requests.
+ */
+public class ContentTypeFilter implements WebFilter, Ordered {
+
+    @Override
+    public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
+        boolean hasBody = exchange.getAttribute(CachedBodyFilter.CACHED_BODY_ATTR) != null;
+        if (hasBody) {
+            MediaType contentType = exchange.getRequest().getHeaders().getContentType();
+            if (contentType == null || !MediaType.APPLICATION_JSON.equalsTypeAndSubtype(contentType)) {
+                exchange.getResponse().setStatusCode(HttpStatus.UNSUPPORTED_MEDIA_TYPE);
+                return exchange.getResponse().setComplete();
+            }
+        }
+        return chain.filter(exchange);
+    }
+
+    @Override
+    public int getOrder() {
+        return Ordered.HIGHEST_PRECEDENCE + 1;
+    }
+
+}

apiml/src/main/resources/application.yml

@@ -206,6 +206,8 @@ apiml:
         securePortEnabled: true
         externalUrl: ${apiml.service.scheme}://${apiml.service.hostname}:${apiml.service.port}
     security:
+        csrf:
+            preserveOriginForCrossSite: true
         headersToBeCleared: X-Certificate-Public,X-Certificate-DistinguishedName,X-Certificate-CommonName
         ssl:
             nonStrictVerifySslCertificatesOfServices: false

apiml/src/test/java/org/zowe/apiml/filter/CachedBodyFilterTest.java

@@ -27,6 +27,8 @@
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 @ExtendWith(MockitoExtension.class)
@@ -55,6 +57,11 @@ void givenBody_thenBodyIsAvailableAndRequestReadable() {
             StepVerifier.create(filter.filter(exchange, chain))
                 .verifyComplete();
 
+            // The downstream chain (and thus the controller) must run exactly once, even
+            // though chain.filter(...) returns Mono<Void>. Otherwise a request with a body
+            // is processed twice: once with the cached body and again with an empty body.
+            verify(chain, times(1)).filter(any());
+
             assertEquals("a readable body", exchange.getAttribute(CachedBodyFilter.CACHED_BODY_ATTR));
 
             StepVerifier.create(DataBufferUtils.join(exchange.getRequest().getBody()))
@@ -82,6 +89,8 @@ void givenEmptyBody_thenClean() {
             StepVerifier.create(filter.filter(exchange, chain))
                 .verifyComplete();
 
+            verify(chain, times(1)).filter(any());
+
             assertNull(exchange.getAttribute(CachedBodyFilter.CACHED_BODY_ATTR));
         }