From 71312afd5f36daf2968edc173883993a7856bdbf Mon Sep 17 00:00:00 2001 From: njshah301 Date: Tue, 16 Sep 2025 11:10:44 +0000 Subject: [PATCH] Skip user loading for proxy service account Reduces database load by skipping the User entity lookup for the proxy service account during OIDC authentication. The high volume of EPP "hello" and "login" commands from the proxy service account results in a constant database load. These lookups are unnecessary as the proxy service account is not expected to have a corresponding User object. This change optimizes the authentication flow by checking for the proxy service account email *before* attempting to load a User from the database. This bypasses the database transaction entirely for these high-volume requests. This approach is more efficient than caching, as it eliminates the database lookup for the proxy service account altogether, rather than just caching the result. --- .../registry/config/RegistryConfig.java | 20 ----- .../config/RegistryConfigSettings.java | 3 - .../registry/config/files/default-config.yaml | 14 ---- .../OidcTokenAuthenticationMechanism.java | 58 ++----------- .../OidcTokenAuthenticationMechanismTest.java | 83 +------------------ 5 files changed, 7 insertions(+), 171 deletions(-) diff --git a/core/src/main/java/google/registry/config/RegistryConfig.java b/core/src/main/java/google/registry/config/RegistryConfig.java index 1ff246049cb..f5421975d10 100644 --- a/core/src/main/java/google/registry/config/RegistryConfig.java +++ b/core/src/main/java/google/registry/config/RegistryConfig.java @@ -1591,26 +1591,6 @@ public static int getEppResourceMaxCachedEntries() { return CONFIG_SETTINGS.get().caching.eppResourceMaxCachedEntries; } - /** Returns if we have enabled caching for User Authentication */ - public static boolean getUserAuthCachingEnabled() { - return CONFIG_SETTINGS.get().caching.userAuthCachingEnabled; - } - - @VisibleForTesting - public static void overrideIsUserAuthCachingEnabledForTesting(boolean enabled) { - CONFIG_SETTINGS.get().caching.userAuthCachingEnabled = enabled; - } - - /** Returns the expiry duration for the user authentication cache. */ - public static java.time.Duration getUserAuthCachingDuration() { - return java.time.Duration.ofSeconds(CONFIG_SETTINGS.get().caching.userAuthCachingSeconds); - } - - /** Returns the maximum number of entries in user authentication cache. */ - public static int getUserAuthMaxCachedEntries() { - return CONFIG_SETTINGS.get().caching.userAuthMaxCachedEntries; - } - /** Returns the amount of time that a particular claims list should be cached. */ public static java.time.Duration getClaimsListCacheDuration() { return java.time.Duration.ofSeconds(CONFIG_SETTINGS.get().caching.claimsListCachingSeconds); diff --git a/core/src/main/java/google/registry/config/RegistryConfigSettings.java b/core/src/main/java/google/registry/config/RegistryConfigSettings.java index 185f5106b7b..32dd08ee84d 100644 --- a/core/src/main/java/google/registry/config/RegistryConfigSettings.java +++ b/core/src/main/java/google/registry/config/RegistryConfigSettings.java @@ -161,9 +161,6 @@ public static class Caching { public int eppResourceCachingSeconds; public int eppResourceMaxCachedEntries; public int claimsListCachingSeconds; - public boolean userAuthCachingEnabled; - public int userAuthCachingSeconds; - public int userAuthMaxCachedEntries; } /** Configuration for ICANN monthly reporting. */ diff --git a/core/src/main/java/google/registry/config/files/default-config.yaml b/core/src/main/java/google/registry/config/files/default-config.yaml index 57e6bea627e..03828e34a3e 100644 --- a/core/src/main/java/google/registry/config/files/default-config.yaml +++ b/core/src/main/java/google/registry/config/files/default-config.yaml @@ -326,20 +326,6 @@ caching: # long duration is acceptable because claims lists don't change frequently. claimsListCachingSeconds: 21600 # six hours - #-- User Authentication Cache Settings --# - - # Whether to cache User objects during OIDC token authentication to reduce database load. - # This helps mitigate high QPS from frequent hello commands and session-less requests. - userAuthCachingEnabled: true - - # The duration in seconds for which a User object is cached after being loaded. - # A short duration is recommended to avoid stale data. - userAuthCachingSeconds: 60 - - # The maximum number of User objects to store in the cache per pod. - # This helps limit the memory footprint of the cache. - userAuthMaxCachedEntries: 200 - # Note: Only allowedServiceAccountEmails and oauthClientId should be configured. # Other fields are related to OAuth-based authentication and will be removed. auth: diff --git a/core/src/main/java/google/registry/request/auth/OidcTokenAuthenticationMechanism.java b/core/src/main/java/google/registry/request/auth/OidcTokenAuthenticationMechanism.java index 601f7dcbc09..768882d117f 100644 --- a/core/src/main/java/google/registry/request/auth/OidcTokenAuthenticationMechanism.java +++ b/core/src/main/java/google/registry/request/auth/OidcTokenAuthenticationMechanism.java @@ -15,20 +15,14 @@ package google.registry.request.auth; import static com.google.common.base.Preconditions.checkState; -import static google.registry.config.RegistryConfig.getUserAuthCachingDuration; -import static google.registry.config.RegistryConfig.getUserAuthCachingEnabled; -import static google.registry.config.RegistryConfig.getUserAuthMaxCachedEntries; -import static google.registry.model.CacheUtils.newCacheBuilder; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; -import com.github.benmanes.caffeine.cache.LoadingCache; import com.google.api.client.json.webtoken.JsonWebSignature; import com.google.auth.oauth2.TokenVerifier.VerificationException; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Splitter; import com.google.common.collect.ImmutableSet; import com.google.common.flogger.FluentLogger; -import google.registry.config.RegistryConfig; import google.registry.config.RegistryConfig.Config; import google.registry.model.console.User; import google.registry.persistence.VKey; @@ -77,40 +71,6 @@ protected OidcTokenAuthenticationMechanism( this.tokenVerifier = tokenVerifier; } - /** - * An in-memory cache for User entities, built using the project's standard utility. - * - *

This cache reduces database load by temporarily storing User objects after they are fetched. - * It is configured to cache negative results (i.e., when a user is not found) to prevent repeated - * lookups for invalid users. The cache's behavior (enabled, expiry, size) is controlled by - * settings in {@link RegistryConfig}. - */ - @VisibleForTesting - static LoadingCache> userCache = - newCacheBuilder(getUserAuthCachingDuration()) - .maximumSize(getUserAuthMaxCachedEntries()) - .build(OidcTokenAuthenticationMechanism::loadUser); - - /** - * A loader function that defines how to fetch a User from the database on a cache miss. - * - *

This is the single point of entry to the database for this authentication flow. It will only - * be invoked by the cache when a requested user is not already in memory. - */ - @VisibleForTesting - static Optional loadUser(String email) { - VKey userVKey = VKey.create(User.class, email); - return tm().transact(() -> tm().loadByKeyIfPresent(userVKey)); - } - - @VisibleForTesting - public static void setCacheForTesting(LoadingCache> cache) { - checkState( - RegistryEnvironment.get() == RegistryEnvironment.UNITTEST, - "Cannot set cache outside of a test environment"); - OidcTokenAuthenticationMechanism.userCache = cache; - } - @Override public AuthResult authenticate(HttpServletRequest request) { if (RegistryEnvironment.get().equals(RegistryEnvironment.UNITTEST) @@ -152,24 +112,18 @@ public AuthResult authenticate(HttpServletRequest request) { logger.atInfo().log("No email address from the OIDC token:\n%s", token.getPayload()); return AuthResult.NOT_AUTHENTICATED; } - Optional maybeUser; - if (getUserAuthCachingEnabled()) { - // If caching is ON, use the cache. - maybeUser = userCache.get(email); - } else { - // If caching is OFF, fall back to the original direct database call. - maybeUser = loadUser(email); + if (serviceAccountEmails.stream().anyMatch(e -> e.equals(email))) { + return AuthResult.createApp(email); } - + logger.atInfo().log("No service account found for email address %s, loading the User", email); + Optional maybeUser = + tm().transact(() -> tm().loadByKeyIfPresent(VKey.create(User.class, email))); stopwatch.tick("OidcTokenAuthenticationMechanism maybeUser loaded"); if (maybeUser.isPresent()) { return AuthResult.createUser(maybeUser.get()); } logger.atInfo().log("No end user found for email address %s", email); - if (serviceAccountEmails.stream().anyMatch(e -> e.equals(email))) { - return AuthResult.createApp(email); - } - logger.atInfo().log("No service account found for email address %s", email); + logger.atWarning().log( "The email address %s is not tied to a principal with access to Nomulus", email); return AuthResult.NOT_AUTHENTICATED; diff --git a/core/src/test/java/google/registry/request/auth/OidcTokenAuthenticationMechanismTest.java b/core/src/test/java/google/registry/request/auth/OidcTokenAuthenticationMechanismTest.java index 90d3416bb72..bcff9e6e24a 100644 --- a/core/src/test/java/google/registry/request/auth/OidcTokenAuthenticationMechanismTest.java +++ b/core/src/test/java/google/registry/request/auth/OidcTokenAuthenticationMechanismTest.java @@ -16,20 +16,13 @@ import static com.google.common.net.HttpHeaders.AUTHORIZATION; import static com.google.common.truth.Truth.assertThat; -import static google.registry.config.RegistryConfig.getUserAuthCachingDuration; -import static google.registry.config.RegistryConfig.getUserAuthMaxCachedEntries; import static google.registry.request.auth.AuthModule.BEARER_PREFIX; import static google.registry.request.auth.AuthModule.IAP_HEADER_NAME; import static google.registry.testing.DatabaseHelper.createAdminUser; import static google.registry.testing.DatabaseHelper.persistResource; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import com.github.benmanes.caffeine.cache.LoadingCache; import com.google.api.client.googleapis.auth.oauth2.GoogleIdToken.Payload; import com.google.api.client.json.webtoken.JsonWebSignature; import com.google.api.client.json.webtoken.JsonWebSignature.Header; @@ -40,9 +33,7 @@ import dagger.Module; import dagger.Provides; import google.registry.config.CredentialModule.ApplicationDefaultCredential; -import google.registry.config.RegistryConfig; import google.registry.config.RegistryConfig.Config; -import google.registry.model.CacheUtils; import google.registry.model.console.GlobalRole; import google.registry.model.console.User; import google.registry.model.console.UserRoles; @@ -53,7 +44,6 @@ import google.registry.util.GoogleCredentialsBundle; import jakarta.inject.Singleton; import jakarta.servlet.http.HttpServletRequest; -import java.util.Optional; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -88,12 +78,6 @@ public class OidcTokenAuthenticationMechanismTest { @BeforeEach void beforeEach() throws Exception { - // 1. Create a brand new cache. - LoadingCache> testCache = - CacheUtils.newCacheBuilder(getUserAuthCachingDuration()) - .maximumSize(getUserAuthMaxCachedEntries()) - .build(OidcTokenAuthenticationMechanism::loadUser); - OidcTokenAuthenticationMechanism.setCacheForTesting(testCache); payload.setEmail(email); payload.setSubject(gaiaId); user = createAdminUser(email); @@ -167,8 +151,7 @@ void testAuthenticate_bothUserAndServiceAccount() throws Exception { payload.setEmail("service@email.test"); authResult = authenticationMechanism.authenticate(request); assertThat(authResult.isAuthenticated()).isTrue(); - assertThat(authResult.authLevel()).isEqualTo(AuthLevel.USER); - assertThat(authResult.user().get()).isEqualTo(serviceUser); + assertThat(authResult.authLevel()).isEqualTo(AuthLevel.APP); } @Test @@ -208,62 +191,6 @@ private void useRegularOidcMechanism() { authenticationMechanism = component.regularOidcAuthenticationMechanism(); } - @Test - void testAuthenticate_ExistentUser_isCached() { - // Arrange: Create a spy of the actual cache object. - // A spy calls the real methods of the object while allowing us to verify interactions. - LoadingCache> spiedCache = - spy(OidcTokenAuthenticationMechanism.userCache); - OidcTokenAuthenticationMechanism.setCacheForTesting(spiedCache); - - // Act: Call the authenticate method. - authenticationMechanism.authenticate(request); - - // Assert: Verify that the cache's "get" method was called exactly once. - // This confirms the cache is being used without checking its internal stats. - verify(spiedCache).get(email); - } - - @Test - void testAuthenticate_nonExistentUser_isCached() { - // Arrange: Use an email that is not in the test database. - - payload.setEmail(unknownEmail); - - LoadingCache> spiedCache = - spy(OidcTokenAuthenticationMechanism.userCache); - OidcTokenAuthenticationMechanism.setCacheForTesting(spiedCache); - // Act: Call the authenticate method. - authenticationMechanism.authenticate(request); - - // Assert: Verify that the cache's "get" method was called for the unverified email. - // This confirms that we attempted to look up the unknown user in the cache. - verify(spiedCache).get(unknownEmail); - } - - @Test - void testAuthenticate_whenCacheIsDisabled_cacheIsNotUsed() { - // Arrange: Explicitly disable the cache and create a spy. - RegistryConfig.overrideIsUserAuthCachingEnabledForTesting(false); - LoadingCache> spiedCache = - spy(OidcTokenAuthenticationMechanism.userCache); - OidcTokenAuthenticationMechanism.setCacheForTesting(spiedCache); - - // Act: Authenticate the user. - AuthResult authResult = authenticationMechanism.authenticate(request); - - // Assert: The authentication should still succeed because the code falls back - // to the direct database call. - assertThat(authResult.isAuthenticated()).isTrue(); - - // Assert: Crucially, verify that the cache's "get" method was NEVER called. - // This proves the cache was correctly bypassed. - verify(spiedCache, never()).get(any(String.class)); - - // Teardown: Restore the default setting for other tests. - RegistryConfig.overrideIsUserAuthCachingEnabledForTesting(true); - } - @Singleton @Component(modules = {AuthModule.class, TestModule.class}) interface TestComponent { @@ -309,12 +236,4 @@ GoogleCredentialsBundle provideGoogleCredentialBundle() { return GoogleCredentialsBundle.create(GoogleCredentials.newBuilder().build()); } } - - private void reinitializeCache() { - OidcTokenAuthenticationMechanism.userCache = - CacheUtils.newCacheBuilder(getUserAuthCachingDuration()) - .maximumSize(getUserAuthMaxCachedEntries()) - .recordStats() - .build(OidcTokenAuthenticationMechanism::loadUser); - } }