diff --git a/CHANGELOG.md b/CHANGELOG.md index ffe95193..995c6b85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ - Fix a data race on `GIDGoogleUser`'s tokens. Concurrent token updates could write the access, refresh and ID tokens from different threads at once, and readers could observe a partially updated set. The three tokens are now published together as one snapshot. - `GIDGoogleUser`'s `accessToken`, `refreshToken` and `idToken` are now all derived from that snapshot, so key-value observers of any one of them are notified whenever any of them changes, not only when that property's own value changes. - Key-value observers of those properties are notified while `GIDGoogleUser` holds its internal lock. Calling back into the same user from the observer is fine, but an observer must not synchronously wait (for example with `dispatch_sync`) on another thread that is updating the same user. +- Fix EMM remediation dialogs being suppressed for the rest of the app's lifetime after a single error arrived while no key window was available. The pending-dialog flag is now cleared on every exit path, and its clearing write is synchronized to match the read. +- Fix a token request that failed with an EMM error sometimes being reported with the underlying OAuth error instead of `kGIDSignInErrorCodeEMM` when no remediation dialog could be presented. +- Fix an EMM error that arrived while a remediation dialog was already showing being reported as `kGIDSignInErrorCodeUnknown` (or as the underlying OAuth error, for a token request) instead of `kGIDSignInErrorCodeEMM`. No second dialog is shown. # 10.0.0 - **BREAKING**: Update to AppAuth 3.0.0 and GTMAppAuth 6.0.0, which raises the minimum deployment targets to iOS 15.0 and macOS 12.0, widens the `GTMSessionFetcher` dependency to allow 4.x and 5.x, and renames the version-specific Swift Package Manager manifest to `Package@swift-5.7.swift`. Projects that must keep supporting earlier OS versions should stay on GoogleSignIn 9.2.0. ([#628](https://github.com/google/GoogleSignIn-iOS/pull/628)) diff --git a/GoogleSignIn/Sources/GIDEMMErrorHandler.h b/GoogleSignIn/Sources/GIDEMMErrorHandler.h index 4ff43ac3..5051f56e 100644 --- a/GoogleSignIn/Sources/GIDEMMErrorHandler.h +++ b/GoogleSignIn/Sources/GIDEMMErrorHandler.h @@ -27,13 +27,13 @@ NS_ASSUME_NONNULL_BEGIN // Retrieve the shared instance of this class. + (instancetype)sharedInstance; -// Handles EMM specific error that is returned in server response. -// Returns whether or not an EMM-specific error is being handled by this invocation. -// If the return value is |YES|, |completion| will be called asynchronously in the main thread -// after the user interacts with the error dialog; -// if the return value is |NO|, |completion| will be called before returning. -- (BOOL)handleErrorFromResponse:(NSDictionary *)response - completion:(void (^)(void))completion; +// Handles EMM-specific errors in the server |response|. |completion| is always called +// exactly once, asynchronously on the main thread, with |YES| if |response| carries an EMM +// error and |NO| otherwise. For an EMM error, a remediation dialog is presented and +// |completion| is called after the user dismisses it — unless another EMM dialog is already +// pending or no dialog can be presented, in which case it is called as soon as possible. +- (void)handleErrorFromResponse:(NSDictionary *)response + completion:(void (^)(BOOL handled))completion; @end diff --git a/GoogleSignIn/Sources/GIDEMMErrorHandler.m b/GoogleSignIn/Sources/GIDEMMErrorHandler.m index 1e00d934..019d6136 100644 --- a/GoogleSignIn/Sources/GIDEMMErrorHandler.m +++ b/GoogleSignIn/Sources/GIDEMMErrorHandler.m @@ -56,13 +56,13 @@ + (instancetype)sharedInstance { return sharedInstance; } -- (BOOL)handleErrorFromResponse:(NSDictionary *)response - completion:(void (^)(void))completion { +- (void)handleErrorFromResponse:(NSDictionary *)response + completion:(void (^)(BOOL handled))completion { ErrorCode errorCode = ErrorCodeNone; NSURL *appVerificationURL; + BOOL dialogAlreadyPending = NO; @synchronized(self) { // for accessing _pendingDialog - if (!_pendingDialog && [UIAlertController class] && - [response isKindOfClass:[NSDictionary class]]) { + if ([UIAlertController class] && [response isKindOfClass:[NSDictionary class]]) { id errorValue = response[kErrorKey]; if ([errorValue isKindOfClass:[NSString class]]) { if ([errorValue isEqual:kScreenlockRequiredError]) { @@ -85,20 +85,38 @@ - (BOOL)handleErrorFromResponse:(NSDictionary *)response } } if (errorCode) { - _pendingDialog = YES; + if (_pendingDialog) { + dialogAlreadyPending = YES; + } else { + _pendingDialog = YES; + } } } } if (!errorCode) { - completion(); - return NO; + dispatch_async(dispatch_get_main_queue(), ^() { + completion(NO); + }); + return; + } + if (dialogAlreadyPending) { + // Another EMM dialog is on screen; report this error as EMM without stacking a second + // dialog on top of it. + dispatch_async(dispatch_get_main_queue(), ^() { + completion(YES); + }); + return; } // All UI must happen in the main thread. dispatch_async(dispatch_get_main_queue(), ^() { + void (^clearPendingDialog)(void) = ^{ + @synchronized(self) { self->_pendingDialog = NO; } + }; UIWindow *keyWindow = [self keyWindow]; if (!keyWindow) { // Shouldn't happen, just in case. - completion(); + clearPendingDialog(); + completion(YES); return; } UIWindow *alertWindow; @@ -121,8 +139,8 @@ - (BOOL)handleErrorFromResponse:(NSDictionary *)response alertWindow.hidden = YES; alertWindow.rootViewController = nil; [keyWindow makeKeyAndVisible]; - self->_pendingDialog = NO; - completion(); + clearPendingDialog(); + completion(YES); }; UIAlertController *alert; switch (errorCode) { @@ -145,7 +163,6 @@ - (BOOL)handleErrorFromResponse:(NSDictionary *)response finish(); } }); - return YES; } // This method is exposed to the unit test. diff --git a/GoogleSignIn/Sources/GIDEMMSupport.m b/GoogleSignIn/Sources/GIDEMMSupport.m index e812c0e3..1940df4f 100644 --- a/GoogleSignIn/Sources/GIDEMMSupport.m +++ b/GoogleSignIn/Sources/GIDEMMSupport.m @@ -68,9 +68,8 @@ + (void)handleTokenFetchEMMError:(nullable NSError *)error completion:(void (^)(NSError *_Nullable))completion { NSDictionary *errorJSON = error.userInfo[OIDOAuthErrorResponseErrorKey]; if (errorJSON) { - __block BOOL handled = NO; - handled = [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:errorJSON - completion:^() { + [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:errorJSON + completion:^(BOOL handled) { if (handled) { completion([NSError errorWithDomain:kGIDSignInErrorDomain code:kGIDSignInErrorCodeEMM diff --git a/GoogleSignIn/Sources/GIDSignIn.m b/GoogleSignIn/Sources/GIDSignIn.m index abbb803e..dc26c07f 100644 --- a/GoogleSignIn/Sources/GIDSignIn.m +++ b/GoogleSignIn/Sources/GIDSignIn.m @@ -984,29 +984,23 @@ - (void)processAuthorizationResponse:(OIDAuthorizationResponse *)authorizationRe [self maybeFetchToken:authFlow]; } else { // There was a failure, convert to appropriate error code. - NSString *errorString; - GIDSignInErrorCode errorCode = kGIDSignInErrorCodeUnknown; NSDictionary *params = authorizationResponse.additionalParameters; - + BOOL deferredToEMMHandler = NO; #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST if (authFlow.emmSupport) { + deferredToEMMHandler = YES; [authFlow wait]; - BOOL isEMMError = [[GIDEMMErrorHandler sharedInstance] - handleErrorFromResponse:params - completion:^{ - [authFlow next]; - }]; - if (isEMMError) { - errorCode = kGIDSignInErrorCodeEMM; - } + [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:params + completion:^(BOOL handled) { + // Set the error before -next, which runs the queued callbacks that read it. + authFlow.error = [self authorizationErrorWithParameters:params isEMMError:handled]; + [authFlow next]; + }]; } #endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST - errorString = (NSString *)params[kOAuth2ErrorKeyName]; - if ([errorString isEqualToString:kOAuth2AccessDenied]) { - errorCode = kGIDSignInErrorCodeCanceled; + if (!deferredToEMMHandler) { + authFlow.error = [self authorizationErrorWithParameters:params isEMMError:NO]; } - - authFlow.error = [self errorWithString:errorString code:errorCode]; } } else { NSString *errorString = [error localizedDescription]; @@ -1289,6 +1283,18 @@ - (BOOL)handleDevicePolicyAppURL:(NSURL *)url { #pragma mark - Helpers +// Returns the error for an authorization response that carried no authorization code. +- (NSError *)authorizationErrorWithParameters:(NSDictionary *)params + isEMMError:(BOOL)isEMMError { + NSString *errorString = (NSString *)params[kOAuth2ErrorKeyName]; + GIDSignInErrorCode errorCode = + isEMMError ? kGIDSignInErrorCodeEMM : kGIDSignInErrorCodeUnknown; + if ([errorString isEqualToString:kOAuth2AccessDenied]) { + errorCode = kGIDSignInErrorCodeCanceled; + } + return [self errorWithString:errorString code:errorCode]; +} + - (NSError *)errorWithString:(NSString *)errorString code:(GIDSignInErrorCode)code { if (errorString == nil) { errorString = @"Unknown error"; diff --git a/GoogleSignIn/Tests/Unit/GIDEMMErrorHandlerTest.m b/GoogleSignIn/Tests/Unit/GIDEMMErrorHandlerTest.m index e73da669..33594c68 100644 --- a/GoogleSignIn/Tests/Unit/GIDEMMErrorHandlerTest.m +++ b/GoogleSignIn/Tests/Unit/GIDEMMErrorHandlerTest.m @@ -35,6 +35,25 @@ NS_ASSUME_NONNULL_BEGIN +// Records how, and whether, a GIDEMMErrorHandler completion was called. +@interface GIDEMMCompletionSpy : NSObject +@property(nonatomic, readonly) NSInteger callCount; +@property(nonatomic, readonly) BOOL handled; +// A completion to pass to -handleErrorFromResponse:completion:. +- (void (^)(BOOL handled))completion; +@end + +@implementation GIDEMMCompletionSpy + +- (void (^)(BOOL handled))completion { + return ^(BOOL handled) { + self->_callCount++; + self->_handled = handled; + }; +} + +@end + // Unit test for GIDEMMErrorHandler. @interface GIDEMMErrorHandlerTest : XCTestCase @end @@ -49,6 +68,9 @@ @implementation GIDEMMErrorHandlerTest { - (void)setUp { [super setUp]; + // GIDEMMErrorHandler is a singleton; a test that fails before dismissing its dialog would + // otherwise leave the pending-dialog flag set and break every test after it. + [[GIDEMMErrorHandler sharedInstance] setValue:@NO forKey:@"pendingDialog"]; _keyWindowSet = NO; _presentedViewController = nil; UIWindow *fakeKeyWindow = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; @@ -143,13 +165,13 @@ - (void)expectOpenURLString:(NSString *)urlString inAction:(void (^)(void))actio // Verifies that the handler doesn't handle non-exist error. - (void)testNoError { - __block BOOL completionCalled = NO; - BOOL result = [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:@{ @"abc" : @123 } - completion:^() { - completionCalled = YES; - }]; - XCTAssertFalse(result); - XCTAssertTrue(completionCalled); + GIDEMMCompletionSpy *spy = [[GIDEMMCompletionSpy alloc] init]; + [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:@{ @"abc" : @123 } + completion:spy.completion]; + XCTAssertEqual(spy.callCount, 0); + [self waitForMainQueue]; + XCTAssertEqual(spy.callCount, 1); + XCTAssertFalse(spy.handled); XCTAssertFalse(_keyWindowSet); XCTAssertNil(_presentedViewController); } @@ -161,76 +183,120 @@ - (void)testNoError { - (void)testNonStringErrorValue { NSArray *nonStringValues = @[ @123, @[ @"emm_passcode_required" ], @{ @"a" : @"b" } ]; for (id nonStringValue in nonStringValues) { - __block BOOL completionCalled = NO; + GIDEMMCompletionSpy *spy = [[GIDEMMCompletionSpy alloc] init]; NSDictionary *response = @{ @"error" : nonStringValue }; - BOOL result = [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response - completion:^() { - completionCalled = YES; - }]; - XCTAssertFalse(result); - XCTAssertTrue(completionCalled); + [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response + completion:spy.completion]; + XCTAssertEqual(spy.callCount, 0); + [self waitForMainQueue]; + XCTAssertEqual(spy.callCount, 1); + XCTAssertFalse(spy.handled); } } // Verifies that the handler doesn't handle non-EMM error. - (void)testNoEMMError { - __block BOOL completionCalled = NO; + GIDEMMCompletionSpy *spy = [[GIDEMMCompletionSpy alloc] init]; NSDictionary *response = @{ @"error" : @"invalid_token" }; - BOOL result = [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response - completion:^() { - completionCalled = YES; - }]; - XCTAssertFalse(result); - XCTAssertTrue(completionCalled); + [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response + completion:spy.completion]; + XCTAssertEqual(spy.callCount, 0); + [self waitForMainQueue]; + XCTAssertEqual(spy.callCount, 1); + XCTAssertFalse(spy.handled); XCTAssertFalse(_keyWindowSet); XCTAssertNil(_presentedViewController); } // Verifies that the handler handles general EMM error with user tapping 'OK'. - (void)testGeneralEMMErrorOK { - __block BOOL completionCalled = NO; + GIDEMMCompletionSpy *spy = [[GIDEMMCompletionSpy alloc] init]; NSDictionary *response = @{ @"error" : @"emm_something_wrong" }; - BOOL result = [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response - completion:^() { - completionCalled = YES; - }]; - XCTAssertTrue(result); - XCTAssertFalse(completionCalled); + [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response + completion:spy.completion]; + // The dialog is presented asynchronously on the main queue, so nothing has happened yet. + XCTAssertEqual(spy.callCount, 0); XCTAssertFalse(_keyWindowSet); XCTAssertNil(_presentedViewController); - // Should handle no more error while the previous one is being handled. - __block BOOL secondCompletionCalled = NO; - BOOL secondResult = [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response - completion:^() { - secondCompletionCalled = YES; - }]; - XCTAssertFalse(secondResult); - XCTAssertTrue(secondCompletionCalled); + // A second EMM error while the first dialog is pending is reported as handled, but must not + // present a second dialog. + GIDEMMCompletionSpy *secondSpy = [[GIDEMMCompletionSpy alloc] init]; + [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response + completion:secondSpy.completion]; + XCTAssertEqual(secondSpy.callCount, 0); XCTAssertFalse(_keyWindowSet); XCTAssertNil(_presentedViewController); [self waitForMainQueue]; + XCTAssertEqual(secondSpy.callCount, 1); + XCTAssertTrue(secondSpy.handled); XCTAssertTrue(_keyWindowSet); UIAlertController *alert = [self presentedAlert]; if (!alert) return; [self assertAlert:alert hasActionTitles:@[ @"OK" ]]; - XCTAssertFalse(completionCalled); + XCTAssertEqual(spy.callCount, 0); + + [self tapActionTitled:@"OK" inAlert:alert]; + XCTAssertEqual(spy.callCount, 1); + XCTAssertTrue(spy.handled); +} + +// Verifies that the pending-dialog flag is cleared when there is no key window, so a later +// EMM error can still present its dialog. `GIDEMMErrorHandler` is a process-wide singleton, +// so before this fix a single windowless error suppressed every dialog that followed. +- (void)testNoKeyWindow_ClearsPendingDialogForNextError { + [GULSwizzler unswizzleClass:[GIDEMMErrorHandler class] + selector:@selector(keyWindow) + isClassSelector:NO]; + [GULSwizzler swizzleClass:[GIDEMMErrorHandler class] + selector:@selector(keyWindow) + isClassSelector:NO + withBlock:^() { return nil; }]; + + GIDEMMCompletionSpy *firstSpy = [[GIDEMMCompletionSpy alloc] init]; + NSDictionary *response = @{ @"error" : @"emm_something_wrong" }; + [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response + completion:firstSpy.completion]; + + [self waitForMainQueue]; + XCTAssertEqual(firstSpy.callCount, 1); + XCTAssertTrue(firstSpy.handled); + XCTAssertNil(_presentedViewController); + + // Restore a working key window. + [GULSwizzler unswizzleClass:[GIDEMMErrorHandler class] + selector:@selector(keyWindow) + isClassSelector:NO]; + UIWindow *fakeKeyWindow = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; + [GULSwizzler swizzleClass:[GIDEMMErrorHandler class] + selector:@selector(keyWindow) + isClassSelector:NO + withBlock:^() { return fakeKeyWindow; }]; + + GIDEMMCompletionSpy *secondSpy = [[GIDEMMCompletionSpy alloc] init]; + [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response + completion:secondSpy.completion]; + // Before the fix, this completed immediately with NO: the pending-dialog flag was still YES. + XCTAssertEqual(secondSpy.callCount, 0); + [self waitForMainQueue]; + UIAlertController *alert = [self presentedAlert]; + if (!alert) return; + [self assertAlert:alert hasActionTitles:@[ @"OK" ]]; [self tapActionTitled:@"OK" inAlert:alert]; - XCTAssertTrue(completionCalled); + XCTAssertEqual(secondSpy.callCount, 1); + XCTAssertTrue(secondSpy.handled); } // Verifies that the handler handles EMM screenlock required error with user tapping 'Cancel'. - (void)testScreenlockRequiredCancel { - __block BOOL completionCalled = NO; + GIDEMMCompletionSpy *spy = [[GIDEMMCompletionSpy alloc] init]; NSDictionary *response = @{ @"error" : @"emm_passcode_required" }; - BOOL result = [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response - completion:^() { - completionCalled = YES; - }]; - XCTAssertTrue(result); - XCTAssertFalse(completionCalled); + [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response + completion:spy.completion]; + // The dialog is presented asynchronously on the main queue, so nothing has happened yet. + XCTAssertEqual(spy.callCount, 0); XCTAssertFalse(_keyWindowSet); XCTAssertNil(_presentedViewController); @@ -239,22 +305,21 @@ - (void)testScreenlockRequiredCancel { UIAlertController *alert = [self presentedAlert]; if (!alert) return; [self assertAlert:alert hasActionTitles:@[ @"Cancel", @"Settings" ]]; - XCTAssertFalse(completionCalled); + XCTAssertEqual(spy.callCount, 0); [self tapActionTitled:@"Cancel" inAlert:alert]; - XCTAssertTrue(completionCalled); + XCTAssertEqual(spy.callCount, 1); + XCTAssertTrue(spy.handled); } // Verifies that the handler handles EMM screenlock required error with user tapping 'Settings'. - (void)testScreenlockRequiredSettings { - __block BOOL completionCalled = NO; + GIDEMMCompletionSpy *spy = [[GIDEMMCompletionSpy alloc] init]; NSDictionary *response = @{ @"error" : @"emm_passcode_required" }; - BOOL result = [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response - completion:^() { - completionCalled = YES; - }]; - XCTAssertTrue(result); - XCTAssertFalse(completionCalled); + [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response + completion:spy.completion]; + // The dialog is presented asynchronously on the main queue, so nothing has happened yet. + XCTAssertEqual(spy.callCount, 0); XCTAssertFalse(_keyWindowSet); XCTAssertNil(_presentedViewController); @@ -263,24 +328,23 @@ - (void)testScreenlockRequiredSettings { UIAlertController *alert = [self presentedAlert]; if (!alert) return; [self assertAlert:alert hasActionTitles:@[ @"Cancel", @"Settings" ]]; - XCTAssertFalse(completionCalled); + XCTAssertEqual(spy.callCount, 0); [self expectOpenURLString:UIApplicationOpenSettingsURLString inAction:^() { [self tapActionTitled:@"Settings" inAlert:alert]; }]; - XCTAssertTrue(completionCalled); + XCTAssertEqual(spy.callCount, 1); + XCTAssertTrue(spy.handled); } // Verifies that the handler handles EMM app verification required error without a URL. - (void)testAppVerificationNoURL { - __block BOOL completionCalled = NO; + GIDEMMCompletionSpy *spy = [[GIDEMMCompletionSpy alloc] init]; NSDictionary *response = @{ @"error" : @"emm_app_verification_required" }; - BOOL result = [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response - completion:^() { - completionCalled = YES; - }]; - XCTAssertTrue(result); - XCTAssertFalse(completionCalled); + [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response + completion:spy.completion]; + // The dialog is presented asynchronously on the main queue, so nothing has happened yet. + XCTAssertEqual(spy.callCount, 0); XCTAssertFalse(_keyWindowSet); XCTAssertNil(_presentedViewController); @@ -289,24 +353,22 @@ - (void)testAppVerificationNoURL { UIAlertController *alert = [self presentedAlert]; if (!alert) return; [self assertAlert:alert hasActionTitles:@[ @"OK" ]]; - XCTAssertFalse(completionCalled); + XCTAssertEqual(spy.callCount, 0); [self tapActionTitled:@"OK" inAlert:alert]; - XCTAssertTrue(completionCalled); + XCTAssertEqual(spy.callCount, 1); + XCTAssertTrue(spy.handled); } - // Verifies that the handler handles EMM app verification required error user tapping 'Cancel'. - (void)testAppVerificationCancel { - __block BOOL completionCalled = NO; + GIDEMMCompletionSpy *spy = [[GIDEMMCompletionSpy alloc] init]; NSDictionary *response = @{ @"error" : @"emm_app_verification_required: https://host.domain/path" }; - BOOL result = [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response - completion:^() { - completionCalled = YES; - }]; - XCTAssertTrue(result); - XCTAssertFalse(completionCalled); + [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response + completion:spy.completion]; + // The dialog is presented asynchronously on the main queue, so nothing has happened yet. + XCTAssertEqual(spy.callCount, 0); XCTAssertFalse(_keyWindowSet); XCTAssertNil(_presentedViewController); @@ -315,23 +377,22 @@ - (void)testAppVerificationCancel { UIAlertController *alert = [self presentedAlert]; if (!alert) return; [self assertAlert:alert hasActionTitles:@[ @"Cancel", @"Connect" ]]; - XCTAssertFalse(completionCalled); + XCTAssertEqual(spy.callCount, 0); [self tapActionTitled:@"Cancel" inAlert:alert]; - XCTAssertTrue(completionCalled); + XCTAssertEqual(spy.callCount, 1); + XCTAssertTrue(spy.handled); } // Verifies that the handler handles EMM app verification required error user tapping 'Connect'. - (void)testAppVerificationConnect { - __block BOOL completionCalled = NO; + GIDEMMCompletionSpy *spy = [[GIDEMMCompletionSpy alloc] init]; NSDictionary *response = @{ @"error" : @"emm_app_verification_required: https://host.domain/path" }; - BOOL result = [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response - completion:^() { - completionCalled = YES; - }]; - XCTAssertTrue(result); - XCTAssertFalse(completionCalled); + [[GIDEMMErrorHandler sharedInstance] handleErrorFromResponse:response + completion:spy.completion]; + // The dialog is presented asynchronously on the main queue, so nothing has happened yet. + XCTAssertEqual(spy.callCount, 0); XCTAssertFalse(_keyWindowSet); XCTAssertNil(_presentedViewController); @@ -340,12 +401,13 @@ - (void)testAppVerificationConnect { UIAlertController *alert = [self presentedAlert]; if (!alert) return; [self assertAlert:alert hasActionTitles:@[ @"Cancel", @"Connect" ]]; - XCTAssertFalse(completionCalled); + XCTAssertEqual(spy.callCount, 0); [self expectOpenURLString:@"https://host.domain/path" inAction:^() { [self tapActionTitled:@"Connect" inAlert:alert]; }]; - XCTAssertTrue(completionCalled); + XCTAssertEqual(spy.callCount, 1); + XCTAssertTrue(spy.handled); } // Verifies that the handler can handle sequential errors independently. diff --git a/GoogleSignIn/Tests/Unit/GIDEMMSupportTest.m b/GoogleSignIn/Tests/Unit/GIDEMMSupportTest.m index c780c11f..7f0e44dc 100644 --- a/GoogleSignIn/Tests/Unit/GIDEMMSupportTest.m +++ b/GoogleSignIn/Tests/Unit/GIDEMMSupportTest.m @@ -220,8 +220,8 @@ - (void)testHandleTokenFetchEMMError_errorIsEMM { userInfo:@{ OIDOAuthErrorResponseErrorKey : errorJSON }]; id mockEMMErrorHandler = OCMStrictClassMock([GIDEMMErrorHandler class]); [[[mockEMMErrorHandler stub] andReturn:mockEMMErrorHandler] sharedInstance]; - __block void (^savedCompletion)(void); - [[[mockEMMErrorHandler stub] andReturnValue:@YES] + __block void (^savedCompletion)(BOOL); + [[mockEMMErrorHandler stub] handleErrorFromResponse:errorJSON completion:[OCMArg checkWithBlock:^(id arg) { savedCompletion = arg; return YES; @@ -239,7 +239,7 @@ - (void)testHandleTokenFetchEMMError_errorIsEMM { }]; [self waitForExpectations:@[ notCalled ] timeout:1]; - savedCompletion(); + savedCompletion(YES); [self waitForExpectations:@[ called ] timeout:1]; } @@ -251,8 +251,8 @@ - (void)testHandleTokenFetchEMMError_errorIsNotEMM { userInfo:@{ OIDOAuthErrorResponseErrorKey : errorJSON }]; id mockEMMErrorHandler = OCMStrictClassMock([GIDEMMErrorHandler class]); [[[mockEMMErrorHandler stub] andReturn:mockEMMErrorHandler] sharedInstance]; - __block void (^savedCompletion)(void); - [[[mockEMMErrorHandler stub] andReturnValue:@NO] + __block void (^savedCompletion)(BOOL); + [[mockEMMErrorHandler stub] handleErrorFromResponse:errorJSON completion:[OCMArg checkWithBlock:^(id arg) { savedCompletion = arg; return YES; @@ -270,7 +270,7 @@ - (void)testHandleTokenFetchEMMError_errorIsNotEMM { }]; [self waitForExpectations:@[ notCalled ] timeout:1]; - savedCompletion(); + savedCompletion(NO); [self waitForExpectations:@[ called ] timeout:1]; } diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index ed959f12..630cd484 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -1819,9 +1819,9 @@ - (void)testAuthEndpointEMMError { id mockEMMErrorHandler = OCMStrictClassMock([GIDEMMErrorHandler class]); [[[mockEMMErrorHandler stub] andReturn:mockEMMErrorHandler] sharedInstance]; - __block void (^completion)(void); + __block void (^completion)(BOOL handled); NSDictionary *callbackParams = @{ @"error" : @"EMM Specific Error" }; - [[[mockEMMErrorHandler expect] andReturnValue:@YES] + [[mockEMMErrorHandler expect] handleErrorFromResponse:callbackParams completion:SAVE_TO_ARG_BLOCK(completion)]; @@ -1837,7 +1837,7 @@ - (void)testAuthEndpointEMMError { [mockEMMErrorHandler verify]; [mockEMMErrorHandler stopMocking]; - completion(); + completion(YES); [self waitForExpectationsWithTimeout:1 handler:nil]; @@ -1849,6 +1849,78 @@ - (void)testAuthEndpointEMMError { XCTAssertNil(_signIn.currentUser, @"should not have current user"); } +// Verifies that an authorization error the EMM handler declines is reported as unknown. +- (void)testAuthEndpointEMMError_NotHandled { + if (!_isEligibleForEMM) { + return; + } + + id mockEMMErrorHandler = OCMStrictClassMock([GIDEMMErrorHandler class]); + [[[mockEMMErrorHandler stub] andReturn:mockEMMErrorHandler] sharedInstance]; + __block void (^completion)(BOOL handled); + NSDictionary *callbackParams = @{ @"error" : @"invalid_request" }; + [[mockEMMErrorHandler expect] + handleErrorFromResponse:callbackParams completion:SAVE_TO_ARG_BLOCK(completion)]; + + [self OAuthLoginWithAddScopesFlow:NO + authError:callbackParams[@"error"] + tokenError:nil + emmPasscodeInfoRequired:NO + claimsAsJSONRequired:NO + keychainError:NO + restoredSignIn:NO + oldAccessToken:NO + modalCancel:NO]; + + [mockEMMErrorHandler verify]; + [mockEMMErrorHandler stopMocking]; + completion(NO); + + [self waitForExpectationsWithTimeout:1 handler:nil]; + + XCTAssertFalse(_keychainSaved, @"should not save to keychain"); + XCTAssertTrue(_completionCalled, @"should call delegate"); + XCTAssertNotNil(_authError, @"should have error"); + XCTAssertEqualObjects(_authError.domain, kGIDSignInErrorDomain); + XCTAssertEqual(_authError.code, kGIDSignInErrorCodeUnknown); + XCTAssertNil(_signIn.currentUser, @"should not have current user"); +} + +// Verifies that access_denied is reported as canceled even when the EMM handler claims it. +- (void)testAuthEndpointEMMError_AccessDeniedTakesPriority { + if (!_isEligibleForEMM) { + return; + } + + id mockEMMErrorHandler = OCMStrictClassMock([GIDEMMErrorHandler class]); + [[[mockEMMErrorHandler stub] andReturn:mockEMMErrorHandler] sharedInstance]; + __block void (^completion)(BOOL handled); + NSDictionary *callbackParams = @{ @"error" : @"access_denied" }; + [[mockEMMErrorHandler expect] + handleErrorFromResponse:callbackParams completion:SAVE_TO_ARG_BLOCK(completion)]; + + [self OAuthLoginWithAddScopesFlow:NO + authError:callbackParams[@"error"] + tokenError:nil + emmPasscodeInfoRequired:NO + claimsAsJSONRequired:NO + keychainError:NO + restoredSignIn:NO + oldAccessToken:NO + modalCancel:NO]; + + [mockEMMErrorHandler verify]; + [mockEMMErrorHandler stopMocking]; + completion(YES); + + [self waitForExpectationsWithTimeout:1 handler:nil]; + + XCTAssertTrue(_completionCalled, @"should call delegate"); + XCTAssertEqualObjects(_authError.domain, kGIDSignInErrorDomain); + XCTAssertEqual(_authError.code, kGIDSignInErrorCodeCanceled); + XCTAssertNil(_signIn.currentUser, @"should not have current user"); +} + - (void)testTokenEndpointEMMError { if (!_isEligibleForEMM) { return;