-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
fix(auth, android): fixed issue with apple sign-in on android #4487
Conversation
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
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/invertase/react-native-firebase/53z36wxpa |
Codecov Report
@@ 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 |
There was a problem hiding this 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
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.
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? |
This is related to a #4487 - attempting to re-login with credential after anonymous user signs in with apple sign-in on android
@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 |
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 |
This is related to a #4487 - attempting to re-login with credential after anonymous user signs in with apple sign-in on android
@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. |
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? |
@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. |
…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
…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
This is related to a invertase#4487 - attempting to re-login with credential after anonymous user signs in with apple sign-in on android
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 whenlinkWithCredential
throwsFirebaseAuthUserCollisionException
Checklist
Android
iOS
e2e
tests added or updated inpackages/\*\*/e2e
jest
tests added or updated inpackages/\*\*/__tests__
Test Plan
Think
react-native-firebase
is great? Please consider supporting the project with any of the below:React Native Firebase
andInvertase
on Twitter