From 53817658d82f07bbd745a58fc1aa74c67a08e25e Mon Sep 17 00:00:00 2001 From: Radhika Agrawal Date: Wed, 28 Oct 2026 22:04:17 +0000 Subject: [PATCH 1/4] feat(util): migrate fetch.go to cloud.google.com/go/auth and bridge legacy functions via oauth2adapt --- util/fetch.go | 295 +++++++++++++++++++++++----- util/fetch_test.go | 475 +++++++++++++++++++++++++++++++++++++++++++++ util/settings.go | 2 - 3 files changed, 719 insertions(+), 53 deletions(-) create mode 100644 util/fetch_test.go diff --git a/util/fetch.go b/util/fetch.go index 4b13910..baff7ee 100644 --- a/util/fetch.go +++ b/util/fetch.go @@ -16,102 +16,295 @@ package util import ( "context" + "encoding/json" "errors" "fmt" "strings" - // Blank import for go mod tidy and vendor; will be imported directly in Phase 4. - _ "cloud.google.com/go/auth/oauth2adapt" + "cloud.google.com/go/auth" + "cloud.google.com/go/auth/credentials" + "cloud.google.com/go/auth/oauth2adapt" "golang.org/x/oauth2" - // Temporary import to convert settings.resolveAuthHandler() for google.CredentialsParams; - // will be removed in the next phase (Phase 4) when google.CredentialsParams is replaced - // with credentials.DetectOptions. - "golang.org/x/oauth2/authhandler" "golang.org/x/oauth2/google" ) var DefaultScope = "https://www.googleapis.com/auth/cloud-platform" -// Default method to return a token source from a given settings. +// defaultAuthState is the 3LO "state" value used when Settings.State is empty. +const defaultAuthState = "state" + +const ( + authorizedUserCredentialsType = "authorized_user" + serviceAccountCredentialsType = "service_account" +) + +// credentialsFileHeader holds the fields of a credentials JSON file that are +// needed to configure credentials.DetectOptions. +type credentialsFileHeader struct { + Type string `json:"type"` + TokenURI string `json:"token_uri"` + RefreshToken string `json:"refresh_token"` +} + +func parseCredentialsFileHeader(b []byte) credentialsFileHeader { + var h credentialsFileHeader + if len(b) > 0 { + // Errors are ignored here; credentials.DetectDefault reports + // malformed JSON with a descriptive error. + _ = json.Unmarshal(b, &h) + } + return h +} + +// newDetectOptions builds credentials.DetectOptions from settings. +// +// cloud.google.com/go/auth rejects options that set both Scopes and +// Audience, so Audience is only used for self-signed JWTs (where it takes +// precedence over Scope); OAuth flows only use Scope. This lets callers keep +// setting both fields (e.g. for STS token exchange) as before. +func newDetectOptions(settings *Settings, useSelfSignedJWT bool) *credentials.DetectOptions { + if settings == nil { + return &credentials.DetectOptions{Scopes: []string{DefaultScope}} + } + opts := &credentials.DetectOptions{ + Subject: settings.Email, + UseSelfSignedJWT: useSelfSignedJWT, + } + if useSelfSignedJWT && settings.Audience != "" { + opts.Audience = settings.Audience + } else if settings.Scope != "" { + opts.Scopes = strings.Split(settings.Scope, " ") + } + if settings.CredentialsJSON != "" { + opts.CredentialsJSON = []byte(settings.CredentialsJSON) + // cloud.google.com/go/auth ignores "token_uri" in "authorized_user" + // files and uses DetectOptions.TokenURL instead. Forward it so that + // custom token endpoints (e.g. the refresh token JSON built by + // BuildAuthRefreshTokenJSON) keep working. + if h := parseCredentialsFileHeader(opts.CredentialsJSON); h.Type == authorizedUserCredentialsType { + opts.TokenURL = h.TokenURI + } + } + if handler := settings.resolveAuthHandler(); handler != nil { + state := settings.State + if state == "" { + state = defaultAuthState + } + opts.AuthHandlerOptions = &auth.AuthorizationHandlerOptions{ + Handler: handler, + State: state, + PKCEOpts: GeneratePKCEOptions(), + } + } + return opts +} + +// newCredentials returns credentials for the given settings. // Returns nil for API keys. -func newTokenSource(ctx context.Context, settings *Settings) (*oauth2.TokenSource, error) { - var ts oauth2.TokenSource - var err error +func newCredentials(ctx context.Context, settings *Settings) (*auth.Credentials, error) { if settings == nil { - ts, err = google.DefaultTokenSource(ctx, DefaultScope) - } else if settings.GetAuthType() == AuthTypeAPIKey { + return FindAuthCredentials(ctx, nil) + } + switch settings.GetAuthType() { + case AuthTypeAPIKey: return nil, nil - } else if settings.GetAuthType() == AuthTypeOAuth { - ts, err = OAuthJSONTokenSource(ctx, settings) - } else if settings.GetAuthType() == AuthTypeJWT { - ts, err = JWTTokenSource(ctx, settings) - } else { + case AuthTypeOAuth: + return FindAuthCredentials(ctx, settings) + case AuthTypeJWT: + return findJWTCredentials(settings) + default: return nil, fmt.Errorf("Unsupported authentcation method: %s", settings.GetAuthType()) } +} + +// FetchAuthToken returns a cloud.google.com/go/auth token from the given +// settings. If settings is nil, Application Default Credentials with +// DefaultScope are used. Returns nil for API keys. +// +// For 3-legged OAuth flows, the refresh token (if any) is available via +// token.MetadataString("refresh_token"). +func FetchAuthToken(ctx context.Context, settings *Settings) (*auth.Token, error) { + if settings != nil && settings.APIKey != "" { + return nil, nil + } + creds, err := newCredentials(ctx, settings) + if err != nil { + return nil, err + } + if creds == nil { + return nil, nil + } + tok, err := creds.Token(ctx) if err != nil { return nil, err } - return &ts, err + return withRefreshToken(tok, creds.JSON()), nil } -// Returns a token from the given settings. -// Returns nil for API keys. -func FetchToken(ctx context.Context, settings *Settings) (*oauth2.Token, error) { - if settings.APIKey != "" { - return nil, nil +// withRefreshToken returns tok with its "refresh_token" metadata set from +// "authorized_user" credentials JSON when the token response omitted it, +// matching golang.org/x/oauth2, which keeps the refresh token on refreshed +// tokens. Other tokens are returned unchanged. +func withRefreshToken(tok *auth.Token, credsJSON []byte) *auth.Token { + if tok == nil || tok.MetadataString("refresh_token") != "" { + return tok + } + h := parseCredentialsFileHeader(credsJSON) + if h.Type != authorizedUserCredentialsType || h.RefreshToken == "" { + return tok } - src, err := newTokenSource(ctx, settings) + // Copy the token and metadata so the cached token is not mutated. + metadata := make(map[string]interface{}, len(tok.Metadata)+1) + for k, v := range tok.Metadata { + metadata[k] = v + } + metadata["refresh_token"] = h.RefreshToken + tok2 := *tok + tok2.Metadata = metadata + return &tok2 +} + +// OAuthJSONTokenProvider returns a cloud.google.com/go/auth token provider +// for OAuth flows (2LO, 3LO, user and external account credentials). +func OAuthJSONTokenProvider(ctx context.Context, settings *Settings) (auth.TokenProvider, error) { + creds, err := FindAuthCredentials(ctx, settings) if err != nil { return nil, err } - ts := oauth2.ReuseTokenSource(nil, *src) - t, err := ts.Token() + return creds, nil +} + +// JWTTokenProvider returns a cloud.google.com/go/auth token provider that +// mints self-signed JWT access tokens from service account credentials. +// settings.Audience takes precedence over settings.Scope. +func JWTTokenProvider(ctx context.Context, settings *Settings) (auth.TokenProvider, error) { + creds, err := findJWTCredentials(settings) if err != nil { return nil, err } - return t, nil + return creds, nil } -func OAuthJSONTokenSource(ctx context.Context, settings *Settings) (oauth2.TokenSource, error) { - creds, err := FindJSONCredentials(ctx, settings) +func findJWTCredentials(settings *Settings) (*auth.Credentials, error) { + if settings == nil || (settings.Audience == "" && settings.Scope == "") { + return nil, errors.New("Neither audience nor scope is provided for JWTTokenProvider") + } + creds, err := credentials.DetectDefault(newDetectOptions(settings, true)) if err != nil { return nil, err } - return creds.TokenSource, nil + // cloud.google.com/go/auth silently ignores UseSelfSignedJWT for + // non-service-account credentials; keep rejecting them as before. + if t := parseCredentialsFileHeader(creds.JSON()).Type; t != serviceAccountCredentialsType { + return nil, fmt.Errorf("self-signed JWT requires %q credentials, got %q", serviceAccountCredentialsType, t) + } + return creds, nil +} +// FindAuthCredentials obtains cloud.google.com/go/auth credentials from +// settings or Application Default Credentials. If settings is nil, +// Application Default Credentials with DefaultScope are used. +func FindAuthCredentials(ctx context.Context, settings *Settings) (*auth.Credentials, error) { + return credentials.DetectDefault(newDetectOptions(settings, false)) } -func JWTTokenSource(ctx context.Context, settings *Settings) (oauth2.TokenSource, error) { - creds, err := FindJSONCredentials(ctx, settings) +// Returns a token from the given settings. +// Returns nil for API keys. +// +// Deprecated: Use FetchAuthToken instead. +func FetchToken(ctx context.Context, settings *Settings) (*oauth2.Token, error) { + tok, err := FetchAuthToken(ctx, settings) + if err != nil { + return nil, oauth2Error(err) + } + if tok == nil { + return nil, nil + } + return oauth2TokenFromAuthToken(tok), nil +} + +// Deprecated: Use OAuthJSONTokenProvider instead. +func OAuthJSONTokenSource(ctx context.Context, settings *Settings) (oauth2.TokenSource, error) { + creds, err := FindAuthCredentials(ctx, settings) if err != nil { return nil, err } - if settings.Audience != "" { - return google.JWTAccessTokenSourceFromJSON(creds.JSON, settings.Audience) - } else if settings.Scope != "" { - return google.JWTAccessTokenSourceWithScope(creds.JSON, settings.Scope) - } else { + return tokenSourceFromCredentials(creds), nil +} + +// Deprecated: Use JWTTokenProvider instead. +func JWTTokenSource(ctx context.Context, settings *Settings) (oauth2.TokenSource, error) { + if settings == nil || (settings.Audience == "" && settings.Scope == "") { return nil, errors.New("Neither audience nor scope is provided for JWTTokenSource") } + creds, err := findJWTCredentials(settings) + if err != nil { + return nil, err + } + return tokenSourceFromCredentials(creds), nil } // FindJSONCredentials obtains credentials from settings or Application Default Credentials +// +// Deprecated: Use FindAuthCredentials instead. func FindJSONCredentials(ctx context.Context, settings *Settings) (*google.Credentials, error) { - var params google.CredentialsParams - params.Scopes = strings.Split(settings.Scope, " ") - params.State = "state" - // Convert the resolved handler back to authhandler.AuthorizationHandler and - // use GeneratePKCEParams() while FindJSONCredentials still constructs - // google.CredentialsParams; Phase 4 replaces this with FindAuthCredentials. - params.AuthHandler = authhandler.AuthorizationHandler(settings.resolveAuthHandler()) - params.Subject = settings.Email - params.PKCE = GeneratePKCEParams() - if settings.CredentialsJSON != "" { - return google.CredentialsFromJSONWithParams(ctx, []byte(settings.CredentialsJSON), - params) + creds, err := FindAuthCredentials(ctx, settings) + if err != nil { + return nil, err + } + c := oauth2adapt.Oauth2CredentialsFromAuthCredentials(creds) + c.TokenSource = tokenSourceFromCredentials(creds) + return c, nil +} + +// tokenSourceFromCredentials adapts auth.Credentials to an +// oauth2.TokenSource. Unlike oauth2adapt.TokenSourceFromTokenProvider, it +// also populates oauth2.Token.RefreshToken so that 3LO refresh tokens are +// preserved for legacy callers. +func tokenSourceFromCredentials(creds *auth.Credentials) oauth2.TokenSource { + return &oauth2TokenSource{creds: creds} +} + +type oauth2TokenSource struct { + creds *auth.Credentials +} + +func (ts *oauth2TokenSource) Token() (*oauth2.Token, error) { + tok, err := ts.creds.Token(context.Background()) + if err != nil { + return nil, oauth2Error(err) + } + return oauth2TokenFromAuthToken(withRefreshToken(tok, ts.creds.JSON())), nil +} - } else { - return google.FindDefaultCredentialsWithParams(ctx, params) +// oauth2TokenFromAuthToken converts an auth.Token to an oauth2.Token, +// preserving the refresh token and the raw token response metadata (exposed +// via oauth2.Token.Extra, e.g. "id_token" and "scope"). +func oauth2TokenFromAuthToken(tok *auth.Token) *oauth2.Token { + t := &oauth2.Token{ + AccessToken: tok.Value, + TokenType: tok.Type, + RefreshToken: tok.MetadataString("refresh_token"), + Expiry: tok.Expiry, + } + if len(tok.Metadata) > 0 { + // Copy the map to avoid sharing it with the cached auth.Token. + extra := make(map[string]interface{}, len(tok.Metadata)) + for k, v := range tok.Metadata { + extra[k] = v + } + t = t.WithExtra(extra) + } + return t +} +// oauth2Error adds an *oauth2.RetrieveError to the chain of *auth.Error +// values so legacy callers can keep using errors.As with +// *oauth2.RetrieveError. +func oauth2Error(err error) error { + var authErr *auth.Error + if errors.As(err, &authErr) { + oauth2adapt.AddRetrieveErrorToAuthError(authErr) } + return err } diff --git a/util/fetch_test.go b/util/fetch_test.go new file mode 100644 index 0000000..bb10e35 --- /dev/null +++ b/util/fetch_test.go @@ -0,0 +1,475 @@ +// +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package util + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "encoding/base64" + "encoding/json" + "encoding/pem" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "reflect" + "strings" + "testing" + + "cloud.google.com/go/auth" + "golang.org/x/oauth2" + "golang.org/x/oauth2/authhandler" +) + +const testRefreshToken = "test-refresh-token" + +// newTokenServer returns a fake OAuth 2.0 token endpoint that responds with +// the given status code and JSON body. +func newTokenServer(t *testing.T, status int, body string) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + fmt.Fprint(w, body) + })) + t.Cleanup(srv.Close) + return srv +} + +func authorizedUserJSON(tokenURI string) string { + return fmt.Sprintf(`{ + "type": "authorized_user", + "client_id": "test-client.apps.googleusercontent.com", + "client_secret": "test-secret", + "refresh_token": %q, + "token_uri": %q +}`, testRefreshToken, tokenURI) +} + +func serviceAccountJSON(t *testing.T) string { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("rsa.GenerateKey() unexpected error: %v", err) + } + der, err := x509.MarshalPKCS8PrivateKey(key) + if err != nil { + t.Fatalf("x509.MarshalPKCS8PrivateKey() unexpected error: %v", err) + } + pemKey := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: der}) + b, err := json.Marshal(map[string]string{ + "type": "service_account", + "project_id": "test-project", + "private_key_id": "test-key-id", + "private_key": string(pemKey), + "client_email": "test-sa@test-project.iam.gserviceaccount.com", + "client_id": "123456789", + "token_uri": "https://oauth2.googleapis.com/token", + }) + if err != nil { + t.Fatalf("json.Marshal() unexpected error: %v", err) + } + return string(b) +} + +// jwtClaims decodes the (unverified) claims of a JWT. +func jwtClaims(t *testing.T, token string) map[string]interface{} { + t.Helper() + parts := strings.Split(token, ".") + if len(parts) != 3 { + t.Fatalf("token %q is not a JWT", token) + } + b, err := base64.RawURLEncoding.DecodeString(parts[1]) + if err != nil { + t.Fatalf("decoding JWT claims: %v", err) + } + var claims map[string]interface{} + if err := json.Unmarshal(b, &claims); err != nil { + t.Fatalf("unmarshaling JWT claims: %v", err) + } + return claims +} + +func TestNewDetectOptions(t *testing.T) { + tests := []struct { + name string + settings *Settings + useSelfSignedJWT bool + wantScopes []string + wantAudience string + wantTokenURL string + wantSubject string + }{ + { + name: "nil settings use DefaultScope", + settings: nil, + wantScopes: []string{DefaultScope}, + }, + { + name: "OAuth splits space-delimited scopes", + settings: &Settings{Scope: "scope1 scope2"}, + wantScopes: []string{"scope1", "scope2"}, + }, + { + name: "OAuth ignores audience when both scope and audience set", + settings: &Settings{Scope: "scope1", Audience: "https://sts.googleapis.com"}, + wantScopes: []string{"scope1"}, + }, + { + name: "JWT prefers audience over scope", + settings: &Settings{Scope: "scope1", Audience: "https://example.googleapis.com/"}, + useSelfSignedJWT: true, + wantAudience: "https://example.googleapis.com/", + }, + { + name: "JWT falls back to scope", + settings: &Settings{Scope: "scope1"}, + useSelfSignedJWT: true, + wantScopes: []string{"scope1"}, + }, + { + name: "authorized_user token_uri forwarded to TokenURL", + settings: &Settings{Scope: "scope1", CredentialsJSON: authorizedUserJSON("http://localhost:8080/token")}, + wantScopes: []string{"scope1"}, + wantTokenURL: "http://localhost:8080/token", + }, + { + name: "token_uri of other credential types is not forwarded", + settings: &Settings{Scope: "scope1", CredentialsJSON: `{"type": "service_account", "token_uri": "http://localhost:8080/token"}`}, + wantScopes: []string{"scope1"}, + }, + { + name: "email used as subject", + settings: &Settings{Scope: "scope1", Email: "user@example.com"}, + wantScopes: []string{"scope1"}, + wantSubject: "user@example.com", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + opts := newDetectOptions(tc.settings, tc.useSelfSignedJWT) + if !reflect.DeepEqual(opts.Scopes, tc.wantScopes) { + t.Errorf("Scopes = %q, want %q", opts.Scopes, tc.wantScopes) + } + if opts.Audience != tc.wantAudience { + t.Errorf("Audience = %q, want %q", opts.Audience, tc.wantAudience) + } + if opts.TokenURL != tc.wantTokenURL { + t.Errorf("TokenURL = %q, want %q", opts.TokenURL, tc.wantTokenURL) + } + if opts.Subject != tc.wantSubject { + t.Errorf("Subject = %q, want %q", opts.Subject, tc.wantSubject) + } + if opts.UseSelfSignedJWT != (tc.useSelfSignedJWT && tc.settings != nil) { + t.Errorf("UseSelfSignedJWT = %v, want %v", opts.UseSelfSignedJWT, tc.useSelfSignedJWT) + } + }) + } +} + +func TestNewDetectOptionsAuthHandlerOptions(t *testing.T) { + handler := auth.AuthorizationHandler(func(string) (string, string, error) { + return "code", "state", nil + }) + legacyHandler := authhandler.AuthorizationHandler(func(string) (string, string, error) { + return "legacy-code", "state", nil + }) + + tests := []struct { + name string + settings *Settings + wantNil bool + wantCode string + wantState string + }{ + { + name: "no handler leaves AuthHandlerOptions unset", + settings: &Settings{Scope: "scope1"}, + wantNil: true, + }, + { + name: "GoogleAuthHandler with default state", + settings: &Settings{Scope: "scope1", GoogleAuthHandler: handler}, + wantCode: "code", + wantState: defaultAuthState, + }, + { + name: "deprecated AuthHandler with custom state", + settings: &Settings{Scope: "scope1", AuthHandler: legacyHandler, State: "custom-state"}, + wantCode: "legacy-code", + wantState: "custom-state", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := newDetectOptions(tc.settings, false).AuthHandlerOptions + if tc.wantNil { + if got != nil { + t.Errorf("AuthHandlerOptions = %+v, want nil", got) + } + return + } + if got == nil || got.Handler == nil { + t.Fatal("AuthHandlerOptions.Handler = nil, want non-nil") + } + if code, _, _ := got.Handler(""); code != tc.wantCode { + t.Errorf("Handler() code = %q, want %q", code, tc.wantCode) + } + if got.State != tc.wantState { + t.Errorf("State = %q, want %q", got.State, tc.wantState) + } + if got.PKCEOpts == nil || got.PKCEOpts.ChallengeMethod != "S256" || got.PKCEOpts.Verifier == "" { + t.Errorf("PKCEOpts = %+v, want S256 challenge with verifier", got.PKCEOpts) + } + }) + } +} + +func TestFetchAuthTokenAPIKey(t *testing.T) { + for _, s := range []*Settings{ + {APIKey: "my-api-key"}, + {AuthType: AuthTypeAPIKey}, + } { + tok, err := FetchAuthToken(context.Background(), s) + if err != nil || tok != nil { + t.Errorf("FetchAuthToken(%+v) = (%v, %v), want (nil, nil)", s, tok, err) + } + oauth2Tok, err := FetchToken(context.Background(), s) + if err != nil || oauth2Tok != nil { + t.Errorf("FetchToken(%+v) = (%v, %v), want (nil, nil)", s, oauth2Tok, err) + } + } +} + +func TestFetchAuthTokenUnsupportedAuthType(t *testing.T) { + _, err := FetchAuthToken(context.Background(), &Settings{AuthType: AuthTypeSSO}) + if err == nil || !strings.Contains(err.Error(), "Unsupported authentcation method: sso") { + t.Errorf("FetchAuthToken() error = %v, want unsupported authentication method", err) + } +} + +func TestFetchAuthTokenAuthorizedUser(t *testing.T) { + tests := []struct { + name string + response string + wantRefreshToken string + }{ + { + name: "refresh token kept from credentials when omitted in response", + response: `{"access_token": "test-access-token", "token_type": "Bearer", "expires_in": 3600, "scope": "scope1", "id_token": "test-id-token"}`, + wantRefreshToken: testRefreshToken, + }, + { + name: "rotated refresh token from response is used", + response: `{"access_token": "test-access-token", "token_type": "Bearer", "expires_in": 3600, "scope": "scope1", "id_token": "test-id-token", "refresh_token": "rotated-refresh-token"}`, + wantRefreshToken: "rotated-refresh-token", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + srv := newTokenServer(t, http.StatusOK, tc.response) + settings := &Settings{ + AuthType: AuthTypeOAuth, + CredentialsJSON: authorizedUserJSON(srv.URL), + Scope: "scope1", + } + + tok, err := FetchAuthToken(context.Background(), settings) + if err != nil { + t.Fatalf("FetchAuthToken() unexpected error: %v", err) + } + if tok.Value != "test-access-token" || tok.Type != "Bearer" || tok.Expiry.IsZero() { + t.Errorf("FetchAuthToken() = %+v, want access token with type and expiry", tok) + } + if got := tok.MetadataString("refresh_token"); got != tc.wantRefreshToken { + t.Errorf("FetchAuthToken() refresh_token = %q, want %q", got, tc.wantRefreshToken) + } + + // The deprecated wrappers expose the same token via golang.org/x/oauth2. + oauth2Tok, err := FetchToken(context.Background(), settings) + if err != nil { + t.Fatalf("FetchToken() unexpected error: %v", err) + } + ts, err := OAuthJSONTokenSource(context.Background(), settings) + if err != nil { + t.Fatalf("OAuthJSONTokenSource() unexpected error: %v", err) + } + tsTok, err := ts.Token() + if err != nil { + t.Fatalf("OAuthJSONTokenSource().Token() unexpected error: %v", err) + } + for name, got := range map[string]*oauth2.Token{"FetchToken": oauth2Tok, "OAuthJSONTokenSource": tsTok} { + if got.AccessToken != "test-access-token" || got.TokenType != "Bearer" || got.Expiry.IsZero() { + t.Errorf("%s() = %+v, want access token with type and expiry", name, got) + } + if got.RefreshToken != tc.wantRefreshToken { + t.Errorf("%s() RefreshToken = %q, want %q", name, got.RefreshToken, tc.wantRefreshToken) + } + if got.Extra("id_token") != "test-id-token" || got.Extra("scope") != "scope1" { + t.Errorf("%s() Extra(id_token, scope) = (%v, %v), want (test-id-token, scope1)", name, got.Extra("id_token"), got.Extra("scope")) + } + data, err := MarshalWithExtras(got, "") + if err != nil { + t.Errorf("MarshalWithExtras(%s()) unexpected error: %v", name, err) + continue + } + var m map[string]interface{} + if err := json.Unmarshal(data, &m); err != nil { + t.Errorf("json.Unmarshal(MarshalWithExtras(%s())) unexpected error: %v", name, err) + } else if m["refresh_token"] != tc.wantRefreshToken { + t.Errorf("MarshalWithExtras(%s()) = %s, want refresh_token %q", name, data, tc.wantRefreshToken) + } + } + }) + } +} + +func TestFetchTokenRetrieveError(t *testing.T) { + srv := newTokenServer(t, http.StatusBadRequest, `{"error": "invalid_grant", "error_description": "Bad Request"}`) + settings := &Settings{ + AuthType: AuthTypeOAuth, + CredentialsJSON: authorizedUserJSON(srv.URL), + Scope: "scope1", + } + + _, err := FetchAuthToken(context.Background(), settings) + var authErr *auth.Error + if !errors.As(err, &authErr) { + t.Fatalf("FetchAuthToken() error = %v, want *auth.Error", err) + } + + _, err = FetchToken(context.Background(), settings) + var retrieveErr *oauth2.RetrieveError + if !errors.As(err, &retrieveErr) { + t.Fatalf("FetchToken() error = %v, want *oauth2.RetrieveError in chain", err) + } + if retrieveErr.ErrorCode != "invalid_grant" { + t.Errorf("RetrieveError.ErrorCode = %q, want %q", retrieveErr.ErrorCode, "invalid_grant") + } +} + +func TestJWTTokenProvider(t *testing.T) { + saJSON := serviceAccountJSON(t) + tests := []struct { + name string + settings *Settings + wantAudience string + wantScope string + }{ + { + name: "audience takes precedence over scope", + settings: &Settings{AuthType: AuthTypeJWT, CredentialsJSON: saJSON, Audience: "https://pubsub.googleapis.com/", Scope: "scope1"}, + wantAudience: "https://pubsub.googleapis.com/", + }, + { + name: "scope only", + settings: &Settings{AuthType: AuthTypeJWT, CredentialsJSON: saJSON, Scope: "scope1 scope2"}, + wantScope: "scope1 scope2", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tok, err := FetchAuthToken(context.Background(), tc.settings) + if err != nil { + t.Fatalf("FetchAuthToken() unexpected error: %v", err) + } + if tok.Type != "Bearer" || tok.Expiry.IsZero() { + t.Errorf("FetchAuthToken() = %+v, want Bearer token with expiry", tok) + } + claims := jwtClaims(t, tok.Value) + if got, _ := claims["aud"].(string); got != tc.wantAudience { + t.Errorf("JWT aud = %q, want %q", got, tc.wantAudience) + } + if got, _ := claims["scope"].(string); got != tc.wantScope { + t.Errorf("JWT scope = %q, want %q", got, tc.wantScope) + } + if got, _ := claims["iss"].(string); got != "test-sa@test-project.iam.gserviceaccount.com" { + t.Errorf("JWT iss = %q, want service account email", got) + } + + ts, err := JWTTokenSource(context.Background(), tc.settings) + if err != nil { + t.Fatalf("JWTTokenSource() unexpected error: %v", err) + } + if _, err := ts.Token(); err != nil { + t.Errorf("JWTTokenSource().Token() unexpected error: %v", err) + } + }) + } +} + +func TestJWTTokenProviderErrors(t *testing.T) { + saJSON := serviceAccountJSON(t) + tests := []struct { + name string + settings *Settings + wantErr string + }{ + { + name: "neither audience nor scope", + settings: &Settings{AuthType: AuthTypeJWT, CredentialsJSON: saJSON}, + wantErr: "Neither audience nor scope is provided", + }, + { + name: "non service account credentials", + settings: &Settings{AuthType: AuthTypeJWT, CredentialsJSON: authorizedUserJSON("http://localhost/token"), Audience: "aud"}, + wantErr: `self-signed JWT requires "service_account" credentials, got "authorized_user"`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if _, err := JWTTokenProvider(context.Background(), tc.settings); err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Errorf("JWTTokenProvider() error = %v, want error containing %q", err, tc.wantErr) + } + if _, err := JWTTokenSource(context.Background(), tc.settings); err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Errorf("JWTTokenSource() error = %v, want error containing %q", err, tc.wantErr) + } + }) + } +} + +func TestFindJSONCredentialsPreservesJSONAndProjectID(t *testing.T) { + saJSON := serviceAccountJSON(t) + settings := &Settings{CredentialsJSON: saJSON, Scope: "scope1"} + + authCreds, err := FindAuthCredentials(context.Background(), settings) + if err != nil { + t.Fatalf("FindAuthCredentials() unexpected error: %v", err) + } + if string(authCreds.JSON()) != saJSON { + t.Errorf("FindAuthCredentials().JSON() = %s, want %s", authCreds.JSON(), saJSON) + } + + creds, err := FindJSONCredentials(context.Background(), settings) + if err != nil { + t.Fatalf("FindJSONCredentials() unexpected error: %v", err) + } + if string(creds.JSON) != saJSON { + t.Errorf("FindJSONCredentials().JSON = %s, want %s", creds.JSON, saJSON) + } + if creds.ProjectID != "test-project" { + t.Errorf("FindJSONCredentials().ProjectID = %q, want %q", creds.ProjectID, "test-project") + } + if creds.TokenSource == nil { + t.Error("FindJSONCredentials().TokenSource = nil, want non-nil") + } +} diff --git a/util/settings.go b/util/settings.go index 20e9a62..89522a4 100644 --- a/util/settings.go +++ b/util/settings.go @@ -16,8 +16,6 @@ package util import ( "cloud.google.com/go/auth" - // Blank import for go mod tidy and vendor; will be imported directly in util/fetch.go. - _ "cloud.google.com/go/auth/credentials" "golang.org/x/oauth2/authhandler" ) From 5fdf009ea7ffebf88d67ab422c2f119f0dc84abe Mon Sep 17 00:00:00 2001 From: Radhika Agrawal Date: Fri, 30 Oct 2026 19:56:27 +0000 Subject: [PATCH 2/4] fix(util): address review feedback on fetch.go migration - Bind legacy token sources to the caller's ctx and honor an *http.Client stored under oauth2.HTTPClient via DetectOptions.Client. - Use strings.Fields for scopes; empty Scope yields no scopes. - Share JWT audience/scope validation; clearer error when ADC has no JSON. - Keep the API key short-circuit in one place (newCredentials). - Fix 'authentcation' typo and 'SHA256 encryption' comment. - Add tests for ctx client/cancellation, refresh JSON token_uri round trip, service account with empty scope, withRefreshToken and JSON-less ADC. --- util/auth-handlers_test.go | 1 + util/fetch.go | 90 ++++++++++------ util/fetch_test.go | 214 ++++++++++++++++++++++++++++++++++++- util/pkce.go | 4 +- 4 files changed, 274 insertions(+), 35 deletions(-) diff --git a/util/auth-handlers_test.go b/util/auth-handlers_test.go index 1fc7610..73b3ff7 100644 --- a/util/auth-handlers_test.go +++ b/util/auth-handlers_test.go @@ -123,6 +123,7 @@ func TestGet3LOAuthHandlerLoopbackError(t *testing.T) { } func TestGet3LOAuthHandlerLoopbackServerNotListening(t *testing.T) { + silenceStdout(t) wantErr := errors.New("server failed to start") fakeSrv := &fakeAuthCodeServer{ listening: false, diff --git a/util/fetch.go b/util/fetch.go index baff7ee..77847de 100644 --- a/util/fetch.go +++ b/util/fetch.go @@ -19,6 +19,7 @@ import ( "encoding/json" "errors" "fmt" + "net/http" "strings" "cloud.google.com/go/auth" @@ -58,22 +59,27 @@ func parseCredentialsFileHeader(b []byte) credentialsFileHeader { // newDetectOptions builds credentials.DetectOptions from settings. // +// An *http.Client stored in ctx under oauth2.HTTPClient is used for token +// requests, as golang.org/x/oauth2/google did. +// // cloud.google.com/go/auth rejects options that set both Scopes and // Audience, so Audience is only used for self-signed JWTs (where it takes // precedence over Scope); OAuth flows only use Scope. This lets callers keep // setting both fields (e.g. for STS token exchange) as before. -func newDetectOptions(settings *Settings, useSelfSignedJWT bool) *credentials.DetectOptions { +func newDetectOptions(ctx context.Context, settings *Settings, useSelfSignedJWT bool) *credentials.DetectOptions { + opts := &credentials.DetectOptions{Client: httpClientFromContext(ctx)} if settings == nil { - return &credentials.DetectOptions{Scopes: []string{DefaultScope}} - } - opts := &credentials.DetectOptions{ - Subject: settings.Email, - UseSelfSignedJWT: useSelfSignedJWT, + opts.Scopes = []string{DefaultScope} + return opts } + opts.Subject = settings.Email + opts.UseSelfSignedJWT = useSelfSignedJWT if useSelfSignedJWT && settings.Audience != "" { opts.Audience = settings.Audience - } else if settings.Scope != "" { - opts.Scopes = strings.Split(settings.Scope, " ") + } else { + // An empty Scope yields no scopes, which produces the same empty + // "scope" claim/parameter as the previous strings.Split("", " "). + opts.Scopes = strings.Fields(settings.Scope) } if settings.CredentialsJSON != "" { opts.CredentialsJSON = []byte(settings.CredentialsJSON) @@ -99,21 +105,34 @@ func newDetectOptions(settings *Settings, useSelfSignedJWT bool) *credentials.De return opts } +// httpClientFromContext returns the *http.Client stored in ctx under +// oauth2.HTTPClient, or nil to use the default client. +func httpClientFromContext(ctx context.Context) *http.Client { + if ctx == nil { + return nil + } + c, _ := ctx.Value(oauth2.HTTPClient).(*http.Client) + return c +} + // newCredentials returns credentials for the given settings. -// Returns nil for API keys. +// Returns nil for API keys (settings.APIKey set or AuthType "apikey"). func newCredentials(ctx context.Context, settings *Settings) (*auth.Credentials, error) { if settings == nil { return FindAuthCredentials(ctx, nil) } + if settings.APIKey != "" { + return nil, nil + } switch settings.GetAuthType() { case AuthTypeAPIKey: return nil, nil case AuthTypeOAuth: return FindAuthCredentials(ctx, settings) case AuthTypeJWT: - return findJWTCredentials(settings) + return findJWTCredentials(ctx, settings, "JWTTokenProvider") default: - return nil, fmt.Errorf("Unsupported authentcation method: %s", settings.GetAuthType()) + return nil, fmt.Errorf("Unsupported authentication method: %s", settings.GetAuthType()) } } @@ -124,9 +143,6 @@ func newCredentials(ctx context.Context, settings *Settings) (*auth.Credentials, // For 3-legged OAuth flows, the refresh token (if any) is available via // token.MetadataString("refresh_token"). func FetchAuthToken(ctx context.Context, settings *Settings) (*auth.Token, error) { - if settings != nil && settings.APIKey != "" { - return nil, nil - } creds, err := newCredentials(ctx, settings) if err != nil { return nil, err @@ -178,24 +194,31 @@ func OAuthJSONTokenProvider(ctx context.Context, settings *Settings) (auth.Token // mints self-signed JWT access tokens from service account credentials. // settings.Audience takes precedence over settings.Scope. func JWTTokenProvider(ctx context.Context, settings *Settings) (auth.TokenProvider, error) { - creds, err := findJWTCredentials(settings) + creds, err := findJWTCredentials(ctx, settings, "JWTTokenProvider") if err != nil { return nil, err } return creds, nil } -func findJWTCredentials(settings *Settings) (*auth.Credentials, error) { +// findJWTCredentials returns service account credentials configured for +// self-signed JWTs. caller names the public function in validation errors. +func findJWTCredentials(ctx context.Context, settings *Settings, caller string) (*auth.Credentials, error) { if settings == nil || (settings.Audience == "" && settings.Scope == "") { - return nil, errors.New("Neither audience nor scope is provided for JWTTokenProvider") + // Capitalized to keep the message JWTTokenSource has always returned. + return nil, fmt.Errorf("Neither audience nor scope is provided for %s", caller) } - creds, err := credentials.DetectDefault(newDetectOptions(settings, true)) + creds, err := credentials.DetectDefault(newDetectOptions(ctx, settings, true)) if err != nil { return nil, err } // cloud.google.com/go/auth silently ignores UseSelfSignedJWT for // non-service-account credentials; keep rejecting them as before. - if t := parseCredentialsFileHeader(creds.JSON()).Type; t != serviceAccountCredentialsType { + credsJSON := creds.JSON() + if len(credsJSON) == 0 { + return nil, errors.New("self-signed JWT requires a service account key file; no JSON credentials were found") + } + if t := parseCredentialsFileHeader(credsJSON).Type; t != serviceAccountCredentialsType { return nil, fmt.Errorf("self-signed JWT requires %q credentials, got %q", serviceAccountCredentialsType, t) } return creds, nil @@ -204,8 +227,11 @@ func findJWTCredentials(settings *Settings) (*auth.Credentials, error) { // FindAuthCredentials obtains cloud.google.com/go/auth credentials from // settings or Application Default Credentials. If settings is nil, // Application Default Credentials with DefaultScope are used. +// +// ctx is only used to look up an *http.Client stored under +// oauth2.HTTPClient; credentials.DetectDefault does not take a context. func FindAuthCredentials(ctx context.Context, settings *Settings) (*auth.Credentials, error) { - return credentials.DetectDefault(newDetectOptions(settings, false)) + return credentials.DetectDefault(newDetectOptions(ctx, settings, false)) } // Returns a token from the given settings. @@ -229,19 +255,16 @@ func OAuthJSONTokenSource(ctx context.Context, settings *Settings) (oauth2.Token if err != nil { return nil, err } - return tokenSourceFromCredentials(creds), nil + return tokenSourceFromCredentials(ctx, creds), nil } // Deprecated: Use JWTTokenProvider instead. func JWTTokenSource(ctx context.Context, settings *Settings) (oauth2.TokenSource, error) { - if settings == nil || (settings.Audience == "" && settings.Scope == "") { - return nil, errors.New("Neither audience nor scope is provided for JWTTokenSource") - } - creds, err := findJWTCredentials(settings) + creds, err := findJWTCredentials(ctx, settings, "JWTTokenSource") if err != nil { return nil, err } - return tokenSourceFromCredentials(creds), nil + return tokenSourceFromCredentials(ctx, creds), nil } // FindJSONCredentials obtains credentials from settings or Application Default Credentials @@ -253,7 +276,7 @@ func FindJSONCredentials(ctx context.Context, settings *Settings) (*google.Crede return nil, err } c := oauth2adapt.Oauth2CredentialsFromAuthCredentials(creds) - c.TokenSource = tokenSourceFromCredentials(creds) + c.TokenSource = tokenSourceFromCredentials(ctx, creds) return c, nil } @@ -261,16 +284,23 @@ func FindJSONCredentials(ctx context.Context, settings *Settings) (*google.Crede // oauth2.TokenSource. Unlike oauth2adapt.TokenSourceFromTokenProvider, it // also populates oauth2.Token.RefreshToken so that 3LO refresh tokens are // preserved for legacy callers. -func tokenSourceFromCredentials(creds *auth.Credentials) oauth2.TokenSource { - return &oauth2TokenSource{creds: creds} +// +// Tokens are fetched with ctx, matching golang.org/x/oauth2/google, whose +// token sources were bound to the context they were created with. +func tokenSourceFromCredentials(ctx context.Context, creds *auth.Credentials) oauth2.TokenSource { + if ctx == nil { + ctx = context.Background() + } + return &oauth2TokenSource{ctx: ctx, creds: creds} } type oauth2TokenSource struct { + ctx context.Context creds *auth.Credentials } func (ts *oauth2TokenSource) Token() (*oauth2.Token, error) { - tok, err := ts.creds.Token(context.Background()) + tok, err := ts.creds.Token(ts.ctx) if err != nil { return nil, oauth2Error(err) } diff --git a/util/fetch_test.go b/util/fetch_test.go index bb10e35..048d795 100644 --- a/util/fetch_test.go +++ b/util/fetch_test.go @@ -162,7 +162,7 @@ func TestNewDetectOptions(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - opts := newDetectOptions(tc.settings, tc.useSelfSignedJWT) + opts := newDetectOptions(context.Background(), tc.settings, tc.useSelfSignedJWT) if !reflect.DeepEqual(opts.Scopes, tc.wantScopes) { t.Errorf("Scopes = %q, want %q", opts.Scopes, tc.wantScopes) } @@ -218,7 +218,7 @@ func TestNewDetectOptionsAuthHandlerOptions(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got := newDetectOptions(tc.settings, false).AuthHandlerOptions + got := newDetectOptions(context.Background(), tc.settings, false).AuthHandlerOptions if tc.wantNil { if got != nil { t.Errorf("AuthHandlerOptions = %+v, want nil", got) @@ -259,7 +259,7 @@ func TestFetchAuthTokenAPIKey(t *testing.T) { func TestFetchAuthTokenUnsupportedAuthType(t *testing.T) { _, err := FetchAuthToken(context.Background(), &Settings{AuthType: AuthTypeSSO}) - if err == nil || !strings.Contains(err.Error(), "Unsupported authentcation method: sso") { + if err == nil || !strings.Contains(err.Error(), "Unsupported authentication method: sso") { t.Errorf("FetchAuthToken() error = %v, want unsupported authentication method", err) } } @@ -473,3 +473,211 @@ func TestFindJSONCredentialsPreservesJSONAndProjectID(t *testing.T) { t.Error("FindJSONCredentials().TokenSource = nil, want non-nil") } } + +func TestNewDetectOptionsScopeFields(t *testing.T) { + tests := []struct { + scope string + wantScopes []string + }{ + {scope: "", wantScopes: nil}, + {scope: "scope1 scope2 ", wantScopes: []string{"scope1", "scope2"}}, + } + for _, tc := range tests { + opts := newDetectOptions(context.Background(), &Settings{Scope: tc.scope}, false) + if len(opts.Scopes) != len(tc.wantScopes) || (len(tc.wantScopes) > 0 && !reflect.DeepEqual(opts.Scopes, tc.wantScopes)) { + t.Errorf("newDetectOptions(Scope: %q).Scopes = %q, want %q", tc.scope, opts.Scopes, tc.wantScopes) + } + } +} + +func TestNewDetectOptionsHTTPClientFromContext(t *testing.T) { + client := &http.Client{} + ctx := context.WithValue(context.Background(), oauth2.HTTPClient, client) + if got := newDetectOptions(ctx, &Settings{Scope: "scope1"}, false).Client; got != client { + t.Errorf("Client = %p, want client from context %p", got, client) + } + if got := newDetectOptions(context.Background(), &Settings{Scope: "scope1"}, false).Client; got != nil { + t.Errorf("Client = %p, want nil without oauth2.HTTPClient in context", got) + } +} + +// recordingTransport records the requested URLs and forwards to +// http.DefaultTransport. +type recordingTransport struct { + urls []string +} + +func (rt *recordingTransport) RoundTrip(r *http.Request) (*http.Response, error) { + rt.urls = append(rt.urls, r.URL.String()) + return http.DefaultTransport.RoundTrip(r) +} + +func TestFetchTokenUsesHTTPClientFromContext(t *testing.T) { + srv := newTokenServer(t, http.StatusOK, `{"access_token": "test-access-token", "token_type": "Bearer", "expires_in": 3600}`) + rt := &recordingTransport{} + ctx := context.WithValue(context.Background(), oauth2.HTTPClient, &http.Client{Transport: rt}) + settings := &Settings{AuthType: AuthTypeOAuth, CredentialsJSON: authorizedUserJSON(srv.URL), Scope: "scope1"} + + if _, err := FetchToken(ctx, settings); err != nil { + t.Fatalf("FetchToken() unexpected error: %v", err) + } + if len(rt.urls) != 1 || rt.urls[0] != srv.URL { + t.Errorf("requests through context client = %q, want [%q]", rt.urls, srv.URL) + } +} + +func TestOAuthJSONTokenSourceUsesContext(t *testing.T) { + srv := newTokenServer(t, http.StatusOK, `{"access_token": "test-access-token", "token_type": "Bearer", "expires_in": 3600}`) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + settings := &Settings{AuthType: AuthTypeOAuth, CredentialsJSON: authorizedUserJSON(srv.URL), Scope: "scope1"} + + ts, err := OAuthJSONTokenSource(ctx, settings) + if err != nil { + t.Fatalf("OAuthJSONTokenSource() unexpected error: %v", err) + } + if _, err := ts.Token(); !errors.Is(err, context.Canceled) { + t.Errorf("Token() with canceled context error = %v, want %v", err, context.Canceled) + } +} + +// TestFetchAuthTokenRefreshTokenJSON checks that the refresh token JSON built +// by BuildAuthRefreshTokenJSON is sent to the client secret's token_uri. +func TestFetchAuthTokenRefreshTokenJSON(t *testing.T) { + var gotForm map[string][]string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if err := r.ParseForm(); err != nil { + t.Errorf("ParseForm() unexpected error: %v", err) + } + gotForm = r.PostForm + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"access_token": "refreshed-access-token", "token_type": "Bearer", "expires_in": 3600}`) + })) + t.Cleanup(srv.Close) + + clientSecretJSON := fmt.Sprintf(`{"installed": { + "client_id": "test-client.apps.googleusercontent.com", + "client_secret": "test-secret", + "auth_uri": "https://accounts.google.com/o/oauth2/auth", + "token_uri": %q, + "redirect_uris": ["http://localhost"] +}}`, srv.URL) + creds := auth.NewCredentials(&auth.CredentialsOptions{JSON: []byte(clientSecretJSON)}) + refreshJSON := BuildAuthRefreshTokenJSON(testRefreshToken, creds) + if refreshJSON == "" { + t.Fatal("BuildAuthRefreshTokenJSON() = \"\", want refresh token JSON") + } + + tok, err := FetchAuthToken(context.Background(), &Settings{AuthType: AuthTypeOAuth, CredentialsJSON: refreshJSON, Scope: "scope1"}) + if err != nil { + t.Fatalf("FetchAuthToken() unexpected error: %v", err) + } + if tok.Value != "refreshed-access-token" { + t.Errorf("FetchAuthToken().Value = %q, want %q", tok.Value, "refreshed-access-token") + } + if got := tok.MetadataString("refresh_token"); got != testRefreshToken { + t.Errorf("FetchAuthToken() refresh_token = %q, want %q", got, testRefreshToken) + } + if g := gotForm["grant_type"]; len(g) != 1 || g[0] != "refresh_token" { + t.Errorf("grant_type = %q, want refresh_token", g) + } + if g := gotForm["refresh_token"]; len(g) != 1 || g[0] != testRefreshToken { + t.Errorf("refresh_token = %q, want %q", g, testRefreshToken) + } +} + +// TestFetchAuthTokenServiceAccountEmptyScope checks that an OAuth fetch with +// service account JSON and no scope still succeeds, sending an empty scope +// claim as golang.org/x/oauth2 did. +func TestFetchAuthTokenServiceAccountEmptyScope(t *testing.T) { + var gotScope interface{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if err := r.ParseForm(); err != nil { + t.Errorf("ParseForm() unexpected error: %v", err) + } + claims := jwtClaims(t, r.PostForm.Get("assertion")) + gotScope = claims["scope"] + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"access_token": "sa-access-token", "token_type": "Bearer", "expires_in": 3600}`) + })) + t.Cleanup(srv.Close) + + var sa map[string]string + if err := json.Unmarshal([]byte(serviceAccountJSON(t)), &sa); err != nil { + t.Fatalf("json.Unmarshal() unexpected error: %v", err) + } + sa["token_uri"] = srv.URL + saJSON, err := json.Marshal(sa) + if err != nil { + t.Fatalf("json.Marshal() unexpected error: %v", err) + } + + tok, err := FetchAuthToken(context.Background(), &Settings{AuthType: AuthTypeOAuth, CredentialsJSON: string(saJSON)}) + if err != nil { + t.Fatalf("FetchAuthToken() unexpected error: %v", err) + } + if tok.Value != "sa-access-token" { + t.Errorf("FetchAuthToken().Value = %q, want %q", tok.Value, "sa-access-token") + } + if gotScope != nil && gotScope != "" { + t.Errorf("JWT assertion scope = %v, want empty", gotScope) + } +} + +func TestWithRefreshToken(t *testing.T) { + userJSON := []byte(authorizedUserJSON("http://localhost/token")) + tests := []struct { + name string + tok *auth.Token + credsJSON []byte + want string + }{ + {name: "nil token", tok: nil, credsJSON: userJSON}, + {name: "metadata already has refresh token", tok: &auth.Token{Metadata: map[string]interface{}{"refresh_token": "existing"}}, credsJSON: userJSON, want: "existing"}, + {name: "non authorized_user credentials", tok: &auth.Token{}, credsJSON: []byte(`{"type": "service_account", "refresh_token": "x"}`)}, + {name: "empty credentials JSON", tok: &auth.Token{}}, + {name: "authorized_user fills refresh token", tok: &auth.Token{Metadata: map[string]interface{}{"scope": "scope1"}}, credsJSON: userJSON, want: testRefreshToken}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var before map[string]interface{} + if tc.tok != nil && tc.tok.Metadata != nil { + before = make(map[string]interface{}, len(tc.tok.Metadata)) + for k, v := range tc.tok.Metadata { + before[k] = v + } + } + got := withRefreshToken(tc.tok, tc.credsJSON) + if tc.tok == nil { + if got != nil { + t.Errorf("withRefreshToken(nil) = %+v, want nil", got) + } + return + } + if s := got.MetadataString("refresh_token"); s != tc.want { + t.Errorf("refresh_token = %q, want %q", s, tc.want) + } + if before != nil && !reflect.DeepEqual(tc.tok.Metadata, before) { + t.Errorf("input token metadata mutated: got %v, want %v", tc.tok.Metadata, before) + } + }) + } +} + +func TestFindJWTCredentialsNoJSON(t *testing.T) { + // Point ADC at nothing so only a JSON-less source could be found; on + // machines without GCE metadata this returns a "could not find default + // credentials" error instead, which is also acceptable here. + t.Setenv("GOOGLE_APPLICATION_CREDENTIALS", "") + t.Setenv("HOME", t.TempDir()) + t.Setenv("APPDATA", t.TempDir()) + t.Setenv("CLOUDSDK_CONFIG", t.TempDir()) + t.Setenv("GCE_METADATA_HOST", "") + _, err := JWTTokenProvider(context.Background(), &Settings{AuthType: AuthTypeJWT, Audience: "aud"}) + if err == nil { + t.Fatal("JWTTokenProvider() without credentials error = nil, want error") + } + if strings.Contains(err.Error(), `got ""`) { + t.Errorf("JWTTokenProvider() error = %v, want a message that does not report an empty type", err) + } +} diff --git a/util/pkce.go b/util/pkce.go index 7bf309d..4d61c0b 100644 --- a/util/pkce.go +++ b/util/pkce.go @@ -24,7 +24,7 @@ import ( ) // GeneratePKCEOptions generates a unique PKCE challenge and verifier combination, -// using UUID, SHA256 encryption, and base64 URL encoding with no padding. +// using UUID, SHA-256 hashing, and base64 URL encoding with no padding. func GeneratePKCEOptions() *auth.PKCEOptions { verifier := uuid.New().String() sha := sha256.Sum256([]byte(verifier)) @@ -38,7 +38,7 @@ func GeneratePKCEOptions() *auth.PKCEOptions { } // GeneratePKCEParams generates a unique PKCE challenge and verifier combination, -// using UUID, SHA256 encryption, and base64 URL encoding with no padding. +// using UUID, SHA-256 hashing, and base64 URL encoding with no padding. // // Deprecated: Use GeneratePKCEOptions instead. func GeneratePKCEParams() *authhandler.PKCEParams { From f9fd6ae7d893ff745742c42747735d1555212ec5 Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Fri, 30 Oct 2026 14:44:38 -0700 Subject: [PATCH 3/4] fix: Remove stdout silencing in test function Remove call to silenceStdout in TestGet3LOAuthHandlerLoopback. --- util/auth-handlers_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/util/auth-handlers_test.go b/util/auth-handlers_test.go index 73b3ff7..32edafc 100644 --- a/util/auth-handlers_test.go +++ b/util/auth-handlers_test.go @@ -68,7 +68,6 @@ func silenceStdout(t *testing.T) { } func TestGet3LOAuthHandlerLoopback(t *testing.T) { - silenceStdout(t) fakeSrv := &fakeAuthCodeServer{ listening: true, authCode: AuthorizationCode{ From c6673584ec389874ba1f282815ab6405cd2574e3 Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Fri, 30 Oct 2026 14:46:41 -0700 Subject: [PATCH 4/4] fix: Enhance OAuth handler tests for loopback cases Added tests for OAuth handler loopback scenarios. --- util/auth-handlers_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/util/auth-handlers_test.go b/util/auth-handlers_test.go index 32edafc..afc0c6f 100644 --- a/util/auth-handlers_test.go +++ b/util/auth-handlers_test.go @@ -68,6 +68,7 @@ func silenceStdout(t *testing.T) { } func TestGet3LOAuthHandlerLoopback(t *testing.T) { + silenceStdout(t) fakeSrv := &fakeAuthCodeServer{ listening: true, authCode: AuthorizationCode{ @@ -122,6 +123,8 @@ func TestGet3LOAuthHandlerLoopbackError(t *testing.T) { } func TestGet3LOAuthHandlerLoopbackServerNotListening(t *testing.T) { + // The handler prints user instructions to stdout before reporting, + // keep them out of the test output. silenceStdout(t) wantErr := errors.New("server failed to start") fakeSrv := &fakeAuthCodeServer{