From 2d8dd7700eff92d4b95027be55e26e1e7aa79181 Mon Sep 17 00:00:00 2001
From: Cody Oss <6331106+codyoss@users.noreply.github.com>
Date: Tue, 3 Sep 2024 12:44:03 -0500
Subject: [PATCH] fix(auth): choose quota project envvar over file when both
 present (#10807)

Because we were explicitly setting the value of quota project in
the credentials package the logic that should have checked the
environment variable was never hit if the value was set in the creds
file. So for now we will not set it in this package. The getter on
the creds will then default to the correct logic in internal.go.

Also mark an integration test as such so it does not run when the
short flag is passed.

Fixes: #10804
---
 auth/credentials/detect_test.go              | 44 ++++++++++++++++++++
 auth/credentials/filetypes.go                | 11 ++---
 auth/credentials/idtoken/integration_test.go |  1 +
 3 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/auth/credentials/detect_test.go b/auth/credentials/detect_test.go
index 4940608069be..4e8672159b66 100644
--- a/auth/credentials/detect_test.go
+++ b/auth/credentials/detect_test.go
@@ -252,6 +252,50 @@ func TestDefaultCredentials_UserCredentialsKey(t *testing.T) {
 	}
 }
 
+func TestDefaultCredentials_QuotaProjectPrecedence(t *testing.T) {
+	want := "take-this-value"
+	t.Setenv("GOOGLE_CLOUD_QUOTA_PROJECT", want)
+	ctx := context.Background()
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		resp := &tokResp{
+			AccessToken: "a_fake_token",
+			TokenType:   internal.TokenTypeBearer,
+			ExpiresIn:   60,
+		}
+		if err := json.NewEncoder(w).Encode(&resp); err != nil {
+			t.Fatal(err)
+		}
+	}))
+
+	creds, err := DetectDefault(&DetectOptions{
+		CredentialsFile: "../internal/testdata/user.json",
+		Scopes:          []string{"https://www.googleapis.com/auth/cloud-platform"},
+		TokenURL:        ts.URL,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	got, err := creds.QuotaProjectID(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got != want {
+		t.Fatalf("got %q, want %q", got, want)
+	}
+
+	// unset the env to fallback to the value in creds
+	t.Setenv("GOOGLE_CLOUD_QUOTA_PROJECT", "")
+	want = "fake_project2"
+	got, err = creds.QuotaProjectID(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got != want {
+		t.Fatalf("got %q, want %q", got, want)
+	}
+}
+
 func TestDefaultCredentials_UserCredentialsKey_UniverseDomain(t *testing.T) {
 	ctx := context.Background()
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
diff --git a/auth/credentials/filetypes.go b/auth/credentials/filetypes.go
index b426e16d2975..cf56b025a237 100644
--- a/auth/credentials/filetypes.go
+++ b/auth/credentials/filetypes.go
@@ -33,7 +33,7 @@ func fileCredentials(b []byte, opts *DetectOptions) (*auth.Credentials, error) {
 		return nil, err
 	}
 
-	var projectID, quotaProjectID, universeDomain string
+	var projectID, universeDomain string
 	var tp auth.TokenProvider
 	switch fileType {
 	case credsfile.ServiceAccountKey:
@@ -56,7 +56,6 @@ func fileCredentials(b []byte, opts *DetectOptions) (*auth.Credentials, error) {
 		if err != nil {
 			return nil, err
 		}
-		quotaProjectID = f.QuotaProjectID
 		universeDomain = f.UniverseDomain
 	case credsfile.ExternalAccountKey:
 		f, err := credsfile.ParseExternalAccount(b)
@@ -67,7 +66,6 @@ func fileCredentials(b []byte, opts *DetectOptions) (*auth.Credentials, error) {
 		if err != nil {
 			return nil, err
 		}
-		quotaProjectID = f.QuotaProjectID
 		universeDomain = resolveUniverseDomain(opts.UniverseDomain, f.UniverseDomain)
 	case credsfile.ExternalAccountAuthorizedUserKey:
 		f, err := credsfile.ParseExternalAccountAuthorizedUser(b)
@@ -78,7 +76,6 @@ func fileCredentials(b []byte, opts *DetectOptions) (*auth.Credentials, error) {
 		if err != nil {
 			return nil, err
 		}
-		quotaProjectID = f.QuotaProjectID
 		universeDomain = f.UniverseDomain
 	case credsfile.ImpersonatedServiceAccountKey:
 		f, err := credsfile.ParseImpersonatedServiceAccount(b)
@@ -108,9 +105,9 @@ func fileCredentials(b []byte, opts *DetectOptions) (*auth.Credentials, error) {
 		TokenProvider: auth.NewCachedTokenProvider(tp, &auth.CachedTokenProviderOptions{
 			ExpireEarly: opts.EarlyTokenRefresh,
 		}),
-		JSON:                   b,
-		ProjectIDProvider:      internalauth.StaticCredentialsProperty(projectID),
-		QuotaProjectIDProvider: internalauth.StaticCredentialsProperty(quotaProjectID),
+		JSON:              b,
+		ProjectIDProvider: internalauth.StaticCredentialsProperty(projectID),
+		// TODO(codyoss): only set quota project here if there was a user override
 		UniverseDomainProvider: internalauth.StaticCredentialsProperty(universeDomain),
 	}), nil
 }
diff --git a/auth/credentials/idtoken/integration_test.go b/auth/credentials/idtoken/integration_test.go
index 4e55d8cd4917..467eb342b7dd 100644
--- a/auth/credentials/idtoken/integration_test.go
+++ b/auth/credentials/idtoken/integration_test.go
@@ -66,6 +66,7 @@ func TestNewCredentials_CredentialsFile(t *testing.T) {
 }
 
 func TestNewCredentials_CredentialsJSON(t *testing.T) {
+	testutil.IntegrationTestCheck(t)
 	ctx := context.Background()
 	f := os.Getenv(credsfile.GoogleAppCredsEnvVar)
 	if f == "" {