feat(pnv): Add Firebase Phone Number Verification support#757
Conversation
…S fetching - Added ClientWithoutAuth helper to internal.HTTPClient to safely strip the oauth2.Transport layer while retaining custom transport configurations (e.g., proxies). - Updated phonenumberverification.NewClient to utilize the App's HTTP client options (conf.Opts) when fetching JWKS, resolving the TODO item.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the Firebase Phone Number Verification (FPNV) service to the Go SDK. Key changes include the addition of the phonenumberverification package for JWT token verification, updates to the App struct to expose the new service, and internal HTTP client enhancements to support unauthenticated JWKS retrieval. Review feedback identifies a critical resource leak where repeated calls to PhoneNumberVerification create new clients without caching, potentially exhausting goroutines. Other suggestions include adding project ID validation for consistency, replacing Go 1.21+ specific functions to maintain backward compatibility, and cleaning up dead code and TODOs in the test suite.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces the Firebase Phone Number Verification (FPNV) service to the Go SDK, adding a new phonenumberverification package, updating the App struct with thread-safe client initialization, and including comprehensive tests. Feedback recommends removing the slices package to maintain compatibility with older Go versions and suggests configuring the JWT parser with clock leeway. Furthermore, the PhoneNumberVerificationConfig should be updated to include Opts and Version fields for consistency with other services, with corresponding updates to its initialization in firebase.go.
jonathanedey
left a comment
jonathanedey
left a comment
There was a problem hiding this comment.
LGTM with one nit, Thanks!
Co-authored-by: Kevin Cheung <kevinthecheung@users.noreply.github.com>
Co-authored-by: Kevin Cheung <kevinthecheung@users.noreply.github.com>
Co-authored-by: Kevin Cheung <kevinthecheung@users.noreply.github.com>
Co-authored-by: Kevin Cheung <kevinthecheung@users.noreply.github.com>
Co-authored-by: Kevin Cheung <kevinthecheung@users.noreply.github.com>
Co-authored-by: Kevin Cheung <kevinthecheung@users.noreply.github.com>
5 participants
Add support for Firebase Phone Number Verification