From ce528eed9b541f828f4b0e6ce4e49177ad4b386b Mon Sep 17 00:00:00 2001 From: Antoine Lauzon <139174762+antoinelauzon-bell@users.noreply.github.com> Date: Mon, 2 Jun 2025 08:38:38 -0400 Subject: [PATCH 1/2] Check user code expiry and invalidity Closes gh-1977 Signed-off-by: Antoine Lauzon <139174762+antoinelauzon-bell@users.noreply.github.com> --- ...iceVerificationAuthenticationProvider.java | 12 ++- ...rificationAuthenticationProviderTests.java | 83 ++++++++++++++++++- 2 files changed, 91 insertions(+), 4 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProvider.java index 6dc4e2de..3009c86b 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProvider.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. @@ -109,6 +109,15 @@ public final class OAuth2DeviceVerificationAuthenticationProvider implements Aut this.logger.trace("Retrieved authorization with user code"); } + OAuth2Authorization.Token userCode = authorization.getToken(OAuth2UserCode.class); + if (!userCode.isActive()) { + if (!userCode.isInvalidated()) { + authorization = OAuth2AuthenticationProviderUtils.invalidate(authorization, userCode.getToken()); + this.authorizationService.save(authorization); + } + throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_GRANT); + } + Authentication principal = (Authentication) deviceVerificationAuthentication.getPrincipal(); if (!isPrincipalAuthenticated(principal)) { if (this.logger.isTraceEnabled()) { @@ -161,7 +170,6 @@ public final class OAuth2DeviceVerificationAuthenticationProvider implements Aut requestedScopes, currentAuthorizedScopes); } - OAuth2Authorization.Token userCode = authorization.getToken(OAuth2UserCode.class); // @formatter:off authorization = OAuth2Authorization.from(authorization) .principalName(principal.getName()) diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.java index a0f3d12b..da379703 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.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. @@ -20,6 +20,7 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Collections; import java.util.Map; +import java.util.function.Consumer; import java.util.function.Function; import org.junit.jupiter.api.BeforeEach; @@ -145,10 +146,79 @@ public class OAuth2DeviceVerificationAuthenticationProviderTests { verifyNoInteractions(this.registeredClientRepository, this.authorizationConsentService); } + @Test + public void authenticateWhenUserCodeIsInvalidedThenThrowOAuth2AuthenticationException() { + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); + // @formatter:off + OAuth2Authorization authorization = TestOAuth2Authorizations + .authorization(registeredClient) + .token(createDeviceCode()) + .token(createUserCode(), withInvalidated()) + .attribute(OAuth2ParameterNames.SCOPE, registeredClient.getScopes()) + .build(); + // @formatter:on + given(this.authorizationService.findByToken(anyString(), any(OAuth2TokenType.class))).willReturn(authorization); + Authentication authentication = createAuthentication(); + // @formatter:off + assertThatExceptionOfType(OAuth2AuthenticationException.class) + .isThrownBy(() -> this.authenticationProvider.authenticate(authentication)) + .extracting(OAuth2AuthenticationException::getError) + .extracting(OAuth2Error::getErrorCode) + .isEqualTo(OAuth2ErrorCodes.INVALID_GRANT); + // @formatter:on + + verify(this.authorizationService).findByToken(USER_CODE, + OAuth2DeviceVerificationAuthenticationProvider.USER_CODE_TOKEN_TYPE); + verifyNoMoreInteractions(this.authorizationService); + verifyNoInteractions(this.registeredClientRepository, this.authorizationConsentService); + } + + @Test + public void authenticateWhenUserCodeIsExpiredButNotInvalidatedThenInvalidateUserCodeAndThrowOAuth2AuthenticationException() { + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); + // @formatter:off + OAuth2Authorization authorization = TestOAuth2Authorizations + .authorization(registeredClient) + // Device code would also be expired but not relevant for this test + .token(createDeviceCode()) + .token(createExpiredUserCode()) + .attribute(OAuth2ParameterNames.SCOPE, registeredClient.getScopes()) + .build(); + // @formatter:on + given(this.authorizationService.findByToken(anyString(), any(OAuth2TokenType.class))).willReturn(authorization); + Authentication authentication = createAuthentication(); + // @formatter:off + assertThatExceptionOfType(OAuth2AuthenticationException.class) + .isThrownBy(() -> this.authenticationProvider.authenticate(authentication)) + .extracting(OAuth2AuthenticationException::getError) + .extracting(OAuth2Error::getErrorCode) + .isEqualTo(OAuth2ErrorCodes.INVALID_GRANT); + // @formatter:on + + ArgumentCaptor authorizationCaptor = ArgumentCaptor.forClass(OAuth2Authorization.class); + verify(this.authorizationService).findByToken(USER_CODE, + OAuth2DeviceVerificationAuthenticationProvider.USER_CODE_TOKEN_TYPE); + verify(this.authorizationService).save(authorizationCaptor.capture()); + verifyNoMoreInteractions(this.authorizationService); + verifyNoInteractions(this.registeredClientRepository, this.authorizationConsentService); + + OAuth2Authorization updatedAuthorization = authorizationCaptor.getValue(); + assertThat(updatedAuthorization.getToken(OAuth2UserCode.class)) + .extracting(isInvalidated()) + .isEqualTo(true); + } + @Test public void authenticateWhenPrincipalNotAuthenticatedThenReturnUnauthenticated() { RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); - OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient).build(); + // @formatter:off + OAuth2Authorization authorization = TestOAuth2Authorizations + .authorization(registeredClient) + .token(createDeviceCode()) + .token(createUserCode()) + .attribute(OAuth2ParameterNames.SCOPE, registeredClient.getScopes()) + .build(); + // @formatter:on TestingAuthenticationToken principal = new TestingAuthenticationToken("user", null); Authentication authentication = new OAuth2DeviceVerificationAuthenticationToken(principal, USER_CODE, Collections.emptyMap()); @@ -331,6 +401,15 @@ public class OAuth2DeviceVerificationAuthenticationProviderTests { return new OAuth2UserCode(USER_CODE, issuedAt, issuedAt.plus(30, ChronoUnit.MINUTES)); } + private static OAuth2UserCode createExpiredUserCode() { + Instant issuedAt = Instant.now().minus(45, ChronoUnit.MINUTES); + return new OAuth2UserCode(USER_CODE, issuedAt, issuedAt.plus(30, ChronoUnit.MINUTES)); + } + + private static Consumer> withInvalidated() { + return (metadata) -> metadata.put(OAuth2Authorization.Token.INVALIDATED_METADATA_NAME, true); + } + private static Function, Boolean> isInvalidated() { return (token) -> token.getMetadata(OAuth2Authorization.Token.INVALIDATED_METADATA_NAME); } From fe4b5ada8c92b1aa8e7d174701b21993fe4bf886 Mon Sep 17 00:00:00 2001 From: Joe Grandja <10884212+jgrandja@users.noreply.github.com> Date: Tue, 3 Jun 2025 09:16:55 -0400 Subject: [PATCH 2/2] Polish gh-1997 --- ...eviceVerificationAuthenticationProvider.java | 5 +++++ ...VerificationAuthenticationProviderTests.java | 17 ++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProvider.java index 3009c86b..a44eb6a7 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProvider.java @@ -22,6 +22,7 @@ import java.util.Set; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogMessage; import org.springframework.security.authentication.AnonymousAuthenticationToken; import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.core.Authentication; @@ -114,6 +115,10 @@ public final class OAuth2DeviceVerificationAuthenticationProvider implements Aut if (!userCode.isInvalidated()) { authorization = OAuth2AuthenticationProviderUtils.invalidate(authorization, userCode.getToken()); this.authorizationService.save(authorization); + if (this.logger.isWarnEnabled()) { + this.logger.warn(LogMessage.format("Invalidated user code used by registered client '%s'", + authorization.getRegisteredClientId())); + } } throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_GRANT); } diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.java index da379703..fd6a54d6 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceVerificationAuthenticationProviderTests.java @@ -56,6 +56,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -147,7 +148,7 @@ public class OAuth2DeviceVerificationAuthenticationProviderTests { } @Test - public void authenticateWhenUserCodeIsInvalidedThenThrowOAuth2AuthenticationException() { + public void authenticateWhenUserCodeIsInvalidatedThenThrowOAuth2AuthenticationException() { RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); // @formatter:off OAuth2Authorization authorization = TestOAuth2Authorizations @@ -157,7 +158,9 @@ public class OAuth2DeviceVerificationAuthenticationProviderTests { .attribute(OAuth2ParameterNames.SCOPE, registeredClient.getScopes()) .build(); // @formatter:on - 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); Authentication authentication = createAuthentication(); // @formatter:off assertThatExceptionOfType(OAuth2AuthenticationException.class) @@ -174,7 +177,7 @@ public class OAuth2DeviceVerificationAuthenticationProviderTests { } @Test - public void authenticateWhenUserCodeIsExpiredButNotInvalidatedThenInvalidateUserCodeAndThrowOAuth2AuthenticationException() { + public void authenticateWhenUserCodeIsExpiredAndNotInvalidatedThenThrowOAuth2AuthenticationException() { RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); // @formatter:off OAuth2Authorization authorization = TestOAuth2Authorizations @@ -185,7 +188,9 @@ public class OAuth2DeviceVerificationAuthenticationProviderTests { .attribute(OAuth2ParameterNames.SCOPE, registeredClient.getScopes()) .build(); // @formatter:on - 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); Authentication authentication = createAuthentication(); // @formatter:off assertThatExceptionOfType(OAuth2AuthenticationException.class) @@ -203,9 +208,7 @@ public class OAuth2DeviceVerificationAuthenticationProviderTests { verifyNoInteractions(this.registeredClientRepository, this.authorizationConsentService); OAuth2Authorization updatedAuthorization = authorizationCaptor.getValue(); - assertThat(updatedAuthorization.getToken(OAuth2UserCode.class)) - .extracting(isInvalidated()) - .isEqualTo(true); + assertThat(updatedAuthorization.getToken(OAuth2UserCode.class)).extracting(isInvalidated()).isEqualTo(true); } @Test