feat(pnv): Add support for Firebase Phone Number Verification#1203
feat(pnv): Add support for Firebase Phone Number Verification#1203lahirumaramba wants to merge 28 commits intomainfrom
Conversation
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request introduces the Firebase Phone Number Verification service, implementing JWT token verification using the Nimbus JOSE + JWT library. It includes the core service entry point, model classes for verified tokens, custom exception handling, and a verifier that checks JWT headers and claims against a remote JWKS. Feedback highlights a security vulnerability where the issuer claim was not validated against the specific project ID, potentially allowing tokens from other projects. Additionally, improvements were suggested for the token model to handle claim types more robustly and prevent potential class cast exceptions.
…abilize headless pipelines
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces the Firebase Phone Number Verification service to the Java Admin SDK. The changes include the main service entry point, internal token verification logic using the Nimbus JOSE + JWT library, custom exception handling, and a comprehensive suite of unit tests. Additionally, a new AGENTS.md file defines coding guidelines for AI assistants, and a minor formatting fix was applied to Template.java. Review feedback suggests improving null safety in the token verification logic by using a more robust equality check for the JWS algorithm.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces the Firebase Phone Number Verification service to the Java SDK, including the main service entry point, custom exception handling, and JWT token verification logic using the Nimbus library. It also adds a new AGENTS.md file to define repository-wide coding standards for AI assistants. The review feedback focuses on improving the robustness and consistency of the FirebasePhoneNumberVerificationTokenVerifier class, specifically by addressing the risks of calling overridable methods in constructors, adopting more idiomatic null-check patterns, and utilizing more specific exception types for internal configuration errors.
… URI.create().toURL()
…on for jwtProcessor to avoid constructor leak
…vate constructor overload
…ow IllegalStateException for hardcoded configuration failures
…ions checks with idiomatic checkNotNull patterns
…r null claims test case
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces the Firebase Phone Number Verification service to the Java SDK, including the main service entry point, token representation, and internal JWT verification logic using the Nimbus library. It also adds a new guidelines file for AI coding assistants and updates the project dependencies. A thread-safety issue was identified in the token verifier class, where a field used in a double-checked locking pattern lacks the volatile keyword, potentially leading to race conditions.
| private static final String HEADER_TYP = "JWT"; | ||
|
|
||
| private final String projectId; | ||
| private DefaultJWTProcessor<SecurityContext> jwtProcessor; |
There was a problem hiding this comment.
The jwtProcessor field is used in a double-checked locking pattern within getJwtProcessor() but is not declared as volatile. In the Java Memory Model, without the volatile keyword, another thread could see a non-null reference to a partially initialized DefaultJWTProcessor object, leading to unpredictable behavior or crashes.
| private DefaultJWTProcessor<SecurityContext> jwtProcessor; | |
| private volatile DefaultJWTProcessor<SecurityContext> jwtProcessor; |
…r strict double-checked locking memory safety
Add support for Firebase Phone Number Verification