feat: add basic fpnv #3030

boikoa-gl · 2025-12-09T05:39:01Z

Hey there! So you want to contribute to a Firebase SDK?
Before you file this pull request, please read these guidelines:

Discussion

Read the contribution guidelines (CONTRIBUTING.md).
If this has been discussed in an issue, make sure to link to the issue here.
If not, go file an issue about this before creating a pull request to discuss.

Testing

Make sure all existing tests in the repository pass after your change.
If you fixed a bug or added a feature, add a new test to cover your code.

API Changes

At this time we cannot accept changes that affect the public API. If you'd like to help
us make Firebase APIs better, please propose your change in an issue so that we
can discuss it together.

gemini-code-assist · 2025-12-09T05:39:21Z

Summary of Changes

Hello @boikoa-gl, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a new Firebase Phone Number Verification (FPNV) module into the Firebase Admin SDK. This feature enables developers to verify phone number tokens, similar to how existing ID tokens are handled. The changes involve adding the necessary service classes, integrating them into the SDK's public API and build configuration, and establishing robust token verification logic, including custom error handling and support for the ES256 JWT algorithm.

Highlights

New FPNV Module: Introduced a new Firebase Phone Number Verification (FPNV) module to the SDK.
SDK Integration: Integrated the FPNV service into the Firebase Admin SDK's public API, including entrypoints.json, package.json exports, and the firebase-namespace-api.ts.
Token Verification Logic: Added core logic for verifying phone number tokens, including decoding, content validation, and signature verification using FirebasePhoneNumberTokenVerifier.
Error Handling: Implemented dedicated error handling for FPNV operations with the new FirebasePnvError class and FpnvErrorCode definitions.
ES256 Algorithm Support: Extended JWT verification utilities to support the ES256 algorithm.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a new fpnv (Firebase Phone Number Verification) service. The overall structure is good and follows the existing patterns in the SDK. However, there are a few critical issues that need to be addressed before merging. The error handling for this new service is incomplete, with placeholder error codes in FpnvErrorCode. This will lead to incorrect error reporting. I've provided a suggestion for a more complete implementation. There's also a potential bug in the JWT audience (aud) claim verification that could cause a runtime error. Finally, I've made some suggestions to use more specific error codes to improve the developer experience. Please review the detailed comments.

gemini-code-assist · 2025-12-09T05:41:24Z

src/utils/error.ts

+export class FpnvErrorCode {
+  static INVALID_TOKEN = 'invalid_token';
+  static EXPIRED_TOKEN = 'expired_token';
+  static COMMON_ISSUE = {
+    code: "code",
+    message: "message"
+  };
+  // TODO: need to update codes properly
+}


The FpnvErrorCode class is incomplete and uses placeholder values, as indicated by the TODO comment. The COMMON_ISSUE object has placeholder values for code and message, which will lead to incorrect error information being propagated from the SDK. Also, INVALID_TOKEN and EXPIRED_TOKEN are defined as strings, but they should be ErrorInfo objects to be consistent with other error code classes in the project. This should be fully implemented before merging.

Suggested change

export class FpnvErrorCode {

static INVALID_TOKEN = 'invalid_token';

static EXPIRED_TOKEN = 'expired_token';

static COMMON_ISSUE = {

code: "code",

message: "message"

};

// TODO: need to update codes properly

}

export class FpnvErrorCode {

static readonly INVALID_ARGUMENT: ErrorInfo = {

code: 'invalid-argument',

message: 'Invalid argument provided to the FPNV method.',

};

static readonly INVALID_TOKEN: ErrorInfo = {

code: 'invalid-token',

message: 'The provided phone number verification token is invalid.',

};

static readonly EXPIRED_TOKEN: ErrorInfo = {

code: 'expired-token',

message: 'The provided phone number verification token has expired.',

};

static readonly PROJECT_NOT_FOUND: ErrorInfo = {

code: 'project-not-found',

message: 'No Firebase project was found for the provided credential.',

};

}

gemini-code-assist · 2025-12-09T05:41:24Z

src/fpnv/token-verifier.ts

+  jwtName: 'Firebase Phone Verification token',
+  shortName: 'FPNV token',
+  typ: 'JWT',
+  expiredErrorCode: FpnvErrorCode.COMMON_ISSUE,


The expiredErrorCode is set to FpnvErrorCode.COMMON_ISSUE, which appears to be a placeholder. When a token expires, a more specific error code like expired-token should be used to provide better context to the developer. This change depends on the FpnvErrorCode class in src/utils/error.ts being properly implemented with an EXPIRED_TOKEN error info object.

Suggested change

expiredErrorCode: FpnvErrorCode.COMMON_ISSUE,

expiredErrorCode: FpnvErrorCode.EXPIRED_TOKEN,

gemini-code-assist · 2025-12-09T05:41:24Z

src/fpnv/token-verifier.ts

+                '"' + header.typ + '".' + verifyJwtTokenDocsMessage;
+    }
+    // FPNV Token
+    else if (!((payload.aud as string[]).some(item => item === this.issuer + projectId))) {


The aud (audience) claim in a JWT can be either a single string or an array of strings. This code assumes it's always an array by casting payload.aud as string[]. If the token has a single string audience, (payload.aud as string[]).some will throw a TypeError, causing an unhandled exception. You should handle both cases by normalizing payload.aud to an array before checking it.

Suggested change

else if (!((payload.aud as string[]).some(item => item === this.issuer + projectId))) {

else if (!((Array.isArray(payload.aud) ? payload.aud : [payload.aud]).some(item => item === this.issuer + projectId))) {

gemini-code-assist · 2025-12-09T05:41:24Z

src/fpnv/token-verifier.ts

+  constructor(
+    clientCertUrl: string,
+        private issuer: string,
+        private tokenInfo: FirebasePhoneNumberTokenInfo,
+        private readonly app: App
+  ) {


Throughout this file, FpnvErrorCode.COMMON_ISSUE is used for various types of errors, including invalid arguments, token validation failures, and internal errors. While this works, it provides less specific information to the developer. Consider using more specific error codes (e.g., invalid-argument, invalid-token, project-not-found) for different failure scenarios. This would improve the developer experience for error handling. This is related to the incomplete implementation of FpnvErrorCode in src/utils/error.ts.

lahirumaramba · 2025-12-09T16:06:37Z

src/fpnv/base-fpnv.ts

@@ -0,0 +1,23 @@
+import { App } from '../app';


Please add the license headers

lahirumaramba · 2025-12-09T16:08:57Z

src/fpnv/fpnv-namespace.ts

@@ -0,0 +1,39 @@
+/*!


Thanks for adding this file, but the namespaces are now deprecated. We don't support namespaces for new features. Please remove the namespaces support from this PR. (You can check src/data-connect for an example of a namespace-less feature).

lahirumaramba · 2025-12-09T16:09:14Z

src/fpnv/fpnv.ts

@@ -0,0 +1,41 @@
+/*!
+ * @license
+ * Copyright 2017 Google LLC


Suggested change

* Copyright 2017 Google LLC

* Copyright 2025 Google LLC

lahirumaramba · 2025-12-09T16:10:35Z

src/fpnv/base-fpnv.ts

+} from './token-verifier';
+
+
+export abstract class BaseFpnv {


You can probably move this class to src/fpnv/fpnv.ts

lahirumaramba · 2025-12-09T16:11:04Z

src/fpnv/index.ts

@@ -0,0 +1,74 @@
+/*!
+ * Copyright 2020 Google LLC


lahirumaramba · 2025-12-09T16:11:50Z

src/fpnv/index.ts

+ * ```javascript
+ * // Get the Fpnv service for a given app
+ * const otherFpnv = getFirebasePnv(otherApp);
+ * ```


Let's get a tech writer review on the new docstrings

lahirumaramba · 2025-12-09T16:15:58Z

src/fpnv/token-verifier.ts

+  PublicKeySignatureVerifier, ALGORITHM_ES256, SignatureVerifier,
+} from '../utils/jwt';
+
+export interface FpnvToken {


Please extract the public API surface (including any public types) to a new file named fpnv-api.ts. See src/data-connect/data-connect-api.ts for example. Add documentation for each new type.

lahirumaramba · 2025-12-09T16:16:51Z

src/fpnv/token-verifier.ts

@@ -0,0 +1,246 @@
+import { App } from '../app';


missing license header

lahirumaramba

Thanks! Added a few comments. Please check the CI errors.
Run npm run api-extractor:local to generate apidocs and update the PR with the new files.

lahirumaramba · 2025-12-09T16:22:43Z

src/utils/error.ts

+    code: "code",
+    message: "message"
+  };
+  // TODO: need to update codes properly


I understand that this is still WIP, as a note: please try not to commit any placeholder code. You can add them when you are ready and when necessary (instead of adding a placeholder for later).

feat: add basic fpnv

f603e65

gemini-code-assist bot reviewed Dec 9, 2025

View reviewed changes

lahirumaramba requested changes Dec 9, 2025

View reviewed changes

lahirumaramba reviewed Dec 9, 2025

View reviewed changes

-export class FpnvErrorCode {
-  static INVALID_TOKEN = 'invalid_token';
-  static EXPIRED_TOKEN = 'expired_token';
-  static COMMON_ISSUE = {
-    code: "code",
-    message: "message"
-  };
-  // TODO: need to update codes properly
-}
+export class FpnvErrorCode {
+  static readonly INVALID_ARGUMENT: ErrorInfo = {
+    code: 'invalid-argument',
+    message: 'Invalid argument provided to the FPNV method.',
+  };
+  static readonly INVALID_TOKEN: ErrorInfo = {
+    code: 'invalid-token',
+    message: 'The provided phone number verification token is invalid.',
+  };
+  static readonly EXPIRED_TOKEN: ErrorInfo = {
+    code: 'expired-token',
+    message: 'The provided phone number verification token has expired.',
+  };
+  static readonly PROJECT_NOT_FOUND: ErrorInfo = {
+    code: 'project-not-found',
+    message: 'No Firebase project was found for the provided credential.',
+  };
+}

	expiredErrorCode: FpnvErrorCode.COMMON_ISSUE,
	expiredErrorCode: FpnvErrorCode.EXPIRED_TOKEN,

	else if (!((payload.aud as string[]).some(item => item === this.issuer + projectId))) {
	else if (!((Array.isArray(payload.aud) ? payload.aud : [payload.aud]).some(item => item === this.issuer + projectId))) {

feat: add basic fpnv #3030

Are you sure you want to change the base?

feat: add basic fpnv #3030

Uh oh!

Conversation

boikoa-gl commented Dec 9, 2025

Discussion

Testing

API Changes

Uh oh!

gemini-code-assist bot commented Dec 9, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lahirumaramba left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants