Add cache for User entities in OIDC auth flow

[njshah301]

Description

This change introduces an in-memory cache for User entities within the OIDC authentication flow to address the performance improvement.

The Problem

The system currently performs a database transaction to load a User entity for every OIDC authentication request. With a high volume of EPP "hello" commands and other session-less requests this results in a constant database load of around 50 QPS. This repeated, uncached lookup is inefficient.

The Solution

This pull request implements a per-pod LoadingCache to temporarily store User objects, keyed by their email address. This change reduces database load by serving authentication requests from a fast, in-memory cache. The database is now only queried on a cache miss.

The implementation includes:

• Caching Logic: Added to OidcTokenAuthenticationMechanism.java, the cache is built using the project's shared CacheUtil.
• Negative Caching: The cache correctly stores "not found" results (as Optional.empty) to prevent repeated lookups for invalid users.
• Configuration: The cache's behavior is controlled by new feature flags in the configuration files (RegistryConfig.java, RegistryConfigSettings.java, and default-config.yaml), allowing it to be enabled, disabled, or tuned per environment.
  • userAuthCachingEnabled
  • userAuthCachingSeconds
  • userAuthMaxCachedEntries

Testing

Comprehensive unit tests have been added to OidcTokenAuthenticationMechanismTest.java to verify the correctness of the caching implementation, including:

• Verification of cache hits for existing users.
• Verification of caching for non-existent users (negative caching).
• A test to ensure the cache is correctly bypassed when the feature is disabled via the userAuthCachingEnabled flag.

---

This change is 

[weiminyu]

Reviewable status: 0 of 5 files reviewed, 5 unresolved discussions

---

core/src/main/java/google/registry/request/auth/OidcTokenAuthenticationMechanism.java line 90 at r1 (raw file):

  @VisibleForTesting
  static LoadingCache<String, Optional<User>> userCache =
      CacheUtils.newCacheBuilder(getUserAuthCachingDuration())

When meaning of method name is clear and unique in this file, consider static import for readability.

Code quote:

newCacheBuilder

---

core/src/main/java/google/registry/request/auth/OidcTokenAuthenticationMechanism.java line 162 at r1 (raw file):

    } else {
      // If caching is OFF, fall back to the original direct database call.
      maybeUser = tm().transact(() -> tm().loadByKeyIfPresent(VKey.create(User.class, email)));

Consider make this a method for use here and by the cache loader.

Code quote:

tm().transact(() -> tm().loadByKeyIfPresent(VKey.create(User.class, email)))

---

core/src/main/java/google/registry/config/files/default-config.yaml line 341 at r1 (raw file):

  # The maximum number of User objects to store in the cache per pod.
  # This helps limit the memory footprint of the cache.
  userAuthMaxCachedEntries: 100

I think we can make this value a bit higher, say 200.

Code quote:

100

---

core/src/test/java/google/registry/request/auth/OidcTokenAuthenticationMechanismTest.java line 88 at r1 (raw file):

        CacheUtils.newCacheBuilder(getUserAuthCachingDuration())
            .maximumSize(getUserAuthMaxCachedEntries())
            .recordStats()

We shouldn't add features not needed in production.

If needed in tests you can add a helper to assign a new one.

Code quote:

.recordStats()

---

core/src/test/java/google/registry/request/auth/OidcTokenAuthenticationMechanismTest.java line 206 at r1 (raw file):

  @Test
  void testAuthenticate_ExistentUser_isCached() {

I would suggest that this method be split into two:

1. A functional test, just verify that authentication succeeded.
2. Implementation test, just verify that the cache is called. Getting into specific values of hits and misses is too much detail.

Code quote:

 void testAuthenticate_ExistentUser_isCached()

[njshah301]

refactored: Address review feedback

• Refactor database call into a single, reusable method
• Increase the default cache size to 200
• Remove .recordStats() and using spy for testing
• Split unit tests into separate implementation test that use Mockito spies instead of checking internal cache stats

Reviewable status: 0 of 5 files reviewed, 5 unresolved discussions (waiting on @weiminyu)

---

core/src/main/java/google/registry/config/files/default-config.yaml line 341 at r1 (raw file):

Previously, weiminyu (Weimin Yu) wrote…

I think we can make this value a bit higher, say 200.

Done with making the value 200

---

core/src/main/java/google/registry/request/auth/OidcTokenAuthenticationMechanism.java line 90 at r1 (raw file):

Previously, weiminyu (Weimin Yu) wrote…

When meaning of method name is clear and unique in this file, consider static import for readability.

added static import for newCacheBuilder

---

core/src/main/java/google/registry/request/auth/OidcTokenAuthenticationMechanism.java line 162 at r1 (raw file):

Previously, weiminyu (Weimin Yu) wrote…

Consider make this a method for use here and by the cache loader.

directly calling loadUser function

---

core/src/test/java/google/registry/request/auth/OidcTokenAuthenticationMechanismTest.java line 88 at r1 (raw file):

Previously, weiminyu (Weimin Yu) wrote…

We shouldn't add features not needed in production.

If needed in tests you can add a helper to assign a new one.

removed it from OidcTokenAuthenticationMechanism file and test file

---

core/src/test/java/google/registry/request/auth/OidcTokenAuthenticationMechanismTest.java line 206 at r1 (raw file):

Previously, weiminyu (Weimin Yu) wrote…

I would suggest that this method be split into two:

1. A functional test, just verify that authentication succeeded.
2. Implementation test, just verify that the cache is called. Getting into specific values of hits and misses is too much detail.

functional test to verify the authentication was already there so just updated implementation test and added only verifying the cache rather than specific hits and misses details

[weiminyu]

@weiminyu reviewed 2 of 5 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @njshah301)

[njshah301]

No action required

Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @njshah301)

---

-- commits line 10 at r2:
No action required