Skip to content

Commit

Permalink
fix(app-check, activate): correctly check activate parameters
Browse files Browse the repository at this point in the history
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!
  • Loading branch information
mikehardy committed Feb 6, 2022
1 parent c8df604 commit 209f0d4
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 2 deletions.
20 changes: 19 additions & 1 deletion packages/app-check/e2e/appcheck.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.only('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'));
}
});
});
});
21 changes: 20 additions & 1 deletion packages/app-check/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
}

Expand Down

0 comments on commit 209f0d4

Please sign in to comment.