From 9ddb20a31528aaca0f4258270566f86f3b304941 Mon Sep 17 00:00:00 2001 From: fine-pine Date: Tue, 26 Aug 2025 18:00:15 +0900 Subject: [PATCH 1/3] Prevent ID token refresh in device code flow - Disallow usage of the `openid` scope in device authorization requests - Allow ID token refresh when an ID token already exists Closes gh-2037 Signed-off-by: fine-pine --- ...rizationRequestAuthenticationProvider.java | 7 +++- ...th2RefreshTokenAuthenticationProvider.java | 2 +- ...ionRequestAuthenticationProviderTests.java | 20 ++++++++++- ...freshTokenAuthenticationProviderTests.java | 33 ++++++++++++++++++- 4 files changed, 58 insertions(+), 4 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java index e32081481..e1e352dae 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.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. @@ -39,6 +39,7 @@ import org.springframework.security.oauth2.core.OAuth2ErrorCodes; import org.springframework.security.oauth2.core.OAuth2UserCode; import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; +import org.springframework.security.oauth2.core.oidc.OidcScopes; import org.springframework.security.oauth2.server.authorization.OAuth2Authorization; import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationService; import org.springframework.security.oauth2.server.authorization.OAuth2TokenType; @@ -122,6 +123,10 @@ public Authentication authenticate(Authentication authentication) throws Authent } } + if(requestedScopes.contains(OidcScopes.OPENID)) { + throwError(OAuth2ErrorCodes.INVALID_SCOPE, OAuth2ParameterNames.SCOPE); + } + if (this.logger.isTraceEnabled()) { this.logger.trace("Validated device authorization request parameters"); } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProvider.java index c4b89d660..93189f1d4 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProvider.java @@ -215,7 +215,7 @@ public Authentication authenticate(Authentication authentication) throws Authent // ----- ID token ----- OidcIdToken idToken; - if (authorizedScopes.contains(OidcScopes.OPENID)) { + if (authorizedScopes.contains(OidcScopes.OPENID) && authorization.getToken(OidcIdToken.class) != null) { // @formatter:off tokenContext = tokenContextBuilder .tokenType(ID_TOKEN_TOKEN_TYPE) diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProviderTests.java index 81b038f6d..b916d000d 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProviderTests.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. @@ -34,6 +34,7 @@ import org.springframework.security.oauth2.core.OAuth2ErrorCodes; import org.springframework.security.oauth2.core.OAuth2UserCode; import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; +import org.springframework.security.oauth2.core.oidc.OidcScopes; import org.springframework.security.oauth2.server.authorization.OAuth2Authorization; import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationService; import org.springframework.security.oauth2.server.authorization.client.RegisteredClient; @@ -165,6 +166,23 @@ public void authenticateWhenInvalidScopesThenThrowOAuth2AuthenticationException( // @formatter:on } + @Test + public void authenticateWhenOpenIdScopeThenThrowOAuth2AuthenticationException() { + RegisteredClient registeredClient = TestRegisteredClients.registeredClient() + .authorizationGrantType(AuthorizationGrantType.DEVICE_CODE) + .scope(OidcScopes.OPENID) + .build(); + Authentication authentication = createAuthentication(registeredClient); + // @formatter:off + assertThatExceptionOfType(OAuth2AuthenticationException.class) + .isThrownBy(() -> this.authenticationProvider.authenticate(authentication)) + .withMessageContaining(OAuth2ParameterNames.SCOPE) + .extracting(OAuth2AuthenticationException::getError) + .extracting(OAuth2Error::getErrorCode) + .isEqualTo(OAuth2ErrorCodes.INVALID_SCOPE); + // @formatter:on + } + @Test public void authenticateWhenDeviceCodeIsNullThenThrowOAuth2AuthenticationException() { @SuppressWarnings("unchecked") diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProviderTests.java index beb2b00af..bb313078b 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProviderTests.java @@ -571,7 +571,17 @@ public void authenticateWhenRefreshTokenNotGeneratedThenThrowOAuth2Authenticatio @Test public void authenticateWhenIdTokenNotGeneratedThenThrowOAuth2AuthenticationException() { RegisteredClient registeredClient = TestRegisteredClients.registeredClient().scope(OidcScopes.OPENID).build(); - OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient).build(); + OidcIdToken authorizedIdToken = OidcIdToken.withTokenValue("id-token") + .issuer("https://provider.com") + .subject("subject") + .issuedAt(Instant.now()) + .expiresAt(Instant.now().plusSeconds(60)) + .claim("sid", "sessionId-1234") + .claim(IdTokenClaimNames.AUTH_TIME, Date.from(Instant.now())) + .build(); + OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient) + .token(authorizedIdToken) + .build(); given(this.authorizationService.findByToken(eq(authorization.getRefreshToken().getToken().getTokenValue()), eq(OAuth2TokenType.REFRESH_TOKEN))) .willReturn(authorization); @@ -600,6 +610,27 @@ public void authenticateWhenIdTokenNotGeneratedThenThrowOAuth2AuthenticationExce }); } + @Test + public void authenticateAuthorizationWithoutIdTokenThenIdTokenNotGenerated() { + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().scope(OidcScopes.OPENID).build(); + OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient).build(); + given(this.authorizationService.findByToken(eq(authorization.getRefreshToken().getToken().getTokenValue()), + eq(OAuth2TokenType.REFRESH_TOKEN))) + .willReturn(authorization); + + OAuth2ClientAuthenticationToken clientPrincipal = new OAuth2ClientAuthenticationToken(registeredClient, + ClientAuthenticationMethod.CLIENT_SECRET_BASIC, registeredClient.getClientSecret()); + OAuth2RefreshTokenAuthenticationToken authentication = new OAuth2RefreshTokenAuthenticationToken( + authorization.getRefreshToken().getToken().getTokenValue(), clientPrincipal, null, null); + + + OAuth2AccessTokenAuthenticationToken accessTokenAuthentication = (OAuth2AccessTokenAuthenticationToken) this.authenticationProvider + .authenticate(authentication); + + assertThat(accessTokenAuthentication.getAdditionalParameters().containsKey(OidcParameterNames.ID_TOKEN)) + .isFalse(); + } + @Test public void authenticateWhenAccessTokenFormatReferenceThenAccessTokenGeneratorCalled() { // @formatter:off From 4ea9a73e3b1567260fc39e4164728bdf73bab140 Mon Sep 17 00:00:00 2001 From: Lee Song Mok Date: Wed, 27 Aug 2025 14:16:05 +0900 Subject: [PATCH 2/3] Resolve lint review Co-authored-by: injae kim Signed-off-by: Lee Song Mok --- .../OAuth2DeviceAuthorizationRequestAuthenticationProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java index e1e352dae..bb97701f7 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java @@ -123,7 +123,7 @@ public Authentication authenticate(Authentication authentication) throws Authent } } - if(requestedScopes.contains(OidcScopes.OPENID)) { + if (requestedScopes.contains(OidcScopes.OPENID)) { throwError(OAuth2ErrorCodes.INVALID_SCOPE, OAuth2ParameterNames.SCOPE); } From 7d06aaa732016453cf2646cf0ec0ace9eccdc666 Mon Sep 17 00:00:00 2001 From: fine-pine Date: Sat, 20 Sep 2025 15:43:34 +0900 Subject: [PATCH 3/3] Resolve reviews Signed-off-by: fine-pine --- ...rizationRequestAuthenticationProvider.java | 7 ++-- ...th2RefreshTokenAuthenticationProvider.java | 2 +- ...ionRequestAuthenticationProviderTests.java | 6 ++-- ...freshTokenAuthenticationProviderTests.java | 33 +------------------ 4 files changed, 8 insertions(+), 40 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java index bb97701f7..5a80768cf 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java @@ -121,10 +121,9 @@ public Authentication authenticate(Authentication authentication) throws Authent throwError(OAuth2ErrorCodes.INVALID_SCOPE, OAuth2ParameterNames.SCOPE); } } - } - - if (requestedScopes.contains(OidcScopes.OPENID)) { - throwError(OAuth2ErrorCodes.INVALID_SCOPE, OAuth2ParameterNames.SCOPE); + if (requestedScopes.contains(OidcScopes.OPENID)) { + throwError(OAuth2ErrorCodes.INVALID_SCOPE, OAuth2ParameterNames.SCOPE); + } } if (this.logger.isTraceEnabled()) { diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProvider.java index 93189f1d4..c4b89d660 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProvider.java @@ -215,7 +215,7 @@ public Authentication authenticate(Authentication authentication) throws Authent // ----- ID token ----- OidcIdToken idToken; - if (authorizedScopes.contains(OidcScopes.OPENID) && authorization.getToken(OidcIdToken.class) != null) { + if (authorizedScopes.contains(OidcScopes.OPENID)) { // @formatter:off tokenContext = tokenContextBuilder .tokenType(ID_TOKEN_TOKEN_TYPE) diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProviderTests.java index b916d000d..a60abf29a 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProviderTests.java @@ -169,9 +169,9 @@ public void authenticateWhenInvalidScopesThenThrowOAuth2AuthenticationException( @Test public void authenticateWhenOpenIdScopeThenThrowOAuth2AuthenticationException() { RegisteredClient registeredClient = TestRegisteredClients.registeredClient() - .authorizationGrantType(AuthorizationGrantType.DEVICE_CODE) - .scope(OidcScopes.OPENID) - .build(); + .authorizationGrantType(AuthorizationGrantType.DEVICE_CODE) + .scope(OidcScopes.OPENID) + .build(); Authentication authentication = createAuthentication(registeredClient); // @formatter:off assertThatExceptionOfType(OAuth2AuthenticationException.class) diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProviderTests.java index bb313078b..beb2b00af 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProviderTests.java @@ -571,17 +571,7 @@ public void authenticateWhenRefreshTokenNotGeneratedThenThrowOAuth2Authenticatio @Test public void authenticateWhenIdTokenNotGeneratedThenThrowOAuth2AuthenticationException() { RegisteredClient registeredClient = TestRegisteredClients.registeredClient().scope(OidcScopes.OPENID).build(); - OidcIdToken authorizedIdToken = OidcIdToken.withTokenValue("id-token") - .issuer("https://provider.com") - .subject("subject") - .issuedAt(Instant.now()) - .expiresAt(Instant.now().plusSeconds(60)) - .claim("sid", "sessionId-1234") - .claim(IdTokenClaimNames.AUTH_TIME, Date.from(Instant.now())) - .build(); - OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient) - .token(authorizedIdToken) - .build(); + OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient).build(); given(this.authorizationService.findByToken(eq(authorization.getRefreshToken().getToken().getTokenValue()), eq(OAuth2TokenType.REFRESH_TOKEN))) .willReturn(authorization); @@ -610,27 +600,6 @@ public void authenticateWhenIdTokenNotGeneratedThenThrowOAuth2AuthenticationExce }); } - @Test - public void authenticateAuthorizationWithoutIdTokenThenIdTokenNotGenerated() { - RegisteredClient registeredClient = TestRegisteredClients.registeredClient().scope(OidcScopes.OPENID).build(); - OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient).build(); - given(this.authorizationService.findByToken(eq(authorization.getRefreshToken().getToken().getTokenValue()), - eq(OAuth2TokenType.REFRESH_TOKEN))) - .willReturn(authorization); - - OAuth2ClientAuthenticationToken clientPrincipal = new OAuth2ClientAuthenticationToken(registeredClient, - ClientAuthenticationMethod.CLIENT_SECRET_BASIC, registeredClient.getClientSecret()); - OAuth2RefreshTokenAuthenticationToken authentication = new OAuth2RefreshTokenAuthenticationToken( - authorization.getRefreshToken().getToken().getTokenValue(), clientPrincipal, null, null); - - - OAuth2AccessTokenAuthenticationToken accessTokenAuthentication = (OAuth2AccessTokenAuthenticationToken) this.authenticationProvider - .authenticate(authentication); - - assertThat(accessTokenAuthentication.getAdditionalParameters().containsKey(OidcParameterNames.ID_TOKEN)) - .isFalse(); - } - @Test public void authenticateWhenAccessTokenFormatReferenceThenAccessTokenGeneratorCalled() { // @formatter:off