This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/lock-revoke in repository https://gitbox.apache.org/repos/asf/cxf.git
commit 07022777f9bee8e8430edd4ce3a3e5d0cdf7f667 Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Wed Jul 29 12:57:25 2026 +0100 Add a lock when revoking tokens --- .../oauth2/provider/AbstractOAuthDataProvider.java | 11 +++ .../DefaultEncryptingOAuthDataProviderTest.java | 110 +++++++++++++++++++++ 2 files changed, 121 insertions(+) diff --git a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/AbstractOAuthDataProvider.java b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/AbstractOAuthDataProvider.java index d425d3c8fbe..5688d5609f1 100644 --- a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/AbstractOAuthDataProvider.java +++ b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/AbstractOAuthDataProvider.java @@ -271,6 +271,17 @@ public abstract class AbstractOAuthDataProvider implements OAuthDataProvider, Cl @Override public void revokeToken(Client client, UserSubject callerSubject, String tokenKey, String tokenTypeHint) throws OAuthServiceException { + if (!recycleRefreshTokens) { + synchronized (refreshTokenLock) { + doRevokeToken(client, callerSubject, tokenKey, tokenTypeHint); + } + return; + } + doRevokeToken(client, callerSubject, tokenKey, tokenTypeHint); + } + + private void doRevokeToken(Client client, UserSubject callerSubject, + String tokenKey, String tokenTypeHint) { ServerAccessToken accessToken = null; if (!OAuthConstants.REFRESH_TOKEN.equals(tokenTypeHint)) { accessToken = revokeAccessToken(client, callerSubject, tokenKey); diff --git a/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/DefaultEncryptingOAuthDataProviderTest.java b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/DefaultEncryptingOAuthDataProviderTest.java index 24025b1fa85..d181ae719ec 100644 --- a/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/DefaultEncryptingOAuthDataProviderTest.java +++ b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/DefaultEncryptingOAuthDataProviderTest.java @@ -20,6 +20,10 @@ package org.apache.cxf.rs.security.oauth2.provider; import java.util.Arrays; import java.util.Collections; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import org.apache.cxf.rs.security.oauth2.common.AccessTokenRegistration; import org.apache.cxf.rs.security.oauth2.common.Client; @@ -35,9 +39,43 @@ import org.junit.Test; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; public class DefaultEncryptingOAuthDataProviderTest { + private static class BlockingRefreshProvider extends DefaultEncryptingOAuthDataProvider { + private final CountDownLatch enteredUpdate = new CountDownLatch(1); + private final CountDownLatch allowUpdate = new CountDownLatch(1); + + BlockingRefreshProvider() { + super(new KeyProperties("AES", 128)); + } + + @Override + protected RefreshToken updateExistingRefreshToken(RefreshToken rt, ServerAccessToken at) { + enteredUpdate.countDown(); + try { + if (!allowUpdate.await(5, TimeUnit.SECONDS)) { + throw new OAuthServiceException("Timed out waiting to continue refresh update"); + } + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + throw new OAuthServiceException("Interrupted while waiting to continue refresh update", ex); + } + // For this lock-ordering regression we only need to hold and release the lock, + // not to exercise refresh-token list mutation semantics. + return rt; + } + + boolean awaitEnteredUpdate(long timeout, TimeUnit unit) throws InterruptedException { + return enteredUpdate.await(timeout, unit); + } + + void continueUpdate() { + allowUpdate.countDown(); + } + } + private DefaultEncryptingOAuthDataProvider provider; @Before @@ -264,4 +302,76 @@ public class DefaultEncryptingOAuthDataProviderTest { assertNull("Revoked refresh token must return null, preventing token minting", provider.getRefreshToken(refreshTokenKey)); } + + @Test + public void testRevokeWaitsForRefreshWhenRecycleDisabled() throws Exception { + BlockingRefreshProvider blockingProvider = new BlockingRefreshProvider(); + try { + blockingProvider.setSupportedScopes(Collections.singletonMap("read", "Read Scope")); + blockingProvider.setSupportedScopes(Collections.singletonMap("refreshToken", "RefreshToken")); + blockingProvider.setRecycleRefreshTokens(false); + + Client c = new Client(); + c.setRedirectUris(Collections.singletonList("http://client/redirect";)); + c.setClientId("race-client"); + c.setClientSecret("secret"); + c.setResourceOwnerSubject(new UserSubject("race-user")); + blockingProvider.setClient(c); + + AccessTokenRegistration atr = new AccessTokenRegistration(); + atr.setClient(c); + atr.setApprovedScope(Arrays.asList("read", "refreshToken")); + atr.setSubject(c.getResourceOwnerSubject()); + + ServerAccessToken initial = blockingProvider.createAccessToken(atr); + String refreshTokenKey = initial.getRefreshToken(); + + AtomicReference<Throwable> refreshFailure = new AtomicReference<>(); + AtomicReference<Throwable> revokeFailure = new AtomicReference<>(); + + Thread refreshThread = new Thread(() -> { + try { + blockingProvider.refreshAccessToken(c, refreshTokenKey, Collections.emptyList()); + } catch (Throwable ex) { + refreshFailure.set(ex); + } + }); + + refreshThread.start(); + assertTrue("Refresh thread should reach the refresh-token update phase", + blockingProvider.awaitEnteredUpdate(5, TimeUnit.SECONDS)); + + Thread revokeThread = new Thread(() -> { + try { + blockingProvider.revokeToken(c, refreshTokenKey, OAuthConstants.REFRESH_TOKEN); + } catch (Throwable ex) { + revokeFailure.set(ex); + } + }); + + revokeThread.start(); + revokeThread.join(200); + assertTrue("Revoke must block while refresh holds refreshTokenLock", revokeThread.isAlive()); + + blockingProvider.continueUpdate(); + + refreshThread.join(5000); + revokeThread.join(5000); + + assertTrue("Refresh thread must finish", !refreshThread.isAlive()); + assertTrue("Revoke thread must finish after refresh releases lock", !revokeThread.isAlive()); + assertNull("Refresh thread must not fail", refreshFailure.get()); + assertNull("Revoke thread must not fail", revokeFailure.get()); + assertNull("Refresh token should be revoked after revoke thread completes", + blockingProvider.getRefreshToken(refreshTokenKey)); + + List<ServerAccessToken> remaining = blockingProvider.getAccessTokens(c, c.getResourceOwnerSubject()); + for (ServerAccessToken token : remaining) { + assertNull("No remaining access token should reference a revoked refresh token", + token.getRefreshToken()); + } + } finally { + blockingProvider.close(); + } + } }
