From 2a97c96e250230e159ade2fa2cf02837b53ed569 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:06:14 -0700 Subject: [PATCH 01/17] g-orchestrated: Add sanitized SDK wrapper identifier to GIDSignInPreferences * Adds the `gidwrapper` logging-parameter key. * Adds a process-global, thread-safe, sanitized wrapper-identifier store with GIDWrapperIdentifier() / GIDSetWrapperIdentifier() accessors. * No emit sites wired yet; no behavior change. --- GoogleSignIn/Sources/GIDSignInPreferences.h | 8 +++ GoogleSignIn/Sources/GIDSignInPreferences.m | 61 +++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/GoogleSignIn/Sources/GIDSignInPreferences.h b/GoogleSignIn/Sources/GIDSignInPreferences.h index 8bdb6719..e5d8c679 100644 --- a/GoogleSignIn/Sources/GIDSignInPreferences.h +++ b/GoogleSignIn/Sources/GIDSignInPreferences.h @@ -20,11 +20,19 @@ NS_ASSUME_NONNULL_BEGIN extern NSString *const kSDKVersionLoggingParameter; extern NSString *const kEnvironmentLoggingParameter; +extern NSString *const kSDKWrapperLoggingParameter; NSString* GIDVersion(void); NSString* GIDEnvironment(void); +// Returns the sanitized SDK wrapper identifier, or nil if none has been set. +NSString* _Nullable GIDWrapperIdentifier(void); + +// Sets the SDK wrapper identifier. The value is sanitized (see implementation); +// empty or fully-invalid input clears it. Last write wins. Thread-safe. +void GIDSetWrapperIdentifier(NSString * _Nullable wrapper); + @interface GIDSignInPreferences : NSObject + (NSString *)googleAuthorizationServer; diff --git a/GoogleSignIn/Sources/GIDSignInPreferences.m b/GoogleSignIn/Sources/GIDSignInPreferences.m index 3f0e27d1..51312085 100644 --- a/GoogleSignIn/Sources/GIDSignInPreferences.m +++ b/GoogleSignIn/Sources/GIDSignInPreferences.m @@ -26,6 +26,9 @@ // The name of the query parameter used for logging the Apple execution environment. NSString *const kEnvironmentLoggingParameter = @"gidenv"; +// The name of the query parameter used to log the embedding SDK / wrapper. +NSString *const kSDKWrapperLoggingParameter = @"gidwrapper"; + // Supported Apple execution environments static NSString *const kAppleEnvironmentUnknown = @"unknown"; static NSString *const kAppleEnvironmentIOS = @"ios"; @@ -34,6 +37,17 @@ static NSString *const kAppleEnvironmentMacOSIOSOnMac = @"macos-ios"; static NSString *const kAppleEnvironmentMacOSMacCatalyst = @"macos-cat"; +static NSString *gWrapperIdentifier = nil; + +static NSObject* GIDWrapperLock(void) { + static NSObject *lock; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + lock = [[NSObject alloc] init]; + }); + return lock; +} + #ifndef GID_SDK_VERSION #error "GID_SDK_VERSION is not defined: add -DGID_SDK_VERSION=x.x.x to the build invocation." #endif @@ -80,6 +94,53 @@ return appleEnvironment; } +static NSString * _Nullable GIDSanitizeWrapperIdentifier(NSString * _Nullable raw) { + if (!raw) { + return nil; + } + + // Trim leading/trailing whitespace and newlines. + NSString *sanitized = [raw stringByTrimmingCharactersInSet: + [NSCharacterSet whitespaceAndNewlineCharacterSet]]; + if (sanitized.length == 0) { + return nil; + } + + // Lowercase (locale-independent). + sanitized = [sanitized lowercaseString]; + + // Keep only characters in the allowlist [A-Za-z0-9-._~]; drop everything else. + NSCharacterSet *allowed = + [NSCharacterSet characterSetWithCharactersInString:@"abcdefghijklmnopqrstuvwxyz0123456789-._~"]; + sanitized = [[sanitized componentsSeparatedByCharactersInSet:[allowed invertedSet]] + componentsJoinedByString:@""]; + + if (sanitized.length == 0) { + return nil; + } + + // If length > 32, take substringToIndex:32. + // Safe because the allowlist is ASCII, so each character is exactly one UTF-16 unit. + if (sanitized.length > 32) { + sanitized = [sanitized substringToIndex:32]; + } + + return sanitized; +} + +NSString* GIDWrapperIdentifier(void) { + @synchronized(GIDWrapperLock()) { + return [gWrapperIdentifier copy]; + } +} + +void GIDSetWrapperIdentifier(NSString * _Nullable wrapper) { + NSString *sanitized = GIDSanitizeWrapperIdentifier(wrapper); + @synchronized(GIDWrapperLock()) { + gWrapperIdentifier = [sanitized copy]; + } +} + @implementation GIDSignInPreferences + (NSString *)googleAuthorizationServer { From cbdf28f54e28e350dee6e235fad18bb7208fb84f Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:09:59 -0700 Subject: [PATCH 02/17] g-orchestrated: Emit gidwrapper logging parameter and expose wrapperIdentifier * Adds the public `GIDSignIn.wrapperIdentifier` property (backed by the GIDSignInPreferences global). * Emits `gidwrapper` on the authorization request, the token exchange/refresh, and the revoke URL (percent-encoded), guarded so nothing changes when unset. * Accessors are explicitly annotated nullable to match the header property inside the file NS_ASSUME_NONNULL region. --- GoogleSignIn/Sources/GIDSignIn.m | 28 +++++++++++++++++++ .../Sources/Public/GoogleSignIn/GIDSignIn.h | 8 ++++++ 2 files changed, 36 insertions(+) diff --git a/GoogleSignIn/Sources/GIDSignIn.m b/GoogleSignIn/Sources/GIDSignIn.m index a8cf1ce7..e8a011c2 100644 --- a/GoogleSignIn/Sources/GIDSignIn.m +++ b/GoogleSignIn/Sources/GIDSignIn.m @@ -582,6 +582,16 @@ - (void)disconnectWithCompletion:(nullable GIDDisconnectCompletion)completion { GIDVersion(), kEnvironmentLoggingParameter, GIDEnvironment()]; + NSString *wrapperIdentifier = GIDWrapperIdentifier(); + if (wrapperIdentifier) { + NSMutableCharacterSet *unreservedSet = + [NSMutableCharacterSet alphanumericCharacterSet]; + [unreservedSet addCharactersInString:@"-._~"]; + NSString *encodedWrapper = [wrapperIdentifier + stringByAddingPercentEncodingWithAllowedCharacters:unreservedSet]; + revokeURLString = [NSString stringWithFormat:@"%@&%@=%@", + revokeURLString, kSDKWrapperLoggingParameter, encodedWrapper]; + } NSURL *revokeURL = [NSURL URLWithString:revokeURLString]; [self startFetchURL:revokeURL fromAuthState:authState @@ -625,6 +635,14 @@ + (GIDSignIn *)sharedInstance { return sharedInstance; } +- (nullable NSString *)wrapperIdentifier { + return GIDWrapperIdentifier(); +} + +- (void)setWrapperIdentifier:(nullable NSString *)wrapperIdentifier { + GIDSetWrapperIdentifier(wrapperIdentifier); +} + #pragma mark - Configuring and pre-warming #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST @@ -921,6 +939,11 @@ - (void)authorizationRequestWithOptions:(GIDSignInInternalOptions *)options comp additionalParameters[kSDKVersionLoggingParameter] = GIDVersion(); additionalParameters[kEnvironmentLoggingParameter] = GIDEnvironment(); + NSString *wrapperIdentifier = GIDWrapperIdentifier(); + if (wrapperIdentifier) { + additionalParameters[kSDKWrapperLoggingParameter] = wrapperIdentifier; + } + return additionalParameters; } @@ -1057,6 +1080,11 @@ - (void)maybeFetchToken:(GIDAuthFlow *)authFlow { additionalParameters[kSDKVersionLoggingParameter] = GIDVersion(); additionalParameters[kEnvironmentLoggingParameter] = GIDEnvironment(); + NSString *wrapperIdentifier = GIDWrapperIdentifier(); + if (wrapperIdentifier) { + additionalParameters[kSDKWrapperLoggingParameter] = wrapperIdentifier; + } + OIDTokenRequest *tokenRequest; if (!authState.lastTokenResponse.accessToken && authState.lastAuthorizationResponse.authorizationCode) { diff --git a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h index a6b95ead..d04a990d 100644 --- a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h +++ b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h @@ -73,6 +73,14 @@ typedef NS_ERROR_ENUM(kGIDSignInErrorDomain, GIDSignInErrorCode) { /// The active configuration for this instance of `GIDSignIn`. @property(nonatomic, nullable) GIDConfiguration *configuration; +/// An optional identifier naming the SDK or wrapper that embeds Google Sign-In. +/// Reported to Google as a diagnostic logging parameter for aggregate metrics only; +/// it is never used for authentication or authorization. Set this once, before your +/// first sign-in call. The value is sanitized: lowercased, restricted to the +/// characters A-Z a-z 0-9 - . _ ~, and capped at 32 characters; input that does not +/// conform is truncated or ignored. +@property(nonatomic, nullable) NSString *wrapperIdentifier; + #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST /// Configures `GIDSignIn` for use. From 19c3415146a80406090fc15e74acfd2d973b5d33 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:08:20 -0700 Subject: [PATCH 03/17] g-orchestrated: Emit gidwrapper on GIDGoogleUser token refresh * The wrapper identifier now accompanies cold-start refreshes, matching the other logging-parameter emit sites. --- GoogleSignIn/Sources/GIDGoogleUser.m | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index 1da8f972..b0063584 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -155,6 +155,10 @@ - (void)refreshTokensIfNeededWithCompletion:(GIDGoogleUserCompletion)completion #endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST additionalParameters[kSDKVersionLoggingParameter] = GIDVersion(); additionalParameters[kEnvironmentLoggingParameter] = GIDEnvironment(); + NSString *wrapperIdentifier = GIDWrapperIdentifier(); + if (wrapperIdentifier) { + additionalParameters[kSDKWrapperLoggingParameter] = wrapperIdentifier; + } OIDTokenRequest *tokenRefreshRequest = [self.authState tokenRefreshRequestWithAdditionalParameters:additionalParameters]; From c13440d4ebd2252b9b48d284ead49377daaa764d Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:08:03 -0700 Subject: [PATCH 04/17] g-orchestrated: Changelog: add wrapperIdentifier / gidwrapper parameter --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 180da5dd..9afaf376 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +# Unreleased +- Add `GIDSignIn.wrapperIdentifier`, an optional identifier that SDKs embedding Google Sign-In can set to self-identify in Google's diagnostic logs via a new `gidwrapper` parameter. The value is sanitized and opt-in; default behavior is unchanged. + # 9.2.0 - Expose the refresh token expiration date ([#577](https://github.com/google/GoogleSignIn-iOS/pull/577)) - Support requesting the `amr` (Authentication Methods References) claim ([#600](https://github.com/google/GoogleSignIn-iOS/pull/600)) From 8a4ca1159b2503e6c2e7a75c8e3e24c174be0135 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:12:08 -0700 Subject: [PATCH 05/17] g-orchestrated: Test wrapper identifier sanitizer and accessors * Adds nine tests covering round-trip, lowercasing, allowlist filtering, whitespace trimming, empty/invalid input, the 32-character cap, and last-write-wins. * Adds a tearDown that clears the process-global identifier between tests. --- .../Tests/Unit/GIDSignInPreferencesTest.m | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/GoogleSignIn/Tests/Unit/GIDSignInPreferencesTest.m b/GoogleSignIn/Tests/Unit/GIDSignInPreferencesTest.m index 80fa8949..e20a72ce 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInPreferencesTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInPreferencesTest.m @@ -44,4 +44,75 @@ - (void)testGIDEnvironment { XCTAssertEqualObjects(environment, expectedEnvironment); } +- (void)tearDown { + GIDSetWrapperIdentifier(nil); + [super tearDown]; +} + +- (void)testWrapperIdentifier_UnsetReturnsNil { + // Test that when no identifier is set (or it is explicitly cleared), nil is returned. + GIDSetWrapperIdentifier(nil); + XCTAssertNil(GIDWrapperIdentifier()); +} + +- (void)testWrapperIdentifier_RoundTripsSimpleValue { + // Test that a simple alphanumeric identifier is correctly stored and retrieved. + GIDSetWrapperIdentifier(@"firebase"); + XCTAssertEqualObjects(GIDWrapperIdentifier(), @"firebase"); +} + +- (void)testWrapperIdentifier_Lowercases { + // Test that identifiers are automatically lowercased when stored. + GIDSetWrapperIdentifier(@"FireBase"); + XCTAssertEqualObjects(GIDWrapperIdentifier(), @"firebase"); +} + +- (void)testWrapperIdentifier_StripsDisallowedCharacters { + // Test that spaces and disallowed punctuation are removed from the identifier. + GIDSetWrapperIdentifier(@"fire base!/&=?"); + XCTAssertEqualObjects(GIDWrapperIdentifier(), @"firebase"); +} + +- (void)testWrapperIdentifier_KeepsAllowedPunctuation { + // Test that allowed characters (A-Za-z0-9-._~) are preserved. + GIDSetWrapperIdentifier(@"my-sdk_1.2~x"); + XCTAssertEqualObjects(GIDWrapperIdentifier(), @"my-sdk_1.2~x"); +} + +- (void)testWrapperIdentifier_TrimsWhitespace { + // Test that leading and trailing whitespace is trimmed from the identifier. + GIDSetWrapperIdentifier(@" firebase "); + XCTAssertEqualObjects(GIDWrapperIdentifier(), @"firebase"); +} + +- (void)testWrapperIdentifier_EmptyOrWhitespaceBecomesNil { + // Test that empty or whitespace-only strings result in a nil identifier. + GIDSetWrapperIdentifier(@" "); + XCTAssertNil(GIDWrapperIdentifier()); + + // Test that a string consisting only of invalid characters results in a nil identifier. + GIDSetWrapperIdentifier(@"!!!"); + XCTAssertNil(GIDWrapperIdentifier()); +} + +- (void)testWrapperIdentifier_CapsAtThirtyTwoCharacters { + // Test that the identifier is truncated to a maximum of 32 characters. + NSString *longString = @"abcdefghijklmnopqrstuvwxyz1234567890"; // 36 characters + GIDSetWrapperIdentifier(longString); + NSString *result = GIDWrapperIdentifier(); + XCTAssertEqual(result.length, (NSUInteger)32); + XCTAssertEqualObjects(result, [longString.lowercaseString substringToIndex:32]); +} + +- (void)testWrapperIdentifier_LastWriteWins { + // Test that subsequent writes overwrite the previous identifier. + GIDSetWrapperIdentifier(@"first"); + GIDSetWrapperIdentifier(@"second"); + XCTAssertEqualObjects(GIDWrapperIdentifier(), @"second"); + + // Test that setting it to nil clears the identifier. + GIDSetWrapperIdentifier(nil); + XCTAssertNil(GIDWrapperIdentifier()); +} + @end From 6765b9725ca6b81d56fc76e259a56a38e00cbe17 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:13:20 -0700 Subject: [PATCH 06/17] g-orchestrated: Test gidwrapper emission on GIDSignIn requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * testWrapperIdentifier_PresentInAuthorizationRequestWhenSet * testWrapperIdentifier_AbsentFromAuthorizationRequestWhenUnset * testWrapperIdentifier_PropertyReflectsSanitizedValue * testWrapperIdentifier_PercentEncodedOnRevokeURL — added, modelled on the existing testDisconnect_accessToken revoke flow and the isFetcherStarted/fetchedURL helpers. * Extends tearDown to clear the process-global identifier between tests. --- GoogleSignIn/Tests/Unit/GIDSignInTest.m | 72 +++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index ab1c4003..e747734d 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -389,6 +389,7 @@ - (void)tearDown { [_testUserDefaults removePersistentDomainForName:kUserDefaultsSuiteName]; [_fakeMainBundle stopFaking]; + GIDSignIn.sharedInstance.wrapperIdentifier = nil; [super tearDown]; } @@ -1168,6 +1169,77 @@ - (void)testOAuthLogin_HostedDomain { XCTAssertEqualObjects(params[@"hd"], kHostedDomain, @"hosted domain should match"); } +- (void)testWrapperIdentifier_PresentInAuthorizationRequestWhenSet { + GIDSignIn.sharedInstance.wrapperIdentifier = @"firebase"; + OCMStub( + [_keychainStore saveAuthSession:OCMOCK_ANY error:OCMArg.anyObjectRef] + ).andDo(^(NSInvocation *invocation) { + self->_keychainSaved = self->_saveAuthorizationReturnValue; + }); + + [self OAuthLoginWithAddScopesFlow:NO + authError:nil + tokenError:nil + emmPasscodeInfoRequired:NO + claimsAsJSONRequired:NO + keychainError:NO + restoredSignIn:NO + oldAccessToken:NO + modalCancel:NO]; + + NSDictionary *params = _savedAuthorizationRequest.additionalParameters; + XCTAssertEqualObjects(params[@"gidwrapper"], @"firebase", + @"The authorization request should contain the 'gidwrapper' parameter " + "when set."); +} + +- (void)testWrapperIdentifier_AbsentFromAuthorizationRequestWhenUnset { + GIDSignIn.sharedInstance.wrapperIdentifier = nil; + OCMStub( + [_keychainStore saveAuthSession:OCMOCK_ANY error:OCMArg.anyObjectRef] + ).andDo(^(NSInvocation *invocation) { + self->_keychainSaved = self->_saveAuthorizationReturnValue; + }); + + [self OAuthLoginWithAddScopesFlow:NO + authError:nil + tokenError:nil + emmPasscodeInfoRequired:NO + claimsAsJSONRequired:NO + keychainError:NO + restoredSignIn:NO + oldAccessToken:NO + modalCancel:NO]; + + NSDictionary *params = _savedAuthorizationRequest.additionalParameters; + XCTAssertNil(params[@"gidwrapper"], + @"The authorization request should not contain the 'gidwrapper' parameter " + "when unset."); +} + +- (void)testWrapperIdentifier_PropertyReflectsSanitizedValue { + GIDSignIn.sharedInstance.wrapperIdentifier = @"Fire Base!"; + XCTAssertEqualObjects(GIDSignIn.sharedInstance.wrapperIdentifier, @"firebase", + @"The wrapperIdentifier property should reflect the sanitized value."); +} + +- (void)testWrapperIdentifier_PercentEncodedOnRevokeURL { + GIDSignIn.sharedInstance.wrapperIdentifier = @"my-sdk"; + + [[[_authorization expect] andReturn:_authState] authState]; + [[[_authState expect] andReturn:_tokenResponse] lastTokenResponse]; + [[[_tokenResponse expect] andReturn:kAccessToken] accessToken]; + [[[_authorization expect] andReturn:_fetcherService] fetcherService]; + + [_signIn disconnectWithCompletion:nil]; + + XCTAssertTrue([self isFetcherStarted], @"should start fetching"); + NSURL *url = [self fetchedURL]; + NSString *urlString = url.absoluteString; + XCTAssertTrue([urlString containsString:@"gidwrapper=my-sdk"], + @"The revoke URL should contain the percent-encoded 'gidwrapper' parameter."); +} + - (void)testOAuthLogin_ConsentCanceled { [self OAuthLoginWithAddScopesFlow:NO authError:@"access_denied" From ed60d79392c6a697bf81c3691d8e642a293330f5 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:14:54 -0700 Subject: [PATCH 07/17] g-orchestrated: Test gidwrapper emission on token refresh * testWrapperIdentifier_PresentOnRefreshRequestWhenSet * testWrapperIdentifier_AbsentOnRefreshRequestWhenUnset * Captures the OIDTokenRequest in the existing performTokenRequest swizzle so the refresh requests additionalParameters can be asserted; no test file had inspected the request before. * Extends tearDown to clear the process-global identifier between tests. --- GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 60 ++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index dec1caaf..69cd8912 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -23,6 +23,7 @@ #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDToken.h" #import "GoogleSignIn/Sources/GIDGoogleUser_Private.h" +#import "GoogleSignIn/Sources/GIDSignInPreferences.h" #import "GoogleSignIn/Tests/Unit/GIDGoogleUser+Testing.h" #import "GoogleSignIn/Tests/Unit/GIDProfileData+Testing.h" #import "GoogleSignIn/Tests/Unit/OIDAuthState+Testing.h" @@ -67,10 +68,13 @@ @interface GIDGoogleUserTest : XCTestCase @implementation GIDGoogleUserTest { // The saved token fetch handler. OIDTokenCallback _tokenFetchHandler; + // The saved token request. + OIDTokenRequest *_savedTokenRequest; } - (void)setUp { _tokenFetchHandler = nil; + _savedTokenRequest = nil; // We need to use swizzle here because OCMock can not stub class method with arguments. [GULSwizzler swizzleClass:[OIDAuthorizationService class] @@ -80,7 +84,8 @@ - (void)setUp { OIDTokenRequest *request, OIDAuthorizationResponse *authorizationResponse, OIDTokenCallback callback) { - // Save the OIDTokenCallback. + // Save the OIDTokenRequest and OIDTokenCallback. + self->_savedTokenRequest = request; self->_tokenFetchHandler = [callback copy]; }]; } @@ -89,6 +94,7 @@ - (void)tearDown { [GULSwizzler unswizzleClass:[OIDAuthorizationService class] selector:@selector(performTokenRequest:originalAuthorizationResponse:callback:) isClassSelector:YES]; + GIDSignIn.sharedInstance.wrapperIdentifier = nil; } #pragma mark - Tests @@ -478,6 +484,58 @@ - (void)testRefreshTokensIfNeededWithCompletion_noRefresh_givenRefreshTokenExpir [self waitForExpectationsWithTimeout:1 handler:nil]; } +- (void)testWrapperIdentifier_PresentOnRefreshRequestWhenSet { + GIDSignIn.sharedInstance.wrapperIdentifier = @"firebase"; + + // Both tokens expired 10 seconds ago. + GIDGoogleUser *user = [self googleUserWithAccessTokenExpiresIn:-10 idTokenExpiresIn:-10]; + + XCTestExpectation *expectation = [self expectationWithDescription:@"Callback is called"]; + + // Save the intermediate states. + [user refreshTokensIfNeededWithCompletion:^(GIDGoogleUser * _Nullable user, + NSError * _Nullable error) { + [expectation fulfill]; + }]; + + XCTAssertEqualObjects(_savedTokenRequest.additionalParameters[@"gidwrapper"], @"firebase"); + + // Clean up the handler by providing a fake response to fulfill any internal state. + OIDTokenResponse *fakeResponse = [OIDTokenResponse testInstanceWithIDToken:nil + accessToken:kNewAccessToken + expiresIn:@(kAccessTokenExpiresIn) + refreshToken:kRefreshToken + tokenRequest:_savedTokenRequest]; + _tokenFetchHandler(fakeResponse, nil); + [self waitForExpectationsWithTimeout:1 handler:nil]; +} + +- (void)testWrapperIdentifier_AbsentOnRefreshRequestWhenUnset { + GIDSignIn.sharedInstance.wrapperIdentifier = nil; + + // Both tokens expired 10 seconds ago. + GIDGoogleUser *user = [self googleUserWithAccessTokenExpiresIn:-10 idTokenExpiresIn:-10]; + + XCTestExpectation *expectation = [self expectationWithDescription:@"Callback is called"]; + + // Save the intermediate states. + [user refreshTokensIfNeededWithCompletion:^(GIDGoogleUser * _Nullable user, + NSError * _Nullable error) { + [expectation fulfill]; + }]; + + XCTAssertNil(_savedTokenRequest.additionalParameters[@"gidwrapper"]); + + // Clean up the handler by providing a fake response. + OIDTokenResponse *fakeResponse = [OIDTokenResponse testInstanceWithIDToken:nil + accessToken:kNewAccessToken + expiresIn:@(kAccessTokenExpiresIn) + refreshToken:kRefreshToken + tokenRequest:_savedTokenRequest]; + _tokenFetchHandler(fakeResponse, nil); + [self waitForExpectationsWithTimeout:1 handler:nil]; +} + # pragma mark - Test `addScopes:` - (void)testAddScopes_success { From 86ea6e97ea341629a89a83e4f2cd1185b49a230e Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:51:18 -0700 Subject: [PATCH 08/17] g-orchestrated: Validate wrapper identifier instead of rewriting it * Rejects invalid identifiers rather than lowercasing, filtering and truncating them, so a malformed value cannot be silently misattributed to a registered wrapper. * Narrows the accepted charset to [a-z0-9-] to remove multiple spellings of the same name. * First valid write wins; a differing second write is ignored and asserted in debug builds, so multi-wrapper apps report deterministically. * Replaces the dispatch_once lock object with a static os_unfair_lock. * Converts the free C functions GIDVersion/GIDEnvironment/GIDWrapperIdentifier/ GIDSetWrapperIdentifier into GIDSignInPreferences class methods, matching the class idiom already used for the server accessors. * Adds +addLoggingParameters: ahead of consolidating the four emit sites. --- GoogleSignIn/Sources/GIDSignInPreferences.h | 24 ++-- GoogleSignIn/Sources/GIDSignInPreferences.m | 116 +++++++++++--------- 2 files changed, 80 insertions(+), 60 deletions(-) diff --git a/GoogleSignIn/Sources/GIDSignInPreferences.h b/GoogleSignIn/Sources/GIDSignInPreferences.h index e5d8c679..9fbb2c64 100644 --- a/GoogleSignIn/Sources/GIDSignInPreferences.h +++ b/GoogleSignIn/Sources/GIDSignInPreferences.h @@ -22,18 +22,26 @@ extern NSString *const kSDKVersionLoggingParameter; extern NSString *const kEnvironmentLoggingParameter; extern NSString *const kSDKWrapperLoggingParameter; -NSString* GIDVersion(void); +@interface GIDSignInPreferences : NSObject -NSString* GIDEnvironment(void); +// Returns the current Google Sign-In SDK version. ++ (NSString *)sdkVersion; -// Returns the sanitized SDK wrapper identifier, or nil if none has been set. -NSString* _Nullable GIDWrapperIdentifier(void); +// Returns the current Apple execution environment (e.g. ios, macos). ++ (NSString *)environment; -// Sets the SDK wrapper identifier. The value is sanitized (see implementation); -// empty or fully-invalid input clears it. Last write wins. Thread-safe. -void GIDSetWrapperIdentifier(NSString * _Nullable wrapper); +// Returns the current identifier, or nil if none is set. ++ (nullable NSString *)wrapperIdentifier; -@interface GIDSignInPreferences : NSObject +// Sets the SDK wrapper identifier. Valid values are 1-32 characters of [a-z0-9-] with no leading +// or trailing '-'; invalid input is ignored (and asserts in debug builds). The FIRST valid write +// wins and later differing writes are ignored (also asserted in debug); passing nil resets the +// stored value. This method is thread-safe. ++ (void)setWrapperIdentifier:(nullable NSString *)wrapperIdentifier; + +// Populates the standard logging parameters (gpsdk, gidenv, and gidwrapper when set) on the +// supplied dictionary. ++ (void)addLoggingParameters:(NSMutableDictionary *)params; + (NSString *)googleAuthorizationServer; + (NSString *)googleTokenServer; diff --git a/GoogleSignIn/Sources/GIDSignInPreferences.m b/GoogleSignIn/Sources/GIDSignInPreferences.m index 51312085..3f440d7d 100644 --- a/GoogleSignIn/Sources/GIDSignInPreferences.m +++ b/GoogleSignIn/Sources/GIDSignInPreferences.m @@ -14,6 +14,8 @@ #import "GoogleSignIn/Sources/GIDSignInPreferences.h" +#import + NS_ASSUME_NONNULL_BEGIN static NSString *const kLSOServer = @"accounts.google.com"; @@ -38,15 +40,7 @@ static NSString *const kAppleEnvironmentMacOSMacCatalyst = @"macos-cat"; static NSString *gWrapperIdentifier = nil; - -static NSObject* GIDWrapperLock(void) { - static NSObject *lock; - static dispatch_once_t onceToken; - dispatch_once(&onceToken, ^{ - lock = [[NSObject alloc] init]; - }); - return lock; -} +static os_unfair_lock gWrapperIdentifierLock = OS_UNFAIR_LOCK_INIT; #ifndef GID_SDK_VERSION #error "GID_SDK_VERSION is not defined: add -DGID_SDK_VERSION=x.x.x to the build invocation." @@ -58,14 +52,35 @@ #define STR(x) STR_EXPAND(x) #define STR_EXPAND(x) #x -// The prefixed sdk version string to differentiate gid version values used with the legacy gpsdk -// logging key. -NSString* GIDVersion(void) { +static BOOL GIDIsValidWrapperIdentifier(NSString * _Nullable candidate) { + if (candidate == nil) { + return NO; + } + + if (candidate.length == 0 || candidate.length > 32) { + return NO; + } + + NSCharacterSet *allowed = + [NSCharacterSet characterSetWithCharactersInString:@"abcdefghijklmnopqrstuvwxyz0123456789-"]; + if ([candidate rangeOfCharacterFromSet:[allowed invertedSet]].location != NSNotFound) { + return NO; + } + + if ([candidate hasPrefix:@"-"] || [candidate hasSuffix:@"-"]) { + return NO; + } + + return YES; +} + +@implementation GIDSignInPreferences + ++ (NSString *)sdkVersion { return [NSString stringWithFormat:@"gid-%@", @STR(GID_SDK_VERSION)]; } -// Get the current Apple execution environment. -NSString* GIDEnvironment(void) { ++ (NSString *)environment { NSString *appleEnvironment = kAppleEnvironmentUnknown; #if TARGET_OS_MACCATALYST @@ -94,55 +109,52 @@ return appleEnvironment; } -static NSString * _Nullable GIDSanitizeWrapperIdentifier(NSString * _Nullable raw) { - if (!raw) { - return nil; - } - - // Trim leading/trailing whitespace and newlines. - NSString *sanitized = [raw stringByTrimmingCharactersInSet: - [NSCharacterSet whitespaceAndNewlineCharacterSet]]; - if (sanitized.length == 0) { - return nil; - } - - // Lowercase (locale-independent). - sanitized = [sanitized lowercaseString]; - - // Keep only characters in the allowlist [A-Za-z0-9-._~]; drop everything else. - NSCharacterSet *allowed = - [NSCharacterSet characterSetWithCharactersInString:@"abcdefghijklmnopqrstuvwxyz0123456789-._~"]; - sanitized = [[sanitized componentsSeparatedByCharactersInSet:[allowed invertedSet]] - componentsJoinedByString:@""]; ++ (nullable NSString *)wrapperIdentifier { + os_unfair_lock_lock(&gWrapperIdentifierLock); + NSString *wrapper = [gWrapperIdentifier copy]; + os_unfair_lock_unlock(&gWrapperIdentifierLock); + return wrapper; +} - if (sanitized.length == 0) { - return nil; ++ (void)setWrapperIdentifier:(nullable NSString *)wrapperIdentifier { + if (wrapperIdentifier == nil) { + os_unfair_lock_lock(&gWrapperIdentifierLock); + gWrapperIdentifier = nil; + os_unfair_lock_unlock(&gWrapperIdentifierLock); + return; } - // If length > 32, take substringToIndex:32. - // Safe because the allowlist is ASCII, so each character is exactly one UTF-16 unit. - if (sanitized.length > 32) { - sanitized = [sanitized substringToIndex:32]; + if (!GIDIsValidWrapperIdentifier(wrapperIdentifier)) { +#if DEBUG + NSAssert(NO, @"SDK wrapper '%@' rejected: must be 1-32 characters of [a-z0-9-] with no " + @"leading or trailing '-'. Value ignored.", wrapperIdentifier); +#endif + return; } - return sanitized; -} - -NSString* GIDWrapperIdentifier(void) { - @synchronized(GIDWrapperLock()) { - return [gWrapperIdentifier copy]; + os_unfair_lock_lock(&gWrapperIdentifierLock); + NSString *current = gWrapperIdentifier; + if (current != nil && ![current isEqualToString:wrapperIdentifier]) { + os_unfair_lock_unlock(&gWrapperIdentifierLock); +#if DEBUG + NSAssert(NO, @"SDK wrapper already set to '%@'; ignoring '%@'. More than one " + @"wrapper appears to be present.", current, wrapperIdentifier); +#endif + return; } + gWrapperIdentifier = [wrapperIdentifier copy]; + os_unfair_lock_unlock(&gWrapperIdentifierLock); } -void GIDSetWrapperIdentifier(NSString * _Nullable wrapper) { - NSString *sanitized = GIDSanitizeWrapperIdentifier(wrapper); - @synchronized(GIDWrapperLock()) { - gWrapperIdentifier = [sanitized copy]; ++ (void)addLoggingParameters:(NSMutableDictionary *)params { + params[kSDKVersionLoggingParameter] = [self sdkVersion]; + params[kEnvironmentLoggingParameter] = [self environment]; + NSString *wrapper = [self wrapperIdentifier]; + if (wrapper != nil) { + params[kSDKWrapperLoggingParameter] = wrapper; } } -@implementation GIDSignInPreferences - + (NSString *)googleAuthorizationServer { return kLSOServer; } From 3c14cba74fc469e98d5e2e086c129b467630995b Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:56:13 -0700 Subject: [PATCH 09/17] g-orchestrated: Build revoke URL structurally and consolidate logging parameters * Replaces stringWithFormat query assembly in the revoke path with NSURLComponents/NSURLQueryItem, making query injection structurally impossible for every parameter rather than filtered for one. * Drops the manual percent-encoding, which was unreachable given the validator. * Routes all three emit sites, including revoke, through +[GIDSignInPreferences addLoggingParameters:]; query items are sorted by name so the emitted URL is deterministic. * Reports kGIDSignInErrorCodeUnknown if the revoke URL cannot be built, rather than signing out and reporting success while the token is still live server-side. * Updates the property accessors to call the GIDSignInPreferences class methods. * Rewrites the wrapperIdentifier doc comment to state naming policy instead of sanitizer mechanics. --- GoogleSignIn/Sources/GIDSignIn.m | 66 +++++++++---------- .../Sources/Public/GoogleSignIn/GIDSignIn.h | 23 +++++-- 2 files changed, 49 insertions(+), 40 deletions(-) diff --git a/GoogleSignIn/Sources/GIDSignIn.m b/GoogleSignIn/Sources/GIDSignIn.m index e8a011c2..9d54dafb 100644 --- a/GoogleSignIn/Sources/GIDSignIn.m +++ b/GoogleSignIn/Sources/GIDSignIn.m @@ -573,26 +573,36 @@ - (void)disconnectWithCompletion:(nullable GIDDisconnectCompletion)completion { } return; } - NSString *revokeURLString = [NSString stringWithFormat:kRevokeTokenURLTemplate, + NSString *baseURLString = [NSString stringWithFormat:kRevokeTokenURLTemplate, [GIDSignInPreferences googleAuthorizationServer], token]; - // Append logging parameter - revokeURLString = [NSString stringWithFormat:@"%@&%@=%@&%@=%@", - revokeURLString, - kSDKVersionLoggingParameter, - GIDVersion(), - kEnvironmentLoggingParameter, - GIDEnvironment()]; - NSString *wrapperIdentifier = GIDWrapperIdentifier(); - if (wrapperIdentifier) { - NSMutableCharacterSet *unreservedSet = - [NSMutableCharacterSet alphanumericCharacterSet]; - [unreservedSet addCharactersInString:@"-._~"]; - NSString *encodedWrapper = [wrapperIdentifier - stringByAddingPercentEncodingWithAllowedCharacters:unreservedSet]; - revokeURLString = [NSString stringWithFormat:@"%@&%@=%@", - revokeURLString, kSDKWrapperLoggingParameter, encodedWrapper]; + NSURLComponents *components = [NSURLComponents componentsWithString:baseURLString]; + NSURL *revokeURL; + if (components) { + NSMutableArray *items = + [components.queryItems mutableCopy] ?: [NSMutableArray array]; + + NSMutableDictionary *loggingParams = [[NSMutableDictionary alloc] init]; + [GIDSignInPreferences addLoggingParameters:loggingParams]; + for (NSString *name in [loggingParams.allKeys sortedArrayUsingSelector:@selector(compare:)]) { + [items addObject:[NSURLQueryItem queryItemWithName:name value:loggingParams[name]]]; + } + + components.queryItems = items; + revokeURL = components.URL; + } + + if (!revokeURL) { + // The revoke URL could not be constructed, so the token was left untouched. + NSError *error = [NSError errorWithDomain:kGIDSignInErrorDomain + code:kGIDSignInErrorCodeUnknown + userInfo:nil]; + if (completion) { + dispatch_async(dispatch_get_main_queue(), ^{ + completion(error); + }); + } + return; } - NSURL *revokeURL = [NSURL URLWithString:revokeURLString]; [self startFetchURL:revokeURL fromAuthState:authState withComment:@"GIDSignIn: revoke tokens" @@ -636,11 +646,11 @@ + (GIDSignIn *)sharedInstance { } - (nullable NSString *)wrapperIdentifier { - return GIDWrapperIdentifier(); + return [GIDSignInPreferences wrapperIdentifier]; } - (void)setWrapperIdentifier:(nullable NSString *)wrapperIdentifier { - GIDSetWrapperIdentifier(wrapperIdentifier); + [GIDSignInPreferences setWrapperIdentifier:wrapperIdentifier]; } #pragma mark - Configuring and pre-warming @@ -936,13 +946,7 @@ - (void)authorizationRequestWithOptions:(GIDSignInInternalOptions *)options comp #elif TARGET_OS_OSX || TARGET_OS_MACCATALYST [additionalParameters addEntriesFromDictionary:options.extraParams]; #endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST - additionalParameters[kSDKVersionLoggingParameter] = GIDVersion(); - additionalParameters[kEnvironmentLoggingParameter] = GIDEnvironment(); - - NSString *wrapperIdentifier = GIDWrapperIdentifier(); - if (wrapperIdentifier) { - additionalParameters[kSDKWrapperLoggingParameter] = wrapperIdentifier; - } + [GIDSignInPreferences addLoggingParameters:additionalParameters]; return additionalParameters; } @@ -1077,13 +1081,7 @@ - (void)maybeFetchToken:(GIDAuthFlow *)authFlow { emmSupport:authFlow.emmSupport isPasscodeInfoRequired:passcodeInfoRequired.length > 0]]; #endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST - additionalParameters[kSDKVersionLoggingParameter] = GIDVersion(); - additionalParameters[kEnvironmentLoggingParameter] = GIDEnvironment(); - - NSString *wrapperIdentifier = GIDWrapperIdentifier(); - if (wrapperIdentifier) { - additionalParameters[kSDKWrapperLoggingParameter] = wrapperIdentifier; - } + [GIDSignInPreferences addLoggingParameters:additionalParameters]; OIDTokenRequest *tokenRequest; if (!authState.lastTokenResponse.accessToken && diff --git a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h index d04a990d..7ccc90a0 100644 --- a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h +++ b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h @@ -73,12 +73,23 @@ typedef NS_ERROR_ENUM(kGIDSignInErrorDomain, GIDSignInErrorCode) { /// The active configuration for this instance of `GIDSignIn`. @property(nonatomic, nullable) GIDConfiguration *configuration; -/// An optional identifier naming the SDK or wrapper that embeds Google Sign-In. -/// Reported to Google as a diagnostic logging parameter for aggregate metrics only; -/// it is never used for authentication or authorization. Set this once, before your -/// first sign-in call. The value is sanitized: lowercased, restricted to the -/// characters A-Z a-z 0-9 - . _ ~, and capped at 32 characters; input that does not -/// conform is truncated or ignored. +/// An optional identifier naming the SDK or wrapper that embeds Google Sign-In, +/// reported to Google as a diagnostic parameter for aggregate metrics only; it +/// is never used for authentication or authorization. +/// +/// Format: 1 to 32 characters, lowercase letters, digits and hyphens only, not +/// starting or ending with a hyphen. Non-conforming values are ignored and +/// assert in debug builds. +/// +/// Policy: +/// * Choose one stable name and keep it stable across your releases. +/// * Do NOT encode your version in it; per-release identifiers make aggregate +/// metrics useless. +/// * Never include anything user-specific, app-specific, or identifying. +/// * Register your identifier with Google before shipping it. +/// +/// Set this once, before your first sign-in call. The first valid value wins; +/// later differing values are ignored. @property(nonatomic, nullable) NSString *wrapperIdentifier; #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST From eb938c5b57f93684396e3fbe2028c797df0e164f Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:53:58 -0700 Subject: [PATCH 10/17] g-orchestrated: Route token refresh through shared logging parameters * Replaces the fourth copy of the logging-parameter block with +[GIDSignInPreferences addLoggingParameters:]; behavior is unchanged. --- GoogleSignIn/Sources/GIDGoogleUser.m | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index b0063584..c6979570 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -153,12 +153,7 @@ - (void)refreshTokensIfNeededWithCompletion:(GIDGoogleUserCompletion)completion [additionalParameters addEntriesFromDictionary: self.authState.lastTokenResponse.request.additionalParameters]; #endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST - additionalParameters[kSDKVersionLoggingParameter] = GIDVersion(); - additionalParameters[kEnvironmentLoggingParameter] = GIDEnvironment(); - NSString *wrapperIdentifier = GIDWrapperIdentifier(); - if (wrapperIdentifier) { - additionalParameters[kSDKWrapperLoggingParameter] = wrapperIdentifier; - } + [GIDSignInPreferences addLoggingParameters:additionalParameters]; OIDTokenRequest *tokenRefreshRequest = [self.authState tokenRefreshRequestWithAdditionalParameters:additionalParameters]; From f5c1c103673c1c57b7409a648e33dd389388bd84 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:53:37 -0700 Subject: [PATCH 11/17] g-orchestrated: Changelog: revise wrapperIdentifier entry * Replaces the "sanitized" wording with the accepted format and the reject-rather- than-reshape behavior. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9afaf376..de50126d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,5 @@ # Unreleased -- Add `GIDSignIn.wrapperIdentifier`, an optional identifier that SDKs embedding Google Sign-In can set to self-identify in Google's diagnostic logs via a new `gidwrapper` parameter. The value is sanitized and opt-in; default behavior is unchanged. +- Add `GIDSignIn.wrapperIdentifier`, an optional property for SDKs embedding Google Sign-In to self-identify in Google's diagnostic logs via a new `gidwrapper` parameter. It accepts 1-32 characters of lowercase letters, digits and hyphens; non-conforming values are ignored. It is opt-in and default behavior is unchanged. # 9.2.0 - Expose the refresh token expiration date ([#577](https://github.com/google/GoogleSignIn-iOS/pull/577)) From f68eb8676a38a793e2ee72b6e88e295b6ca8a6f0 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:59:35 -0700 Subject: [PATCH 12/17] g-orchestrated: Test wrapper identifier validation and write-once Deletes nine obsolete tests that asserted the old normalizing behavior: Lowercases, StripsDisallowedCharacters, KeepsAllowedPunctuation, TrimsWhitespace, CapsAtThirtyTwoCharacters, LastWriteWins, EmptyOrWhitespaceBecomesNil, RoundTripsSimpleValue, UnsetReturnsNil. Adds fifteen tests for the new contract: four acceptance cases, six rejection cases, and five covering write-once, nil-reset and rejected-write-preserves-value. Rejection cases are included. The debug NSAssert raises NSInternalInconsistencyException, so each invalid write is wrapped in XCTAssertThrowsSpecificNamed and followed by an assertion that the store was left untouched. This couples the suite to a debug build, which is how the unit tests already run. Also updates testGIDVersion/testGIDEnvironment to the new +[GIDSignInPreferences sdkVersion] / +environment class methods. --- .../Tests/Unit/GIDSignInPreferencesTest.m | 166 ++++++++++++------ 1 file changed, 117 insertions(+), 49 deletions(-) diff --git a/GoogleSignIn/Tests/Unit/GIDSignInPreferencesTest.m b/GoogleSignIn/Tests/Unit/GIDSignInPreferencesTest.m index e20a72ce..c141e207 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInPreferencesTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInPreferencesTest.m @@ -22,12 +22,12 @@ @interface GIDSignInPreferencesTest : XCTestCase @implementation GIDSignInPreferencesTest - (void)testGIDVersion { - NSString *version = GIDVersion(); + NSString *version = [GIDSignInPreferences sdkVersion]; XCTAssertTrue([version hasPrefix:@"gid-"]); } - (void)testGIDEnvironment { - NSString *environment = GIDEnvironment(); + NSString *environment = [GIDSignInPreferences environment]; NSString *expectedEnvironment; #if TARGET_OS_MACCATALYST @@ -45,74 +45,142 @@ - (void)testGIDEnvironment { } - (void)tearDown { - GIDSetWrapperIdentifier(nil); + [GIDSignInPreferences setWrapperIdentifier:nil]; [super tearDown]; } -- (void)testWrapperIdentifier_UnsetReturnsNil { - // Test that when no identifier is set (or it is explicitly cleared), nil is returned. - GIDSetWrapperIdentifier(nil); - XCTAssertNil(GIDWrapperIdentifier()); +- (void)testWrapperIdentifier_UnsetIsNil { + // Test that when no identifier is set, nil is returned. + [GIDSignInPreferences setWrapperIdentifier:nil]; + XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); } -- (void)testWrapperIdentifier_RoundTripsSimpleValue { - // Test that a simple alphanumeric identifier is correctly stored and retrieved. - GIDSetWrapperIdentifier(@"firebase"); - XCTAssertEqualObjects(GIDWrapperIdentifier(), @"firebase"); +- (void)testWrapperIdentifier_AcceptsSimpleValue { + // Test that a simple lowercase alphanumeric identifier is accepted. + [GIDSignInPreferences setWrapperIdentifier:@"firebase"]; + XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], @"firebase"); } -- (void)testWrapperIdentifier_Lowercases { - // Test that identifiers are automatically lowercased when stored. - GIDSetWrapperIdentifier(@"FireBase"); - XCTAssertEqualObjects(GIDWrapperIdentifier(), @"firebase"); +- (void)testWrapperIdentifier_AcceptsHyphenatedValue { + // Test that a value with internal hyphens is accepted. + [GIDSignInPreferences setWrapperIdentifier:@"react-native"]; + XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], @"react-native"); } -- (void)testWrapperIdentifier_StripsDisallowedCharacters { - // Test that spaces and disallowed punctuation are removed from the identifier. - GIDSetWrapperIdentifier(@"fire base!/&=?"); - XCTAssertEqualObjects(GIDWrapperIdentifier(), @"firebase"); +- (void)testWrapperIdentifier_AcceptsDigits { + // Test that a value with digits is accepted. + [GIDSignInPreferences setWrapperIdentifier:@"wrapper2"]; + XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], @"wrapper2"); } -- (void)testWrapperIdentifier_KeepsAllowedPunctuation { - // Test that allowed characters (A-Za-z0-9-._~) are preserved. - GIDSetWrapperIdentifier(@"my-sdk_1.2~x"); - XCTAssertEqualObjects(GIDWrapperIdentifier(), @"my-sdk_1.2~x"); +- (void)testWrapperIdentifier_AcceptsMaximumLength { + // Test that a 32-character valid identifier is accepted and not truncated. + NSString *maxLength = @"abcdefghijklmnopqrstuvwxyz123456"; // 32 chars + [GIDSignInPreferences setWrapperIdentifier:maxLength]; + XCTAssertEqual([GIDSignInPreferences wrapperIdentifier].length, (NSUInteger)32); + XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], maxLength); } -- (void)testWrapperIdentifier_TrimsWhitespace { - // Test that leading and trailing whitespace is trimmed from the identifier. - GIDSetWrapperIdentifier(@" firebase "); - XCTAssertEqualObjects(GIDWrapperIdentifier(), @"firebase"); +- (void)testWrapperIdentifier_RejectsUppercase { + // Test that uppercase characters cause a rejection. + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"Firebase"], + NSException, + NSInternalInconsistencyException); + XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); } -- (void)testWrapperIdentifier_EmptyOrWhitespaceBecomesNil { - // Test that empty or whitespace-only strings result in a nil identifier. - GIDSetWrapperIdentifier(@" "); - XCTAssertNil(GIDWrapperIdentifier()); +- (void)testWrapperIdentifier_RejectsWhitespace { + // Test that leading/trailing or internal whitespace cause a rejection. + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@" firebase "], + NSException, + NSInternalInconsistencyException); + XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); + + [GIDSignInPreferences setWrapperIdentifier:nil]; + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"fire base"], + NSException, + NSInternalInconsistencyException); + XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); +} + +- (void)testWrapperIdentifier_RejectsPunctuation { + // Test that disallowed punctuation causes a rejection. + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"my-sdk_1.2~x"], + NSException, + NSInternalInconsistencyException); + XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); + + [GIDSignInPreferences setWrapperIdentifier:nil]; + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"fire!base"], + NSException, + NSInternalInconsistencyException); + XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); +} - // Test that a string consisting only of invalid characters results in a nil identifier. - GIDSetWrapperIdentifier(@"!!!"); - XCTAssertNil(GIDWrapperIdentifier()); +- (void)testWrapperIdentifier_RejectsEmpty { + // Test that an empty string is rejected. + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@""], + NSException, + NSInternalInconsistencyException); + XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); } -- (void)testWrapperIdentifier_CapsAtThirtyTwoCharacters { - // Test that the identifier is truncated to a maximum of 32 characters. - NSString *longString = @"abcdefghijklmnopqrstuvwxyz1234567890"; // 36 characters - GIDSetWrapperIdentifier(longString); - NSString *result = GIDWrapperIdentifier(); - XCTAssertEqual(result.length, (NSUInteger)32); - XCTAssertEqualObjects(result, [longString.lowercaseString substringToIndex:32]); +- (void)testWrapperIdentifier_RejectsOverLength { + // Test that a 33-character identifier is rejected. + NSString *overLength = @"abcdefghijklmnopqrstuvwxyz1234567"; // 33 chars + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:overLength], + NSException, + NSInternalInconsistencyException); + XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); } -- (void)testWrapperIdentifier_LastWriteWins { - // Test that subsequent writes overwrite the previous identifier. - GIDSetWrapperIdentifier(@"first"); - GIDSetWrapperIdentifier(@"second"); - XCTAssertEqualObjects(GIDWrapperIdentifier(), @"second"); +- (void)testWrapperIdentifier_RejectsLeadingOrTrailingHyphen { + // Test that leading or trailing hyphens cause a rejection. + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"-sdk"], + NSException, + NSInternalInconsistencyException); + XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); + + [GIDSignInPreferences setWrapperIdentifier:nil]; + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"sdk-"], + NSException, + NSInternalInconsistencyException); + XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); +} + +- (void)testWrapperIdentifier_FirstValidWriteWins { + // Test that the first valid write is persistent and subsequent differing writes are rejected. + [GIDSignInPreferences setWrapperIdentifier:@"first"]; + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"second"], + NSException, + NSInternalInconsistencyException); + XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], @"first"); +} + +- (void)testWrapperIdentifier_RepeatedIdenticalWriteIsAccepted { + // Test that writing the same valid value again does not throw or change the state. + [GIDSignInPreferences setWrapperIdentifier:@"firebase"]; + XCTAssertNoThrow([GIDSignInPreferences setWrapperIdentifier:@"firebase"]); + XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], @"firebase"); +} + +- (void)testWrapperIdentifier_NilResets { + // Test that passing nil resets the store, allowing a new first-write. + [GIDSignInPreferences setWrapperIdentifier:@"firebase"]; + [GIDSignInPreferences setWrapperIdentifier:nil]; + XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); + + [GIDSignInPreferences setWrapperIdentifier:@"second"]; + XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], @"second"); +} - // Test that setting it to nil clears the identifier. - GIDSetWrapperIdentifier(nil); - XCTAssertNil(GIDWrapperIdentifier()); +- (void)testWrapperIdentifier_RejectedWriteLeavesPreviousValue { + // Test that a rejected write does not clear or change a previously set valid value. + [GIDSignInPreferences setWrapperIdentifier:@"firebase"]; + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"Invalid!"], + NSException, + NSInternalInconsistencyException); + XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], @"firebase"); } @end From 75f29a744984467af0e3fe5b7236da99127793de Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Tue, 4 Aug 2026 19:04:10 -0700 Subject: [PATCH 13/17] g-orchestrated: Test gidwrapper on revoke URL via query items * Replaces the vacuous testWrapperIdentifier_PercentEncodedOnRevokeURL, which substring-matched "gidwrapper=my-sdk" with an input percent-encoding never altered, so the assertion could not fail. The percent-encoding it was named for is gone. * Adds testWrapperIdentifier_PresentOnRevokeURL and testWrapperIdentifier_AbsentFromRevokeURLWhenUnset, which parse the captured URL with NSURLComponents and assert on queryItems. Both also assert gpsdk, gidenv and the token parameter survive, guarding the NSURLComponents rewrite against dropping them. * Replaces testWrapperIdentifier_PropertyReflectsSanitizedValue with testWrapperIdentifier_InvalidValueIsIgnored, asserting the rejected assignment throws and leaves the property nil. * Adds a local valueForQueryItemName:inArray: helper. * Updates the pre-existing gpsdk/gidenv assertions to the new +[GIDSignInPreferences sdkVersion] / +environment class methods. * No test skipped; the existing revoke capture harness was reused throughout. --- GoogleSignIn/Tests/Unit/GIDSignInTest.m | 81 +++++++++++++++++++++---- 1 file changed, 69 insertions(+), 12 deletions(-) diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index e747734d..60d332b9 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -1217,13 +1217,15 @@ - (void)testWrapperIdentifier_AbsentFromAuthorizationRequestWhenUnset { "when unset."); } -- (void)testWrapperIdentifier_PropertyReflectsSanitizedValue { - GIDSignIn.sharedInstance.wrapperIdentifier = @"Fire Base!"; - XCTAssertEqualObjects(GIDSignIn.sharedInstance.wrapperIdentifier, @"firebase", - @"The wrapperIdentifier property should reflect the sanitized value."); +- (void)testWrapperIdentifier_InvalidValueIsIgnored { + XCTAssertThrowsSpecificNamed(GIDSignIn.sharedInstance.wrapperIdentifier = @"Fire Base!", + NSException, NSInternalInconsistencyException, + @"Setting an invalid wrapper identifier should throw."); + XCTAssertNil(GIDSignIn.sharedInstance.wrapperIdentifier, + @"The wrapper identifier should be nil after an invalid assignment."); } -- (void)testWrapperIdentifier_PercentEncodedOnRevokeURL { +- (void)testWrapperIdentifier_PresentOnRevokeURL { GIDSignIn.sharedInstance.wrapperIdentifier = @"my-sdk"; [[[_authorization expect] andReturn:_authState] authState]; @@ -1235,9 +1237,51 @@ - (void)testWrapperIdentifier_PercentEncodedOnRevokeURL { XCTAssertTrue([self isFetcherStarted], @"should start fetching"); NSURL *url = [self fetchedURL]; - NSString *urlString = url.absoluteString; - XCTAssertTrue([urlString containsString:@"gidwrapper=my-sdk"], - @"The revoke URL should contain the percent-encoded 'gidwrapper' parameter."); + NSURLComponents *components = [NSURLComponents componentsWithURL:url + resolvingAgainstBaseURL:NO]; + NSArray *queryItems = components.queryItems; + + XCTAssertEqualObjects([self valueForQueryItemName:@"gidwrapper" inArray:queryItems], + @"my-sdk", @"The revoke URL should contain the 'gidwrapper' parameter."); + XCTAssertEqualObjects([self valueForQueryItemName:kSDKVersionLoggingParameter inArray:queryItems], + [GIDSignInPreferences sdkVersion], + @"The revoke URL should contain the SDK version parameter."); + XCTAssertEqualObjects([self valueForQueryItemName:kEnvironmentLoggingParameter + inArray:queryItems], + [GIDSignInPreferences environment], + @"The revoke URL should contain the environment parameter."); + XCTAssertEqualObjects([self valueForQueryItemName:@"token" inArray:queryItems], + kAccessToken, @"The revoke URL should contain the 'token' parameter."); +} + +- (void)testWrapperIdentifier_AbsentFromRevokeURLWhenUnset { + GIDSignIn.sharedInstance.wrapperIdentifier = nil; + + [[[_authorization expect] andReturn:_authState] authState]; + [[[_authState expect] andReturn:_tokenResponse] lastTokenResponse]; + [[[_tokenResponse expect] andReturn:kAccessToken] accessToken]; + [[[_authorization expect] andReturn:_fetcherService] fetcherService]; + + [_signIn disconnectWithCompletion:nil]; + + XCTAssertTrue([self isFetcherStarted], @"should start fetching"); + NSURL *url = [self fetchedURL]; + NSURLComponents *components = [NSURLComponents componentsWithURL:url + resolvingAgainstBaseURL:NO]; + NSArray *queryItems = components.queryItems; + + XCTAssertNil([self valueForQueryItemName:@"gidwrapper" inArray:queryItems], + @"The revoke URL should not contain the 'gidwrapper' parameter when unset."); + XCTAssertEqualObjects([self valueForQueryItemName:kSDKVersionLoggingParameter inArray:queryItems], + [GIDSignInPreferences sdkVersion], + @"The revoke URL should still contain the SDK version parameter."); + XCTAssertEqualObjects([self valueForQueryItemName:kEnvironmentLoggingParameter + inArray:queryItems], + [GIDSignInPreferences environment], + @"The revoke URL should still contain the environment parameter."); + XCTAssertEqualObjects([self valueForQueryItemName:@"token" inArray:queryItems], + kAccessToken, + @"The revoke URL should still contain the 'token' parameter."); } - (void)testOAuthLogin_ConsentCanceled { @@ -1753,6 +1797,17 @@ - (void)testTokenEndpointEMMError { #pragma mark - Helpers +// Returns the value for the query item with the given name in the array of query items. +- (nullable NSString *)valueForQueryItemName:(NSString *)name + inArray:(NSArray *)queryItems { + for (NSURLQueryItem *item in queryItems) { + if ([item.name isEqualToString:name]) { + return item.value; + } + } + return nil; +} + // Whether or not a fetcher has been started. - (BOOL)isFetcherStarted { NSUInteger count = _fetcherService.fetchers.count; @@ -1795,9 +1850,11 @@ - (void)verifyAndRevokeToken:(NSString *)token NSDictionary *> *params = queryComponent.dictionaryValue; XCTAssertEqualObjects([params valueForKey:@"token"], token, @"token parameter should match"); - XCTAssertEqualObjects([params valueForKey:kSDKVersionLoggingParameter], GIDVersion(), + XCTAssertEqualObjects([params valueForKey:kSDKVersionLoggingParameter], + [GIDSignInPreferences sdkVersion], @"SDK version logging parameter should match"); - XCTAssertEqualObjects([params valueForKey:kEnvironmentLoggingParameter], GIDEnvironment(), + XCTAssertEqualObjects([params valueForKey:kEnvironmentLoggingParameter], + [GIDSignInPreferences environment], @"Environment logging parameter should match"); // Emulate result back from server. [self didFetch:nil error:nil]; @@ -1963,8 +2020,8 @@ - (void)OAuthLoginWithAddScopesFlow:(BOOL)addScopesFlow XCTAssertNotNil(_savedAuthorizationRequest); NSDictionary *params = _savedAuthorizationRequest.additionalParameters; XCTAssertEqualObjects(params[@"include_granted_scopes"], @"true"); - XCTAssertEqualObjects(params[kSDKVersionLoggingParameter], GIDVersion()); - XCTAssertEqualObjects(params[kEnvironmentLoggingParameter], GIDEnvironment()); + XCTAssertEqualObjects(params[kSDKVersionLoggingParameter], [GIDSignInPreferences sdkVersion]); + XCTAssertEqualObjects(params[kEnvironmentLoggingParameter], [GIDSignInPreferences environment]); XCTAssertNotNil(_savedAuthorizationCallback); #if TARGET_OS_IOS || TARGET_OS_MACCATALYST XCTAssertEqual(_savedPresentingViewController, _presentingViewController); From 186d12ec0bf5aef7f5d11f2f51bff391c7c9e701 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:59:59 -0700 Subject: [PATCH 14/17] g-orchestrated: Test logging parameters on refresh after consolidation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Adds testWrapperIdentifier_AbsentOnRefreshRequestWhenInvalid, using the invalid-value variant: the rejected write is asserted to throw NSInternalInconsistencyException and gidwrapper is then absent from the refresh request. * Also asserts gpsdk and gidenv ARE still present, which is the point of the test — all three parameters now come from +[GIDSignInPreferences addLoggingParameters:], so a mistake in that consolidation would drop them together. * The two existing wrapper refresh tests are unchanged; both values remain valid. --- GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 38 +++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index 69cd8912..8cd60e76 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -536,6 +536,44 @@ - (void)testWrapperIdentifier_AbsentOnRefreshRequestWhenUnset { [self waitForExpectationsWithTimeout:1 handler:nil]; } +- (void)testWrapperIdentifier_AbsentOnRefreshRequestWhenInvalid { + // Assert that attempting to set an invalid identifier throws NSInternalInconsistencyException. + XCTAssertThrowsSpecificNamed(GIDSignIn.sharedInstance.wrapperIdentifier = @"Fire Base!", + NSException, NSInternalInconsistencyException); + + // The rejection leaves the store nil. + XCTAssertNil(GIDSignIn.sharedInstance.wrapperIdentifier); + + // Both tokens expired 10 seconds ago. + GIDGoogleUser *user = [self googleUserWithAccessTokenExpiresIn:-10 idTokenExpiresIn:-10]; + + XCTestExpectation *expectation = [self expectationWithDescription:@"Callback is called"]; + + // Save the intermediate states. + [user refreshTokensIfNeededWithCompletion:^(GIDGoogleUser * _Nullable user, + NSError * _Nullable error) { + [expectation fulfill]; + }]; + + // Assert the captured token request additionalParameters does NOT contain key @"gidwrapper". + XCTAssertNil(_savedTokenRequest.additionalParameters[@"gidwrapper"]); + + // Assert it DOES contain kSDKVersionLoggingParameter and kEnvironmentLoggingParameter. + XCTAssertEqualObjects(_savedTokenRequest.additionalParameters[kSDKVersionLoggingParameter], + [GIDSignInPreferences sdkVersion]); + XCTAssertEqualObjects(_savedTokenRequest.additionalParameters[kEnvironmentLoggingParameter], + [GIDSignInPreferences environment]); + + // Clean up the handler by providing a fake response. + OIDTokenResponse *fakeResponse = [OIDTokenResponse testInstanceWithIDToken:nil + accessToken:kNewAccessToken + expiresIn:@(kAccessTokenExpiresIn) + refreshToken:kRefreshToken + tokenRequest:_savedTokenRequest]; + _tokenFetchHandler(fakeResponse, nil); + [self waitForExpectationsWithTimeout:1 handler:nil]; +} + # pragma mark - Test `addScopes:` - (void)testAddScopes_success { From 467893c5c94c9a0f00f109c30fad4367a1d20b9f Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Fri, 7 Aug 2026 18:26:34 -0700 Subject: [PATCH 15/17] g-orchestrated: Match Android's wrapper identifier sanitization rules --- CHANGELOG.md | 2 +- GoogleSignIn/Sources/GIDSignInPreferences.h | 8 +-- GoogleSignIn/Sources/GIDSignInPreferences.m | 50 +++++++++++-------- .../Sources/Public/GoogleSignIn/GIDSignIn.h | 7 +-- 4 files changed, 39 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de50126d..13b13ffe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,5 @@ # Unreleased -- Add `GIDSignIn.wrapperIdentifier`, an optional property for SDKs embedding Google Sign-In to self-identify in Google's diagnostic logs via a new `gidwrapper` parameter. It accepts 1-32 characters of lowercase letters, digits and hyphens; non-conforming values are ignored. It is opt-in and default behavior is unchanged. +- Add `GIDSignIn.wrapperIdentifier`, an optional property for SDKs embedding Google Sign-In to self-identify in Google's diagnostic logs via a new `gidwrapper` parameter. It accepts up to 100 printable ASCII characters; longer values are truncated, and values containing non-ASCII or control characters are dropped. It is opt-in and default behavior is unchanged. # 9.2.0 - Expose the refresh token expiration date ([#577](https://github.com/google/GoogleSignIn-iOS/pull/577)) diff --git a/GoogleSignIn/Sources/GIDSignInPreferences.h b/GoogleSignIn/Sources/GIDSignInPreferences.h index 9fbb2c64..1b2ea949 100644 --- a/GoogleSignIn/Sources/GIDSignInPreferences.h +++ b/GoogleSignIn/Sources/GIDSignInPreferences.h @@ -33,10 +33,10 @@ extern NSString *const kSDKWrapperLoggingParameter; // Returns the current identifier, or nil if none is set. + (nullable NSString *)wrapperIdentifier; -// Sets the SDK wrapper identifier. Valid values are 1-32 characters of [a-z0-9-] with no leading -// or trailing '-'; invalid input is ignored (and asserts in debug builds). The FIRST valid write -// wins and later differing writes are ignored (also asserted in debug); passing nil resets the -// stored value. This method is thread-safe. +// Sets the SDK wrapper identifier. Values may be up to 100 printable ASCII characters; longer +// values are truncated to 100; a value containing any non-ASCII or control character, or an empty +// string, is dropped entirely (and asserts in debug builds); the first accepted write wins and +// later differing writes are ignored; nil resets; the method is thread-safe. + (void)setWrapperIdentifier:(nullable NSString *)wrapperIdentifier; // Populates the standard logging parameters (gpsdk, gidenv, and gidwrapper when set) on the diff --git a/GoogleSignIn/Sources/GIDSignInPreferences.m b/GoogleSignIn/Sources/GIDSignInPreferences.m index 3f440d7d..256e6a48 100644 --- a/GoogleSignIn/Sources/GIDSignInPreferences.m +++ b/GoogleSignIn/Sources/GIDSignInPreferences.m @@ -52,26 +52,35 @@ #define STR(x) STR_EXPAND(x) #define STR_EXPAND(x) #x -static BOOL GIDIsValidWrapperIdentifier(NSString * _Nullable candidate) { - if (candidate == nil) { - return NO; +// Returns the sanitized form of `candidate`, or nil if it must be dropped. +// Callers must not pass nil. +static NSString * _Nullable GIDSanitizedWrapperIdentifier(NSString *candidate) { + static NSCharacterSet *allowedSet; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + // The range of printable ASCII characters is U+0020 through U+007E inclusive. + allowedSet = [NSCharacterSet characterSetWithRange:NSMakeRange(0x20, 0x5F)]; + }); + + // DROP CHECK FIRST: if it contains any character outside the printable ASCII range, drop it. + if ([candidate rangeOfCharacterFromSet:[allowedSet invertedSet]].location != NSNotFound) { + return nil; } - if (candidate.length == 0 || candidate.length > 32) { - return NO; + // An empty string is also discarded. + if (candidate.length == 0) { + return nil; } - NSCharacterSet *allowed = - [NSCharacterSet characterSetWithCharactersInString:@"abcdefghijklmnopqrstuvwxyz0123456789-"]; - if ([candidate rangeOfCharacterFromSet:[allowed invertedSet]].location != NSNotFound) { - return NO; + // TRUNCATE SECOND: A surviving string longer than 100 characters is truncated to 100. + if (candidate.length > 100) { + // Truncating with -substringToIndex:100 is safe here precisely because the drop check has + // already guaranteed every character is single-unit ASCII, so there is no risk of splitting + // a surrogate pair. + return [candidate substringToIndex:100]; } - if ([candidate hasPrefix:@"-"] || [candidate hasSuffix:@"-"]) { - return NO; - } - - return YES; + return candidate; } @implementation GIDSignInPreferences @@ -124,25 +133,26 @@ + (void)setWrapperIdentifier:(nullable NSString *)wrapperIdentifier { return; } - if (!GIDIsValidWrapperIdentifier(wrapperIdentifier)) { + NSString *sanitized = GIDSanitizedWrapperIdentifier(wrapperIdentifier); + if (sanitized == nil) { #if DEBUG - NSAssert(NO, @"SDK wrapper '%@' rejected: must be 1-32 characters of [a-z0-9-] with no " - @"leading or trailing '-'. Value ignored.", wrapperIdentifier); + NSAssert(NO, @"SDK wrapper '%@' rejected: must not be empty and must only contain printable " + @"ASCII characters (U+0020 to U+007E). Value ignored.", wrapperIdentifier); #endif return; } os_unfair_lock_lock(&gWrapperIdentifierLock); NSString *current = gWrapperIdentifier; - if (current != nil && ![current isEqualToString:wrapperIdentifier]) { + if (current != nil && ![current isEqualToString:sanitized]) { os_unfair_lock_unlock(&gWrapperIdentifierLock); #if DEBUG NSAssert(NO, @"SDK wrapper already set to '%@'; ignoring '%@'. More than one " - @"wrapper appears to be present.", current, wrapperIdentifier); + @"wrapper appears to be present.", current, sanitized); #endif return; } - gWrapperIdentifier = [wrapperIdentifier copy]; + gWrapperIdentifier = [sanitized copy]; os_unfair_lock_unlock(&gWrapperIdentifierLock); } diff --git a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h index 7ccc90a0..c7995c79 100644 --- a/GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h +++ b/GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h @@ -77,9 +77,10 @@ typedef NS_ERROR_ENUM(kGIDSignInErrorDomain, GIDSignInErrorCode) { /// reported to Google as a diagnostic parameter for aggregate metrics only; it /// is never used for authentication or authorization. /// -/// Format: 1 to 32 characters, lowercase letters, digits and hyphens only, not -/// starting or ending with a hyphen. Non-conforming values are ignored and -/// assert in debug builds. +/// Format: up to 100 printable ASCII characters (U+0020 to U+007E). A longer +/// value is truncated to its first 100 characters. A value containing any +/// non-ASCII character or any ASCII control character is dropped in its +/// entirety, and asserts in debug builds. /// /// Policy: /// * Choose one stable name and keep it stable across your releases. From f9510880f89a1807a5e1792b1d62c1c35ec22b92 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Fri, 7 Aug 2026 18:31:00 -0700 Subject: [PATCH 16/17] g-orchestrated: Test the Android-matching wrapper identifier rules --- GoogleSignIn/Sources/GIDSignInPreferences.m | 5 +- GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m | 6 +- .../Tests/Unit/GIDSignInPreferencesTest.m | 130 +++++++++--------- GoogleSignIn/Tests/Unit/GIDSignInTest.m | 8 +- 4 files changed, 78 insertions(+), 71 deletions(-) diff --git a/GoogleSignIn/Sources/GIDSignInPreferences.m b/GoogleSignIn/Sources/GIDSignInPreferences.m index 256e6a48..bebe6cb0 100644 --- a/GoogleSignIn/Sources/GIDSignInPreferences.m +++ b/GoogleSignIn/Sources/GIDSignInPreferences.m @@ -62,7 +62,8 @@ allowedSet = [NSCharacterSet characterSetWithRange:NSMakeRange(0x20, 0x5F)]; }); - // DROP CHECK FIRST: if it contains any character outside the printable ASCII range, drop it. + // The drop check happens before truncation: if the original string contains any character + // outside the printable ASCII range, we drop the entire value. if ([candidate rangeOfCharacterFromSet:[allowedSet invertedSet]].location != NSNotFound) { return nil; } @@ -72,7 +73,7 @@ return nil; } - // TRUNCATE SECOND: A surviving string longer than 100 characters is truncated to 100. + // A surviving string longer than 100 characters is truncated to 100. if (candidate.length > 100) { // Truncating with -substringToIndex:100 is safe here precisely because the drop check has // already guaranteed every character is single-unit ASCII, so there is no risk of splitting diff --git a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m index 8cd60e76..1afa6de7 100644 --- a/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m +++ b/GoogleSignIn/Tests/Unit/GIDGoogleUserTest.m @@ -536,9 +536,9 @@ - (void)testWrapperIdentifier_AbsentOnRefreshRequestWhenUnset { [self waitForExpectationsWithTimeout:1 handler:nil]; } -- (void)testWrapperIdentifier_AbsentOnRefreshRequestWhenInvalid { - // Assert that attempting to set an invalid identifier throws NSInternalInconsistencyException. - XCTAssertThrowsSpecificNamed(GIDSignIn.sharedInstance.wrapperIdentifier = @"Fire Base!", +- (void)testWrapperIdentifier_AbsentOnRefreshRequestWhenDropped { + // Assert that attempting to set a dropped identifier throws NSInternalInconsistencyException. + XCTAssertThrowsSpecificNamed(GIDSignIn.sharedInstance.wrapperIdentifier = @"firebasé", NSException, NSInternalInconsistencyException); // The rejection leaves the store nil. diff --git a/GoogleSignIn/Tests/Unit/GIDSignInPreferencesTest.m b/GoogleSignIn/Tests/Unit/GIDSignInPreferencesTest.m index c141e207..260a4d92 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInPreferencesTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInPreferencesTest.m @@ -74,113 +74,119 @@ - (void)testWrapperIdentifier_AcceptsDigits { } - (void)testWrapperIdentifier_AcceptsMaximumLength { - // Test that a 32-character valid identifier is accepted and not truncated. - NSString *maxLength = @"abcdefghijklmnopqrstuvwxyz123456"; // 32 chars + // Test that a 100-character valid identifier is accepted and not truncated. + NSString *maxLength = [@"a" stringByPaddingToLength:100 withString:@"a" startingAtIndex:0]; [GIDSignInPreferences setWrapperIdentifier:maxLength]; - XCTAssertEqual([GIDSignInPreferences wrapperIdentifier].length, (NSUInteger)32); + XCTAssertEqual([GIDSignInPreferences wrapperIdentifier].length, (NSUInteger)100); XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], maxLength); } -- (void)testWrapperIdentifier_RejectsUppercase { - // Test that uppercase characters cause a rejection. - XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"Firebase"], +- (void)testWrapperIdentifier_AcceptsMixedCaseSpacesAndPunctuation { + // Test that mixed case, spaces and punctuation are accepted. + NSString *value = @"React Native SDK (v2.0)"; + [GIDSignInPreferences setWrapperIdentifier:value]; + XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], value); +} + +- (void)testWrapperIdentifier_DropsEmptyString { + // Test that an empty string is dropped. + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@""], NSException, NSInternalInconsistencyException); XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); } -- (void)testWrapperIdentifier_RejectsWhitespace { - // Test that leading/trailing or internal whitespace cause a rejection. - XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@" firebase "], +- (void)testWrapperIdentifier_FirstValidWriteWins { + // Test that the first valid write is persistent and subsequent differing writes are rejected. + [GIDSignInPreferences setWrapperIdentifier:@"first"]; + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"second"], NSException, NSInternalInconsistencyException); - XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); + XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], @"first"); +} +- (void)testWrapperIdentifier_RepeatedIdenticalWriteIsAccepted { + // Test that writing the same valid value again does not throw or change the state. + [GIDSignInPreferences setWrapperIdentifier:@"firebase"]; + XCTAssertNoThrow([GIDSignInPreferences setWrapperIdentifier:@"firebase"]); + XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], @"firebase"); +} + +- (void)testWrapperIdentifier_NilResets { + // Test that passing nil resets the store, allowing a new first-write. + [GIDSignInPreferences setWrapperIdentifier:@"firebase"]; [GIDSignInPreferences setWrapperIdentifier:nil]; - XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"fire base"], - NSException, - NSInternalInconsistencyException); XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); + + [GIDSignInPreferences setWrapperIdentifier:@"second"]; + XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], @"second"); } -- (void)testWrapperIdentifier_RejectsPunctuation { - // Test that disallowed punctuation causes a rejection. - XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"my-sdk_1.2~x"], +- (void)testWrapperIdentifier_DroppedWriteLeavesPreviousValue { + // Test that a dropped write does not clear or change a previously set valid value. + [GIDSignInPreferences setWrapperIdentifier:@"firebase"]; + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"firebasé"], NSException, NSInternalInconsistencyException); - XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); + XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], @"firebase"); +} - [GIDSignInPreferences setWrapperIdentifier:nil]; - XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"fire!base"], - NSException, - NSInternalInconsistencyException); - XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); +- (void)testWrapperIdentifier_TruncatesOverLongValue { + // Test that a legal string longer than 100 characters is truncated to its first 100 characters. + NSString *overLong = [@"a" stringByPaddingToLength:150 withString:@"a" startingAtIndex:0]; + XCTAssertNoThrow([GIDSignInPreferences setWrapperIdentifier:overLong]); + XCTAssertEqual([GIDSignInPreferences wrapperIdentifier].length, (NSUInteger)100); + XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], [overLong substringToIndex:100]); } -- (void)testWrapperIdentifier_RejectsEmpty { - // Test that an empty string is rejected. - XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@""], +- (void)testWrapperIdentifier_DropsNonASCII { + // Test that a value containing a non-ASCII character throws and leaves the store nil. + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"firebasé"], NSException, NSInternalInconsistencyException); XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); } -- (void)testWrapperIdentifier_RejectsOverLength { - // Test that a 33-character identifier is rejected. - NSString *overLength = @"abcdefghijklmnopqrstuvwxyz1234567"; // 33 chars - XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:overLength], +- (void)testWrapperIdentifier_DropsControlCharacters { + // Test that values containing ASCII control characters throw and leave the store nil. + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"fire\nbase"], NSException, NSInternalInconsistencyException); XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); -} -- (void)testWrapperIdentifier_RejectsLeadingOrTrailingHyphen { - // Test that leading or trailing hyphens cause a rejection. - XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"-sdk"], + [GIDSignInPreferences setWrapperIdentifier:nil]; + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"fire\tbase"], NSException, NSInternalInconsistencyException); XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); [GIDSignInPreferences setWrapperIdentifier:nil]; - XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"sdk-"], + NSString *del = [NSString stringWithFormat:@"fire%Cbase", (unichar)0x7F]; + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:del], NSException, NSInternalInconsistencyException); XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); } -- (void)testWrapperIdentifier_FirstValidWriteWins { - // Test that the first valid write is persistent and subsequent differing writes are rejected. - [GIDSignInPreferences setWrapperIdentifier:@"first"]; - XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"second"], +- (void)testWrapperIdentifier_DropsWhenDisallowedCharacterIsPastTruncationPoint { + // Test that the drop check deliberately runs on the untruncated string so a payload hidden + // past the truncation point cannot survive. + NSString *prefix = [@"a" stringByPaddingToLength:120 withString:@"a" startingAtIndex:0]; + NSString *overLong = [prefix stringByReplacingCharactersInRange:NSMakeRange(110, 1) + withString:@"é"]; + XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:overLong], NSException, NSInternalInconsistencyException); - XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], @"first"); -} - -- (void)testWrapperIdentifier_RepeatedIdenticalWriteIsAccepted { - // Test that writing the same valid value again does not throw or change the state. - [GIDSignInPreferences setWrapperIdentifier:@"firebase"]; - XCTAssertNoThrow([GIDSignInPreferences setWrapperIdentifier:@"firebase"]); - XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], @"firebase"); -} - -- (void)testWrapperIdentifier_NilResets { - // Test that passing nil resets the store, allowing a new first-write. - [GIDSignInPreferences setWrapperIdentifier:@"firebase"]; - [GIDSignInPreferences setWrapperIdentifier:nil]; XCTAssertNil([GIDSignInPreferences wrapperIdentifier]); - - [GIDSignInPreferences setWrapperIdentifier:@"second"]; - XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], @"second"); } -- (void)testWrapperIdentifier_RejectedWriteLeavesPreviousValue { - // Test that a rejected write does not clear or change a previously set valid value. - [GIDSignInPreferences setWrapperIdentifier:@"firebase"]; - XCTAssertThrowsSpecificNamed([GIDSignInPreferences setWrapperIdentifier:@"Invalid!"], - NSException, - NSInternalInconsistencyException); - XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], @"firebase"); +- (void)testWrapperIdentifier_WriteOnceComparesSanitizedValue { + // Test that the write-once check compares the sanitized value, allowing a repeated + // write of a value that truncates to the same result. + NSString *overLong = [@"a" stringByPaddingToLength:150 withString:@"a" startingAtIndex:0]; + [GIDSignInPreferences setWrapperIdentifier:overLong]; + XCTAssertNoThrow([GIDSignInPreferences setWrapperIdentifier:overLong]); + XCTAssertEqualObjects([GIDSignInPreferences wrapperIdentifier], [overLong substringToIndex:100]); } @end diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index 60d332b9..2465399a 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -1217,12 +1217,12 @@ - (void)testWrapperIdentifier_AbsentFromAuthorizationRequestWhenUnset { "when unset."); } -- (void)testWrapperIdentifier_InvalidValueIsIgnored { - XCTAssertThrowsSpecificNamed(GIDSignIn.sharedInstance.wrapperIdentifier = @"Fire Base!", +- (void)testWrapperIdentifier_DroppedValueIsIgnored { + XCTAssertThrowsSpecificNamed(GIDSignIn.sharedInstance.wrapperIdentifier = @"firebasé", NSException, NSInternalInconsistencyException, - @"Setting an invalid wrapper identifier should throw."); + @"Setting a dropped wrapper identifier should throw."); XCTAssertNil(GIDSignIn.sharedInstance.wrapperIdentifier, - @"The wrapper identifier should be nil after an invalid assignment."); + @"The wrapper identifier should be nil after a dropped assignment."); } - (void)testWrapperIdentifier_PresentOnRevokeURL { From b15b891e29e5cd4bf122a62368b98550fc33f30f Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:29:47 -0700 Subject: [PATCH 17/17] g-orchestrated: Format GIDSignInPreferences docs in the house style Header declarations use `///` with a summary line, a blank `///` line, detail, and `@param` tags, matching GIDSignIn_Private.h. Identifiers and literals are backticked. --- GoogleSignIn/Sources/GIDSignInPreferences.h | 26 ++++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/GoogleSignIn/Sources/GIDSignInPreferences.h b/GoogleSignIn/Sources/GIDSignInPreferences.h index 1b2ea949..02ad2d4b 100644 --- a/GoogleSignIn/Sources/GIDSignInPreferences.h +++ b/GoogleSignIn/Sources/GIDSignInPreferences.h @@ -24,23 +24,31 @@ extern NSString *const kSDKWrapperLoggingParameter; @interface GIDSignInPreferences : NSObject -// Returns the current Google Sign-In SDK version. +/// Returns the current Google Sign-In SDK version. + (NSString *)sdkVersion; -// Returns the current Apple execution environment (e.g. ios, macos). +/// Returns the current Apple execution environment, such as `ios` or `macos`. + (NSString *)environment; -// Returns the current identifier, or nil if none is set. +/// Returns the current SDK wrapper identifier, or `nil` if none is set. + (nullable NSString *)wrapperIdentifier; -// Sets the SDK wrapper identifier. Values may be up to 100 printable ASCII characters; longer -// values are truncated to 100; a value containing any non-ASCII or control character, or an empty -// string, is dropped entirely (and asserts in debug builds); the first accepted write wins and -// later differing writes are ignored; nil resets; the method is thread-safe. +/// Sets the SDK wrapper identifier. +/// +/// A value may be up to 100 printable ASCII characters; a longer value is truncated to its first +/// 100 characters. A value that is empty, or that contains any non-ASCII or ASCII control +/// character, is dropped entirely and asserts in debug builds. +/// +/// The first accepted write wins; later differing writes are ignored. This method is thread-safe. +/// +/// @param wrapperIdentifier The identifier to report, or `nil` to reset it. + (void)setWrapperIdentifier:(nullable NSString *)wrapperIdentifier; -// Populates the standard logging parameters (gpsdk, gidenv, and gidwrapper when set) on the -// supplied dictionary. +/// Adds the standard logging parameters to the supplied dictionary. +/// +/// The parameters are `gpsdk`, `gidenv`, and, when a wrapper identifier is set, `gidwrapper`. +/// +/// @param params The dictionary to add the logging parameters to. + (void)addLoggingParameters:(NSMutableDictionary *)params; + (NSString *)googleAuthorizationServer;