search

hippware / rn-chat

MIT License
5 stars 0 forks source link

Unable to login to the app #3750

Closed aksonov closed 4 years ago

aksonov commented 5 years ago

Error in tinyrobot

verify_confirmation_fail in node_modules/react-native/Libraries/BatchedBridge/NativeModules.js:155 The SMS code has expired. Please re-send the verification code to try again.

View on Bugsnag

Stacktrace

node_modules/react-native/Libraries/BatchedBridge/NativeModules.js:155 - createErrorFromErrorData

View full stacktrace

Created by Miranda via Bugsnag

mstidham commented 5 years ago

Repeatedly receiving "Error confirming code please try again or resend code" It appears after receiving the sms code the app is trying to auto-register and the button says Connecting but never goes away. User hasn't added the sms code. If user adds the sms code the button says Verify but that doesn't let the user in either. We were able to login using someone else's Firebase number but still can't login using our actual Firebase number.

bengtan commented 5 years ago

This error 'The SMS code has expired. Please re-send the verification code to try again.' has happened before. See #3662.

It doesn't happen very often and so we haven't been able to observe it enough.

Since it happened again recently, I'll look into it.

Also, Slack conversation when it happened recently:

https://hippware.slack.com/archives/C2V6L53TQ/p1558446147017700 (and the subsequent 20-30+ messages)

bengtan commented 5 years ago

Tried to reproduce it. Unsuccessful.

Did some reading and research.

Grepping the iOS Firebase cocoapod (native code) seems to indicate it's some sort of internal (ie. internal to firebase) error.

Searching google, I found two clusters of results.

1. The error can occur if auto-verify isn't implemented properly. But this only applies to Android, and we have implemented auto-verify. So this isn't it.

2. I've seen three separate instances of people suggesting the following as a work-around:

Don't call confirm(). Get the intermediate AuthCredential object and call signInWithCredential() instead.

References:

https://github.com/invertase/react-native-firebase/issues/734#issuecomment-471282064 https://stackoverflow.com/a/46614222 https://stackoverflow.com/a/55074936

bengtan commented 5 years ago

I've published a speculative fix via codepush.

It's codepush StagingBeng-v62 (Description: '3724, 3717, 3737, 3300, (speculative) 3750 (commit f7d8587a)' ).

To use this:

I have no idea if it will work as I haven't been able to reproduce the fault. Fingers crossed.

Off-topic: The codepush can also be used to 'codepush-verify' the ticket numbers in the description.

@mstidham, @thescurry

thescurry commented 5 years ago

I just tried, it unfortunately did not work.

bengtan commented 5 years ago

I submitted the text below (phone numbers redacted) to firebase support via https://support.google.com/firebase/contact/support?page=

Hi,

We're encountering a peculiar error and would like some assistance and information.

We have two beta users who are unable to login via firebase phone/sms auth. It seems to affect only these two beta users (or rather, their phone numbers). Everyone else is fine.

Some version numbers:

Our firebase project is: my-project-1480497595993

The error code our app is receiving from the firebase library is auth/code-expired or ERROR_SESSION_EXPIRED and the error message is 'The SMS code has expired. Please re-send the verification code to try again.'

The sms is received okay. It's just the confirmation part which throws up the error.

And yes, the sms code was entered very promptly ie. within a minute.

AFAIK, it only affects the following two numbers: +1408xxxxxxx and +1580xxxxxxx and seems to be specific only to them.

For example, if we try another number on the beta user's phone, login via sms code works fine. Conversely, if we try the number +1408xxxxxxx on multiple phones, it still throws up the error.

So it seems there is some sort of setting or (mis-?)configuration for those two numbers.

Nominal steps to reproduce: Try to log in with phone number +1408xxxxxxx or +1580xxxxxxx

(Also, I have searched the Internet and I'm aware that the error message 'The SMS code has expired. Please re-send the verification code to try again.' also occurs on Android if auto-verify isn't correctly implemented, but we are experiencing this on iOS where auto-verify is not applicable.)

Are you able to offer any tips or insights please?

