From 779d0c9d3772d1e15eb2baa9bce46d70f84bcb3c Mon Sep 17 00:00:00 2001 From: Lai Jiang Date: Thu, 9 Nov 2023 16:05:14 -0500 Subject: [PATCH] Add a fallback token verifier (#2216) This allows us to switch the proxy to a different client ID without disrupting the service. This is a temporary measure and will be removed once the switch is complete. --- .../registry/config/RegistryConfig.java | 6 ++++ .../config/RegistryConfigSettings.java | 1 + .../registry/config/files/default-config.yaml | 4 +++ .../registry/request/auth/AuthModule.java | 11 +++++++ .../OidcTokenAuthenticationMechanism.java | 31 ++++++++++++++++--- .../OidcTokenAuthenticationMechanismTest.java | 28 ++++++++++++++--- 6 files changed, 72 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/google/registry/config/RegistryConfig.java b/core/src/main/java/google/registry/config/RegistryConfig.java index 3a6a2295a7d..0a2ec5e2593 100644 --- a/core/src/main/java/google/registry/config/RegistryConfig.java +++ b/core/src/main/java/google/registry/config/RegistryConfig.java @@ -1192,6 +1192,12 @@ public static String provideOauthClientId(RegistryConfigSettings config) { return config.auth.oauthClientId; } + @Provides + @Config("fallbackOauthClientId") + public static String provideFallbackOauthClientId(RegistryConfigSettings config) { + return config.auth.fallbackOauthClientId; + } + /** * Provides the OAuth scopes required for accessing Google APIs using the default credential. */ diff --git a/core/src/main/java/google/registry/config/RegistryConfigSettings.java b/core/src/main/java/google/registry/config/RegistryConfigSettings.java index efcfe14e627..4d7f07ee4ef 100644 --- a/core/src/main/java/google/registry/config/RegistryConfigSettings.java +++ b/core/src/main/java/google/registry/config/RegistryConfigSettings.java @@ -61,6 +61,7 @@ public static class GcpProject { public static class Auth { public List allowedServiceAccountEmails; public String oauthClientId; + public String fallbackOauthClientId; } /** Configuration options for accessing Google APIs. */ 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 83c94405309..401683b0aa6 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 @@ -321,6 +321,10 @@ auth: # the same as this one. oauthClientId: iap-oauth-clientid + # Same as above, but serve as a fallback, so we can switch the client ID of + # the proxy without downtime. + fallbackOauthClientId: fallback-oauth-clientid + credentialOAuth: # OAuth scopes required for accessing Google APIs using the default # credential. diff --git a/core/src/main/java/google/registry/request/auth/AuthModule.java b/core/src/main/java/google/registry/request/auth/AuthModule.java index 1ac8d8cb772..671f1cc7054 100644 --- a/core/src/main/java/google/registry/request/auth/AuthModule.java +++ b/core/src/main/java/google/registry/request/auth/AuthModule.java @@ -55,6 +55,9 @@ ImmutableList provideApiAuthenticationMechanisms( @Qualifier @interface RegularOidc {} + @Qualifier + @interface RegularOidcFallback {} + @Provides @IapOidc @Singleton @@ -71,6 +74,14 @@ TokenVerifier provideRegularTokenVerifier(@Config("oauthClientId") String client return TokenVerifier.newBuilder().setAudience(clientId).setIssuer(REGULAR_ISSUER_URL).build(); } + @Provides + @RegularOidcFallback + @Singleton + TokenVerifier provideFallbackRegularTokenVerifier( + @Config("fallbackOauthClientId") String clientId) { + return TokenVerifier.newBuilder().setAudience(clientId).setIssuer(REGULAR_ISSUER_URL).build(); + } + @Provides @IapOidc @Singleton 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 7a5f31f9bc1..928c70074cd 100644 --- a/core/src/main/java/google/registry/request/auth/OidcTokenAuthenticationMechanism.java +++ b/core/src/main/java/google/registry/request/auth/OidcTokenAuthenticationMechanism.java @@ -25,6 +25,7 @@ import google.registry.model.console.UserDao; import google.registry.request.auth.AuthModule.IapOidc; import google.registry.request.auth.AuthModule.RegularOidc; +import google.registry.request.auth.AuthModule.RegularOidcFallback; import google.registry.request.auth.AuthSettings.AuthLevel; import java.util.Optional; import javax.annotation.Nullable; @@ -53,6 +54,8 @@ public abstract class OidcTokenAuthenticationMechanism implements Authentication protected final TokenVerifier tokenVerifier; + protected final Optional fallbackTokenVerifier; + protected final TokenExtractor tokenExtractor; private final ImmutableSet serviceAccountEmails; @@ -60,9 +63,11 @@ public abstract class OidcTokenAuthenticationMechanism implements Authentication protected OidcTokenAuthenticationMechanism( ImmutableSet serviceAccountEmails, TokenVerifier tokenVerifier, + @Nullable TokenVerifier fallbackTokenVerifier, TokenExtractor tokenExtractor) { this.serviceAccountEmails = serviceAccountEmails; this.tokenVerifier = tokenVerifier; + this.fallbackTokenVerifier = Optional.ofNullable(fallbackTokenVerifier); this.tokenExtractor = tokenExtractor; } @@ -77,7 +82,7 @@ public AuthResult authenticate(HttpServletRequest request) { if (rawIdToken == null) { return AuthResult.NOT_AUTHENTICATED; } - JsonWebSignature token; + JsonWebSignature token = null; try { token = tokenVerifier.verify(rawIdToken); } catch (Exception e) { @@ -86,8 +91,25 @@ public AuthResult authenticate(HttpServletRequest request) { RegistryEnvironment.get().equals(RegistryEnvironment.PRODUCTION) ? "Raw token redacted in prod" : rawIdToken); - return AuthResult.NOT_AUTHENTICATED; } + + if (token == null) { + if (fallbackTokenVerifier.isPresent()) { + try { + token = fallbackTokenVerifier.get().verify(rawIdToken); + } catch (Exception e) { + logger.atInfo().withCause(e).log( + "Failed OIDC fallback verification attempt:\n%s", + RegistryEnvironment.get().equals(RegistryEnvironment.PRODUCTION) + ? "Raw token redacted in prod" + : rawIdToken); + return AuthResult.NOT_AUTHENTICATED; + } + } else { + return AuthResult.NOT_AUTHENTICATED; + } + } + String email = (String) token.getPayload().get("email"); if (email == null) { logger.atWarning().log("No email address from the OIDC token:\n%s", token.getPayload()); @@ -141,7 +163,7 @@ protected IapOidcAuthenticationMechanism( @Config("allowedServiceAccountEmails") ImmutableSet serviceAccountEmails, @IapOidc TokenVerifier tokenVerifier, @IapOidc TokenExtractor tokenExtractor) { - super(serviceAccountEmails, tokenVerifier, tokenExtractor); + super(serviceAccountEmails, tokenVerifier, null, tokenExtractor); } } @@ -161,8 +183,9 @@ static class RegularOidcAuthenticationMechanism extends OidcTokenAuthenticationM protected RegularOidcAuthenticationMechanism( @Config("allowedServiceAccountEmails") ImmutableSet serviceAccountEmails, @RegularOidc TokenVerifier tokenVerifier, + @RegularOidcFallback TokenVerifier fallbackTokenVerifier, @RegularOidc TokenExtractor tokenExtractor) { - super(serviceAccountEmails, tokenVerifier, tokenExtractor); + super(serviceAccountEmails, tokenVerifier, fallbackTokenVerifier, tokenExtractor); } } } 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 3a53cf59505..ad423204f6a 100644 --- a/core/src/test/java/google/registry/request/auth/OidcTokenAuthenticationMechanismTest.java +++ b/core/src/test/java/google/registry/request/auth/OidcTokenAuthenticationMechanismTest.java @@ -19,7 +19,6 @@ 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.insertInDb; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -70,7 +69,7 @@ public class OidcTokenAuthenticationMechanismTest { private AuthResult authResult; private OidcTokenAuthenticationMechanism authenticationMechanism = - new OidcTokenAuthenticationMechanism(serviceAccounts, tokenVerifier, e -> rawToken) {}; + new OidcTokenAuthenticationMechanism(serviceAccounts, tokenVerifier, null, e -> rawToken) {}; @RegisterExtension public final JpaTestExtensions.JpaUnitTestExtension jpaExtension = @@ -78,7 +77,7 @@ public class OidcTokenAuthenticationMechanismTest { @BeforeEach void beforeEach() throws Exception { - when(tokenVerifier.verify(eq(rawToken))).thenReturn(jwt); + when(tokenVerifier.verify(rawToken)).thenReturn(jwt); payload.setEmail(email); payload.setSubject(gaiaId); insertInDb(user); @@ -98,18 +97,30 @@ void testAuthResultBypass() { @Test void testAuthenticate_noTokenFromRequest() { authenticationMechanism = - new OidcTokenAuthenticationMechanism(serviceAccounts, tokenVerifier, e -> null) {}; + new OidcTokenAuthenticationMechanism(serviceAccounts, tokenVerifier, null, e -> null) {}; authResult = authenticationMechanism.authenticate(request); assertThat(authResult).isEqualTo(AuthResult.NOT_AUTHENTICATED); } @Test void testAuthenticate_invalidToken() throws Exception { - when(tokenVerifier.verify(eq(rawToken))).thenThrow(new VerificationException("Bad token")); + when(tokenVerifier.verify(rawToken)).thenThrow(new VerificationException("Bad token")); authResult = authenticationMechanism.authenticate(request); assertThat(authResult).isEqualTo(AuthResult.NOT_AUTHENTICATED); } + @Test + void testAuthenticate_fallbackVerifier() throws Exception { + TokenVerifier fallbackVerifier = mock(TokenVerifier.class); + when(tokenVerifier.verify(rawToken)).thenThrow(new VerificationException("Bad token")); + when(fallbackVerifier.verify(rawToken)).thenReturn(jwt); + authenticationMechanism = + new OidcTokenAuthenticationMechanism( + serviceAccounts, tokenVerifier, fallbackVerifier, e -> rawToken) {}; + authResult = authenticationMechanism.authenticate(request); + assertThat(authResult.isAuthenticated()).isEqualTo(true); + } + @Test void testAuthenticate_noEmailAddress() throws Exception { payload.setEmail(null); @@ -223,5 +234,12 @@ ImmutableSet provideAllowedServiceAccountEmails() { String provideOauthClientId() { return "client-id"; } + + @Provides + @Singleton + @Config("fallbackOauthClientId") + String provideFallbackOauthClientId() { + return "fallback-client-id"; + } } }