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

Firestore fails with an Internal Error due to invalid Auth #3197

Closed
jlaws opened this issue Jun 16, 2019 · 10 comments
Closed

Firestore fails with an Internal Error due to invalid Auth #3197

jlaws opened this issue Jun 16, 2019 · 10 comments
Assignees
Milestone

Comments

@jlaws
Copy link

@jlaws jlaws commented Jun 16, 2019

[READ] Step 1: Are you in the right place?

YES

[REQUIRED] Step 2: Describe your environment

  • Xcode version: 10.2.1
  • Firebase SDK version: 6.2.0
  • Firebase Component: Auth, Firestore
  • Component version: 6.2.0 for both

[REQUIRED] Step 3: Describe the problem

After restarting the app from a cold start all future Firestore queries fail due to an Internal Error with no other information or details on what went wrong. I traced the call chain back to the FIRSecureTokenService where it is failing to retrieve fresh access tokens due to _requestConfiguration being nil. The error, before it is suppressed by all the call chains in Firestore, can be seen here:

Printing description of error:
Error Domain=FIRAuthErrorDomain Code=17999 "An internal error has occurred, print and inspect the error details for more information." UserInfo={FIRAuthErrorUserInfoNameKey=ERROR_INTERNAL_ERROR, NSLocalizedDescription=An internal error has occurred, print and inspect the error details for more information., NSUnderlyingError=0x2809d08a0 {Error Domain=FIRAuthInternalErrorDomain Code=3 "(null)" UserInfo={FIRAuthErrorUserInfoDeserializedResponseKey={
   code = 400;
   details =     (
               {
           "@type" = "type.googleapis.com/google.rpc.Help";
           links =             (
                               {
                   description = "Google developers console";
                   url = "https://console.developers.google.com";
               }
           );
       }
   );
   message = "API key not valid. Please pass a valid API key.";
   status = "INVALID_ARGUMENT";
}}}}

The problem is that FIRSecureTokenService is created via initWithCoder and never has the API key set. In the encodeWithCode method it says this

  // The API key is encoded even it is not used in decoding to be compatible with previous versions
  // of the library.
  [aCoder encodeObject:_requestConfiguration.APIKey forKey:kAPIKeyCodingKey];

I am not sure why the API key was only added to the encoding and not decoded, perhaps there was a plan to set it externally after this object was decoded in FIRUser (line 330) since FIRUser also has a _requestConfiguration. Regardless of why it was removed, I was able to fix the issue by parsing the stored APIKey and correctly creating the _requestConfiguration

- (nullable instancetype)initWithCoder:(NSCoder *)aDecoder {
  NSString *APIKey = [aDecoder decodeObjectOfClass:[NSString class] forKey:kAPIKeyCodingKey];
  NSString *refreshToken = [aDecoder decodeObjectOfClass:[NSString class] forKey:kRefreshTokenKey];
  NSString *accessToken = [aDecoder decodeObjectOfClass:[NSString class] forKey:kAccessTokenKey];
  NSDate *accessTokenExpirationDate =
      [aDecoder decodeObjectOfClass:[NSDate class] forKey:kAccessTokenExpirationDateKey];
  if (!refreshToken) {
    return nil;
  }
  self = [self init];
  if (self) {
    _refreshToken = refreshToken;
    _accessToken = accessToken;
    _accessTokenExpirationDate = accessTokenExpirationDate;
    _requestConfiguration = [[FIRAuthRequestConfiguration alloc] initWithAPIKey:APIKey];
  }
  return self;
}

Also, while debugging this issue I also noticed a bug on line 335 of FIRUser.m the correct parsing of APIKey should be

  NSString *APIKey =
      [aDecoder decodeObjectOfClass:[NSString class] forKey:kAPIKeyCodingKey];

not

  NSString *APIKey =
      [aDecoder decodeObjectOfClass:[FIRUserMetadata class] forKey:kAPIKeyCodingKey];

Steps to reproduce:

