From 30699dff05984dfc4d5f710c6d151f5c3a86a707 Mon Sep 17 00:00:00 2001 From: pinlu Date: Mon, 19 Sep 2022 17:55:05 -0700 Subject: [PATCH 01/17] Add KVO in GIDGoogleUser Send KVO notifications when authState updates tokens. --- GoogleSignIn/Sources/GIDGoogleUser.m | 57 +++++++++++++++------------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index b03bd55d..6e0242c4 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -102,36 +102,14 @@ - (GIDConfiguration *)configuration { } - (GIDToken *)accessToken { - @synchronized(self) { - if (!_cachedAccessToken) { - _cachedAccessToken = [[GIDToken alloc] initWithTokenString:_authState.lastTokenResponse.accessToken - expirationDate:_authState.lastTokenResponse. - accessTokenExpirationDate]; - } - } return _cachedAccessToken; } - (GIDToken *)refreshToken { - @synchronized(self) { - if (!_cachedRefreshToken) { - _cachedRefreshToken = [[GIDToken alloc] initWithTokenString:_authState.refreshToken - expirationDate:nil]; - } - } return _cachedRefreshToken; } - (nullable GIDToken *)idToken { - @synchronized(self) { - NSString *idTokenString = _authState.lastTokenResponse.idToken; - if (!_cachedIdToken && idTokenString) { - NSDate *idTokenExpirationDate = [[[OIDIDToken alloc] - initWithIDTokenString:idTokenString] expiresAt]; - _cachedIdToken = [[GIDToken alloc] initWithTokenString:idTokenString - expirationDate:idTokenExpirationDate]; - } - } return _cachedIdToken; } @@ -153,10 +131,25 @@ - (void)updateAuthState:(OIDAuthState *)authState _authentication = [[GIDAuthentication alloc] initWithAuthState:authState]; _profile = profileData; - // These three tokens will be generated in the getter and cached . - _cachedAccessToken = nil; - _cachedRefreshToken = nil; - _cachedIdToken = nil; + [self sendKVONotificationsBeforeChanges]; + [self updateTokensWithAuthState:authState]; + [self sendKVONotificationAfterChanges]; + } +} + +- (void)updateTokensWithAuthState:(OIDAuthState *)authState { + _cachedAccessToken = [[GIDToken alloc] initWithTokenString:authState.lastTokenResponse.accessToken + expirationDate:authState.lastTokenResponse. + accessTokenExpirationDate]; + _cachedRefreshToken = [[GIDToken alloc] initWithTokenString:authState.refreshToken + expirationDate:nil]; + _cachedIdToken = nil; + NSString *idTokenString = authState.lastTokenResponse.idToken; + if (idTokenString) { + NSDate *idTokenExpirationDate = [[[OIDIDToken alloc] + initWithIDTokenString:idTokenString] expiresAt]; + _cachedIdToken = [[GIDToken alloc] initWithTokenString:idTokenString + expirationDate:idTokenExpirationDate]; } } @@ -173,6 +166,18 @@ - (nullable NSString *)hostedDomain { return nil; } +- (void)sendKVONotificationsBeforeChanges { + [self willChangeValueForKey:NSStringFromSelector(@selector(accessToken))]; + [self willChangeValueForKey:NSStringFromSelector(@selector(refreshToken))]; + [self willChangeValueForKey:NSStringFromSelector(@selector(idToken))]; +} + +- (void)sendKVONotificationAfterChanges { + [self didChangeValueForKey:NSStringFromSelector(@selector(accessToken))]; + [self didChangeValueForKey:NSStringFromSelector(@selector(refreshToken))]; + [self didChangeValueForKey:NSStringFromSelector(@selector(idToken))]; +} + #pragma mark - NSSecureCoding + (BOOL)supportsSecureCoding { From 9ac54783eb4f3c2daa67b4e3dbe0aeb84ce931f7 Mon Sep 17 00:00:00 2001 From: pinlu Date: Mon, 19 Sep 2022 18:14:36 -0700 Subject: [PATCH 02/17] Observe KVO in GIDGoogleUserTest Uses bit mask to verify KVO notifications are sent. --- GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 105 ++++++++++++++++++-- 1 file changed, 98 insertions(+), 7 deletions(-) diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index 2ff9e567..9b2e5ddc 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -41,10 +41,45 @@ // It should be larger than kTimeAccuracy which is used in the method `XCTAssertEqualWithAccuracy`. static NSTimeInterval const kTimeIncrement = 100; +// List of observed properties of the class being tested. +static NSString *const kObservedProperties[] = { + @"accessToken", + @"refreshToken", + @"idToken", +}; +static const NSUInteger kNumberOfObservedProperties = + sizeof(kObservedProperties) / sizeof(*kObservedProperties); + +// Bit position for notification change type bitmask flags. +// Must match the list of observed properties above. +typedef NS_ENUM(NSUInteger, ChangeType) { + kChangeTypeAccessTokenPrior, + kChangeTypeAccessToken, + kChangeTypeRefreshTokenPrior, + kChangeTypeRefreshToken, + kChangeTypeIDTokenPrior, + kChangeTypeIDToken, + kChangeTypeEnd // not a real change type but an end mark for calculating |kChangeAll| +}; + +static const NSUInteger kChangeAll = (1u << kChangeTypeEnd) - 1u; + +#if __has_feature(c_static_assert) || __has_extension(c_static_assert) +_Static_assert(kChangeTypeEnd == (sizeof(kObservedProperties) / sizeof(*kObservedProperties)) * 2, + "List of observed properties must match list of change notification enums"); +#endif + @interface GIDGoogleUserTest : XCTestCase @end -@implementation GIDGoogleUserTest +@implementation GIDGoogleUserTest { + // Bitmask flags for observed changes, as specified in |ChangeType|. + NSUInteger _changesObserved; +} + +- (void)setUp { + _changesObserved = 0; +} #pragma mark - Tests @@ -102,12 +137,8 @@ - (void)testUpdateAuthState { NSTimeInterval accessTokenExpireTime = [NSDate timeIntervalSinceReferenceDate]; NSTimeInterval idTokenExpireTime = accessTokenExpireTime + kTimeIncrement; - NSString *idToken = [self idTokenWithExpireTime:idTokenExpireTime]; - OIDAuthState *authState = [OIDAuthState testInstanceWithIDToken:idToken - accessToken:kAccessToken - accessTokenExpireTime:accessTokenExpireTime]; - - GIDGoogleUser *user = [[GIDGoogleUser alloc] initWithAuthState:authState profileData:nil]; + GIDGoogleUser *user = [self observedGoogleUserWithAccessTokenExpireTime:accessTokenExpireTime + idTokenExpireTime:idTokenExpireTime]; NSTimeInterval updatedAccessTokenExpireTime = idTokenExpireTime + kTimeIncrement; NSTimeInterval updatedIDTokenExpireTime = updatedAccessTokenExpireTime + kTimeIncrement; @@ -126,12 +157,72 @@ - (void)testUpdateAuthState { XCTAssertEqualWithAccuracy([user.idToken.expirationDate timeIntervalSinceReferenceDate], updatedIDTokenExpireTime, kTimeAccuracy); XCTAssertEqual(user.profile, updatedProfileData); + XCTAssertEqual(_changesObserved, kChangeAll); } #pragma mark - Helpers +- (GIDGoogleUser *)observedGoogleUserWithAccessTokenExpireTime:(NSTimeInterval)accessTokenExpireTime + idTokenExpireTime:(NSTimeInterval)idTokenExpireTime { + GIDGoogleUser *user = [self googleUserWithAccessTokenExpireTime:accessTokenExpireTime + idTokenExpireTime:idTokenExpireTime]; + for (unsigned int i = 0; i < kNumberOfObservedProperties; ++i) { + [user addObserver:self + forKeyPath:kObservedProperties[i] + options:NSKeyValueObservingOptionPrior + context:NULL]; + } + return user; +} + +- (GIDGoogleUser *)googleUserWithAccessTokenExpireTime:(NSTimeInterval)accessTokenExpireTime + idTokenExpireTime:(NSTimeInterval)idTokenExpireTime { + + NSString *idToken = [self idTokenWithExpireTime:idTokenExpireTime]; + OIDAuthState *authState = [OIDAuthState testInstanceWithIDToken:idToken + accessToken:kAccessToken + accessTokenExpireTime:accessTokenExpireTime]; + + return [[GIDGoogleUser alloc] initWithAuthState:authState profileData:nil]; +} + - (NSString *)idTokenWithExpireTime:(NSTimeInterval)expireTime { return [OIDTokenResponse idTokenWithSub:kUserID exp:@(expireTime + NSTimeIntervalSince1970)]; } +#pragma mark - NSKeyValueObserving + +- (void)observeValueForKeyPath:(NSString *)keyPath + ofObject:(id)object + change:(NSDictionary *)change + context:(void *)context { + ChangeType changeType; + if ([keyPath isEqualToString:@"accessToken"]) { + if (change[NSKeyValueChangeNotificationIsPriorKey]) { + changeType = kChangeTypeAccessTokenPrior; + } else { + changeType = kChangeTypeAccessToken; + } + } else if ([keyPath isEqualToString:@"refreshToken"]) { + if (change[NSKeyValueChangeNotificationIsPriorKey]) { + changeType = kChangeTypeRefreshTokenPrior; + } else { + changeType = kChangeTypeRefreshToken; + } + } else if ([keyPath isEqualToString:@"idToken"]) { + if (change[NSKeyValueChangeNotificationIsPriorKey]) { + changeType = kChangeTypeIDTokenPrior; + } else { + changeType = kChangeTypeIDToken; + } + } else { + XCTFail(@"unexpected keyPath"); + return; // so compiler knows |changeType| is always assigned + } + + NSInteger changeMask = 1 << changeType; + XCTAssertFalse(_changesObserved & changeMask); // each change type should only fire once + _changesObserved |= changeMask; +} + @end From 26784fe5ac207be94d4f9b71ebdc476f9bb49bad Mon Sep 17 00:00:00 2001 From: pinlu Date: Thu, 22 Sep 2022 11:07:26 -0700 Subject: [PATCH 03/17] Utilize the class extension and automatic KVO notifications --- GoogleSignIn/Sources/GIDGoogleUser.m | 58 ++++++++++------------------ 1 file changed, 20 insertions(+), 38 deletions(-) diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index 6e0242c4..7e394441 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -42,12 +42,19 @@ NS_ASSUME_NONNULL_BEGIN +@interface GIDGoogleUser() + +@property(nonatomic, readwrite) GIDToken *accessToken; + +@property(nonatomic, readwrite) GIDToken *refreshToken; + +@property(nonatomic, readwrite, nullable) GIDToken *idToken; + +@end + @implementation GIDGoogleUser { OIDAuthState *_authState; GIDConfiguration *_cachedConfiguration; - GIDToken *_cachedAccessToken; - GIDToken *_cachedRefreshToken; - GIDToken *_cachedIdToken; } - (nullable NSString *)userID { @@ -59,7 +66,6 @@ - (nullable NSString *)userID { return [idTokenDecoded.subject copy]; } } - return nil; } @@ -97,22 +103,9 @@ - (GIDConfiguration *)configuration { openIDRealm:openIDRealm]; }; } - return _cachedConfiguration; } -- (GIDToken *)accessToken { - return _cachedAccessToken; -} - -- (GIDToken *)refreshToken { - return _cachedRefreshToken; -} - -- (nullable GIDToken *)idToken { - return _cachedIdToken; -} - #pragma mark - Private Methods - (instancetype)initWithAuthState:(OIDAuthState *)authState @@ -131,25 +124,26 @@ - (void)updateAuthState:(OIDAuthState *)authState _authentication = [[GIDAuthentication alloc] initWithAuthState:authState]; _profile = profileData; - [self sendKVONotificationsBeforeChanges]; [self updateTokensWithAuthState:authState]; - [self sendKVONotificationAfterChanges]; } } - (void)updateTokensWithAuthState:(OIDAuthState *)authState { - _cachedAccessToken = [[GIDToken alloc] initWithTokenString:authState.lastTokenResponse.accessToken - expirationDate:authState.lastTokenResponse. - accessTokenExpirationDate]; - _cachedRefreshToken = [[GIDToken alloc] initWithTokenString:authState.refreshToken - expirationDate:nil]; - _cachedIdToken = nil; + self.accessToken = [[GIDToken alloc] initWithTokenString:authState.lastTokenResponse.accessToken + expirationDate:authState.lastTokenResponse. + accessTokenExpirationDate]; + + self.refreshToken = [[GIDToken alloc] initWithTokenString:authState.refreshToken + expirationDate:nil]; + NSString *idTokenString = authState.lastTokenResponse.idToken; if (idTokenString) { NSDate *idTokenExpirationDate = [[[OIDIDToken alloc] initWithIDTokenString:idTokenString] expiresAt]; - _cachedIdToken = [[GIDToken alloc] initWithTokenString:idTokenString + self.idToken = [[GIDToken alloc] initWithTokenString:idTokenString expirationDate:idTokenExpirationDate]; + } else { + self.idToken = nil; } } @@ -166,18 +160,6 @@ - (nullable NSString *)hostedDomain { return nil; } -- (void)sendKVONotificationsBeforeChanges { - [self willChangeValueForKey:NSStringFromSelector(@selector(accessToken))]; - [self willChangeValueForKey:NSStringFromSelector(@selector(refreshToken))]; - [self willChangeValueForKey:NSStringFromSelector(@selector(idToken))]; -} - -- (void)sendKVONotificationAfterChanges { - [self didChangeValueForKey:NSStringFromSelector(@selector(accessToken))]; - [self didChangeValueForKey:NSStringFromSelector(@selector(refreshToken))]; - [self didChangeValueForKey:NSStringFromSelector(@selector(idToken))]; -} - #pragma mark - NSSecureCoding + (BOOL)supportsSecureCoding { From 6e5d21adfa5e29158a387aed0b6c31e4fdfb0ae1 Mon Sep 17 00:00:00 2001 From: pinlu Date: Thu, 22 Sep 2022 13:36:03 -0700 Subject: [PATCH 04/17] Only update tokens if necessary --- GoogleSignIn/Sources/GIDGoogleUser.m | 20 ++++++++++++++----- .../Tests/Unit/GIDAuthenticationTest.m | 2 ++ GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 9 +++++++-- GoogleSignIn/Tests/Unit/GIDSignInTest.m | 1 + .../Tests/Unit/OIDAuthState+Testing.h | 3 ++- .../Tests/Unit/OIDAuthState+Testing.m | 4 +++- .../Tests/Unit/OIDTokenResponse+Testing.h | 1 + .../Tests/Unit/OIDTokenResponse+Testing.m | 4 +++- 8 files changed, 34 insertions(+), 10 deletions(-) diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index 7e394441..979a164f 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -42,7 +42,7 @@ NS_ASSUME_NONNULL_BEGIN -@interface GIDGoogleUser() +@interface GIDGoogleUser () @property(nonatomic, readwrite) GIDToken *accessToken; @@ -129,21 +129,31 @@ - (void)updateAuthState:(OIDAuthState *)authState } - (void)updateTokensWithAuthState:(OIDAuthState *)authState { - self.accessToken = [[GIDToken alloc] initWithTokenString:authState.lastTokenResponse.accessToken + GIDToken *accessToken = [[GIDToken alloc] initWithTokenString:authState.lastTokenResponse.accessToken expirationDate:authState.lastTokenResponse. accessTokenExpirationDate]; + if (![self.accessToken isEqualToToken:accessToken]) { + self.accessToken = accessToken; + } - self.refreshToken = [[GIDToken alloc] initWithTokenString:authState.refreshToken + GIDToken *refreshToken = [[GIDToken alloc] initWithTokenString:authState.refreshToken expirationDate:nil]; + if (![self.refreshToken isEqualToToken:refreshToken]) { + self.refreshToken = refreshToken; + } + GIDToken *idToken; NSString *idTokenString = authState.lastTokenResponse.idToken; if (idTokenString) { NSDate *idTokenExpirationDate = [[[OIDIDToken alloc] initWithIDTokenString:idTokenString] expiresAt]; - self.idToken = [[GIDToken alloc] initWithTokenString:idTokenString + idToken = [[GIDToken alloc] initWithTokenString:idTokenString expirationDate:idTokenExpirationDate]; } else { - self.idToken = nil; + idToken = nil; + } + if ((self.idToken || idToken) && ![self.idToken isEqualToToken:idToken]) { + self.idToken = idToken; } } diff --git a/GoogleSignIn/Tests/Unit/GIDAuthenticationTest.m b/GoogleSignIn/Tests/Unit/GIDAuthenticationTest.m index ad57bead..769d3235 100644 --- a/GoogleSignIn/Tests/Unit/GIDAuthenticationTest.m +++ b/GoogleSignIn/Tests/Unit/GIDAuthenticationTest.m @@ -561,6 +561,7 @@ - (GIDAuthentication *)auth { [OIDTokenResponse testInstanceWithIDToken:idToken accessToken:kAccessToken expiresIn:accessTokenExpiresIn + refreshToken:kRefreshToken tokenRequest:tokenRequest]; return [[GIDAuthentication alloc] initWithAuthState:[OIDAuthState testInstanceWithTokenResponse:tokenResponse]]; @@ -599,6 +600,7 @@ - (OIDTokenResponse *)tokenResponseWithNewTokens { return [OIDTokenResponse testInstanceWithIDToken:(_hasIDToken ? [self idTokenNew] : nil) accessToken:kNewAccessToken expiresIn:expiresIn + refreshToken:kRefreshToken tokenRequest:_tokenRequest ?: nil]; } diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index 9b2e5ddc..56e0815b 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -36,6 +36,8 @@ #endif static NSString *const kNewAccessToken = @"new_access_token"; +static NSString *const kNewRefreshToken = @"new_refresh_token"; + static NSTimeInterval const kTimeAccuracy = 10; // The difference between times. // It should be larger than kTimeAccuracy which is used in the method `XCTAssertEqualWithAccuracy`. @@ -145,7 +147,8 @@ - (void)testUpdateAuthState { NSString *updatedIDToken = [self idTokenWithExpireTime:updatedIDTokenExpireTime]; OIDAuthState *updatedAuthState = [OIDAuthState testInstanceWithIDToken:updatedIDToken accessToken:kNewAccessToken - accessTokenExpireTime:updatedAccessTokenExpireTime]; + accessTokenExpireTime:updatedAccessTokenExpireTime + refreshToken:kNewRefreshToken]; GIDProfileData *updatedProfileData = [GIDProfileData testInstance]; [user updateAuthState:updatedAuthState profileData:updatedProfileData]; @@ -156,6 +159,7 @@ - (void)testUpdateAuthState { XCTAssertEqualObjects(user.idToken.tokenString, updatedIDToken); XCTAssertEqualWithAccuracy([user.idToken.expirationDate timeIntervalSinceReferenceDate], updatedIDTokenExpireTime, kTimeAccuracy); + XCTAssertEqualObjects(user.refreshToken.tokenString, kNewRefreshToken); XCTAssertEqual(user.profile, updatedProfileData); XCTAssertEqual(_changesObserved, kChangeAll); } @@ -181,7 +185,8 @@ - (GIDGoogleUser *)googleUserWithAccessTokenExpireTime:(NSTimeInterval)accessTok NSString *idToken = [self idTokenWithExpireTime:idTokenExpireTime]; OIDAuthState *authState = [OIDAuthState testInstanceWithIDToken:idToken accessToken:kAccessToken - accessTokenExpireTime:accessTokenExpireTime]; + accessTokenExpireTime:accessTokenExpireTime + refreshToken:kRefreshToken]; return [[GIDGoogleUser alloc] initWithAuthState:authState profileData:nil]; } diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index 5d9dac77..fdcd1027 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -1202,6 +1202,7 @@ - (void)OAuthLoginWithAddScopesFlow:(BOOL)addScopesFlow [OIDTokenResponse testInstanceWithIDToken:[OIDTokenResponse fatIDToken] accessToken:restoredSignIn ? kAccessToken : nil expiresIn:oldAccessToken ? @(300) : nil + refreshToken:kRefreshToken tokenRequest:nil]; OIDTokenRequest *tokenRequest = [[OIDTokenRequest alloc] diff --git a/GoogleSignIn/Tests/Unit/OIDAuthState+Testing.h b/GoogleSignIn/Tests/Unit/OIDAuthState+Testing.h index b255352f..f660e34e 100644 --- a/GoogleSignIn/Tests/Unit/OIDAuthState+Testing.h +++ b/GoogleSignIn/Tests/Unit/OIDAuthState+Testing.h @@ -30,6 +30,7 @@ + (instancetype)testInstanceWithIDToken:(NSString *)idToken accessToken:(NSString *)accessToken - accessTokenExpireTime:(NSTimeInterval)accessTokenExpireTime; + accessTokenExpireTime:(NSTimeInterval)accessTokenExpireTime + refreshToken:(NSString *)refreshToken; @end diff --git a/GoogleSignIn/Tests/Unit/OIDAuthState+Testing.m b/GoogleSignIn/Tests/Unit/OIDAuthState+Testing.m index 6c019820..a9f7c49c 100644 --- a/GoogleSignIn/Tests/Unit/OIDAuthState+Testing.m +++ b/GoogleSignIn/Tests/Unit/OIDAuthState+Testing.m @@ -35,13 +35,15 @@ + (instancetype)testInstanceWithTokenResponse:(OIDTokenResponse *)tokenResponse + (instancetype)testInstanceWithIDToken:(NSString *)idToken accessToken:(NSString *)accessToken - accessTokenExpireTime:(NSTimeInterval)accessTokenExpireTime { + accessTokenExpireTime:(NSTimeInterval)accessTokenExpireTime + refreshToken:(NSString *)refreshToken { NSNumber *accessTokenExpiresIn = @(accessTokenExpireTime - [[NSDate date] timeIntervalSinceReferenceDate]); OIDTokenResponse *newResponse = [OIDTokenResponse testInstanceWithIDToken:idToken accessToken:accessToken expiresIn:accessTokenExpiresIn + refreshToken:refreshToken tokenRequest:nil]; return [self testInstanceWithTokenResponse:newResponse]; } diff --git a/GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.h b/GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.h index 990fda0f..bd26a66a 100644 --- a/GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.h +++ b/GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.h @@ -56,6 +56,7 @@ extern NSString * const kFatPictureURL; + (instancetype)testInstanceWithIDToken:(NSString *)idToken accessToken:(NSString *)accessToken expiresIn:(NSNumber *)expiresIn + refreshToken:(NSString *)refreshToken tokenRequest:(OIDTokenRequest *)tokenRequest; + (NSString *)idToken; diff --git a/GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.m b/GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.m index 1e1b95bf..3f3df182 100644 --- a/GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.m +++ b/GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.m @@ -61,19 +61,21 @@ + (instancetype)testInstanceWithIDToken:(NSString *)idToken { return [OIDTokenResponse testInstanceWithIDToken:idToken accessToken:nil expiresIn:nil + refreshToken:kRefreshToken tokenRequest:nil]; } + (instancetype)testInstanceWithIDToken:(NSString *)idToken accessToken:(NSString *)accessToken expiresIn:(NSNumber *)expiresIn + refreshToken:(NSString *)refreshToken tokenRequest:(OIDTokenRequest *)tokenRequest { NSMutableDictionary *parameters; parameters = [[NSMutableDictionary alloc] initWithDictionary:@{ @"access_token" : accessToken ?: kAccessToken, @"expires_in" : expiresIn ?: @(kAccessTokenExpiresIn), @"token_type" : @"example_token_type", - @"refresh_token" : kRefreshToken, + @"refresh_token" : refreshToken, @"scope" : [OIDScopeUtilities scopesWithArray:@[ OIDAuthorizationRequestTestingScope2 ]], @"server_code" : kServerAuthCode, }]; From d45e398d697f2cffed73d1eb06b2cb4ceef781dc Mon Sep 17 00:00:00 2001 From: pinlu Date: Thu, 22 Sep 2022 14:25:29 -0700 Subject: [PATCH 05/17] Remove the KVO testing Since we use NSObject automatic change notifications we can remove the tests for manual notifications emission. --- GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 91 +------------------ .../Tests/Unit/OIDTokenResponse+Testing.m | 4 +- 2 files changed, 5 insertions(+), 90 deletions(-) diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index 999d617d..aa326d35 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -43,45 +43,10 @@ // It should be larger than kTimeAccuracy which is used in the method `XCTAssertEqualWithAccuracy`. static NSTimeInterval const kTimeIncrement = 100; -// List of observed properties of the class being tested. -static NSString *const kObservedProperties[] = { - @"accessToken", - @"refreshToken", - @"idToken", -}; -static const NSUInteger kNumberOfObservedProperties = - sizeof(kObservedProperties) / sizeof(*kObservedProperties); - -// Bit position for notification change type bitmask flags. -// Must match the list of observed properties above. -typedef NS_ENUM(NSUInteger, ChangeType) { - kChangeTypeAccessTokenPrior, - kChangeTypeAccessToken, - kChangeTypeRefreshTokenPrior, - kChangeTypeRefreshToken, - kChangeTypeIDTokenPrior, - kChangeTypeIDToken, - kChangeTypeEnd // not a real change type but an end mark for calculating |kChangeAll| -}; - -static const NSUInteger kChangeAll = (1u << kChangeTypeEnd) - 1u; - -#if __has_feature(c_static_assert) || __has_extension(c_static_assert) -_Static_assert(kChangeTypeEnd == (sizeof(kObservedProperties) / sizeof(*kObservedProperties)) * 2, - "List of observed properties must match list of change notification enums"); -#endif - @interface GIDGoogleUserTest : XCTestCase @end -@implementation GIDGoogleUserTest { - // Bitmask flags for observed changes, as specified in |ChangeType|. - NSUInteger _changesObserved; -} - -- (void)setUp { - _changesObserved = 0; -} +@implementation GIDGoogleUserTest #pragma mark - Tests @@ -139,8 +104,8 @@ - (void)testUpdateAuthState { NSTimeInterval accessTokenExpireTime = [[NSDate date] timeIntervalSince1970]; NSTimeInterval idTokenExpireTime = accessTokenExpireTime + kTimeIncrement; - GIDGoogleUser *user = [self observedGoogleUserWithAccessTokenExpireTime:accessTokenExpireTime - idTokenExpireTime:idTokenExpireTime]; + GIDGoogleUser *user = [self googleUserWithAccessTokenExpireTime:accessTokenExpireTime + idTokenExpireTime:idTokenExpireTime]; NSTimeInterval updatedAccessTokenExpireTime = idTokenExpireTime + kTimeIncrement; NSTimeInterval updatedIDTokenExpireTime = updatedAccessTokenExpireTime + kTimeIncrement; @@ -161,27 +126,12 @@ - (void)testUpdateAuthState { updatedIDTokenExpireTime, kTimeAccuracy); XCTAssertEqualObjects(user.refreshToken.tokenString, kNewRefreshToken); XCTAssertEqual(user.profile, updatedProfileData); - XCTAssertEqual(_changesObserved, kChangeAll); } #pragma mark - Helpers -- (GIDGoogleUser *)observedGoogleUserWithAccessTokenExpireTime:(NSTimeInterval)accessTokenExpireTime - idTokenExpireTime:(NSTimeInterval)idTokenExpireTime { - GIDGoogleUser *user = [self googleUserWithAccessTokenExpireTime:accessTokenExpireTime - idTokenExpireTime:idTokenExpireTime]; - for (unsigned int i = 0; i < kNumberOfObservedProperties; ++i) { - [user addObserver:self - forKeyPath:kObservedProperties[i] - options:NSKeyValueObservingOptionPrior - context:NULL]; - } - return user; -} - - (GIDGoogleUser *)googleUserWithAccessTokenExpireTime:(NSTimeInterval)accessTokenExpireTime idTokenExpireTime:(NSTimeInterval)idTokenExpireTime { - NSString *idToken = [self idTokenWithExpireTime:idTokenExpireTime]; OIDAuthState *authState = [OIDAuthState testInstanceWithIDToken:idToken accessToken:kAccessToken @@ -196,39 +146,4 @@ - (NSString *)idTokenWithExpireTime:(NSTimeInterval)expireTime { return [OIDTokenResponse idTokenWithSub:kUserID exp:@(expireTime)]; } -#pragma mark - NSKeyValueObserving - -- (void)observeValueForKeyPath:(NSString *)keyPath - ofObject:(id)object - change:(NSDictionary *)change - context:(void *)context { - ChangeType changeType; - if ([keyPath isEqualToString:@"accessToken"]) { - if (change[NSKeyValueChangeNotificationIsPriorKey]) { - changeType = kChangeTypeAccessTokenPrior; - } else { - changeType = kChangeTypeAccessToken; - } - } else if ([keyPath isEqualToString:@"refreshToken"]) { - if (change[NSKeyValueChangeNotificationIsPriorKey]) { - changeType = kChangeTypeRefreshTokenPrior; - } else { - changeType = kChangeTypeRefreshToken; - } - } else if ([keyPath isEqualToString:@"idToken"]) { - if (change[NSKeyValueChangeNotificationIsPriorKey]) { - changeType = kChangeTypeIDTokenPrior; - } else { - changeType = kChangeTypeIDToken; - } - } else { - XCTFail(@"unexpected keyPath"); - return; // so compiler knows |changeType| is always assigned - } - - NSInteger changeMask = 1 << changeType; - XCTAssertFalse(_changesObserved & changeMask); // each change type should only fire once - _changesObserved |= changeMask; -} - @end diff --git a/GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.m b/GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.m index 3f3df182..49823be7 100644 --- a/GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.m +++ b/GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.m @@ -61,7 +61,7 @@ + (instancetype)testInstanceWithIDToken:(NSString *)idToken { return [OIDTokenResponse testInstanceWithIDToken:idToken accessToken:nil expiresIn:nil - refreshToken:kRefreshToken + refreshToken:nil tokenRequest:nil]; } @@ -75,7 +75,7 @@ + (instancetype)testInstanceWithIDToken:(NSString *)idToken @"access_token" : accessToken ?: kAccessToken, @"expires_in" : expiresIn ?: @(kAccessTokenExpiresIn), @"token_type" : @"example_token_type", - @"refresh_token" : refreshToken, + @"refresh_token" : refreshToken ?: kRefreshToken, @"scope" : [OIDScopeUtilities scopesWithArray:@[ OIDAuthorizationRequestTestingScope2 ]], @"server_code" : kServerAuthCode, }]; From a56786b6024cdb1edb31a432245d6d15c74a15b5 Mon Sep 17 00:00:00 2001 From: pinlu Date: Fri, 23 Sep 2022 15:10:51 -0700 Subject: [PATCH 06/17] Test tokens unchanged Added a new test case `testUpdateAuthStateWithUnchangedRefreshTokenAndIDToken ` --- GoogleSignIn/Sources/GIDGoogleUser.m | 14 ++++++------ GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 25 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index 979a164f..cb215f7f 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -129,15 +129,15 @@ - (void)updateAuthState:(OIDAuthState *)authState } - (void)updateTokensWithAuthState:(OIDAuthState *)authState { - GIDToken *accessToken = [[GIDToken alloc] initWithTokenString:authState.lastTokenResponse.accessToken - expirationDate:authState.lastTokenResponse. - accessTokenExpirationDate]; + GIDToken *accessToken = + [[GIDToken alloc] initWithTokenString:authState.lastTokenResponse.accessToken + expirationDate:authState.lastTokenResponse.accessTokenExpirationDate]; if (![self.accessToken isEqualToToken:accessToken]) { self.accessToken = accessToken; } GIDToken *refreshToken = [[GIDToken alloc] initWithTokenString:authState.refreshToken - expirationDate:nil]; + expirationDate:nil]; if (![self.refreshToken isEqualToToken:refreshToken]) { self.refreshToken = refreshToken; } @@ -145,10 +145,10 @@ - (void)updateTokensWithAuthState:(OIDAuthState *)authState { GIDToken *idToken; NSString *idTokenString = authState.lastTokenResponse.idToken; if (idTokenString) { - NSDate *idTokenExpirationDate = [[[OIDIDToken alloc] - initWithIDTokenString:idTokenString] expiresAt]; + NSDate *idTokenExpirationDate = + [[[OIDIDToken alloc] initWithIDTokenString:idTokenString] expiresAt]; idToken = [[GIDToken alloc] initWithTokenString:idTokenString - expirationDate:idTokenExpirationDate]; + expirationDate:idTokenExpirationDate]; } else { idToken = nil; } diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index aa326d35..f356057b 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -128,6 +128,31 @@ - (void)testUpdateAuthState { XCTAssertEqual(user.profile, updatedProfileData); } +- (void)testUpdateAuthStateWithUnchangedRefreshTokenAndIDToken { + NSTimeInterval accessTokenExpireTime = [[NSDate date] timeIntervalSince1970]; + NSTimeInterval idTokenExpireTime = [[NSDate date] timeIntervalSince1970]; + + GIDGoogleUser *user = [self googleUserWithAccessTokenExpireTime:accessTokenExpireTime + idTokenExpireTime:idTokenExpireTime]; + + GIDToken *accessTokenBeforeUpdate = user.accessToken; + GIDToken *refreshTokenBeforeUpdate = user.refreshToken; + GIDToken *idTokenBeforeUpdate = user.idToken; + + NSString *updatedIDToken = [self idTokenWithExpireTime:idTokenExpireTime]; + OIDAuthState *updatedAuthState = [OIDAuthState testInstanceWithIDToken:updatedIDToken + accessToken:kNewAccessToken + accessTokenExpireTime:accessTokenExpireTime + refreshToken:kRefreshToken]; + GIDProfileData *updatedProfileData = [GIDProfileData testInstance]; + + [user updateAuthState:updatedAuthState profileData:updatedProfileData]; + + XCTAssertIdentical(user.idToken, idTokenBeforeUpdate); + XCTAssertIdentical(user.refreshToken, refreshTokenBeforeUpdate); + XCTAssertNotIdentical(user.accessToken, accessTokenBeforeUpdate); +} + #pragma mark - Helpers - (GIDGoogleUser *)googleUserWithAccessTokenExpireTime:(NSTimeInterval)accessTokenExpireTime From 58d2acf457f4ad6454ed034d2c8ad794bc7ed432 Mon Sep 17 00:00:00 2001 From: pinlu Date: Mon, 26 Sep 2022 01:12:34 -0700 Subject: [PATCH 07/17] Move fetcherAuthorizer from GIDAuthentication to GIDGoogleUser --- GoogleSignIn/Sources/GIDGoogleUser.m | 152 +++++++++++++++++- GoogleSignIn/Sources/GIDGoogleUser_Private.h | 15 +- .../Public/GoogleSignIn/GIDGoogleUser.h | 14 ++ 3 files changed, 179 insertions(+), 2 deletions(-) diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index cb215f7f..034a49be 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -25,7 +25,14 @@ #ifdef SWIFT_PACKAGE @import AppAuth; #else -#import +#import +#import +#import +#import +#import +#import +#import +#import #endif // The ID Token claim key for the hosted domain value. @@ -40,8 +47,111 @@ static NSString *const kAudienceParameter = @"audience"; static NSString *const kOpenIDRealmParameter = @"openid.realm"; +// Additional parameter names for EMM. +static NSString *const kEMMSupportParameterName = @"emm_support"; + NS_ASSUME_NONNULL_BEGIN +#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST + +// The specialized GTMAppAuthFetcherAuthorization delegate that handles potential EMM error +// responses. +@interface GTMAppAuthFetcherAuthorizationEMMChainedDelegate : NSObject + +// Initializes with chained delegate and selector. +- (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector; + +// The callback method for GTMAppAuthFetcherAuthorization to invoke. +- (void)authentication:(GTMAppAuthFetcherAuthorization *)auth + request:(NSMutableURLRequest *)request + finishedWithError:(nullable NSError *)error; + +@end + +@implementation GTMAppAuthFetcherAuthorizationEMMChainedDelegate { + // We use a weak reference here to match GTMAppAuthFetcherAuthorization. + __weak id _delegate; + SEL _selector; + // We need to maintain a reference to the chained delegate because GTMAppAuthFetcherAuthorization + // only keeps a weak reference. + GTMAppAuthFetcherAuthorizationEMMChainedDelegate *_retained_self; +} + +- (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector { + self = [super init]; + if (self) { + _delegate = delegate; + _selector = selector; + _retained_self = self; + } + return self; +} + +- (void)authentication:(GTMAppAuthFetcherAuthorization *)auth + request:(NSMutableURLRequest *)request + finishedWithError:(nullable NSError *)error { + [GIDAuthentication handleTokenFetchEMMError:error completion:^(NSError *_Nullable error) { + if (!self->_delegate || !self->_selector) { + return; + } + NSMethodSignature *signature = [self->_delegate methodSignatureForSelector:self->_selector]; + if (!signature) { + return; + } + id argument1 = auth; + id argument2 = request; + id argument3 = error; + NSInvocation *invocation = [NSInvocation invocationWithMethodSignature:signature]; + [invocation setTarget:self->_delegate]; // index 0 + [invocation setSelector:self->_selector]; // index 1 + [invocation setArgument:&argument1 atIndex:2]; + [invocation setArgument:&argument2 atIndex:3]; + [invocation setArgument:&argument3 atIndex:4]; + [invocation invoke]; + }]; + // Prepare to deallocate the chained delegate instance because the above block will retain the + // iVar references it uses. + _retained_self = nil; +} + +@end + +// A specialized GTMAppAuthFetcherAuthorization subclass with EMM support. +@interface GTMAppAuthFetcherAuthorizationWithEMMSupport : GTMAppAuthFetcherAuthorization +@end + +@implementation GTMAppAuthFetcherAuthorizationWithEMMSupport + +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-implementations" +- (void)authorizeRequest:(nullable NSMutableURLRequest *)request + delegate:(id)delegate + didFinishSelector:(SEL)sel { +#pragma clang diagnostic pop + GTMAppAuthFetcherAuthorizationEMMChainedDelegate *chainedDelegate = + [[GTMAppAuthFetcherAuthorizationEMMChainedDelegate alloc] initWithDelegate:delegate + selector:sel]; +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + [super authorizeRequest:request + delegate:chainedDelegate + didFinishSelector:@selector(authentication:request:finishedWithError:)]; +#pragma clang diagnostic pop +} + +- (void)authorizeRequest:(nullable NSMutableURLRequest *)request + completionHandler:(GTMAppAuthFetcherAuthorizationCompletion)handler { + [super authorizeRequest:request completionHandler:^(NSError *_Nullable error) { + [GIDAuthentication handleTokenFetchEMMError:error completion:^(NSError *_Nullable error) { + handler(error); + }]; + }]; +} + +@end + +#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST + @interface GIDGoogleUser () @property(nonatomic, readwrite) GIDToken *accessToken; @@ -55,6 +165,10 @@ @interface GIDGoogleUser () @implementation GIDGoogleUser { OIDAuthState *_authState; GIDConfiguration *_cachedConfiguration; + + // A queue for pending authentication handlers so we don't fire multiple requests in parallel. + // Access to this ivar should be synchronized. + NSMutableArray *_authenticationHandlerQueue; } - (nullable NSString *)userID { @@ -106,12 +220,36 @@ - (GIDConfiguration *)configuration { return _cachedConfiguration; } +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" +- (id)fetcherAuthorizer { +#pragma clang diagnostic pop +#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST + GTMAppAuthFetcherAuthorization *authorization = self.emmSupport ? + [[GTMAppAuthFetcherAuthorizationWithEMMSupport alloc] initWithAuthState:_authState] : + [[GTMAppAuthFetcherAuthorization alloc] initWithAuthState:_authState]; +#elif TARGET_OS_OSX || TARGET_OS_MACCATALYST + GTMAppAuthFetcherAuthorization *authorization = + [[GTMAppAuthFetcherAuthorization alloc] initWithAuthState:_authState]; +#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST + authorization.tokenRefreshDelegate = self; + return authorization; +} + #pragma mark - Private Methods +#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST +- (NSString *)emmSupport { + return + _authState.lastAuthorizationResponse.request.additionalParameters[kEMMSupportParameterName]; +} +#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST + - (instancetype)initWithAuthState:(OIDAuthState *)authState profileData:(nullable GIDProfileData *)profileData { self = [super init]; if (self) { + _authenticationHandlerQueue = [[NSMutableArray alloc] init]; [self updateAuthState:authState profileData:profileData]; } return self; @@ -170,6 +308,18 @@ - (nullable NSString *)hostedDomain { return nil; } +#pragma mark - GTMAppAuthFetcherAuthorizationTokenRefreshDelegate + +- (nullable NSDictionary *)additionalRefreshParameters: + (GTMAppAuthFetcherAuthorization *)authorization { +#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST + return [GIDAuthentication updatedEMMParametersWithParameters: + authorization.authState.lastTokenResponse.request.additionalParameters]; +#elif TARGET_OS_OSX || TARGET_OS_MACCATALYST + return authorization.authState.lastTokenResponse.request.additionalParameters; +#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST +} + #pragma mark - NSSecureCoding + (BOOL)supportsSecureCoding { diff --git a/GoogleSignIn/Sources/GIDGoogleUser_Private.h b/GoogleSignIn/Sources/GIDGoogleUser_Private.h index c6a1fc84..46b85168 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser_Private.h +++ b/GoogleSignIn/Sources/GIDGoogleUser_Private.h @@ -16,12 +16,25 @@ #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h" +#ifdef SWIFT_PACKAGE +@import AppAuth; +@import GTMAppAuth; +#else +#import +#import +#endif + NS_ASSUME_NONNULL_BEGIN @class OIDAuthState; // Internal methods for the class that are not part of the public API. -@interface GIDGoogleUser () +@interface GIDGoogleUser () + +#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST +// A string indicating support for Enterprise Mobility Management. +@property(nonatomic, readonly) NSString *emmSupport; +#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST // Create a object with an auth state, scopes, and profile data. - (instancetype)initWithAuthState:(OIDAuthState *)authState diff --git a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h index 1d3f09c6..bc7ef324 100644 --- a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h +++ b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h @@ -18,6 +18,14 @@ NS_ASSUME_NONNULL_BEGIN +// We have to import GTMAppAuth because forward declaring the protocol does +// not generate the `fetcherAuthorizer` method below for Swift. +#ifdef SWIFT_PACKAGE +@import GTMAppAuth; +#else +#import +#endif + @class GIDAuthentication; @class GIDConfiguration; @class GIDToken; @@ -53,6 +61,12 @@ NS_ASSUME_NONNULL_BEGIN /// see https://developers.google.com/identity/sign-in/ios/backend-auth. @property(nonatomic, readonly, nullable) GIDToken *idToken; +/// A new authorizer for `GTLService`, `GTMSessionFetcher`, or `GTMHTTPFetcher`. +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" +@property(nonatomic, readonly) (id)fetcherAuthorizer; +#pragma clang diagnostic pop + @end NS_ASSUME_NONNULL_END From 3d2921240dee8ccfb7d8a9d8792c93e347827571 Mon Sep 17 00:00:00 2001 From: pinlu Date: Mon, 26 Sep 2022 16:30:27 -0700 Subject: [PATCH 08/17] Move `fetcherAuthorizer` into GIDGoogleUser API --- GoogleSignIn/Sources/GIDAuthentication.m | 137 ------------ .../Sources/GIDAuthentication_Private.h | 7 +- GoogleSignIn/Sources/GIDGoogleUser.m | 210 +++++++++--------- .../Public/GoogleSignIn/GIDAuthentication.h | 8 - .../Public/GoogleSignIn/GIDGoogleUser.h | 6 +- .../Tests/Unit/GIDAuthenticationTest.m | 13 -- GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 14 ++ 7 files changed, 119 insertions(+), 276 deletions(-) diff --git a/GoogleSignIn/Sources/GIDAuthentication.m b/GoogleSignIn/Sources/GIDAuthentication.m index 7965db1e..927be7a1 100644 --- a/GoogleSignIn/Sources/GIDAuthentication.m +++ b/GoogleSignIn/Sources/GIDAuthentication.m @@ -57,106 +57,6 @@ // New UIDevice system name for iOS. static NSString *const kNewIOSSystemName = @"iOS"; -#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST - -// The specialized GTMAppAuthFetcherAuthorization delegate that handles potential EMM error -// responses. -@interface GTMAppAuthFetcherAuthorizationEMMChainedDelegate : NSObject - -// Initializes with chained delegate and selector. -- (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector; - -// The callback method for GTMAppAuthFetcherAuthorization to invoke. -- (void)authentication:(GTMAppAuthFetcherAuthorization *)auth - request:(NSMutableURLRequest *)request - finishedWithError:(nullable NSError *)error; - -@end - -@implementation GTMAppAuthFetcherAuthorizationEMMChainedDelegate { - // We use a weak reference here to match GTMAppAuthFetcherAuthorization. - __weak id _delegate; - SEL _selector; - // We need to maintain a reference to the chained delegate because GTMAppAuthFetcherAuthorization - // only keeps a weak reference. - GTMAppAuthFetcherAuthorizationEMMChainedDelegate *_retained_self; -} - -- (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector { - self = [super init]; - if (self) { - _delegate = delegate; - _selector = selector; - _retained_self = self; - } - return self; -} - -- (void)authentication:(GTMAppAuthFetcherAuthorization *)auth - request:(NSMutableURLRequest *)request - finishedWithError:(nullable NSError *)error { - [GIDAuthentication handleTokenFetchEMMError:error completion:^(NSError *_Nullable error) { - if (!self->_delegate || !self->_selector) { - return; - } - NSMethodSignature *signature = [self->_delegate methodSignatureForSelector:self->_selector]; - if (!signature) { - return; - } - id argument1 = auth; - id argument2 = request; - id argument3 = error; - NSInvocation *invocation = [NSInvocation invocationWithMethodSignature:signature]; - [invocation setTarget:self->_delegate]; // index 0 - [invocation setSelector:self->_selector]; // index 1 - [invocation setArgument:&argument1 atIndex:2]; - [invocation setArgument:&argument2 atIndex:3]; - [invocation setArgument:&argument3 atIndex:4]; - [invocation invoke]; - }]; - // Prepare to deallocate the chained delegate instance because the above block will retain the - // iVar references it uses. - _retained_self = nil; -} - -@end - -// A specialized GTMAppAuthFetcherAuthorization subclass with EMM support. -@interface GTMAppAuthFetcherAuthorizationWithEMMSupport : GTMAppAuthFetcherAuthorization -@end - -@implementation GTMAppAuthFetcherAuthorizationWithEMMSupport - -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wdeprecated-implementations" -- (void)authorizeRequest:(nullable NSMutableURLRequest *)request - delegate:(id)delegate - didFinishSelector:(SEL)sel { -#pragma clang diagnostic pop - GTMAppAuthFetcherAuthorizationEMMChainedDelegate *chainedDelegate = - [[GTMAppAuthFetcherAuthorizationEMMChainedDelegate alloc] initWithDelegate:delegate - selector:sel]; -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wdeprecated-declarations" - [super authorizeRequest:request - delegate:chainedDelegate - didFinishSelector:@selector(authentication:request:finishedWithError:)]; -#pragma clang diagnostic pop -} - -- (void)authorizeRequest:(nullable NSMutableURLRequest *)request - completionHandler:(GTMAppAuthFetcherAuthorizationCompletion)handler { - [super authorizeRequest:request completionHandler:^(NSError *_Nullable error) { - [GIDAuthentication handleTokenFetchEMMError:error completion:^(NSError *_Nullable error) { - handler(error); - }]; - }]; -} - -@end - -#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST - @implementation GIDAuthentication { // A queue for pending authentication handlers so we don't fire multiple requests in parallel. // Access to this ivar should be synchronized. @@ -201,33 +101,8 @@ - (nullable NSDate *)idTokenExpirationDate { return [[[OIDIDToken alloc] initWithIDTokenString:self.idToken] expiresAt]; } -#pragma mark - Private property accessors - -#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST -- (NSString *)emmSupport { - return - _authState.lastAuthorizationResponse.request.additionalParameters[kEMMSupportParameterName]; -} -#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST - #pragma mark - Public methods -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wdeprecated-declarations" -- (id)fetcherAuthorizer { -#pragma clang diagnostic pop -#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST - GTMAppAuthFetcherAuthorization *authorization = self.emmSupport ? - [[GTMAppAuthFetcherAuthorizationWithEMMSupport alloc] initWithAuthState:_authState] : - [[GTMAppAuthFetcherAuthorization alloc] initWithAuthState:_authState]; -#elif TARGET_OS_OSX || TARGET_OS_MACCATALYST - GTMAppAuthFetcherAuthorization *authorization = - [[GTMAppAuthFetcherAuthorization alloc] initWithAuthState:_authState]; -#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST - authorization.tokenRefreshDelegate = self; - return authorization; -} - - (void)doWithFreshTokens:(GIDAuthenticationCompletion)completion { if (!([self.accessTokenExpirationDate timeIntervalSinceNow] < kMinimalTimeToExpire || (self.idToken && [self.idTokenExpirationDate timeIntervalSinceNow] < kMinimalTimeToExpire))) { @@ -360,18 +235,6 @@ + (void)handleTokenFetchEMMError:(nullable NSError *)error #endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST -#pragma mark - GTMAppAuthFetcherAuthorizationTokenRefreshDelegate - -- (nullable NSDictionary *)additionalRefreshParameters: - (GTMAppAuthFetcherAuthorization *)authorization { -#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST - return [GIDAuthentication updatedEMMParametersWithParameters: - authorization.authState.lastTokenResponse.request.additionalParameters]; -#elif TARGET_OS_OSX || TARGET_OS_MACCATALYST - return authorization.authState.lastTokenResponse.request.additionalParameters; -#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST -} - #pragma mark - NSSecureCoding + (BOOL)supportsSecureCoding { diff --git a/GoogleSignIn/Sources/GIDAuthentication_Private.h b/GoogleSignIn/Sources/GIDAuthentication_Private.h index c967a0da..37245aa4 100644 --- a/GoogleSignIn/Sources/GIDAuthentication_Private.h +++ b/GoogleSignIn/Sources/GIDAuthentication_Private.h @@ -27,16 +27,11 @@ NS_ASSUME_NONNULL_BEGIN // Internal methods for the class that are not part of the public API. -@interface GIDAuthentication () +@interface GIDAuthentication () // A representation of the state of the OAuth session for this instance. @property(nonatomic, readonly) OIDAuthState *authState; -#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST -// A string indicating support for Enterprise Mobility Management. -@property(nonatomic, readonly) NSString *emmSupport; -#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST - - (instancetype)initWithAuthState:(OIDAuthState *)authState; #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index 034a49be..3bd78c09 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -25,14 +25,7 @@ #ifdef SWIFT_PACKAGE @import AppAuth; #else -#import -#import -#import -#import -#import -#import -#import -#import +#import #endif // The ID Token claim key for the hosted domain value. @@ -52,106 +45,10 @@ NS_ASSUME_NONNULL_BEGIN -#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST - -// The specialized GTMAppAuthFetcherAuthorization delegate that handles potential EMM error -// responses. -@interface GTMAppAuthFetcherAuthorizationEMMChainedDelegate : NSObject - -// Initializes with chained delegate and selector. -- (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector; - -// The callback method for GTMAppAuthFetcherAuthorization to invoke. -- (void)authentication:(GTMAppAuthFetcherAuthorization *)auth - request:(NSMutableURLRequest *)request - finishedWithError:(nullable NSError *)error; - -@end - -@implementation GTMAppAuthFetcherAuthorizationEMMChainedDelegate { - // We use a weak reference here to match GTMAppAuthFetcherAuthorization. - __weak id _delegate; - SEL _selector; - // We need to maintain a reference to the chained delegate because GTMAppAuthFetcherAuthorization - // only keeps a weak reference. - GTMAppAuthFetcherAuthorizationEMMChainedDelegate *_retained_self; -} - -- (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector { - self = [super init]; - if (self) { - _delegate = delegate; - _selector = selector; - _retained_self = self; - } - return self; -} - -- (void)authentication:(GTMAppAuthFetcherAuthorization *)auth - request:(NSMutableURLRequest *)request - finishedWithError:(nullable NSError *)error { - [GIDAuthentication handleTokenFetchEMMError:error completion:^(NSError *_Nullable error) { - if (!self->_delegate || !self->_selector) { - return; - } - NSMethodSignature *signature = [self->_delegate methodSignatureForSelector:self->_selector]; - if (!signature) { - return; - } - id argument1 = auth; - id argument2 = request; - id argument3 = error; - NSInvocation *invocation = [NSInvocation invocationWithMethodSignature:signature]; - [invocation setTarget:self->_delegate]; // index 0 - [invocation setSelector:self->_selector]; // index 1 - [invocation setArgument:&argument1 atIndex:2]; - [invocation setArgument:&argument2 atIndex:3]; - [invocation setArgument:&argument3 atIndex:4]; - [invocation invoke]; - }]; - // Prepare to deallocate the chained delegate instance because the above block will retain the - // iVar references it uses. - _retained_self = nil; -} - -@end - // A specialized GTMAppAuthFetcherAuthorization subclass with EMM support. @interface GTMAppAuthFetcherAuthorizationWithEMMSupport : GTMAppAuthFetcherAuthorization @end -@implementation GTMAppAuthFetcherAuthorizationWithEMMSupport - -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wdeprecated-implementations" -- (void)authorizeRequest:(nullable NSMutableURLRequest *)request - delegate:(id)delegate - didFinishSelector:(SEL)sel { -#pragma clang diagnostic pop - GTMAppAuthFetcherAuthorizationEMMChainedDelegate *chainedDelegate = - [[GTMAppAuthFetcherAuthorizationEMMChainedDelegate alloc] initWithDelegate:delegate - selector:sel]; -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wdeprecated-declarations" - [super authorizeRequest:request - delegate:chainedDelegate - didFinishSelector:@selector(authentication:request:finishedWithError:)]; -#pragma clang diagnostic pop -} - -- (void)authorizeRequest:(nullable NSMutableURLRequest *)request - completionHandler:(GTMAppAuthFetcherAuthorizationCompletion)handler { - [super authorizeRequest:request completionHandler:^(NSError *_Nullable error) { - [GIDAuthentication handleTokenFetchEMMError:error completion:^(NSError *_Nullable error) { - handler(error); - }]; - }]; -} - -@end - -#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST - @interface GIDGoogleUser () @property(nonatomic, readwrite) GIDToken *accessToken; @@ -165,10 +62,6 @@ @interface GIDGoogleUser () @implementation GIDGoogleUser { OIDAuthState *_authState; GIDConfiguration *_cachedConfiguration; - - // A queue for pending authentication handlers so we don't fire multiple requests in parallel. - // Access to this ivar should be synchronized. - NSMutableArray *_authenticationHandlerQueue; } - (nullable NSString *)userID { @@ -249,7 +142,6 @@ - (instancetype)initWithAuthState:(OIDAuthState *)authState profileData:(nullable GIDProfileData *)profileData { self = [super init]; if (self) { - _authenticationHandlerQueue = [[NSMutableArray alloc] init]; [self updateAuthState:authState profileData:profileData]; } return self; @@ -351,4 +243,104 @@ - (void)encodeWithCoder:(NSCoder *)encoder { @end +#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST + +#pragma mark - GTMAppAuthFetcherAuthorizationEMMChainedDelegate + +// The specialized GTMAppAuthFetcherAuthorization delegate that handles potential EMM error +// responses. +@interface GTMAppAuthFetcherAuthorizationEMMChainedDelegate : NSObject + +// Initializes with chained delegate and selector. +- (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector; + +// The callback method for GTMAppAuthFetcherAuthorization to invoke. +- (void)authentication:(GTMAppAuthFetcherAuthorization *)auth + request:(NSMutableURLRequest *)request + finishedWithError:(nullable NSError *)error; + +@end + +@implementation GTMAppAuthFetcherAuthorizationEMMChainedDelegate { + // We use a weak reference here to match GTMAppAuthFetcherAuthorization. + __weak id _delegate; + SEL _selector; + // We need to maintain a reference to the chained delegate because GTMAppAuthFetcherAuthorization + // only keeps a weak reference. + GTMAppAuthFetcherAuthorizationEMMChainedDelegate *_retained_self; +} + +- (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector { + self = [super init]; + if (self) { + _delegate = delegate; + _selector = selector; + _retained_self = self; + } + return self; +} + +- (void)authentication:(GTMAppAuthFetcherAuthorization *)auth + request:(NSMutableURLRequest *)request + finishedWithError:(nullable NSError *)error { + [GIDAuthentication handleTokenFetchEMMError:error completion:^(NSError *_Nullable error) { + if (!self->_delegate || !self->_selector) { + return; + } + NSMethodSignature *signature = [self->_delegate methodSignatureForSelector:self->_selector]; + if (!signature) { + return; + } + id argument1 = auth; + id argument2 = request; + id argument3 = error; + NSInvocation *invocation = [NSInvocation invocationWithMethodSignature:signature]; + [invocation setTarget:self->_delegate]; // index 0 + [invocation setSelector:self->_selector]; // index 1 + [invocation setArgument:&argument1 atIndex:2]; + [invocation setArgument:&argument2 atIndex:3]; + [invocation setArgument:&argument3 atIndex:4]; + [invocation invoke]; + }]; + // Prepare to deallocate the chained delegate instance because the above block will retain the + // iVar references it uses. + _retained_self = nil; +} + +@end + +#pragma mark - GTMAppAuthFetcherAuthorizationWithEMMSupport + +@implementation GTMAppAuthFetcherAuthorizationWithEMMSupport + +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-implementations" +- (void)authorizeRequest:(nullable NSMutableURLRequest *)request + delegate:(id)delegate + didFinishSelector:(SEL)sel { +#pragma clang diagnostic pop + GTMAppAuthFetcherAuthorizationEMMChainedDelegate *chainedDelegate = + [[GTMAppAuthFetcherAuthorizationEMMChainedDelegate alloc] initWithDelegate:delegate + selector:sel]; +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + [super authorizeRequest:request + delegate:chainedDelegate + didFinishSelector:@selector(authentication:request:finishedWithError:)]; +#pragma clang diagnostic pop +} + +- (void)authorizeRequest:(nullable NSMutableURLRequest *)request + completionHandler:(GTMAppAuthFetcherAuthorizationCompletion)handler { + [super authorizeRequest:request completionHandler:^(NSError *_Nullable error) { + [GIDAuthentication handleTokenFetchEMMError:error completion:^(NSError *_Nullable error) { + handler(error); + }]; + }]; +} + +@end + +#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST + NS_ASSUME_NONNULL_END diff --git a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDAuthentication.h b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDAuthentication.h index 13ae1920..cba4cac6 100644 --- a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDAuthentication.h +++ b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDAuthentication.h @@ -56,14 +56,6 @@ typedef void (^GIDAuthenticationCompletion)(GIDAuthentication *_Nullable authent /// The estimated expiration date of the ID token. @property(nonatomic, readonly, nullable) NSDate *idTokenExpirationDate; -/// Gets a new authorizer for `GTLService`, `GTMSessionFetcher`, or `GTMHTTPFetcher`. -/// -/// @return A new authorizer -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wdeprecated-declarations" -- (id)fetcherAuthorizer; -#pragma clang diagnostic pop - /// Get a valid access token and a valid ID token, refreshing them first if they have expired or are /// about to expire. /// diff --git a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h index bc7ef324..32a26440 100644 --- a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h +++ b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h @@ -16,8 +16,6 @@ #import -NS_ASSUME_NONNULL_BEGIN - // We have to import GTMAppAuth because forward declaring the protocol does // not generate the `fetcherAuthorizer` method below for Swift. #ifdef SWIFT_PACKAGE @@ -31,6 +29,8 @@ NS_ASSUME_NONNULL_BEGIN @class GIDToken; @class GIDProfileData; +NS_ASSUME_NONNULL_BEGIN + /// This class represents a user account. @interface GIDGoogleUser : NSObject @@ -64,7 +64,7 @@ NS_ASSUME_NONNULL_BEGIN /// A new authorizer for `GTLService`, `GTMSessionFetcher`, or `GTMHTTPFetcher`. #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wdeprecated-declarations" -@property(nonatomic, readonly) (id)fetcherAuthorizer; +@property(nonatomic, readonly) id fetcherAuthorizer; #pragma clang diagnostic pop @end diff --git a/GoogleSignIn/Tests/Unit/GIDAuthenticationTest.m b/GoogleSignIn/Tests/Unit/GIDAuthenticationTest.m index 970ccdd9..41c094a6 100644 --- a/GoogleSignIn/Tests/Unit/GIDAuthenticationTest.m +++ b/GoogleSignIn/Tests/Unit/GIDAuthenticationTest.m @@ -240,19 +240,6 @@ - (void)testLegacyCoding { } #endif // TARGET_OS_IOS || TARGET_OS_MACCATALYST -- (void)testFetcherAuthorizer { - // This is really hard to test without assuming how GTMAppAuthFetcherAuthorization works - // internally, so let's just take the shortcut here by asserting we get a - // GTMAppAuthFetcherAuthorization object. - GIDAuthentication *auth = [self auth]; -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wdeprecated-declarations" - id fetcherAuthroizer = auth.fetcherAuthorizer; -#pragma clang diagnostic pop - XCTAssertTrue([fetcherAuthroizer isKindOfClass:[GTMAppAuthFetcherAuthorization class]]); - XCTAssertTrue([fetcherAuthroizer canAuthorize]); -} - - (void)testDoWithFreshTokensWithBothExpired { // Both tokens expired 10 seconds ago. [self setExpireTimeForAccessToken:-10 IDToken:-10]; diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index 1199fc48..0c4f1d26 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -149,6 +149,20 @@ - (void)testUpdateAuthState_tokensAreNotChanged { XCTAssertIdentical(user.refreshToken, refreshTokenBeforeUpdate); } +- (void)testFetcherAuthorizer { + // This is really hard to test without assuming how GTMAppAuthFetcherAuthorization works + // internally, so let's just take the shortcut here by asserting we get a + // GTMAppAuthFetcherAuthorization object. + GIDGoogleUser *user = [self googleUserWithAccessTokenExpiresIn:kAccessTokenExpiresIn + idTokenExpiresIn:kIDTokenExpiresIn]; +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + id fetcherAuthroizer = user.fetcherAuthorizer; +#pragma clang diagnostic pop + XCTAssertTrue([fetcherAuthroizer isKindOfClass:[GTMAppAuthFetcherAuthorization class]]); + XCTAssertTrue([fetcherAuthroizer canAuthorize]); +} + #pragma mark - Helpers - (GIDGoogleUser *)googleUserWithAccessTokenExpiresIn:(NSTimeInterval)accessTokenExpiresIn From ca97c303ac0f563690573c5da37184e4325a9a89 Mon Sep 17 00:00:00 2001 From: pinlu Date: Wed, 28 Sep 2022 15:38:21 -0700 Subject: [PATCH 09/17] Create GIDAppAuthFetcherAuthorizationWithEMMSupport class --- ...ppAuthFetcherAuthorizationWithEMMSupport.h | 47 +++++ ...ppAuthFetcherAuthorizationWithEMMSupport.m | 192 ++++++++++++++++++ .../Sources/GIDAuthentication_Private.h | 14 -- GoogleSignIn/Sources/GIDGoogleUser.m | 112 +--------- GoogleSignIn/Sources/GIDGoogleUser_Private.h | 2 +- GoogleSignIn/Sources/GIDSignIn.m | 17 +- .../Public/GoogleSignIn/GIDGoogleUser.h | 2 +- GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 9 +- GoogleSignIn/Tests/Unit/GIDSignInTest.m | 8 +- .../ViewModels/AuthenticationViewModel.swift | 2 + 10 files changed, 269 insertions(+), 136 deletions(-) create mode 100644 GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h create mode 100644 GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m diff --git a/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h b/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h new file mode 100644 index 00000000..674a072b --- /dev/null +++ b/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h @@ -0,0 +1,47 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST + +#import + +#ifdef SWIFT_PACKAGE +@import GTMAppAuth; +#else +#import +#endif + +NS_ASSUME_NONNULL_BEGIN + +// A specialized GTMAppAuthFetcherAuthorization subclass with EMM support. +@interface GIDAppAuthFetcherAuthorizationWithEMMSupport : GTMAppAuthFetcherAuthorization + +// Handles potential EMM error from token fetch response. ++ (void)handleTokenFetchEMMError:(nullable NSError *)error + completion:(void (^)(NSError *_Nullable))completion; + +// Gets a new set of URL parameters that contains updated EMM-related URL parameters if needed. ++ (NSDictionary *)updatedEMMParametersWithParameters:(NSDictionary *)parameters; + +// Gets a new set of URL parameters that also contains EMM-related URL parameters if needed. ++ (NSDictionary *)parametersWithParameters:(NSDictionary *)parameters + emmSupport:(nullable NSString *)emmSupport + isPasscodeInfoRequired:(BOOL)isPasscodeInfoRequired; + +@end + +NS_ASSUME_NONNULL_END + +#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST diff --git a/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m b/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m new file mode 100644 index 00000000..cab44c66 --- /dev/null +++ b/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m @@ -0,0 +1,192 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST + +#import "GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h" + +#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST +#import "GoogleSignIn/Sources/GIDEMMErrorHandler.h" +#import "GoogleSignIn/Sources/GIDMDMPasscodeState.h" +#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST + +#import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h" + +#ifdef SWIFT_PACKAGE +@import AppAuth; +@import GTMAppAuth; +#else +#import +#import +#endif + +// Additional parameter names for EMM. +static NSString *const kEMMSupportParameterName = @"emm_support"; +static NSString *const kEMMOSVersionParameterName = @"device_os"; +static NSString *const kEMMPasscodeInfoParameterName = @"emm_passcode_info"; + +// Old UIDevice system name for iOS. +static NSString *const kOldIOSSystemName = @"iPhone OS"; + +// New UIDevice system name for iOS. +static NSString *const kNewIOSSystemName = @"iOS"; + +NS_ASSUME_NONNULL_BEGIN + +// The specialized GTMAppAuthFetcherAuthorization delegate that handles potential EMM error +// responses. +@interface GTMAppAuthFetcherAuthorizationEMMChainedDelegate : NSObject + +// Initializes with chained delegate and selector. +- (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector; + +// The callback method for GTMAppAuthFetcherAuthorization to invoke. +- (void)authentication:(GTMAppAuthFetcherAuthorization *)auth + request:(NSMutableURLRequest *)request + finishedWithError:(nullable NSError *)error; + +@end + +@implementation GIDAppAuthFetcherAuthorizationWithEMMSupport + +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-implementations" +- (void)authorizeRequest:(nullable NSMutableURLRequest *)request + delegate:(id)delegate + didFinishSelector:(SEL)sel { +#pragma clang diagnostic pop + GTMAppAuthFetcherAuthorizationEMMChainedDelegate *chainedDelegate = + [[GTMAppAuthFetcherAuthorizationEMMChainedDelegate alloc] initWithDelegate:delegate + selector:sel]; +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + [super authorizeRequest:request + delegate:chainedDelegate + didFinishSelector:@selector(authentication:request:finishedWithError:)]; +#pragma clang diagnostic pop +} + +- (void)authorizeRequest:(nullable NSMutableURLRequest *)request + completionHandler:(GTMAppAuthFetcherAuthorizationCompletion)handler { + [super authorizeRequest:request completionHandler:^(NSError *_Nullable error) { + [[self class] handleTokenFetchEMMError:error completion:^(NSError *_Nullable error) { + handler(error); + }]; + }]; +} + ++ (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:^() { + if (handled) { + completion([NSError errorWithDomain:kGIDSignInErrorDomain + code:kGIDSignInErrorCodeEMM + userInfo:error.userInfo]); + } else { + completion(error); + } + }]; + } else { + completion(error); + } +} + ++ (NSDictionary *)updatedEMMParametersWithParameters:(NSDictionary *)parameters { + return [[self class] parametersWithParameters:parameters + emmSupport:parameters[kEMMSupportParameterName] + isPasscodeInfoRequired:parameters[kEMMPasscodeInfoParameterName] != nil]; +} + + ++ (NSDictionary *)parametersWithParameters:(NSDictionary *)parameters + emmSupport:(nullable NSString *)emmSupport + isPasscodeInfoRequired:(BOOL)isPasscodeInfoRequired { + if (!emmSupport) { + return parameters; + } + NSMutableDictionary *allParameters = [(parameters ?: @{}) mutableCopy]; + allParameters[kEMMSupportParameterName] = emmSupport; + UIDevice *device = [UIDevice currentDevice]; + NSString *systemName = device.systemName; + if ([systemName isEqualToString:kOldIOSSystemName]) { + systemName = kNewIOSSystemName; + } + allParameters[kEMMOSVersionParameterName] = + [NSString stringWithFormat:@"%@ %@", systemName, device.systemVersion]; + if (isPasscodeInfoRequired) { + allParameters[kEMMPasscodeInfoParameterName] = [GIDMDMPasscodeState passcodeState].info; + } + return allParameters; +} + +@end + +#pragma mark - GTMAppAuthFetcherAuthorizationEMMChainedDelegate + +@implementation GTMAppAuthFetcherAuthorizationEMMChainedDelegate { + // We use a weak reference here to match GTMAppAuthFetcherAuthorization. + __weak id _delegate; + SEL _selector; + // We need to maintain a reference to the chained delegate because GTMAppAuthFetcherAuthorization + // only keeps a weak reference. + GTMAppAuthFetcherAuthorizationEMMChainedDelegate *_retained_self; +} + +- (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector { + self = [super init]; + if (self) { + _delegate = delegate; + _selector = selector; + _retained_self = self; + } + return self; +} + +- (void)authentication:(GTMAppAuthFetcherAuthorization *)auth + request:(NSMutableURLRequest *)request + finishedWithError:(nullable NSError *)error { + [GIDAppAuthFetcherAuthorizationWithEMMSupport handleTokenFetchEMMError:error + completion:^(NSError *_Nullable error) { + if (!self->_delegate || !self->_selector) { + return; + } + NSMethodSignature *signature = [self->_delegate methodSignatureForSelector:self->_selector]; + if (!signature) { + return; + } + id argument1 = auth; + id argument2 = request; + id argument3 = error; + NSInvocation *invocation = [NSInvocation invocationWithMethodSignature:signature]; + [invocation setTarget:self->_delegate]; // index 0 + [invocation setSelector:self->_selector]; // index 1 + [invocation setArgument:&argument1 atIndex:2]; + [invocation setArgument:&argument2 atIndex:3]; + [invocation setArgument:&argument3 atIndex:4]; + [invocation invoke]; + }]; + // Prepare to deallocate the chained delegate instance because the above block will retain the + // iVar references it uses. + _retained_self = nil; +} + +@end + +NS_ASSUME_NONNULL_END + +#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST diff --git a/GoogleSignIn/Sources/GIDAuthentication_Private.h b/GoogleSignIn/Sources/GIDAuthentication_Private.h index 37245aa4..7bdb46c1 100644 --- a/GoogleSignIn/Sources/GIDAuthentication_Private.h +++ b/GoogleSignIn/Sources/GIDAuthentication_Private.h @@ -34,20 +34,6 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithAuthState:(OIDAuthState *)authState; -#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST -// Gets a new set of URL parameters that also contains EMM-related URL parameters if needed. -+ (NSDictionary *)parametersWithParameters:(NSDictionary *)parameters - emmSupport:(nullable NSString *)emmSupport - isPasscodeInfoRequired:(BOOL)isPasscodeInfoRequired; - -// Gets a new set of URL parameters that contains updated EMM-related URL parameters if needed. -+ (NSDictionary *)updatedEMMParametersWithParameters:(NSDictionary *)parameters; - -// Handles potential EMM error from token fetch response. -+ (void)handleTokenFetchEMMError:(nullable NSError *)error - completion:(void (^)(NSError *_Nullable))completion; -#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST - @end NS_ASSUME_NONNULL_END diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index 3bd78c09..50d0d59a 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -18,6 +18,7 @@ #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDConfiguration.h" +#import "GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h" #import "GoogleSignIn/Sources/GIDAuthentication_Private.h" #import "GoogleSignIn/Sources/GIDProfileData_Private.h" #import "GoogleSignIn/Sources/GIDToken_Private.h" @@ -45,10 +46,6 @@ NS_ASSUME_NONNULL_BEGIN -// A specialized GTMAppAuthFetcherAuthorization subclass with EMM support. -@interface GTMAppAuthFetcherAuthorizationWithEMMSupport : GTMAppAuthFetcherAuthorization -@end - @interface GIDGoogleUser () @property(nonatomic, readwrite) GIDToken *accessToken; @@ -113,13 +110,14 @@ - (GIDConfiguration *)configuration { return _cachedConfiguration; } +// TODO(pinlu): Create fetcherAuthorizer at initialization and observe authState changes. #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wdeprecated-declarations" - (id)fetcherAuthorizer { #pragma clang diagnostic pop #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST GTMAppAuthFetcherAuthorization *authorization = self.emmSupport ? - [[GTMAppAuthFetcherAuthorizationWithEMMSupport alloc] initWithAuthState:_authState] : + [[GIDAppAuthFetcherAuthorizationWithEMMSupport alloc] initWithAuthState:_authState] : [[GTMAppAuthFetcherAuthorization alloc] initWithAuthState:_authState]; #elif TARGET_OS_OSX || TARGET_OS_MACCATALYST GTMAppAuthFetcherAuthorization *authorization = @@ -132,7 +130,7 @@ - (GIDConfiguration *)configuration { #pragma mark - Private Methods #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST -- (NSString *)emmSupport { +- (nullable NSString *)emmSupport { return _authState.lastAuthorizationResponse.request.additionalParameters[kEMMSupportParameterName]; } @@ -205,7 +203,7 @@ - (nullable NSString *)hostedDomain { - (nullable NSDictionary *)additionalRefreshParameters: (GTMAppAuthFetcherAuthorization *)authorization { #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST - return [GIDAuthentication updatedEMMParametersWithParameters: + return [GIDAppAuthFetcherAuthorizationWithEMMSupport updatedEMMParametersWithParameters: authorization.authState.lastTokenResponse.request.additionalParameters]; #elif TARGET_OS_OSX || TARGET_OS_MACCATALYST return authorization.authState.lastTokenResponse.request.additionalParameters; @@ -243,104 +241,4 @@ - (void)encodeWithCoder:(NSCoder *)encoder { @end -#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST - -#pragma mark - GTMAppAuthFetcherAuthorizationEMMChainedDelegate - -// The specialized GTMAppAuthFetcherAuthorization delegate that handles potential EMM error -// responses. -@interface GTMAppAuthFetcherAuthorizationEMMChainedDelegate : NSObject - -// Initializes with chained delegate and selector. -- (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector; - -// The callback method for GTMAppAuthFetcherAuthorization to invoke. -- (void)authentication:(GTMAppAuthFetcherAuthorization *)auth - request:(NSMutableURLRequest *)request - finishedWithError:(nullable NSError *)error; - -@end - -@implementation GTMAppAuthFetcherAuthorizationEMMChainedDelegate { - // We use a weak reference here to match GTMAppAuthFetcherAuthorization. - __weak id _delegate; - SEL _selector; - // We need to maintain a reference to the chained delegate because GTMAppAuthFetcherAuthorization - // only keeps a weak reference. - GTMAppAuthFetcherAuthorizationEMMChainedDelegate *_retained_self; -} - -- (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector { - self = [super init]; - if (self) { - _delegate = delegate; - _selector = selector; - _retained_self = self; - } - return self; -} - -- (void)authentication:(GTMAppAuthFetcherAuthorization *)auth - request:(NSMutableURLRequest *)request - finishedWithError:(nullable NSError *)error { - [GIDAuthentication handleTokenFetchEMMError:error completion:^(NSError *_Nullable error) { - if (!self->_delegate || !self->_selector) { - return; - } - NSMethodSignature *signature = [self->_delegate methodSignatureForSelector:self->_selector]; - if (!signature) { - return; - } - id argument1 = auth; - id argument2 = request; - id argument3 = error; - NSInvocation *invocation = [NSInvocation invocationWithMethodSignature:signature]; - [invocation setTarget:self->_delegate]; // index 0 - [invocation setSelector:self->_selector]; // index 1 - [invocation setArgument:&argument1 atIndex:2]; - [invocation setArgument:&argument2 atIndex:3]; - [invocation setArgument:&argument3 atIndex:4]; - [invocation invoke]; - }]; - // Prepare to deallocate the chained delegate instance because the above block will retain the - // iVar references it uses. - _retained_self = nil; -} - -@end - -#pragma mark - GTMAppAuthFetcherAuthorizationWithEMMSupport - -@implementation GTMAppAuthFetcherAuthorizationWithEMMSupport - -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wdeprecated-implementations" -- (void)authorizeRequest:(nullable NSMutableURLRequest *)request - delegate:(id)delegate - didFinishSelector:(SEL)sel { -#pragma clang diagnostic pop - GTMAppAuthFetcherAuthorizationEMMChainedDelegate *chainedDelegate = - [[GTMAppAuthFetcherAuthorizationEMMChainedDelegate alloc] initWithDelegate:delegate - selector:sel]; -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wdeprecated-declarations" - [super authorizeRequest:request - delegate:chainedDelegate - didFinishSelector:@selector(authentication:request:finishedWithError:)]; -#pragma clang diagnostic pop -} - -- (void)authorizeRequest:(nullable NSMutableURLRequest *)request - completionHandler:(GTMAppAuthFetcherAuthorizationCompletion)handler { - [super authorizeRequest:request completionHandler:^(NSError *_Nullable error) { - [GIDAuthentication handleTokenFetchEMMError:error completion:^(NSError *_Nullable error) { - handler(error); - }]; - }]; -} - -@end - -#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST - NS_ASSUME_NONNULL_END diff --git a/GoogleSignIn/Sources/GIDGoogleUser_Private.h b/GoogleSignIn/Sources/GIDGoogleUser_Private.h index 46b85168..a56996c9 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser_Private.h +++ b/GoogleSignIn/Sources/GIDGoogleUser_Private.h @@ -33,7 +33,7 @@ NS_ASSUME_NONNULL_BEGIN #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST // A string indicating support for Enterprise Mobility Management. -@property(nonatomic, readonly) NSString *emmSupport; +@property(nonatomic, readonly, nullable) NSString *emmSupport; #endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST // Create a object with an auth state, scopes, and profile data. diff --git a/GoogleSignIn/Sources/GIDSignIn.m b/GoogleSignIn/Sources/GIDSignIn.m index 682c1995..66fac4ef 100644 --- a/GoogleSignIn/Sources/GIDSignIn.m +++ b/GoogleSignIn/Sources/GIDSignIn.m @@ -22,6 +22,7 @@ #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDProfileData.h" #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDUserAuth.h" +#import "GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h" #import "GoogleSignIn/Sources/GIDSignInInternalOptions.h" #import "GoogleSignIn/Sources/GIDSignInPreferences.h" #import "GoogleSignIn/Sources/GIDCallbackQueue.h" @@ -583,9 +584,9 @@ - (void)authenticateInteractivelyWithOptions:(GIDSignInInternalOptions *)options #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST [additionalParameters addEntriesFromDictionary: - [GIDAuthentication parametersWithParameters:options.extraParams - emmSupport:emmSupport - isPasscodeInfoRequired:NO]]; + [GIDAppAuthFetcherAuthorizationWithEMMSupport parametersWithParameters:options.extraParams + emmSupport:emmSupport + isPasscodeInfoRequired:NO]]; #elif TARGET_OS_OSX || TARGET_OS_MACCATALYST [additionalParameters addEntriesFromDictionary:options.extraParams]; #endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST @@ -732,9 +733,10 @@ - (void)maybeFetchToken:(GIDAuthFlow *)authFlow { authState.lastAuthorizationResponse.additionalParameters; NSString *passcodeInfoRequired = (NSString *)params[kEMMPasscodeInfoRequiredKeyName]; [additionalParameters addEntriesFromDictionary: - [GIDAuthentication parametersWithParameters:@{} - emmSupport:authFlow.emmSupport - isPasscodeInfoRequired:passcodeInfoRequired.length > 0]]; + [GIDAppAuthFetcherAuthorizationWithEMMSupport + parametersWithParameters:@{} + emmSupport:authFlow.emmSupport + isPasscodeInfoRequired:passcodeInfoRequired.length > 0]]; #endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST additionalParameters[kSDKVersionLoggingParameter] = GIDVersion(); additionalParameters[kEnvironmentLoggingParameter] = GIDEnvironment(); @@ -760,7 +762,8 @@ - (void)maybeFetchToken:(GIDAuthFlow *)authFlow { #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST if (authFlow.emmSupport) { - [GIDAuthentication handleTokenFetchEMMError:error completion:^(NSError *error) { + [GIDAppAuthFetcherAuthorizationWithEMMSupport + handleTokenFetchEMMError:error completion:^(NSError *error) { authFlow.error = error; [authFlow next]; }]; diff --git a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h index 32a26440..ae8408a8 100644 --- a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h +++ b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h @@ -17,7 +17,7 @@ #import // We have to import GTMAppAuth because forward declaring the protocol does -// not generate the `fetcherAuthorizer` method below for Swift. +// not generate the `fetcherAuthorizer` property below for Swift. #ifdef SWIFT_PACKAGE @import GTMAppAuth; #else diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index 0c4f1d26..b033c785 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -157,12 +157,15 @@ - (void)testFetcherAuthorizer { idTokenExpiresIn:kIDTokenExpiresIn]; #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wdeprecated-declarations" - id fetcherAuthroizer = user.fetcherAuthorizer; + id fetcherAuthorizer = user.fetcherAuthorizer; #pragma clang diagnostic pop - XCTAssertTrue([fetcherAuthroizer isKindOfClass:[GTMAppAuthFetcherAuthorization class]]); - XCTAssertTrue([fetcherAuthroizer canAuthorize]); + XCTAssertTrue([fetcherAuthorizer isKindOfClass:[GTMAppAuthFetcherAuthorization class]]); + XCTAssertTrue([fetcherAuthorizer canAuthorize]); } +// TODO(pinlu): Add a test that the property `fetcherAuthorizer` returns the same instance +// after authState is updated. + #pragma mark - Helpers - (GIDGoogleUser *)googleUserWithAccessTokenExpiresIn:(NSTimeInterval)accessTokenExpiresIn diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index fdcd1027..373265ef 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -26,6 +26,7 @@ // Test module imports @import GoogleSignIn; +#import "GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h" #import "GoogleSignIn/Sources/GIDGoogleUser_Private.h" #import "GoogleSignIn/Sources/GIDSignIn_Private.h" #import "GoogleSignIn/Sources/GIDSignInPreferences.h" @@ -1068,9 +1069,10 @@ - (void)testTokenEndpointEMMError { NSError *emmError = [NSError errorWithDomain:@"anydomain" code:12345 userInfo:@{ OIDOAuthErrorFieldError : errorJSON }]; - [[_authentication expect] handleTokenFetchEMMError:emmError - completion:SAVE_TO_ARG_BLOCK(completion)]; - + id appAuthFetcherAuthorization = + OCMStrictClassMock([GIDAppAuthFetcherAuthorizationWithEMMSupport class]); + [[appAuthFetcherAuthorization expect] handleTokenFetchEMMError:emmError + completion:SAVE_TO_ARG_BLOCK(completion)]; [self OAuthLoginWithAddScopesFlow:NO authError:nil diff --git a/Samples/Swift/DaysUntilBirthday/Shared/ViewModels/AuthenticationViewModel.swift b/Samples/Swift/DaysUntilBirthday/Shared/ViewModels/AuthenticationViewModel.swift index 3ad14289..7f92ee71 100644 --- a/Samples/Swift/DaysUntilBirthday/Shared/ViewModels/AuthenticationViewModel.swift +++ b/Samples/Swift/DaysUntilBirthday/Shared/ViewModels/AuthenticationViewModel.swift @@ -40,6 +40,8 @@ final class AuthenticationViewModel: ObservableObject { init() { if let user = GIDSignIn.sharedInstance.currentUser { self.state = .signedIn(user) + + let authorization = user.fetcherAuthorizer } else { self.state = .signedOut } From ad40f6c795afa9bb86cf1c3d0b313710aa07f55f Mon Sep 17 00:00:00 2001 From: pinlu Date: Wed, 28 Sep 2022 18:37:40 -0700 Subject: [PATCH 10/17] Minor improvements. --- .../GIDAppAuthFetcherAuthorizationWithEMMSupport.h | 4 ++-- .../GIDAppAuthFetcherAuthorizationWithEMMSupport.m | 5 ++--- GoogleSignIn/Sources/GIDAuthentication_Private.h | 8 -------- GoogleSignIn/Tests/Unit/GIDSignInTest.m | 2 +- .../Shared/ViewModels/AuthenticationViewModel.swift | 2 -- 5 files changed, 5 insertions(+), 16 deletions(-) diff --git a/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h b/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h index 674a072b..5594670e 100644 --- a/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h +++ b/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h @@ -13,10 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST - #import +#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST + #ifdef SWIFT_PACKAGE @import GTMAppAuth; #else diff --git a/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m b/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m index cab44c66..cb831cf3 100644 --- a/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m +++ b/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m @@ -12,15 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. +#import + #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST #import "GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h" -#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST #import "GoogleSignIn/Sources/GIDEMMErrorHandler.h" #import "GoogleSignIn/Sources/GIDMDMPasscodeState.h" -#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST - #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h" #ifdef SWIFT_PACKAGE diff --git a/GoogleSignIn/Sources/GIDAuthentication_Private.h b/GoogleSignIn/Sources/GIDAuthentication_Private.h index 7bdb46c1..113728bf 100644 --- a/GoogleSignIn/Sources/GIDAuthentication_Private.h +++ b/GoogleSignIn/Sources/GIDAuthentication_Private.h @@ -16,14 +16,6 @@ #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDAuthentication.h" -#ifdef SWIFT_PACKAGE -@import AppAuth; -@import GTMAppAuth; -#else -#import -#import -#endif - NS_ASSUME_NONNULL_BEGIN // Internal methods for the class that are not part of the public API. diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index 373265ef..5ff4f5b6 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -1091,7 +1091,7 @@ - (void)testTokenEndpointEMMError { [self waitForExpectationsWithTimeout:1 handler:nil]; - [_authentication verify]; + [appAuthFetcherAuthorization verify]; XCTAssertFalse(_keychainSaved, @"should not save to keychain"); XCTAssertTrue(_completionCalled, @"should call delegate"); XCTAssertNotNil(_authError, @"should have error"); diff --git a/Samples/Swift/DaysUntilBirthday/Shared/ViewModels/AuthenticationViewModel.swift b/Samples/Swift/DaysUntilBirthday/Shared/ViewModels/AuthenticationViewModel.swift index 7f92ee71..3ad14289 100644 --- a/Samples/Swift/DaysUntilBirthday/Shared/ViewModels/AuthenticationViewModel.swift +++ b/Samples/Swift/DaysUntilBirthday/Shared/ViewModels/AuthenticationViewModel.swift @@ -40,8 +40,6 @@ final class AuthenticationViewModel: ObservableObject { init() { if let user = GIDSignIn.sharedInstance.currentUser { self.state = .signedIn(user) - - let authorization = user.fetcherAuthorizer } else { self.state = .signedOut } From bd3257d7b4ed0acfa408338c1ce948cf16db2e95 Mon Sep 17 00:00:00 2001 From: pinlu Date: Thu, 29 Sep 2022 11:19:31 -0700 Subject: [PATCH 11/17] Update GIDAppAuthFetcherAuthorizationWithEMMSupport.m --- ...ppAuthFetcherAuthorizationWithEMMSupport.m | 100 +++++++++--------- 1 file changed, 49 insertions(+), 51 deletions(-) diff --git a/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m b/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m index cb831cf3..96a8b579 100644 --- a/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m +++ b/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m @@ -57,6 +57,55 @@ - (void)authentication:(GTMAppAuthFetcherAuthorization *)auth @end +@implementation GTMAppAuthFetcherAuthorizationEMMChainedDelegate { + // We use a weak reference here to match GTMAppAuthFetcherAuthorization. + __weak id _delegate; + SEL _selector; + // We need to maintain a reference to the chained delegate because GTMAppAuthFetcherAuthorization + // only keeps a weak reference. + GTMAppAuthFetcherAuthorizationEMMChainedDelegate *_retained_self; +} + +- (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector { + self = [super init]; + if (self) { + _delegate = delegate; + _selector = selector; + _retained_self = self; + } + return self; +} + +- (void)authentication:(GTMAppAuthFetcherAuthorization *)auth + request:(NSMutableURLRequest *)request + finishedWithError:(nullable NSError *)error { + [GIDAppAuthFetcherAuthorizationWithEMMSupport handleTokenFetchEMMError:error + completion:^(NSError *_Nullable error) { + if (!self->_delegate || !self->_selector) { + return; + } + NSMethodSignature *signature = [self->_delegate methodSignatureForSelector:self->_selector]; + if (!signature) { + return; + } + id argument1 = auth; + id argument2 = request; + id argument3 = error; + NSInvocation *invocation = [NSInvocation invocationWithMethodSignature:signature]; + [invocation setTarget:self->_delegate]; // index 0 + [invocation setSelector:self->_selector]; // index 1 + [invocation setArgument:&argument1 atIndex:2]; + [invocation setArgument:&argument2 atIndex:3]; + [invocation setArgument:&argument3 atIndex:4]; + [invocation invoke]; + }]; + // Prepare to deallocate the chained delegate instance because the above block will retain the + // iVar references it uses. + _retained_self = nil; +} + +@end + @implementation GIDAppAuthFetcherAuthorizationWithEMMSupport #pragma clang diagnostic push @@ -135,57 +184,6 @@ + (NSDictionary *)parametersWithParameters:(NSDictionary *)parameters @end -#pragma mark - GTMAppAuthFetcherAuthorizationEMMChainedDelegate - -@implementation GTMAppAuthFetcherAuthorizationEMMChainedDelegate { - // We use a weak reference here to match GTMAppAuthFetcherAuthorization. - __weak id _delegate; - SEL _selector; - // We need to maintain a reference to the chained delegate because GTMAppAuthFetcherAuthorization - // only keeps a weak reference. - GTMAppAuthFetcherAuthorizationEMMChainedDelegate *_retained_self; -} - -- (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector { - self = [super init]; - if (self) { - _delegate = delegate; - _selector = selector; - _retained_self = self; - } - return self; -} - -- (void)authentication:(GTMAppAuthFetcherAuthorization *)auth - request:(NSMutableURLRequest *)request - finishedWithError:(nullable NSError *)error { - [GIDAppAuthFetcherAuthorizationWithEMMSupport handleTokenFetchEMMError:error - completion:^(NSError *_Nullable error) { - if (!self->_delegate || !self->_selector) { - return; - } - NSMethodSignature *signature = [self->_delegate methodSignatureForSelector:self->_selector]; - if (!signature) { - return; - } - id argument1 = auth; - id argument2 = request; - id argument3 = error; - NSInvocation *invocation = [NSInvocation invocationWithMethodSignature:signature]; - [invocation setTarget:self->_delegate]; // index 0 - [invocation setSelector:self->_selector]; // index 1 - [invocation setArgument:&argument1 atIndex:2]; - [invocation setArgument:&argument2 atIndex:3]; - [invocation setArgument:&argument3 atIndex:4]; - [invocation invoke]; - }]; - // Prepare to deallocate the chained delegate instance because the above block will retain the - // iVar references it uses. - _retained_self = nil; -} - -@end - NS_ASSUME_NONNULL_END #endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST From f87f063cf83cf56a3b3983d7855c86a147c64b9a Mon Sep 17 00:00:00 2001 From: pinlu Date: Thu, 29 Sep 2022 15:15:04 -0700 Subject: [PATCH 12/17] Create fetcherAuthorizer at initialization --- GoogleSignIn/Sources/GIDGoogleUser.m | 30 +++++++++------------ GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 11 ++++++-- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index 50d0d59a..82d8e159 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -54,6 +54,8 @@ @interface GIDGoogleUser () @property(nonatomic, readwrite, nullable) GIDToken *idToken; +@property(nonatomic, readwrite) id fetcherAuthorizer; + @end @implementation GIDGoogleUser { @@ -110,23 +112,6 @@ - (GIDConfiguration *)configuration { return _cachedConfiguration; } -// TODO(pinlu): Create fetcherAuthorizer at initialization and observe authState changes. -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wdeprecated-declarations" -- (id)fetcherAuthorizer { -#pragma clang diagnostic pop -#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST - GTMAppAuthFetcherAuthorization *authorization = self.emmSupport ? - [[GIDAppAuthFetcherAuthorizationWithEMMSupport alloc] initWithAuthState:_authState] : - [[GTMAppAuthFetcherAuthorization alloc] initWithAuthState:_authState]; -#elif TARGET_OS_OSX || TARGET_OS_MACCATALYST - GTMAppAuthFetcherAuthorization *authorization = - [[GTMAppAuthFetcherAuthorization alloc] initWithAuthState:_authState]; -#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST - authorization.tokenRefreshDelegate = self; - return authorization; -} - #pragma mark - Private Methods #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST @@ -140,6 +125,17 @@ - (instancetype)initWithAuthState:(OIDAuthState *)authState profileData:(nullable GIDProfileData *)profileData { self = [super init]; if (self) { +#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST + GTMAppAuthFetcherAuthorization *authorization = self.emmSupport ? + [[GIDAppAuthFetcherAuthorizationWithEMMSupport alloc] initWithAuthState:_authState] : + [[GTMAppAuthFetcherAuthorization alloc] initWithAuthState:_authState]; +#elif TARGET_OS_OSX || TARGET_OS_MACCATALYST + GTMAppAuthFetcherAuthorization *authorization = + [[GTMAppAuthFetcherAuthorization alloc] initWithAuthState:_authState]; +#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST + authorization.tokenRefreshDelegate = self; + self.fetcherAuthorizer = authorization; + [self updateAuthState:authState profileData:profileData]; } return self; diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index b033c785..da29db8e 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -163,8 +163,15 @@ - (void)testFetcherAuthorizer { XCTAssertTrue([fetcherAuthorizer canAuthorize]); } -// TODO(pinlu): Add a test that the property `fetcherAuthorizer` returns the same instance -// after authState is updated. +- (void)testFetcherAuthorizer_returnTheSameInstance { + GIDGoogleUser *user = [self googleUserWithAccessTokenExpiresIn:kAccessTokenExpiresIn + idTokenExpiresIn:kIDTokenExpiresIn]; + + id fetcherAuthorizer = user.fetcherAuthorizer; + id fetcherAuthorizer2 = user.fetcherAuthorizer; + + XCTAssertIdentical(fetcherAuthorizer, fetcherAuthorizer2); +} #pragma mark - Helpers From 9f9f85ab547dd0407ae63551ccbfc25dd9436af1 Mon Sep 17 00:00:00 2001 From: pinlu Date: Thu, 29 Sep 2022 15:50:56 -0700 Subject: [PATCH 13/17] Fix test failures --- GoogleSignIn/Sources/GIDGoogleUser.m | 22 +++++++++++----------- GoogleSignIn/Tests/Unit/GIDSignInTest.m | 1 + 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index 82d8e159..e73f9273 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -125,17 +125,6 @@ - (instancetype)initWithAuthState:(OIDAuthState *)authState profileData:(nullable GIDProfileData *)profileData { self = [super init]; if (self) { -#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST - GTMAppAuthFetcherAuthorization *authorization = self.emmSupport ? - [[GIDAppAuthFetcherAuthorizationWithEMMSupport alloc] initWithAuthState:_authState] : - [[GTMAppAuthFetcherAuthorization alloc] initWithAuthState:_authState]; -#elif TARGET_OS_OSX || TARGET_OS_MACCATALYST - GTMAppAuthFetcherAuthorization *authorization = - [[GTMAppAuthFetcherAuthorization alloc] initWithAuthState:_authState]; -#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST - authorization.tokenRefreshDelegate = self; - self.fetcherAuthorizer = authorization; - [self updateAuthState:authState profileData:profileData]; } return self; @@ -148,6 +137,17 @@ - (void)updateAuthState:(OIDAuthState *)authState _authentication = [[GIDAuthentication alloc] initWithAuthState:authState]; _profile = profileData; +#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST + GTMAppAuthFetcherAuthorization *authorization = self.emmSupport ? + [[GIDAppAuthFetcherAuthorizationWithEMMSupport alloc] initWithAuthState:_authState] : + [[GTMAppAuthFetcherAuthorization alloc] initWithAuthState:_authState]; +#elif TARGET_OS_OSX || TARGET_OS_MACCATALYST + GTMAppAuthFetcherAuthorization *authorization = + [[GTMAppAuthFetcherAuthorization alloc] initWithAuthState:_authState]; +#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST + authorization.tokenRefreshDelegate = self; + self.fetcherAuthorizer = authorization; + [self updateTokensWithAuthState:authState]; } } diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index 5ff4f5b6..8fa0fbb7 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -397,6 +397,7 @@ - (void)testShareInstance { - (void)testRestorePreviousSignInNoRefresh_hasPreviousUser { [[[_authorization expect] andReturn:_authState] authState]; + [[_authorization expect] setTokenRefreshDelegate:OCMOCK_ANY]; OCMStub([_authState lastTokenResponse]).andReturn(_tokenResponse); OCMStub([_authState refreshToken]).andReturn(kRefreshToken); From c622c9733d2d518ffcdbc7f1497fe04ae48213b5 Mon Sep 17 00:00:00 2001 From: pinlu Date: Thu, 29 Sep 2022 16:09:51 -0700 Subject: [PATCH 14/17] Suppress the deprecation warnings. --- GoogleSignIn/Sources/GIDGoogleUser.m | 3 +++ GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h | 2 +- GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 5 ++++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index e73f9273..9a0fa89a 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -54,7 +54,10 @@ @interface GIDGoogleUser () @property(nonatomic, readwrite, nullable) GIDToken *idToken; +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" @property(nonatomic, readwrite) id fetcherAuthorizer; +#pragma clang diagnostic pop @end diff --git a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h index ae8408a8..39176735 100644 --- a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h +++ b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h @@ -61,7 +61,7 @@ NS_ASSUME_NONNULL_BEGIN /// see https://developers.google.com/identity/sign-in/ios/backend-auth. @property(nonatomic, readonly, nullable) GIDToken *idToken; -/// A new authorizer for `GTLService`, `GTMSessionFetcher`, or `GTMHTTPFetcher`. +/// The authorizer for `GTLService`, `GTMSessionFetcher`, or `GTMHTTPFetcher`. #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wdeprecated-declarations" @property(nonatomic, readonly) id fetcherAuthorizer; diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index da29db8e..ae2e561d 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -167,9 +167,12 @@ - (void)testFetcherAuthorizer_returnTheSameInstance { GIDGoogleUser *user = [self googleUserWithAccessTokenExpiresIn:kAccessTokenExpiresIn idTokenExpiresIn:kIDTokenExpiresIn]; +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" id fetcherAuthorizer = user.fetcherAuthorizer; id fetcherAuthorizer2 = user.fetcherAuthorizer; - +#pragma clang diagnostic pop + XCTAssertIdentical(fetcherAuthorizer, fetcherAuthorizer2); } From 51bb4246bb65697615f75d44807a6b1480425bc8 Mon Sep 17 00:00:00 2001 From: pinlu Date: Sat, 1 Oct 2022 12:45:45 -0700 Subject: [PATCH 15/17] Added Utility class GIDEMMSupport to handle EMM related issues. --- ...ppAuthFetcherAuthorizationWithEMMSupport.h | 13 +-- ...ppAuthFetcherAuthorizationWithEMMSupport.m | 78 ++------------ GoogleSignIn/Sources/GIDEMMSupport.h | 44 ++++++++ GoogleSignIn/Sources/GIDEMMSupport.m | 101 ++++++++++++++++++ GoogleSignIn/Sources/GIDGoogleUser.m | 22 +--- GoogleSignIn/Sources/GIDGoogleUser_Private.h | 11 ++ GoogleSignIn/Sources/GIDSignIn.m | 18 ++-- GoogleSignIn/Tests/Unit/GIDSignInTest.m | 11 +- 8 files changed, 182 insertions(+), 116 deletions(-) create mode 100644 GoogleSignIn/Sources/GIDEMMSupport.h create mode 100644 GoogleSignIn/Sources/GIDEMMSupport.m diff --git a/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h b/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h index 5594670e..baffe3c1 100644 --- a/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h +++ b/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + #import #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST @@ -28,18 +29,6 @@ NS_ASSUME_NONNULL_BEGIN // A specialized GTMAppAuthFetcherAuthorization subclass with EMM support. @interface GIDAppAuthFetcherAuthorizationWithEMMSupport : GTMAppAuthFetcherAuthorization -// Handles potential EMM error from token fetch response. -+ (void)handleTokenFetchEMMError:(nullable NSError *)error - completion:(void (^)(NSError *_Nullable))completion; - -// Gets a new set of URL parameters that contains updated EMM-related URL parameters if needed. -+ (NSDictionary *)updatedEMMParametersWithParameters:(NSDictionary *)parameters; - -// Gets a new set of URL parameters that also contains EMM-related URL parameters if needed. -+ (NSDictionary *)parametersWithParameters:(NSDictionary *)parameters - emmSupport:(nullable NSString *)emmSupport - isPasscodeInfoRequired:(BOOL)isPasscodeInfoRequired; - @end NS_ASSUME_NONNULL_END diff --git a/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m b/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m index 96a8b579..2fa68e3f 100644 --- a/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m +++ b/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m @@ -18,9 +18,7 @@ #import "GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h" -#import "GoogleSignIn/Sources/GIDEMMErrorHandler.h" -#import "GoogleSignIn/Sources/GIDMDMPasscodeState.h" -#import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h" +#import "GoogleSignIn/Sources/GIDEMMSupport.h" #ifdef SWIFT_PACKAGE @import AppAuth; @@ -30,22 +28,11 @@ #import #endif -// Additional parameter names for EMM. -static NSString *const kEMMSupportParameterName = @"emm_support"; -static NSString *const kEMMOSVersionParameterName = @"device_os"; -static NSString *const kEMMPasscodeInfoParameterName = @"emm_passcode_info"; - -// Old UIDevice system name for iOS. -static NSString *const kOldIOSSystemName = @"iPhone OS"; - -// New UIDevice system name for iOS. -static NSString *const kNewIOSSystemName = @"iOS"; - NS_ASSUME_NONNULL_BEGIN // The specialized GTMAppAuthFetcherAuthorization delegate that handles potential EMM error // responses. -@interface GTMAppAuthFetcherAuthorizationEMMChainedDelegate : NSObject +@interface GIDAppAuthFetcherAuthorizationEMMChainedDelegate : NSObject // Initializes with chained delegate and selector. - (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector; @@ -57,13 +44,13 @@ - (void)authentication:(GTMAppAuthFetcherAuthorization *)auth @end -@implementation GTMAppAuthFetcherAuthorizationEMMChainedDelegate { +@implementation GIDAppAuthFetcherAuthorizationEMMChainedDelegate { // We use a weak reference here to match GTMAppAuthFetcherAuthorization. __weak id _delegate; SEL _selector; // We need to maintain a reference to the chained delegate because GTMAppAuthFetcherAuthorization // only keeps a weak reference. - GTMAppAuthFetcherAuthorizationEMMChainedDelegate *_retained_self; + GIDAppAuthFetcherAuthorizationEMMChainedDelegate *_retained_self; } - (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector { @@ -79,8 +66,7 @@ - (instancetype)initWithDelegate:(id)delegate selector:(SEL)selector { - (void)authentication:(GTMAppAuthFetcherAuthorization *)auth request:(NSMutableURLRequest *)request finishedWithError:(nullable NSError *)error { - [GIDAppAuthFetcherAuthorizationWithEMMSupport handleTokenFetchEMMError:error - completion:^(NSError *_Nullable error) { + [GIDEMMSupport handleTokenFetchEMMError:error completion:^(NSError *_Nullable error) { if (!self->_delegate || !self->_selector) { return; } @@ -114,8 +100,8 @@ - (void)authorizeRequest:(nullable NSMutableURLRequest *)request delegate:(id)delegate didFinishSelector:(SEL)sel { #pragma clang diagnostic pop - GTMAppAuthFetcherAuthorizationEMMChainedDelegate *chainedDelegate = - [[GTMAppAuthFetcherAuthorizationEMMChainedDelegate alloc] initWithDelegate:delegate + GIDAppAuthFetcherAuthorizationEMMChainedDelegate *chainedDelegate = + [[GIDAppAuthFetcherAuthorizationEMMChainedDelegate alloc] initWithDelegate:delegate selector:sel]; #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wdeprecated-declarations" @@ -128,60 +114,12 @@ - (void)authorizeRequest:(nullable NSMutableURLRequest *)request - (void)authorizeRequest:(nullable NSMutableURLRequest *)request completionHandler:(GTMAppAuthFetcherAuthorizationCompletion)handler { [super authorizeRequest:request completionHandler:^(NSError *_Nullable error) { - [[self class] handleTokenFetchEMMError:error completion:^(NSError *_Nullable error) { + [GIDEMMSupport handleTokenFetchEMMError:error completion:^(NSError *_Nullable error) { handler(error); }]; }]; } -+ (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:^() { - if (handled) { - completion([NSError errorWithDomain:kGIDSignInErrorDomain - code:kGIDSignInErrorCodeEMM - userInfo:error.userInfo]); - } else { - completion(error); - } - }]; - } else { - completion(error); - } -} - -+ (NSDictionary *)updatedEMMParametersWithParameters:(NSDictionary *)parameters { - return [[self class] parametersWithParameters:parameters - emmSupport:parameters[kEMMSupportParameterName] - isPasscodeInfoRequired:parameters[kEMMPasscodeInfoParameterName] != nil]; -} - - -+ (NSDictionary *)parametersWithParameters:(NSDictionary *)parameters - emmSupport:(nullable NSString *)emmSupport - isPasscodeInfoRequired:(BOOL)isPasscodeInfoRequired { - if (!emmSupport) { - return parameters; - } - NSMutableDictionary *allParameters = [(parameters ?: @{}) mutableCopy]; - allParameters[kEMMSupportParameterName] = emmSupport; - UIDevice *device = [UIDevice currentDevice]; - NSString *systemName = device.systemName; - if ([systemName isEqualToString:kOldIOSSystemName]) { - systemName = kNewIOSSystemName; - } - allParameters[kEMMOSVersionParameterName] = - [NSString stringWithFormat:@"%@ %@", systemName, device.systemVersion]; - if (isPasscodeInfoRequired) { - allParameters[kEMMPasscodeInfoParameterName] = [GIDMDMPasscodeState passcodeState].info; - } - return allParameters; -} - @end NS_ASSUME_NONNULL_END diff --git a/GoogleSignIn/Sources/GIDEMMSupport.h b/GoogleSignIn/Sources/GIDEMMSupport.h new file mode 100644 index 00000000..d6f4e92e --- /dev/null +++ b/GoogleSignIn/Sources/GIDEMMSupport.h @@ -0,0 +1,44 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#import + +#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST + +#import + +NS_ASSUME_NONNULL_BEGIN + +// A class to support EMM (Enterprise Mobility Management). +@interface GIDEMMSupport : NSObject + +// Handles potential EMM error from token fetch response. ++ (void)handleTokenFetchEMMError:(nullable NSError *)error + completion:(void (^)(NSError *_Nullable))completion; + +// Gets a new set of URL parameters that contains updated EMM-related URL parameters if needed. ++ (NSDictionary *)updatedEMMParametersWithParameters:(NSDictionary *)parameters; + +// Gets a new set of URL parameters that also contains EMM-related URL parameters if needed. ++ (NSDictionary *)parametersWithParameters:(NSDictionary *)parameters + emmSupport:(nullable NSString *)emmSupport + isPasscodeInfoRequired:(BOOL)isPasscodeInfoRequired; + +@end + +NS_ASSUME_NONNULL_END + +#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST diff --git a/GoogleSignIn/Sources/GIDEMMSupport.m b/GoogleSignIn/Sources/GIDEMMSupport.m new file mode 100644 index 00000000..654aaeb3 --- /dev/null +++ b/GoogleSignIn/Sources/GIDEMMSupport.m @@ -0,0 +1,101 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#import + +#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST + +#import "GoogleSignIn/Sources/GIDEMMSupport.h" + +#import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h" + +#import "GoogleSignIn/Sources/GIDEMMErrorHandler.h" +#import "GoogleSignIn/Sources/GIDMDMPasscodeState.h" + +#ifdef SWIFT_PACKAGE +@import AppAuth; +#else +#import +#endif + +NS_ASSUME_NONNULL_BEGIN + +// Additional parameter names for EMM. +static NSString *const kEMMSupportParameterName = @"emm_support"; +static NSString *const kEMMOSVersionParameterName = @"device_os"; +static NSString *const kEMMPasscodeInfoParameterName = @"emm_passcode_info"; + +// Old UIDevice system name for iOS. +static NSString *const kOldIOSSystemName = @"iPhone OS"; + +// New UIDevice system name for iOS. +static NSString *const kNewIOSSystemName = @"iOS"; + +@implementation GIDEMMSupport + ++ (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:^() { + if (handled) { + completion([NSError errorWithDomain:kGIDSignInErrorDomain + code:kGIDSignInErrorCodeEMM + userInfo:error.userInfo]); + } else { + completion(error); + } + }]; + } else { + completion(error); + } +} + ++ (NSDictionary *)updatedEMMParametersWithParameters:(NSDictionary *)parameters { + return [[self class] parametersWithParameters:parameters + emmSupport:parameters[kEMMSupportParameterName] + isPasscodeInfoRequired:parameters[kEMMPasscodeInfoParameterName] != nil]; +} + + ++ (NSDictionary *)parametersWithParameters:(NSDictionary *)parameters + emmSupport:(nullable NSString *)emmSupport + isPasscodeInfoRequired:(BOOL)isPasscodeInfoRequired { + if (!emmSupport) { + return parameters; + } + NSMutableDictionary *allParameters = [(parameters ?: @{}) mutableCopy]; + allParameters[kEMMSupportParameterName] = emmSupport; + UIDevice *device = [UIDevice currentDevice]; + NSString *systemName = device.systemName; + if ([systemName isEqualToString:kOldIOSSystemName]) { + systemName = kNewIOSSystemName; + } + allParameters[kEMMOSVersionParameterName] = + [NSString stringWithFormat:@"%@ %@", systemName, device.systemVersion]; + if (isPasscodeInfoRequired) { + allParameters[kEMMPasscodeInfoParameterName] = [GIDMDMPasscodeState passcodeState].info; + } + return allParameters; +} + +@end + +NS_ASSUME_NONNULL_END + +#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index 9a0fa89a..241798c2 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -20,6 +20,7 @@ #import "GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h" #import "GoogleSignIn/Sources/GIDAuthentication_Private.h" +#import "GoogleSignIn/Sources/GIDEMMSupport.h" #import "GoogleSignIn/Sources/GIDProfileData_Private.h" #import "GoogleSignIn/Sources/GIDToken_Private.h" @@ -29,6 +30,8 @@ #import #endif +NS_ASSUME_NONNULL_BEGIN + // The ID Token claim key for the hosted domain value. static NSString *const kHostedDomainIDTokenClaimKey = @"hd"; @@ -44,23 +47,6 @@ // Additional parameter names for EMM. static NSString *const kEMMSupportParameterName = @"emm_support"; -NS_ASSUME_NONNULL_BEGIN - -@interface GIDGoogleUser () - -@property(nonatomic, readwrite) GIDToken *accessToken; - -@property(nonatomic, readwrite) GIDToken *refreshToken; - -@property(nonatomic, readwrite, nullable) GIDToken *idToken; - -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wdeprecated-declarations" -@property(nonatomic, readwrite) id fetcherAuthorizer; -#pragma clang diagnostic pop - -@end - @implementation GIDGoogleUser { OIDAuthState *_authState; GIDConfiguration *_cachedConfiguration; @@ -202,7 +188,7 @@ - (nullable NSString *)hostedDomain { - (nullable NSDictionary *)additionalRefreshParameters: (GTMAppAuthFetcherAuthorization *)authorization { #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST - return [GIDAppAuthFetcherAuthorizationWithEMMSupport updatedEMMParametersWithParameters: + return [GIDEMMSupport updatedEMMParametersWithParameters: authorization.authState.lastTokenResponse.request.additionalParameters]; #elif TARGET_OS_OSX || TARGET_OS_MACCATALYST return authorization.authState.lastTokenResponse.request.additionalParameters; diff --git a/GoogleSignIn/Sources/GIDGoogleUser_Private.h b/GoogleSignIn/Sources/GIDGoogleUser_Private.h index a56996c9..3687bfa0 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser_Private.h +++ b/GoogleSignIn/Sources/GIDGoogleUser_Private.h @@ -31,6 +31,17 @@ NS_ASSUME_NONNULL_BEGIN // Internal methods for the class that are not part of the public API. @interface GIDGoogleUser () +@property(nonatomic, readwrite) GIDToken *accessToken; + +@property(nonatomic, readwrite) GIDToken *refreshToken; + +@property(nonatomic, readwrite, nullable) GIDToken *idToken; + +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" +@property(nonatomic, readwrite) id fetcherAuthorizer; +#pragma clang diagnostic pop + #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST // A string indicating support for Enterprise Mobility Management. @property(nonatomic, readonly, nullable) NSString *emmSupport; diff --git a/GoogleSignIn/Sources/GIDSignIn.m b/GoogleSignIn/Sources/GIDSignIn.m index 66fac4ef..e84682ee 100644 --- a/GoogleSignIn/Sources/GIDSignIn.m +++ b/GoogleSignIn/Sources/GIDSignIn.m @@ -22,7 +22,7 @@ #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDProfileData.h" #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDUserAuth.h" -#import "GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h" +#import "GoogleSignIn/Sources/GIDEMMSupport.h" #import "GoogleSignIn/Sources/GIDSignInInternalOptions.h" #import "GoogleSignIn/Sources/GIDSignInPreferences.h" #import "GoogleSignIn/Sources/GIDCallbackQueue.h" @@ -584,9 +584,9 @@ - (void)authenticateInteractivelyWithOptions:(GIDSignInInternalOptions *)options #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST [additionalParameters addEntriesFromDictionary: - [GIDAppAuthFetcherAuthorizationWithEMMSupport parametersWithParameters:options.extraParams - emmSupport:emmSupport - isPasscodeInfoRequired:NO]]; + [GIDEMMSupport parametersWithParameters:options.extraParams + emmSupport:emmSupport + isPasscodeInfoRequired:NO]]; #elif TARGET_OS_OSX || TARGET_OS_MACCATALYST [additionalParameters addEntriesFromDictionary:options.extraParams]; #endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST @@ -733,10 +733,9 @@ - (void)maybeFetchToken:(GIDAuthFlow *)authFlow { authState.lastAuthorizationResponse.additionalParameters; NSString *passcodeInfoRequired = (NSString *)params[kEMMPasscodeInfoRequiredKeyName]; [additionalParameters addEntriesFromDictionary: - [GIDAppAuthFetcherAuthorizationWithEMMSupport - parametersWithParameters:@{} - emmSupport:authFlow.emmSupport - isPasscodeInfoRequired:passcodeInfoRequired.length > 0]]; + [GIDEMMSupport parametersWithParameters:@{} + emmSupport:authFlow.emmSupport + isPasscodeInfoRequired:passcodeInfoRequired.length > 0]]; #endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST additionalParameters[kSDKVersionLoggingParameter] = GIDVersion(); additionalParameters[kEnvironmentLoggingParameter] = GIDEnvironment(); @@ -762,8 +761,7 @@ - (void)maybeFetchToken:(GIDAuthFlow *)authFlow { #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST if (authFlow.emmSupport) { - [GIDAppAuthFetcherAuthorizationWithEMMSupport - handleTokenFetchEMMError:error completion:^(NSError *error) { + [GIDEMMSupport handleTokenFetchEMMError:error completion:^(NSError *error) { authFlow.error = error; [authFlow next]; }]; diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index 8fa0fbb7..a5f0c21f 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -26,7 +26,7 @@ // Test module imports @import GoogleSignIn; -#import "GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.h" +#import "GoogleSignIn/Sources/GIDEMMSupport.h" #import "GoogleSignIn/Sources/GIDGoogleUser_Private.h" #import "GoogleSignIn/Sources/GIDSignIn_Private.h" #import "GoogleSignIn/Sources/GIDSignInPreferences.h" @@ -1070,10 +1070,9 @@ - (void)testTokenEndpointEMMError { NSError *emmError = [NSError errorWithDomain:@"anydomain" code:12345 userInfo:@{ OIDOAuthErrorFieldError : errorJSON }]; - id appAuthFetcherAuthorization = - OCMStrictClassMock([GIDAppAuthFetcherAuthorizationWithEMMSupport class]); - [[appAuthFetcherAuthorization expect] handleTokenFetchEMMError:emmError - completion:SAVE_TO_ARG_BLOCK(completion)]; + id emmSupport = OCMStrictClassMock([GIDEMMSupport class]); + [[emmSupport expect] handleTokenFetchEMMError:emmError + completion:SAVE_TO_ARG_BLOCK(completion)]; [self OAuthLoginWithAddScopesFlow:NO authError:nil @@ -1092,7 +1091,7 @@ - (void)testTokenEndpointEMMError { [self waitForExpectationsWithTimeout:1 handler:nil]; - [appAuthFetcherAuthorization verify]; + [emmSupport verify]; XCTAssertFalse(_keychainSaved, @"should not save to keychain"); XCTAssertTrue(_completionCalled, @"should call delegate"); XCTAssertNotNil(_authError, @"should have error"); From befe97774ee7de5cab1e7acbed77d77f6c667651 Mon Sep 17 00:00:00 2001 From: pinlu Date: Sun, 2 Oct 2022 21:40:10 -0700 Subject: [PATCH 16/17] Minor improvement. --- GoogleSignIn/Sources/GIDEMMSupport.m | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/GoogleSignIn/Sources/GIDEMMSupport.m b/GoogleSignIn/Sources/GIDEMMSupport.m index 654aaeb3..a796f5fb 100644 --- a/GoogleSignIn/Sources/GIDEMMSupport.m +++ b/GoogleSignIn/Sources/GIDEMMSupport.m @@ -67,9 +67,9 @@ + (void)handleTokenFetchEMMError:(nullable NSError *)error } + (NSDictionary *)updatedEMMParametersWithParameters:(NSDictionary *)parameters { - return [[self class] parametersWithParameters:parameters - emmSupport:parameters[kEMMSupportParameterName] - isPasscodeInfoRequired:parameters[kEMMPasscodeInfoParameterName] != nil]; + return [self parametersWithParameters:parameters + emmSupport:parameters[kEMMSupportParameterName] + isPasscodeInfoRequired:parameters[kEMMPasscodeInfoParameterName] != nil]; } From d8ae1af3c55b9112992c35dbb4745f26771db6d4 Mon Sep 17 00:00:00 2001 From: pinlu Date: Mon, 3 Oct 2022 14:49:17 -0700 Subject: [PATCH 17/17] Update GIDAppAuthFetcherAuthorizationWithEMMSupport.m --- ...ppAuthFetcherAuthorizationWithEMMSupport.m | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m b/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m index 2fa68e3f..814a73fc 100644 --- a/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m +++ b/GoogleSignIn/Sources/GIDAppAuthFetcherAuthorizationWithEMMSupport.m @@ -1,16 +1,18 @@ -// Copyright 2022 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ #import