Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,14 @@
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.concurrent.TimeUnit;

import javax.cache.Cache;
import javax.cache.CacheManager;
import javax.cache.Caching;
import javax.cache.configuration.MutableConfiguration;
import javax.cache.expiry.CreatedExpiryPolicy;
import javax.cache.expiry.Duration;
import javax.cache.spi.CachingProvider;

import org.apache.cxf.Bus;
Expand All @@ -43,6 +46,13 @@

import static org.apache.cxf.jaxrs.utils.ResourceUtils.getClasspathResourceURL;

/**
* By default the client/access-token/refresh-token JCache caches are eternal at the cache
* infrastructure level: expired tokens are only ever removed when looked up (e.g. via
* {@link #getAccessToken}/{@link #getAccessTokens}), so entries that are never queried again
* accumulate for the life of the process. Pass a {@link CacheTTLs} to one of the constructors
* to have the JCache provider itself evict expired access/refresh token entries.
*/
public class JCacheOAuthDataProvider extends AbstractOAuthDataProvider {
public static final String CLIENT_CACHE_KEY = "cxf.oauth2.client.cache";
public static final String ACCESS_TOKEN_CACHE_KEY = "cxf.oauth2.accesstoken.cache";
Expand Down Expand Up @@ -86,18 +96,65 @@ public JCacheOAuthDataProvider(String configFileURL,
String accessTokenCacheKey,
String refreshTokenCacheKey,
boolean storeJwtTokenKeyOnly) {
this(configFileURL, bus, clientCacheKey, accessTokenCacheKey, refreshTokenCacheKey,
storeJwtTokenKeyOnly, CacheTTLs.ETERNAL);
}

// cacheTTLs lets the access/refresh token caches be evicted by the JCache infrastructure
// itself once entries age out, independently of any application-level expiry check;
// a TTL <= 0 (CacheTTLs.ETERNAL by default) leaves the corresponding cache eternal
public JCacheOAuthDataProvider(String configFileURL,

@reta reta Aug 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coheigea thanks for the change, what concerns me is that cacheTTLs gives a wrong impression that TTL will be configured for each cache, but that is not the case: if the configuration has dedicated cache preset (like below)

  <cache alias="cxf.oauth2.accesstoken.cache">
    <key-type>java.lang.String</key-type>
    <value-type>org.apache.cxf.rs.security.oauth2.common.ServerAccessToken</value-type>
    <heap unit="entries">100</heap>
    <jsr107:mbeans enable-management="false" enable-statistics="false"/>
  </cache>

the cacheTTL will be effectively ignored. I don't want to complicate it but also looking for intuitive API usage, may be instead of CacheTTLs cacheTTLs we introduce something like JCacheCreator (or alike) that will be used to create a cache when the is no one in configuration? Wdyt?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something along these lines:

interface JCacheCreator  {
    <K, V> Cache<K, V> createCache(CacheManager cacheManager, String cacheKey, MutableConfiguration<K, V> config);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @reta , maybe in that case we don't need this improvement at all if the user can configure the TTL in the config?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @coheigea , that is also a valid point, we may not even need it

Bus bus,
String clientCacheKey,
String accessTokenCacheKey,
String refreshTokenCacheKey,
boolean storeJwtTokenKeyOnly,
CacheTTLs cacheTTLs) {

cacheManager = createCacheManager(configFileURL, bus);
clientCache = createCache(cacheManager, clientCacheKey, String.class, Client.class);
// clients are persistent configuration, not time-bound tokens, so the cache stays eternal
clientCache = createCache(cacheManager, clientCacheKey, String.class, Client.class, -1);

this.storeJwtTokenKeyOnly = storeJwtTokenKeyOnly;
if (storeJwtTokenKeyOnly) {
jwtAccessTokenCache = createCache(cacheManager, accessTokenCacheKey, String.class, String.class);
jwtAccessTokenCache = createCache(cacheManager, accessTokenCacheKey, String.class, String.class,
cacheTTLs.getAccessTokenSeconds());
} else {
accessTokenCache = createCache(cacheManager, accessTokenCacheKey, String.class, ServerAccessToken.class);
accessTokenCache = createCache(cacheManager, accessTokenCacheKey, String.class, ServerAccessToken.class,
cacheTTLs.getAccessTokenSeconds());
}

refreshTokenCache = createCache(cacheManager, refreshTokenCacheKey, String.class, RefreshToken.class,
cacheTTLs.getRefreshTokenSeconds());
}

// immutable holder so the access/refresh token cache TTLs can be passed as a single
// constructor argument without exceeding the parameter-count limit
/**
* Cache-infrastructure-level time-to-live for the access/refresh token caches, in seconds.
* Not enabled by default: {@link #ETERNAL} (both values {@code -1}) means neither cache
* expires entries on its own, matching the pre-existing default behavior. A value {@code <= 0}
* for either field leaves that specific cache eternal, e.g. to match an eternal-by-default
* ({@code refreshTokenLifetime == 0}) refresh token configuration.
*/
public static final class CacheTTLs {
public static final CacheTTLs ETERNAL = new CacheTTLs(-1, -1);

private final long accessTokenSeconds;
private final long refreshTokenSeconds;

public CacheTTLs(long accessTokenSeconds, long refreshTokenSeconds) {
this.accessTokenSeconds = accessTokenSeconds;
this.refreshTokenSeconds = refreshTokenSeconds;
}

public long getAccessTokenSeconds() {
return accessTokenSeconds;
}

refreshTokenCache = createCache(cacheManager, refreshTokenCacheKey, String.class, RefreshToken.class);
public long getRefreshTokenSeconds() {
return refreshTokenSeconds;
}
}

@Override
Expand Down Expand Up @@ -299,16 +356,24 @@ protected static CacheManager createCacheManager(String configFile, Bus bus) {

protected static <K, V> Cache<K, V> createCache(CacheManager cacheManager,
String cacheKey, Class<K> keyType, Class<V> valueType) {
return createCache(cacheManager, cacheKey, keyType, valueType, -1);
}

protected static <K, V> Cache<K, V> createCache(CacheManager cacheManager,
String cacheKey, Class<K> keyType, Class<V> valueType,
long ttlSeconds) {

Cache<K, V> cache = cacheManager.getCache(cacheKey, keyType, valueType);
if (cache == null) {
cache = cacheManager.createCache(
cacheKey,
new MutableConfiguration<K, V>()
.setTypes(keyType, valueType)
.setStoreByValue(true)
.setStatisticsEnabled(false)
);
MutableConfiguration<K, V> configuration = new MutableConfiguration<K, V>()
.setTypes(keyType, valueType)
.setStoreByValue(true)
.setStatisticsEnabled(false);
if (ttlSeconds > 0) {
configuration.setExpiryPolicyFactory(
CreatedExpiryPolicy.factoryOf(new Duration(TimeUnit.SECONDS, ttlSeconds)));
}
cache = cacheManager.createCache(cacheKey, configuration);
}

return cache;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,22 @@
import java.util.Collections;
import java.util.List;

import javax.cache.Cache;
import javax.cache.configuration.CompleteConfiguration;
import javax.cache.expiry.Duration;

import org.apache.cxf.BusFactory;
import org.apache.cxf.rs.security.oauth2.common.AccessTokenRegistration;
import org.apache.cxf.rs.security.oauth2.common.Client;
import org.apache.cxf.rs.security.oauth2.common.ServerAccessToken;
import org.apache.cxf.rs.security.oauth2.tokens.refresh.RefreshToken;

import org.junit.Before;
import org.junit.Test;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;

public class JCacheOAuthDataProviderTest extends AbstractOAuthDataProviderTest {

Expand Down Expand Up @@ -61,4 +68,80 @@ public void testAddGetExpiredAccessToken() throws InterruptedException {

getProvider().removeClient(c.getClientId());
}

// Without an explicitly configured CacheTTLs, the access/refresh token caches stay eternal
// at the JCache infrastructure level (backwards-compatible default).
@Test
public void testCachesAreEternalByDefault() {
JCacheOAuthDataProvider provider = createIsolatedProvider("eternal", JCacheOAuthDataProvider.CacheTTLs.ETERNAL);

assertEquals(Duration.ETERNAL, expiryForCreation(rawAccessTokenCache(provider, "eternal")));
assertEquals(Duration.ETERNAL, expiryForCreation(rawRefreshTokenCache(provider, "eternal")));
}

// An access token that is never looked up again must still be reaped by the JCache
// infrastructure once the configured cache TTL elapses, independently of any
// application-level expiry check (CWE-400: otherwise abandoned entries never get evicted).
@Test
public void testUnaccessedExpiredAccessTokenEvictedFromCacheWhenTTLConfigured() throws InterruptedException {
JCacheOAuthDataProvider provider = createIsolatedProvider(
"ttl", new JCacheOAuthDataProvider.CacheTTLs(1 /* access token cache TTL, seconds */, -1));

Client c = addClient(provider, "103", "bob");
AccessTokenRegistration atr = new AccessTokenRegistration();
atr.setClient(c);
atr.setApprovedScope(Collections.singletonList("a"));
atr.setSubject(c.getResourceOwnerSubject());
// long-lived at the app level: proves the eviction is cache-driven, not isExpired()-driven
provider.setAccessTokenLifetime(3600);
ServerAccessToken at = provider.createAccessToken(atr);

Thread.sleep(2000); /* entry is never touched during this window */

assertNull("entry should have been evicted at the cache-infrastructure level",
rawAccessTokenCache(provider, "ttl").get(at.getTokenKey()));
}

// the CacheManager and its caches are shared per config file, so each test needs its own
// cache names; the provider is deliberately not closed, as that would also close the
// CacheManager still in use by the provider under test
private static JCacheOAuthDataProvider createIsolatedProvider(String suffix,
JCacheOAuthDataProvider.CacheTTLs ttls) {
JCacheOAuthDataProvider provider = new JCacheOAuthDataProvider(
JCacheOAuthDataProvider.DEFAULT_CONFIG_URL, BusFactory.getThreadDefaultBus(true),
JCacheOAuthDataProvider.CLIENT_CACHE_KEY + '.' + suffix,
JCacheOAuthDataProvider.ACCESS_TOKEN_CACHE_KEY + '.' + suffix,
JCacheOAuthDataProvider.REFRESH_TOKEN_CACHE_KEY + '.' + suffix,
false, ttls);
initializeProvider(provider);
return provider;
}

@SuppressWarnings("unchecked")
private static Duration expiryForCreation(Cache<?, ?> cache) {
CompleteConfiguration<?, ?> config = cache.getConfiguration(CompleteConfiguration.class);
return config.getExpiryPolicyFactory().create().getExpiryForCreation();
}

private static Cache<String, ServerAccessToken> rawAccessTokenCache(JCacheOAuthDataProvider provider,
String suffix) {
return provider.cacheManager.getCache(
JCacheOAuthDataProvider.ACCESS_TOKEN_CACHE_KEY + '.' + suffix, String.class, ServerAccessToken.class);
}

private static Cache<String, RefreshToken> rawRefreshTokenCache(JCacheOAuthDataProvider provider,
String suffix) {
return provider.cacheManager.getCache(
JCacheOAuthDataProvider.REFRESH_TOKEN_CACHE_KEY + '.' + suffix, String.class, RefreshToken.class);
}

private static Client addClient(JCacheOAuthDataProvider provider, String clientId, String userLogin) {
Client c = new Client();
c.setRedirectUris(Collections.singletonList("http://client/redirect"));
c.setClientId(clientId);
c.setClientSecret("123");
c.setResourceOwnerSubject(new org.apache.cxf.rs.security.oauth2.common.UserSubject(userLogin));
provider.setClient(c);
return c;
}
}
Loading