diff options
author | Zsika Phillip <protocol86@users.noreply.github.com> | 2017-09-10 10:05:08 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-09-10 10:05:08 -0700 |
commit | af23702b560c2c10b54f063817fcd6bca369795e (patch) | |
tree | 6d01c384e44dbe7d55994dbdc49898afcbef0f31 | |
parent | e27a3072f6287f2cc05762e61f017bd6b506ab58 (diff) |
ReCAPTCHA verification error handling (#241)
* reCAPTCHA error handling
* Improvements
* Addresses comments
-rw-r--r-- | Example/Auth/Tests/FIRPhoneAuthProviderTests.m | 168 | ||||
-rw-r--r-- | Firebase/Auth/Source/AuthProviders/Phone/FIRPhoneAuthProvider.m | 52 | ||||
-rw-r--r-- | Firebase/Auth/Source/FIRAuthErrorUtils.h | 17 | ||||
-rw-r--r-- | Firebase/Auth/Source/FIRAuthErrorUtils.m | 41 | ||||
-rw-r--r-- | Firebase/Auth/Source/FIRAuthInternalErrors.h | 10 | ||||
-rw-r--r-- | Firebase/Auth/Source/Public/FIRAuthErrors.h | 8 |
6 files changed, 287 insertions, 9 deletions
diff --git a/Example/Auth/Tests/FIRPhoneAuthProviderTests.m b/Example/Auth/Tests/FIRPhoneAuthProviderTests.m index 8f25a31..6e88006 100644 --- a/Example/Auth/Tests/FIRPhoneAuthProviderTests.m +++ b/Example/Auth/Tests/FIRPhoneAuthProviderTests.m @@ -100,6 +100,26 @@ static NSString *const kFakeRedirectURLStringWithoutReCAPTCHAToken = @"com.googl "23456://firebaseauth/link?deep_link_id=https%3A%2F%2Fexample.firebaseapp.com%2F__%2Fauth%2Fcal" "lback%3FauthType%3DverifyApp%26recaptchaToken%3D"; +/** @var kFakeRedirectURLStringFormat + @brief The format for a fake redirect URL string with invalid client error. + */ +static NSString *const kFakeRedirectURLStringInvalidClientID = @"com.googleusercontent.apps.1" + "23456://firebaseauth/link?deep_link_id=https%3A%2F%2Fexample.firebaseapp.com%2F__%2Fauth%2Fcal" + "lback%3FfirebaseError%3D%257B%2522code%2522%253A%2522auth%252Finvalid-oauth-client-id%2522%252" + "C%2522message%2522%253A%2522The%2520OAuth%2520client%2520ID%2520provided%2520is%2520either%252" + "0invalid%2520or%2520does%2520not%2520match%2520the%2520specified%2520API%2520key.%2522%257D%26" + "authType%3DverifyApp"; + +/** @var kFakeRedirectURLStringUknownError + @brief The format for a fake redirect URL string with unknown error response. + */ +static NSString *const kFakeRedirectURLStringUknownError = @"com.googleusercontent.apps.1" + "23456://firebaseauth/link?deep_link_id=https%3A%2F%2Fexample.firebaseapp.com%2F__%2Fauth%2Fcal" + "lback%3FfirebaseError%3D%257B%2522code%2522%253A%2522auth%252Funknown-error-id%2522%252" + "C%2522message%2522%253A%2522The%2520OAuth%2520client%2520ID%2520provided%2520is%2520either%252" + "0invalid%2520or%2520does%2520not%2520match%2520the%2520specified%2520API%2520key.%2522%257D%26" + "authType%3DverifyApp"; + /** @var kTestTimeout @brief A fake timeout value for waiting for push notification. */ @@ -363,6 +383,154 @@ static const NSTimeInterval kExpectationTimeout = 1; } } +/** @fn testVerifyPhoneNumberUIDelegateInvalidClientID + @brief Tests a invocation of @c verifyPhoneNumber:UIDelegate:completion: which results in an + invalid client ID. + */ +- (void)testVerifyPhoneNumberUIDelegateInvalidClientID { + if ([SFSafariViewController class]) { + FIRApp *app = [FIRApp appForAuthUnitTestsWithName:@"testApp"]; + OCMStub([_mockAuth app]).andReturn(app); + // Simulate missing app token error. + OCMExpect([_mockNotificationManager checkNotificationForwardingWithCallback:OCMOCK_ANY]) + .andCallBlock1(^(FIRAuthNotificationForwardingCallback callback) { callback(YES); }); + OCMExpect([_mockAppCredentialManager credential]).andReturn(nil); + OCMExpect([_mockAPNSTokenManager getTokenWithCallback:OCMOCK_ANY]) + .andCallBlock1(^(FIRAuthAPNSTokenCallback callback) { + NSError *error = [NSError errorWithDomain:FIRAuthErrorDomain + code:FIRAuthErrorCodeMissingAppToken + userInfo:nil]; + callback(nil, error); + }); + OCMExpect([_mockBackend getProjectConfig:[OCMArg any] callback:[OCMArg any]]) + .andCallBlock2(^(FIRGetProjectConfigRequest *request, + FIRGetProjectConfigResponseCallback callback) { + XCTAssertNotNil(request); + dispatch_async(FIRAuthGlobalWorkQueue(), ^() { + id mockGetProjectConfigResponse = OCMClassMock([FIRGetProjectConfigResponse class]); + OCMStub([mockGetProjectConfigResponse authorizedDomains]). + andReturn(@[ @"test.firebaseapp.com"]); + callback(mockGetProjectConfigResponse, nil); + }); + }); + // Mock UIDelegate. + id mockUIDelegate = OCMProtocolMock(@protocol(FIRAuthUIDelegate)); + // Expect view controller presentation by UIDelegate. + id presenterArg = [OCMArg isKindOfClass:[SFSafariViewController class]]; + OCMExpect([mockUIDelegate presentViewController:presenterArg + animated:YES + completion:nil]).andDo(^(NSInvocation *invocation) { + __unsafe_unretained id unretainedArgument; + // Indices 0 and 1 indicate the hidden arguments self and _cmd. + // `presentViewController` is at index 2. + [invocation getArgument:&unretainedArgument atIndex:2]; + SFSafariViewController *viewController = unretainedArgument; + XCTAssertEqual(viewController.delegate, [_mockAuth authURLPresenter]); + XCTAssertTrue([viewController isKindOfClass:[SFSafariViewController class]]); + NSLog(@"here: %@",[NSURL URLWithString:kFakeRedirectURLStringInvalidClientID].absoluteString); + [[_mockAuth authURLPresenter] + canHandleURL:[NSURL URLWithString:kFakeRedirectURLStringInvalidClientID]]; + }); + // Expect view controller dismissal by UIDelegate. + OCMExpect([mockUIDelegate dismissViewControllerAnimated:OCMOCK_ANY completion:OCMOCK_ANY]). + andDo(^(NSInvocation *invocation) { + __unsafe_unretained id unretainedArgument; + // Indices 0 and 1 indicate the hidden arguments self and _cmd. + // `completion` is at index 3. + [invocation getArgument:&unretainedArgument atIndex:3]; + void (^finishBlock)() = unretainedArgument; + finishBlock(); + }); + + XCTestExpectation *expectation = [self expectationWithDescription:@"callback"]; + [_provider verifyPhoneNumber:kTestPhoneNumber + UIDelegate:mockUIDelegate + completion:^(NSString *_Nullable verificationID, NSError *_Nullable error) { + XCTAssertTrue([NSThread isMainThread]); + XCTAssertEqual(error.code, FIRAuthErrorCodeInvalidClientID); + XCTAssertNil(verificationID); + [expectation fulfill]; + }]; + [self waitForExpectationsWithTimeout:kExpectationTimeout handler:nil]; + OCMVerifyAll(_mockBackend); + OCMVerifyAll(_mockNotificationManager); + } +} + +/** @fn testVerifyPhoneNumberUIDelegateUnexpectedError + @brief Tests a invocation of @c verifyPhoneNumber:UIDelegate:completion: which results in an + invalid client ID. + */ +- (void)testVerifyPhoneNumberUIDelegateUnexpectedError { + if ([SFSafariViewController class]) { + FIRApp *app = [FIRApp appForAuthUnitTestsWithName:@"testApp"]; + OCMStub([_mockAuth app]).andReturn(app); + // Simulate missing app token error. + OCMExpect([_mockNotificationManager checkNotificationForwardingWithCallback:OCMOCK_ANY]) + .andCallBlock1(^(FIRAuthNotificationForwardingCallback callback) { callback(YES); }); + OCMExpect([_mockAppCredentialManager credential]).andReturn(nil); + OCMExpect([_mockAPNSTokenManager getTokenWithCallback:OCMOCK_ANY]) + .andCallBlock1(^(FIRAuthAPNSTokenCallback callback) { + NSError *error = [NSError errorWithDomain:FIRAuthErrorDomain + code:FIRAuthErrorCodeMissingAppToken + userInfo:nil]; + callback(nil, error); + }); + OCMExpect([_mockBackend getProjectConfig:[OCMArg any] callback:[OCMArg any]]) + .andCallBlock2(^(FIRGetProjectConfigRequest *request, + FIRGetProjectConfigResponseCallback callback) { + XCTAssertNotNil(request); + dispatch_async(FIRAuthGlobalWorkQueue(), ^() { + id mockGetProjectConfigResponse = OCMClassMock([FIRGetProjectConfigResponse class]); + OCMStub([mockGetProjectConfigResponse authorizedDomains]). + andReturn(@[ @"test.firebaseapp.com"]); + callback(mockGetProjectConfigResponse, nil); + }); + }); + // Mock UIDelegate. + id mockUIDelegate = OCMProtocolMock(@protocol(FIRAuthUIDelegate)); + // Expect view controller presentation by UIDelegate. + id presenterArg = [OCMArg isKindOfClass:[SFSafariViewController class]]; + OCMExpect([mockUIDelegate presentViewController:presenterArg + animated:YES + completion:nil]).andDo(^(NSInvocation *invocation) { + __unsafe_unretained id unretainedArgument; + // Indices 0 and 1 indicate the hidden arguments self and _cmd. + // `presentViewController` is at index 2. + [invocation getArgument:&unretainedArgument atIndex:2]; + SFSafariViewController *viewController = unretainedArgument; + XCTAssertEqual(viewController.delegate, [_mockAuth authURLPresenter]); + XCTAssertTrue([viewController isKindOfClass:[SFSafariViewController class]]); + NSLog(@"here: %@",[NSURL URLWithString:kFakeRedirectURLStringInvalidClientID].absoluteString); + [[_mockAuth authURLPresenter] + canHandleURL:[NSURL URLWithString:kFakeRedirectURLStringUknownError]]; + }); + // Expect view controller dismissal by UIDelegate. + OCMExpect([mockUIDelegate dismissViewControllerAnimated:OCMOCK_ANY completion:OCMOCK_ANY]). + andDo(^(NSInvocation *invocation) { + __unsafe_unretained id unretainedArgument; + // Indices 0 and 1 indicate the hidden arguments self and _cmd. + // `completion` is at index 3. + [invocation getArgument:&unretainedArgument atIndex:3]; + void (^finishBlock)() = unretainedArgument; + finishBlock(); + }); + + XCTestExpectation *expectation = [self expectationWithDescription:@"callback"]; + [_provider verifyPhoneNumber:kTestPhoneNumber + UIDelegate:mockUIDelegate + completion:^(NSString *_Nullable verificationID, NSError *_Nullable error) { + XCTAssertTrue([NSThread isMainThread]); + XCTAssertEqual(error.code, FIRAuthErrorCodeAppVerificationUserInteractionFailure); + XCTAssertNil(verificationID); + [expectation fulfill]; + }]; + [self waitForExpectationsWithTimeout:kExpectationTimeout handler:nil]; + OCMVerifyAll(_mockBackend); + OCMVerifyAll(_mockNotificationManager); + } +} + /** @fn testNotForwardingNotification @brief Tests returning an error for the app failing to forward notification. */ diff --git a/Firebase/Auth/Source/AuthProviders/Phone/FIRPhoneAuthProvider.m b/Firebase/Auth/Source/AuthProviders/Phone/FIRPhoneAuthProvider.m index 1fe4d15..32b5da0 100644 --- a/Firebase/Auth/Source/AuthProviders/Phone/FIRPhoneAuthProvider.m +++ b/Firebase/Auth/Source/AuthProviders/Phone/FIRPhoneAuthProvider.m @@ -47,8 +47,7 @@ NS_ASSUME_NONNULL_BEGIN @param reCAPTCHAURL The reCAPTCHA URL. @param error The error that occured while fetching the reCAPTCHAURL, if any. */ -typedef void (^FIRReCAPTCHAURLCallBack)(NSURL *_Nullable reCAPTCHAURL, - NSError *_Nullable error); +typedef void (^FIRReCAPTCHAURLCallBack)(NSURL *_Nullable reCAPTCHAURL, NSError *_Nullable error); /** @typedef FIRVerifyClientCallback @brief The callback invoked at the end of a client verification flow. @@ -158,15 +157,16 @@ NSString *const kReCAPTCHAURLStringFormat = @"https://%@/__/auth/handler?%@"; completion(nil, error); return; } - NSDictionary<NSString *, NSString *> *URLQueryItems = - [NSDictionary gtm_dictionaryWithHttpArgumentsString:callbackURL.query];; - URLQueryItems = - [NSDictionary gtm_dictionaryWithHttpArgumentsString:URLQueryItems[@"deep_link_id"]]; - NSString *reCAPTCHA = URLQueryItems[@"recaptchaToken"]; + NSError *reCAPTCHAError; + NSString *reCAPTCHAToken = [self reCAPTCHATokenForURL:callbackURL error:&reCAPTCHAError]; + if (!reCAPTCHAToken) { + callBackOnMainThread(nil, reCAPTCHAError); + return; + } FIRSendVerificationCodeRequest *request = [[FIRSendVerificationCodeRequest alloc] initWithPhoneNumber:phoneNumber appCredential:nil - reCAPTCHAToken:reCAPTCHA + reCAPTCHAToken:reCAPTCHAToken requestConfiguration:_auth.requestConfiguration]; [FIRAuthBackend sendVerificationCode:request callback:^(FIRSendVerificationCodeResponse @@ -201,6 +201,42 @@ NSString *const kReCAPTCHAURLStringFormat = @"https://%@/__/auth/handler?%@"; } #pragma mark - Internal Methods +/** @fn reCAPTCHATokenForURL:error: + @brief Parses the reCAPTCHA URL and returns. + @param URL The url to be parsed for a reCAPTCHA token. + @param error The error that occurred if any. + @return The reCAPTCHA token if successful. + */ +- (NSString *)reCAPTCHATokenForURL:(NSURL *)URL error:(NSError **)error { + NSDictionary<NSString *, NSString *> *URLQueryItems = + [NSDictionary gtm_dictionaryWithHttpArgumentsString:URL.query]; + NSURL *deepLinkURL = [NSURL URLWithString:URLQueryItems[@"deep_link_id"]]; + URLQueryItems = + [NSDictionary gtm_dictionaryWithHttpArgumentsString:deepLinkURL.query]; + if (URLQueryItems[@"recaptchaToken"]) { + return URLQueryItems[@"recaptchaToken"]; + } + NSData *errorData = [URLQueryItems[@"firebaseError"] dataUsingEncoding:NSUTF8StringEncoding]; + NSError *jsonError; + NSDictionary *errorDict = [NSJSONSerialization JSONObjectWithData:errorData + options:0 + error:&jsonError]; + if (jsonError) { + *error = [FIRAuthErrorUtils JSONSerializationErrorWithUnderlyingError:jsonError]; + return nil; + } + *error = [FIRAuthErrorUtils URLResponseErrorWithCode:errorDict[@"code"] + message:errorDict[@"message"]]; + if (!*error) { + NSString *reason; + if(errorDict[@"code"] && errorDict[@"message"]) { + reason = [NSString stringWithFormat:@"[%@] - %@",errorDict[@"code"], errorDict[@"message"]]; + } + *error = [FIRAuthErrorUtils appVerificationUserInteractionFailureWithReason:reason]; + } + return nil; +} + /** @fn isVerifyAppURL: @brief Parses a URL into all available query items. @param URL The url to be checked against the authType string. diff --git a/Firebase/Auth/Source/FIRAuthErrorUtils.h b/Firebase/Auth/Source/FIRAuthErrorUtils.h index 1537553..af4ee28 100644 --- a/Firebase/Auth/Source/FIRAuthErrorUtils.h +++ b/Firebase/Auth/Source/FIRAuthErrorUtils.h @@ -460,12 +460,27 @@ NS_ASSUME_NONNULL_BEGIN + (NSError *)webContextAlreadyPresentedErrorWithMessage:(nullable NSString *)message; /** @fn webContextCancelledErrorWithMessage: - @brief Constructs an @c NSError with the @c FIRAuthErrorCodeWebContextCancelledcode. + @brief Constructs an @c NSError with the @c FIRAuthErrorCodeWebContextCancelled code. @param message Error message from the backend, if any. @return The NSError instance associated with the given FIRAuthError. */ + (NSError *)webContextCancelledErrorWithMessage:(nullable NSString *)message; +/** @fn appVerificationUserInteractionFailureWithReason: + @brief Constructs an @c NSError with the @c + FIRAuthErrorCodeAppVerificationUserInteractionFailure code. + @param reason Reason for error, returned via URL response. + @return The NSError instance associated with the given FIRAuthError. + */ ++ (NSError *)appVerificationUserInteractionFailureWithReason:(nullable NSString *)reason; + +/** @fn URLResponseErrorWithCode:message: + @brief Constructs an @c NSError with the code and message provided. + @param message Error message from the backend, if any. + @return The nullable NSError instance associated with the given error message, if one is found. + */ ++ (NSError *)URLResponseErrorWithCode:(NSString *)code message:(nullable NSString *)message; + /** @fn keychainErrorWithFunction:status: @brief Constructs an @c NSError with the @c FIRAuthErrorCodeKeychainError code. @param keychainFunction The keychain function which was invoked and yielded an unexpected diff --git a/Firebase/Auth/Source/FIRAuthErrorUtils.m b/Firebase/Auth/Source/FIRAuthErrorUtils.m index d5b1c9b..bfa0c41 100644 --- a/Firebase/Auth/Source/FIRAuthErrorUtils.m +++ b/Firebase/Auth/Source/FIRAuthErrorUtils.m @@ -42,6 +42,13 @@ NSString *const FIRAuthUpdatedCredentialKey = @"FIRAuthUpdatedCredentialKey"; */ static NSString *const kServerErrorDetailMarker = @" : "; +#pragma mark - URL response error codes + +/** @var kURLResponseErrorCodeInvalidClientID + @brief Error code that indicates that the client ID provided was invalid. + */ +static NSString *const kURLResponseErrorCodeInvalidClientID = @"auth/invalid-oauth-client-id"; + #pragma mark - Standard Error Messages /** @var kFIRAuthErrorMessageInvalidCustomToken @@ -357,6 +364,18 @@ static NSString *const kFIRAuthErrorMessageWebContextAlreadyPresented = @"User i static NSString *const kFIRAuthErrorMessageWebContextCancelled = @"The interaction was cancelled " "by the user."; +/** @var kFIRAuthErrorMessageInvalidClientID + @brief Message for @c FIRAuthErrorCodeInvalidClientID error code. + */ +static NSString *const kFIRAuthErrorMessageInvalidClientID = @"The OAuth client ID provided is " + "either invalid or does not match the specified API key."; + +/** @var kFIRAuthErrorMessageAppVerificationUserInteractionFailure + @brief Message for @c FIRAuthErrorCodeInvalidClientID error code. + */ +static NSString *const kFIRAuthErrorMessageAppVerificationUserInteractionFailure = @"The app " + "verification process has failed, print and inspect the error details for more information"; + /** @var kFIRAuthErrorMessageInternalError @brief Message for @c FIRAuthErrorCodeInternalError error code. */ @@ -471,6 +490,10 @@ static NSString *FIRAuthErrorDescription(FIRAuthErrorCode code) { return kFIRAuthErrorMessageWebContextAlreadyPresented; case FIRAuthErrorCodeWebContextCancelled: return kFIRAuthErrorMessageWebContextCancelled; + case FIRAuthErrorCodeInvalidClientID: + return kFIRAuthErrorMessageInvalidClientID; + case FIRAuthErrorCodeAppVerificationUserInteractionFailure: + return kFIRAuthErrorMessageAppVerificationUserInteractionFailure; } } @@ -582,6 +605,10 @@ static NSString *const FIRAuthErrorCodeString(FIRAuthErrorCode code) { return @"ERROR_WEB_CONTEXT_ALREADY_PRESENTED"; case FIRAuthErrorCodeWebContextCancelled: return @"ERROR_WEB_CONTEXT_CANCELLED"; + case FIRAuthErrorCodeInvalidClientID: + return @"ERROR_INVALID_CLIENT_ID"; + case FIRAuthErrorCodeAppVerificationUserInteractionFailure: + return @"ERROR_APP_VERIFICATION_FAILED"; } } @@ -902,6 +929,20 @@ static NSString *const FIRAuthErrorCodeString(FIRAuthErrorCode code) { return [self errorWithCode:FIRAuthInternalErrorCodeWebContextCancelled message:message]; } ++ (NSError *)appVerificationUserInteractionFailureWithReason:(nullable NSString *)reason { + return [self errorWithCode:FIRAuthInternalErrorCodeAppVerificationUserInteractionFailure + userInfo:@{ + NSLocalizedFailureReasonErrorKey : reason + }]; +} + ++ (NSError *)URLResponseErrorWithCode:(NSString *)code message:(nullable NSString *)message { + if ([code isEqualToString:kURLResponseErrorCodeInvalidClientID]) { + return [self errorWithCode:FIRAuthInternalErrorCodeInvalidClientID message:message]; + } + return nil; +} + + (NSError *)keychainErrorWithFunction:(NSString *)keychainFunction status:(OSStatus)status { NSString *failureReason = [NSString stringWithFormat:@"%@ (%li)", keychainFunction, (long)status]; return [self errorWithCode:FIRAuthInternalErrorCodeKeychainError userInfo:@{ diff --git a/Firebase/Auth/Source/FIRAuthInternalErrors.h b/Firebase/Auth/Source/FIRAuthInternalErrors.h index ffdb068..323fb51 100644 --- a/Firebase/Auth/Source/FIRAuthInternalErrors.h +++ b/Firebase/Auth/Source/FIRAuthInternalErrors.h @@ -333,6 +333,16 @@ typedef NS_ENUM(NSInteger, FIRAuthInternalErrorCode) { FIRAuthInternalErrorCodeWebContextCancelled = FIRAuthPublicErrorCodeFlag | FIRAuthErrorCodeWebContextCancelled, + /** Indicates a general failure during the app verification flow. + */ + FIRAuthInternalErrorCodeAppVerificationUserInteractionFailure = + FIRAuthPublicErrorCodeFlag | FIRAuthErrorCodeAppVerificationUserInteractionFailure, + + /** Indicates that the clientID used to invoke a web flow is invalid. + */ + FIRAuthInternalErrorCodeInvalidClientID = + FIRAuthPublicErrorCodeFlag | FIRAuthErrorCodeInvalidClientID, + // The enum values between 17046 and 17051 are reserved and should NOT be used for new error // codes. diff --git a/Firebase/Auth/Source/Public/FIRAuthErrors.h b/Firebase/Auth/Source/Public/FIRAuthErrors.h index 2b7ef96..6b0d8bc 100644 --- a/Firebase/Auth/Source/Public/FIRAuthErrors.h +++ b/Firebase/Auth/Source/Public/FIRAuthErrors.h @@ -285,6 +285,14 @@ typedef NS_ENUM(NSInteger, FIRAuthErrorCode) { */ FIRAuthErrorCodeWebContextCancelled = 17058, + /** Indicates a general failure during the app verification flow. + */ + FIRAuthErrorCodeAppVerificationUserInteractionFailure = 17059, + + /** Indicates that the clientID used to invoke a web flow is invalid. + */ + FIRAuthErrorCodeInvalidClientID = 17060, + /** Indicates an error occurred while attempting to access the keychain. */ FIRAuthErrorCodeKeychainError = 17995, |