From 1906075b0c4e2d47737df8b3c50d687fae1c499c Mon Sep 17 00:00:00 2001 From: Joe Grandja <10884212+jgrandja@users.noreply.github.com> Date: Tue, 10 Mar 2026 14:14:42 -0400 Subject: [PATCH] OAuth2DeviceVerificationEndpointFilter is applied after AuthorizationFilter Closes gh-18873 --- ...2DeviceVerificationEndpointConfigurer.java | 5 ++--- .../OAuth2DeviceCodeGrantTests.java | 4 ++-- ...iceVerificationAuthenticationProvider.java | 4 +--- ...Auth2DeviceVerificationEndpointFilter.java | 9 --------- ...rificationAuthenticationProviderTests.java | 20 ++++++++++++------- ...DeviceVerificationEndpointFilterTests.java | 15 -------------- 6 files changed, 18 insertions(+), 39 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2DeviceVerificationEndpointConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2DeviceVerificationEndpointConfigurer.java index a2f7b1c885..7b72e2b0ae 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2DeviceVerificationEndpointConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2DeviceVerificationEndpointConfigurer.java @@ -40,11 +40,11 @@ import org.springframework.security.oauth2.server.authorization.settings.Authori import org.springframework.security.oauth2.server.authorization.web.OAuth2DeviceVerificationEndpointFilter; import org.springframework.security.oauth2.server.authorization.web.authentication.OAuth2DeviceAuthorizationConsentAuthenticationConverter; import org.springframework.security.oauth2.server.authorization.web.authentication.OAuth2DeviceVerificationAuthenticationConverter; +import org.springframework.security.web.access.intercept.AuthorizationFilter; import org.springframework.security.web.authentication.AuthenticationConverter; import org.springframework.security.web.authentication.AuthenticationFailureHandler; import org.springframework.security.web.authentication.AuthenticationSuccessHandler; import org.springframework.security.web.authentication.DelegatingAuthenticationConverter; -import org.springframework.security.web.authentication.preauth.AbstractPreAuthenticatedProcessingFilter; import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -279,8 +279,7 @@ public final class OAuth2DeviceVerificationEndpointConfigurer extends AbstractOA if (StringUtils.hasText(this.consentPage)) { deviceVerificationEndpointFilter.setConsentPage(this.consentPage); } - builder.addFilterBefore(postProcess(deviceVerificationEndpointFilter), - AbstractPreAuthenticatedProcessingFilter.class); + builder.addFilterAfter(postProcess(deviceVerificationEndpointFilter), AuthorizationFilter.class); } @Override diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2DeviceCodeGrantTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2DeviceCodeGrantTests.java index a2938fddc8..5740406885 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2DeviceCodeGrantTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2DeviceCodeGrantTests.java @@ -359,7 +359,7 @@ public class OAuth2DeviceCodeGrantTests { } @Test - public void requestWhenDeviceAuthorizationConsentRequestUnauthenticatedThenBadRequest() throws Exception { + public void requestWhenDeviceAuthorizationConsentRequestUnauthenticatedThenUnauthorized() throws Exception { this.spring.register(AuthorizationServerConfiguration.class).autowire(); // @formatter:off @@ -392,7 +392,7 @@ public class OAuth2DeviceCodeGrantTests { // @formatter:off this.mvc.perform(post(DEFAULT_DEVICE_VERIFICATION_ENDPOINT_URI) .params(parameters)) - .andExpect(status().isBadRequest()); + .andExpect(status().isUnauthorized()); // @formatter:on } diff --git a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProvider.java b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProvider.java index 95261d36fc..b047e8007c 100644 --- a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProvider.java +++ b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProvider.java @@ -132,9 +132,7 @@ public final class OAuth2DeviceVerificationAuthenticationProvider implements Aut if (this.logger.isTraceEnabled()) { this.logger.trace("Did not authenticate device verification request since principal not authenticated"); } - // Return the device verification request as-is where isAuthenticated() is - // false - return deviceVerificationAuthentication; + throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_REQUEST); } RegisteredClient registeredClient = this.registeredClientRepository diff --git a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilter.java b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilter.java index b52461b782..e0b01e0923 100644 --- a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilter.java +++ b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilter.java @@ -161,15 +161,6 @@ public final class OAuth2DeviceVerificationEndpointFilter extends OncePerRequest } Authentication authenticationResult = this.authenticationManager.authenticate(authentication); - if (!authenticationResult.isAuthenticated()) { - // If the Principal (Resource Owner) is not authenticated then pass - // through the chain - // with the expectation that the authentication process will commence via - // AuthenticationEntryPoint - filterChain.doFilter(request, response); - return; - } - if (authenticationResult instanceof OAuth2DeviceAuthorizationConsentAuthenticationToken) { if (this.logger.isTraceEnabled()) { this.logger.trace("Device authorization consent is required"); diff --git a/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.java b/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.java index aee8013d94..c9b3ed306f 100644 --- a/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.java +++ b/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.java @@ -227,7 +227,7 @@ public class OAuth2DeviceVerificationAuthenticationProviderTests { } @Test - public void authenticateWhenPrincipalNotAuthenticatedThenReturnUnauthenticated() { + public void authenticateWhenPrincipalNotAuthenticatedThenThrowOAuth2AuthenticationException() { RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); // @formatter:off OAuth2Authorization authorization = TestOAuth2Authorizations @@ -237,15 +237,21 @@ public class OAuth2DeviceVerificationAuthenticationProviderTests { .attribute(OAuth2ParameterNames.SCOPE, registeredClient.getScopes()) .build(); // @formatter:on - TestingAuthenticationToken principal = new TestingAuthenticationToken("user", null); + TestingAuthenticationToken principal = new TestingAuthenticationToken("anonymous", null); + principal.setAuthenticated(false); Authentication authentication = new OAuth2DeviceVerificationAuthenticationToken(principal, USER_CODE, Collections.emptyMap()); - given(this.authorizationService.findByToken(anyString(), any(OAuth2TokenType.class))).willReturn(authorization); + given(this.authorizationService.findByToken(eq(USER_CODE), + eq(OAuth2DeviceVerificationAuthenticationProvider.USER_CODE_TOKEN_TYPE))) + .willReturn(authorization); - OAuth2DeviceVerificationAuthenticationToken authenticationResult = (OAuth2DeviceVerificationAuthenticationToken) this.authenticationProvider - .authenticate(authentication); - assertThat(authenticationResult).isEqualTo(authentication); - assertThat(authenticationResult.isAuthenticated()).isFalse(); + // @formatter:off + assertThatExceptionOfType(OAuth2AuthenticationException.class) + .isThrownBy(() -> this.authenticationProvider.authenticate(authentication)) + .extracting(OAuth2AuthenticationException::getError) + .extracting(OAuth2Error::getErrorCode) + .isEqualTo(OAuth2ErrorCodes.INVALID_REQUEST); + // @formatter:on verify(this.authorizationService).findByToken(USER_CODE, OAuth2DeviceVerificationAuthenticationProvider.USER_CODE_TOKEN_TYPE); diff --git a/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilterTests.java b/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilterTests.java index b46338bfab..74a279791d 100644 --- a/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilterTests.java +++ b/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilterTests.java @@ -166,21 +166,6 @@ public class OAuth2DeviceVerificationEndpointFilterTests { verifyNoInteractions(this.authenticationManager); } - @Test - public void doFilterWhenUnauthenticatedThenPassThrough() throws Exception { - TestingAuthenticationToken unauthenticatedResult = new TestingAuthenticationToken("user", null); - given(this.authenticationManager.authenticate(any(Authentication.class))).willReturn(unauthenticatedResult); - - MockHttpServletRequest request = createRequest(); - request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE); - updateQueryString(request); - MockHttpServletResponse response = new MockHttpServletResponse(); - FilterChain filterChain = mock(FilterChain.class); - this.filter.doFilter(request, response, filterChain); - verify(this.authenticationManager).authenticate(any(Authentication.class)); - verify(filterChain).doFilter(request, response); - } - @Test public void doFilterWhenDeviceAuthorizationConsentRequestThenSuccess() throws Exception { Authentication authenticationResult = createDeviceVerificationAuthentication();