Give iamSigner pointer receivers so the email cache works

[omlahore]

iamSigner.Email is written to discover the service account once and cache it:

func (s iamSigner) Email(ctx context.Context) (string, error) {
	if s.serviceAcct != "" {
		return s.serviceAcct, nil
	}

	s.mutex.Lock()
	defer s.mutex.Unlock()
	result, err := s.callMetadataService(ctx)
	...
	s.serviceAcct = result
	return result, nil
}

The receiver is a value, so s.serviceAcct = result writes to a copy that is discarded when the method returns. The guard at the top therefore never sees a cached value, and every call re-queries the GCE metadata server over HTTP. Sign calls Email, so that is an extra round trip per custom token minted through the IAM path.

Two things say caching is the intent rather than the value receiver being deliberate: the early-return check exists at all, and mutex is held as a *sync.Mutex so that copies of the struct still share one lock. Guarding a write that cannot persist is only explicable as an oversight.

staticcheck reports it as SA4005, "ineffective assignment to field iamSigner.serviceAcct".

The change moves all four iamSigner methods to pointer receivers together, so the method set stays consistent. newIAMSigner already returns *iamSigner, newCryptoSigner passes that straight through, and auth_test.go:129 asserts client.signer.(*iamSigner), so nothing stores a bare iamSigner value and the cryptoSigner interface is still satisfied.

Verified: go build ./... passes, go test ./auth/... green, gofmt -l clean, SA4005 gone under GOOS=linux and GOOS=darwin.

Separately, and not included here since it is a different concern: auth/user_mgt.go:1463-1464 and auth/project_config_mgt.go:29 spell the struct tag option omitEmpty, which encoding/json does not recognise (it only accepts omitempty). Both structs are decode-only today so nothing misbehaves at runtime, but the tags do not do what they read as. Say the word and it goes in a separate PR.

[gemini-code-assist]

Code Review

This pull request updates the receiver type of iamSigner methods in auth/token_generator.go from value to pointer receivers. The review feedback highlights that this change introduces a data race and a concurrency bug in the Email method due to unprotected concurrent access to s.serviceAcct, and suggests acquiring the lock at the beginning of the method to ensure thread safety and prevent redundant metadata service calls.

[gemini-code-assist]

Changing iamSigner to use pointer receivers enables caching, but it also introduces a data race and a concurrency bug in the Email method:\n\n1. Data Race: s.serviceAcct is read on line 221 without holding s.mutex, but it is written on line 236 under the lock. Since iamSigner is now shared via pointer, concurrent calls to Email will result in concurrent read/write access to s.serviceAcct, which is a data race.\n2. Redundant HTTP Calls: If multiple goroutines call Email concurrently while s.serviceAcct is empty, they will all pass the initial s.serviceAcct != "" check. The first goroutine will acquire the lock and fetch the email. The subsequent goroutines will block at s.mutex.Lock(), and once unblocked, they will proceed to call callMetadataService again because there is no double-check inside the lock.\n\n### Suggested Fix\n\nTo resolve both issues, acquire the lock at the very beginning of the method and perform the check inside the lock:\n\ngo\nfunc (s *iamSigner) Email(ctx context.Context) (string, error) {\n\ts.mutex.Lock()\n\tdefer s.mutex.Unlock()\n\n\tif s.serviceAcct != "" {\n\t return s.serviceAcct, nil\n\t}\n\n\tresult, err := s.callMetadataService(ctx)\n\tif err != nil {\n\t msg := "failed to determine service account: %v; initialize the SDK with service " +\n\t "account credentials or specify a service account with iam.serviceAccounts.signBlob " +\n\t "permission; refer to https://firebase.google.com/docs/auth/admin/create-custom-tokens " +\n\t "for more details on creating custom tokens"\n\t return "", fmt.Errorf(msg, err)\n\t}\n\n\ts.serviceAcct = result\n\treturn result, nil\n}\n

[omlahore]

Both points are correct, and the second one is worse than it reads. Fixed in d28f511 by taking the lock before the cache check rather than after it.

I added a test that runs 50 concurrent Email() calls against a counting metadata server. On the previous commit it reports WARNING: DATA RACE under -race and, more usefully, 50 metadata requests for 50 calls: with the check outside the lock every goroutine passes it, then serialises on the mutex and fetches anyway. So the pointer receiver alone moved the caching from "never works" to "never works when it matters".

With the lock moved up it is 1 request, and go test -race ./auth/... is clean.