From 53969da57c4ae7be0467343744876d5aced65621 Mon Sep 17 00:00:00 2001
From: Mike Hardy <github@mikehardy.net>
Date: Sun, 6 Feb 2022 17:59:44 -0500
Subject: [PATCH] fix(app-check, activate): correctly check activate parameters

previously, the required string argument was not validated, and
the optional boolean argument was not supplied to native, causing
an android native crash if not supplied

omitting the string now correctly throws an error, and omitting the
token refresh boolean will default correctly to app check token refresh
as configured in firebase.json, app-wide data collection if app check
does not have a config, and the default true if there is no config

Fixes #5981 - thanks to @rawatnaresh for catching this!
---
 packages/app-check/e2e/appcheck.e2e.js | 20 +++++++++++++++++++-
 packages/app-check/lib/index.js        | 21 ++++++++++++++++++++-
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/packages/app-check/e2e/appcheck.e2e.js b/packages/app-check/e2e/appcheck.e2e.js
index 56b6657394..dcd3ae2d02 100644
--- a/packages/app-check/e2e/appcheck.e2e.js
+++ b/packages/app-check/e2e/appcheck.e2e.js
@@ -65,12 +65,30 @@ describe('appCheck()', function () {
     });
   });
   describe('activate())', function () {
-    it('should activate with default provider and default token refresh', function () {
+    it('should activate with default provider and defined token refresh', function () {
       firebase
         .appCheck()
         .activate('ignored', false)
         .then(value => expect(value).toBe(undefined))
         .catch(e => new Error('app-check activate failed? ' + e));
     });
+
+    it('should error if activate gets no parameters', async function () {
+      try {
+        firebase.appCheck().activate();
+        return Promise.reject(new Error('should have thrown an error'));
+      } catch (e) {
+        e.message.should.containEql('siteKeyOrProvider must be a string value');
+      }
+    });
+
+    it('should not error if activate called with no token refresh value', async function () {
+      try {
+        firebase.appCheck().activate('ignored');
+        return Promise.resolve(true);
+      } catch (e) {
+        return Promise.reject(new Error('should not have thrown an error - ' + e));
+      }
+    });
   });
 });
diff --git a/packages/app-check/lib/index.js b/packages/app-check/lib/index.js
index 126888d2e8..210b9ffba2 100644
--- a/packages/app-check/lib/index.js
+++ b/packages/app-check/lib/index.js
@@ -15,7 +15,7 @@
  *
  */
 
-import { isIOS } from '@react-native-firebase/app/lib/common';
+import { isBoolean, isIOS, isString } from '@react-native-firebase/app/lib/common';
 import {
   createModuleNamespace,
   FirebaseModule,
@@ -32,6 +32,25 @@ const nativeModuleName = 'RNFBAppCheckModule';
 
 class FirebaseAppCheckModule extends FirebaseModule {
   activate(siteKeyOrProvider, isTokenAutoRefreshEnabled) {
+    if (!isString(siteKeyOrProvider)) {
+      throw new Error('siteKeyOrProvider must be a string value to match firebase-js-sdk API');
+    }
+
+    // If the caller did not specify token refresh, attempt to use app-check specific setting:
+    if (!isBoolean(isTokenAutoRefreshEnabled)) {
+      isTokenAutoRefreshEnabled = this.firebaseJson.app_check_token_auto_refresh;
+    }
+
+    // If that was not defined, attempt to use app-wide data collection setting per docs:
+    if (!isBoolean(isTokenAutoRefreshEnabled)) {
+      isTokenAutoRefreshEnabled = this.firebaseJson.app_data_collection_default_enabled;
+    }
+
+    // If that also was not defined, the default is documented as true.
+    if (!isBoolean(isTokenAutoRefreshEnabled)) {
+      isTokenAutoRefreshEnabled = true;
+    }
+
     return this.native.activate(siteKeyOrProvider, isTokenAutoRefreshEnabled);
   }