Skip to content

Commit baa238e

Browse files
committed
Fix non-standard HTTP method for CsrfWebFilter
Closes gh-8452
1 parent cf5bd52 commit baa238e

File tree

2 files changed

+26
-9
lines changed

2 files changed

+26
-9
lines changed

web/src/main/java/org/springframework/security/web/server/csrf/CsrfWebFilter.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 the original author or authors.
2+
* Copyright 2002-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -54,6 +54,7 @@
5454
* </p>
5555
*
5656
* @author Rob Winch
57+
* @author Parikshit Dutta
5758
* @since 5.0
5859
*/
5960
public class CsrfWebFilter implements WebFilter {
@@ -133,7 +134,7 @@ private static class DefaultRequireCsrfProtectionMatcher implements ServerWebExc
133134
@Override
134135
public Mono<MatchResult> matches(ServerWebExchange exchange) {
135136
return Mono.just(exchange.getRequest())
136-
.map(r -> r.getMethod())
137+
.flatMap(r -> Mono.justOrEmpty(r.getMethod()))
137138
.filter(m -> ALLOWED_METHODS.contains(m))
138139
.flatMap(m -> MatchResult.notMatch())
139140
.switchIfEmpty(MatchResult.match());

web/src/test/java/org/springframework/security/web/server/csrf/CsrfWebFilterTests.java

+23-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 the original author or authors.
2+
* Copyright 2002-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -20,10 +20,14 @@
2020
import org.junit.runner.RunWith;
2121
import org.mockito.Mock;
2222
import org.mockito.junit.MockitoJUnitRunner;
23+
24+
import org.springframework.http.HttpMethod;
2325
import org.springframework.http.HttpStatus;
2426
import org.springframework.http.MediaType;
2527
import org.springframework.mock.http.server.reactive.MockServerHttpRequest;
2628
import org.springframework.mock.web.server.MockServerWebExchange;
29+
import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher;
30+
import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher.MatchResult;
2731
import org.springframework.web.server.WebFilterChain;
2832
import org.springframework.web.server.WebSession;
2933
import reactor.core.publisher.Mono;
@@ -33,9 +37,11 @@
3337
import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
3438
import static org.mockito.ArgumentMatchers.any;
3539
import static org.mockito.Mockito.when;
40+
import static org.springframework.mock.web.server.MockServerWebExchange.from;
3641

3742
/**
3843
* @author Rob Winch
44+
* @author Parikshit Dutta
3945
* @since 5.0
4046
*/
4147
@RunWith(MockitoJUnitRunner.class)
@@ -49,10 +55,10 @@ public class CsrfWebFilterTests {
4955

5056
private CsrfWebFilter csrfFilter = new CsrfWebFilter();
5157

52-
private MockServerWebExchange get = MockServerWebExchange.from(
58+
private MockServerWebExchange get = from(
5359
MockServerHttpRequest.get("/"));
5460

55-
private MockServerWebExchange post = MockServerWebExchange.from(
61+
private MockServerWebExchange post = from(
5662
MockServerHttpRequest.post("/"));
5763

5864
@Test
@@ -104,7 +110,7 @@ public void filterWhenPostAndEstablishedCsrfTokenAndRequestParamInvalidTokenThen
104110
this.csrfFilter.setCsrfTokenRepository(this.repository);
105111
when(this.repository.loadToken(any()))
106112
.thenReturn(Mono.just(this.token));
107-
this.post = MockServerWebExchange.from(MockServerHttpRequest.post("/")
113+
this.post = from(MockServerHttpRequest.post("/")
108114
.body(this.token.getParameterName() + "="+this.token.getToken()+"INVALID"));
109115

110116
Mono<Void> result = this.csrfFilter.filter(this.post, this.chain);
@@ -125,7 +131,7 @@ public void filterWhenPostAndEstablishedCsrfTokenAndRequestParamValidTokenThenCo
125131
.thenReturn(Mono.just(this.token));
126132
when(this.repository.generateToken(any()))
127133
.thenReturn(Mono.just(this.token));
128-
this.post = MockServerWebExchange.from(MockServerHttpRequest.post("/")
134+
this.post = from(MockServerHttpRequest.post("/")
129135
.contentType(MediaType.APPLICATION_FORM_URLENCODED)
130136
.body(this.token.getParameterName() + "="+this.token.getToken()));
131137

@@ -142,7 +148,7 @@ public void filterWhenPostAndEstablishedCsrfTokenAndHeaderInvalidTokenThenCsrfEx
142148
this.csrfFilter.setCsrfTokenRepository(this.repository);
143149
when(this.repository.loadToken(any()))
144150
.thenReturn(Mono.just(this.token));
145-
this.post = MockServerWebExchange.from(MockServerHttpRequest.post("/")
151+
this.post = from(MockServerHttpRequest.post("/")
146152
.header(this.token.getHeaderName(), this.token.getToken()+"INVALID"));
147153

148154
Mono<Void> result = this.csrfFilter.filter(this.post, this.chain);
@@ -163,7 +169,7 @@ public void filterWhenPostAndEstablishedCsrfTokenAndHeaderValidTokenThenContinue
163169
.thenReturn(Mono.just(this.token));
164170
when(this.repository.generateToken(any()))
165171
.thenReturn(Mono.just(this.token));
166-
this.post = MockServerWebExchange.from(MockServerHttpRequest.post("/")
172+
this.post = from(MockServerHttpRequest.post("/")
167173
.header(this.token.getHeaderName(), this.token.getToken()));
168174

169175
Mono<Void> result = this.csrfFilter.filter(this.post, this.chain);
@@ -173,4 +179,14 @@ public void filterWhenPostAndEstablishedCsrfTokenAndHeaderValidTokenThenContinue
173179

174180
chainResult.assertWasSubscribed();
175181
}
182+
183+
@Test
184+
// gh-8452
185+
public void matchesRequireCsrfProtectionWhenNonStandardHTTPMethodIsUsed() {
186+
HttpMethod customHttpMethod = HttpMethod.resolve("non-standard-http-method");
187+
MockServerWebExchange nonStandardHttpRequest = from(MockServerHttpRequest.method(customHttpMethod, "/"));
188+
189+
ServerWebExchangeMatcher serverWebExchangeMatcher = CsrfWebFilter.DEFAULT_CSRF_MATCHER;
190+
assertThat(serverWebExchangeMatcher.matches(nonStandardHttpRequest).map(MatchResult::isMatch).block()).isTrue();
191+
}
176192
}

0 commit comments

Comments
 (0)