diff --git a/CHANGELOG.md b/CHANGELOG.md index 2636f7cf..7703fa75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ # Unreleased +- Fix a crash when a server error response carries a non-string value under its `error` key. The EMM error handler sent `-hasPrefix:` to whatever value was present, raising an unrecognized selector exception on a number, array or object. - Fix a custom `nonce` and requested token `claims` being dropped when a sign-in is continued after a Device Policy app restart. # 10.0.0 diff --git a/GoogleSignIn/Sources/GIDEMMErrorHandler.m b/GoogleSignIn/Sources/GIDEMMErrorHandler.m index 1429a435..1e00d934 100644 --- a/GoogleSignIn/Sources/GIDEMMErrorHandler.m +++ b/GoogleSignIn/Sources/GIDEMMErrorHandler.m @@ -64,23 +64,25 @@ - (BOOL)handleErrorFromResponse:(NSDictionary *)response if (!_pendingDialog && [UIAlertController class] && [response isKindOfClass:[NSDictionary class]]) { id errorValue = response[kErrorKey]; - if ([errorValue isEqual:kScreenlockRequiredError]) { - errorCode = ErrorCodeScreenlockRequired; - } else if ([errorValue hasPrefix:kAppVerificationRequiredErrorPrefix]) { - errorCode = ErrorCodeAppVerificationRequired; - NSString *appVerificationString = - [errorValue substringFromIndex:kAppVerificationRequiredErrorPrefix.length]; - if ([appVerificationString hasPrefix:kErrorPayloadSeparator]) { - appVerificationString = - [appVerificationString substringFromIndex:kErrorPayloadSeparator.length]; + if ([errorValue isKindOfClass:[NSString class]]) { + if ([errorValue isEqual:kScreenlockRequiredError]) { + errorCode = ErrorCodeScreenlockRequired; + } else if ([errorValue hasPrefix:kAppVerificationRequiredErrorPrefix]) { + errorCode = ErrorCodeAppVerificationRequired; + NSString *appVerificationString = + [errorValue substringFromIndex:kAppVerificationRequiredErrorPrefix.length]; + if ([appVerificationString hasPrefix:kErrorPayloadSeparator]) { + appVerificationString = + [appVerificationString substringFromIndex:kErrorPayloadSeparator.length]; + } + appVerificationString = [appVerificationString + stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceCharacterSet]]; + if (appVerificationString.length) { + appVerificationURL = [NSURL URLWithString:appVerificationString]; + } + } else if ([errorValue hasPrefix:kGeneralErrorPrefix]) { + errorCode = ErrorCodeDeviceNotCompliant; } - appVerificationString = [appVerificationString - stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceCharacterSet]]; - if (appVerificationString.length) { - appVerificationURL = [NSURL URLWithString:appVerificationString]; - } - } else if ([errorValue hasPrefix:kGeneralErrorPrefix]) { - errorCode = ErrorCodeDeviceNotCompliant; } if (errorCode) { _pendingDialog = YES; diff --git a/GoogleSignIn/Tests/Unit/GIDEMMErrorHandlerTest.m b/GoogleSignIn/Tests/Unit/GIDEMMErrorHandlerTest.m index b51519c6..c623d92d 100644 --- a/GoogleSignIn/Tests/Unit/GIDEMMErrorHandlerTest.m +++ b/GoogleSignIn/Tests/Unit/GIDEMMErrorHandlerTest.m @@ -120,6 +120,24 @@ - (void)testNoError { XCTAssertNil(_presentedViewController); } +// Verifies that a non-string value under the `error` key is ignored rather than crashing. +// The value comes straight from a server JSON response and is typed `id`, so it can be any +// plist type. `-hasPrefix:` is an `NSString` method, so before the type check was added it +// raised an unrecognized selector exception on a number, array or dictionary. +- (void)testNonStringErrorValue { + NSArray *nonStringValues = @[ @123, @[ @"emm_passcode_required" ], @{ @"a" : @"b" } ]; + for (id nonStringValue in nonStringValues) { + __block BOOL completionCalled = NO; + NSDictionary *response = @{ @"error" : nonStringValue }; + BOOL result = [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response + completion:^() { + completionCalled = YES; + }]; + XCTAssertFalse(result); + XCTAssertTrue(completionCalled); + } +} + // Verifies that the handler doesn't handle non-EMM error. - (void)testNoEMMError { __block BOOL completionCalled = NO;