[PAID] [$250] Disable NFC on onfido react-native-sdk

[AndrewGable]

Problem

The Apple developer team who reviews our iOS application commonly rejects our New Expensify app because they ask for a video of how the New Expensify app uses NFC. We know that this NFC support comes from the onfido react-native-sdk per their documentation. However, we do not currently use the passport NFC scanning.

Solution

Remove the NFC support as described here: https://github.com/onfido/react-native-sdk?tab=readme-ov-file#disabling-nfc-and-excluding-dependencies

Also make sure to remove the dependencies as mentioned in the documentation to prevent future iOS app rejections.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021836544665597870593
• Upwork Job ID: 1836544665597870593
• Last Price Increase: 2024-09-18
• Automatic offers:
• allgandalf | Reviewer | 104028950
• cretadn22 | Contributor | 104028951

Issue Owner

Current Issue Owner: @

Issue Owner

Current Issue Owner: @strepanier03

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @allgandalf (External)

[melvin-bot]

Triggered auto assignment to @strepanier03 (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[cretadn22]

Edited by proposal-police: This proposal was edited at 2024-09-12 10:28:35 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

This section has already been noted in the BUG description

The Apple developer team who reviews our iOS application commonly rejects our New Expensify app because they ask for a video of how the New Expensify app uses NFC. We know that this NFC support comes from the onfido react-native-sdk per their documentation. However, we do not currently use the passport NFC scanning.

What is the root cause of that problem?

We don't turn off NFC when initializing the connection to Onfido

What changes do you think we should make in order to solve the problem?

Currently, we're using "@onfido/react-native-sdk": "10.6.0", which allows us to pass disableNFC: true to disable NFC. However, in the latest release (v13.0.0) available at https://github.com/onfido/react-native-sdk/releases/tag/v13.0.0, the disableNFC parameter has been deprecated, and a new parameter called nfcOption has been introduced.

Therefore, we have two options:

1. Using disableNFC with the current version
2. Upgrade "@onfido/react-native-sdk" to "13.0.0" and use nfcOption parameter as guided in document

The OnfidoSDK.start function in the react-native-sdk relies on the native start function to initialize the connection. You can review the implementation here: Onfido.ts Line 93. So we need to clean up any unused libraries or packages in the native code after NFC is disabled.

1. For iOS, there don't appear to be any redundant libraries or packages when NFC is disabled.
2. For Android, we need to remove certain packages from the build process as outlined in the documentation.

What alternative solutions did you explore? (Optional)

[allgandalf]

For Android, we need to remove certain packages from the build process as outlined in the documentation.

@cretadn22 can you be more specific here please, which certain packages do you imply? please be more technical for me to know you know what you propose to do

[cretadn22]

@allgandalf Let's see this document: https://github.com/onfido/react-native-sdk?tab=readme-ov-file#disabling-nfc-and-excluding-dependencies

We need to apply the change from document to here

dependencies {
  implementation 'com.onfido.sdk.capture:onfido-capture-sdk:x.y.z' {
    exclude group: 'net.sf.scuba', module: 'scuba-sc-android'
    exclude group: 'org.jmrtd', module: 'jmrtd'
    exclude group: 'com.madgag.spongycastle', module: 'prov'
  }
}

To modify the @onfido/react-native-sdk, you can either use patch-package or fork the library.

[allgandalf]

Lets go with @cretadn22 proposal here,

🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @rafecolton, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[rafecolton]

@cretadn22 can you please clarify a couple things about your proposal:

1. You mentioned two options - use the disableNFC parameter or upgrade the library and use the new parameter. Which of the two are you proposing?
2. Regarding removing the android dependencies...
   a. Is it necessary to patch the library or is there some other way to configure those to be excluded?
   b. Do we actually need to exclude them since the issue here concerns iOS?

[cretadn22]

@rafecolton

1. Use disableNFC for simplicity.

2a. I don’t see any alternative; we can only remove it from the react-native-sdk repository.

2b. This step is optional. There’s no issue if we choose to leave it as is.

[rafecolton]

1 sounds good 👍 @AndrewGable what do you think about doing a patch to exclude the nfc dependencies on android? It's not necessary to solve the problem in the OP but I assume it will make the android build a little smaller and/or faster. Seems overkill to me to maintain a fork for this, since the change would never get merged.

[AndrewGable]

Why do we need to patch? I assumed these changes were made to our own build.gradle and not a patch to the library?

[rafecolton]

Great question! I don't know much about grade builds. I tried searching the App code for "com.onfido.sdk.capture" and the only instance I found was in patches/@onfido+react-native-sdk+10.6.0.patch so it looks like we already patch the library's android/bin/build.gradle file. I don't see any explicit inclusion in that file or our patch of the dependencies they're saying to remove.

So I think that means we can add that block to our own build file as you're suggesting, but I'm not sure. If that sounds right to you, would it get added to android/app/build.gradle or android/build.gradle? I'm guessing the former based on the comments in the file.

[AndrewGable]

Yes I would assume it would be changed in android/app/build.gradle, but wanted to confirm with the contributor and C+ that this was the planned on solution.

[cretadn22]

Yes I would assume it would be changed in android/app/build.gradle

@AndrewGable, I don't think we can proceed with that approach. We need to make this change in the onfido/react-native-sdk library.

@rafecolton, we already have the patches/@onfido+react-native-sdk+10.6.0.patch file. We can easily modify this file to exclude the redundant parts.

[AndrewGable]

I don't think we can proceed with that approach.

Why not?

[cretadn22]

@AndrewGable, hold on for a moment. I’ll give it another try and get back to you

[rafecolton]

Not overdue Melvin, we are discussing

[cretadn22]

@AndrewGable @rafecolton After checking, I see that we don't need to exclude any dependencies since we haven't imported them yet. You can confirm this by looking in android/app/build.gradle

We only need exclude these dependencies if they are imported in android/app/build.gradle, similar to the sampleApp referenced in the library.

[rafecolton]

Nice, even better! So we can skip that part of the proposal entirely. @cretadn22 assigning you to implement the disableNFC parameter.

[rafecolton]

Not sure why the upwork automation isn't working 🤔

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021836544665597870593

[melvin-bot]

Current assignee @allgandalf is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

📣 @allgandalf 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @cretadn22 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[cretadn22]

@AndrewGable We're using "@onfido/react-native-sdk": "10.6.0," and we need to refer to the correct documentation for that version, as it differs from the current one.

Please visit https://github.com/onfido/react-native-sdk/releases/tag/v10.6.0 to download the zip file. Then, check the README file for the appropriate documentation version.

[cretadn22]

Sorry everyone, I should have investigated more carefully to provide this information earlier in my proposal to avoid any confusion.

[AndrewGable]

Ah nice find @cretadn22! 👍

[allgandalf]

PR approved from my side ♻️ , waiting on @rafecolton for their review 🙇

[rafecolton]

@strepanier03, making you issue owner now since the PR is merged

[allgandalf]

This should be ready for payment on 4th

[strepanier03]

Working on this now that I'm back from OoO and more caught up.

[strepanier03]

Okay, all contracts paid and closed. Thanks everyone!