Ensure a user is logged in and their auth is stored via |encodeWithCoder:| on FIRUser.m. I used Google Authentication for this. Perform some Firestore queries, it should work fine after authenticating. Restart the app after the cached access token expires so that FIRSecureTokenService |requestAccessToken:| is called for an updated token during future Firestore queries. See the internal error.

@ryanwilson
Copy link
Member

@ryanwilson ryanwilson commented Jun 17, 2019

Thank you @jlaws for the incredibly detailed explanation and the debugging! Assigning to the Auth team who has more context about the decoding issue.

@wilhuff
Copy link
Contributor

@wilhuff wilhuff commented Jun 17, 2019

It would be super helpful if you could point out where you saw Firestore suppressing this error. You shouldn’t have to work this hard to be able to report a problem.

@renkelvin
Copy link
Contributor

@renkelvin renkelvin commented Jun 18, 2019

@jlaws Thanks for your detailed explanation. I've verified that this should be a bug on Auth side. I'm investigating and working on the fix.

@renkelvin
Copy link
Contributor

@renkelvin renkelvin commented Jun 21, 2019

@jlaws _requestConfiguration of tokenService is not persisted but set by FIRAuth (code) when the user is retrieved from local cache (code). This is intentional in case API key is changed.
Could you help check why it's not updated in your case?

@jlaws
Copy link
Author

@jlaws jlaws commented Jun 21, 2019

@renkelvin I use

Auth.auth().useUserAccessGroup("MY_GROUP")

which causes the setAuth to not be called.

Note: You need to use a device to reproduce it. On a simulator it goes through a different code path.

@renkelvin
Copy link
Contributor

@renkelvin renkelvin commented Jun 21, 2019

@jlaws Could you please help check if the fix works for you?

You can apply the fix by adding
pod 'FirebaseAuth', :git => 'https://github.com/firebase/firebase-ios-sdk.git', :branch => 'keychain-fix'
to your podfile.

@jlaws
Copy link
Author

@jlaws jlaws commented Jun 21, 2019

Applied the fix from your PR manually and it works.

Putting on my old obj-c readability reviewer @ Google hat:

user.auth is set in both parts of the if, so it is safe to pull it out.
When both parts of an if/else are handled its best to simplify the logic in the if check for quicker/easier readability. That means remove the ! and flipping the if/else.

That would turn the method in FIRAuth.m to this:

- (nullable FIRUser *)getStoredUserForAccessGroup:(NSString *_Nullable)accessGroup
                                            error:(NSError *_Nullable *_Nullable)outError {
  FIRUser *user;
  if (accessGroup) {
    user = [self.storedUserManager getStoredUserForAccessGroup:self.userAccessGroup
                                             projectIdentifier:self.app.options.APIKey
                                                         error:outError];
  } else {
    NSString *userKey = [NSString stringWithFormat:kUserKey, _firebaseAppName];
    NSData *encodedUserData = [_keychain dataForKey:userKey error:outError];
    if (!encodedUserData) {
      return nil;
    }

    NSKeyedUnarchiver *unarchiver =
      [[NSKeyedUnarchiver alloc] initForReadingWithData:encodedUserData];
    user = [unarchiver decodeObjectOfClass:[FIRUser class] forKey:userKey];
  }
  user.auth = self;
  return user;
}
@renkelvin
Copy link
Contributor

@renkelvin renkelvin commented Jun 22, 2019

Yeah, agree.

@ValCanBuild
Copy link

@ValCanBuild ValCanBuild commented Jul 3, 2019

I'm facing the same issue. I see the fix is already in 6.4.0 - When would that be released to carthage?

@paulb777
Copy link
Member

@paulb777 paulb777 commented Jul 3, 2019

@ValCanBuild 6.4.0 is stabilizing now and should be published next week.

@paulb777 paulb777 added this to the 6.4.0 milestone Jul 3, 2019
@paulb777 paulb777 closed this Jul 3, 2019
@firebase firebase locked and limited conversation to collaborators Oct 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants