OAuth2DeviceVerificationEndpointFilter is applied after AuthorizationFilter

Closes gh-18873

Author: jgrandja

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.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 void configure(HttpSecurity builder) {
 		if (StringUtils.hasText(this.consentPage)) {
 			deviceVerificationEndpointFilter.setConsentPage(this.consentPage);
 		}
-		builder.addFilterBefore(postProcess(deviceVerificationEndpointFilter),
-				AbstractPreAuthenticatedProcessingFilter.class);
+		builder.addFilterAfter(postProcess(deviceVerificationEndpointFilter), AuthorizationFilter.class);
 	}
 
 	@Override

config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2DeviceCodeGrantTests.java

@@ -359,7 +359,7 @@ public void requestWhenDeviceVerificationRequestValidThenDisplaysConsentPage() t
 	}
 
 	@Test
-	public void requestWhenDeviceAuthorizationConsentRequestUnauthenticatedThenBadRequest() throws Exception {
+	public void requestWhenDeviceAuthorizationConsentRequestUnauthenticatedThenUnauthorized() throws Exception {
 		this.spring.register(AuthorizationServerConfiguration.class).autowire();
 
 		// @formatter:off
@@ -392,7 +392,7 @@ public void requestWhenDeviceAuthorizationConsentRequestUnauthenticatedThenBadRe
 		// @formatter:off
 		this.mvc.perform(post(DEFAULT_DEVICE_VERIFICATION_ENDPOINT_URI)
 				.params(parameters))
-				.andExpect(status().isBadRequest());
+				.andExpect(status().isUnauthorized());
 		// @formatter:on
 	}
 

oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProvider.java

@@ -132,9 +132,7 @@ public Authentication authenticate(Authentication authentication) throws Authent
 			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

oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilter.java

@@ -161,15 +161,6 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
 			}
 
 			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");

oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.java

@@ -227,7 +227,7 @@ public void authenticateWhenUserCodeIsExpiredAndNotInvalidatedThenThrowOAuth2Aut
 	}
 
 	@Test
-	public void authenticateWhenPrincipalNotAuthenticatedThenReturnUnauthenticated() {
+	public void authenticateWhenPrincipalNotAuthenticatedThenThrowOAuth2AuthenticationException() {
 		RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
 		// @formatter:off
 		OAuth2Authorization authorization = TestOAuth2Authorizations
@@ -237,15 +237,21 @@ public void authenticateWhenPrincipalNotAuthenticatedThenReturnUnauthenticated()
 				.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);

oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilterTests.java

@@ -166,21 +166,6 @@ public void doFilterWhenNotDeviceVerificationRequestThenNotProcessed() throws Ex
 		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();