IllegalArgumentException thrown by startActivityForLinkWithProvider when SMS multi-factor authentication enabled

SteveByrneOutput commented 1 month ago

Environment

Android Studio version: Ladybug | 2024.2.1
Firebase Component: Auth
Component version: firebaseBom = "33.1.2"

Issue

startActivityForLinkWithProvider throws an IllegalArgumentException if the firebaseUser has SMS multi-factor authentication enabled and configured.

If the firebaseUser does not have SMS multi-factor authentication enabled and configured the call will run as expected and trigger either addOnSuccessListener or addOnFailureListener depending on the outcome.

When SMS multi-factor authentication is enabled the callbacks are not triggered. The user can log in on chrome as expected with their Microsoft credentials and when the app is brought to the foreground again the exception is thrown. There are no other calls to Firebase auth at this time.

Prior to calling startActivityForLinkWithProvider(...) the user is successfully reauthenticated.

FirebaseAuth.getInstance().currentUser!!.reauthenticate(...) throws FirebaseAuthMultiFactorException as expected

multiFactorResolver.resolveSignIn(...) returns a success as expected.

Wrapping the block in a try/catch does not catch the exception.

Steps to reproduce:

Enable and configure SMS multi-factor authentication in Firebase
Enable and configure Microsoft authentication in Firebase
Enable MFA on Firebase user account
Attempt to link Microsoft account to Firebase User account

Relevant Code:

val activity = (LocalContext.current as ComponentActivity)

val provider = OAuthProvider.newBuilder("microsoft.com")
    .addCustomParameter("prompt", "consent").build()

FirebaseAuth.getInstance().currentUser!!.startActivityForLinkWithProvider(
        activity,
        provider
    ).addOnSuccessListener {
        Timber.i("Success")
    }.addOnFailureListener {
        Timber.e(it)
    }

Stacktrace:

FATAL EXCEPTION: Firebase Blocking Thread #5
            java.lang.IllegalArgumentException: Given String is empty or null
                    at com.google.android.gms.common.internal.Preconditions.checkNotEmpty(com.google.android.gms:play-services-basement@@18.3.0:2)
                    at com.google.android.gms.internal.firebase-auth-api.zzagc.<init>(com.google.firebase:firebase-auth@@23.1.0:5)
                    at com.google.android.gms.internal.firebase-auth-api.zzzk.zza(com.google.firebase:firebase-auth@@23.1.0:114)
                    at com.google.android.gms.internal.firebase-auth-api.zzzk.zza(com.google.firebase:firebase-auth@@23.1.0:20)
                    at com.google.android.gms.internal.firebase-auth-api.zzaah.zza(com.google.firebase:firebase-auth@@23.1.0:5)
                    at com.google.android.gms.internal.firebase-auth-api.zzaeo.zza(com.google.firebase:firebase-auth@@23.1.0:20)
                    at com.google.android.gms.internal.firebase-auth-api.zzaeo.zza(com.google.firebase:firebase-auth@@23.1.0:64)
                    at com.google.android.gms.internal.firebase-auth-api.zzadv.zza(com.google.firebase:firebase-auth@@23.1.0:168)
                    at com.google.android.gms.internal.firebase-auth-api.zzaae.zza(com.google.firebase:firebase-auth@@23.1.0:9)
                    at com.google.android.gms.internal.firebase-auth-api.zzzk.zza(com.google.firebase:firebase-auth@@23.1.0:88)
                    at com.google.android.gms.internal.firebase-auth-api.zzzk.zza(com.google.firebase:firebase-auth@@23.1.0:143)
                    at com.google.android.gms.internal.firebase-auth-api.zzadh.zza(com.google.firebase:firebase-auth@@23.1.0:106)
                    at com.google.android.gms.internal.firebase-auth-api.zzabu.zza(com.google.firebase:firebase-auth@@23.1.0:15)
                    at com.google.android.gms.internal.firebase-auth-api.zzaeh.run(com.google.firebase:firebase-auth@@23.1.0:2)
                    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
                    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:644)
                    at com.google.firebase.concurrent.CustomThreadFactory.lambda$newThread$0$com-google-firebase-concurrent-CustomThreadFactory(CustomThreadFactory.java:47)
                    at com.google.firebase.concurrent.CustomThreadFactory$$ExternalSyntheticLambda0.run(D8$$SyntheticClass:0)
                    at java.lang.Thread.run(Thread.java:1012)

Dependencies

firebaseBom = "33.1.2"

firebase-bom = { group = "com.google.firebase", name = "firebase-bom", version.ref = "firebaseBom" }
firebase-analytics = { group = "com.google.firebase", name = "firebase-analytics" }
firebase-config = { group = "com.google.firebase", name = "firebase-config" }
firebase-crashlytics = { group = "com.google.firebase", name = "firebase-crashlytics" }
firebase-firestore = { group = "com.google.firebase", name = "firebase-firestore" }
firebase-storage = { group = "com.google.firebase", name = "firebase-storage" }
firebase-appcheck-playintegrity = { group = "com.google.firebase", name = "firebase-appcheck-playintegrity" }
firebase-auth = { group = "com.google.firebase", name = "firebase-auth" }

