Firebase Installations: validation of FIROptions parameters (#4683)

maksymmalyhin · web-flow · commit 1bc325543614 · 2020-01-16T16:13:49.000-05:00
* FIS: Throw exception on incomplete Firebase config.

* API docs updated.

* FirebaseInstallations bump minor version: 1.1.0

* Fix tests

* Text fixed

* Make exception message more informative

* Fix Remote Config tests.

* Tests updated to use GCMSenderID if projectID is not available.

* Use `GCMSenderID` when `projectID` is not available.

* ./scripts/style.sh

* Comments

* GCMSenderID and projectID validation updated.

* Comment

* FirebaseInstallations: version bump to 1.0.1.

* FIS changelog

* FirebaseInstallations: version bump to 1.1.0

* FIS and Core changelogs.

* Changelogs remove GCMSenderID

* Typo

* Typo

* fix link

* Revert Core changelog

* FIS changelog fix.
diff --git a/Example/InstanceID/Tests/FIRInstanceIDTest.m b/Example/InstanceID/Tests/FIRInstanceIDTest.m
@@ -156,6 +156,8 @@ - (void)testSharedInstance {
   // The shared instance relies on the default app being configured. Configure it.
   FIROptions *options = [[FIROptions alloc] initWithGoogleAppID:kGoogleAppID
                                                     GCMSenderID:kGCMSenderID];
+  options.APIKey = @"api-key";
+  options.projectID = @"project-id";
   [FIRApp configureWithName:kFIRDefaultAppName options:options];
   FIRInstanceID *instanceID = [FIRInstanceID instanceID];
   XCTAssertNotNil(instanceID);
diff --git a/Example/Messaging/Tests/FIRMessagingInstanceTest.swift b/Example/Messaging/Tests/FIRMessagingInstanceTest.swift
@@ -24,6 +24,8 @@ class FIRMessagingInstanceTest: XCTestCase {
     // This is an example of a functional test case.
     // Use XCTAssert and related functions to verify your tests produce the correct results.
     let options = FirebaseOptions(googleAppID: "1:123:ios:123abc", gcmSenderID: "valid-sender-id")
+    options.apiKey = "api-key"
+    options.projectID = "project-id"
     FirebaseApp.configure(options: options)
     let original = Messaging.messaging()
 
diff --git a/FirebaseInstallations.podspec b/FirebaseInstallations.podspec
@@ -1,6 +1,6 @@
 Pod::Spec.new do |s|
   s.name             = 'FirebaseInstallations'
-  s.version          = '1.0.0'
+  s.version          = '1.1.0'
   s.summary          = 'Firebase Installations for iOS'
 
   s.description      = <<-DESC
diff --git a/FirebaseInstallations/CHANGELOG.md b/FirebaseInstallations/CHANGELOG.md
@@ -1,3 +1,7 @@
+# v1.1.0 -- M62.1
+
+- [changed] Throw an exception when there are missing required `FirebaseOptions` parameters (`APIKey`, `googleAppID`, and `projectID`). Please make sure your `GoogleServices-Info.plist` (or `FirebaseOptions` if you configure Firebase in code) is up to date. The file and settings can be downloaded from the [Firebase Console](https://console.firebase.google.com/).  (#4683)
+
 # v1.0.0 -- M62
 
 - [added] The Firebase Installations Service is an infrastructure service for Firebase services that creates unique identifiers and authentication tokens for Firebase clients (called "Firebase Installations") enabling Firebase Targeting, i.e. interoperation between Firebase services.
diff --git a/FirebaseInstallations/Source/Library/FIRInstallations.m b/FirebaseInstallations/Source/Library/FIRInstallations.m
@@ -34,6 +34,7 @@
 #import "FIRInstallationsErrorUtil.h"
 #import "FIRInstallationsIDController.h"
 #import "FIRInstallationsItem.h"
+#import "FIRInstallationsLogger.h"
 #import "FIRInstallationsStoredAuthToken.h"
 #import "FIRInstallationsVersion.h"
 
@@ -101,6 +102,7 @@ - (instancetype)initWithAppOptions:(FIROptions *)appOptions
                  prefetchAuthToken:(BOOL)prefetchAuthToken {
   self = [super init];
   if (self) {
+    [[self class] validateAppOptions:appOptions appName:appName];
     [[self class] assertCompatibleIIDVersion];
 
     _appOptions = [appOptions copy];
@@ -117,12 +119,44 @@ - (instancetype)initWithAppOptions:(FIROptions *)appOptions
   return self;
 }
 
++ (void)validateAppOptions:(FIROptions *)appOptions appName:(NSString *)appName {
+  NSMutableArray *missingFields = [NSMutableArray array];
+  if (appName.length < 1) {
+    [missingFields addObject:@"`FirebaseApp.name`"];
+  }
+  if (appOptions.APIKey.length < 1) {
+    [missingFields addObject:@"`FirebaseOptions.APIKey`"];
+  }
+  if (appOptions.googleAppID.length < 1) {
+    [missingFields addObject:@"`FirebaseOptions.googleAppID`"];
+  }
+
+  // TODO(#4692): Check for `appOptions.projectID.length < 1` only.
+  // We can use `GCMSenderID` instead of `projectID` temporary.
+  if (appOptions.projectID.length < 1 && appOptions.GCMSenderID.length < 1) {
+    [missingFields addObject:@"`FirebaseOptions.projectID`"];
+  }
+
+  if (missingFields.count > 0) {
+    [NSException
+         raise:kFirebaseInstallationsErrorDomain
+        format:
+            @"%@[%@] Could not configure Firebase Installations due to invalid FirebaseApp "
+            @"options. The following parameters are nil or empty: %@. If you use "
+            @"GoogleServices-Info.plist please download the most recent version from the Firebase "
+            @"Console. If you configure Firebase in code, please make sure you specify all "
+            @"required parameters.",
+            kFIRLoggerInstallations, kFIRInstallationsMessageCodeInvalidFirebaseAppOptions,
+            [missingFields componentsJoinedByString:@", "]];
+  }
+}
+
 #pragma mark - Public
 
 + (FIRInstallations *)installations {
   FIRApp *defaultApp = [FIRApp defaultApp];
   if (!defaultApp) {
-    [NSException raise:NSInternalInconsistencyException
+    [NSException raise:kFirebaseInstallationsErrorDomain
                 format:@"The default FirebaseApp instance must be configured before the default"
                        @"FirebaseApp instance can be initialized. One way to ensure that is to "
                        @"call `[FIRApp configure];` (`FirebaseApp.configure()` in Swift) in the App"
@@ -191,7 +225,7 @@ + (void)assertCompatibleIIDVersion {
   return;
 #else
   if (![self isIIDVersionCompatible]) {
-    [NSException raise:NSInternalInconsistencyException
+    [NSException raise:kFirebaseInstallationsErrorDomain
                 format:@"FirebaseInstallations will not work correctly with current version of "
                        @"Firebase Instance ID. Please update your Firebase Instance ID version."];
   }
diff --git a/FirebaseInstallations/Source/Library/FIRInstallationsLogger.h b/FirebaseInstallations/Source/Library/FIRInstallationsLogger.h
@@ -46,3 +46,6 @@ extern NSString *const kFIRInstallationsMessageCodeAuthTokenCoderVersionMismatch
 // FIRInstallationsStoredIIDCheckin.m
 extern NSString *const kFIRInstallationsMessageCodeIIDCheckinCoderVersionMismatch;
 extern NSString *const kFIRInstallationsMessageCodeIIDCheckinFailedToDecode;
+
+// FIRInstallations.m
+extern NSString *const kFIRInstallationsMessageCodeInvalidFirebaseAppOptions;
diff --git a/FirebaseInstallations/Source/Library/FIRInstallationsLogger.m b/FirebaseInstallations/Source/Library/FIRInstallationsLogger.m
@@ -44,3 +44,6 @@
 // FIRInstallationsStoredIIDCheckin.m
 NSString *const kFIRInstallationsMessageCodeIIDCheckinCoderVersionMismatch = @"I-FIS007000";
 NSString *const kFIRInstallationsMessageCodeIIDCheckinFailedToDecode = @"I-FIS007001";
+
+// FIRInstallations.m
+NSString *const kFIRInstallationsMessageCodeInvalidFirebaseAppOptions = @"I-FIS008000";
diff --git a/FirebaseInstallations/Source/Library/InstallationsIDController/FIRInstallationsIDController.h b/FirebaseInstallations/Source/Library/InstallationsIDController/FIRInstallationsIDController.h
@@ -31,7 +31,7 @@ NS_ASSUME_NONNULL_BEGIN
                              APIKey:(NSString *)APIKey
                           projectID:(NSString *)projectID
                         GCMSenderID:(NSString *)GCMSenderID
-                        accessGroup:(NSString *)accessGroup;
+                        accessGroup:(nullable NSString *)accessGroup;
 
 - (FBLPromise<FIRInstallationsItem *> *)getInstallationItem;
 
diff --git a/FirebaseInstallations/Source/Library/InstallationsIDController/FIRInstallationsIDController.m b/FirebaseInstallations/Source/Library/InstallationsIDController/FIRInstallationsIDController.m
@@ -75,8 +75,12 @@ - (instancetype)initWithGoogleAppID:(NSString *)appID
   FIRSecureStorage *secureStorage = [[FIRSecureStorage alloc] init];
   FIRInstallationsStore *installationsStore =
       [[FIRInstallationsStore alloc] initWithSecureStorage:secureStorage accessGroup:accessGroup];
+
+  // Use `GCMSenderID` as project identifier when `projectID` is not available.
+  NSString *APIServiceProjectID = (projectID.length > 0) ? projectID : GCMSenderID;
   FIRInstallationsAPIService *apiService =
-      [[FIRInstallationsAPIService alloc] initWithAPIKey:APIKey projectID:projectID];
+      [[FIRInstallationsAPIService alloc] initWithAPIKey:APIKey projectID:APIServiceProjectID];
+
   FIRInstallationsIIDStore *IIDStore = [[FIRInstallationsIIDStore alloc] init];
   FIRInstallationsIIDTokenStore *IIDCheckingStore =
       [[FIRInstallationsIIDTokenStore alloc] initWithGCMSenderID:GCMSenderID];
diff --git a/FirebaseInstallations/Source/Library/Public/FIRInstallations.h b/FirebaseInstallations/Source/Library/Public/FIRInstallations.h
@@ -59,15 +59,17 @@ NS_SWIFT_NAME(Installations)
 
 /**
  * Returns a default instance of `Installations`.
- * @return Returns an instance of `Installations` for `FirebaseApp.defaultApp(). Throws an exception
- * if the default app is not configured yet.
+ * @returns An instance of `Installations` for `FirebaseApp.defaultApp().
+ * @throw Throws an exception if the default app is not configured yet or required  `FirebaseApp`
+ * options are missing.
  */
 + (FIRInstallations *)installations NS_SWIFT_NAME(installations());
 
 /**
  * Returns an instance of `Installations` for an application.
  * @param application A configured `FirebaseApp` instance.
- * @return Returns an instance of `Installations` corresponding to the passed application.
+ * @returns An instance of `Installations` corresponding to the passed application.
+ * @throw Throws an exception if required `FirebaseApp` options are missing.
  */
 + (FIRInstallations *)installationsWithApp:(FIRApp *)application NS_SWIFT_NAME(installations(app:));
 
diff --git a/FirebaseInstallations/Source/Tests/Integration/FIRInstallationsIntegrationTests.m b/FirebaseInstallations/Source/Tests/Integration/FIRInstallationsIntegrationTests.m
@@ -205,6 +205,8 @@ - (FIRApp *)createAndConfigureAppWithName:(NSString *)name {
   FIROptions *options =
       [[FIROptions alloc] initWithGoogleAppID:@"1:100000000000:ios:aaaaaaaaaaaaaaaaaaaaaaaa"
                                   GCMSenderID:@"valid_sender_id"];
+  options.APIKey = @"some_api_key";
+  options.projectID = @"project_id";
   [FIRApp configureWithName:name options:options];
 
   return [FIRApp appNamed:name];
diff --git a/FirebaseInstallations/Source/Tests/Unit/FIRInstallationsIDControllerTests.m b/FirebaseInstallations/Source/Tests/Unit/FIRInstallationsIDControllerTests.m
@@ -87,6 +87,46 @@ - (void)tearDown {
   self.appName = nil;
 }
 
+#pragma mark - Initialization
+
+- (void)testInitWhenProjectIDSetThenItIsPassedToAPIService {
+  NSString *APIKey = @"api-key";
+  NSString *projectID = @"project-id";
+  OCMExpect([self.mockAPIService alloc]).andReturn(self.mockAPIService);
+  OCMExpect([self.mockAPIService initWithAPIKey:APIKey projectID:projectID])
+      .andReturn(self.mockAPIService);
+
+  FIRInstallationsIDController *controller =
+      [[FIRInstallationsIDController alloc] initWithGoogleAppID:@"app-id"
+                                                        appName:@"app-name"
+                                                         APIKey:APIKey
+                                                      projectID:projectID
+                                                    GCMSenderID:@"sender-id"
+                                                    accessGroup:nil];
+  XCTAssertNotNil(controller);
+
+  OCMVerifyAll(self.mockAPIService);
+}
+
+- (void)testInitWhenProjectIDIsNilThenGCMSenderIDIsPassedToAPIServiceAsProjectID {
+  NSString *APIKey = @"api-key";
+  NSString *GCMSenderID = @"sender-id";
+  OCMExpect([self.mockAPIService alloc]).andReturn(self.mockAPIService);
+  OCMExpect([self.mockAPIService initWithAPIKey:APIKey projectID:GCMSenderID])
+      .andReturn(self.mockAPIService);
+
+  FIRInstallationsIDController *controller =
+      [[FIRInstallationsIDController alloc] initWithGoogleAppID:@"app-id"
+                                                        appName:@"app-name"
+                                                         APIKey:APIKey
+                                                      projectID:@""
+                                                    GCMSenderID:GCMSenderID
+                                                    accessGroup:nil];
+  XCTAssertNotNil(controller);
+
+  OCMVerifyAll(self.mockAPIService);
+}
+
 #pragma mark - Get Installation
 
 - (void)testGetInstallationItem_WhenFIDExists_ThenItIsReturned {
@@ -1134,4 +1174,8 @@ - (FIRInstallationsItem *)expectAuthTokenRefreshForInstallation:
   return responseInstallation;
 }
 
+- (void)expectAPIServiceInitWithAPIKey:(NSString *)APIKey projectID:(NSString *)projectID {
+  OCMExpect([self.mockAPIService initWithAPIKey:APIKey projectID:projectID]);
+}
+
 @end
diff --git a/FirebaseInstallations/Source/Tests/Unit/FIRInstallationsTests.m b/FirebaseInstallations/Source/Tests/Unit/FIRInstallationsTests.m
@@ -45,6 +45,9 @@ - (void)setUp {
 
   self.appOptions = [[FIROptions alloc] initWithGoogleAppID:@"GoogleAppID"
                                                 GCMSenderID:@"GCMSenderID"];
+  self.appOptions.APIKey = @"APIKey";
+  self.appOptions.projectID = @"ProjectID";
+
   self.mockIDController = OCMClassMock([FIRInstallationsIDController class]);
   self.installations = [[FIRInstallations alloc] initWithAppOptions:self.appOptions
                                                             appName:@"appName"
@@ -234,4 +237,68 @@ - (void)testDeleteError {
   [self waitForExpectations:@[ deleteExpectation ] timeout:0.5];
 }
 
+#pragma mark - Invalid Firebase configuration
+
+- (void)testInitWhenProjectIDMissingThenNoThrow {
+  FIROptions *options = [self.appOptions copy];
+  options.projectID = nil;
+  XCTAssertNoThrow([self createInstallationsWithAppOptions:options appName:@"missingProjectID"]);
+
+  options.projectID = @"";
+  XCTAssertNoThrow([self createInstallationsWithAppOptions:options appName:@"emptyProjectID"]);
+}
+
+- (void)testInitWhenAPIKeyMissingThenThrows {
+  FIROptions *options = [self.appOptions copy];
+  options.APIKey = nil;
+  XCTAssertThrows([self createInstallationsWithAppOptions:options appName:@"missingAPIKey"]);
+
+  options.APIKey = @"";
+  XCTAssertThrows([self createInstallationsWithAppOptions:options appName:@"emptyAPIKey"]);
+}
+
+- (void)testInitWhenGoogleAppIDMissingThenThrows {
+  FIROptions *options = [self.appOptions copy];
+  options.googleAppID = @"";
+  XCTAssertThrows([self createInstallationsWithAppOptions:options appName:@"emptyGoogleAppID"]);
+}
+
+- (void)testInitWhenGCMSenderIDMissingThenThrows {
+  FIROptions *options = [self.appOptions copy];
+  options.GCMSenderID = @"";
+  XCTAssertNoThrow([self createInstallationsWithAppOptions:options appName:@"emptyGCMSenderID"]);
+}
+
+- (void)testInitWhenProjectIDAndGCMSenderIDMissingThenNoThrow {
+  FIROptions *options = [self.appOptions copy];
+  options.GCMSenderID = @"";
+
+  options.projectID = nil;
+  XCTAssertThrows([self createInstallationsWithAppOptions:options appName:@"missingProjectID"]);
+
+  options.projectID = @"";
+  XCTAssertThrows([self createInstallationsWithAppOptions:options appName:@"emptyProjectID"]);
+}
+
+- (void)testInitWhenAppNameMissingThenThrows {
+  FIROptions *options = [self.appOptions copy];
+  XCTAssertThrows([self createInstallationsWithAppOptions:options appName:@""]);
+  XCTAssertThrows([self createInstallationsWithAppOptions:options appName:nil]);
+}
+
+- (void)testInitWhenAppOptionsMissingThenThrows {
+  XCTAssertThrows([self createInstallationsWithAppOptions:nil appName:@"missingOptions"]);
+}
+
+#pragma mark - Helpers
+
+- (FIRInstallations *)createInstallationsWithAppOptions:(FIROptions *)options
+                                                appName:(NSString *)appName {
+  id mockIDController = OCMClassMock([FIRInstallationsIDController class]);
+  return [[FIRInstallations alloc] initWithAppOptions:options
+                                              appName:appName
+                            installationsIDController:mockIDController
+                                    prefetchAuthToken:NO];
+}
+
 @end
diff --git a/FirebaseRemoteConfig/Tests/Unit/FIRRemoteConfigComponentTest.m b/FirebaseRemoteConfig/Tests/Unit/FIRRemoteConfigComponentTest.m
@@ -173,8 +173,11 @@ - (void)testThrowsWithNilGCMSenderID {
 #pragma mark - Helpers
 
 - (FIROptions *)fakeOptions {
-  return [[FIROptions alloc] initWithGoogleAppID:@"1:123:ios:123abc"
-                                     GCMSenderID:@"correct_gcm_sender_id"];
+  FIROptions *options = [[FIROptions alloc] initWithGoogleAppID:@"1:123:ios:123abc"
+                                                    GCMSenderID:@"correct_gcm_sender_id"];
+  options.APIKey = @"api-key";
+  options.projectID = @"project-id";
+  return options;
 }
 
 - (NSString *)generatedTestAppName {
