From cd3fd6762f621f8d36ff5792cc427d1609dd3540 Mon Sep 17 00:00:00 2001 From: Erik Bakker Date: Thu, 4 Jun 2020 11:34:00 +0200 Subject: [PATCH] Don't Consume Request Body Per the servlet spec, getParameter(name) consumes the request body for POST requests. This commit prevents DefaultOAuth2AuthorizationRequestResolver from consuming the request body for non-Authorization requests. Closes gh-8650 --- ...ultOAuth2AuthorizationRequestResolver.java | 3 +++ ...uth2AuthorizationRequestResolverTests.java | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java index a97db4a26b..8060660f2c 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java @@ -87,6 +87,9 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au @Override public OAuth2AuthorizationRequest resolve(HttpServletRequest request) { String registrationId = this.resolveRegistrationId(request); + if (registrationId == null) { + return null; + } String redirectUriAction = getAction(request, "login"); return resolve(request, registrationId, redirectUriAction); } diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java index 2f1f315101..f34f40046b 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java @@ -15,8 +15,12 @@ */ package org.springframework.security.oauth2.client.web; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import javax.servlet.http.HttpServletRequest; import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.security.oauth2.client.registration.ClientRegistration; import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; @@ -99,6 +103,24 @@ public class DefaultOAuth2AuthorizationRequestResolverTests { assertThat(authorizationRequest).isNull(); } + @Test + public void resolveWhenNotAuthorizationRequestThenRequestBodyNotConsumed() throws IOException { + String requestUri = "/path"; + MockHttpServletRequest request = new MockHttpServletRequest("POST", requestUri); + request.setContent("foo".getBytes(StandardCharsets.UTF_8)); + request.setCharacterEncoding(StandardCharsets.UTF_8.name()); + HttpServletRequest spyRequest = Mockito.spy(request); + + this.resolver.resolve(spyRequest); + + Mockito.verify(spyRequest, Mockito.never()).getReader(); + Mockito.verify(spyRequest, Mockito.never()).getInputStream(); + Mockito.verify(spyRequest, Mockito.never()).getParameter(Mockito.anyString()); + Mockito.verify(spyRequest, Mockito.never()).getParameterMap(); + Mockito.verify(spyRequest, Mockito.never()).getParameterNames(); + Mockito.verify(spyRequest, Mockito.never()).getParameterValues(Mockito.anyString()); + } + @Test public void resolveWhenAuthorizationRequestWithInvalidClientThenThrowIllegalArgumentException() { ClientRegistration clientRegistration = this.registration1;