google-oss-bot commented 1 month ago

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

lehcar09 commented 3 weeks ago

Hi @SteveByrneOutput, thank you for reaching put and reporting the issue. May I ask a few questions for us to narrow down the issue.

Does this issue only occurring on the specific devices? or different API versions?
Did your users experience this issue after updating a version of Firebase? Could you share us the stable version before experiencing this issue?

Also, could you try using the latest version on our Firebase (BOM 33.5.1) products to see if that resolves the issue? Thanks!

SteveByrneOutput commented 3 weeks ago

Hi @lehcar09 This is the first iteration of this feature so our users did not upgrade from a previous stable version.

The issue is reproducible across multiple APIs including 29 30 31 34

Upgrading to BOM 33.5.1 unfortunately doesn’t resolve the issue

lehcar09 commented 3 weeks ago

Hi @SteveByrneOutput, thank you for checking the issue in the latest version. By any chance, have you tried using another provider? If so, does the issue also occurs?

I'll raise this issue to our engineers and get back to you.

SteveByrneOutput commented 3 weeks ago

@lehcar09 Unfortunately we haven't tried any other providers. Using the Microsoft is a user request as it's part of our customers security requirements.

Thank you for raising the issue. Let me know if there is any other information I can provide

NhienLam commented 2 weeks ago

Hi @SteveByrneOutput, thanks for filing the issue. I wasn't able to reproduce it. I was able to link a user with SMS MFA to a Microsoft provider without issues. Here are the steps I followed:

-Sign in with Google provider -Enroll in SMS MFA -Link this user with a Microsoft account

[Java]

OAuthProvider provider = OAuthProvider.newBuilder("microsoft.com", firebaseAuth).build();
currentUser.startActivityForLinkWithProvider(currentActivity, provider)
                 .addOnSuccessListener(
                    new OnSuccessListener<AuthResult>() {
                      @Override
                      public void onSuccess(AuthResult authResult) {
                        ...
                      }
                    })
                .addOnFailureListener(
                    new OnFailureListener() {
                      @Override
                      public void onFailure(Exception e) {
                        ...
                      }
                    });

Please let me know if I miss anything. What is the provider of your current user (Google, email password)?

SteveByrneOutput commented 2 weeks ago

Hi @NhienLam thanks for looking into the issue.

My steps to repoduce are as follows:

User reauthenticates with EmailAuthProvider
This throws FirebaseAuthMultiFactorException
User reauthenticates with PhoneAuthProvider  
Call startActivityForLinkWithProvider on Firebase user
Sign in to Microsoft in browser. This navigates back to app 
Exception is thrown.

addOnFailureListener is not triggered. wrapping the call in a try/catch doesn't catch the exception either

The exception is only thrown when the user is enrolled with SMS MFA.   The Microsoft provider does actually link with the firebase user in the background. Once the app relaunches and the user reauthenticates again I can see the Microsoft in the user’s providerData There appears to be some background call being made by Firebase that is throwing this exception.

The only difference I can see between your code and mine is the added firebaseAuth param Unfortunately adding the param doesn’t resolve the issue.

Can you please provide some information on what might be causing the crash to happen. Is there a workaround for this? Can a Microsoft provider be linked in another way? Why doesn’t the call catch the exception in the OnFailureListener ?  Is there any way to debug the stack trace to find exactly where the call that is crashing is coming from?  Do you have sample code for the full solution?

SteveByrneOutput commented 2 weeks ago

Hi @NhienLam Any update on the inquires above?

NhienLam commented 2 weeks ago

Hi @SteveByrneOutput. I think I was able to reproduce the issue.

My stacktrace is slightly different from yours, but it seems to have the same root cause. When linking a user with MFA enabled to an OAuth provider, the server response does not contain an idToken, even though linking does not require second-factor authentication. However, the SDK still checks for the presence of an idToken, so IllegalArgumentException: Given String is empty or null is thrown. This error only occurs for MFA users because the server response does include an idToken when MFA is not enabled.

Currently, there's no workaround for this.

We will investigate a fix, but I cannot provide a timeline at this time. However, since the linking still succeeds, I hope this does not pose a significant blocker for you. Please correct me if I'm wrong. Does the exception thrown in the background cause any crashes or other issues?

SteveByrneOutput commented 2 weeks ago

@NhienLam

Thank you for investigating the issue.

Unfortunately this does pose as a blocker for us. Majority of our users have MFA enabled. Since the exception cannot be caught be wrapping the call in a try/catch the app will crash for these users. Furthermore if the exception could be caught we cannot guarantee a correct flow as the onSuccessListener is not called on a successful link.

We wont be able to deliver this feature until a fix is provided

firebase / firebase-android-sdk