diff --git a/core/src/main/java/google/registry/config/RegistryConfig.java b/core/src/main/java/google/registry/config/RegistryConfig.java index 1ff246049..f5421975d 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 final class RegistryConfig { 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 185f5106b..32dd08ee8 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 class RegistryConfigSettings { 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 57e6bea62..03828e34a 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 601f7dcbc..65fe13739 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 @@ public abstract class OidcTokenAuthenticationMechanism implements Authentication 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,23 @@ public abstract class OidcTokenAuthenticationMechanism implements Authentication 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); - } + // Short-circuit the user lookup for known service accounts. + // This check bypasses the database lookup for high-volume + // traffic from trusted system accounts to reduce database load. + + if (serviceAccountEmails.contains(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 90d3416bb..bcff9e6e2 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 @@ package google.registry.request.auth; 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.Component; 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.request.auth.OidcTokenAuthenticationMechanism.RegularOidc 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 @@ public class OidcTokenAuthenticationMechanismTest { 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 @@ public class OidcTokenAuthenticationMechanismTest { 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 @@ public class OidcTokenAuthenticationMechanismTest { return GoogleCredentialsBundle.create(GoogleCredentials.newBuilder().build()); } } - - private void reinitializeCache() { - OidcTokenAuthenticationMechanism.userCache = - CacheUtils.newCacheBuilder(getUserAuthCachingDuration()) - .maximumSize(getUserAuthMaxCachedEntries()) - .recordStats() - .build(OidcTokenAuthenticationMechanism::loadUser); - } }