fix(core): allow valid international phone numbers and reject empty SMS codes#1390
fix(core): allow valid international phone numbers and reject empty SMS codes#1390jithinrajtk wants to merge 2 commits into
Conversation
…MS codes The phone number schema capped input at 10 digits, but that field holds the national number only (the dial code is prepended later by formatPhoneNumber), so valid 11-digit numbers such as China and Germany mobiles were rejected in the browser. Raise the cap to 15, the ITU-T E.164 maximum. The phone verification schema allowed an empty code to pass via a "!val ||" short-circuit, inconsistent with the TOTP verification schema. Drop the guard so an empty code fails.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![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 updates the phone authentication schemas to support up to 15-digit national phone numbers (aligning with the ITU-T E.164 maximum) and ensures that empty verification codes are rejected. Corresponding unit tests have been added and updated. The reviewer suggested using Zod's built-in .min() validator instead of a custom .refine() check for the verification code schema to make the code more idiomatic and consistent.
| verificationCode: z.string().refine((val) => val.length >= 6, { | ||
| error: getTranslation(ui, "errors", "invalidVerificationCode"), | ||
| }), |
There was a problem hiding this comment.
Using Zod's built-in .min() validator is more idiomatic, concise, and consistent with the rest of the schemas in this file (such as the password and display name validations) compared to a custom .refine() check.
verificationCode: z.string().min(6, getTranslation(ui, "errors", "invalidVerificationCode")),
|
@googlebot I signed it! |
Matches the password/display-name validations in this file and is more idiomatic than a custom refine check. No behavior change.
|
Good call — switched to |
1 participant
Two fixes to the phone-auth validation schemas in
packages/core/src/schemas.ts.1. Phone number validation rejects valid international numbers
createPhoneAuthNumberFormSchemacaps the phone number at.max(10). That field holds the national number only — the country dial code is prepended afterwards byformatPhoneNumber(country-data.ts) before the request reaches Firebase. National numbers commonly exceed 10 digits (China mobile is 11, Germany up to 11), so those users are blocked in the browser before any request is made.The library's own libphonenumber-based formatter accepts numbers the schema rejects:
createPhoneAuthNumberFormSchema({ phoneNumber: "13800138000" })nonetheless fails validation, purely because of the length cap.Fix: raise the bound to
.max(15), the ITU-T E.164 maximum digit count for a full number. The national number handled here is always shorter, so this keeps a sanity limit without rejecting valid input. The change also covers the MFA phone schemas, which reusecreatePhoneAuthNumberFormSchema.2. Empty SMS verification code passes validation
createPhoneAuthVerifyFormSchemavalidates the code withrefine((val) => !val || val.length >= 6, …). The!val ||short-circuits, soverificationCode: ""passes. This is inconsistent withcreateMultiFactorTotpAuthVerifyFormSchema, which validates the same field withrefine((val) => val.length === 6, …)and no empty short-circuit.Fix: drop the
!val ||guard so an empty code fails, matching the TOTP schema.Tests
packages/core/src/schemas.test.ts:pnpm --filter @firebase-oss/ui-core test:unitpasses (243 tests);pnpm --filter @firebase-oss/ui-core lintis clean.