Fix race condition where checkin is not reset properly when app first installed (#8513)

Author: charlotteliang

FirebaseMessaging/Apps/Sample/Sample.xcodeproj/project.pbxproj

@@ -3,12 +3,13 @@
 	archiveVersion = 1;
 	classes = {
 	};
-	objectVersion = 50;
+	objectVersion = 51;
 	objects = {
 
 /* Begin PBXBuildFile section */
 		5125CCA22437F471006CA5D0 /* Sample.xcdatamodeld in Sources */ = {isa = PBXBuildFile; fileRef = 5125CCA02437F471006CA5D0 /* Sample.xcdatamodeld */; };
 		5125CCA92437F472006CA5D0 /* Preview Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 5125CCA82437F472006CA5D0 /* Preview Assets.xcassets */; };
+		515617FB26C20330006AEA6E /* GoogleService-Info.plist in Resources */ = {isa = PBXBuildFile; fileRef = 515617FA26C20330006AEA6E /* GoogleService-Info.plist */; };
 		51944B6E258B12E300297021 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 51944B6D258B12E300297021 /* Assets.xcassets */; };
 		51A1F3D92588405A0025932B /* Identity.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51A1F3D52588405A0025932B /* Identity.swift */; };
 		51A1F3DA2588405A0025932B /* UserSettings.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51A1F3D62588405A0025932B /* UserSettings.swift */; };
