This is an automated email from the ASF dual-hosted git repository. rombert pushed a commit to branch issue/SLING-12999 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-auth-oauth-client.git
commit 6a28343fce0b448ca7ffed6bb3dc1d100caa3d2a Author: Robert Munteanu <[email protected]> AuthorDate: Tue Nov 11 23:29:18 2025 +0100 SLING-12999 - Invalid refresh tokens are not cleared Always clear access/refresh tokens in case the refreshing the token fails. --- .../oauth_client/impl/OAuthTokenRefresherImpl.java | 14 +++---- .../auth/oauth_client/impl/TokenAccessImpl.java | 23 ++++++++--- .../auth/oauth_client/TokenAccessImplTest.java | 44 ++++++++++++++++++++-- 3 files changed, 64 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/apache/sling/auth/oauth_client/impl/OAuthTokenRefresherImpl.java b/src/main/java/org/apache/sling/auth/oauth_client/impl/OAuthTokenRefresherImpl.java index 2d61849..0f9798c 100644 --- a/src/main/java/org/apache/sling/auth/oauth_client/impl/OAuthTokenRefresherImpl.java +++ b/src/main/java/org/apache/sling/auth/oauth_client/impl/OAuthTokenRefresherImpl.java @@ -27,6 +27,7 @@ import com.nimbusds.oauth2.sdk.ParseException; import com.nimbusds.oauth2.sdk.RefreshTokenGrant; import com.nimbusds.oauth2.sdk.TokenErrorResponse; import com.nimbusds.oauth2.sdk.TokenRequest; +import com.nimbusds.oauth2.sdk.TokenResponse; import com.nimbusds.oauth2.sdk.auth.ClientAuthentication; import com.nimbusds.oauth2.sdk.auth.ClientSecretBasic; import com.nimbusds.oauth2.sdk.auth.Secret; @@ -41,11 +42,12 @@ import org.osgi.service.component.annotations.Component; public class OAuthTokenRefresherImpl implements OAuthTokenRefresher { @Override - public @NotNull OAuthTokens refreshTokens(@NotNull ClientConnection connection, @NotNull String refreshToken) { + public @NotNull OAuthTokens refreshTokens(@NotNull ClientConnection connection, @NotNull String refreshToken) + throws OAuthException { return Converter.toSlingOAuthTokens(refreshTokensInternal(connection, refreshToken)); } - private static @NotNull Tokens refreshTokensInternal( + private @NotNull Tokens refreshTokensInternal( @NotNull ClientConnection connection, @NotNull String refreshTokenString) throws OAuthException { try { // Construct the grant from the saved refresh token @@ -65,14 +67,12 @@ public class OAuthTokenRefresherImpl implements OAuthTokenRefresher { // Make the token request TokenRequest request = new TokenRequest.Builder(tokenEndpoint, clientAuth, refreshTokenGrant).build(); - AccessTokenResponse response = - AccessTokenResponse.parse(request.toHTTPRequest().send()); + TokenResponse response = TokenResponse.parse(request.toHTTPRequest().send()); if (!response.indicatesSuccess()) { - // We got an error response... TokenErrorResponse errorResponse = response.toErrorResponse(); - throw new OAuthException("Failed refreshing the access token " - + errorResponse.getErrorObject().getCode() + " : " + throw new OAuthException("Failed refreshing the access token. Code: " + + errorResponse.getErrorObject().getCode() + ", description: " + errorResponse.getErrorObject().getDescription()); } diff --git a/src/main/java/org/apache/sling/auth/oauth_client/impl/TokenAccessImpl.java b/src/main/java/org/apache/sling/auth/oauth_client/impl/TokenAccessImpl.java index 80b29ce..3fb2562 100644 --- a/src/main/java/org/apache/sling/auth/oauth_client/impl/TokenAccessImpl.java +++ b/src/main/java/org/apache/sling/auth/oauth_client/impl/TokenAccessImpl.java @@ -81,15 +81,26 @@ public class TokenAccessImpl implements OAuthTokenAccess { request.getUserPrincipal()); } - OAuthTokens newTokens = tokenRefresher.refreshTokens(connection, refreshToken.getValue()); - if (newTokens.refreshToken() == null) { - // retain old refresh token if none was returned - newTokens = - new OAuthTokens(newTokens.accessToken(), newTokens.expiresAt(), refreshToken.getValue()); + OAuthTokens newTokens; + try { + newTokens = tokenRefresher.refreshTokens(connection, refreshToken.getValue()); + if (newTokens.refreshToken() == null) { + // retain old refresh token if none was returned but call was successful + newTokens = new OAuthTokens( + newTokens.accessToken(), newTokens.expiresAt(), refreshToken.getValue()); + } + } catch (OAuthException e) { + logger.warn( + "Failed to refresh access token for connection {} and user {}. Clearing all tokens", + connection.name(), + request.getRemoteUser(), + e); + newTokens = new OAuthTokens(null, 0, null); } tokenStore.persistTokens(connection, resolver, newTokens); - return new OAuthTokenResponse(Optional.of(newTokens.accessToken()), connection, request, redirectPath); + return new OAuthTokenResponse( + Optional.ofNullable(newTokens.accessToken()), connection, request, redirectPath); } } diff --git a/src/test/java/org/apache/sling/auth/oauth_client/TokenAccessImplTest.java b/src/test/java/org/apache/sling/auth/oauth_client/TokenAccessImplTest.java index 8beb4bf..5549ebc 100644 --- a/src/test/java/org/apache/sling/auth/oauth_client/TokenAccessImplTest.java +++ b/src/test/java/org/apache/sling/auth/oauth_client/TokenAccessImplTest.java @@ -19,10 +19,13 @@ package org.apache.sling.auth.oauth_client; import org.apache.sling.auth.oauth_client.impl.MockOidcConnection; +import org.apache.sling.auth.oauth_client.impl.OAuthException; +import org.apache.sling.auth.oauth_client.impl.OAuthToken; import org.apache.sling.auth.oauth_client.impl.OAuthTokenRefresher; import org.apache.sling.auth.oauth_client.impl.OAuthTokenStore; import org.apache.sling.auth.oauth_client.impl.OAuthTokens; import org.apache.sling.auth.oauth_client.impl.TokenAccessImpl; +import org.apache.sling.auth.oauth_client.impl.TokenState; import org.apache.sling.testing.mock.sling.junit5.SlingContext; import org.apache.sling.testing.mock.sling.junit5.SlingContextExtension; import org.jetbrains.annotations.NotNull; @@ -87,7 +90,7 @@ class TokenAccessImplTest { OAuthTokenStore tokenStore = new InMemoryOAuthTokenStore(); - TokenAccessImpl tokenAccess = getTokenAccess(expiredTokens, refreshedTokens, tokenStore); + TokenAccessImpl tokenAccess = getTokenAccess(expiredTokens.refreshToken(), refreshedTokens, tokenStore); tokenStore.persistTokens(MockOidcConnection.DEFAULT_CONNECTION, slingContext.resourceResolver(), expiredTokens); @@ -108,13 +111,14 @@ class TokenAccessImplTest { } private static @NotNull TokenAccessImpl getTokenAccess( - OAuthTokens expiredTokens, OAuthTokens refreshedTokens, OAuthTokenStore tokenStore) { + String expectedRefreshToken, OAuthTokens refreshedTokens, OAuthTokenStore tokenStore) { OAuthTokenRefresher tokenRefresher = new OAuthTokenRefresher() { @Override public @NotNull OAuthTokens refreshTokens( @NotNull ClientConnection connection, @NotNull String refreshToken) { - if (!refreshToken.equals(expiredTokens.refreshToken())) { - throw new IllegalArgumentException("Invalid refresh token"); + if (!refreshToken.equals(expectedRefreshToken)) { + throw new OAuthException("Invalid refresh token. Expected '" + expectedRefreshToken + "' but got '" + + refreshToken + "'"); } return refreshedTokens; } @@ -169,4 +173,36 @@ class TokenAccessImplTest { assertThat(tokenStore.allTokens()).as("all persisted tokens").isEmpty(); } + + @Test + void refreshToken_invalid() { + + OAuthTokens expiredTokens = new OAuthTokens("access", -1, "refresh"); + OAuthTokens refreshedTokens = new OAuthTokens("access2", 0, null); + + OAuthTokenStore tokenStore = new InMemoryOAuthTokenStore(); + + // ensure that the refresher fails when an unexpected refresh token is used + TokenAccessImpl tokenAccess = + getTokenAccess("NOT " + expiredTokens.refreshToken(), refreshedTokens, tokenStore); + + tokenStore.persistTokens(MockOidcConnection.DEFAULT_CONNECTION, slingContext.resourceResolver(), expiredTokens); + + OAuthTokenResponse tokenResponse = + tokenAccess.getAccessToken(MockOidcConnection.DEFAULT_CONNECTION, slingContext.request(), "/"); + + assertThat(tokenResponse).as("tokenResponse").isNotNull().satisfies(tr -> { + assertThat(tr.hasValidToken()).as("hasValidToken").isFalse(); + }); + + assertThat(tokenStore.getRefreshToken(MockOidcConnection.DEFAULT_CONNECTION, slingContext.resourceResolver())) + .as("refresh token after failed refresh") + .extracting(OAuthToken::getState) + .isEqualTo(TokenState.MISSING); + + assertThat(tokenStore.getAccessToken(MockOidcConnection.DEFAULT_CONNECTION, slingContext.resourceResolver())) + .as("acess token after failed refresh") + .extracting(OAuthToken::getState) + .isEqualTo(TokenState.MISSING); + } }
