From 21116161f92eda06389daaef670aa593aa588bcd Mon Sep 17 00:00:00 2001 From: Morgan Chen Date: Thu, 21 Jun 2018 12:10:05 -0700 Subject: Fix bad parsing of JWT dates --- Example/Auth/Tests/FIRAuthTests.m | 21 ++++++++++---------- Example/Auth/Tests/FIRPhoneAuthProviderTests.m | 3 ++- Example/Auth/Tests/FIRUserTests.m | 4 ++-- Firebase/Auth/Source/FIRAuthErrorUtils.h | 13 +++++++++++++ Firebase/Auth/Source/FIRAuthErrorUtils.m | 22 +++++++++++++++++++++ Firebase/Auth/Source/FIRAuthInternalErrors.h | 3 +++ Firebase/Auth/Source/FIRUser.m | 27 +++++++++++++++++++------- Firebase/Auth/Source/Public/FIRAuthErrors.h | 5 +++++ 8 files changed, 78 insertions(+), 20 deletions(-) diff --git a/Example/Auth/Tests/FIRAuthTests.m b/Example/Auth/Tests/FIRAuthTests.m index 34c0499..8859cb3 100644 --- a/Example/Auth/Tests/FIRAuthTests.m +++ b/Example/Auth/Tests/FIRAuthTests.m @@ -1863,16 +1863,17 @@ static const NSTimeInterval kWaitInterval = .5; [self waitForSignInWithAccessToken:kTestAccessToken APIKey:kTestAPIKey completion:nil]; - NSString *kTestAPIKey2 = @"fakeAPIKey2"; - FIRUser *user2 = [FIRAuth auth].currentUser; - user2.requestConfiguration = [[FIRAuthRequestConfiguration alloc]initWithAPIKey:kTestAPIKey2]; - OCMExpect([_mockBackend getAccountInfo:[OCMArg any] callback:[OCMArg any]]) - .andDispatchError2([FIRAuthErrorUtils networkErrorWithUnderlyingError:[NSError new]]); - XCTestExpectation *expectation = [self expectationWithDescription:@"callback"]; - [[FIRAuth auth] updateCurrentUser:user2 completion:^(NSError *_Nullable error) { - XCTAssertEqual(error.code, FIRAuthErrorCodeNetworkError); - [expectation fulfill]; - }]; + NSString *kTestAPIKey2 = @"fakeAPIKey2"; + FIRUser *user2 = [FIRAuth auth].currentUser; + user2.requestConfiguration = [[FIRAuthRequestConfiguration alloc]initWithAPIKey:kTestAPIKey2]; + NSError *underlyingError = [NSError errorWithDomain:@"Test Error" code:1 userInfo:nil]; + OCMExpect([_mockBackend getAccountInfo:[OCMArg any] callback:[OCMArg any]]) + .andDispatchError2([FIRAuthErrorUtils networkErrorWithUnderlyingError:underlyingError]); + XCTestExpectation *expectation = [self expectationWithDescription:@"callback"]; + [[FIRAuth auth] updateCurrentUser:user2 completion:^(NSError *_Nullable error) { + XCTAssertEqual(error.code, FIRAuthErrorCodeNetworkError); + [expectation fulfill]; + }]; [self waitForExpectationsWithTimeout:kExpectationTimeout handler:nil]; OCMVerifyAll(_mockBackend); } diff --git a/Example/Auth/Tests/FIRPhoneAuthProviderTests.m b/Example/Auth/Tests/FIRPhoneAuthProviderTests.m index 1bb42b1..6c925a5 100644 --- a/Example/Auth/Tests/FIRPhoneAuthProviderTests.m +++ b/Example/Auth/Tests/FIRPhoneAuthProviderTests.m @@ -453,7 +453,8 @@ static const NSTimeInterval kExpectationTimeout = 2; // Assert that the app credential is nil when in test mode. XCTAssertNil(request.appCredential); dispatch_async(FIRAuthGlobalWorkQueue(), ^() { - callback(nil, [FIRAuthErrorUtils networkErrorWithUnderlyingError:[NSError new]]); + NSError *underlying = [NSError errorWithDomain:@"Test Error" code:1 userInfo:nil]; + callback(nil, [FIRAuthErrorUtils networkErrorWithUnderlyingError:underlying]); }); }); diff --git a/Example/Auth/Tests/FIRUserTests.m b/Example/Auth/Tests/FIRUserTests.m index 8bb6786..957fd24 100644 --- a/Example/Auth/Tests/FIRUserTests.m +++ b/Example/Auth/Tests/FIRUserTests.m @@ -1151,8 +1151,8 @@ static const NSTimeInterval kExpectationTimeout = 2; XCTAssertEqualObjects(request.APIKey, kAPIKey); dispatch_async(FIRAuthGlobalWorkQueue(), ^() { - - callback(nil, [FIRAuthErrorUtils networkErrorWithUnderlyingError:[NSError new]]); + NSError *underlying = [NSError errorWithDomain:@"Test Error" code:1 userInfo:nil]; + callback(nil, [FIRAuthErrorUtils networkErrorWithUnderlyingError:underlying]); }); }); [user getIDTokenResultForcingRefresh:YES diff --git a/Firebase/Auth/Source/FIRAuthErrorUtils.h b/Firebase/Auth/Source/FIRAuthErrorUtils.h index 5b8205f..6cc22ab 100644 --- a/Firebase/Auth/Source/FIRAuthErrorUtils.h +++ b/Firebase/Auth/Source/FIRAuthErrorUtils.h @@ -85,6 +85,19 @@ NS_ASSUME_NONNULL_BEGIN */ + (NSError *)unexpectedErrorResponseWithDeserializedResponse:(id)deserializedResponse; +/** @fn malformedJWTErrorWithToken:underlyingError: + @brief Constructs an @c NSError with the code set to @c FIRAuthErrorCodeMalformedJWT and + populates the userInfo dictionary with an error message, the bad token, and an underlying + error that may have occurred when parsing. + @param token The token that failed to parse. + @param underlyingError The error that caused this error. If this parameter is nil, the + NSUnderlyingErrorKey value will not be set. + @remarks This error is returned when JWT parsing fails. + @returns An @c FIRAuthErrorCodeMalformedJWT error wrapping an underlying error, if available. + */ ++ (NSError *)malformedJWTErrorWithToken:(NSString *)token + underlyingError:(NSError *_Nullable)underlyingError; + /** @fn unexpectedResponseWithData:underlyingError: @brief Constructs an @c NSError with the @c FIRAuthInternalErrorCodeUnexpectedResponse code, and a populated @c FIRAuthErrorUserInfoDataKey key in the @c NSError.userInfo diff --git a/Firebase/Auth/Source/FIRAuthErrorUtils.m b/Firebase/Auth/Source/FIRAuthErrorUtils.m index 62c569c..b91231a 100644 --- a/Firebase/Auth/Source/FIRAuthErrorUtils.m +++ b/Firebase/Auth/Source/FIRAuthErrorUtils.m @@ -413,6 +413,12 @@ static NSString *const kFIRAuthErrorMessageNullUser = @"A null user object was p static NSString *const kFIRAuthErrorMessageInternalError = @"An internal error has occurred, " "print and inspect the error details for more information."; +/** @var kFIRAuthErrorMessageMalformedJWT + @brief Error message constant describing @c FIRAuthErrorCodeMalformedJWT errors. + */ +static NSString *const kFIRAuthErrorMessageMalformedJWT = + @"Failed to parse JWT. Check the userInfo dictionary for the full token."; + /** @var FIRAuthErrorDescription @brief The error descrioption, based on the error code. @remarks No default case so that we get a compiler warning if a new value was added to the enum. @@ -531,6 +537,8 @@ static NSString *FIRAuthErrorDescription(FIRAuthErrorCode code) { return kFIRAuthErrorMessageNullUser; case FIRAuthErrorCodeWebInternalError: return kFIRAuthErrorMessageWebInternalError; + case FIRAuthErrorCodeMalformedJWT: + return kFIRAuthErrorMessageMalformedJWT; } } @@ -652,6 +660,8 @@ static NSString *const FIRAuthErrorCodeString(FIRAuthErrorCode code) { return @"ERROR_NULL_USER"; case FIRAuthErrorCodeWebInternalError: return @"ERROR_WEB_INTERNAL_ERROR"; + case FIRAuthErrorCodeMalformedJWT: + return @"ERROR_MALFORMED_JWT"; } } @@ -735,6 +745,18 @@ static NSString *const FIRAuthErrorCodeString(FIRAuthErrorCode code) { }]; } ++ (NSError *)malformedJWTErrorWithToken:(NSString *)token + underlyingError:(NSError *_Nullable)underlyingError { + NSMutableDictionary *userInfo = + [NSMutableDictionary dictionaryWithObject:kFIRAuthErrorMessageMalformedJWT + forKey:NSLocalizedDescriptionKey]; + [userInfo setObject:token forKey:FIRAuthErrorUserInfoDataKey]; + if (underlyingError != nil) { + [userInfo setObject:underlyingError forKey:NSUnderlyingErrorKey]; + } + return [self errorWithCode:FIRAuthInternalErrorCodeMalformedJWT userInfo:[userInfo copy]]; +} + + (NSError *)unexpectedResponseWithData:(NSData *)data underlyingError:(NSError *)underlyingError { return [self errorWithCode:FIRAuthInternalErrorCodeUnexpectedResponse userInfo:@{ diff --git a/Firebase/Auth/Source/FIRAuthInternalErrors.h b/Firebase/Auth/Source/FIRAuthInternalErrors.h index fd08022..4cdd8cf 100644 --- a/Firebase/Auth/Source/FIRAuthInternalErrors.h +++ b/Firebase/Auth/Source/FIRAuthInternalErrors.h @@ -376,6 +376,9 @@ typedef NS_ENUM(NSInteger, FIRAuthInternalErrorCode) { FIRAuthInternalErrorCodeNullUser = FIRAuthPublicErrorCodeFlag | FIRAuthErrorCodeNullUser, + FIRAuthInternalErrorCodeMalformedJWT = + FIRAuthPublicErrorCodeFlag | FIRAuthErrorCodeMalformedJWT, + /** @var FIRAuthInternalErrorCodeRPCRequestEncodingError @brief Indicates an error encoding the RPC request. @remarks This is typically due to some sort of unexpected input value. diff --git a/Firebase/Auth/Source/FIRUser.m b/Firebase/Auth/Source/FIRUser.m index 9bae744..ad0b1d4 100644 --- a/Firebase/Auth/Source/FIRUser.m +++ b/Firebase/Auth/Source/FIRUser.m @@ -851,9 +851,17 @@ static void callInMainThreadWithAuthDataResultAndError( "error" out parameter. */ - (FIRAuthTokenResult *)parseIDToken:(NSString *)token error:(NSError **)error { + // Though this is an internal method, errors returned here are surfaced in user-visible + // callbacks. *error = nil; NSArray *tokenStringArray = [token componentsSeparatedByString:@"."]; + // The JWT should have three parts, though we only use the second in this method. + if (tokenStringArray.count != 3) { + *error = [FIRAuthErrorUtils malformedJWTErrorWithToken:token underlyingError:nil]; + return nil; + } + // The token payload is always the second index of the array. NSString *idToken = tokenStringArray[1]; @@ -863,8 +871,10 @@ static void callInMainThreadWithAuthDataResultAndError( [[idToken stringByReplacingOccurrencesOfString:@"_" withString:@"/"] mutableCopy]; // Replace "-" with "+" - tokenPayload = - [[tokenPayload stringByReplacingOccurrencesOfString:@"-" withString:@"+"] mutableCopy]; + [tokenPayload replaceOccurrencesOfString:@"-" + withString:@"+" + options:kNilOptions + range:NSMakeRange(0, tokenPayload.length)]; // Pad the token payload with "=" signs if the payload's length is not a multiple of 4. while ((tokenPayload.length % 4) != 0) { @@ -874,19 +884,22 @@ static void callInMainThreadWithAuthDataResultAndError( [[NSData alloc] initWithBase64EncodedString:tokenPayload options:NSDataBase64DecodingIgnoreUnknownCharacters]; if (!decodedTokenPayloadData) { - *error = [FIRAuthErrorUtils unexpectedResponseWithDeserializedResponse:token]; + *error = [FIRAuthErrorUtils malformedJWTErrorWithToken:token underlyingError:nil]; return nil; } + NSError *jsonError = nil; + NSJSONReadingOptions options = NSJSONReadingMutableContainers|NSJSONReadingAllowFragments; NSDictionary *tokenPayloadDictionary = [NSJSONSerialization JSONObjectWithData:decodedTokenPayloadData - options:NSJSONReadingMutableContainers|NSJSONReadingAllowFragments - error:error]; - if (*error) { + options:options + error:&jsonError]; + if (jsonError != nil) { + *error = [FIRAuthErrorUtils malformedJWTErrorWithToken:token underlyingError:jsonError]; return nil; } if (!tokenPayloadDictionary) { - *error = [FIRAuthErrorUtils unexpectedResponseWithDeserializedResponse:token]; + *error = [FIRAuthErrorUtils malformedJWTErrorWithToken:token underlyingError:nil]; return nil; } diff --git a/Firebase/Auth/Source/Public/FIRAuthErrors.h b/Firebase/Auth/Source/Public/FIRAuthErrors.h index 4a1a6f1..a3fbe26 100644 --- a/Firebase/Auth/Source/Public/FIRAuthErrors.h +++ b/Firebase/Auth/Source/Public/FIRAuthErrors.h @@ -311,6 +311,11 @@ typedef NS_ENUM(NSInteger, FIRAuthErrorCode) { /** Indicates an internal error occurred. */ FIRAuthErrorCodeInternalError = 17999, + + /** Raised when a JWT fails to parse correctly. May be accompanied by an underlying error + describing which step of the JWT parsing process failed. + */ + FIRAuthErrorCodeMalformedJWT = 18000, } NS_SWIFT_NAME(AuthErrorCode); @end -- cgit v1.2.3