Fix migration bugs (#533)

Author: camden-king

GoogleSignIn/Sources/GIDAuthStateMigration/Fake/GIDFakeAuthStateMigration.h

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2025 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#import "GoogleSignIn/Sources/GIDAuthStateMigration/GIDAuthStateMigration.h"
+
+/// A fake |GIDAuthStateMigration| for testing.
+@interface GIDFakeAuthStateMigration : GIDAuthStateMigration
+
+/// Callback that is called when `migrateIfNeededWithTokenURL` is invoked.
+@property (nonatomic, nullable) void (^migrationInvokedCallback)
+  (NSURL * _Nullable tokenURL, NSString * _Nullable callbackPath, BOOL isFreshInstall);
+
+@end

GoogleSignIn/Sources/GIDAuthStateMigration/Fake/GIDFakeAuthStateMigration.m

@@ -0,0 +1,41 @@
+/*
+ * Copyright 2025 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#import "GoogleSignIn/Sources/GIDAuthStateMigration/Fake/GIDFakeAuthStateMigration.h"
+
+NS_ASSUME_NONNULL_BEGIN
+
+@implementation GIDFakeAuthStateMigration
+
+@synthesize migrationInvokedCallback = _migrationInvokedCallback;
+
+- (instancetype)initWithKeychainStore:(GTMKeychainStore *)keychainStore {
+  self = [super initWithKeychainStore:keychainStore];
+  return self;
+}
+
+- (void)migrateIfNeededWithTokenURL:(NSURL *)tokenURL
+                       callbackPath:(NSString *)callbackPath
+                     isFreshInstall:(BOOL)isFreshInstall {
+  if (_migrationInvokedCallback) {
+    _migrationInvokedCallback(tokenURL, callbackPath, isFreshInstall);
+  }
+  return;
+}
+
+@end
+
+NS_ASSUME_NONNULL_END

…leSignIn/Sources/GIDAuthStateMigration.h …thStateMigration/GIDAuthStateMigration.hGoogleSignIn/Sources/GIDAuthStateMigration.h renamed to GoogleSignIn/Sources/GIDAuthStateMigration/GIDAuthStateMigration.h GoogleSignIn/Sources/GIDAuthStateMigration.h renamed to GoogleSignIn/Sources/GIDAuthStateMigration/GIDAuthStateMigration.h

@@ -30,7 +30,6 @@ NS_ASSUME_NONNULL_BEGIN
 /// Perform necessary migrations from legacy auth state storage to most recent GTMAppAuth version.
 - (void)migrateIfNeededWithTokenURL:(NSURL *)tokenURL
                        callbackPath:(NSString *)callbackPath
-                       keychainName:(NSString *)keychainName
                      isFreshInstall:(BOOL)isFreshInstall;
 
 @end

…leSignIn/Sources/GIDAuthStateMigration.m …n/Implementation/GIDAuthStateMigration.mGoogleSignIn/Sources/GIDAuthStateMigration.m renamed to GoogleSignIn/Sources/GIDAuthStateMigration/Implementation/GIDAuthStateMigration.m GoogleSignIn/Sources/GIDAuthStateMigration.m renamed to GoogleSignIn/Sources/GIDAuthStateMigration/Implementation/GIDAuthStateMigration.m

@@ -12,8 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#import "GoogleSignIn/Sources/GIDAuthStateMigration.h"
-
+#import "GoogleSignIn/Sources/GIDAuthStateMigration/GIDAuthStateMigration.h"
 #import "GoogleSignIn/Sources/GIDSignInCallbackSchemes.h"
 
 @import GTMAppAuth;
@@ -65,30 +64,28 @@ - (instancetype)init {
 
 - (void)migrateIfNeededWithTokenURL:(NSURL *)tokenURL
                        callbackPath:(NSString *)callbackPath
-                       keychainName:(NSString *)keychainName
                      isFreshInstall:(BOOL)isFreshInstall {
   // If this is a fresh install, take no action and mark the migration checks as having been
   // performed.
   if (isFreshInstall) {
     NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults];
-#if TARGET_OS_OSX || TARGET_OS_MACCATALYST
+#if TARGET_OS_OSX
     [defaults setBool:YES forKey:kDataProtectedMigrationCheckPerformedKey];
-#elif TARGET_OS_IOS
+#elif TARGET_OS_IOS && !TARGET_OS_MACCATALYST
     [defaults setBool:YES forKey:kGTMAppAuthMigrationCheckPerformedKey];
-#endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST
+#endif // TARGET_OS_OSX
     return;
   }
 
-#if TARGET_OS_OSX || TARGET_OS_MACCATALYST
+#if TARGET_OS_OSX
   [self performDataProtectedMigrationIfNeeded];
-#elif TARGET_OS_IOS
+#elif TARGET_OS_IOS && !TARGET_OS_MACCATALYST
   [self performGIDMigrationIfNeededWithTokenURL:tokenURL
-                                   callbackPath:callbackPath
-                                   keychainName:keychainName];
-#endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST
+                                   callbackPath:callbackPath];
+#endif // TARGET_OS_OSX
 }
 
-#if TARGET_OS_OSX || TARGET_OS_MACCATALYST
+#if TARGET_OS_OSX
 // Migrate from the fileBasedKeychain to dataProtectedKeychain with GTMAppAuth 5.0.
 - (void)performDataProtectedMigrationIfNeeded {
   // See if we've performed the migration check previously.
@@ -107,23 +104,18 @@ - (void)performDataProtectedMigrationIfNeeded {
   if (authSession) {
     NSError *err;
     [self.keychainStore saveAuthSession:authSession error:&err];
-    // If we're unable to save to the keychain, return without marking migration performed.
-    if (err) {
-      return;
-    };
     [keychainStoreLegacy removeAuthSessionWithError:nil];
   }
 
   // Mark the migration check as having been performed.
   [defaults setBool:YES forKey:kDataProtectedMigrationCheckPerformedKey];
 }
 
-#elif TARGET_OS_IOS
+#elif TARGET_OS_IOS && !TARGET_OS_MACCATALYST
 // Migrate from GPPSignIn 1.x or GIDSignIn 1.0 - 4.x to the GTMAppAuth storage introduced in
 // GIDSignIn 5.0.
 - (void)performGIDMigrationIfNeededWithTokenURL:(NSURL *)tokenURL
-                                   callbackPath:(NSString *)callbackPath
-                                   keychainName:(NSString *)keychainName {
+                                   callbackPath:(NSString *)callbackPath {
   // See if we've performed the migration check previously.
   NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults];
   if ([defaults boolForKey:kGTMAppAuthMigrationCheckPerformedKey]) {
@@ -138,10 +130,6 @@ - (void)performGIDMigrationIfNeededWithTokenURL:(NSURL *)tokenURL
   if (authSession) {
     NSError *err;
     [self.keychainStore saveAuthSession:authSession error:&err];
-    // If we're unable to save to the keychain, return without marking migration performed.
-    if (err) {
-      return;
-    };
   }
 
   // Mark the migration check as having been performed.
@@ -246,7 +234,7 @@ + (nullable NSString *)passwordForService:(NSString *)service {
   NSString *password = [[NSString alloc] initWithData:passwordData encoding:NSUTF8StringEncoding];
   return password;
 }
-#endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST
+#endif // TARGET_OS_OSX
 
 @end
 

GoogleSignIn/Sources/GIDSignIn.m

@@ -21,7 +21,7 @@
 #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDProfileData.h"
 #import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignInResult.h"
 
-#import "GoogleSignIn/Sources/GIDAuthStateMigration.h"
+#import "GoogleSignIn/Sources/GIDAuthStateMigration/GIDAuthStateMigration.h"
 #import "GoogleSignIn/Sources/GIDEMMSupport.h"
 #import "GoogleSignIn/Sources/GIDSignInInternalOptions.h"
 #import "GoogleSignIn/Sources/GIDSignInPreferences.h"
@@ -490,15 +490,19 @@ + (GIDSignIn *)sharedInstance {
   dispatch_once(&once, ^{
     GTMKeychainStore *keychainStore =
         [[GTMKeychainStore alloc] initWithItemName:kGTMAppAuthKeychainName];
+    GIDAuthStateMigration *authStateMigrationService =
+        [[GIDAuthStateMigration alloc] initWithKeychainStore:keychainStore];
 #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST
     if (@available(iOS 14.0, *)) {
       GIDAppCheck *appCheck = [GIDAppCheck appCheckUsingAppAttestProvider];
       sharedInstance = [[self alloc] initWithKeychainStore:keychainStore
+                                 authStateMigrationService:authStateMigrationService
                                                   appCheck:appCheck];
     }
 #endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST
     if (!sharedInstance) {
-      sharedInstance = [[self alloc] initWithKeychainStore:keychainStore];
+      sharedInstance = [[self alloc] initWithKeychainStore:keychainStore
+                                 authStateMigrationService:authStateMigrationService];
     }
   });
   return sharedInstance;
@@ -533,7 +537,8 @@ - (void)configureDebugProviderWithAPIKey:(NSString *)APIKey
 
 #pragma mark - Private methods
 
-- (instancetype)initWithKeychainStore:(GTMKeychainStore *)keychainStore {
+- (instancetype)initWithKeychainStore:(GTMKeychainStore *)keychainStore
+            authStateMigrationService:(GIDAuthStateMigration *)authStateMigrationService {
   self = [super init];
   if (self) {
     // Get the bundle of the current executable.
@@ -561,20 +566,19 @@ - (instancetype)initWithKeychainStore:(GTMKeychainStore *)keychainStore {
                              tokenEndpoint:[NSURL URLWithString:tokenEndpointURL]];
     _keychainStore = keychainStore;
     // Perform migration of auth state from old versions of the SDK if needed.
-    GIDAuthStateMigration *migration =
-        [[GIDAuthStateMigration alloc] initWithKeychainStore:_keychainStore];
-    [migration migrateIfNeededWithTokenURL:_appAuthConfiguration.tokenEndpoint
-                              callbackPath:kBrowserCallbackPath
-                              keychainName:kGTMAppAuthKeychainName
-                            isFreshInstall:isFreshInstall];
+    [authStateMigrationService migrateIfNeededWithTokenURL:_appAuthConfiguration.tokenEndpoint
+                                              callbackPath:kBrowserCallbackPath
+                                            isFreshInstall:isFreshInstall];
   }
   return self;
 }
 
 #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST
 - (instancetype)initWithKeychainStore:(GTMKeychainStore *)keychainStore
+            authStateMigrationService:(GIDAuthStateMigration *)authStateMigrationService
                              appCheck:(GIDAppCheck *)appCheck {
-  self = [self initWithKeychainStore:keychainStore];
+  self = [self initWithKeychainStore:keychainStore
+           authStateMigrationService:authStateMigrationService];
   if (self) {
     _appCheck = appCheck;
     _configureAppCheckCalled = NO;

GoogleSignIn/Sources/GIDSignIn_Private.h

@@ -30,6 +30,7 @@ NS_ASSUME_NONNULL_BEGIN
 @class GIDSignInInternalOptions;
 @class GTMKeychainStore;
 @class GIDAppCheck;
+@class GIDAuthStateMigration;
 
 /// Represents a completion block that takes a `GIDSignInResult` on success or an error if the
 /// operation was unsuccessful.
@@ -46,11 +47,13 @@ typedef void (^GIDDisconnectCompletion)(NSError *_Nullable error);
 @property(nonatomic, readwrite, nullable) GIDGoogleUser *currentUser;
 
 /// Private initializer taking a `GTMKeychainStore`.
-- (instancetype)initWithKeychainStore:(GTMKeychainStore *)keychainStore;
+- (instancetype)initWithKeychainStore:(GTMKeychainStore *)keychainStore
+            authStateMigrationService:(GIDAuthStateMigration *)authStateMigrationService;
 
 #if TARGET_OS_IOS && !TARGET_OS_MACCATALYST
 /// Private initializer taking a `GTMKeychainStore` and `GIDAppCheckProvider`.
 - (instancetype)initWithKeychainStore:(GTMKeychainStore *)keychainStore
+            authStateMigrationService:(GIDAuthStateMigration *)authStateMigrationService
                              appCheck:(GIDAppCheck *)appCheck
 API_AVAILABLE(ios(14));
 #endif // TARGET_OS_IOS || !TARGET_OS_MACCATALYST