Issue #1099: Fixing the usage of ExternalAccountCredentials (#1102)

CHANGES.md

@@ -5,6 +5,7 @@
 * PR #1057: Enable async writes for greater throughput
 * PR #1094: CVE-2023-5072: Upgrading the org.json:json dependency
 * PR #1095: CVE-2023-4586: Upgrading the netty dependencies
+* Issue #1099: Fixing the usage of ExternalAccountCredentials
 * BigQuery API has been upgraded to version 2.33.2
 * BigQuery Storage API has been upgraded to version 2.44.0
 * GAX has been upgraded to version 2.35.0

bigquery-connector-common/src/main/java/com/google/cloud/bigquery/connector/common/BigQueryClientFactory.java

@@ -35,6 +35,7 @@
 import java.io.IOException;
 import java.io.Serializable;
 import java.io.UncheckedIOException;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
@@ -60,6 +61,8 @@ public class BigQueryClientFactory implements Serializable {
   private final HeaderProvider headerProvider;
   private final BigQueryConfig bqConfig;
 
+  private int cachedHashCode = 0;
+
   @Inject
   public BigQueryClientFactory(
       BigQueryCredentialsSupplier bigQueryCredentialsSupplier,
@@ -104,20 +107,26 @@ public BigQueryWriteClient getBigQueryWriteClient() {
 
   @Override
   public int hashCode() {
-    // Here, credentials is an instance of GoogleCredentials which can be one out of
-    // GoogleCredentials, UserCredentials, ServiceAccountCredentials, ExternalAccountCredentials or
-    // ImpersonatedCredentials (See the class BigQueryCredentialsSupplier which supplies these
-    // Credentials). Subclasses of the abstract class ExternalAccountCredentials do not have the
-    // hashCode method defined on them and hence we get the byte array of the
-    // ExternalAccountCredentials first and then compare their hashCodes.
-    if (credentials instanceof ExternalAccountCredentials) {
-      return Objects.hashCode(
-          BigQueryUtil.getCredentialsByteArray(credentials),
-          headerProvider,
-          bqConfig.getClientCreationHashCode());
+    // caching the hash code, as we use it 3 times (potentially) get*Client() method
+    if (cachedHashCode == 0) {
+      // Here, credentials is an instance of GoogleCredentials which can be one out of
+      // GoogleCredentials, UserCredentials, ServiceAccountCredentials, ExternalAccountCredentials
+      // or ImpersonatedCredentials (See the class BigQueryCredentialsSupplier which supplies these
+      // Credentials). Subclasses of the abstract class ExternalAccountCredentials do not have the
+      // hashCode method defined on them and hence we get the byte array of the
+      // ExternalAccountCredentials first and then compare their hashCodes.
+      if (credentials instanceof ExternalAccountCredentials) {
+        cachedHashCode =
+            Objects.hashCode(
+                Arrays.hashCode(BigQueryUtil.getCredentialsByteArray(credentials)),
+                headerProvider,
+                bqConfig.getClientCreationHashCode());
+      } else {
+        cachedHashCode =
+            Objects.hashCode(credentials, headerProvider, bqConfig.getClientCreationHashCode());
+      }
     }
-
-    return Objects.hashCode(credentials, headerProvider, bqConfig.getClientCreationHashCode());
+    return cachedHashCode;
   }
 
   @Override

bigquery-connector-common/src/test/java/com/google/cloud/bigquery/connector/common/BigQueryClientFactoryTest.java

@@ -15,14 +15,18 @@
  */
 package com.google.cloud.bigquery.connector.common;
 
+import static com.google.common.truth.Truth.*;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertSame;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import com.google.api.gax.retrying.RetrySettings;
+import com.google.api.gax.rpc.FixedHeaderProvider;
 import com.google.api.gax.rpc.HeaderProvider;
+import com.google.auth.Credentials;
+import com.google.auth.oauth2.GoogleCredentials;
 import com.google.auth.oauth2.ServiceAccountCredentials;
 import com.google.cloud.bigquery.QueryJobConfiguration.Priority;
 import com.google.cloud.bigquery.storage.v1.BigQueryReadClient;
@@ -421,6 +425,26 @@ public void testGetWriteClientWithSameAndDifferentBQConfig() {
     assertSame(writeClient2, writeClient3);
   }
 
+  @Test
+  public void testHashCodeWithExternalAccountCredentials() throws Exception {
+    // Credentials taken from https://google.aip.dev/auth/4117:
+    Credentials credentials =
+        GoogleCredentials.fromStream(
+            getClass().getResourceAsStream("/external-account-credentials.json"));
+
+    when(bigQueryCredentialsSupplier.getCredentials()).thenReturn(credentials);
+
+    BigQueryClientFactory factory =
+        new BigQueryClientFactory(
+            bigQueryCredentialsSupplier,
+            FixedHeaderProvider.create("foo", "bar"),
+            new TestBigQueryConfig(Optional.empty()));
+
+    int hashCode1 = factory.hashCode();
+    int hashCode2 = factory.hashCode();
+    assertThat(hashCode2).isEqualTo(hashCode1);
+  }
+
   private ServiceAccountCredentials createServiceAccountCredentials(String clientId) {
     return ServiceAccountCredentials.newBuilder()
         .setClientId(clientId)

bigquery-connector-common/src/test/resources/external-account-credentials.json

@@ -0,0 +1,18 @@
+{
+  "type": "external_account",
+  "audience": "//iam.googleapis.com/projects/1234/locations/global/workloadIdentityPools/some-pool/providers/test",
+  "subject_token_type": "urn:ietf:params:oauth:token-type:jwt",
+  "service_account_impersonation_url": "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/test:generateAccessToken",
+  "token_url": "https://sts.googleapis.com/v1/token",
+  "credential_source": {
+    "headers": {
+      "Metadata": "True"
+    },
+    "url": 
+"http://169.254.169.254/metadata/identity/oauth2/token?api-version=2018-02-01&resource=https://iam.googleapis.com/projects/$PROJECT_NUMBER/locations/global/workloadIdentityPools/$POOL_ID/providers/$PROVIDER_ID",
+    "format": {
+      "type": "json",
+      "subject_token_field_name": "access_token"
+    }
+  }
+}