From a081c2a0abfc1863334199af96c9a73bae274a1b Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:58:29 -0700 Subject: [PATCH 1/2] g-orchestrated: Drop token refresh responses for a replaced authorization --- CHANGELOG.md | 1 + GoogleSignIn/Sources/GIDGoogleUser.m | 12 ++++++-- GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 34 +++++++++++++++++++++ 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa5d4193..3b73c848 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - `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 a token refresh that was in flight while scopes were added overwriting the newer tokens. `GIDGoogleUser` now drops a refresh response when the authorization it was requested for has been replaced, so `grantedScopes` and `accessToken` no longer revert to their pre-add-scopes values. - 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 6cd7e47a..ad4ba2bc 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -257,8 +257,14 @@ - (void)refreshTokensIfNeededWithCompletion:(GIDGoogleUserCompletion)completion NSError *_Nullable error) { // Update the auth state under `_authStateLock` so this refresh cannot interleave with // -updateWithTokenResponse:authorizationResponse:profileData:. + NSError *effectiveError = error; @synchronized(self->_authStateLock) { - if (tokenResponse) { + if (self.authState.lastAuthorizationResponse != authorizationResponse) { + // Drop the response if a new authorization (such as from add-scopes) replaced the one this + // refresh was requested for while it was in flight, so the old grant cannot overwrite the + // newer tokens. + effectiveError = nil; + } else if (tokenResponse) { [self.authState updateWithTokenResponse:tokenResponse error:nil]; } else { if (error.domain == OIDOAuthTokenErrorDomain) { @@ -267,7 +273,7 @@ - (void)refreshTokensIfNeededWithCompletion:(GIDGoogleUserCompletion)completion } } #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST - [GIDEMMSupport handleTokenFetchEMMError:error completion:^(NSError *_Nullable error) { + [GIDEMMSupport handleTokenFetchEMMError:effectiveError completion:^(NSError *_Nullable error) { // Process the handler queue to call back. NSArray *refreshTokensHandlerQueue; @synchronized(self->_tokenRefreshHandlerQueue) { @@ -288,7 +294,7 @@ - (void)refreshTokensIfNeededWithCompletion:(GIDGoogleUserCompletion)completion } for (GIDGoogleUserCompletion completion in refreshTokensHandlerQueue) { dispatch_async(dispatch_get_main_queue(), ^{ - completion(error ? nil : self, error); + completion(effectiveError ? nil : self, effectiveError); }); } #endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index 9566a271..ecdb482e 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -1041,6 +1041,40 @@ - (void)testRefreshTokensIfNeededWithCompletion_noRefresh_givenRefreshTokenExpir [self waitForExpectationsWithTimeout:1 handler:nil]; } +- (void)testRefreshTokensIfNeeded_dropsResponseForReplacedAuthorization { + GIDGoogleUser *user = [self googleUserWithAccessTokenExpiresIn:-10 + idTokenExpiresIn:kIDTokenExpiresIn]; + + XCTestExpectation *expectation = [self expectationWithDescription:@"Callback is called"]; + [user refreshTokensIfNeededWithCompletion:^(GIDGoogleUser *_Nullable refreshedUser, + NSError *_Nullable error) { + [expectation fulfill]; + XCTAssertNotNil(refreshedUser); + XCTAssertNil(error); + }]; + + NSString *updatedIDToken = [self idTokenWithExpiresIn:kNewIDTokenExpiresIn]; + OIDAuthState *updatedAuthState = + [OIDAuthState testInstanceWithIDToken:updatedIDToken + accessToken:@"access_token_after_add_scopes" + accessTokenExpiresIn:kAccessTokenExpiresIn + refreshToken:kNewRefreshToken]; + [user updateWithTokenResponse:updatedAuthState.lastTokenResponse + authorizationResponse:updatedAuthState.lastAuthorizationResponse + profileData:nil]; + + OIDTokenResponse *staleResponse = + [OIDTokenResponse testInstanceWithIDToken:updatedIDToken + accessToken:kNewAccessToken + expiresIn:@(kAccessTokenExpiresIn) + refreshToken:kRefreshToken + tokenRequest:nil]; + _tokenFetchHandler(staleResponse, nil); + [self waitForExpectationsWithTimeout:1 handler:nil]; + + XCTAssertEqualObjects(user.accessToken.tokenString, @"access_token_after_add_scopes"); +} + - (void)testWrapperIdentifier_PresentOnRefreshRequestWhenSet { GIDSignIn.wrapperIdentifier = @"firebase"; From a858c9387563df9a8efd4a4eba7b1972e770ddaf Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:14:24 -0700 Subject: [PATCH 2/2] g-orchestrated: Test scopes and errors for a dropped stale refresh --- GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 46 ++++++++++++++++++--- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index ecdb482e..cf142bea 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -1044,6 +1044,7 @@ - (void)testRefreshTokensIfNeededWithCompletion_noRefresh_givenRefreshTokenExpir - (void)testRefreshTokensIfNeeded_dropsResponseForReplacedAuthorization { GIDGoogleUser *user = [self googleUserWithAccessTokenExpiresIn:-10 idTokenExpiresIn:kIDTokenExpiresIn]; + XCTAssertEqualObjects(user.grantedScopes, @[ OIDAuthorizationRequestTestingScope2 ]); XCTestExpectation *expectation = [self expectationWithDescription:@"Callback is called"]; [user refreshTokensIfNeededWithCompletion:^(GIDGoogleUser *_Nullable refreshedUser, @@ -1053,18 +1054,18 @@ - (void)testRefreshTokensIfNeeded_dropsResponseForReplacedAuthorization { XCTAssertNil(error); }]; - NSString *updatedIDToken = [self idTokenWithExpiresIn:kNewIDTokenExpiresIn]; + // The add-scopes flow replaces the authorization while the refresh is in flight. + NSArray *scopesAfterAddScopes = @[ OIDAuthorizationRequestTestingScope2, kNewScope ]; OIDAuthState *updatedAuthState = - [OIDAuthState testInstanceWithIDToken:updatedIDToken - accessToken:@"access_token_after_add_scopes" - accessTokenExpiresIn:kAccessTokenExpiresIn - refreshToken:kNewRefreshToken]; + [self authStateWithGrantedScopes:scopesAfterAddScopes + accessToken:@"access_token_after_add_scopes"]; [user updateWithTokenResponse:updatedAuthState.lastTokenResponse authorizationResponse:updatedAuthState.lastAuthorizationResponse profileData:nil]; + // The stale response carries the old grant's scope and a different access token. OIDTokenResponse *staleResponse = - [OIDTokenResponse testInstanceWithIDToken:updatedIDToken + [OIDTokenResponse testInstanceWithIDToken:[self idTokenWithExpiresIn:kNewIDTokenExpiresIn] accessToken:kNewAccessToken expiresIn:@(kAccessTokenExpiresIn) refreshToken:kRefreshToken @@ -1073,6 +1074,39 @@ - (void)testRefreshTokensIfNeeded_dropsResponseForReplacedAuthorization { [self waitForExpectationsWithTimeout:1 handler:nil]; XCTAssertEqualObjects(user.accessToken.tokenString, @"access_token_after_add_scopes"); + XCTAssertEqualObjects(user.grantedScopes, scopesAfterAddScopes); +} + +- (void)testRefreshTokensIfNeeded_dropsErrorForReplacedAuthorization { + GIDGoogleUser *user = [self googleUserWithAccessTokenExpiresIn:-10 + idTokenExpiresIn:kIDTokenExpiresIn]; + + XCTestExpectation *expectation = [self expectationWithDescription:@"Callback is called"]; + [user refreshTokensIfNeededWithCompletion:^(GIDGoogleUser *_Nullable refreshedUser, + NSError *_Nullable error) { + [expectation fulfill]; + XCTAssertNotNil(refreshedUser); + XCTAssertNil(error); + }]; + + // The add-scopes flow replaces the authorization while the refresh is in flight. + OIDAuthState *updatedAuthState = + [self authStateWithGrantedScopes:@[ OIDAuthorizationRequestTestingScope2, kNewScope ] + accessToken:@"access_token_after_add_scopes"]; + [user updateWithTokenResponse:updatedAuthState.lastTokenResponse + authorizationResponse:updatedAuthState.lastAuthorizationResponse + profileData:nil]; + + // A stale OAuth token error for the old grant must not invalidate the new authorization. + NSError *staleError = [NSError errorWithDomain:OIDOAuthTokenErrorDomain + code:OIDErrorCodeOAuthTokenInvalidGrant + userInfo:nil]; + _tokenFetchHandler(nil, staleError); + [self waitForExpectationsWithTimeout:1 handler:nil]; + + XCTAssertTrue(user.authState.isAuthorized); + XCTAssertNil(user.authState.authorizationError); + XCTAssertEqualObjects(user.accessToken.tokenString, @"access_token_after_add_scopes"); } - (void)testWrapperIdentifier_PresentOnRefreshRequestWhenSet {