Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[🐛] AppCheck's .activate() does not return a Promise on iOS #6052

Closed
1 of 10 tasks
birdofpreyru opened this issue Feb 4, 2022 · 2 comments · Fixed by #6056
Closed
1 of 10 tasks

[🐛] AppCheck's .activate() does not return a Promise on iOS #6052

birdofpreyru opened this issue Feb 4, 2022 · 2 comments · Fixed by #6056
Labels
platform: ios plugin: app-check Firebase AppCheck type: bug New bug report

Comments

@birdofpreyru
Copy link

Issue

According to the docs .activate() returns a Promise. It does so on Android, however it seems to return nothing on iOS. At least, I ended up with such code block in my app:

import getFbAppCheck from '@react-native-firebase/app-check';

// Irrelevant code skipped here

  if (!initPromise) {
    initPromise = getFbAppCheck()
      .activate('', true)
      .catch((error) => {
        initPromise = null;
        throw error;
      });
  }

// Irrelevant code skipped here

which works fine on Android, but on iOS it crashes with the error:

Cannot read property 'catch' of undefined

Sure, not a big trouble to workaround, as per docs it is not needed to call it on iOS at all, but still a bug, I believe.


Project Files

Javascript

Click To Expand

package.json:

# N/A

firebase.json for react-native-firebase v6:

# N/A

iOS

Click To Expand

ios/Podfile:

  • I'm not using Pods
  • I'm using Pods and my Podfile looks like:
# N/A

AppDelegate.m:

// N/A


Android

Click To Expand

Have you converted to AndroidX?

  • my application is an AndroidX application?
  • I am using android/gradle.settings jetifier=true for Android compatibility?
  • I am using the NPM package jetifier for react-native compatibility?

android/build.gradle:

// N/A

android/app/build.gradle:

// N/A

android/settings.gradle:

// N/A

MainApplication.java:

// N/A

AndroidManifest.xml:

<!-- N/A -->


Environment

Click To Expand

react-native info output:

 OUTPUT GOES HERE
  • Platform that you're experiencing the issue on:
    • iOS
    • Android
    • iOS but have not tested behavior on Android
    • Android but have not tested behavior on iOS
    • Both
  • react-native-firebase version you're using that has this issue:
    • e.g. 5.4.3
  • Firebase module(s) you're using that has the issue:
    • e.g. Instance ID
  • Are you using TypeScript?
    • Y/N & VERSION


@birdofpreyru birdofpreyru added help: needs-triage Issue needs additional investigation/triaging. type: bug New bug report labels Feb 4, 2022
@mikehardy
Copy link
Collaborator

Hey @birdofpreyru 👋 - nice to see you over here. Sorry about this, I believe you are exactly right - I need to add the resolve/reject parameters to the ios native module method signature here and resolve it at the end so that the ios platform is consistent with android like it should be

RCT_EXPORT_METHOD(activate
: (FIRApp *)firebaseApp
: (nonnull NSString *)siteKeyOrProvider
: (BOOL)isTokenAutoRefreshEnabled) {
// From SDK docs:
// NOTE: Make sure to call this method before FirebaseApp.configure().
// If this method is called after configuring Firebase, the changes will not take effect.
// So in react-native-firebase we will only use this to set the isTokenAutoRefreshEnabled
// parameter, but if AppCheck is included on iOS it wlil be active with DeviceCheckProviderFactory
FIRAppCheck *appCheck = [FIRAppCheck appCheckWithApp:firebaseApp];
appCheck.isTokenAutoRefreshEnabled = isTokenAutoRefreshEnabled;
}

@mikehardy mikehardy added plugin: app-check Firebase AppCheck Workflow: Needs Review Pending feedback or review from a maintainer. platform: ios and removed help: needs-triage Issue needs additional investigation/triaging. labels Feb 6, 2022
@mikehardy
Copy link
Collaborator

Ok - I've altered the e2e test to more thoroughly probe the return value there and now I've got the e2e test failing as it should be vs the current false positive for the ios platform. I'll have a fix shortly

mikehardy added a commit that referenced this issue Feb 6, 2022
previously iOS was not returning a promise, but Android was, documentation
was for a Promise to be returned

Fixes #6052 - Thanks to @birdofpreyru
@mikehardy mikehardy removed the Workflow: Needs Review Pending feedback or review from a maintainer. label Feb 6, 2022
mikehardy added a commit that referenced this issue Feb 7, 2022
previously iOS was not returning a promise, but Android was, documentation
was for a Promise to be returned

Fixes #6052 - Thanks to @birdofpreyru
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: ios plugin: app-check Firebase AppCheck type: bug New bug report
Projects
None yet
2 participants