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

[🐛] iOS App Crashes getting Multi-factor session for second factor enroll #8177

Open
1 of 9 tasks
BlindDev opened this issue Dec 4, 2024 · 6 comments · Fixed by firebase/firebase-ios-sdk#14238
Open
1 of 9 tasks
Labels

Comments

@BlindDev
Copy link

BlindDev commented Dec 4, 2024

Issue

Getting a crash attempting to get MFA User session before enrolling second factor:

image

In our app we have two firebase projects (default and secondary). The app can dynamically switch between two projects:

const getApp = async () => {
    const apps = FirebaseApp.apps;
    // get app name from selected environment then find instance from apps array
    ...
};

const auth = async () => {
  if (!instance) {
      instance = FirebaseAuth(await getApp());
    }
    return instance;
};

All other methods for authentication including MFA verification (when second factor enrolled) work with both projects.

I managed to patch the iOS Pod and it fixed the issue, but I'm not sure if I did everything right and there are no security concerns with my approach.

It looks like when the app requests a multi-factor user session, it still tries to get the user from the default app instance.
In the ios/Pods/FirebaseAuth/FirebaseAuth/Sources/Swift/MultiFactor/MultiFactorSession.swift I've added new class method:

    class var sessionForCurrentUser: MultiFactorSession {
      guard let currentUser = Auth.auth().currentUser else {
       // place that causes the crash
        fatalError("Internal Auth Error: missing user for multifactor auth")
      }
      return .init(idToken: currentUser.tokenService.accessToken, currentUser: currentUser)
    }
      
   // new method that inits the session when user is provided as an argument
    class func sessionForUser(_ user: User) -> MultiFactorSession {
        return .init(idToken: user.tokenService.accessToken, currentUser: user)
    }

Then in the ios/Pods/FirebaseAuth/FirebaseAuth/Sources/Swift/MultiFactor/MultiFactor.swift I've replaced old method for session with the new one:

@objc(getSessionWithCompletion:)
    open func getSessionWithCompletion(_ completion: ((MultiFactorSession?, Error?) -> Void)?) {
        // this class already knows the user, it has weak reference to it
        guard let user = user else {
            fatalError("Internal Auth error: failed to get user enrolling in MultiFactor")
        }
       // now we can use the user for initiating the session
        let session = MultiFactorSession.sessionForUser(user)
      if let completion {
        completion(session, nil)
      }
    }

Project Files

Javascript

Click To Expand

package.json:

{
    "@react-native-firebase/analytics": "^21.6.1",
    "@react-native-firebase/app": "^21.6.1",
    "@react-native-firebase/auth": "^21.6.1",
    "@react-native-firebase/crashlytics": "^21.6.1",
    "@react-native-firebase/dynamic-links": "^21.6.1",
    "@react-native-firebase/in-app-messaging": "^21.6.1",
    "@react-native-firebase/installations": "^21.6.1",
    "@react-native-firebase/messaging": "^21.6.1",
    "@react-native-firebase/remote-config": "^21.6.1",
}

firebase.json for react-native-firebase v6:

{
  "react-native": {
    "crashlytics_disable_auto_disabler": true,
    "google_analytics_automatic_screen_reporting_enabled": false
  }
}

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:

 System:
  OS: macOS 14.6.1
  CPU: (16) arm64 Apple M3 Max
  Memory: 1.13 GB / 64.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 20.9.0
    path: /usr/local/bin/node
  Yarn: Not Found
  npm:
    version: 10.2.4
    path: /usr/local/bin/npm
  Watchman: Not Found
Managers:
  CocoaPods: Not Found
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 24.1
      - iOS 18.1
      - macOS 15.1
      - tvOS 18.1
      - visionOS 2.1
      - watchOS 11.1
  Android SDK: Not Found
IDEs:
  Android Studio: 2022.3 AI-223.8836.35.2231.11005911
  Xcode:
    version: 16.1/16B40
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 17.0.9
    path: /usr/bin/javac
  Ruby:
    version: 2.6.10
    path: /usr/bin/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.73.5
    wanted: 0.73.5
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: true
  newArchEnabled: false
  • Platform that you're experiencing the issue on:
    • iOS
    • Android
    • [x ] 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:
    • 21.6.1
  • Firebase module(s) you're using that has the issue:
    • e.g. Instance ID
  • Are you using TypeScript?
    • Y & 5.0.4


@mikehardy
Copy link
Collaborator

Interesting - seems like your approach is sound but I have to defer upstream, have you opened an issue / PR upstream in firebase-ios-sdk ? I've found them to be excellent to work with if you've got your details straight, and quick to integrate change if you propose good solutions

https://github.com/firebase/firebase-ios-sdk/blob/454f3999ce8fd4d21f4b7e17c469c9b872c58741/FirebaseAuth/Sources/Swift/MultiFactor/MultiFactor.swift#L36

@BlindDev
Copy link
Author

BlindDev commented Dec 5, 2024

I haven't yet. I was hopping to confirm it's the ios-sdk issue and not like I've implemented it wrong in rn-firebase

@mikehardy
Copy link
Collaborator

Given you basically have an upstream patch that seems to work, this might be something worth a little pre-emptive ping for a headsup from @paulb777 - Paul, looks like a secondary / non-default app auth issue in MFA - not sure about you all upstream but at this layer the secondary-app pathways are exercised less frequently and MFA doubly-so, thus they are a frequent source of error despite the age of the code. Might be the same over there, and this might be a real issue that seems obvious on first glance ?

@paulb777
Copy link
Contributor

paulb777 commented Dec 5, 2024

Agreed that it looks like an issue to be getting user from currentUser there. Please open an issue (or PR) at https://github.com/firebase/firebase-ios-sdk, references this one, and we should be able to get the fix into the next release.

Thanks @BlindDev and @mikehardy !

@paulb777
Copy link
Contributor

paulb777 commented Dec 9, 2024

@BlindDev I made a PR at firebase/firebase-ios-sdk#14238 inspired by your suggestion. would you confirm that it fixes your use case?

@BlindDev
Copy link
Author

@paulb777 yes, it works as expected. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants