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.

src/utils/error.ts

src/fpnv/token-verifier.ts

src/fpnv/base-fpnv.ts

src/fpnv/fpnv-api.ts

src/fpnv/fpnv.ts

src/fpnv/base-fpnv.ts

src/fpnv/index.ts

src/fpnv/token-verifier.ts

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.

src/utils/error.ts

src/fpnv/base-fpnv.ts

src/fpnv/fpnv.ts

src/fpnv/index.ts

src/fpnv/fpnv.ts

src/fpnv/token-verifier.ts

lahirumaramba

Thanks! It is looking good!
Added a few more comments

lahirumaramba · 2025-12-10T18:41:23Z

src/fpnv/fpnv.ts

+  private readonly app_: App;
+
+  protected readonly fpnvVerifier: FirebasePhoneNumberTokenVerifier;
+


Suggested change

/**

* @param app - The app for this `Fpnv` service.

* @constructor

* @internal

*/

lahirumaramba · 2025-12-10T18:45:31Z

src/fpnv/fpnv.ts

+ * Fpnv service bound to the provided app.
+ */
+export class Fpnv  {
+  private readonly app_: App;


nit: you can simplify this by using constructor parameter properties.

constructor(readonly app: App) { ... // this.app

lahirumaramba · 2025-12-10T18:46:59Z

src/fpnv/fpnv.ts

+  /**
+   * Returns the app associated with this Auth instance.
+   *
+   * @returns The app associated with this Auth instance.


Did you mean Fpnv instance here?

lahirumaramba · 2025-12-10T18:47:17Z

src/fpnv/fpnv.ts

+   *
+   * @returns The app associated with this Auth instance.
+   */
+  get app(): App {


Is this used for anything? If not, we can probably remove this getter

lahirumaramba · 2025-12-10T18:49:17Z

src/fpnv/fpnv.ts

+    return this.app_;
+  }
+
+  public async verifyToken(idToken: string): Promise<FpnvToken> {


Is this really an idToken?

lahirumaramba · 2025-12-10T20:26:28Z

src/fpnv/token-verifier.ts

+
+export function createFPNTVerifier(app: App): FirebasePhoneNumberTokenVerifier {
+  return new FirebasePhoneNumberTokenVerifier(
+    CLIENT_CERT_URL,


Do we expect these to change in the future? If not, I don't think we would need a factory here. FirebasePhoneNumberTokenVerifier already has access to these consts. If you have used the auth token verification code as an example for this, we use a factory method there because we support two different token types (custom token and session tokens)

lahirumaramba · 2025-12-10T20:29:18Z

src/fpnv/fpnv.ts

+  constructor(app: App) {
+
+    this.app_ = app;
+    this.fpnvVerifier = createFPNTVerifier(app);


Can this be simplified to a new FirebasePhoneNumberTokenVerifier(app)?

lahirumaramba · 2025-12-10T20:32:06Z

etc/firebase-admin.fpnv.api.md

+    // (undocumented)
+    iss: string;
+    // (undocumented)
+    sub: string;


Please add documentation to these types. For example see:

firebase-admin-node/src/app-check/app-check-api.ts

Line 87 in 179dab7

* The audience for which this token is intended.

lahirumaramba · 2025-12-10T20:50:42Z

src/fpnv/fpnv.ts

+    return this.app_;
+  }
+
+  public async verifyToken(idToken: string): Promise<FpnvToken> {


Suggested change

public async verifyToken(idToken: string): Promise<FpnvToken> {

public async verifyToken(fpnvJwt: string): Promise<FpnvToken> {

lahirumaramba · 2025-12-10T20:53:12Z

src/fpnv/fpnv.ts

+  get app(): App {
+    return this.app_;
+  }
+


Missing documentation here

feat: add basic fpnv

f603e65

gemini-code-assist bot reviewed Dec 9, 2025

View reviewed changes

src/utils/error.ts Show resolved Hide resolved

src/fpnv/token-verifier.ts Outdated Show resolved Hide resolved

src/fpnv/token-verifier.ts Outdated Show resolved Hide resolved

src/fpnv/token-verifier.ts Show resolved Hide resolved

lahirumaramba requested changes Dec 9, 2025

View reviewed changes

lahirumaramba reviewed Dec 9, 2025

View reviewed changes

src/utils/error.ts Outdated Show resolved Hide resolved

sam-gc reviewed Dec 10, 2025

View reviewed changes

chore: resolve comments

23868e8

boikoa-gl requested review from lahirumaramba and sam-gc December 10, 2025 14:28

boikoa-gl added 5 commits December 10, 2025 15:32

fix: remove unused interface

7f71eed

chore: update export

8413368

chore: linting

e90c2bd

chore: update export

2a759df

feat: add apidocs

fabcf80

lahirumaramba requested changes Dec 10, 2025

View reviewed changes

		private readonly app_: App;

		protected readonly fpnvVerifier: FirebasePhoneNumberTokenVerifier;

+  /**
+ * @param app - The app for this `Fpnv` service.
+ * @constructor
+ * @internal
+ */

	public async verifyToken(idToken: string): Promise<FpnvToken> {
	public async verifyToken(fpnvJwt: string): Promise<FpnvToken> {

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lahirumaramba left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lahirumaramba left a comment

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!

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!

3 participants