From 66f32472d6fa5b39a1a747715ec8843d21a888e8 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Fri, 7 Aug 2026 13:31:08 -0700 Subject: [PATCH 1/6] g-orchestrated: Build the revoke URL with NSURLComponents Assemble the token revocation URL from components and query items rather than by string formatting, so the token and the logging parameters are percent-encoded rather than interpolated raw into a URL string. --- GoogleSignIn/Sources/GIDSignIn.m | 37 +++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/GoogleSignIn/Sources/GIDSignIn.m b/GoogleSignIn/Sources/GIDSignIn.m index 82a8b039..b4b2a865 100644 --- a/GoogleSignIn/Sources/GIDSignIn.m +++ b/GoogleSignIn/Sources/GIDSignIn.m @@ -83,8 +83,14 @@ // The URL template for the URL to get user info. static NSString *const kUserInfoURLTemplate = @"https://%@/oauth2/v3/userinfo?access_token=%@"; -// The URL template for the URL to revoke the token. -static NSString *const kRevokeTokenURLTemplate = @"https://%@/o/oauth2/revoke?token=%@"; +// The path for the endpoint to revoke the token. +static NSString *const kRevokeTokenPath = @"/o/oauth2/revoke"; + +// The name of the query parameter carrying the token to be revoked. +static NSString *const kRevokeTokenParameter = @"token"; + +// The scheme used for requests to Google's servers. +static NSString *const kHTTPSScheme = @"https"; // Expected path in the URL scheme to be handled. static NSString *const kBrowserCallbackPath = @"/oauth2callback"; @@ -573,17 +579,22 @@ - (void)disconnectWithCompletion:(nullable GIDDisconnectCompletion)completion { } return; } - NSString *revokeURLString = [NSString stringWithFormat:kRevokeTokenURLTemplate, - [GIDSignInPreferences googleAuthorizationServer], token]; - // Append logging parameter - revokeURLString = [NSString stringWithFormat:@"%@&%@=%@&%@=%@", - revokeURLString, - kSDKVersionLoggingParameter, - [GIDSignInPreferences sdkVersion], - kEnvironmentLoggingParameter, - [GIDSignInPreferences environment]]; - NSURL *revokeURL = [NSURL URLWithString:revokeURLString]; - [self startFetchURL:revokeURL + NSURLComponents *revokeURLComponents = [[NSURLComponents alloc] init]; + revokeURLComponents.scheme = kHTTPSScheme; + revokeURLComponents.host = [GIDSignInPreferences googleAuthorizationServer]; + revokeURLComponents.path = kRevokeTokenPath; + + NSMutableArray *queryItems = [NSMutableArray array]; + [queryItems addObject:[NSURLQueryItem queryItemWithName:kRevokeTokenParameter value:token]]; + NSDictionary *loggingParameters = + [GIDSignInPreferences loggingParameters]; + for (NSString *name in [loggingParameters.allKeys sortedArrayUsingSelector:@selector(compare:)]) { + [queryItems addObject:[NSURLQueryItem queryItemWithName:name + value:loggingParameters[name]]]; + } + revokeURLComponents.queryItems = queryItems; + + [self startFetchURL:revokeURLComponents.URL fromAuthState:authState withComment:@"GIDSignIn: revoke tokens" withCompletionHandler:^(NSData *data, NSError *error) { From ba08b36912d06ca07c27545e7efb056c6cced87d Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Fri, 7 Aug 2026 13:56:30 -0700 Subject: [PATCH 2/6] g-orchestrated: Build the user info URL with NSURLComponents Assemble the user info URL from components and query items so the access token is percent-encoded rather than interpolated raw into a URL string, matching the revoke URL construction. --- GoogleSignIn/Sources/GIDSignIn.m | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/GoogleSignIn/Sources/GIDSignIn.m b/GoogleSignIn/Sources/GIDSignIn.m index b4b2a865..8de10d71 100644 --- a/GoogleSignIn/Sources/GIDSignIn.m +++ b/GoogleSignIn/Sources/GIDSignIn.m @@ -80,8 +80,11 @@ // The URL template for the token endpoint. static NSString *const kTokenURLTemplate = @"https://%@/token"; -// The URL template for the URL to get user info. -static NSString *const kUserInfoURLTemplate = @"https://%@/oauth2/v3/userinfo?access_token=%@"; +// The path for the endpoint to get user info. +static NSString *const kUserInfoPath = @"/oauth2/v3/userinfo"; + +// The name of the query parameter carrying the access token for the user info request. +static NSString *const kAccessTokenParameter = @"access_token"; // The path for the endpoint to revoke the token. static NSString *const kRevokeTokenPath = @"/o/oauth2/revoke"; @@ -1146,11 +1149,15 @@ - (void)addDecodeIdTokenCallback:(GIDAuthFlow *)authFlow { // If we can't retrieve profile data from the ID token, make a userInfo request to fetch them. if (!handlerAuthFlow.profileData) { [handlerAuthFlow wait]; - NSURL *infoURL = [NSURL URLWithString: - [NSString stringWithFormat:kUserInfoURLTemplate, - [GIDSignInPreferences googleUserInfoServer], - authState.lastTokenResponse.accessToken]]; - [self startFetchURL:infoURL + NSURLComponents *infoURLComponents = [[NSURLComponents alloc] init]; + infoURLComponents.scheme = kHTTPSScheme; + infoURLComponents.host = [GIDSignInPreferences googleUserInfoServer]; + infoURLComponents.path = kUserInfoPath; + infoURLComponents.queryItems = @[ + [NSURLQueryItem queryItemWithName:kAccessTokenParameter + value:authState.lastTokenResponse.accessToken], + ]; + [self startFetchURL:infoURLComponents.URL fromAuthState:authState withComment:@"GIDSignIn: fetch basic profile info" withCompletionHandler:^(NSData *data, NSError *error) { From a6c69021e44c8bada6e49bfd2853c75f008da867 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Fri, 7 Aug 2026 14:06:23 -0700 Subject: [PATCH 3/6] g-orchestrated: Test that reserved characters in a token are encoded Revoke a token containing "&", "=" and "#" and assert it round-trips through the revoke URL intact, along with the logging parameters. Against the previous string-formatted URL this fails: the token is truncated at the "&" and both logging parameters are lost to the fragment. --- GoogleSignIn/Tests/Unit/GIDSignInTest.m | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index 36cb3047..8e4b8259 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -1349,6 +1349,23 @@ - (void)testDisconnectNoCallback_accessToken { [_tokenResponse verify]; } +// Verifies a token containing characters that are reserved in a URL query is percent-encoded +// in the revoke URL, so that it arrives at the server intact. +- (void)testDisconnectNoCallback_tokenWithReservedCharacters { + NSString *tokenWithReservedCharacters = @"token&with=reserved#characters"; + [[[_authorization expect] andReturn:_authState] authState]; + [[[_authState expect] andReturn:_tokenResponse] lastTokenResponse]; + [[[_tokenResponse expect] andReturn:tokenWithReservedCharacters] accessToken]; + [[[_authorization expect] andReturn:_fetcherService] fetcherService]; + [_signIn disconnectWithCompletion:nil]; + [self verifyAndRevokeToken:tokenWithReservedCharacters + hasCallback:NO + waitingForExpectations:@[]]; + [_authorization verify]; + [_authState verify]; + [_tokenResponse verify]; +} + // Verifies disconnect calls callback with no errors if refresh token is present. - (void)testDisconnect_refreshToken { [[[_authorization expect] andReturn:_authState] authState]; From b22d5c6c1ea2c01bd8754d856b70fa760218742f Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Thu, 13 Aug 2026 17:12:01 -0700 Subject: [PATCH 4/6] g-orchestrated: Test that "+" in a token survives the revoke URL Revoke a token containing "+" and assert it round-trips through the revoke URL intact. --- GoogleSignIn/Tests/Unit/GIDSignInTest.m | 41 +++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index 8e4b8259..ecaa51c0 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -1366,6 +1366,47 @@ - (void)testDisconnectNoCallback_tokenWithReservedCharacters { [_tokenResponse verify]; } +// "+" is a sub-delimiter that RFC 3986 permits literally in a URL query, so it is left +// unescaped in the revoke URL. While many servers decode query strings as +// application/x-www-form-urlencoded, where "+" means a space, decoding an OAuth token that way +// would be a server-side error. +- (void)testDisconnectNoCallback_tokenWithPlusCharacter { + NSString *tokenWithPlusCharacter = @"token+with+plus"; + [[[_authorization expect] andReturn:_authState] authState]; + [[[_authState expect] andReturn:_tokenResponse] lastTokenResponse]; + [[[_tokenResponse expect] andReturn:tokenWithPlusCharacter] accessToken]; + [[[_authorization expect] andReturn:_fetcherService] fetcherService]; + [_signIn disconnectWithCompletion:nil]; + + XCTAssertTrue([self isFetcherStarted], @"should start fetching"); + NSURL *url = [self fetchedURL]; + XCTAssertEqualObjects([url scheme], @"https", @"scheme must match"); + XCTAssertEqualObjects([url host], @"accounts.google.com", @"host must match"); + XCTAssertEqualObjects([url path], @"/o/oauth2/revoke", @"path must match"); + + NSString *query = [[self fetchedURL] query]; + XCTAssertTrue([query containsString:@"token=token+with+plus"], + @"'+' should be preserved literally in the query string"); + + NSURLComponents *components = + [NSURLComponents componentsWithURL:[self fetchedURL] resolvingAgainstBaseURL:NO]; + NSURLQueryItem *tokenItem; + for (NSURLQueryItem *item in components.queryItems) { + if ([item.name isEqualToString:@"token"]) { + tokenItem = item; + break; + } + } + XCTAssertEqualObjects(tokenItem.value, tokenWithPlusCharacter); + + [self didFetch:nil error:nil]; + XCTAssertTrue(_keychainRemoved, @"should clear saved keychain name"); + + [_authorization verify]; + [_authState verify]; + [_tokenResponse verify]; +} + // Verifies disconnect calls callback with no errors if refresh token is present. - (void)testDisconnect_refreshToken { [[[_authorization expect] andReturn:_authState] authState]; From b4c852c56e98766bbdd1c4deb35de5cbd330d908 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Thu, 13 Aug 2026 17:46:28 -0700 Subject: [PATCH 5/6] g-orchestrated: Percent-encode "+" in token query parameters OAuth parameters are application/x-www-form-urlencoded per RFC 6749 Appendix B, where "+" means a space and a literal plus is sent as "%2B". NSURLComponents encodes per RFC 3986, which permits "+" literally in a query, so encode it explicitly for the revoke and user info URLs. --- GoogleSignIn/Sources/GIDSignIn.m | 11 +++++++++++ GoogleSignIn/Tests/Unit/GIDSignInTest.m | 13 +++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/GoogleSignIn/Sources/GIDSignIn.m b/GoogleSignIn/Sources/GIDSignIn.m index 8de10d71..f910e72d 100644 --- a/GoogleSignIn/Sources/GIDSignIn.m +++ b/GoogleSignIn/Sources/GIDSignIn.m @@ -559,6 +559,15 @@ - (void)signOut { [self removeAllKeychainEntries]; } +// OAuth parameters are application/x-www-form-urlencoded (RFC 6749 Appendix B), where "+" +// means a space, but NSURLComponents leaves "+" literal because RFC 3986 permits it in a +// query. Percent-encode it so a "+" in a token survives form decoding on the server. +static void GIDPercentEncodePlusInQuery(NSURLComponents *components) { + components.percentEncodedQuery = + [components.percentEncodedQuery stringByReplacingOccurrencesOfString:@"+" + withString:@"%2B"]; +} + - (void)disconnectWithCompletion:(nullable GIDDisconnectCompletion)completion { OIDAuthState *authState = _currentUser.authState; if (!authState) { @@ -596,6 +605,7 @@ - (void)disconnectWithCompletion:(nullable GIDDisconnectCompletion)completion { value:loggingParameters[name]]]; } revokeURLComponents.queryItems = queryItems; + GIDPercentEncodePlusInQuery(revokeURLComponents); [self startFetchURL:revokeURLComponents.URL fromAuthState:authState @@ -1157,6 +1167,7 @@ - (void)addDecodeIdTokenCallback:(GIDAuthFlow *)authFlow { [NSURLQueryItem queryItemWithName:kAccessTokenParameter value:authState.lastTokenResponse.accessToken], ]; + GIDPercentEncodePlusInQuery(infoURLComponents); [self startFetchURL:infoURLComponents.URL fromAuthState:authState withComment:@"GIDSignIn: fetch basic profile info" diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index ecaa51c0..eaac689e 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -1366,10 +1366,9 @@ - (void)testDisconnectNoCallback_tokenWithReservedCharacters { [_tokenResponse verify]; } -// "+" is a sub-delimiter that RFC 3986 permits literally in a URL query, so it is left -// unescaped in the revoke URL. While many servers decode query strings as -// application/x-www-form-urlencoded, where "+" means a space, decoding an OAuth token that way -// would be a server-side error. +// OAuth parameters use application/x-www-form-urlencoded (RFC 6749 Appendix B), where "+" +// means a space, so a literal "+" in a token is sent as "%2B" even though RFC 3986 +// would permit it unescaped in a query. - (void)testDisconnectNoCallback_tokenWithPlusCharacter { NSString *tokenWithPlusCharacter = @"token+with+plus"; [[[_authorization expect] andReturn:_authState] authState]; @@ -1385,8 +1384,10 @@ - (void)testDisconnectNoCallback_tokenWithPlusCharacter { XCTAssertEqualObjects([url path], @"/o/oauth2/revoke", @"path must match"); NSString *query = [[self fetchedURL] query]; - XCTAssertTrue([query containsString:@"token=token+with+plus"], - @"'+' should be preserved literally in the query string"); + XCTAssertTrue([query containsString:@"token=token%2Bwith%2Bplus"], + @"'+' should be percent-encoded in the query string"); + XCTAssertFalse([query containsString:@"token=token+with+plus"], + @"'+' should not be literal in the query string"); NSURLComponents *components = [NSURLComponents componentsWithURL:[self fetchedURL] resolvingAgainstBaseURL:NO]; From 4f07029efcc3f3d8939ee434863f49f883823a95 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Thu, 13 Aug 2026 17:59:58 -0700 Subject: [PATCH 6/6] g-orchestrated: Cover space encoding and form decoding of revoke tokens One test pins NSURLComponents encoding a space as "%20" rather than "+", which the "+" to "%2B" rewrite depends on. The other checks that a "+" token survives a form-urlencoded read of the revoke URL, the way a server would parse it. --- GoogleSignIn/Tests/Unit/GIDSignInTest.m | 39 +++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index eaac689e..a98cdf25 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -1408,6 +1408,45 @@ - (void)testDisconnectNoCallback_tokenWithPlusCharacter { [_tokenResponse verify]; } +// Guard the "+" to "%2B" rewrite, which is only safe if a space encodes as "%20", never as "+". +// Whilst this is technically testing Foundation behaviour, it's undocumented behaviour. +- (void)testDisconnectNoCallback_tokenWithSpace { + NSString *tokenWithSpace = @"token with space"; + [[[_authorization expect] andReturn:_authState] authState]; + [[[_authState expect] andReturn:_tokenResponse] lastTokenResponse]; + [[[_tokenResponse expect] andReturn:tokenWithSpace] accessToken]; + [[[_authorization expect] andReturn:_fetcherService] fetcherService]; + [_signIn disconnectWithCompletion:nil]; + + NSString *query = [[self fetchedURL] query]; + XCTAssertTrue([query containsString:@"token=token%20with%20space"], + @"a space should be percent-encoded in the query string"); + XCTAssertFalse([query containsString:@"+"], @"a space should never be encoded as '+'"); + + [self didFetch:nil error:nil]; + XCTAssertTrue(_keychainRemoved, @"should clear saved keychain name"); + [_authorization verify]; + [_authState verify]; + [_tokenResponse verify]; +} + +// Round-trip the revoke URL through OIDURLQueryComponent, a pretend server, to check "+" survives. +- (void)testDisconnectNoCallback_tokenWithPlusCharacterFormDecoded { + NSString *tokenWithPlusCharacter = @"token+with+plus"; + [[[_authorization expect] andReturn:_authState] authState]; + [[[_authState expect] andReturn:_tokenResponse] lastTokenResponse]; + [[[_tokenResponse expect] andReturn:tokenWithPlusCharacter] accessToken]; + [[[_authorization expect] andReturn:_fetcherService] fetcherService]; + [_signIn disconnectWithCompletion:nil]; + + [self verifyAndRevokeToken:tokenWithPlusCharacter + hasCallback:NO + waitingForExpectations:@[]]; + [_authorization verify]; + [_authState verify]; + [_tokenResponse verify]; +} + // Verifies disconnect calls callback with no errors if refresh token is present. - (void)testDisconnect_refreshToken { [[[_authorization expect] andReturn:_authState] authState];