From e68d38af662891b5e8af78e4c91d39ec896f5329 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:56:10 -0700 Subject: [PATCH 1/6] g-orchestrated: Move granted scopes, configuration and profile into the snapshot --- CHANGELOG.md | 5 +- GoogleSignIn/Sources/GIDGoogleUser.m | 158 ++++++++++++------- GoogleSignIn/Sources/GIDGoogleUser_Private.h | 21 ++- GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 68 ++++++++ GoogleSignIn/Tests/Unit/GIDSignInTest.m | 4 +- 5 files changed, 189 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a19b39d..fa5d4193 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,10 @@ - Add Swift Package Manager support for AppCheckCore 12.x, enabling compatibility with Firebase 13. CocoaPods installations remain restricted to AppCheckCore 11.x. ([#626](https://github.com/google/GoogleSignIn-iOS/pull/626)) - Fix a failed App Check preparation being recorded as successful, which made later calls to `-[GIDSignIn configureWithCompletion:]` report success without retrying. ([#626](https://github.com/google/GoogleSignIn-iOS/pull/626)) - With AppCheckCore 11.x (including all CocoaPods installations), a custom App Check provider that returns neither a token nor an error is now treated as success instead of failing with `kGIDAppCheckUnexpectedError`. ([#626](https://github.com/google/GoogleSignIn-iOS/pull/626)) -- Fix a data race on `GIDGoogleUser`'s tokens. Concurrent token updates could write the access, refresh and ID tokens from different threads at once, and readers could observe a partially updated set. The three tokens are now published together as one snapshot. -- `GIDGoogleUser`'s `accessToken`, `refreshToken` and `idToken` are now all derived from that snapshot, so key-value observers of any one of them are notified whenever any of them changes, not only when that property's own value changes. +- Fix a data race on `GIDGoogleUser`'s tokens. Concurrent token updates could write the access, refresh and ID tokens from different threads at once, and readers could observe a partially updated set. The tokens, granted scopes, configuration and profile are now published together as one snapshot. +- `GIDGoogleUser`'s `accessToken`, `refreshToken`, `idToken` and `profile` are now all derived from that snapshot, so key-value observers of any one of them are notified whenever any of them changes, not only when that property's own value changes. `profile` now sends key-value notifications; previously it sent none. - Key-value observers of those properties are notified while `GIDGoogleUser` holds its internal lock. Calling back into the same user from the observer is fine, but an observer must not synchronously wait (for example with `dispatch_sync`) on another thread that is updating the same user. +- `GIDGoogleUser`'s `configuration` is now computed once, when the user is created, and stays fixed for the user's lifetime. Previously it was computed on first access, so a first read after a token update could reflect that update's client ID, server client ID or hosted domain. - Fix EMM remediation dialogs being suppressed for the rest of the app's lifetime after a single error arrived while no key window was available. The pending-dialog flag is now cleared on every exit path, and its clearing write is synchronized to match the read. - Fix a token request that failed with an EMM error sometimes being reported with the underlying OAuth error instead of `kGIDSignInErrorCodeEMM` when no remediation dialog could be presented. - Fix an EMM error that arrived while a remediation dialog was already showing being reported as `kGIDSignInErrorCodeUnknown` (or as the underlying OAuth error, for a token request) instead of `kGIDSignInErrorCodeEMM`. No second dialog is shown. diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index 5310cb99..3cc8d6c0 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -65,24 +65,62 @@ @implementation GIDGoogleUserTokens - (instancetype)initWithAccessToken:(GIDToken *)accessToken refreshToken:(GIDToken *)refreshToken - idToken:(nullable GIDToken *)idToken { + idToken:(nullable GIDToken *)idToken + grantedScopes:(nullable NSArray *)grantedScopes + configuration:(GIDConfiguration *)configuration + profile:(nullable GIDProfileData *)profile { self = [super init]; if (self) { _accessToken = accessToken; _refreshToken = refreshToken; _idToken = idToken; + _grantedScopes = [grantedScopes copy]; + _configuration = configuration; + _profile = profile; } return self; } @end +// Parses the `scope` parameter returned by the backend, which is authoritative when present. +static NSArray *_Nullable GIDGrantedScopesFromScopeString( + NSString *_Nullable scopeString) { + if (!scopeString) { + return nil; + } + // Remove leading and trailing whitespace. + scopeString = + [scopeString stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceCharacterSet]]; + // Tokenize with space as a delimiter. + NSMutableArray *parsedScopes = + [[scopeString componentsSeparatedByString:@" "] mutableCopy]; + // Remove empty strings. + [parsedScopes removeObject:@""]; + return [parsedScopes copy]; +} + +// Returns the hosted domain claim of the given ID token, if it has one. +static NSString *_Nullable GIDHostedDomainFromIDTokenString(NSString *_Nullable idTokenString) { + if (idTokenString) { + OIDIDToken *idTokenDecoded = [[OIDIDToken alloc] initWithIDTokenString:idTokenString]; + if (idTokenDecoded && idTokenDecoded.claims[kHostedDomainIDTokenClaimKey]) { + return idTokenDecoded.claims[kHostedDomainIDTokenClaimKey]; + } + } + return nil; +} + @implementation GIDGoogleUser { - GIDConfiguration *_cachedConfiguration; - // A queue for pending token refresh handlers so we don't fire multiple requests in parallel. // Access to this ivar should be synchronized. NSMutableArray *_tokenRefreshHandlerQueue; + + // A pending profile set by -initWithAuthState:profileData: or + // -updateWithTokenResponse:authorizationResponse:profileData:, folded into the next snapshot. + // Guarded by `@synchronized(self)`. + GIDProfileData *_pendingProfile; + BOOL _hasPendingProfile; } - (GIDToken *)accessToken { @@ -97,8 +135,13 @@ - (nullable GIDToken *)idToken { return self.tokens.idToken; } -// The three token properties are derived from `tokens`, so KVO observers of each one are notified -// whenever `tokens` is replaced (a change to any of them notifies observers of all three). +- (nullable GIDProfileData *)profile { + return self.tokens.profile; +} + +// The token and profile properties are derived from `tokens`, so KVO observers of each one are +// notified whenever `tokens` is replaced (a change to any of them notifies observers of all of +// them). + (NSSet *)keyPathsForValuesAffectingAccessToken { return [NSSet setWithObject:NSStringFromSelector(@selector(tokens))]; } @@ -111,6 +154,10 @@ - (nullable GIDToken *)idToken { return [NSSet setWithObject:NSStringFromSelector(@selector(tokens))]; } ++ (NSSet *)keyPathsForValuesAffectingProfile { + return [NSSet setWithObject:NSStringFromSelector(@selector(tokens))]; +} + - (nullable NSString *)userID { NSString *idTokenString = self.idToken.tokenString; if (idTokenString) { @@ -123,40 +170,11 @@ - (nullable NSString *)userID { } - (nullable NSArray *)grantedScopes { - NSArray *grantedScopes; - NSString *grantedScopeString = self.authState.lastTokenResponse.scope; - if (grantedScopeString) { - // If we have a 'scope' parameter from the backend, this is authoritative. - // Remove leading and trailing whitespace. - grantedScopeString = [grantedScopeString stringByTrimmingCharactersInSet: - [NSCharacterSet whitespaceCharacterSet]]; - // Tokenize with space as a delimiter. - NSMutableArray *parsedScopes = - [[grantedScopeString componentsSeparatedByString:@" "] mutableCopy]; - // Remove empty strings. - [parsedScopes removeObject:@""]; - grantedScopes = [parsedScopes copy]; - } - return grantedScopes; + return self.tokens.grantedScopes; } - (GIDConfiguration *)configuration { - @synchronized(self) { - // Caches the configuration since it would not change for one GIDGoogleUser instance. - if (!_cachedConfiguration) { - NSString *clientID = self.authState.lastAuthorizationResponse.request.clientID; - NSString *serverClientID = - self.authState.lastTokenResponse.request.additionalParameters[kAudienceParameter]; - NSString *openIDRealm = - self.authState.lastTokenResponse.request.additionalParameters[kOpenIDRealmParameter]; - - _cachedConfiguration = [[GIDConfiguration alloc] initWithClientID:clientID - serverClientID:serverClientID - hostedDomain:[self hostedDomain] - openIDRealm:openIDRealm]; - }; - } - return _cachedConfiguration; + return self.tokens.configuration; } - (void)refreshTokensIfNeededWithCompletion:(GIDGoogleUserCompletion)completion { @@ -291,7 +309,8 @@ - (instancetype)initWithAuthState:(OIDAuthState *)authState self = [super init]; if (self) { _tokenRefreshHandlerQueue = [[NSMutableArray alloc] init]; - _profile = profileData; + _pendingProfile = profileData; + _hasPendingProfile = YES; GTMAuthSession *authSession = [[GTMAuthSession alloc] initWithAuthState:authState]; #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST @@ -310,8 +329,9 @@ - (void)updateWithTokenResponse:(OIDTokenResponse *)tokenResponse authorizationResponse:(OIDAuthorizationResponse *)authorizationResponse profileData:(nullable GIDProfileData *)profileData { @synchronized(self) { - _profile = profileData; - + _pendingProfile = profileData; + _hasPendingProfile = YES; + // We don't want to trigger the delegate before we update authState completely. So we unset the // delegate before the first update. Also the order of updates is important because // `updateWithAuthorizationResponse` would clear the last token reponse and refresh token. @@ -320,15 +340,21 @@ - (void)updateWithTokenResponse:(OIDTokenResponse *)tokenResponse [self.authState updateWithAuthorizationResponse:authorizationResponse error:nil]; self.authState.stateChangeDelegate = self; [self.authState updateWithTokenResponse:tokenResponse error:nil]; + if (_hasPendingProfile) { + [self updateTokensWithAuthState:self.authState]; + } } } -// KVO observers of the token properties run while `@synchronized(self)` is held; same-thread -// re-entry is fine because it is recursive, but an observer must not synchronously wait on another -// thread that is updating the same user. +// KVO observers of the token and profile properties run while `@synchronized(self)` is held; +// same-thread re-entry is fine because it is recursive, but an observer must not synchronously +// wait on another thread that is updating the same user. - (void)updateTokensWithAuthState:(OIDAuthState *)authState { @synchronized(self) { GIDGoogleUserTokens *current = self.tokens; + GIDProfileData *profile = _hasPendingProfile ? _pendingProfile : current.profile; + _pendingProfile = nil; + _hasPendingProfile = NO; // Build the access token GIDToken *accessToken = @@ -360,6 +386,27 @@ - (void)updateTokensWithAuthState:(OIDAuthState *)authState { idToken = nil; } + NSArray *grantedScopes = + GIDGrantedScopesFromScopeString(authState.lastTokenResponse.scope); + + GIDConfiguration *configuration; + if (current.configuration) { + // The configuration is fixed for the lifetime of a user, so it is computed once. + configuration = current.configuration; + } else { + NSString *clientID = authState.lastAuthorizationResponse.request.clientID; + NSString *serverClientID = + authState.lastTokenResponse.request.additionalParameters[kAudienceParameter]; + NSString *openIDRealm = + authState.lastTokenResponse.request.additionalParameters[kOpenIDRealmParameter]; + + configuration = [[GIDConfiguration alloc] + initWithClientID:clientID + serverClientID:serverClientID + hostedDomain:GIDHostedDomainFromIDTokenString(idToken.tokenString) + openIDRealm:openIDRealm]; + } + // If the computed values are equal to the existing ones, keep the existing ones. In that case, // an update leaves `tokens` untouched and sends no KVO notifications. if ([current.accessToken isEqualToToken:accessToken]) { @@ -372,30 +419,25 @@ - (void)updateTokensWithAuthState:(OIDAuthState *)authState { idToken = current.idToken; } + BOOL grantedScopesChanged = !((!grantedScopes && !current.grantedScopes) || + [grantedScopes isEqualToArray:current.grantedScopes]); + if (!current || accessToken != current.accessToken || refreshToken != current.refreshToken || - idToken != current.idToken) { + idToken != current.idToken || + grantedScopesChanged || + profile != current.profile) { self.tokens = [[GIDGoogleUserTokens alloc] initWithAccessToken:accessToken refreshToken:refreshToken - idToken:idToken]; + idToken:idToken + grantedScopes:grantedScopes + configuration:configuration + profile:profile]; } } } -#pragma mark - Helpers - -- (nullable NSString *)hostedDomain { - NSString *idTokenString = self.idToken.tokenString; - if (idTokenString) { - OIDIDToken *idTokenDecoded = [[OIDIDToken alloc] initWithIDTokenString:idTokenString]; - if (idTokenDecoded && idTokenDecoded.claims[kHostedDomainIDTokenClaimKey]) { - return idTokenDecoded.claims[kHostedDomainIDTokenClaimKey]; - } - } - return nil; -} - #pragma mark - OIDAuthStateChangeDelegate - (void)didChangeState:(OIDAuthState *)state { @@ -425,7 +467,7 @@ - (nullable instancetype)initWithCoder:(NSCoder *)decoder { } - (void)encodeWithCoder:(NSCoder *)encoder { - [encoder encodeObject:_profile forKey:kProfileDataKey]; + [encoder encodeObject:self.profile forKey:kProfileDataKey]; [encoder encodeObject:self.authState forKey:kAuthStateKey]; } diff --git a/GoogleSignIn/Sources/GIDGoogleUser_Private.h b/GoogleSignIn/Sources/GIDGoogleUser_Private.h index b6ec421c..343f4baf 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser_Private.h +++ b/GoogleSignIn/Sources/GIDGoogleUser_Private.h @@ -22,6 +22,8 @@ #import #endif +@class GIDConfiguration; +@class GIDProfileData; @class GIDToken; @class OIDAuthState; @@ -30,17 +32,24 @@ NS_ASSUME_NONNULL_BEGIN /// A completion block that takes a `GIDGoogleUser` or an error if the attempt to refresh tokens was unsuccessful. typedef void (^GIDGoogleUserCompletion)(GIDGoogleUser *_Nullable user, NSError *_Nullable error); -/// An immutable snapshot of a user's access, refresh and ID tokens. -/// This value is replaced as a whole so that readers never see a mix of old and new tokens. +/// An immutable snapshot of a user's tokens, profile, and the values derived from the same +/// auth state. +/// This value is replaced as a whole so that readers never see a mix of old and new values. @interface GIDGoogleUserTokens : NSObject @property(nonatomic, readonly) GIDToken *accessToken; @property(nonatomic, readonly) GIDToken *refreshToken; @property(nonatomic, readonly, nullable) GIDToken *idToken; +@property(nonatomic, readonly, nullable) NSArray *grantedScopes; +@property(nonatomic, readonly) GIDConfiguration *configuration; +@property(nonatomic, readonly, nullable) GIDProfileData *profile; - (instancetype)initWithAccessToken:(GIDToken *)accessToken refreshToken:(GIDToken *)refreshToken - idToken:(nullable GIDToken *)idToken NS_DESIGNATED_INITIALIZER; + idToken:(nullable GIDToken *)idToken + grantedScopes:(nullable NSArray *)grantedScopes + configuration:(GIDConfiguration *)configuration + profile:(nullable GIDProfileData *)profile NS_DESIGNATED_INITIALIZER; - (instancetype)init NS_UNAVAILABLE; @end @@ -51,9 +60,9 @@ typedef void (^GIDGoogleUserCompletion)(GIDGoogleUser *_Nullable user, NSError * /// A representation of the state of the OAuth session for this instance. @property(nonatomic, readonly) OIDAuthState *authState; -/// The user's current tokens. Read once - accessing individual properties in sequence is not -/// recommended. Reading once ensures that each property is from the same update. Writes are -/// serialized by `@synchronized(self)`. +/// The user's current tokens, granted scopes, configuration and profile. Read once - accessing +/// individual properties in sequence is not recommended. Reading once ensures that each property is +/// from the same update. Writes are serialized by `@synchronized(self)`. @property(atomic, strong, nullable) GIDGoogleUserTokens *tokens; #pragma clang diagnostic push diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index d5e14f92..3ffa3a68 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -247,6 +247,51 @@ - (void)testUpdateAuthState_tokensAreNotChanged { XCTAssertIdentical(user.refreshToken, refreshTokenBeforeUpdate); } +- (void)testUpdateAuthState_publishesProfileWithTokens { + NSString *idToken = [self idTokenWithExpiresIn:kIDTokenExpiresIn]; + OIDAuthState *authState = [OIDAuthState testInstanceWithIDToken:idToken + accessToken:kAccessToken + accessTokenExpiresIn:kAccessTokenExpiresIn + refreshToken:kRefreshToken]; + + GIDGoogleUser *user = [[GIDGoogleUser alloc] initWithAuthState:authState profileData:nil]; + + GIDToken *accessTokenBeforeUpdate = user.accessToken; + GIDToken *refreshTokenBeforeUpdate = user.refreshToken; + GIDToken *idTokenBeforeUpdate = user.idToken; + GIDProfileData *updatedProfileData = [GIDProfileData testInstance]; + + [user updateWithTokenResponse:authState.lastTokenResponse + authorizationResponse:authState.lastAuthorizationResponse + profileData:updatedProfileData]; + + XCTAssertEqual(user.profile, updatedProfileData); + XCTAssertIdentical(user.accessToken, accessTokenBeforeUpdate); + XCTAssertIdentical(user.idToken, idTokenBeforeUpdate); + XCTAssertIdentical(user.refreshToken, refreshTokenBeforeUpdate); +} + +- (void)testConfiguration_isFixedAfterTokenUpdate { + // The configuration is computed once at init and is fixed for the user's lifetime. + GIDGoogleUser *user = [self googleUserWithAccessTokenExpiresIn:kAccessTokenExpiresIn + idTokenExpiresIn:kIDTokenExpiresIn]; + GIDConfiguration *configurationBefore = user.configuration; + + NSString *updatedIDToken = [self idTokenWithExpiresIn:kNewIDTokenExpiresIn]; + OIDAuthState *updatedAuthState = [OIDAuthState testInstanceWithIDToken:updatedIDToken + accessToken:kNewAccessToken + accessTokenExpiresIn:kAccessTokenExpiresIn + refreshToken:kNewRefreshToken]; + + [user updateWithTokenResponse:updatedAuthState.lastTokenResponse + authorizationResponse:updatedAuthState.lastAuthorizationResponse + profileData:nil]; + + XCTAssertIdentical(user.configuration, configurationBefore); + XCTAssertEqualObjects(user.configuration.clientID, OIDAuthorizationRequestTestingClientID); + XCTAssertEqualObjects(user.configuration.hostedDomain, kHostedDomain); +} + - (void)testUpdateTokens_concurrentUpdates_leaveConsistentTokenSet { GIDGoogleUser *user = [self googleUserWithAccessTokenExpiresIn:kAccessTokenExpiresIn idTokenExpiresIn:kIDTokenExpiresIn]; @@ -542,6 +587,29 @@ - (void)testTokenObserver_reentersUserDuringUpdate { XCTAssertEqualObjects(observedTokenStrings.lastObject, @"access_token_B"); } +- (void)testProfile_notifiesKVOObserversOnUpdate { + GIDGoogleUser *user = [self googleUserWithAccessTokenExpiresIn:kAccessTokenExpiresIn + idTokenExpiresIn:kIDTokenExpiresIn]; + + NSString *profileKeyPath = NSStringFromSelector(@selector(profile)); + GIDGoogleUserTestKVOObserver *observer = [[GIDGoogleUserTestKVOObserver alloc] init]; + __block NSInteger notificationCount = 0; + observer.onChange = ^{ + notificationCount += 1; + }; + + [user addObserver:observer forKeyPath:profileKeyPath options:0 context:NULL]; + + OIDAuthState *authState = [OIDAuthState testInstance]; + [user updateWithTokenResponse:authState.lastTokenResponse + authorizationResponse:authState.lastAuthorizationResponse + profileData:[GIDProfileData testInstance]]; + + [user removeObserver:observer forKeyPath:profileKeyPath context:NULL]; + + XCTAssertGreaterThanOrEqual(notificationCount, 1); +} + - (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 diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index b633cc32..adcad2d5 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -535,7 +535,8 @@ - (void)testRestorePreviousSignInNoRefresh_hasPreviousUser { OCMStub([idTokenDecoded initWithIDTokenString:OCMOCK_ANY]).andReturn(idTokenDecoded); OCMStub([idTokenDecoded subject]).andReturn(kFakeGaiaID); - // Mock generating a GIDConfiguration when initializing GIDGoogleUser. + // Mock generating the token snapshot (tokens, granted scopes and GIDConfiguration) when + // initializing GIDGoogleUser. OIDAuthorizationResponse *authResponse = [OIDAuthorizationResponse testInstance]; @@ -545,6 +546,7 @@ - (void)testRestorePreviousSignInNoRefresh_hasPreviousUser { OCMStub([_tokenRequest additionalParameters]).andReturn(nil); OCMStub([_tokenResponse accessToken]).andReturn(kAccessToken); OCMStub([_tokenResponse accessTokenExpirationDate]).andReturn(nil); + OCMStub([_tokenResponse scope]).andReturn(nil); [_signIn restorePreviousSignInNoRefresh]; From 033bc244813ee7a09226ff56bf8d1d3dbc008a77 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:11:09 -0700 Subject: [PATCH 2/6] g-orchestrated: Test that granted scopes update with the snapshot --- GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 36 +++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index 3ffa3a68..80d81da9 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -46,6 +46,7 @@ #import #import #import +#import #import #import #import @@ -271,6 +272,23 @@ - (void)testUpdateAuthState_publishesProfileWithTokens { XCTAssertIdentical(user.refreshToken, refreshTokenBeforeUpdate); } +- (void)testUpdateAuthState_publishesGrantedScopesWithTokens { + GIDGoogleUser *user = [self googleUserWithAccessTokenExpiresIn:kAccessTokenExpiresIn + idTokenExpiresIn:kIDTokenExpiresIn]; + XCTAssertEqualObjects(user.grantedScopes, @[ OIDAuthorizationRequestTestingScope2 ]); + + NSArray *updatedScopes = @[ OIDAuthorizationRequestTestingScope2, kNewScope ]; + OIDAuthState *updatedAuthState = [self authStateWithGrantedScopes:updatedScopes + accessToken:kNewAccessToken]; + + [user updateWithTokenResponse:updatedAuthState.lastTokenResponse + authorizationResponse:updatedAuthState.lastAuthorizationResponse + profileData:nil]; + + XCTAssertEqualObjects(user.grantedScopes, updatedScopes); + XCTAssertEqualObjects(user.accessToken.tokenString, kNewAccessToken); +} + - (void)testConfiguration_isFixedAfterTokenUpdate { // The configuration is computed once at init and is fixed for the user's lifetime. GIDGoogleUser *user = [self googleUserWithAccessTokenExpiresIn:kAccessTokenExpiresIn @@ -1079,6 +1097,24 @@ - (GIDGoogleUser *)googleUserWithAccessTokenExpiresIn:(NSTimeInterval)accessToke return [[GIDGoogleUser alloc] initWithAuthState:authState profileData:nil]; } +// Returns an auth state whose token response grants `scopes`. The OIDTokenResponse test helpers +// always grant OIDAuthorizationRequestTestingScope2, so this builds the response directly. +- (OIDAuthState *)authStateWithGrantedScopes:(NSArray *)scopes + accessToken:(NSString *)accessToken { + NSDictionary *> *parameters = @{ + @"access_token" : accessToken, + @"expires_in" : @(kAccessTokenExpiresIn), + @"token_type" : @"example_token_type", + @"refresh_token" : kNewRefreshToken, + @"scope" : [OIDScopeUtilities scopesWithArray:scopes], + @"id_token" : [self idTokenWithExpiresIn:kNewIDTokenExpiresIn], + }; + OIDTokenResponse *tokenResponse = + [[OIDTokenResponse alloc] initWithRequest:[OIDTokenRequest testInstance] + parameters:parameters]; + return [OIDAuthState testInstanceWithTokenResponse:tokenResponse]; +} + - (NSString *)idTokenWithExpiresIn:(NSTimeInterval)expiresIn { // The expireTime should be based on 1970. NSTimeInterval expireTime = [[NSDate date] timeIntervalSince1970] + expiresIn; From faf5640287784a4bd56a8945cde21be11f28a394 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:35:11 -0700 Subject: [PATCH 3/6] g-orchestrated: Address review comments on the snapshot change --- GoogleSignIn/Sources/GIDGoogleUser.m | 33 ++++++++++----- .../Public/GoogleSignIn/GIDGoogleUser.h | 6 +++ GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 41 +++++++++++++++++-- 3 files changed, 66 insertions(+), 14 deletions(-) diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index 3cc8d6c0..f020dc48 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -83,7 +83,10 @@ - (instancetype)initWithAccessToken:(GIDToken *)accessToken @end -// Parses the `scope` parameter returned by the backend, which is authoritative when present. +// Parses the `scope` parameter of a token response, which is authoritative when present +// (https://datatracker.ietf.org/doc/html/rfc6749#section-5.1). Scopes are space-delimited +// (https://datatracker.ietf.org/doc/html/rfc6749#section-3.3); surrounding whitespace and runs of +// spaces are tolerated. Returns nil when the response carries no `scope` parameter. static NSArray *_Nullable GIDGrantedScopesFromScopeString( NSString *_Nullable scopeString) { if (!scopeString) { @@ -116,9 +119,11 @@ @implementation GIDGoogleUser { // Access to this ivar should be synchronized. NSMutableArray *_tokenRefreshHandlerQueue; - // A pending profile set by -initWithAuthState:profileData: or - // -updateWithTokenResponse:authorizationResponse:profileData:, folded into the next snapshot. - // Guarded by `@synchronized(self)`. + // The profile is not part of the auth state, so a profile supplied to + // -initWithAuthState:profileData: or -updateWithTokenResponse:authorizationResponse:profileData: + // is parked here and folded into the next snapshot that -updateTokensWithAuthState: builds. + // `_hasPendingProfile` distinguishes a supplied nil profile, which replaces the current one, + // from no profile having been supplied, which keeps the current one. GIDProfileData *_pendingProfile; BOOL _hasPendingProfile; } @@ -309,6 +314,8 @@ - (instancetype)initWithAuthState:(OIDAuthState *)authState self = [super init]; if (self) { _tokenRefreshHandlerQueue = [[NSMutableArray alloc] init]; + + // Park the profile for the first snapshot, built by -updateTokensWithAuthState: below. _pendingProfile = profileData; _hasPendingProfile = YES; @@ -329,6 +336,8 @@ - (void)updateWithTokenResponse:(OIDTokenResponse *)tokenResponse authorizationResponse:(OIDAuthorizationResponse *)authorizationResponse profileData:(nullable GIDProfileData *)profileData { @synchronized(self) { + // Park the profile for the snapshot built when the token response update below calls back + // through -didChangeState:. _pendingProfile = profileData; _hasPendingProfile = YES; @@ -340,15 +349,14 @@ - (void)updateWithTokenResponse:(OIDTokenResponse *)tokenResponse [self.authState updateWithAuthorizationResponse:authorizationResponse error:nil]; self.authState.stateChangeDelegate = self; [self.authState updateWithTokenResponse:tokenResponse error:nil]; + // AppAuth does not call -didChangeState: for a nil token response, so if the profile was not + // folded into a snapshot above, build one here. if (_hasPendingProfile) { [self updateTokensWithAuthState:self.authState]; } } } -// KVO observers of the token and profile properties run while `@synchronized(self)` is held; -// same-thread re-entry is fine because it is recursive, but an observer must not synchronously -// wait on another thread that is updating the same user. - (void)updateTokensWithAuthState:(OIDAuthState *)authState { @synchronized(self) { GIDGoogleUserTokens *current = self.tokens; @@ -418,16 +426,19 @@ - (void)updateTokensWithAuthState:(OIDAuthState *)authState { if ([current.idToken isEqualToToken:idToken]) { idToken = current.idToken; } - - BOOL grantedScopesChanged = !((!grantedScopes && !current.grantedScopes) || - [grantedScopes isEqualToArray:current.grantedScopes]); + if ((!grantedScopes && !current.grantedScopes) || + [grantedScopes isEqualToArray:current.grantedScopes]) { + grantedScopes = current.grantedScopes; + } if (!current || accessToken != current.accessToken || refreshToken != current.refreshToken || idToken != current.idToken || - grantedScopesChanged || + grantedScopes != current.grantedScopes || profile != current.profile) { + // Replacing `tokens` notifies KVO observers of the derived properties while the lock is + // held; see the key-value observing note on GIDGoogleUser in GIDGoogleUser.h. self.tokens = [[GIDGoogleUserTokens alloc] initWithAccessToken:accessToken refreshToken:refreshToken idToken:idToken diff --git a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h index a7026366..9a8ce6bd 100644 --- a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h +++ b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDGoogleUser.h @@ -33,6 +33,12 @@ NS_ASSUME_NONNULL_BEGIN /// This class represents a signed-in user. +/// +/// The `accessToken`, `refreshToken`, `idToken` and `profile` properties are published together, +/// so key-value observers of any one of them are notified whenever any of them changes. Observers +/// are notified while the user holds an internal lock: an observer may call back into the same +/// user on the notifying thread, but must not synchronously wait (for example with +/// `dispatch_sync`) on another thread that is updating the same user. @interface GIDGoogleUser : NSObject /// The Google user ID. diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index 80d81da9..5a2b79a9 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -289,6 +289,31 @@ - (void)testUpdateAuthState_publishesGrantedScopesWithTokens { XCTAssertEqualObjects(user.accessToken.tokenString, kNewAccessToken); } +// The `scope` parameter is space-delimited (RFC 6749 section 3.3); surrounding whitespace and runs +// of spaces must not produce empty scopes. +- (void)testGrantedScopes_parsesSpaceDelimitedScopeString { + NSString *scopeString = [NSString stringWithFormat:@" %@ %@ ", + OIDAuthorizationRequestTestingScope, + OIDAuthorizationRequestTestingScope2]; + OIDAuthState *authState = [self authStateWithScopeString:scopeString + accessToken:kAccessToken]; + + GIDGoogleUser *user = [[GIDGoogleUser alloc] initWithAuthState:authState profileData:nil]; + + NSArray *expectedScopes = + @[ OIDAuthorizationRequestTestingScope, OIDAuthorizationRequestTestingScope2 ]; + XCTAssertEqualObjects(user.grantedScopes, expectedScopes); +} + +// A token response without a `scope` parameter (RFC 6749 section 5.1) yields nil granted scopes. +- (void)testGrantedScopes_isNilWithoutScopeParameter { + OIDAuthState *authState = [self authStateWithScopeString:nil accessToken:kAccessToken]; + + GIDGoogleUser *user = [[GIDGoogleUser alloc] initWithAuthState:authState profileData:nil]; + + XCTAssertNil(user.grantedScopes); +} + - (void)testConfiguration_isFixedAfterTokenUpdate { // The configuration is computed once at init and is fixed for the user's lifetime. GIDGoogleUser *user = [self googleUserWithAccessTokenExpiresIn:kAccessTokenExpiresIn @@ -1101,14 +1126,24 @@ - (GIDGoogleUser *)googleUserWithAccessTokenExpiresIn:(NSTimeInterval)accessToke // always grant OIDAuthorizationRequestTestingScope2, so this builds the response directly. - (OIDAuthState *)authStateWithGrantedScopes:(NSArray *)scopes accessToken:(NSString *)accessToken { - NSDictionary *> *parameters = @{ + return [self authStateWithScopeString:[OIDScopeUtilities scopesWithArray:scopes] + accessToken:accessToken]; +} + +// Returns an auth state whose token response carries `scopeString` verbatim as its `scope` +// parameter, or no `scope` parameter at all when `scopeString` is nil. +- (OIDAuthState *)authStateWithScopeString:(nullable NSString *)scopeString + accessToken:(NSString *)accessToken { + NSMutableDictionary *> *parameters = [@{ @"access_token" : accessToken, @"expires_in" : @(kAccessTokenExpiresIn), @"token_type" : @"example_token_type", @"refresh_token" : kNewRefreshToken, - @"scope" : [OIDScopeUtilities scopesWithArray:scopes], @"id_token" : [self idTokenWithExpiresIn:kNewIDTokenExpiresIn], - }; + } mutableCopy]; + if (scopeString) { + parameters[@"scope"] = scopeString; + } OIDTokenResponse *tokenResponse = [[OIDTokenResponse alloc] initWithRequest:[OIDTokenRequest testInstance] parameters:parameters]; From 410ed2c4a8d3eeb23699aaf325bbcbf9ec11543a Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Thu, 15 Oct 2026 16:05:59 -0700 Subject: [PATCH 4/6] g-orchestrated: Pin the simulator OS for the SPM App Check 11 job --- .github/workflows/builds.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index 21e07d69..c2cf7d3d 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -60,7 +60,7 @@ jobs: xcodebuild \ -scheme GoogleSignIn-Package \ -sdk iphonesimulator \ - -destination "platform=iOS Simulator,name=iPhone 16" \ + -destination "platform=iOS Simulator,name=iPhone 16,OS=18.6" \ -onlyUsePackageVersionsFromResolvedFile \ build-for-testing - name: Run unit test target @@ -68,7 +68,7 @@ jobs: xcodebuild \ -scheme GoogleSignIn-Package \ -sdk iphonesimulator \ - -destination "platform=iOS Simulator,name=iPhone 16,OS=18.5" \ + -destination "platform=iOS Simulator,name=iPhone 16,OS=18.6" \ -onlyUsePackageVersionsFromResolvedFile \ test-without-building From ac7e4d308ef23f1ca38e9f36e7200032dfbc1dc5 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Mon, 19 Oct 2026 10:37:04 -0700 Subject: [PATCH 5/6] g-orchestrated: Use early exits in the hosted domain helper --- GoogleSignIn/Sources/GIDGoogleUser.m | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index f020dc48..f43f8f74 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -105,13 +105,14 @@ - (instancetype)initWithAccessToken:(GIDToken *)accessToken // Returns the hosted domain claim of the given ID token, if it has one. static NSString *_Nullable GIDHostedDomainFromIDTokenString(NSString *_Nullable idTokenString) { - if (idTokenString) { - OIDIDToken *idTokenDecoded = [[OIDIDToken alloc] initWithIDTokenString:idTokenString]; - if (idTokenDecoded && idTokenDecoded.claims[kHostedDomainIDTokenClaimKey]) { - return idTokenDecoded.claims[kHostedDomainIDTokenClaimKey]; - } + if (!idTokenString) { + return nil; } - return nil; + OIDIDToken *idTokenDecoded = [[OIDIDToken alloc] initWithIDTokenString:idTokenString]; + if (!idTokenDecoded) { + return nil; + } + return idTokenDecoded.claims[kHostedDomainIDTokenClaimKey]; } @implementation GIDGoogleUser { From fb05c82c57d919485181abeeda81d61d3ee606e4 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:52:13 -0700 Subject: [PATCH 6/6] g-orchestrated: Explain the nil check on granted scopes --- GoogleSignIn/Sources/GIDGoogleUser.m | 1 + 1 file changed, 1 insertion(+) diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index f43f8f74..11f36149 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -427,6 +427,7 @@ - (void)updateTokensWithAuthState:(OIDAuthState *)authState { if ([current.idToken isEqualToToken:idToken]) { idToken = current.idToken; } + // Both nil must be checked explicitly: isEqualToArray: returns NO when either side is nil. if ((!grantedScopes && !current.grantedScopes) || [grantedScopes isEqualToArray:current.grantedScopes]) { grantedScopes = current.grantedScopes;