Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Crash on attempted signup with email prior associated with updated user #1644

Closed
bxcd opened this issue Jun 2, 2019 · 6 comments
Closed

Comments

@bxcd
Copy link

bxcd commented Jun 2, 2019

Welcome to FirebaseUI and thanks for submitting an issue!

Please take a look at open issues, as well as resolved issues, to see if your issue is either already being addressed, or has been solved by someone else.

If not, please feel free to fill in the following info so we can help faster!

Step 1: Are you in the right place?

  • For issues or feature requests related to the code in this repository file a GitHub issue.
  • For general technical questions, post a question on StackOverflow tagged appropriately.
  • For general Firebase discussion, use the firebase-talk google group
  • For help troubleshooting your application that does not fall under one of the above categories, reach out to the personalized Firebase support channel

Step 2: Describe your environment

  • Android device: Pixel 3 XL (virtual), Pixel (vitrual)
  • Android OS version: Q, 28
  • Google Play Services version: 49
  • Firebase/Play Services SDK version: 16.0.1/17.0.0
  • FirebaseUI version: 5.0

Relevant build dependencies:

Project-level

    classpath 'com.android.tools.build:gradle:3.4.1'
    classpath 'com.google.gms:google-services:4.2.0'

App-level

    implementation 'com.google.firebase:firebase-core:16.0.9'
    implementation 'com.google.firebase:firebase-database:17.0.0'
    implementation 'com.google.firebase:firebase-auth:17.0.0'
    implementation 'com.firebaseui:firebase-ui-auth:5.0.0'

Step 3: Describe the problem:

I reproduce this bug on attempting to sign up using the email address that was previously associated with a FirebaseUser before a successful call to updateEmail. The same response is generated at the FirebaseUI sign up screen after both a successful call to signOut with the FirebaseAuth instance associated with the updated FirebaseUser as well as deleting and re-installing the application.

Steps to reproduce:

  1. Successfully invoke updateEmail on a FirebaseUser
  2. Return to the FirebaseUI sign in screen. The following approaches were used:
    a. Successfully invoke signOut on the FirebaseAuth instance associated with the same FirebaseUser, or
    b. Delete and reinstall the application
  3. Attempt to sign up with the email address previously associated with, prior to successfully invoking updateEmail on, the same FirebaseUser

Observed Results:

The screen becomes blank before reloading the initial FirebaseUI sign in screen. onActivityResult is never invoked. Concurrently, the following logcat output is produced:

FATAL EXCEPTION: main
    Process: com.github.rjbx.givetrack, PID: 3076
    java.lang.IllegalStateException: User has no providers even though we got a collision.
        at com.firebase.ui.auth.viewmodel.email.EmailProviderResponseHandler$StartWelcomeBackFlow.onSuccess(EmailProviderResponseHandler.java:101)
        at com.firebase.ui.auth.viewmodel.email.EmailProviderResponseHandler$StartWelcomeBackFlow.onSuccess(EmailProviderResponseHandler.java:91)
        at com.google.android.gms.tasks.zzn.run(Unknown Source:4)
        at android.os.Handler.handleCallback(Handler.java:873)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:193)
        at android.app.ActivityThread.main(ActivityThread.java:6669)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)

Expected Results:

onActivityResult is invoked with a result code to indicate the status of the request.

Relevant Code:

List<AuthUI.IdpConfig> providers = new ArrayList<>();
                  providers.add(new AuthUI.IdpConfig.GoogleBuilder().build());
                  providers.add(new AuthUI.IdpConfig.EmailBuilder().build());
                  providers.add(new AuthUI.IdpConfig.AnonymousBuilder().build());
                  Intent signIn = AuthUI.getInstance().createSignInIntentBuilder()
                          .setLogo(R.drawable.logo)
                          .setTosAndPrivacyPolicyUrls("https://github.com/rjbx/giftab/terms.md", "https://github.com/rjbx/giftab/privacy.md")
                          .setTheme(R.style.AppTheme_AuthOverlay)
                          .setIsSmartLockEnabled(false, true)
                          .setAvailableProviders(providers)
                          .build();
                  startActivityForResult(signIn, REQUEST_SIGN_IN);
@samtstern
Copy link
Contributor

@rjbx thanks this is a really interesting edge case!

@lsirac @malcolmdeck when a user calls updateEmail what happens to their old email? Is it "reserved" somehow and not allowed for future signups? It looks like what happens here is that the old email shows up as a user with no linked providers.

@lsirac
Copy link
Contributor

lsirac commented Jun 4, 2019

Yeah, the old email is reserved and you can't use it. When changing from A to B, a revocation link is sent to A and A is reserved so that someone can't just create an account with it.

@malcolmdeck
Copy link
Collaborator

malcolmdeck commented Jun 4, 2019 via email

@bxcd
Copy link
Author

bxcd commented Jun 7, 2019

Thanks @samtstern and @lsirac for the feedback.

I understand reservation is necessary to preclude the old email address from subsequent registration, which would invalidate the existing revocation link. A possible feature addition might then entail including a confirmation link in the same email message in order to unreserve the old email address.

When registering a reserved email address, would any of the following make sense?

  1. During the registration flow, check the entered email address against those reserved. When matched, throw an InvalidCredentialsException as when entering an incorrect password.
  2. When handling the provider response, recognize reserved email addresses as providers with inactive states. Provide a result independent of the no-provider condition that specifies the underlying cause.
  3. Depending on the nature and certainty of other cases that meet the no-provider condition: Override the thrown exception. Provide a result if nullified dependencies required for the call are operable or can otherwise can be remediated.
       @Override
        public void onSuccess(String provider) {
            if (provider == null) {
                /* throw new IllegalStateException(
                       "User has no providers even though we got a collision."); */
                 setResult(...);
            } else
        ...
        }

@samtstern samtstern modified the milestones: 5.0.1, 6.0.1 Aug 30, 2019
@samtstern samtstern modified the milestones: 6.0.1, 6.0.2, 6.0.3 Sep 13, 2019
@samtstern samtstern modified the milestones: 6.1.0, 6.2.0 Nov 13, 2019
@samtstern samtstern modified the milestones: 6.2.1, 6.3.0 Mar 25, 2020
@samtstern
Copy link
Contributor

Looking at the Firebase Auth REST API there are two fields returned from fetchProviders:
https://firebase.google.com/docs/reference/rest/auth#section-fetch-providers-for-email

  • allProviders - list of strings
  • registered - boolean to see if the user is registered

However the Android SDK only provides the former, not the latter:
https://firebase.google.com/docs/reference/android/com/google/firebase/auth/SignInMethodQueryResult

That means when we fetch the providers we can't see registered which makes it really hard for us to understand this case. However I do think a crash is too harsh, so we can treat this as an unknown error and log a warning.

@bxcd
Copy link
Author

bxcd commented Jul 22, 2020

Thanks @samtstern for implementing a fix.

I repeated the process of successfully invoking updateEmail on a FirebaseUser, returning to the FirebaseUI sign in screen, and attempting to sign up with the email address previously associated with, prior to successfully invoking updateEmail on, the same FirebaseUser. This sign up attempt returns a resultCode of -1 from onActivityResult indicating that the operation was successful, as no exception is thrown from onSuccess indicating that providers is non-null.

This issue is being marked closed subject to no further findings. Please comment if your experience is different or if you identify another edge case.

@bxcd bxcd closed this as completed Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants