From 0f92415395c99ff2c7cb79896123356873c5046a Mon Sep 17 00:00:00 2001 From: Parikshit Dutta Date: Sat, 2 May 2020 00:07:33 +0530 Subject: [PATCH 1/2] Fix non-standard HTTP method for CsrfWebFilter Closes gh-8149 --- .../web/server/csrf/CsrfWebFilter.java | 5 +++-- .../web/server/csrf/CsrfWebFilterTests.java | 19 ++++++++++++++++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/server/csrf/CsrfWebFilter.java b/web/src/main/java/org/springframework/security/web/server/csrf/CsrfWebFilter.java index 33d06d39da..64dcc1b6af 100644 --- a/web/src/main/java/org/springframework/security/web/server/csrf/CsrfWebFilter.java +++ b/web/src/main/java/org/springframework/security/web/server/csrf/CsrfWebFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -60,6 +60,7 @@ import static java.lang.Boolean.TRUE; *

* * @author Rob Winch + * @author Parikshit Dutta * @since 5.0 */ public class CsrfWebFilter implements WebFilter { @@ -187,7 +188,7 @@ public class CsrfWebFilter implements WebFilter { @Override public Mono matches(ServerWebExchange exchange) { return Mono.just(exchange.getRequest()) - .map(r -> r.getMethod()) + .flatMap(r -> Mono.justOrEmpty(r.getMethod())) .filter(m -> ALLOWED_METHODS.contains(m)) .flatMap(m -> MatchResult.notMatch()) .switchIfEmpty(MatchResult.match()); diff --git a/web/src/test/java/org/springframework/security/web/server/csrf/CsrfWebFilterTests.java b/web/src/test/java/org/springframework/security/web/server/csrf/CsrfWebFilterTests.java index cd3df76693..ad9ea7e89a 100644 --- a/web/src/test/java/org/springframework/security/web/server/csrf/CsrfWebFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/server/csrf/CsrfWebFilterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,8 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; + +import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; @@ -40,11 +42,14 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.spy; import static org.springframework.mock.web.server.MockServerWebExchange.from; import static org.springframework.web.reactive.function.BodyInserters.fromMultipartData; /** * @author Rob Winch + * @author Parikshit Dutta * @since 5.0 */ @RunWith(MockitoJUnitRunner.class) @@ -183,6 +188,18 @@ public class CsrfWebFilterTests { chainResult.assertWasSubscribed(); } + @Test + public void matchesRequireCSRFProtectionWhenNonStandardHTTPMethodIsUsed() { + final String NON_STANDARD_HTTP_METHOD = "non-standard-http-method"; + MockServerWebExchange nonStandardHttpRequest = from(MockServerHttpRequest.method(HttpMethod.resolve(NON_STANDARD_HTTP_METHOD), "/")); + + ServerWebExchangeMatcher serverWebExchangeMatcher = spy(CsrfWebFilter.DEFAULT_CSRF_MATCHER); + serverWebExchangeMatcher.matches(nonStandardHttpRequest); + + verify(serverWebExchangeMatcher).matches(nonStandardHttpRequest); + assertThat(serverWebExchangeMatcher.matches(nonStandardHttpRequest).block().isMatch()).isTrue(); + } + @Test public void doFilterWhenSkipExchangeInvokedThenSkips() { PublisherProbe chainResult = PublisherProbe.empty(); From 4473dca02201c1a296ff1eda5a1c322a2275dfe1 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Mon, 11 May 2020 16:41:59 -0500 Subject: [PATCH 2/2] Polish matchesRequireCsrfProtectionWhenNonStandardHTTPMethodIsUsed Issue gh-8149 --- .../web/server/csrf/CsrfWebFilterTests.java | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/web/src/test/java/org/springframework/security/web/server/csrf/CsrfWebFilterTests.java b/web/src/test/java/org/springframework/security/web/server/csrf/CsrfWebFilterTests.java index ad9ea7e89a..94d0334e18 100644 --- a/web/src/test/java/org/springframework/security/web/server/csrf/CsrfWebFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/server/csrf/CsrfWebFilterTests.java @@ -27,6 +27,7 @@ import org.springframework.http.MediaType; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import org.springframework.mock.web.server.MockServerWebExchange; import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher; +import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher.MatchResult; import org.springframework.test.web.reactive.server.WebTestClient; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; @@ -42,8 +43,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.spy; import static org.springframework.mock.web.server.MockServerWebExchange.from; import static org.springframework.web.reactive.function.BodyInserters.fromMultipartData; @@ -189,15 +188,13 @@ public class CsrfWebFilterTests { } @Test - public void matchesRequireCSRFProtectionWhenNonStandardHTTPMethodIsUsed() { - final String NON_STANDARD_HTTP_METHOD = "non-standard-http-method"; - MockServerWebExchange nonStandardHttpRequest = from(MockServerHttpRequest.method(HttpMethod.resolve(NON_STANDARD_HTTP_METHOD), "/")); + // gh-8452 + public void matchesRequireCsrfProtectionWhenNonStandardHTTPMethodIsUsed() { + HttpMethod customHttpMethod = HttpMethod.resolve("non-standard-http-method"); + MockServerWebExchange nonStandardHttpRequest = from(MockServerHttpRequest.method(customHttpMethod, "/")); - ServerWebExchangeMatcher serverWebExchangeMatcher = spy(CsrfWebFilter.DEFAULT_CSRF_MATCHER); - serverWebExchangeMatcher.matches(nonStandardHttpRequest); - - verify(serverWebExchangeMatcher).matches(nonStandardHttpRequest); - assertThat(serverWebExchangeMatcher.matches(nonStandardHttpRequest).block().isMatch()).isTrue(); + ServerWebExchangeMatcher serverWebExchangeMatcher = CsrfWebFilter.DEFAULT_CSRF_MATCHER; + assertThat(serverWebExchangeMatcher.matches(nonStandardHttpRequest).map(MatchResult::isMatch).block()).isTrue(); } @Test