Skip to content

Commit 75172cf

Browse files
quartzmohongalex
andauthored
fix(idtoken): avoid double impersonation in tokenSourceFromBytes (#3576)
This PR fixes a parallel double impersonation bug in the `idtoken` package. The library incorrectly does not use the `source_credentials` subfield in the JSON struct when constructing the inner client, and instead passes the entire credential JSON. This causes the lower layers (`htransport.NewClient`) to correctly (but unexpectedly for this context) build an authenticated HTTP client that is already impersonated, leading to self-impersonation when calling `generateIdToken`. This PR fixes the issue by extracting or recreating non-impersonated credentials before calling `impersonate.IDTokenSource`, avoiding the double wrap. Note: This PR does not add new unit tests for the call sequence because `impersonate.IDTokenSource` hardcodes the IAM credentials endpoint, making it impossible to intercept with a mock client or server without modifying that package. The existing unit tests in this package only cover type validation and do not successfully execute the full impersonation flow due to this same limitation. closes: #2301 Co-authored-by: Alex Hong <9397363+hongalex@users.noreply.github.com>
1 parent 2de1a5a commit 75172cf

1 file changed

Lines changed: 49 additions & 1 deletion

File tree

idtoken/idtoken.go

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,19 @@ func tokenSourceFromBytes(ctx context.Context, data []byte, audience string, ds
231231
TargetPrincipal: account,
232232
IncludeEmail: true,
233233
}
234-
ts, err := impersonate.IDTokenSource(ctx, config, option.WithAuthCredentialsJSON(credType, data))
234+
235+
baseData, err := baseDataForImpersonation(credType, data)
236+
if err != nil {
237+
return nil, err
238+
}
239+
240+
var opts []option.ClientOption
241+
opts = append(opts, option.WithCredentialsJSON(baseData))
242+
if ds.HTTPClient != nil {
243+
opts = append(opts, option.WithHTTPClient(ds.HTTPClient))
244+
}
245+
246+
ts, err := impersonate.IDTokenSource(ctx, config, opts...)
235247
if err != nil {
236248
return nil, err
237249
}
@@ -241,6 +253,42 @@ func tokenSourceFromBytes(ctx context.Context, data []byte, audience string, ds
241253
}
242254
}
243255

256+
// baseDataForImpersonation extracts or recreates non-impersonated credentials
257+
// from the provided JSON data to avoid double impersonation. The problem is
258+
// that passing the entire credential JSON causes the lower layers to
259+
// automatically build an authenticated HTTP client that is already impersonated.
260+
// To fix this, we extract the non-impersonated source credentials or remove the
261+
// impersonation instructions before creating the client. This avoids leaky
262+
// abstractions and respects the separation of concerns by letting the lower
263+
// layers act as general loaders while handling the specific needs of idtoken
264+
// generation here.
265+
func baseDataForImpersonation(credType credentialstype.CredType, data []byte) ([]byte, error) {
266+
var baseData []byte
267+
if credType == ImpersonatedServiceAccount {
268+
type source struct {
269+
SourceCredentials json.RawMessage `json:"source_credentials"`
270+
}
271+
var s source
272+
if err := json.Unmarshal(data, &s); err != nil {
273+
return nil, err
274+
}
275+
baseData = s.SourceCredentials
276+
} else {
277+
// For ExternalAccount, we remove the service_account_impersonation_url
278+
var m map[string]interface{}
279+
if err := json.Unmarshal(data, &m); err != nil {
280+
return nil, err
281+
}
282+
delete(m, "service_account_impersonation_url")
283+
var err error
284+
baseData, err = json.Marshal(m)
285+
if err != nil {
286+
return nil, err
287+
}
288+
}
289+
return baseData, nil
290+
}
291+
244292
// WithCustomClaims optionally specifies custom private claims for an ID token.
245293
func WithCustomClaims(customClaims map[string]interface{}) ClientOption {
246294
return withCustomClaims(customClaims)

0 commit comments

Comments
 (0)