firebase-android-sdk icon indicating copy to clipboard operation
firebase-android-sdk copied to clipboard

PhoneAuthProvider.OnVerificationStateChangedCallbacks are called in destroyed activity

Open aromano272 opened this issue 3 years ago • 4 comments

[REQUIRED] Step 2: Describe your environment

  • Android Studio version: 4.1.2
  • Firebase Component: com.google.firebase:firebase-auth-ktx:20.0.3
  • Component version: 20.0.3

[REQUIRED] Step 3: Describe the problem

Steps to reproduce:

  • Have "Don't keep activies" turned ON
  • Have the user see the Captcha while authenticating with phone

After starting the Phone Auth flow with: PhoneAuthProvider.verifyPhoneNumber(options) the user goes to the captcha(this will destroy the activity because of "Don't keep activities"), when the user comes back the OnVerificationStateChangedCallbacks .onCodeSent will be called in the destroyed activity.

Simplified code of my Auth activity:

class SmsValidationActivity : AppCompatActivity() {

    private val args: SmsValidationActivityArgs by navArgs()

    private var stateIsVerificationInProgress: Boolean = false
    private var stateHasCodeAlreadyBeenSent: Boolean = false
    private var stateVerificationId: String? = null
    private var stateForceResendingToken: PhoneAuthProvider.ForceResendingToken? = null

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.sms_validation_activity)

        if (savedInstanceState != null) onRestoreInstanceState(savedInstanceState)

        // When the user enter the screen the verification automatically starts
        verifyPhoneNumber(stateForceResendingToken)
    }

    private fun verifyPhoneNumber(resendingToken: PhoneAuthProvider.ForceResendingToken?) {
        stateIsVerificationInProgress = true
        if (!stateIsVerificationInProgress) stateHasCodeAlreadyBeenSent = false

        val options = PhoneAuthOptions.newBuilder()
            .setPhoneNumber(args.phone)
            .setTimeout(60L, TimeUnit.SECONDS)
            .setActivity(this)
            .setCallbacks(object : PhoneAuthProvider.OnVerificationStateChangedCallbacks() {
                override fun onVerificationCompleted(credential: PhoneAuthCredential) {
                    stateIsVerificationInProgress = false
                }

                override fun onVerificationFailed(ex: FirebaseException) {
                    stateIsVerificationInProgress = false
                }

                override fun onCodeSent(verificationId: String, forceResendingToken: PhoneAuthProvider.ForceResendingToken) {
                    stateVerificationId = verificationId
                    stateForceResendingToken = forceResendingToken
                    stateHasCodeAlreadyBeenSent = true
                }
            })
            .apply {
                if (resendingToken != null) setForceResendingToken(resendingToken)
            }
            .build()

        PhoneAuthProvider.verifyPhoneNumber(options)
    }

    override fun onSaveInstanceState(outState: Bundle) {
        super.onSaveInstanceState(outState)
        outState.putBoolean(STATE_IS_VERIFICATION_IN_PROGRESS, stateIsVerificationInProgress)
        outState.putBoolean(STATE_HAS_CODE_ALREADY_BEEN_SENT, stateHasCodeAlreadyBeenSent)
        outState.putString(STATE_VERIFICATION_ID, stateVerificationId)
        outState.putParcelable(STATE_FORCE_RESENDING_TOKEN, stateForceResendingToken)
    }

    override fun onRestoreInstanceState(savedInstanceState: Bundle) {
        stateIsVerificationInProgress = savedInstanceState.getBoolean(STATE_IS_VERIFICATION_IN_PROGRESS)
        stateHasCodeAlreadyBeenSent = savedInstanceState.getBoolean(STATE_HAS_CODE_ALREADY_BEEN_SENT)
        stateVerificationId = savedInstanceState.getString(STATE_VERIFICATION_ID)
        stateForceResendingToken = savedInstanceState.getParcelable(STATE_FORCE_RESENDING_TOKEN)
    }

    companion object {
        const val STATE_IS_VERIFICATION_IN_PROGRESS: String = "STATE_IS_VERIFICATION_IN_PROGRESS"
        const val STATE_HAS_CODE_ALREADY_BEEN_SENT: String = "STATE_HAS_CODE_ALREADY_BEEN_SENT"
        const val STATE_VERIFICATION_ID: String = "STATE_VERIFICATION_ID"
        const val STATE_FORCE_RESENDING_TOKEN: String = "STATE_FORCE_RESENDING_TOKEN"
    }

}

aromano272 avatar Mar 10 '21 19:03 aromano272

I found a few problems with this issue:

  • I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.
  • This issue does not seem to follow the issue template. Make sure you provide all the required information.

google-oss-bot avatar Mar 10 '21 19:03 google-oss-bot

I've tracked this internally as b/183427017.

yuchenshi avatar Mar 22 '21 20:03 yuchenshi

Hi, I've reproduced this on version 29.3.1.

Not only does firebase retain the first callback instance (and any references that callback holds), the second callback is never triggered.

So even if don't keep activities is turned off, there is unexpected behavior, as calling verifyPhoneNumber a second time will never give you a result with the new callback. So it seems the method is no longer safely reentrant, and savedInstanceState path is broken.

We can also trigger this behavior with don't keep activities turned off by rotating the phone at any point after verifyPhoneNumber is called but before the corresponding callback is invoked. For example if we rotate the phone while on the captcha screen, the user will come back to a new activity, firebase will trigger the old callback (which now does nothing), and the new call to verifyPhoneNumber will never give a result.

Any updates on the progress of this issue?

Appreciate your time and effort! Please let me know if there's any extra information I can provide on my end. 🙏

jeran avatar Apr 24 '22 16:04 jeran