Remove migration from legacy keychain (#5237)

Author: charlotteliang

Example/InstanceID/Tests/FIRInstanceIDCheckinStoreTest.m

@@ -191,62 +191,6 @@ - (void)testCheckinSaveSuccess {
   [self waitForExpectationsWithTimeout:kExpectationTimeout handler:nil];
 }
 
-// Write fake checkin data to legacy location, then test if migration worked.
-- (void)testCheckinMigrationMovesToNewLocationInKeychain {
-  XCTestExpectation *checkinMigrationExpectation =
-      [self expectationWithDescription:@"checkin migration should move to the new location"];
-  // Create checkin store class.
-  FIRInstanceIDBackupExcludedPlist *checkinPlist =
-      [[FIRInstanceIDBackupExcludedPlist alloc] initWithFileName:kFakeCheckinPlistName
-                                                    subDirectory:kSubDirectoryName];
-
-  FIRInstanceIDFakeKeychain *fakeKeychain = [[FIRInstanceIDFakeKeychain alloc] init];
-  FIRInstanceIDFakeKeychain *weakKeychain = fakeKeychain;
-
-  // Create fake checkin preferences object.
-  FIRInstanceIDCheckinPreferences *preferences =
-      [[FIRInstanceIDCheckinPreferences alloc] initWithDeviceID:kAuthID secretToken:kSecret];
-  [preferences updateWithCheckinPlistContents:[[self class] newCheckinPlistPreferences]];
-
-  // Write checkin into legacy location in Fake keychain.
-  NSString *checkinKeychainContent = [preferences checkinKeychainContent];
-  NSData *data = [checkinKeychainContent dataUsingEncoding:NSUTF8StringEncoding];
-  [fakeKeychain setData:data
-             forService:kFIRInstanceIDLegacyCheckinKeychainService
-                account:kFIRInstanceIDLegacyCheckinKeychainAccount
-                handler:^(NSError *error) {
-                  XCTAssertNil(error);
-                  // Check that we saved it correctly to the legacy location.
-                  NSData *dataInLegacyLocation =
-                      [weakKeychain dataForService:kFIRInstanceIDLegacyCheckinKeychainService
-                                           account:kFIRInstanceIDLegacyCheckinKeychainAccount];
-                  XCTAssertNotNil(dataInLegacyLocation);
-
-                  FIRInstanceIDCheckinStore *checkinStore =
-                      [[FIRInstanceIDCheckinStore alloc] initWithCheckinPlist:checkinPlist
-                                                                     keychain:weakKeychain];
-                  // Perform migration.
-                  [checkinStore migrateCheckinItemIfNeeded];
-
-                  // Ensure the item is no longer in the old location.
-                  dataInLegacyLocation =
-                      [weakKeychain dataForService:kFIRInstanceIDLegacyCheckinKeychainService
-                                           account:kFIRInstanceIDLegacyCheckinKeychainAccount];
-                  XCTAssertNil(dataInLegacyLocation);
-                  // Check that it exists in the new location.
-                  NSData *dataInMigratedLocation =
-                      [weakKeychain dataForService:kFIRInstanceIDCheckinKeychainService
-                                           account:checkinStore.bundleIdentifierForKeychainAccount];
-                  XCTAssertNotNil(dataInMigratedLocation);
-                  // Ensure that the data is the same as what we originally saved.
-                  XCTAssertEqualObjects(dataInMigratedLocation, data);
-
-                  [checkinMigrationExpectation fulfill];
-                }];
-
-  [self waitForExpectationsWithTimeout:kExpectationTimeout handler:nil];
-}
-
 #pragma mark - Private Helpers
 
 - (BOOL)savePreferencesToPlist:(NSDictionary *)preferences {

Example/InstanceID/Tests/FIRInstanceIDStoreTest.m

@@ -229,20 +229,6 @@ - (void)testResetCredentialsWithFreshInstall {
   OCMVerifyAll(_mockCheckinStore);
 }
 
-- (void)testResetCredentialsWithoutFreshInstall {
-  FIRInstanceIDCheckinPreferences *checkinPreferences =
-      [[FIRInstanceIDCheckinPreferences alloc] initWithDeviceID:kAuthID secretToken:kSecret];
-  // Expect migration happens if it's not a fresh install.
-  [[_mockCheckinStore expect] migrateCheckinItemIfNeeded];
-  // Always setting up stub after expect.
-  OCMStub([_mockCheckinStore cachedCheckinPreferences]).andReturn(checkinPreferences);
-  // Mock plist exists, meaning this is not a fresh install.
-  OCMStub([_mockCheckinStore hasCheckinPlist]).andReturn(YES);
-
-  [_mockInstanceIDStore resetCredentialsIfNeeded];
-  OCMVerifyAll(_mockCheckinStore);
-}
-
 - (void)testResetCredentialsWithNoCachedCheckin {
   id niceMockCheckinStore = [OCMockObject niceMockForClass:[FIRInstanceIDCheckinStore class]];
   [[niceMockCheckinStore reject]

Firebase/InstanceID/CHANGELOG.md

@@ -1,3 +1,7 @@
+# 2020-03 -- 4.3.3
+- [fixed] Fixed provisioning profile location for catalyst. (#5048)
+- [changed] Remove obsolete logic to improve performance and reduce keychain operations. (#5211, #5237)
+
 # 2020-02 -- 4.3.2
 - [changed] Removed unused files (#4881).
 

Firebase/InstanceID/FIRInstanceIDCheckinStore.h

@@ -22,8 +22,6 @@
 
 // These values exposed for testing
 extern NSString *const kFIRInstanceIDCheckinKeychainService;
-extern NSString *const kFIRInstanceIDLegacyCheckinKeychainAccount;
-extern NSString *const kFIRInstanceIDLegacyCheckinKeychainService;
 
 /**
  *  Checkin preferences backing store.
@@ -95,14 +93,4 @@ extern NSString *const kFIRInstanceIDLegacyCheckinKeychainService;
  */
 - (FIRInstanceIDCheckinPreferences *)cachedCheckinPreferences;
 
-/**
- * Migrate the checkin item from old service/account to the new one.
- * The new account is dynamic as it uses bundle ID.
- * This is to ensure checkin is not shared across apps, but still the same
- * if app has used GCM before.
- * This call should only happen once.
- *
- */
-- (void)migrateCheckinItemIfNeeded;
-
 @end

Firebase/InstanceID/FIRInstanceIDCheckinStore.m

@@ -29,8 +29,6 @@
 static NSString *const kFIRInstanceIDCheckinKeychainGeneric = @"com.google.iid";
 
 NSString *const kFIRInstanceIDCheckinKeychainService = @"com.google.iid.checkin";
-NSString *const kFIRInstanceIDLegacyCheckinKeychainAccount = @"com.google.iid.checkin-account";
-NSString *const kFIRInstanceIDLegacyCheckinKeychainService = @"com.google.iid.checkin-service";
 
 @interface FIRInstanceIDCheckinStore ()
 
@@ -152,19 +150,11 @@ - (void)removeCheckinPreferencesWithHandler:(void (^)(NSError *error))handler {
   FIRInstanceIDLoggerDebug(kFIRInstanceIDMessageCodeCheckinStoreCheckinPlistDeleted,
                            @"Deleted checkin plist file.");
   // Remove deviceID and secret from Keychain
-  [self.keychain
-      removeItemsMatchingService:kFIRInstanceIDCheckinKeychainService
-                         account:self.bundleIdentifierForKeychainAccount
-                         handler:^(NSError *error) {
-                           // Try to remove from old location as well because migration
-                           // is no longer needed. Consider this is either a fresh install
-                           // or an identity wipe.
-                           [self.keychain
-                               removeItemsMatchingService:kFIRInstanceIDLegacyCheckinKeychainService
-                                                  account:kFIRInstanceIDLegacyCheckinKeychainAccount
-                                                  handler:nil];
-                           handler(error);
-                         }];
+  [self.keychain removeItemsMatchingService:kFIRInstanceIDCheckinKeychainService
+                                    account:self.bundleIdentifierForKeychainAccount
+                                    handler:^(NSError *error) {
+                                      handler(error);
+                                    }];
 }
 
 - (FIRInstanceIDCheckinPreferences *)cachedCheckinPreferences {
@@ -201,23 +191,4 @@ - (FIRInstanceIDCheckinPreferences *)cachedCheckinPreferences {
   return checkinPreferences;
 }
 
-- (void)migrateCheckinItemIfNeeded {
-  // Check for checkin in the old location, using the legacy keys
-  // Query the keychain for deviceID and secret
-  NSData *dataInOldLocation =
-      [self.keychain dataForService:kFIRInstanceIDLegacyCheckinKeychainService
-                            account:kFIRInstanceIDLegacyCheckinKeychainAccount];
-  if (dataInOldLocation) {
-    // Save to new location
-    [self.keychain setData:dataInOldLocation
-                forService:kFIRInstanceIDCheckinKeychainService
-                   account:self.bundleIdentifierForKeychainAccount
-                   handler:nil];
-    // Remove from old location
-    [self.keychain removeItemsMatchingService:kFIRInstanceIDLegacyCheckinKeychainService
-                                      account:kFIRInstanceIDLegacyCheckinKeychainAccount
-                                      handler:nil];
-  }
-}
-
 @end

Firebase/InstanceID/FIRInstanceIDStore.m

@@ -140,13 +140,11 @@ - (void)resetCredentialsIfNeeded {
   BOOL checkinPlistExists = [self.checkinStore hasCheckinPlist];
   // Checkin info existed in backup excluded plist. Should not be a fresh install.
   if (checkinPlistExists) {
-    // FCM user can still have the old version of checkin, migration should only happen once.
-    [self.checkinStore migrateCheckinItemIfNeeded];
     return;
   }
 
-  // reset checkin in keychain if a fresh install.
-  // set the old checkin preferences to unregister pre-registered tokens
+  // Resets checkin in keychain if a fresh install.
+  // Keychain can still exist even if app is uninstalled.
   FIRInstanceIDCheckinPreferences *oldCheckinPreferences =
       [self.checkinStore cachedCheckinPreferences];