firebase-admin-node icon indicating copy to clipboard operation
firebase-admin-node copied to clipboard

Can't create user with `phoneNumber: null`

Open gustavopch opened this issue 3 years ago • 7 comments

Describe your environment

  • Operating System version: macOS Monterey 12.0.1
  • Firebase SDK version: 9.10.0
  • Firebase Product: auth
  • Node.js version: 12.22.3
  • NPM version: 6.14.13

Describe the problem

I receive the following error when creating a user with phoneNumber: null:

FirebaseAuthError: The phone number must be a non-empty E.164 standard compliant identifier string.

But the function params say it should be allowed:

https://github.com/firebase/firebase-admin-node/blob/aea280d325c202fedc3890850d8c04f2f7e9cd54/src/auth/auth-config.ts#L160

Steps to reproduce:

Run the code below.

Relevant Code:

import firebase from 'firebase-admin'

await firebase.auth().createUser({
  displayName: 'John',
  email: '[email protected]',
  phoneNumber: null,
})

gustavopch avatar Nov 27 '21 14:11 gustavopch

Hey @bojeil-google, do you know if the BE accepts null here?

lahirumaramba avatar Dec 02 '21 22:12 lahirumaramba

I don't think it makes any sense to "create" a user with a null phone number. It's same as creating one with no phone number specified. In case of update it specifically means to remove the phone number from the account.

hiranya911 avatar Dec 02 '21 22:12 hiranya911

@hiranya911 My understanding is that null means I want that field to be empty. If the user previously existed and had a phone number, I'm erasing it; if it didn't have a phone number or didn't exist, I'm being explicit that the field will be kept empty.

  • Implicit: "Register a user with display name John and email [email protected]" (equivalent to omitting the property or passing undefined which is basically the same in JavaScript AFAIK)
  • Explicit: "Register a user with display name John and email [email protected], but leave the phone number empty" (equivalent to passing null)

IME, it's quite common to pass null when creating stuff in JavaScript. Anyway, if null won't be supported, the type declaration must be fixed because it currently allows that.

gustavopch avatar Dec 02 '21 22:12 gustavopch

+1 to @hiranya911, null is used for edit operations to delete a phone number. When creating a user with no phone number, you can choose not to pass any field or use undefined.

bojeil-google avatar Dec 02 '21 23:12 bojeil-google

If the user previously existed and had a phone number, I'm erasing it; if it didn't have a phone number or didn't exist, I'm being explicit that the field will be kept empty.

If the user already exists, you can't create it again. createUser() will throw in this case. Given that phone number is not set on new user accounts unless explicitly requested, {phoneNumber: null} is redundant on a createUser() call.

hiranya911 avatar Dec 02 '21 23:12 hiranya911

@hiranya911 Oh sure, I was talking about how I use to view null Vs. undefined vs. omission in a more general perspective. I don't think allowing null on creation would "not make sense". I think it's just an opinion. At least, as I said, in my experience, it's quite common to be allowed to use null when creating stuff.

gustavopch avatar Dec 03 '21 00:12 gustavopch

@hiranya911 For example, there's nothing wrong with:

await firebase
  .firestore()
  .collection('users')
  .add({
    displayName: 'John',
    email: '[email protected]',
    phoneNumber: null,
  })

In fact, that's sometimes even necessary if you need to filter on phoneNumber — if the property is just omitted, it won't be indexed.

If null makes sense when adding a user to Firestore, intuitively, it also makes sense for Firebase Auth. I understand those are two different services with different implementations, but I'm talking from the perspective of someone who's looking from the outside, consuming them.

gustavopch avatar Dec 03 '21 00:12 gustavopch

Closing due to inactivity.

lahirumaramba avatar Jan 17 '23 20:01 lahirumaramba