firebase / FirebaseUI-Android

Optimized UI components for Firebase
https://firebaseopensource.com/projects/firebase/firebaseui-android/
Apache License 2.0
4.63k stars 1.84k forks source link

Remove previous callbacks for wrong number entered. #1396

Open quantumInfection opened 6 years ago

quantumInfection commented 6 years ago

Step 1: Are you in the right place?

Yes.

Step 2: Describe your environment

Step 3: Describe the problem:

Steps to reproduce:

  1. Enter wrong number.
  2. Enter another wrong number again.
  3. Enter correct number.
  4. Wait for the Sms auto fill. First it shows 'Wrong code entered' and then logs in.

Observed Results:

It generates 3 different callbacks for onVerificationCompleted for the first two callbacks signInWithCredential fails due to wrong number and it goes to success for the last one.

Expected Results:

It should auto cancel all previous callbacks or there should a way to cancel previous callbacks.

SUPERCILEX commented 6 years ago

Can you try with FUI v4.1.0? We fixed tons of bugs in that area.

quantumInfection commented 6 years ago

I'll get back to you asap after trying FUI 4.1.0

quantumInfection commented 6 years ago

@SUPERCILEX Issue persists in FUI v4.1.0. Just tested.

ghost commented 6 years ago

@SUPERCILEX I think this is a Firebase Auth issue, not a Firebase UI issue.

abdulwasae commented 6 years ago

I have made my own Firebase Authentication UI and I also face this issue. The OnVerificationStateChangedCallbacks get called for older, incorrect verification attempts, upon a correct attempt eventually.

I suspect the problem is with FirebaseAuth itself. Can others please confirm?

samtstern commented 6 years ago

Trying to understand exactly what's going on here: are you saying that you get a bunch of OnVerificationStateChangedCallbacks all at once after the end of a series of requests?

ghost commented 6 years ago

Hi @samtstern, think I can answer this (pls correct me if I misrepresented the problem, @abdulwasae).

Here is the sequence of steps:

  1. A user enters their phone number.
  2. The app then calls verifyPhoneNumber on an instance of the Firebase PhoneAuthProvider.
  3. The user realizes they entered the wrong phone number. So the user goes back and enters their phone number again.
  4. The app calls verifyPhoneNumber again, with the new phone number. There is no way to cancel the old verifyPhoneNumber request
  5. User receives an SMS code, which is automatically read by Firebase.
  6. THE PROBLEM: The first verifyPhoneNumber request triggers the onVerificationFailed callback, for the first (incorrect) number entered.
  7. The second verifyPhoneNumber request triggers the onVerificationCompleted callback.

Steps 6 and 7 are where we see the problem. Ideally, Firebase PhoneAuth does not try to use the same code to verify two different numbers. Instead, Firebase should allow us to cancel the first verification request. Even better, Firebase should cancel the first verification request as soon as the app tries to verify a second number, so that at any given time only one verification callback is triggered.

I hope this clears up the issue.

samtstern commented 6 years ago

@ismail-a-khan thanks for the very detailed steps! This sounds like a bug I should file with the Firebase Auth folks. Because this is not a FirebaseUI issue I will close it here, but I will come back to this thread and update once I hear back from the Firebase Auth engineering team.

ghost commented 6 years ago

@samtstern, I think you might want to consider patching this in Firebase UI as well. You can work around the Firebase auth issue by doing the following:

  1. Keep track of the 'current' phone number to verify, perhaps in an instance variable.
  2. Immediately before you call verifyPhoneNumber (in the same method where you call it), save the 'current' phone number in a final local variable. Each callback can now reference a snapshot of the phone number that is being verified.
  3. In the onVerificationCompleted method of the OnVerificationStateChangedCallbacks object that is passed to verifyPhoneNumber, check if the actual 'current' phone number to verify is equal to the phone number that was captured/enclosed by the callback object -- if the number's don't match, stop execution. Do the same in onVerificationFailed as well.

This should solve the problem, because now each OnVerificationStateChangedCallbacks is aware of which number it is trying to verify. By comparing the callback object's own number to the actual number that we want to verify, we can ignore all the callbacks that are associated with old numbers.

It's a bit of a hack, sure, but will at least solve the problem until the Firebase Auth team fixes the issue (which may take ages, given my experience with Cloud Firestore bugs...). If my explanation is not clear I could go ahead and submit a PR of the fix.


Btw, I was slightly incorrect in my steps-to-recreate above. In step #6, it is actually onVerificationCompleted that is called, not onVerificationFailed, such that onVerificationCompleted is called twice overall. The fix is the same either way.

samtstern commented 6 years ago

Thanks @ismail-a-khan you are right, we could fix this here as well.

Puneet1796 commented 3 years ago

@samtstern Any update on the bug from Firebase Auth?

CarlosMesquita-Gorillas commented 2 years ago

@samtstern Having the same issue with those callbacks, any progress? Any updates?