@@ -26,6 +27,7 @@
 		5125CCA82437F472006CA5D0 /* Preview Assets.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = "Preview Assets.xcassets"; sourceTree = "<group>"; };
 		5125CCAD2437F472006CA5D0 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
 		5125CCB32437F9A9006CA5D0 /* Sample.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = Sample.entitlements; sourceTree = "<group>"; };
+		515617FA26C20330006AEA6E /* GoogleService-Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; name = "GoogleService-Info.plist"; path = "../../Shared/GoogleService-Info.plist"; sourceTree = "<group>"; };
 		51944B6D258B12E300297021 /* Assets.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; name = Assets.xcassets; path = ../../Shared/Assets.xcassets; sourceTree = "<group>"; };
 		51A1F3D52588405A0025932B /* Identity.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = Identity.swift; path = ../../Shared/Identity.swift; sourceTree = "<group>"; };
 		51A1F3D62588405A0025932B /* UserSettings.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = UserSettings.swift; path = ../../Shared/UserSettings.swift; sourceTree = "<group>"; };
@@ -75,6 +77,7 @@
 		5125CC9B2437F471006CA5D0 /* Sample */ = {
 			isa = PBXGroup;
 			children = (
+				515617FA26C20330006AEA6E /* GoogleService-Info.plist */,
 				51C24C732589614800236F25 /* logo.png */,
 				51C24C5025894EDB00236F25 /* LaunchScreen.storyboard */,
 				51A1F3DE2588406A0025932B /* AppDelegate.swift */,
@@ -161,6 +164,7 @@
 			files = (
 				51C24C742589614800236F25 /* logo.png in Resources */,
 				5125CCA92437F472006CA5D0 /* Preview Assets.xcassets in Resources */,
+				515617FB26C20330006AEA6E /* GoogleService-Info.plist in Resources */,
 				51C24C5225894EDB00236F25 /* LaunchScreen.storyboard in Resources */,
 				51944B6E258B12E300297021 /* Assets.xcassets in Resources */,
 			);

FirebaseMessaging/CHANGELOG.md

@@ -1,6 +1,7 @@
 # 2021-08 -- v8.6.0
 - [changed] Removed iOS version check from `FIRMessagingExtensionHelper.h` (#8492).
 - [added] Added new API `FIRMessagingExtensionHelper exportDeliveryMetricsToBigQuery` that allows developers to enable notification delivery metrics that exports to BigQuery. (#6181)
+- [fixed] Fixed an issue that delete token no longer works. (#8491)
 
 # 2021-06 -- v8.2.0
 - [fixed] Fixed an issue that local scheduled notification is not set correctly due to sound type. (#8172)

FirebaseMessaging/Sources/Token/FIRMessagingAuthService.h

@@ -20,7 +20,6 @@
 NS_ASSUME_NONNULL_BEGIN
 
 @class FIRMessagingCheckinPreferences;
-@class FIRMessagingCheckinStore;
 /**
  *  @related FIRInstanceIDCheckinService
  *
@@ -42,14 +41,10 @@ typedef void (^FIRMessagingDeviceCheckinCompletion)(
  */
 @interface FIRMessagingAuthService : NSObject
 
-/**
- *  Initializes the auth service given a store (which provides the local caching of checkin info).
- *  This initializer will create its own instance of FIRMessagingCheckinService.
- */
-- (instancetype)initWithCheckinStore:(FIRMessagingCheckinStore *)store;
-
 #pragma mark - Checkin Service
 
+- (BOOL)hasCheckinPlist;
+
 /**
  *  Checks if the current deviceID and secret are valid or not.
  *

FirebaseMessaging/Sources/Token/FIRMessagingAuthService.m

@@ -53,10 +53,10 @@ @interface FIRMessagingAuthService ()
 
 @implementation FIRMessagingAuthService
 
-- (instancetype)initWithCheckinStore:(FIRMessagingCheckinStore *)checkinStore {
+- (instancetype)init {
   self = [super init];
   if (self) {
-    _checkinStore = checkinStore;
+    _checkinStore = [[FIRMessagingCheckinStore alloc] init];
     _checkinPreferences = [_checkinStore cachedCheckinPreferences];
     _checkinService = [[FIRMessagingCheckinService alloc] init];
     _checkinHandlers = [[NSMutableArray alloc] init];
@@ -70,6 +70,10 @@ - (void)dealloc {
 
 #pragma mark - Schedule Checkin
 
+- (BOOL)hasCheckinPlist {
+  return [_checkinStore hasCheckinPlist];
+}
+
 - (void)scheduleCheckin:(BOOL)immediately {
   // Checkin is still valid, so a remote checkin is not required.
   if ([self.checkinPreferences hasValidCheckinInfo]) {

FirebaseMessaging/Sources/Token/FIRMessagingTokenManager.m

@@ -51,8 +51,7 @@ - (instancetype)init {
   self = [super init];
   if (self) {
     _tokenStore = [[FIRMessagingTokenStore alloc] init];
-    _checkinStore = [[FIRMessagingCheckinStore alloc] init];
-    _authService = [[FIRMessagingAuthService alloc] initWithCheckinStore:_checkinStore];
+    _authService = [[FIRMessagingAuthService alloc] init];
     [self resetCredentialsIfNeeded];
     [self configureTokenOperations];
     _installations = [FIRInstallations installations];
@@ -458,41 +457,40 @@ - (void)deleteWithHandler:(void (^)(NSError *))handler {
  *  since the Keychain items are marked with `*BackupThisDeviceOnly`.
  */
 - (void)resetCredentialsIfNeeded {
-  BOOL checkinPlistExists = [_checkinStore hasCheckinPlist];
+  BOOL checkinPlistExists = [_authService hasCheckinPlist];
   // Checkin info existed in backup excluded plist. Should not be a fresh install.
   if (checkinPlistExists) {
     return;
   }
-
-  // Resets checkin in keychain if a fresh install.
   // Keychain can still exist even if app is uninstalled.
-  FIRMessagingCheckinPreferences *oldCheckinPreferences = [_checkinStore cachedCheckinPreferences];
-
-  if (oldCheckinPreferences) {
-    [_checkinStore removeCheckinPreferencesWithHandler:^(NSError *error) {
-      if (!error) {
-        FIRMessagingLoggerDebug(
-            kFIRMessagingMessageCodeStore002,
-            @"Removed cached checkin preferences from Keychain because this is a fresh install.");
-      } else {
-        FIRMessagingLoggerError(
-            kFIRMessagingMessageCodeStore003,
-            @"Couldn't remove cached checkin preferences for a fresh install. Error: %@", error);
-      }
-      if (oldCheckinPreferences.deviceID.length && oldCheckinPreferences.secretToken.length) {
-        FIRMessagingLoggerDebug(kFIRMessagingMessageCodeStore006,
-                                @"App reset detected. Will delete server registrations.");
-        // We don't really need to delete old FCM tokens created via IID auth tokens since
-        // those tokens are already hashed by APNS token as the has so creating a new
-        // token should automatically delete the old-token.
-        [self didDeleteFCMScopedTokensForCheckin:oldCheckinPreferences];
-      } else {
-        FIRMessagingLoggerDebug(kFIRMessagingMessageCodeStore009,
-                                @"App reset detected but no valid checkin auth preferences found."
-                                @" Will not delete server registrations.");
-      }
-    }];
+  FIRMessagingCheckinPreferences *oldCheckinPreferences = _authService.checkinPreferences;
+
+  if (!oldCheckinPreferences) {
+    FIRMessagingLoggerDebug(kFIRMessagingMessageCodeStore009,
+                            @"App reset detected but no valid checkin auth preferences found."
+                            @" Will not delete server token registrations.");
+    return;
   }
+  [_authService resetCheckinWithHandler:^(NSError *_Nonnull error) {
+    if (!error) {
+      FIRMessagingLoggerDebug(
+          kFIRMessagingMessageCodeStore002,
+          @"Removed cached checkin preferences from Keychain because this is a fresh install.");
+    } else {
+      FIRMessagingLoggerError(
+          kFIRMessagingMessageCodeStore003,
+          @"Couldn't remove cached checkin preferences for a fresh install. Error: %@", error);
+    }
+
+    if (oldCheckinPreferences.deviceID.length && oldCheckinPreferences.secretToken.length) {
+      FIRMessagingLoggerDebug(kFIRMessagingMessageCodeStore006,
+                              @"Resetting old checkin and deleting server token registrations.");
+      // We don't really need to delete old FCM tokens created via IID auth tokens since
+      // those tokens are already hashed by APNS token as the has so creating a new
+      // token should automatically delete the old-token.
+      [self didDeleteFCMScopedTokensForCheckin:oldCheckinPreferences];
+    }
+  }];
 }
 
 - (void)didDeleteFCMScopedTokensForCheckin:(FIRMessagingCheckinPreferences *)checkin {

FirebaseMessaging/Tests/UnitTests/FIRMessagingAuthServiceTest.m

@@ -37,6 +37,7 @@ @interface FIRMessagingAuthService (ExposedForTest)
 @property(nonatomic, readonly, strong)
     NSMutableArray<FIRMessagingDeviceCheckinCompletion> *checkinHandlers;
 @property(nonatomic, readwrite, strong) FIRMessagingCheckinService *checkinService;
+@property(nonatomic, readwrite, strong) FIRMessagingCheckinStore *checkinStore;
 @end
 
 @interface FIRMessagingAuthServiceTest : XCTestCase
@@ -53,8 +54,9 @@ @implementation FIRMessagingAuthServiceTest
 
 - (void)setUp {
   [super setUp];
-  _mockStore = OCMClassMock([FIRMessagingCheckinStore class]);
-  _authService = [[FIRMessagingAuthService alloc] initWithCheckinStore:_mockStore];
+  _authService = [[FIRMessagingAuthService alloc] init];
+  _mockStore = OCMPartialMock(_authService.checkinStore);
+
   _mockCheckinService = OCMPartialMock(_authService.checkinService);
   // The tests here are to focus on checkin interval not locale change, so always set locale as
   // non-changed.

FirebaseMessaging/Tests/UnitTests/FIRMessagingTokenManagerTest.m

@@ -17,6 +17,7 @@
 #import <OCMock/OCMock.h>
 #import <XCTest/XCTest.h>
 
+#import "FirebaseMessaging/Sources/Token/FIRMessagingAuthService.h"
 #import "FirebaseMessaging/Sources/Token/FIRMessagingCheckinPreferences.h"
 #import "FirebaseMessaging/Sources/Token/FIRMessagingCheckinStore.h"
 #import "FirebaseMessaging/Sources/Token/FIRMessagingTokenManager.h"
@@ -30,19 +31,24 @@ @interface FIRMessaging (ExposedForTest)
 
 @interface FIRMessagingTokenManager (ExposedForTest)
 
-@property(nonatomic, readwrite, strong) FIRMessagingCheckinStore *checkinStore;
-
 - (void)resetCredentialsIfNeeded;
 
 @end
 
+@interface FIRMessagingAuthService (ExposedForTest)
+
+@property(nonatomic, readwrite, strong) FIRMessagingCheckinStore *checkinStore;
+
+@end
+
 @interface FIRMessagingTokenManagerTest : XCTestCase {
   FIRMessaging *_messaging;
   id _mockMessaging;
   id _mockPubSub;
   id _mockTokenManager;
   id _mockInstallations;
   id _mockCheckinStore;
+  id _mockAuthService;
   FIRMessagingTestUtilities *_testUtil;
 }
 
@@ -59,10 +65,13 @@ - (void)setUp {
   _mockMessaging = _testUtil.mockMessaging;
   _messaging = _testUtil.messaging;
   _mockTokenManager = _testUtil.mockTokenManager;
-  _mockCheckinStore = OCMPartialMock(_messaging.tokenManager.checkinStore);
+  _mockAuthService = OCMPartialMock(_messaging.tokenManager.authService);
+  _mockCheckinStore = OCMPartialMock(_messaging.tokenManager.authService.checkinStore);
 }
 
 - (void)tearDown {
+  [_mockCheckinStore stopMocking];
+  [_mockAuthService stopMocking];
   [_testUtil cleanupAfterTest:self];
   _messaging = nil;
   [[[NSUserDefaults alloc] initWithSuiteName:kFIRMessagingDefaultsTestDomain]
@@ -110,30 +119,41 @@ - (void)testTokenChangeMethod {
 }
 
 - (void)testResetCredentialsWithNoCachedCheckin {
-  id niceMockCheckinStore = [OCMockObject niceMockForClass:[FIRMessagingCheckinStore class]];
-  [[niceMockCheckinStore reject]
-      removeCheckinPreferencesWithHandler:[OCMArg invokeBlockWithArgs:[NSNull null], nil]];
+  id completionArg = [OCMArg invokeBlockWithArgs:[NSNull null], nil];
+  OCMReject([_mockCheckinStore removeCheckinPreferencesWithHandler:completionArg]);
+  // Always setting up stub after expect.
+  OCMStub([_mockAuthService checkinPreferences]).andReturn(nil);
+
+  [_messaging.tokenManager resetCredentialsIfNeeded];
+
+  OCMVerifyAll(_mockCheckinStore);
+}
+
+- (void)testResetCredentialsWithoutFreshInstall {
+  id completionArg = [OCMArg invokeBlockWithArgs:[NSNull null], nil];
+  OCMReject([_mockCheckinStore removeCheckinPreferencesWithHandler:completionArg]);
   // Always setting up stub after expect.
-  OCMStub([_mockCheckinStore cachedCheckinPreferences]).andReturn(nil);
+  OCMStub([_mockAuthService hasCheckinPlist]).andReturn(YES);
 
   [_messaging.tokenManager resetCredentialsIfNeeded];
 
-  OCMVerifyAll(niceMockCheckinStore);
+  OCMVerifyAll(_mockCheckinStore);
 }
 
 - (void)testResetCredentialsWithFreshInstall {
   FIRMessagingCheckinPreferences *checkinPreferences =
       [[FIRMessagingCheckinPreferences alloc] initWithDeviceID:@"test-auth-id"
                                                    secretToken:@"test-secret"];
   // Expect checkin is removed if it's a fresh install.
-  [[_mockCheckinStore expect]
-      removeCheckinPreferencesWithHandler:[OCMArg invokeBlockWithArgs:[NSNull null], nil]];
+  id completionArg = [OCMArg invokeBlockWithArgs:[NSNull null], nil];
+  OCMExpect([_mockCheckinStore removeCheckinPreferencesWithHandler:completionArg]);
   // Always setting up stub after expect.
-  OCMStub([_mockCheckinStore cachedCheckinPreferences]).andReturn(checkinPreferences);
+  OCMStub([_mockAuthService checkinPreferences]).andReturn(checkinPreferences);
   // Plist file doesn't exist, meaning this is a fresh install.
   OCMStub([_mockCheckinStore hasCheckinPlist]).andReturn(NO);
 
   [_messaging.tokenManager resetCredentialsIfNeeded];
   OCMVerifyAll(_mockCheckinStore);
 }
+
 @end

FirebaseMessaging/Tests/UnitTests/FIRMessagingTokenOperationsTest.m

@@ -99,8 +99,7 @@ - (void)setUp {
   _checkinService = [[FIRMessagingCheckinService alloc] init];
   _mockCheckinService = OCMPartialMock(_checkinService);
 
-  _authService = [[FIRMessagingAuthService alloc]
-      initWithCheckinStore:[[FIRMessagingCheckinStore alloc] init]];
+  _authService = [[FIRMessagingAuthService alloc] init];
   _instanceID = @"instanceID";
 
   // `FIRMessagingTokenOperation` uses `FIRInstallations` under the hood to get FIS auth token.