firebase / firebase-ios-sdk

Firebase SDK for Apple App Development
https://firebase.google.com
Apache License 2.0
5.59k stars 1.46k forks source link

Firebase Authentication's error code seems to be broken after upgrading to 5.4.0 #2522

Closed mono0926 closed 5 years ago

mono0926 commented 5 years ago

Describe your environment

Describe the problem

My app's auth logic was broken after upgrading to Firebase Authentication 5.4.0, so I investigated that. It seems that FIRAuthErrorCodeInternalError(17999) error started occurring for many requests.

Steps to reproduce:

This is one of the steps to reproduce the issue.

  1. Login with anonymous Firebase user: success.
  2. Tried to link the Firebase user to exiting linked Facebook account.

After that, FIRAuthErrorCodeCredentialAlreadyInUse(17025) should return, but after upgrading to Firebase Authentication 5.4.0, FIRAuthErrorCodeInternalError(17999)started returning.

import FirebaseAuth
import FBSDKLoginKit

let firUser = Auth.auth().currentUser!
FBSDKLoginManager().logIn(withReadPermissions: ["public_profile"], from: viewController) { (result, error) in
    let fbToken = result!.token!

    let credential = FacebookAuthProvider.credential(withAccessToken: fbToken.tokenString)

    firUser.linkAndRetrieveData(with: credential, completion: { (result, error) in
        // error code should be `FIRAuthErrorCodeCredentialAlreadyInUse(17025)`
        // error code is `FIRAuthErrorCodeInternalError(17999)` after upgrading to Firebase Authentication 5.4.0
    })
}

I suspect that #2405 is related? 🤔

ryanwilson commented 5 years ago

@mono0926 thanks for the reply, @renkelvin or @Yue-Wang-Google can you please take a look?

renkelvin commented 5 years ago

@mono0926 Thanks for reporting! It's verified that the issue was introduced in #2405 and we're working on the fix. Thanks!

Yue-Wang-Google commented 5 years ago

@mono0926 could you please try #2530 and see if it works for you? It works for me.

Yue-Wang-Google commented 5 years ago

@mono0926 I'll close this bug for now as it works for me plus we're freezing the code tonight. Feel free to comment below whether it resolves your problem or not.

mono0926 commented 5 years ago

@Yue-Wang-Google

Thanks, I checked that, but the issue seems to be not fixed unfortunately, so I'd like you to reopen this issue.

I checked it by running the code which pasted here( https://github.com/firebase/firebase-ios-sdk/issues/2522#issue-419835788 ), and add breakpoints to the code you modified.

v5.4.0 + #2530

shortErrorMessage is MISSING_ID_TOKEN.

Screen Shot 2019-03-13 at 14 39 06 Screen Shot 2019-03-13 at 14 40 33

v5.3.1

shortErrorMessage should be FEDERATED_USER_ID_ALREADY_LINKED.

Screen Shot 2019-03-13 at 14 44 05 Screen Shot 2019-03-13 at 14 54 07

You tried to fix kFederatedUserIDAlreadyLinkedMessage specific problem at #2530, but It might be more global problem as I said 🤔

My app's auth logic was broken after upgrading to Firebase Authentication 5.4.0, so I investigated that. It seems that FIRAuthErrorCodeInternalError(17999) error started occurring for many requests.

Yue-Wang-Google commented 5 years ago

Thanks for reporting. #2530 did fix the issue when the user is new. However for existing users it seems the new code did return an wrong error. I'm going to reopen the issue. For now you could comment out FIRVerifyAssertionRequest.m's L154 (which set body[kReturnIDPCredentialKey] to @YES) or simply revert to the old version, if you need an urgent fix. I'll fix the bug tomorrow.

mono0926 commented 5 years ago

@Yue-Wang-Google

comment out FIRVerifyAssertionRequest.m's L154 (which set body[kReturnIDPCredentialKey] to @YES)

This fixed the problem, thanks!!

image
mono0926 commented 5 years ago

@Yue-Wang-Google

I believe I found the official fix: #2541

Oh, it should be fixed.

But, unfortunately without FIRVerifyAssertionRequest.m's L154 change, the problem isn't fixed.

1 revert the comment in FIRVerifyAssertionRequest.m's L154 (set it back to YES) if you commented it out. kReturnIDPCredentialKey needs to be true in order to make OAuth working.

Screen Shot 2019-03-13 at 16 44 23

2 You need to apply #2541

Screen Shot 2019-03-13 at 16 45 27

3 You also need to apply the previous fix #2530 --- both changes are necessary.

Screen Shot 2019-03-13 at 16 44 00

Result:

image

In summary, the results is below:

mono0926 commented 5 years ago

I believe I found the official fix: #2541

Oh, it should be fixed.

I suspect that it was correct because initial implementation(May 16, 2017) used response.IDToken 🤔

https://github.com/firebase/firebase-ios-sdk/blob/98ba64449a632518bd2b86fe8d927f4a960d3ddc/Firebase/Auth/Source/FIRUser.m#L884

Yue-Wang-Google commented 5 years ago

I'm very sorry. @mono0926 I must be drunk. Please try #2542 with #2530. (you need both patches, and you need to revert the kReturnIDPCredentialKey comment) I'm very confident that this is the right fix.

Again, thank you for testing and reporting the bug to us! Really appreciated (next time I am in Tokyo I'll invite you for a beer)!

mono0926 commented 5 years ago

Please try #2542 with #2530. (you need both patches, and you need to revert the kReturnIDPCredentialKey comment)

It works fine! Thanks for the quick fix 🙏

Yue-Wang-Google commented 5 years ago

It works fine! Thanks for the quick fix 🙏

Great! I'll wait for it to be approved by my colleague tomorrow, and after that v5.4.1 will be released.

Thanks for your bug report and testing my fixes. I will also keep my promise to buy you a beer, by the way:)

sentex commented 5 years ago

I have the same problem. Get FIRAuthErrorCodeInternalError(17999) after upgrading to Firebase Authentication 5.4.0

Yue-Wang-Google commented 5 years ago

@sentex it's fixed in 5.4.1. See https://github.com/firebase/firebase-ios-sdk/releases/tag/Auth-5.4.1

sentex commented 5 years ago

how can I install this version ?

Yue-Wang-Google commented 5 years ago

@sentex you can wait a day or two and see if it is upgraded on Cocoapods. Or, you can simply patch in #2542 and #2530.

paulb777 commented 5 years ago

Or see instructions at https://github.com/firebase/firebase-ios-sdk#accessing-firebase-source-snapshots or wait an hour or so. We're publishing to CocoaPods now.