(Please cc replies to eric@xxxxxxxx.com and scurry@xxxxxxxx.com)

Thanks, Beng Tan

bengtan commented 5 years ago

Random assortment of thoughts:

I'm still unable to reproduce.

Testing indicates the problem is specific to the two phone numbers (belonging to scurry and miranda each). scurry tried his number on a different phone and also encountered the issue.

Actually, this suggests that any of us can reproduce by attempting to log in with scurry's phone number (and have scurry nearby to pass the sms code across). If we want a dev to look at this ... timezone-wise, it'd be more convenient for @southerneer to reproduce it.

I'm not entirely sure how useful it would be ... if a dev did reproduce it. Shrugs. Dunno.

I posted a support request to firebase. See immediately preceding comment.

I have a suggestion which might be able to work around this glitch (if it is a glitch). I don't know if it will work though, and it might cause unexpected consequences. Buyer beware.

The idea is: Locate the relevant phone number/account at

https://console.firebase.google.com/u/0/project/my-project-1480497595993/authentication/users

and delete it.

This would delete the phone number from firebase's records but doesn't delete the account from our servers. Then, upon sign-in, a new firebase account is created but our server-side code is clever enough to associate the firebase account with the existing/old wocky account.

I tested this with one of my accounts (@bengtanmsia) and it seemed fine with no loss of data.

So ... if this issue is becoming incredibly inconvenient, we could try deleting the firebase account for the affected number(s). And then hope.

southerneer commented 5 years ago

The Firebase console at least thinks that both of those accounts have signed in today:

Steve... image

Miranda... image

bengtan commented 5 years ago

Got this back from firebase support. The date/time below is in timezone UTC+8.

---------- Forwarded message --------- From: firebase-help@google.com Date: Wed, 22 May 2019 at 22:02 Subject: RE: [9-3263000026418] The SMS code has expired. Please re-send the verification code to try again. To:

Hello Beng,

This is Bea from Firebase Support. I'll be happy to help you with your issue on Firebase Authentication.

This is unusual for iOS Phone Auth. Just to give context, this issue is mostly encountered by Android devices because the device being tested or used has an instant verification mechanism that instantly verifies the OTP without letting the user input it manually. Instant verification happens any time we have seen the number / Android device combination before (assuming all other security checks pass); therefore, some numbers experience this and some do not. If this happens, the OTP will be expired at once (as soon as the SMS Authentication pushes through and gets a successful verification). You may also refer on our documentation here.

With this, could you elaborate more on "The sms is received okay. It's just the confirmation part which throws up the error." by providing us the following details:

A minimal, complete and verifiable example that I can run locally Exact steps to replicate this issue (screenshot of each step and the result would be great) Any other relevant information

Regards, Bea

bengtan commented 5 years ago

Discussion ...

So ... I received the email reply from firebase support about 1.5 hours after I posted the question.

Then, about 2-4 hours afterwards, it was reported on Slack that both phone numbers no longer experience this problem. It seems to have automagically fixed itself.

https://hippware.slack.com/archives/C2V6L53TQ/p1558540668067000

Judging from the email response, I think this is some sort of rare-ish corner case in firebase that isn't handled properly since ... Apparently this error message only applies to Android but we're seeing it on iOS.

(I wonder if we might have confused firebase by sometimes logging in on Android (ie. for testing) with the same phone numbers?)

OTOH, it's possible that this is just a scripted response from firebase's level 1 technical support and ... for all we know, they might have poked those two numbers at the firebase back-end without letting anyone know.

So ... what to do now? I'm not sure that there's much we can do because the situation has been perturbed/altered and it's no longer happening/reproducible.

So I guess we monitor the situation and continue debug if it happens again. If.

bengtan commented 5 years ago

This happened again to @mstidham recently but it 'fixed itself' shortly afterwards.

Surprising thing is ... apparently instant verification works on iOS even though firebase says it's an android only thing.

See https://hippware.slack.com/archives/C2V6L53TQ/p1559048791026500 (with screenshot)

