Skip to content

feat(pnv): Add support for Firebase Phone Number Verification#1203

Open
lahirumaramba wants to merge 19 commits intomainfrom
lm-fpnv
Open

feat(pnv): Add support for Firebase Phone Number Verification#1203
lahirumaramba wants to merge 19 commits intomainfrom
lm-fpnv

Conversation

@lahirumaramba
Copy link
Copy Markdown
Member

Add support for Firebase Phone Number Verification

@lahirumaramba lahirumaramba added release-note release:stage Stage a release candidate labels May 5, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +144 to +148
if (claims.getAudience().isEmpty() || !claims.getAudience().contains(issuer)) {
throw new FirebasePhoneNumberVerificationException(FirebasePhoneNumberVerificationErrorCode.INVALID_TOKEN,
"Invalid audience. Expected to contain: " + issuer + " but found: " + claims.getAudience()
);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The token verification logic should validate that the iss (issuer) claim matches the expected value for the current project (https://fpnv.googleapis.com/projects/{projectId}). Since the JWKS is shared across all Firebase projects, failing to verify the issuer allows tokens from other projects to be accepted, which is a security vulnerability.

    String expectedIssuer = "https://fpnv.googleapis.com/projects/" + projectId;
    if (!expectedIssuer.equals(issuer)) {
      throw new FirebasePhoneNumberVerificationException(FirebasePhoneNumberVerificationErrorCode.INVALID_TOKEN,
          "Invalid issuer. Expected: " + expectedIssuer + " but found: " + issuer);
    }

    if (claims.getAudience().isEmpty() || !claims.getAudience().contains(issuer)) {
      throw new FirebasePhoneNumberVerificationException(FirebasePhoneNumberVerificationErrorCode.INVALID_TOKEN,
          "Invalid audience. Expected to contain: " + issuer + " but found: " + claims.getAudience()
      );
    }

* Returns the expiration time in seconds since the Unix epoch.
*/
public long getExpirationTime() {
return ((java.util.Date) claims.get("exp")).getTime() / 1000L;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Casting the exp claim directly to java.util.Date is fragile. While the Nimbus library typically returns Date objects, this class is public and could be instantiated with a map where these values are Long or Integer (e.g., if parsed from raw JSON). It's safer to handle both Date and Number types to avoid potential ClassCastException or NullPointerException.

    Object exp = claims.get("exp");
    if (exp instanceof java.util.Date) {
      return ((java.util.Date) exp).getTime() / 1000L;
    }
    return exp instanceof Number ? ((Number) exp).longValue() : 0L;

* Returns the issued-at time in seconds since the Unix epoch.
*/
public long getIssuedAt() {
return ((java.util.Date) claims.get("iat")).getTime() / 1000L;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the exp claim, the iat claim should be handled robustly to avoid potential ClassCastException or NullPointerException if the claim is missing or represented as a numeric type.

    Object iat = claims.get("iat");
    if (iat instanceof java.util.Date) {
      return ((java.util.Date) iat).getTime() / 1000L;
    }
    return iat instanceof Number ? ((Number) iat).longValue() : 0L;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:stage Stage a release candidate release-note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants