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

fix(auth, android): fixed issue with apple sign-in on android #4487

Merged
merged 1 commit into from
Nov 10, 2020

Conversation

nealmanaktola
Copy link
Contributor

@nealmanaktola nealmanaktola commented Nov 2, 2020

Description

This fixes an issue with Apple sign on Android. iOS was fixed with this commit. iOS was fixed with this PR #4359. Note the solution is different. On iOS, it looks like we bubble the auth credentials back to js land and then retry login. Here, we attempt another login on auth collision errors

Also, note this deviates from the iOS solution. I am unable to grab the actual credentials from the AuthCredential object and return an error. So, instead, I proceed with a login

Related issues

Fixes #3952 on Android

Release Summary

Call signInWithCredential with updated credential when linkWithCredential throws FirebaseAuthUserCollisionException

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan


Think react-native-firebase is great? Please consider supporting the project with any of the below:

This fixes an issue with Apple sign on Android. iOS was fixed with this commit. iOS was fixed with this PR invertase#4359. Note the solution is different. On iOS, it looks like we bubble the auth credentials back to js land and then retry login. Here, we attempt another login on auth collision errors
@vercel
Copy link

vercel bot commented Nov 2, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/53z36wxpa
✅ Preview: https://react-native-firebase-git-fix-android-apple-auth.invertase.vercel.app

@CLAassistant
Copy link

CLAassistant commented Nov 2, 2020

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Nov 2, 2020

Codecov Report

Merging #4487 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4487   +/-   ##
=======================================
  Coverage   41.92%   41.92%           
=======================================
  Files          35       35           
  Lines        1119     1119           
  Branches      278      278           
=======================================
  Hits          469      469           
  Misses        489      489           
  Partials      161      161           

@nealmanaktola nealmanaktola changed the title bug(auth): fixed issue with apple sign-in on android fix(auth): fixed issue with apple sign-in on android Nov 2, 2020
@nealmanaktola nealmanaktola changed the title fix(auth): fixed issue with apple sign-in on android fix(auth, android): fixed issue with apple sign-in on android Nov 2, 2020
@mikehardy mikehardy added the Workflow: Needs Review Pending feedback or review from a maintainer. label Nov 10, 2020
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! Thank you for your patience, merging this and will release it in an hour or so

@mikehardy mikehardy removed the Workflow: Needs Review Pending feedback or review from a maintainer. label Nov 10, 2020
@mikehardy mikehardy merged commit 6a8f8ad into invertase:master Nov 10, 2020
@mikehardy
Copy link
Collaborator

I'm working on the auth emulator now in #4552 , which lets me re-enable the auth e2e tests, and it pointed out an error here.
In the non-Apple cause it fails and causes a NullPointerException, crashing the app now:

11-15 16:37:44.494 31029 31319 E Auth    : com.google.firebase.auth.FirebaseAuthUserCollisionException: The email address is already in use by another account.
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zzem.zza(com.google.firebase:firebase-auth@@20.0.0:5)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zzfx.zza(com.google.firebase:firebase-auth@@20.0.0:21)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zzfq.zza(com.google.firebase:firebase-auth@@20.0.0:35)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zzfs.zza(com.google.firebase:firebase-auth@@20.0.0:74)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zzel.zza(com.google.firebase:firebase-auth@@20.0.0:61)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zzp.zza(com.google.firebase:firebase-auth@@20.0.0:3)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zzi.zza(com.google.firebase:firebase-auth@@20.0.0:2)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zzgb.zza(com.google.firebase:firebase-auth@@20.0.0:29)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zzfn.zza(com.google.firebase:firebase-auth@@20.0.0:83)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zza.zza(com.google.firebase:firebase-auth@@20.0.0:92)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zza.zza(com.google.firebase:firebase-auth@@20.0.0:241)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zzf.zza(com.google.firebase:firebase-auth@@20.0.0:8)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zzgb.zza(com.google.firebase:firebase-auth@@20.0.0:34)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zzfn.zza(com.google.firebase:firebase-auth@@20.0.0:77)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zza.zza(com.google.firebase:firebase-auth@@20.0.0:85)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zza.zza(com.google.firebase:firebase-auth@@20.0.0:239)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zzp.zza(com.google.firebase:firebase-auth@@20.0.0:11)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zza.zza(com.google.firebase:firebase-auth@@20.0.0:50)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zza.zzc(com.google.firebase:firebase-auth@@20.0.0:156)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zzep.zza(com.google.firebase:firebase-auth@@20.0.0:156)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zzbt.zza(com.google.firebase:firebase-auth@@20.0.0:29)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.firebase.auth.api.internal.zzbs.accept(Unknown Source:6)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.android.gms.common.api.internal.zach.doExecute(com.google.android.gms:play-services-base@@17.3.0:2)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.android.gms.common.api.internal.zah.zaa(com.google.android.gms:play-services-base@@17.3.0:9)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.android.gms.common.api.internal.GoogleApiManager$zaa.zac(com.google.android.gms:play-services-base@@17.3.0:192)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.android.gms.common.api.internal.GoogleApiManager$zaa.zab(com.google.android.gms:play-services-base@@17.3.0:157)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.android.gms.common.api.internal.GoogleApiManager$zaa.zaa(com.google.android.gms:play-services-base@@17.3.0:125)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.android.gms.common.api.internal.GoogleApiManager.handleMessage(com.google.android.gms:play-services-base@@17.3.0:144)
11-15 16:37:44.494 31029 31319 E Auth    : 	at android.os.Handler.dispatchMessage(Handler.java:102)
11-15 16:37:44.494 31029 31319 E Auth    : 	at com.google.android.gms.internal.base.zap.dispatchMessage(com.google.android.gms:play-services-base@@17.3.0:8)
11-15 16:37:44.494 31029 31319 E Auth    : 	at android.os.Looper.loop(Looper.java:193)
11-15 16:37:44.494 31029 31319 E Auth    : 	at android.os.HandlerThread.run(HandlerThread.java:65)
11-15 16:37:44.496 31029 31319 E AndroidRuntime: FATAL EXCEPTION: pool-53-thread-1
11-15 16:37:44.496 31029 31319 E AndroidRuntime: Process: com.invertase.testing, PID: 31029
11-15 16:37:44.496 31029 31319 E AndroidRuntime: java.lang.NullPointerException: null reference
11-15 16:37:44.496 31029 31319 E AndroidRuntime: 	at com.google.android.gms.common.internal.Preconditions.checkNotNull(com.google.android.gms:play-services-basement@@17.3.0:2)
11-15 16:37:44.496 31029 31319 E AndroidRuntime: 	at com.google.firebase.auth.FirebaseAuth.signInWithCredential(com.google.firebase:firebase-auth@@20.0.0:144)
11-15 16:37:44.496 31029 31319 E AndroidRuntime: 	at io.invertase.firebase.auth.ReactNativeFirebaseAuthModule.lambda$linkWithCredential$28$ReactNativeFirebaseAuthModule(ReactNativeFirebaseAuthModule.java:1310)
11-15 16:37:44.496 31029 31319 E AndroidRuntime: 	at io.invertase.firebase.auth.-$$Lambda$ReactNativeFirebaseAuthModule$s2BlkFr6NYa4r5jx1U5v1tfWQT0.onComplete(Unknown Source:6)
11-15 16:37:44.496 31029 31319 E AndroidRuntime: 	at com.google.android.gms.tasks.zzj.run(com.google.android.gms:play-services-tasks@@17.1.0:4)
11-15 16:37:44.496 31029 31319 E AndroidRuntime: 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
11-15 16:37:44.496 31029 31319 E AndroidRuntime: 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)

I'm going to wrap it in a try/catch with a reject in the style of the other rejects but I'm not sure what this indicates about the functionality - @nealmanaktola - you attest that this works for you on upgrade of anonymous to users to regular users from an apple sign in flow?

mikehardy added a commit that referenced this pull request Nov 15, 2020
This is related to a #4487 - attempting to re-login with credential
after anonymous user signs in with apple sign-in on android
@nealmanaktola
Copy link
Contributor Author

@mikehardy yep i can attest to that. hmm. i'll take a look to see if there is a cleaner fix, but this should work for now

@mikehardy
Copy link
Collaborator

cool - I think it's just that one set of use cases (which are exercised in the E2E tests but which I had temporarily disabled, so we didn't notice) works a little different than your use case. I'm confident enough in my java that I think both cases will be handled now but I will of course welcome any deeper thinking on your part, or (after the next release) a quick check to make sure we are still handling the apple sign on android flow since E2E is not exercising that apparently

mikehardy added a commit that referenced this pull request Nov 16, 2020
This is related to a #4487 - attempting to re-login with credential
after anonymous user signs in with apple sign-in on android
@nealmanaktola
Copy link
Contributor Author

@mikehardy it looks like with the latest native update with firebase auth bom v26 this fix does not work. I originally tested with the v25 and it worked.getUpdatedCredential returns null now. I think we should revert this commit, what do you think?

@mikehardy
Copy link
Collaborator

Hi @nealmanaktola! but doesn't that imply that it is not working at all? Is there anything else we need to do, or does reverting it mean that things work as expected for android + sign in with apple?

@nealmanaktola
Copy link
Contributor Author

@mikehardy yeah, unfortunately, that means it is not working at all with the latest native firebase auth sdk. I also don't know of a possible fix.

@mikehardy
Copy link
Collaborator

Reverting this and related #4552 in #5694

mikehardy pushed a commit that referenced this pull request Sep 4, 2021
…om anon user (matches iOS) (#5694)

Description
When doing "linkWithCredential" in Android and the users exists relogin with this new user instead of launch exception 
Error: [auth/credential-already-in-use] This credential is already associated with a different user account.

Related  PR #4487 and PR #4552
androidIsForVivek pushed a commit to androidIsForVivek/react-native-firebase that referenced this pull request Sep 15, 2021
…nvertase#4487)

This fixes an issue with Apple sign on Android. iOS was fixed with this commit. iOS was fixed with this PR invertase#4359. Note the solution is different. On iOS, it looks like we bubble the auth credentials back to js land and then retry login. Here, we attempt another login on auth collision errors
androidIsForVivek pushed a commit to androidIsForVivek/react-native-firebase that referenced this pull request Sep 15, 2021
This is related to a invertase#4487 - attempting to re-login with credential
after anonymous user signs in with apple sign-in on android
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🔥 Access to FIRAuthErrorUserInfoUpdatedCredentialKey with Apple Sign In
3 participants