I’m back in and also confirming that the instant-verification is happening on ios. I just did clean install and logged in and was taken to the “Allow Location Access” screen before I entered/received the sms code.

southerneer commented 5 years ago

apparently instant verification works on iOS

Maybe...I would also look into whether Firebase data is cached between installs. If Miranda still had a valid Firebase token this might explain the behavior that's very similar to instant verification on Android. All speculation at this point, I'm not even sure how to go about testing for that kind of thing.

thescurry commented 5 years ago

@bengtan any thoughts on Eric's last comment?

bengtan commented 5 years ago

We have one or two anecdotal observations in the past that Firebase does cache some data, but somehow it manages to persist across uninstall/reinstalls. So for example, if you uninstall without logging out (of a firebase phone number) first, then it 'auto-logins' after you reinstall, or something like that. I say 'auto-logins' because I forget the exact steps that are skipped.

So I think Firebase is caching something, but I think it's at the server side. Or else, they're remember the device-ID/phone-number pair and whitelisting it, or something like that.

bengtan commented 5 years ago

See for example: #3154 and the comment https://github.com/hippware/rn-chat/issues/3154#issuecomment-456384739:

https://stackoverflow.com/questions/27893418/firebase-deleting-and-reinstalling-app-does-not-un-authenticate-a-user/27894791 may help

bengtan commented 5 years ago

Oh, this is interesting:

The Firebase authentication session is persisted on the user's device in the iOS keychain. The keychain data for the application is not removed when the application is uninstalled. ...

https://stackoverflow.com/a/27894791

bengtan commented 5 years ago

Crap, this has started happening again, on Prod 4.16.0. See https://app.bugsnag.com/hippware/tinyrobot-1/errors/5cdb449fdef87a0019dc1239

bengtan commented 5 years ago

Sent an email to firebase support to re-open their previous suppot ticket.

Hi,

Re-opening this ticket please.

Unfortunately, this error message has started appearing again but it's appearing for our LIVE users.

We haven't been able to reproduce it ourselves, but we know it's happening in the wild because we see it in our analytics and bug reporting tools.

We recently released a new iOS version of our app to the App Store, and it has happened about 5-10 times in the last 24 hours or so.

This is really bad. It's affecting our new users and preventing them from signing up to our app.

Please note that this is happening on iOS.

You previously said that the error message 'The SMS code has expired. Please re-send the verification code to try again.' only occurs on Android. This is wrong. We're seeing it on iOS.

We haven't even released an android version of our app, so this is happening to people who only have iOS.

Please ask your engineers under what conditions this error message can occur on iOS.

And please don't ask me to supply you with a 'A minimal, complete and verifiable example that I can run locally' because it won't work. The error is specific to individual phone numbers. It's not reproducible unless you are lucky enough to have a phone number which is affected by it.

Get your engineers to check your logs for this Firebase account. If you say this error message only applies to Android, but we're seeing it on iOS, then the onus is on you to explain under what conditions it can appear on iOS.

aksonov commented 5 years ago

@bengtan During today testing I've got very weird behaviour with prod app for registration of totally new phone number (+79255702254), but old iphone device - after entering sms code I have been logged under different phone number - +16505550001 (test0001), do we have bypass users on prod?

aksonov commented 5 years ago

@southerneer Maybe we should clear firebase user/token when user press "Get Started" button?

bengtan commented 5 years ago

Reply from firebase support (7 hours before time of posting):

Apologies for the inconveniences this issue have caused on your end. We find this unusual as the issue occurs in iOS and not in Android. With this, I have escalated it to our engineers, and I'll let you know once we have our findings as soon as possible.

bengtan commented 5 years ago

During today testing I've got very weird behaviour with prod app for registration of totally new phone number (+79255702254), but old iphone device - after entering sms code I have been logged under different phone number - +16505550001 (test0001), do we have bypass users on prod?

This has been ticketed as: Firebase testing number was previously logged account #3873

It's a (recently new?) known behaviour.

Maybe we should clear firebase user/token when user press "Get Started" button?

Yes, I think we should do something for #3873. I was gonna wait for Miranda to test whether it also occurs on Android before deciding on how exactly to fix it.

So the behaviour of #3873 might be related to this ticket, or it might not. Mystery.

(cc @aksonov @southerneer)

bengtan commented 5 years ago

A fix has been merged for a possibly-related ticket 3873.

It's not known whether the fix for 3873 might also work-around this. Maybe. Maybe not.

bengtan commented 5 years ago

The instance of this problem which started happening on Prod recently has stopped happening. The last instance was about 30 hours ago (according to mixpanel).

I think the 'fix' for this (the next time it happens) is ... send an email rant to firebase support.

bengtan commented 5 years ago

Hasn't happened since the previous comment (ie. no new reports in bugsnag).

bengtan commented 5 years ago

Crap. It happened for one user on Prod 4.15.1 about a minute or two ago.

(courtesy of bugsnag)

aksonov commented 5 years ago

An error linked to this issue has been reopened in Bugsnag verify_confirmation_fail in node_modules/react-native/Libraries/BatchedBridge/NativeModules.js:155

bengtan commented 5 years ago

Some assorted notes:

We can get email notifications (ie. an email alarm) for each occurrence of this issue by snoozing the bugsnag event 'until this error occurs 1 more time'.

I did some very speculative experimenting and managed to produce the same error message (and hence, also un-snoozed the bugsnag item) on Staging 4.16.0 after loading a codepush with the following change:

diff --git a/src/store/FirebaseStore.ts b/src/store/FirebaseStore.ts
index 9598872..13985b8 100644
--- a/src/store/FirebaseStore.ts
+++ b/src/store/FirebaseStore.ts
@@ -217,6 +217,10 @@ const FirebaseStore = types
         }
         analytics.track('verify_confirmation_try', {code, resource})
         yield confirmResult.confirm(code)
