firebase / FirebaseUI-iOS

iOS UI bindings for Firebase.
Apache License 2.0
1.51k stars 475 forks source link

"Sign in with Apple" captures "Sign in with Email" and crashes app #856

Open ghost opened 4 years ago

ghost commented 4 years ago

Step 2: Describe your environment

Step 3: Describe the problem:

In my app I currently support sign in with Google, Apple, Facebook and E-Mail. This is a vanilla FirebaseUIAuth installation without any customization. "One account per email" address is enabled.

In the past one user signed in with Apple where the Apple ID was the same email address that the same person used to sign in with email (before sign in with Apple was available).

In the Firebase console both sign in methods are linked to the same user account. So far, so good.

Case 1: If the user now tries to sign in with email, enters his email address and taps "Next", a popup opens that says "You already have an account. You've already used x@y.z. Sign in with Apple to continue." Then there are two options: "Sign in with Apple" or "Cancel".

There is no way to sign with with email anymore for this user, even if this was selected as sign in method.

Case 2: On a second device that is logged into another iCloud account with a different email address, the same user tries to sign in with email using the same email address as in case 1. FUIAuth again takes over the login process when entering the email address, but then offers to log in with Apple using the email address of the iCloud account. When confirming to do this, the app crashes with this message:

*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: 'One of IDToken, accessToken, pendingToken, or requestURI must be supplied.' *** First throw call stack: (0x19de455f0 0x19db67bcc 0x19dd3bb28 0x105351f50 0x105311934 0x10530f5cc 0x10530dac0 0x1052feb78 0x1052fe2f0 0x109e12338 0x109e13730 0x109e1a740 0x109e1b2e0 0x109e266c4 0x19db5cb74 0x19db5f740) libc++abi.dylib: terminating with uncaught exception of type NSException

Expected Results:

I would expect "Sign in with email" to work as before and not "Sign in with Apple" taking control of the login process if sign in with email and sign in with Apple are linked to the same account.

ghost commented 4 years ago

@morganchen12 Any idea what is happening here? This is still current with the latest SDK. Thanks for checking!

morganchen12 commented 4 years ago

Hey @ghowen, sorry for the slow response. The crash is definitely a bug--can you symbolicate the stack trace? If not, it should be pretty consistently reproducible.

This behavior stems from Sign in with Apple being considered a trusted email provider. If you prompt the user for a password and set the account's password (which currently isn't supported via FirebaseUI) you should then be able to use email/password sign-in for that account again.

ghost commented 4 years ago

No need to stack trace IMHO. The crash occurs if the email address used for sign in with email does not match the email address of the currently logged in Apple-ID.

If it matches, log in with Apple still captures the login process, but it succeeds (as the user has both log in methods associated with his email address).

If it does not match, log in with Apple takes over and tries to log into the user account for email address A with the Apple-ID email address B. As email address B has no registered account and no token on the server, it crashes.

ghost commented 4 years ago

@morganchen12 And yes, it is consistently reproducible. Take any user on a device and log the user into Firebase with email address A and then with an Apple-ID that matches email address A. Both will succeed and both login methods will be linked to the account of this user. Then log the user out of the device and log onto the device with a different Apple-ID B. Now try to log into Firebase with the original email address A. It will consistently crash the app.

ghost commented 4 years ago

@morganchen12 I agree that it is an edge case, but I think the current implementation is based on a wrong assumption. It assumes that if someone tries to log in with email and this account is also associated with login with Apple, that this Firebase account's email address is the always the same as the currently active Apple-ID on device. This is probably the case for most people who only ever use one Apple-ID in their life. But as soon as you start to go back and forth (like a business ID and a private ID, your spouse or child and you) then this implementation will fail at some point.

I also feel that the user's choice should always be respected. If the user wants to log in with email, let him or her do that. If he wants to sign in with Apple, likewise.

Thanks for listening and considering :)

morganchen12 commented 4 years ago

Makes sense. I'll look at resolving the crash first, and then see if there's a simple resolution for the other issue you described.

pietroleggero commented 4 years ago

We have also experience the same issue. Is there any update on this topic? it starts becoming a bigger problem

pietroleggero commented 4 years ago

(void)authorizationController:(ASAuthorizationController )controller didCompleteWithAuthorization:(ASAuthorization )authorization API_AVAILABLE(ios(13.0)) { ASAuthorizationAppleIDCredential appleIDCredential = authorization.credential; NSString idToken = [NSString stringWithUTF8String:[appleIDCredential.identityToken bytes]]; FIROAuthCredential *credential = [FIROAuthProvider credentialWithProviderID:@"apple.com" IDToken:idToken accessToken:nil]; _providerSignInCompletion(credential, nil, nil, nil); } When idToken is null you have the crash

ghost commented 4 years ago

@pietroleggero No progress that I am aware of. As a consequence I have started to remove social logins from my app...

Social logins require you to add third party SDKs which can crash your app (Facebook...), can spy on your users without you as a developer knowing it, can slow down the app and increase its size, transfer data from within the EU outside of the EU which at the moment is not covered by any solid legal grounds, etc.

If they then do not work as you would expect, the cons overweigh the pros at this point in time IMHO. Sorry that this does not contribute to a solution of the technical problem :)

morganchen12 commented 3 years ago

Quick update: The crash originally described here happens in upstream Firebase Auth, and is likely not fixable in this SDK. This is where the exception is thrown.

The crash described by @pietroleggero is a separate crash caused by Apple changing their identityToken property from nonnull to nullable (thanks Apple) and is a fix will be released soon.

JonahElbaz commented 3 years ago

Going to weigh in here since I couldn't find any answers as to the error initially mentioned.

So I had the same issue while trying to use apple sign in to generate an OAuth credential. Then passing it to firebase signInWithCredential when i did that my app crashed with the same error about an ID token missing. That ended up being misleading since the issue was that it was saying missing IDToken or missing accessToken but what the firebase function wanted was just an object with secret: {nonce}, token: {appleCredential.idToken}. When i passed it that mapping instead of the full appleCredential created from OAuth, it stopped crashing and worked.