use secure coding and secure archiving for messaging (#4848)

Author: charlotteliang

Example/Messaging/Tests/FIRMessagingPendingTopicsListTest.m

@@ -20,47 +20,7 @@
 #import "Firebase/Messaging/FIRMessagingDefines.h"
 #import "Firebase/Messaging/FIRMessagingPendingTopicsList.h"
 #import "Firebase/Messaging/FIRMessagingTopicsCommon.h"
-
-typedef void (^MockDelegateSubscriptionHandler)(NSString *topic,
-                                                FIRMessagingTopicAction action,
-                                                FIRMessagingTopicOperationCompletion completion);
-
-/**
- * This object lets us provide a stub delegate where we can customize the behavior by providing
- * blocks. We need to use this instead of stubbing a OCMockProtocol because our delegate methods
- * take primitive values (e.g. action), which is not easy to use from OCMock
- * @see http://stackoverflow.com/a/6332023
- */
-@interface MockPendingTopicsListDelegate: NSObject <FIRMessagingPendingTopicsListDelegate>
-
-@property(nonatomic, assign) BOOL isReady;
-@property(nonatomic, copy) MockDelegateSubscriptionHandler subscriptionHandler;
-@property(nonatomic, copy) void(^updateHandler)(void);
-
-@end
-
-@implementation MockPendingTopicsListDelegate
-
-- (BOOL)pendingTopicsListCanRequestTopicUpdates:(FIRMessagingPendingTopicsList *)list {
-  return self.isReady;
-}
-
-- (void)pendingTopicsList:(FIRMessagingPendingTopicsList *)list
-  requestedUpdateForTopic:(NSString *)topic
-                   action:(FIRMessagingTopicAction)action
-               completion:(FIRMessagingTopicOperationCompletion)completion {
-  if (self.subscriptionHandler) {
-    self.subscriptionHandler(topic, action, completion);
-  }
-}
-
-- (void)pendingTopicsListDidUpdate:(FIRMessagingPendingTopicsList *)list {
-  if (self.updateHandler) {
-    self.updateHandler();
-  }
-}
-
-@end
+#import "Example/Messaging/Tests/FIRMessagingTestUtilities.h"
 
 @interface FIRMessagingPendingTopicsListTest : XCTestCase
 
@@ -220,7 +180,6 @@ - (void)testCompletionOfTopicUpdatesInSameThread {
 }
 
 - (void)testAddingTopicToCurrentBatchWhileCurrentBatchTopicsInFlight {
-
   FIRMessagingPendingTopicsList *pendingTopics = [[FIRMessagingPendingTopicsList alloc] init];
   pendingTopics.delegate = self.alwaysReadyDelegate;
 

Example/Messaging/Tests/FIRMessagingPubSubTest.m

@@ -15,8 +15,29 @@
  */
 
 #import <XCTest/XCTest.h>
+#import <OCMock/OCMock.h>
 
+#import <GoogleUtilities/GULReachabilityChecker.h>
+
+#import "Firebase/Messaging/FIRMessagingClient.h"
 #import "Firebase/Messaging/FIRMessagingPubSub.h"
+#import "Firebase/Messaging/FIRMessagingPendingTopicsList.h"
+#import "Firebase/Messaging/FIRMessagingRmqManager.h"
+#import "Example/Messaging/Tests/FIRMessagingTestUtilities.h"
+
+@interface FIRMessagingPubSub (ExposedForTest)
+
+@property(nonatomic, readwrite, strong) FIRMessagingPendingTopicsList *pendingTopicUpdates;
+- (void)archivePendingTopicsList:(FIRMessagingPendingTopicsList *)topicsList;
+- (void)restorePendingTopicsList;
+
+@end
+
+@interface FIRMessagingPendingTopicsList (ExposedForTest)
+
+@property(nonatomic, strong) NSMutableArray <FIRMessagingTopicBatch *> *topicBatches;
+
+@end
 
 @interface FIRMessagingPubSubTest : XCTestCase
 @end
@@ -87,4 +108,33 @@ - (void)testRemoveTopicPrefix {
   XCTAssertEqualObjects(topic, kTopicName);
 }
 
+-(void)testTopicListArchive {
+  MockPendingTopicsListDelegate *notReadyDelegate = [[MockPendingTopicsListDelegate alloc] init];
+  notReadyDelegate.isReady = NO;
+  FIRMessagingPendingTopicsList *topicList = [[FIRMessagingPendingTopicsList alloc] init];
+  topicList.delegate = notReadyDelegate;
+
+  // There should be 3 batches as actions are different than the last ones.
+  [topicList addOperationForTopic:@"/topics/0" withAction:FIRMessagingTopicActionSubscribe completion:nil];
+  [topicList addOperationForTopic:@"/topics/1" withAction:FIRMessagingTopicActionUnsubscribe completion:nil];
+  [topicList addOperationForTopic:@"/topics/2" withAction:FIRMessagingTopicActionSubscribe completion:nil];
+  XCTAssertEqual(topicList.numberOfBatches, 3);
+
+  id mockClientDelegate = OCMStrictProtocolMock(@protocol(FIRMessagingClientDelegate));
+  id mockReachability = OCMClassMock([GULReachabilityChecker class]);
+  id mockRmqManager = OCMClassMock([FIRMessagingRmqManager class]);
+  FIRMessagingClient *client = [[FIRMessagingClient alloc]
+                                initWithDelegate:mockClientDelegate
+                                reachability:mockReachability
+                                rmq2Manager:mockRmqManager];
+  FIRMessagingPubSub *pubSub = [[FIRMessagingPubSub alloc] initWithClient:client];
+  [pubSub archivePendingTopicsList:topicList];
+  [pubSub restorePendingTopicsList];
+  XCTAssertEqual(pubSub.pendingTopicUpdates.numberOfBatches, 3);
+  NSMutableArray <FIRMessagingTopicBatch *> *topicBatches = pubSub.pendingTopicUpdates.topicBatches;
+  XCTAssertTrue([topicBatches[0].topics containsObject:@"/topics/0"]);
+  XCTAssertTrue([topicBatches[1].topics containsObject:@"/topics/1"]);
+  XCTAssertTrue([topicBatches[2].topics containsObject:@"/topics/2"]);
+}
+
 @end

Example/Messaging/Tests/FIRMessagingTestUtilities.h

@@ -19,10 +19,31 @@
 #import <FirebaseMessaging/FIRMessaging.h>
 #import <FirebaseInstanceID/FIRInstanceID.h>
 
+#import "Firebase/Messaging/FIRMessagingTopicsCommon.h"
+#import "Firebase/Messaging/FIRMessagingPendingTopicsList.h"
+
 @class GULUserDefaults;
 
 NS_ASSUME_NONNULL_BEGIN
 
+typedef void (^MockDelegateSubscriptionHandler)(NSString *topic,
+                                                FIRMessagingTopicAction action,
+                                                FIRMessagingTopicOperationCompletion completion);
+
+/**
+ * This object lets us provide a stub delegate where we can customize the behavior by providing
+ * blocks. We need to use this instead of stubbing a OCMockProtocol because our delegate methods
+ * take primitive values (e.g. action), which is not easy to use from OCMock
+ * @see http://stackoverflow.com/a/6332023
+ */
+@interface MockPendingTopicsListDelegate: NSObject <FIRMessagingPendingTopicsListDelegate>
+
+@property(nonatomic, assign) BOOL isReady;
+@property(nonatomic, copy) MockDelegateSubscriptionHandler subscriptionHandler;
+@property(nonatomic, copy) void(^updateHandler)(void);
+
+@end
+
 @interface FIRMessaging (TestUtilities)
 // Surface the user defaults instance to clean up after tests.
 @property(nonatomic, strong) NSUserDefaults *messagingUserDefaults;
@@ -45,4 +66,6 @@ NS_ASSUME_NONNULL_BEGIN
 
 @end
 
+
+
 NS_ASSUME_NONNULL_END

Example/Messaging/Tests/FIRMessagingTestUtilities.m

@@ -65,6 +65,29 @@ - (void)removeDatabase;
 
 @end
 
+@implementation MockPendingTopicsListDelegate
+
+- (BOOL)pendingTopicsListCanRequestTopicUpdates:(FIRMessagingPendingTopicsList *)list {
+  return self.isReady;
+}
+
+- (void)pendingTopicsList:(FIRMessagingPendingTopicsList *)list
+  requestedUpdateForTopic:(NSString *)topic
+                   action:(FIRMessagingTopicAction)action
+               completion:(FIRMessagingTopicOperationCompletion)completion {
+  if (self.subscriptionHandler) {
+    self.subscriptionHandler(topic, action, completion);
+  }
+}
+
+- (void)pendingTopicsListDidUpdate:(FIRMessagingPendingTopicsList *)list {
+  if (self.updateHandler) {
+    self.updateHandler();
+  }
+}
+
+@end
+
 @implementation FIRMessagingTestUtilities
 
 - (instancetype)initWithUserDefaults:(GULUserDefaults *)userDefaults withRMQManager:(BOOL)withRMQManager {

Firebase/Messaging/CHANGELOG.md

@@ -1,3 +1,6 @@
+# unreleased
+- [fixed] Use secure coding for Messaging's pending topics. (#3686)
+
 # 2020-02 -- v 4.2.1
 - [added] Firebase Pod support for watchOS: `pod 'Firebase/Messaging'` in addition to `pod 'FirebaseMessaging'`. (#4807)
 - [fixed] Fix FIRMessagingExtensionHelper crash in unit tests when `attachment == nil`. (#4689)

Firebase/Messaging/FIRMMessageCode.h

@@ -119,6 +119,9 @@ typedef NS_ENUM(NSInteger, FIRMessagingMessageCode) {
   kFIRMessagingMessageCodePubSub001 = 9001,  // I-FCM009001
   kFIRMessagingMessageCodePubSub002 = 9002,  // I-FCM009002
   kFIRMessagingMessageCodePubSub003 = 9003,  // I-FCM009003
+  kFIRMessagingMessageCodePubSubArchiveError = 9004,
+  kFIRMessagingMessageCodePubSubUnarchiveError = 9005,
+
   // FIRMessagingReceiver.m
   kFIRMessagingMessageCodeReceiver000 = 10000,  // I-FCM010000
   kFIRMessagingMessageCodeReceiver001 = 10001,  // I-FCM010001

Firebase/Messaging/FIRMessagingPendingTopicsList.h

@@ -30,7 +30,7 @@ NS_ASSUME_NONNULL_BEGIN
  *  topics is unique, as it doesn't make sense to apply the same action to the same topic
  *  repeatedly; the result would be the same as the first time.
  */
-@interface FIRMessagingTopicBatch : NSObject <NSCoding>
+@interface FIRMessagingTopicBatch : NSObject <NSSecureCoding>
 
 @property(nonatomic, readonly, assign) FIRMessagingTopicAction action;
 @property(nonatomic, readonly, copy) NSMutableSet <NSString *> *topics;
@@ -101,7 +101,7 @@ NS_ASSUME_NONNULL_BEGIN
  *
  *  @see FIRMessagingPendingTopicsListDelegate
  */
-@interface FIRMessagingPendingTopicsList : NSObject <NSCoding>
+@interface FIRMessagingPendingTopicsList : NSObject <NSSecureCoding>
 
 @property(nonatomic, weak) NSObject <FIRMessagingPendingTopicsListDelegate> *delegate;
 

Firebase/Messaging/FIRMessagingPendingTopicsList.m

@@ -17,11 +17,10 @@
 #import "Firebase/Messaging/FIRMessagingPendingTopicsList.h"
 
 #import "Firebase/Messaging/FIRMessaging_Private.h"
+#import "Firebase/Messaging/FIRMessagingDefines.h"
 #import "Firebase/Messaging/FIRMessagingLogger.h"
 #import "Firebase/Messaging/FIRMessagingPubSub.h"
 
-#import "Firebase/Messaging/FIRMessagingDefines.h"
-
 NSString *const kPendingTopicBatchActionKey = @"action";
 NSString *const kPendingTopicBatchTopicsKey = @"topics";
 
@@ -48,7 +47,11 @@ - (instancetype)initWithAction:(FIRMessagingTopicAction)action {
   return self;
 }
 
-#pragma mark NSCoding
+#pragma mark NSSecureCoding
+
++ (BOOL)supportsSecureCoding {
+  return YES;
+}
 
 - (void)encodeWithCoder:(NSCoder *)aCoder {
   [aCoder encodeInteger:self.action forKey:kPendingTopicBatchActionKey];
@@ -65,10 +68,9 @@ - (instancetype)initWithCoder:(NSCoder *)aDecoder {
   }
 
   if (self = [self initWithAction:action]) {
-    NSSet *topics = [aDecoder decodeObjectForKey:kPendingTopicBatchTopicsKey];
-    if ([topics isKindOfClass:[NSSet class]]) {
-      _topics = [topics mutableCopy];
-    }
+    _topics = [aDecoder decodeObjectOfClasses:
+                     [NSSet setWithObjects:NSMutableSet.class, NSString.class, nil]
+                                             forKey:kPendingTopicBatchTopicsKey];
     _topicHandlers = [NSMutableDictionary dictionary];
   }
   return self;
@@ -109,7 +111,11 @@ + (void)pruneTopicBatches:(NSMutableArray <FIRMessagingTopicBatch *> *)topicBatc
   }
 }
 
-#pragma mark NSCoding
+#pragma mark NSSecureCoding
+
++ (BOOL)supportsSecureCoding {
+  return YES;
+}
 
 - (void)encodeWithCoder:(NSCoder *)aCoder {
   [aCoder encodeObject:[NSDate date] forKey:kPendingTopicsTimestampEncodingKey];
@@ -119,10 +125,13 @@ - (void)encodeWithCoder:(NSCoder *)aCoder {
 - (nullable instancetype)initWithCoder:(NSCoder *)aDecoder {
 
   if (self = [self init]) {
-    _archiveDate = [aDecoder decodeObjectForKey:kPendingTopicsTimestampEncodingKey];
-    NSArray *archivedBatches = [aDecoder decodeObjectForKey:kPendingBatchesEncodingKey];
-    if (archivedBatches) {
-      _topicBatches = [archivedBatches mutableCopy];
+    _archiveDate = [aDecoder decodeObjectOfClass:NSDate.class
+                                          forKey:kPendingTopicsTimestampEncodingKey];
+    _topicBatches =
+    [aDecoder decodeObjectOfClasses:
+     [NSSet setWithObjects:NSMutableArray.class,FIRMessagingTopicBatch.class, nil]
+                             forKey:kPendingBatchesEncodingKey];
+    if (_topicBatches) {
       [FIRMessagingPendingTopicsList pruneTopicBatches:_topicBatches];
     }
     _topicsInFlight = [NSMutableSet set];

Firebase/Messaging/FIRMessagingPubSub.m

@@ -16,6 +16,7 @@
 
 #import "Firebase/Messaging/FIRMessagingPubSub.h"
 
+#import <GoogleUtilities/GULSecureCoding.h>
 #import <GoogleUtilities/GULUserDefaults.h>
 #import <FirebaseMessaging/FIRMessaging.h>
 
@@ -183,10 +184,14 @@ - (BOOL)pendingTopicsListCanRequestTopicUpdates:(FIRMessagingPendingTopicsList *
 
 - (void)archivePendingTopicsList:(FIRMessagingPendingTopicsList *)topicsList {
   GULUserDefaults *defaults = [GULUserDefaults standardUserDefaults];
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-  NSData *pendingData = [NSKeyedArchiver archivedDataWithRootObject:topicsList];
-#pragma clang diagnostic pop
+  NSError *error;
+  NSData *pendingData = [GULSecureCoding archivedDataWithRootObject:topicsList error:&error];
+  if (error) {
+    FIRMessagingLoggerError(kFIRMessagingMessageCodePubSubArchiveError,
+                            @"Failed to archive topic list data %@",
+                            error);
+    return;
+  }
   [defaults setObject:pendingData forKey:kPendingSubscriptionsListKey];
   [defaults synchronize];
 }
@@ -195,23 +200,24 @@ - (void)restorePendingTopicsList {
   GULUserDefaults *defaults = [GULUserDefaults standardUserDefaults];
   NSData *pendingData = [defaults objectForKey:kPendingSubscriptionsListKey];
   FIRMessagingPendingTopicsList *subscriptions;
-  @try {
-    if (pendingData) {
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-      subscriptions = [NSKeyedUnarchiver unarchiveObjectWithData:pendingData];
-#pragma clang diagnostic pop
-    }
-  } @catch (NSException *exception) {
-    // Nothing we can do, just continue as if we don't have pending subscriptions
-  } @finally {
-    if (subscriptions) {
-      self.pendingTopicUpdates = subscriptions;
-    } else {
-      self.pendingTopicUpdates = [[FIRMessagingPendingTopicsList alloc] init];
+  if (pendingData) {
+    NSError *error;
+    subscriptions = [GULSecureCoding unarchivedObjectOfClasses:
+     [NSSet setWithObjects:FIRMessagingPendingTopicsList.class, nil]
+                                                      fromData:pendingData
+                                                         error:&error];
+    if (error) {
+      FIRMessagingLoggerError(kFIRMessagingMessageCodePubSubUnarchiveError,
+                              @"Failed to unarchive topic list data %@",
+                              error);
     }
-    self.pendingTopicUpdates.delegate = self;
   }
+  if (subscriptions) {
+    self.pendingTopicUpdates = subscriptions;
+  } else {
+    self.pendingTopicUpdates = [[FIRMessagingPendingTopicsList alloc] init];
+  }
+  self.pendingTopicUpdates.delegate = self;
 }
 
 #pragma mark - Private Helpers

GoogleUtilities/Environment/GULSecureCoding.m

@@ -23,7 +23,7 @@ + (nullable id)unarchivedObjectOfClasses:(NSSet<Class> *)classes
                                    error:(NSError **)outError {
   id object;
 #if __has_builtin(__builtin_available)
-  if (@available(macOS 10.13, iOS 11.0, tvOS 11.0, *)) {
+  if (@available(macOS 10.13, iOS 11.0, tvOS 11.0, watchOS 4.0, *)) {
     object = [NSKeyedUnarchiver unarchivedObjectOfClasses:classes fromData:data error:outError];
   } else
 #endif  // __has_builtin(__builtin_available)
@@ -62,7 +62,7 @@ + (nullable id)unarchivedObjectOfClass:(Class)class
 + (nullable NSData *)archivedDataWithRootObject:(id<NSCoding>)object error:(NSError **)outError {
   NSData *archiveData;
 #if __has_builtin(__builtin_available)
-  if (@available(macOS 10.13, iOS 11.0, tvOS 11.0, *)) {
+  if (@available(macOS 10.13, iOS 11.0, tvOS 11.0, watchOS 4.0, *)) {
     archiveData = [NSKeyedArchiver archivedDataWithRootObject:object
                                         requiringSecureCoding:YES
                                                         error:outError];