From 0ff94920dccd31cc1dec44e29ce85cff7d94b0bc Mon Sep 17 00:00:00 2001 From: pinlu Date: Tue, 19 Jul 2022 18:09:34 -0700 Subject: [PATCH 1/3] Add the property GIDConfiguration to GIDGoogleUser - Add a GIDConfiguration to replace hostedDomain, serverClientID, openRealm. - Remove the property serverAuthCode. --- GoogleSignIn/Sources/GIDGoogleUser.m | 57 +++++++++++-------- GoogleSignIn/Sources/GIDSignIn.m | 12 +--- .../Public/GoogleSignIn/GIDGoogleUser.h | 15 +---- .../Tests/Unit/GIDGoogleUser+Testing.m | 8 +-- GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 5 +- GoogleSignIn/Tests/Unit/GIDSignInTest.m | 23 ++++---- .../Tests/Unit/OIDTokenResponse+Testing.m | 2 +- 7 files changed, 59 insertions(+), 63 deletions(-) diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index 34e636d6..094466bd 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -16,6 +16,8 @@ #import "GoogleSignIn/Sources/GIDAuthentication_Private.h" #import "GoogleSignIn/Sources/GIDProfileData_Private.h" +#import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDConfiguration.h" + #ifdef SWIFT_PACKAGE @import AppAuth; @@ -53,30 +55,6 @@ - (nullable NSString *)userID { return nil; } -- (nullable NSString *)hostedDomain { - NSString *idToken = [self idToken]; - if (idToken) { - OIDIDToken *idTokenDecoded = [[OIDIDToken alloc] initWithIDTokenString:idToken]; - if (idTokenDecoded && idTokenDecoded.claims[kHostedDomainIDTokenClaimKey]) { - return [idTokenDecoded.claims[kHostedDomainIDTokenClaimKey] copy]; - } - } - - return nil; -} - -- (nullable NSString *)serverAuthCode { - return [_authState.lastTokenResponse.additionalParameters[@"server_code"] copy]; -} - -- (nullable NSString *)serverClientID { - return [_authState.lastTokenResponse.request.additionalParameters[kAudienceParameter] copy]; -} - -- (nullable NSString *)openIDRealm { - return [_authState.lastTokenResponse.request.additionalParameters[kOpenIDRealmParameter] copy]; -} - - (nullable NSArray *)grantedScopes { NSArray *grantedScopes; NSString *grantedScopeString = _authState.lastTokenResponse.scope; @@ -111,14 +89,41 @@ - (void)updateAuthState:(OIDAuthState *)authState _authState = authState; _authentication = [[GIDAuthentication alloc] initWithAuthState:authState]; _profile = profileData; + _configuration = [[GIDConfiguration alloc] initWithClientID:[self clientID] + serverClientID:[self serverClientID] + hostedDomain:[self hostedDomain] + openIDRealm:[self openIDRealm]]; } #pragma mark - Helpers +- (NSString *)clientID { + return _authState.lastAuthorizationResponse.request.clientID; +} + +- (nullable NSString *)hostedDomain { + NSString *idToken = [self idToken]; + if (idToken) { + OIDIDToken *idTokenDecoded = [[OIDIDToken alloc] initWithIDTokenString:idToken]; + if (idTokenDecoded && idTokenDecoded.claims[kHostedDomainIDTokenClaimKey]) { + return [idTokenDecoded.claims[kHostedDomainIDTokenClaimKey] copy]; + } + } + return nil; +} + - (NSString *)idToken { return _authState ? _authState.lastTokenResponse.idToken : nil; } +- (nullable NSString *)serverClientID { + return [_authState.lastTokenResponse.request.additionalParameters[kAudienceParameter] copy]; +} + +- (nullable NSString *)openIDRealm { + return [_authState.lastTokenResponse.request.additionalParameters[kOpenIDRealmParameter] copy]; +} + #pragma mark - NSSecureCoding + (BOOL)supportsSecureCoding { @@ -137,6 +142,10 @@ - (nullable instancetype)initWithCoder:(NSCoder *)decoder { _authState = authentication.authState; } _authentication = [[GIDAuthentication alloc] initWithAuthState:_authState]; + _configuration = [[GIDConfiguration alloc] initWithClientID:[self clientID] + serverClientID:[self serverClientID] + hostedDomain:[self hostedDomain] + openIDRealm:[self openIDRealm]]; } return self; } diff --git a/GoogleSignIn/Sources/GIDSignIn.m b/GoogleSignIn/Sources/GIDSignIn.m index c9cc844b..f6e3280f 100644 --- a/GoogleSignIn/Sources/GIDSignIn.m +++ b/GoogleSignIn/Sources/GIDSignIn.m @@ -278,11 +278,7 @@ - (void)addScopes:(NSArray *)scopes return; } - GIDConfiguration *configuration = - [[GIDConfiguration alloc] initWithClientID:self.currentUser.authentication.clientID - serverClientID:self.currentUser.serverClientID - hostedDomain:self.currentUser.hostedDomain - openIDRealm:self.currentUser.openIDRealm]; + GIDConfiguration *configuration = self.currentUser.configuration; GIDSignInInternalOptions *options = [GIDSignInInternalOptions defaultOptionsWithConfiguration:configuration presentingViewController:presentingViewController @@ -371,11 +367,7 @@ - (void)addScopes:(NSArray *)scopes return; } - GIDConfiguration *configuration = - [[GIDConfiguration alloc] initWithClientID:self.currentUser.authentication.clientID - serverClientID:self.currentUser.serverClientID - hostedDomain:self.currentUser.hostedDomain - openIDRealm:self.currentUser.openIDRealm]; + GIDConfiguration *configuration = self.currentUser.configuration; GIDSignInInternalOptions *options = [GIDSignInInternalOptions defaultOptionsWithConfiguration:configuration presentingWindow:presentingWindow diff --git a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h index f5f130df..160a2fb5 100644 --- a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h +++ b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h @@ -19,6 +19,7 @@ NS_ASSUME_NONNULL_BEGIN @class GIDAuthentication; +@class GIDConfiguration; @class GIDProfileData; /// This class represents a user account. @@ -36,18 +37,8 @@ NS_ASSUME_NONNULL_BEGIN /// The API scopes granted to the app in an array of `NSString`. @property(nonatomic, readonly, nullable) NSArray *grantedScopes; -/// For Google Apps hosted accounts, the domain of the user. -@property(nonatomic, readonly, nullable) NSString *hostedDomain; - -/// The client ID of the home server. -@property(nonatomic, readonly, nullable) NSString *serverClientID; - -/// An OAuth2 authorization code for the home server. -@property(nonatomic, readonly, nullable) NSString *serverAuthCode; - -/// The OpenID2 realm of the home server. -@property(nonatomic, readonly, nullable) NSString *openIDRealm; - +/// The client configuration. +@property(nonatomic, readonly) GIDConfiguration *configuration; @end diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUser+Testing.m b/GoogleSignIn/Tests/Unit/GIDGoogleUser+Testing.m index 64e00fed..303452d0 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUser+Testing.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUser+Testing.m @@ -14,6 +14,7 @@ #import "GoogleSignIn/Tests/Unit/GIDGoogleUser+Testing.h" +#import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDConfiguration.h" #import "GoogleSignIn/Tests/Unit/GIDAuthentication+Testing.h" #import "GoogleSignIn/Tests/Unit/GIDProfileData+Testing.h" @@ -32,15 +33,14 @@ - (BOOL)isEqual:(id)object { - (BOOL)isEqualToGoogleUser:(GIDGoogleUser *)other { return [self.authentication isEqual:other.authentication] && [self.userID isEqual:other.userID] && - [self.serverAuthCode isEqual:other.serverAuthCode] && [self.profile isEqual:other.profile] && - [self.hostedDomain isEqual:other.hostedDomain]; + [self.configuration isEqual:other.configuration]; } // Not the hash implemention you want to use on prod, but just to match |isEqual:| here. - (NSUInteger)hash { - return [self.authentication hash] ^ [self.userID hash] ^ [self.serverAuthCode hash] ^ - [self.profile hash] ^ [self.hostedDomain hash]; + return [self.authentication hash] ^ [self.userID hash] ^ [self.configuration hash] ^ + [self.profile hash] ; } @end diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index 6dee9267..dea0abad 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -17,6 +17,7 @@ #import #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDAuthentication.h" +#import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDConfiguration.h" #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDProfileData.h" #import "GoogleSignIn/Sources/GIDAuthentication_Private.h" @@ -49,8 +50,8 @@ - (void)testInitWithAuthState { XCTAssertEqualObjects(user.authentication, authentication); XCTAssertEqualObjects(user.grantedScopes, @[ OIDAuthorizationRequestTestingScope2 ]); XCTAssertEqualObjects(user.userID, kUserID); - XCTAssertEqualObjects(user.hostedDomain, kHostedDomain); - XCTAssertEqualObjects(user.serverAuthCode, kServerAuthCode); + XCTAssertEqualObjects(user.configuration.hostedDomain, kHostedDomain); + XCTAssertEqualObjects(user.configuration.clientID, OIDAuthorizationRequestTestingClientID); XCTAssertEqualObjects(user.profile, [GIDProfileData testInstance]); } diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index 46e4d36e..f2464640 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -397,16 +397,21 @@ - (void)testShareInstance { - (void)testRestorePreviousSignInNoRefresh_hasPreviousUser { [[[_authorization expect] andReturn:_authState] authState]; OCMStub([_authState lastTokenResponse]).andReturn(_tokenResponse); - OCMStub([_tokenResponse scope]).andReturn(nil); - OCMStub([_tokenResponse additionalParameters]).andReturn(nil); - OCMStub([_tokenResponse idToken]).andReturn(kFakeIDToken); - OCMStub([_tokenResponse request]).andReturn(_tokenRequest); - OCMStub([_tokenRequest additionalParameters]).andReturn(nil); id idTokenDecoded = OCMClassMock([OIDIDToken class]); OCMStub([idTokenDecoded alloc]).andReturn(idTokenDecoded); OCMStub([idTokenDecoded initWithIDTokenString:OCMOCK_ANY]).andReturn(idTokenDecoded); OCMStub([idTokenDecoded subject]).andReturn(kFakeGaiaID); + + // Mock generating a GIDConfiguration when initializing GIDGoogleUser. + OIDAuthorizationResponse *authResponse = + [OIDAuthorizationResponse testInstanceWithAdditionalParameters:nil + errorString:nil]; + + OCMStub([_authState lastAuthorizationResponse]).andReturn(authResponse); + OCMStub([_tokenResponse idToken]).andReturn(kFakeIDToken); + OCMStub([_tokenResponse request]).andReturn(_tokenRequest); + OCMStub([_tokenRequest additionalParameters]).andReturn(nil); [_signIn restorePreviousSignInNoRefresh]; @@ -569,11 +574,9 @@ - (void)testAddScopes { OCMStub([profile email]).andReturn(kUserEmail); OCMStub([_user authentication]).andReturn(_authentication); - OCMStub([_authentication clientID]).andReturn(kClientId); - OCMStub([_user serverClientID]).andReturn(nil); - OCMStub([_user hostedDomain]).andReturn(nil); - - OCMStub([_user openIDRealm]).andReturn(kOpenIDRealm); + + // Mock for the method `addScopes`. + OCMStub([_user configuration]).andReturn(_configuration); OCMStub([_user profile]).andReturn(profile); OCMStub([_user grantedScopes]).andReturn(@[kGrantedScope]); diff --git a/GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.m b/GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.m index b8a5974d..1e1b95bf 100644 --- a/GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.m +++ b/GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.m @@ -112,7 +112,7 @@ + (NSString *)idTokenWithSub:(NSString *)sub exp:(NSNumber *)exp fat:(BOOL)fat { NSMutableDictionary *payloadContents = [NSMutableDictionary dictionaryWithDictionary:@{ @"sub" : sub, - @"hd" : kHostedDomain, + @"hd" : kHostedDomain, @"iss" : kIssuer, @"aud" : kAudience, @"exp" : exp, From a7a5cd8913a67b3deaa7377c036a365ad9d47b35 Mon Sep 17 00:00:00 2001 From: pinlu Date: Fri, 22 Jul 2022 16:32:39 -0700 Subject: [PATCH 2/3] Resolve comments. --- GoogleSignIn/Sources/GIDGoogleUser.m | 26 ++++++++++++------- .../Public/GoogleSignIn/GIDGoogleUser.h | 2 +- .../Tests/Unit/GIDConfiguration+Testing.m | 6 +++++ .../Tests/Unit/GIDGoogleUser+Testing.m | 1 + GoogleSignIn/Tests/Unit/GIDSignInTest.m | 4 +++ 5 files changed, 28 insertions(+), 11 deletions(-) diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index 094466bd..b4e07e08 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -12,12 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. +#import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h" + #import "GoogleSignIn/Sources/GIDGoogleUser_Private.h" -#import "GoogleSignIn/Sources/GIDAuthentication_Private.h" -#import "GoogleSignIn/Sources/GIDProfileData_Private.h" #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDConfiguration.h" +#import "GoogleSignIn/Sources/GIDAuthentication_Private.h" +#import "GoogleSignIn/Sources/GIDProfileData_Private.h" #ifdef SWIFT_PACKAGE @import AppAuth; @@ -73,6 +75,18 @@ - (nullable NSString *)userID { return grantedScopes; } +- (GIDConfiguration *)configuration { + __block GIDConfiguration *configuration; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + configuration = [[GIDConfiguration alloc] initWithClientID:[self clientID] + serverClientID:[self serverClientID] + hostedDomain:[self hostedDomain] + openIDRealm:[self openIDRealm]]; + }); + return configuration; +} + #pragma mark - Private Methods - (instancetype)initWithAuthState:(OIDAuthState *)authState @@ -89,10 +103,6 @@ - (void)updateAuthState:(OIDAuthState *)authState _authState = authState; _authentication = [[GIDAuthentication alloc] initWithAuthState:authState]; _profile = profileData; - _configuration = [[GIDConfiguration alloc] initWithClientID:[self clientID] - serverClientID:[self serverClientID] - hostedDomain:[self hostedDomain] - openIDRealm:[self openIDRealm]]; } #pragma mark - Helpers @@ -142,10 +152,6 @@ - (nullable instancetype)initWithCoder:(NSCoder *)decoder { _authState = authentication.authState; } _authentication = [[GIDAuthentication alloc] initWithAuthState:_authState]; - _configuration = [[GIDConfiguration alloc] initWithClientID:[self clientID] - serverClientID:[self serverClientID] - hostedDomain:[self hostedDomain] - openIDRealm:[self openIDRealm]]; } return self; } diff --git a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h index 160a2fb5..9a17af70 100644 --- a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h +++ b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h @@ -37,7 +37,7 @@ NS_ASSUME_NONNULL_BEGIN /// The API scopes granted to the app in an array of `NSString`. @property(nonatomic, readonly, nullable) NSArray *grantedScopes; -/// The client configuration. +/// The configuration that was used to sign in this user. @property(nonatomic, readonly) GIDConfiguration *configuration; @end diff --git a/GoogleSignIn/Tests/Unit/GIDConfiguration+Testing.m b/GoogleSignIn/Tests/Unit/GIDConfiguration+Testing.m index 8fd52fc1..72bc9106 100644 --- a/GoogleSignIn/Tests/Unit/GIDConfiguration+Testing.m +++ b/GoogleSignIn/Tests/Unit/GIDConfiguration+Testing.m @@ -46,6 +46,12 @@ - (BOOL)isEqualToConfiguration:(GIDConfiguration *)other { self.openIDRealm == other.openIDRealm); } +// Not the hash implemention you want to use on prod, but just to match |isEqual:| here. +- (NSUInteger)hash { + return [self.clientID hash] ^ [self.serverClientID hash] ^ [self.hostedDomain hash] ^ + [self.openIDRealm hash]; +} + + (instancetype)testInstance { return [[GIDConfiguration alloc] initWithClientID:OIDAuthorizationRequestTestingClientID serverClientID:kServerClientID diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUser+Testing.m b/GoogleSignIn/Tests/Unit/GIDGoogleUser+Testing.m index 303452d0..de89f8d3 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUser+Testing.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUser+Testing.m @@ -16,6 +16,7 @@ #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDConfiguration.h" #import "GoogleSignIn/Tests/Unit/GIDAuthentication+Testing.h" +#import "GoogleSignIn/Tests/Unit/GIDConfiguration+Testing.h" #import "GoogleSignIn/Tests/Unit/GIDProfileData+Testing.h" @implementation GIDGoogleUser (Testing) diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index f2464640..c37fed3c 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -418,6 +418,8 @@ - (void)testRestorePreviousSignInNoRefresh_hasPreviousUser { [_authorization verify]; [_authState verify]; [_tokenResponse verify]; + [_tokenRequest verify]; + [idTokenDecoded verify]; XCTAssertEqual(_signIn.currentUser.userID, kFakeGaiaID); [idTokenDecoded stopMocking]; @@ -606,6 +608,8 @@ - (void)testAddScopes { NSArray *expectedScopes = @[kNewScope, kGrantedScope]; XCTAssertEqualObjects(grantedScopes, expectedScopes); + [_user verify]; + [profile verify]; [profile stopMocking]; } From 6b366800f6a2cf533b50e9899f403015686109ec Mon Sep 17 00:00:00 2001 From: pinlu Date: Mon, 25 Jul 2022 12:29:59 -0700 Subject: [PATCH 3/3] Fix the getter of GIDConfiguration --- GoogleSignIn/Sources/GIDGoogleUser.m | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index b4e07e08..8cefa6fa 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -43,6 +43,7 @@ @implementation GIDGoogleUser { OIDAuthState *_authState; + GIDConfiguration *_cachedConfiguration; } - (nullable NSString *)userID { @@ -76,15 +77,17 @@ - (nullable NSString *)userID { } - (GIDConfiguration *)configuration { - __block GIDConfiguration *configuration; - static dispatch_once_t onceToken; - dispatch_once(&onceToken, ^{ - configuration = [[GIDConfiguration alloc] initWithClientID:[self clientID] - serverClientID:[self serverClientID] - hostedDomain:[self hostedDomain] - openIDRealm:[self openIDRealm]]; - }); - return configuration; + @synchronized(self) { + // Caches the configuration since it would not change for one GIDGoogleUser instance. + if (!_cachedConfiguration) { + _cachedConfiguration = [[GIDConfiguration alloc] initWithClientID:[self clientID] + serverClientID:[self serverClientID] + hostedDomain:[self hostedDomain] + openIDRealm:[self openIDRealm]]; + }; + } + + return _cachedConfiguration; } #pragma mark - Private Methods