From 8d57c893fb54525ffcd94ef9574a20b5a7dd066c Mon Sep 17 00:00:00 2001 From: Anoop Garlapati Date: Mon, 22 Feb 2021 19:35:08 +0530 Subject: [PATCH] Redirect URI validation for loopback address Modified redirect_uri validation as per OAuth 2.1 to accomodate for redirections on loopback address interface. Closes gh-243 --- .../OAuth2AuthorizationEndpointFilter.java | 50 +++++++++- ...Auth2AuthorizationEndpointFilterTests.java | 95 ++++++++++++++++++- 2 files changed, 142 insertions(+), 3 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java index e977a82a..2be1d5de 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java @@ -26,6 +26,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.regex.Pattern; import javax.servlet.FilterChain; import javax.servlet.ServletException; @@ -50,10 +51,10 @@ import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; import org.springframework.security.oauth2.core.endpoint.PkceParameterNames; import org.springframework.security.oauth2.core.oidc.OidcScopes; import org.springframework.security.oauth2.server.authorization.OAuth2Authorization; +import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationCode; import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationService; import org.springframework.security.oauth2.server.authorization.client.RegisteredClient; import org.springframework.security.oauth2.server.authorization.client.RegisteredClientRepository; -import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationCode; import org.springframework.security.web.DefaultRedirectStrategy; import org.springframework.security.web.RedirectStrategy; import org.springframework.security.web.util.matcher.AndRequestMatcher; @@ -66,6 +67,7 @@ import org.springframework.util.CollectionUtils; import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; import org.springframework.web.filter.OncePerRequestFilter; +import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; /** @@ -75,6 +77,7 @@ import org.springframework.web.util.UriComponentsBuilder; * @author Joe Grandja * @author Paurav Munshi * @author Daniel Garnier-Moiroux + * @author Anoop Garlapati * @since 0.0.1 * @see RegisteredClientRepository * @see OAuth2AuthorizationService @@ -91,6 +94,8 @@ public class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilter { private static final OAuth2TokenType STATE_TOKEN_TYPE = new OAuth2TokenType(OAuth2ParameterNames.STATE); private static final String PKCE_ERROR_URI = "https://tools.ietf.org/html/rfc7636#section-4.4.1"; + private static final Pattern LOOPBACK_ADDRESS_PATTERN = + Pattern.compile("^127(?:\\.[0-9]+){0,2}\\.[0-9]+$|^\\[(?:0*:)*?:?0*1]$"); private final RegisteredClientRepository registeredClientRepository; private final OAuth2AuthorizationService authorizationService; @@ -318,7 +323,7 @@ public class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilter { // redirect_uri (OPTIONAL) if (StringUtils.hasText(authorizationRequestContext.getRedirectUri())) { - if (!registeredClient.getRedirectUris().contains(authorizationRequestContext.getRedirectUri()) || + if (!isValidRedirectUri(authorizationRequestContext.getRedirectUri(), registeredClient) || authorizationRequestContext.getParameters().get(OAuth2ParameterNames.REDIRECT_URI).size() != 1) { authorizationRequestContext.setError( createError(OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ParameterNames.REDIRECT_URI)); @@ -483,6 +488,47 @@ public class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilter { principal.isAuthenticated(); } + private static boolean isValidRedirectUri(String requestedRedirectUri, RegisteredClient registeredClient) { + UriComponents requestedRedirect; + try { + requestedRedirect = UriComponentsBuilder.fromUriString(requestedRedirectUri).build(); + if (requestedRedirect.getFragment() != null) { + return false; + } + } catch (Exception ex) { + return false; + } + + String requestedRedirectHost = requestedRedirect.getHost(); + if (requestedRedirectHost == null || requestedRedirectHost.equals("localhost")) { + // As per https://tools.ietf.org/html/draft-ietf-oauth-v2-1-01#section-9.7.1 + // While redirect URIs using localhost (i.e., + // "http://localhost:{port}/{path}") function similarly to loopback IP + // redirects described in Section 10.3.3, the use of "localhost" is NOT RECOMMENDED. + return false; + } + if (!LOOPBACK_ADDRESS_PATTERN.matcher(requestedRedirectHost).matches()) { + // As per https://tools.ietf.org/html/draft-ietf-oauth-v2-1-01#section-9.7 + // When comparing client redirect URIs against pre-registered URIs, + // authorization servers MUST utilize exact string matching. + return registeredClient.getRedirectUris().contains(requestedRedirectUri); + } + + // As per https://tools.ietf.org/html/draft-ietf-oauth-v2-1-01#section-10.3.3 + // The authorization server MUST allow any port to be specified at the + // time of the request for loopback IP redirect URIs, to accommodate + // clients that obtain an available ephemeral port from the operating + // system at the time of the request. + for (String registeredRedirectUri : registeredClient.getRedirectUris()) { + UriComponentsBuilder registeredRedirect = UriComponentsBuilder.fromUriString(registeredRedirectUri); + registeredRedirect.port(requestedRedirect.getPort()); + if (registeredRedirect.build().toString().equals(requestedRedirect.toString())) { + return true; + } + } + return false; + } + private static class OAuth2AuthorizationRequestContext extends AbstractRequestContext { private final String responseType; private final String redirectUri; diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java index 8675c7af..c1ad437c 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java @@ -70,6 +70,7 @@ import static org.mockito.Mockito.when; * @author Paurav Munshi * @author Joe Grandja * @author Daniel Garnier-Moiroux + * @author Anoop Garlapati * @since 0.0.1 */ public class OAuth2AuthorizationEndpointFilterTests { @@ -189,7 +190,7 @@ public class OAuth2AuthorizationEndpointFilterTests { } @Test - public void doFilterWhenAuthorizationRequestInvalidRedirectUriThenInvalidRequestError() throws Exception { + public void doFilterWhenAuthorizationRequestUnregisteredRedirectUriThenInvalidRequestError() throws Exception { RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); when(this.registeredClientRepository.findByClientId((eq(registeredClient.getClientId())))) .thenReturn(registeredClient); @@ -201,6 +202,20 @@ public class OAuth2AuthorizationEndpointFilterTests { request -> request.setParameter(OAuth2ParameterNames.REDIRECT_URI, "https://invalid-example.com")); } + // gh-243 + @Test + public void doFilterWhenAuthorizationRequestInvalidRedirectUriHostThenInvalidRequestError() throws Exception { + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); + when(this.registeredClientRepository.findByClientId((eq(registeredClient.getClientId())))) + .thenReturn(registeredClient); + + doFilterWhenAuthorizationRequestInvalidParameterThenError( + registeredClient, + OAuth2ParameterNames.REDIRECT_URI, + OAuth2ErrorCodes.INVALID_REQUEST, + request -> request.setParameter(OAuth2ParameterNames.REDIRECT_URI, "https:///invalid")); + } + @Test public void doFilterWhenAuthorizationRequestMultipleRedirectUriThenInvalidRequestError() throws Exception { RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); @@ -823,6 +838,84 @@ public class OAuth2AuthorizationEndpointFilterTests { .isEqualTo(registeredClient.getScopes()); } + // gh-243 + @Test + public void doFilterWhenAuthorizationRequestIPv4LoopbackRedirectUriAndDifferentPortThenAuthorizationResponse() + throws Exception { + RegisteredClient registeredClient = TestRegisteredClients.registeredClient() + .redirectUri("http://127.0.0.1:8080") + .build(); + MockHttpServletRequest request = createAuthorizationRequest(registeredClient); + request.removeParameter(OAuth2ParameterNames.REDIRECT_URI); + request.addParameter(OAuth2ParameterNames.REDIRECT_URI, "http://127.0.0.1:5000"); + + when(this.registeredClientRepository.findByClientId((eq(registeredClient.getClientId())))) + .thenReturn(registeredClient); + + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + + this.filter.doFilter(request, response, filterChain); + + verifyNoInteractions(filterChain); + + assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); + assertThat(response.getRedirectedUrl()).matches("http://127.0.0.1:5000\\?code=.{15,}&state=state"); + } + + // gh-243 + @Test + public void doFilterWhenAuthorizationRequestIPv6LoopbackRedirectUriAndDifferentPortThenAuthorizationResponse() + throws Exception { + RegisteredClient registeredClient = TestRegisteredClients.registeredClient() + .redirectUri("http://[::1]:8080") + .build(); + MockHttpServletRequest request = createAuthorizationRequest(registeredClient); + request.removeParameter(OAuth2ParameterNames.REDIRECT_URI); + request.addParameter(OAuth2ParameterNames.REDIRECT_URI, "http://[::1]:5000"); + + when(this.registeredClientRepository.findByClientId((eq(registeredClient.getClientId())))) + .thenReturn(registeredClient); + + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + + this.filter.doFilter(request, response, filterChain); + + verifyNoInteractions(filterChain); + + assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); + assertThat(response.getRedirectedUrl()).matches("http://\\[::1]:5000\\?code=.{15,}&state=state"); + } + + // gh-243 + @Test + public void doFilterWhenAuthorizationRequestInvalidRedirectUriFragmentThenInvalidRequestError() throws Exception { + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); + when(this.registeredClientRepository.findByClientId((eq(registeredClient.getClientId())))) + .thenReturn(registeredClient); + + doFilterWhenAuthorizationRequestInvalidParameterThenError( + registeredClient, + OAuth2ParameterNames.REDIRECT_URI, + OAuth2ErrorCodes.INVALID_REQUEST, + request -> request.setParameter(OAuth2ParameterNames.REDIRECT_URI, "https://example.com#fragment")); + } + + // gh-243 + @Test + public void doFilterWhenAuthorizationRequestLocalhostRedirectUriThenInvalidRequestError() throws Exception { + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); + when(this.registeredClientRepository.findByClientId((eq(registeredClient.getClientId())))) + .thenReturn(registeredClient); + + doFilterWhenAuthorizationRequestInvalidParameterThenError( + registeredClient, + OAuth2ParameterNames.REDIRECT_URI, + OAuth2ErrorCodes.INVALID_REQUEST, + request -> request.setParameter(OAuth2ParameterNames.REDIRECT_URI, "http://localhost:5000")); + } + private void doFilterWhenAuthorizationRequestInvalidParameterThenError(RegisteredClient registeredClient, String parameterName, String errorCode) throws Exception { doFilterWhenAuthorizationRequestInvalidParameterThenError(registeredClient, parameterName, errorCode, request -> {});