From 9eefb46bca4a425bcc1c7685677ce5724c96b0cb Mon Sep 17 00:00:00 2001 From: Pavel Efros Date: Thu, 25 May 2023 15:09:56 +0200 Subject: [PATCH 1/3] Fix issue with NPE on access token in OAuth2AuthorizationCodeAuthenticationProvider Currently, if access tokens are not persisted, then line 159 is causing a NullPointerException, as we do not check if the access token is null, before calling `getToken()` on it. This issue occurs on a reuse of authorization codes. --- .../OAuth2AuthorizationCodeAuthenticationProvider.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java index 83602d754..5e75cdf0c 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java @@ -154,12 +154,12 @@ public Authentication authenticate(Authentication authentication) throws Authent if (!authorizationCode.isActive()) { if (authorizationCode.isInvalidated()) { - OAuth2Token token = authorization.getRefreshToken() != null ? - authorization.getRefreshToken().getToken() : - authorization.getAccessToken().getToken(); + var token = authorization.getRefreshToken() != null ? + authorization.getRefreshToken() : + authorization.getAccessToken(); if (token != null) { // Invalidate the access (and refresh) token as the client is attempting to use the authorization code more than once - authorization = OAuth2AuthenticationProviderUtils.invalidate(authorization, token); + authorization = OAuth2AuthenticationProviderUtils.invalidate(authorization, token.getToken()); this.authorizationService.save(authorization); if (this.logger.isWarnEnabled()) { this.logger.warn(LogMessage.format("Invalidated authorization token(s) previously issued to registered client '%s'", registeredClient.getId())); From 54294a2a2e6e0843898d203a4d8671a32f0d1ba1 Mon Sep 17 00:00:00 2001 From: pavelefros Date: Mon, 12 Jun 2023 09:22:32 +0200 Subject: [PATCH 2/3] use explicit type for token and add test --- ...thorizationCodeAuthenticationProvider.java | 12 ++------- ...zationCodeAuthenticationProviderTests.java | 27 +++++++++++++++++++ 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java index 5e75cdf0c..12ddd56e0 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java @@ -36,15 +36,7 @@ import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.session.SessionInformation; import org.springframework.security.core.session.SessionRegistry; -import org.springframework.security.oauth2.core.AuthorizationGrantType; -import org.springframework.security.oauth2.core.ClaimAccessor; -import org.springframework.security.oauth2.core.ClientAuthenticationMethod; -import org.springframework.security.oauth2.core.OAuth2AccessToken; -import org.springframework.security.oauth2.core.OAuth2AuthenticationException; -import org.springframework.security.oauth2.core.OAuth2Error; -import org.springframework.security.oauth2.core.OAuth2ErrorCodes; -import org.springframework.security.oauth2.core.OAuth2RefreshToken; -import org.springframework.security.oauth2.core.OAuth2Token; +import org.springframework.security.oauth2.core.*; import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; import org.springframework.security.oauth2.core.oidc.OidcIdToken; @@ -154,7 +146,7 @@ public Authentication authenticate(Authentication authentication) throws Authent if (!authorizationCode.isActive()) { if (authorizationCode.isInvalidated()) { - var token = authorization.getRefreshToken() != null ? + OAuth2Authorization.Token token = authorization.getRefreshToken() != null ? authorization.getRefreshToken() : authorization.getAccessToken(); if (token != null) { diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProviderTests.java index 2378a559e..f50757721 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProviderTests.java @@ -283,6 +283,33 @@ public void authenticateWhenInvalidatedCodeThenThrowOAuth2AuthenticationExceptio assertThat(updatedAuthorization.getRefreshToken().isInvalidated()).isTrue(); } + // gh PR 1233 + @Test + public void authenticateWhenInvalidatedCodeAndNullRefreshAndAccessTokensThenThrowOAuth2AuthenticationException() { + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); + OAuth2AuthorizationCode authorizationCode = new OAuth2AuthorizationCode( + AUTHORIZATION_CODE, Instant.now(), Instant.now().plusSeconds(120)); + OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient, authorizationCode) + .token(authorizationCode, (metadata) -> metadata.put(OAuth2Authorization.Token.INVALIDATED_METADATA_NAME, true)) + .build(); + + when(this.authorizationService.findByToken(eq(AUTHORIZATION_CODE), eq(AUTHORIZATION_CODE_TOKEN_TYPE))) + .thenReturn(authorization); + + OAuth2ClientAuthenticationToken clientPrincipal = new OAuth2ClientAuthenticationToken( + registeredClient, ClientAuthenticationMethod.CLIENT_SECRET_BASIC, registeredClient.getClientSecret()); + OAuth2AuthorizationRequest authorizationRequest = authorization.getAttribute( + OAuth2AuthorizationRequest.class.getName()); + OAuth2AuthorizationCodeAuthenticationToken authentication = + new OAuth2AuthorizationCodeAuthenticationToken(AUTHORIZATION_CODE, clientPrincipal, authorizationRequest.getRedirectUri(), null); + + assertThatThrownBy(() -> this.authenticationProvider.authenticate(authentication)) + .isInstanceOf(OAuth2AuthenticationException.class) + .extracting(ex -> ((OAuth2AuthenticationException) ex).getError()) + .extracting("errorCode") + .isEqualTo(OAuth2ErrorCodes.INVALID_GRANT); + } + // gh-290 @Test public void authenticateWhenExpiredCodeThenThrowOAuth2AuthenticationException() { From 861f477440289d8385353ea6ded2e061fa77cdca Mon Sep 17 00:00:00 2001 From: pavelefros Date: Mon, 12 Jun 2023 09:24:29 +0200 Subject: [PATCH 3/3] fix imports --- ...OAuth2AuthorizationCodeAuthenticationProvider.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java index 12ddd56e0..6535136fa 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java @@ -36,7 +36,16 @@ import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.session.SessionInformation; import org.springframework.security.core.session.SessionRegistry; -import org.springframework.security.oauth2.core.*; +import org.springframework.security.oauth2.core.AbstractOAuth2Token; +import org.springframework.security.oauth2.core.AuthorizationGrantType; +import org.springframework.security.oauth2.core.ClaimAccessor; +import org.springframework.security.oauth2.core.ClientAuthenticationMethod; +import org.springframework.security.oauth2.core.OAuth2AccessToken; +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; +import org.springframework.security.oauth2.core.OAuth2Error; +import org.springframework.security.oauth2.core.OAuth2ErrorCodes; +import org.springframework.security.oauth2.core.OAuth2RefreshToken; +import org.springframework.security.oauth2.core.OAuth2Token; import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; import org.springframework.security.oauth2.core.oidc.OidcIdToken;