From 629239fde191acb242cf8ad5c54d2a40062a7eb7 Mon Sep 17 00:00:00 2001 From: Joe Grandja <10884212+jgrandja@users.noreply.github.com> Date: Mon, 24 Feb 2025 11:52:13 -0500 Subject: [PATCH] Polish gh-1874 --- .../OAuth2AuthorizationEndpointFilter.java | 2 +- ...ionCodeRequestAuthenticationConverter.java | 13 ++++--- ...izationConsentAuthenticationConverter.java | 20 ++++++++-- ...Auth2AuthorizationEndpointFilterTests.java | 38 +++++++++++++++++-- 4 files changed, 60 insertions(+), 13 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 c4e26129..6df237aa 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 @@ -1,5 +1,5 @@ /* - * Copyright 2020-2024 the original author or authors. + * Copyright 2020-2025 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. diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationCodeRequestAuthenticationConverter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationCodeRequestAuthenticationConverter.java index 521ddd40..9443fe14 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationCodeRequestAuthenticationConverter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationCodeRequestAuthenticationConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2024 the original author or authors. + * Copyright 2020-2025 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. @@ -38,6 +38,7 @@ import org.springframework.security.oauth2.server.authorization.authentication.O import org.springframework.security.oauth2.server.authorization.web.OAuth2AuthorizationEndpointFilter; import org.springframework.security.web.authentication.AuthenticationConverter; import org.springframework.security.web.util.matcher.AndRequestMatcher; +import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.CollectionUtils; import org.springframework.util.MultiValueMap; @@ -64,11 +65,11 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationConverter impleme private static final Authentication ANONYMOUS_AUTHENTICATION = new AnonymousAuthenticationToken("anonymous", "anonymousUser", AuthorityUtils.createAuthorityList("ROLE_ANONYMOUS")); - private static final RequestMatcher POST_WITH_RESPONSE_TYPE_REQUEST_MATCHER = createPostWithResponseTypeRequestMatcher(); + private final RequestMatcher requestMatcher = createDefaultRequestMatcher(); @Override public Authentication convert(HttpServletRequest request) { - if (!"GET".equals(request.getMethod()) && !POST_WITH_RESPONSE_TYPE_REQUEST_MATCHER.matches(request)) { + if (!this.requestMatcher.matches(request)) { return null; } @@ -153,11 +154,13 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationConverter impleme state, scopes, additionalParameters); } - private static RequestMatcher createPostWithResponseTypeRequestMatcher() { + private static RequestMatcher createDefaultRequestMatcher() { + RequestMatcher getMethodMatcher = (request) -> "GET".equals(request.getMethod()); RequestMatcher postMethodMatcher = (request) -> "POST".equals(request.getMethod()); RequestMatcher responseTypeParameterMatcher = ( request) -> request.getParameter(OAuth2ParameterNames.RESPONSE_TYPE) != null; - return new AndRequestMatcher(postMethodMatcher, responseTypeParameterMatcher); + return new OrRequestMatcher(getMethodMatcher, + new AndRequestMatcher(postMethodMatcher, responseTypeParameterMatcher)); } private static void throwError(String errorCode, String parameterName) { diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationConsentAuthenticationConverter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationConsentAuthenticationConverter.java index c6350c11..8bca4ca2 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationConsentAuthenticationConverter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationConsentAuthenticationConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2023 the original author or authors. + * Copyright 2020-2025 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. @@ -33,6 +33,9 @@ import org.springframework.security.oauth2.server.authorization.authentication.O import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationConsentAuthenticationToken; import org.springframework.security.oauth2.server.authorization.web.OAuth2AuthorizationEndpointFilter; import org.springframework.security.web.authentication.AuthenticationConverter; +import org.springframework.security.web.util.matcher.AndRequestMatcher; +import org.springframework.security.web.util.matcher.NegatedRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; @@ -55,14 +58,16 @@ public final class OAuth2AuthorizationConsentAuthenticationConverter implements private static final Authentication ANONYMOUS_AUTHENTICATION = new AnonymousAuthenticationToken("anonymous", "anonymousUser", AuthorityUtils.createAuthorityList("ROLE_ANONYMOUS")); + private final RequestMatcher requestMatcher = createDefaultRequestMatcher(); + @Override public Authentication convert(HttpServletRequest request) { - MultiValueMap parameters = OAuth2EndpointUtils.getFormParameters(request); - - if (!"POST".equals(request.getMethod()) || parameters.getFirst(OAuth2ParameterNames.RESPONSE_TYPE) != null) { + if (!this.requestMatcher.matches(request)) { return null; } + MultiValueMap parameters = OAuth2EndpointUtils.getFormParameters(request); + String authorizationUri = request.getRequestURL().toString(); // client_id (REQUIRED) @@ -100,6 +105,13 @@ public final class OAuth2AuthorizationConsentAuthenticationConverter implements additionalParameters); } + private static RequestMatcher createDefaultRequestMatcher() { + RequestMatcher postMethodMatcher = (request) -> "POST".equals(request.getMethod()); + RequestMatcher responseTypeParameterMatcher = ( + request) -> request.getParameter(OAuth2ParameterNames.RESPONSE_TYPE) != null; + return new AndRequestMatcher(postMethodMatcher, new NegatedRequestMatcher(responseTypeParameterMatcher)); + } + private static void throwError(String errorCode, String parameterName) { OAuth2Error error = new OAuth2Error(errorCode, "OAuth 2.0 Parameter: " + parameterName, DEFAULT_ERROR_URI); throw new OAuth2AuthorizationCodeRequestAuthenticationException(error, null); 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 ada590fd..ac4f81e1 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 @@ -1,5 +1,5 @@ /* - * Copyright 2020-2024 the original author or authors. + * Copyright 2020-2025 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. @@ -609,9 +609,41 @@ public class OAuth2AuthorizationEndpointFilterTests { .isEqualTo("https://example.com?param=encoded%20parameter%20value&code=code&state=client%20state"); } + @Test + public void doFilterWhenPostAuthorizationRequestAuthenticatedThenAuthorizationResponse() throws Exception { + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().redirectUris((redirectUris) -> { + redirectUris.clear(); + redirectUris.add("https://example.com?param=encoded%20parameter%20value"); + }).build(); + OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthenticationResult = new OAuth2AuthorizationCodeRequestAuthenticationToken( + AUTHORIZATION_URI, registeredClient.getClientId(), this.principal, this.authorizationCode, + registeredClient.getRedirectUris().iterator().next(), "client state", registeredClient.getScopes()); + authorizationCodeRequestAuthenticationResult.setAuthenticated(true); + given(this.authenticationManager.authenticate(any())).willReturn(authorizationCodeRequestAuthenticationResult); + + MockHttpServletRequest request = createAuthorizationRequest(registeredClient); + request.setMethod("POST"); + request.setQueryString(null); + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + + this.filter.doFilter(request, response, filterChain); + + verify(this.authenticationManager).authenticate(any()); + verifyNoInteractions(filterChain); + + assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); + assertThat(response.getRedirectedUrl()) + .isEqualTo("https://example.com?param=encoded%20parameter%20value&code=code&state=client%20state"); + } + @Test public void doFilterWhenAuthenticationRequestAuthenticatedThenAuthorizationResponse() throws Exception { - RegisteredClient registeredClient = TestRegisteredClients.registeredClient().scopes(Set::clear).build(); + // Setup OpenID Connect request + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().scopes((scopes) -> { + scopes.clear(); + scopes.add(OidcScopes.OPENID); + }).build(); OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthenticationResult = new OAuth2AuthorizationCodeRequestAuthenticationToken( AUTHORIZATION_URI, registeredClient.getClientId(), this.principal, this.authorizationCode, registeredClient.getRedirectUris().iterator().next(), STATE, registeredClient.getScopes()); @@ -619,7 +651,7 @@ public class OAuth2AuthorizationEndpointFilterTests { given(this.authenticationManager.authenticate(any())).willReturn(authorizationCodeRequestAuthenticationResult); MockHttpServletRequest request = createAuthorizationRequest(registeredClient); - request.setMethod("POST"); + request.setMethod("POST"); // OpenID Connect supports POST method request.setQueryString(null); MockHttpServletResponse response = new MockHttpServletResponse(); FilterChain filterChain = mock(FilterChain.class);