+
+        console.log('BT confirmResult.confirm() 2')
+
+        yield confirmResult.confirm(code)
         register()
         analytics.track('verify_confirmation_success')
       } catch (err) {

I dunno if it's the same cause, but it's the same error message.

With the codepush, I'm (deliberately running incorrect code) and calling confirmResult.confirm(code) twice.

When I run it, I get an error message 'Error confirming code, please try again or resend code' on the screen but the app successfully logins a few seconds later anyway. If I then look in bugsnag, I see that the bugsnag item has a new occurrence.

This observation supports the idea that the error message arises when either the sms code has already been used (and was successful) or the user was already authenticated (according to firebase), or something like that.

But it's all just speculation at this point.

mstidham commented 5 years ago

I received this again after downloading Version: 4.18.0 on Android Prod. The phone number (580-334-7168) should not be associated with any account. Verified with Phil that he doesn't see it on Prod.

Screenshot_20190718-154801_tinyrobot

image

bengtan commented 5 years ago

I received this again after downloading Version: 4.18.0 on Android Prod. The phone number (580-334-7168) should not be associated with any account. Verified with Phil that he doesn't see it on Prod.

Firstly, the dreaded 'The SMS code has expired. Please re-send the verification code to try again.' bugsnag hasn't occurred for 2-ish days so it wasn't triggered by this attempt to log in via (580-334-7168). So it's a separate thing from this ticket.

Secondly, I don't understand why this is being reported? Is it being reported as a bug? Or as a test of some sort?

According to firebase, this number registered with firebase on 3 April 2019 and the last firebase login was 19 April 2019. However, there are no user accounts on Staging nor Prod associated with this phone number. It looks like a legitimate error. Maybe the phone number doesn't exist anymore.

bengtan commented 5 years ago

I received this again after downloading Version: 4.18.0 on Android Prod. The phone number (580-334-7168) should not be associated with any account. Verified with Phil that he doesn't see it on Prod.

From Slack conversations, this is a false alarm and has been resolved.

aksonov commented 5 years ago

An error linked to this issue has been reopened in Bugsnag verify_confirmation_fail in node_modules/react-native/Libraries/BatchedBridge/NativeModules.js:155

aksonov commented 5 years ago

An error linked to this issue has been reopened in Bugsnag verify_confirmation_fail in MainActivity

aksonov commented 5 years ago

An error linked to this issue has been reopened in Bugsnag verify_confirmation_fail in node_modules/react-native/Libraries/BatchedBridge/NativeModules.js:155

bengtan commented 5 years ago

This happened again a few times overnight when QA was trying to test 4.19.0.

From mixpanel logging that I put in previously, I can see in mixpanel that firebase_auth_change_user happens just before verify_confirmation_fail.

It seems that the error message (in bugsnag) 'The SMS code has expired. Please re-send the verification code to try again.' happens when:

Later, when the app tries to verify the sms code with firebase, firebase throws this error because the user is already logged in.

I was able to artificially reproduce this:

The account should be automatically logged out. Then:

This will fail because 4.19.0 has bug #3963 which cause the app's login to fail (even though firebase login was successful beforehand). This induces the conditions mentioned (in this comment) above.

However, upgrading to 4.19.1 fixes this. 4.19.1 has a fix for #3963.

I did some more digging around.

The following is speculation (and notes for my own future reference).

I think the fix for #3873 might actually fix this.

This ticket occurs due to some sort of auto-verify feature for iOS (even though firebase support insists auto-verify only applies to android). When I artificially reproduce this via the '4.19.0' method above, and if I experimentally undo #3873, I observe auto-verify happening (on iOS). I speculate that, with auto-verify, sometimes there is a race condition which causes this issue to happen.

However, the change for #3873 prevents auto-verify happening (by forcefully doing a firebase logout at the beginning of onboarding). So hence, if auto-verify can't happen anymore, then (hopefully) this ticket won't happen anymore either.

Need to continue monitoring.

aksonov commented 5 years ago

An error linked to this issue has been reopened in Bugsnag verify_confirmation_fail in node_modules/react-native/Libraries/BatchedBridge/NativeModules.js:155

bengtan commented 5 years ago

This happened twice recently, for a prospective user in Mexico, and another in Germany. Production 4.18.0

Mexico

https://app.bugsnag.com/hippware/tinyrobot-1/errors/5cdb449fdef87a0019dc1239?filters%5Bevent.since%5D%5B%5D=30d&filters%5Berror.status%5D%5B%5D=open&event_id=5d44973e004c28e634820000

https://mixpanel.com/report/1119030/explore#user?distinct_id=F491ADC8-FED9-43BB-84FA-D81E767D2477

Screenshot from 2019-08-06 14-02-05

Germany

https://app.bugsnag.com/hippware/tinyrobot-1/errors/5cdb449fdef87a0019dc1239?filters%5Bevent.since%5D%5B%5D=30d&filters%5Berror.status%5D%5B%5D=open&event_id=5d4605e0004c14a086000000

https://mixpanel.com/report/1119030/explore#user?distinct_id=28A94528-AAA4-4F08-B4F4-0AE32475CB8E

Screenshot from 2019-08-06 14-02-46

bengtan commented 5 years ago

It's been about a month since this last happened (aside from a couple of false alarms).

bengtan commented 5 years ago

It's been about a month since this last happened (aside from a couple of false alarms).

Right after I post this, it happens a few hours later. However, I think it's a false alarm.

Here's the mixpanel log of the user:

https://mixpanel.com/report/1119030/explore#user?distinct_id=3216EF8A-7B4B-447D-AC63-5A6CC31D7DCF

However, according to the database, this user account logged in a second afterwards (at 2019-09-03 18:35:28) and also made some changes later (updated_at is 2019-09-03 18:35:49).

I'm calling this a false alarm.

bengtan commented 5 years ago

Another false alarm at Sep 7th, 11:41:24 UTC

bengtan commented 5 years ago

A PR was merged to stop the false alarms from occurring. Will continue to monitor.

bengtan commented 4 years ago

I made a change some time ago (PR #4386, 4.32.1) so that the dreaded 'The SMS code has expired. ...' errors gets posted to a different bugsnag message. Once that gets deployed to the App Stores, all further instances of this bugsnag should be false alarms.

Hence, preemptively closing.