[android] apply Network Security Config file (fixes #22375) (part 2 of #23105)

[Salakar]

This is a follow-up PR for #23105 - as mentioned on discord.

---

This PR applies the network security config for the RN template project only. New RN projects started with the updated template will be able to connect to the packager on builds built with Android API 28 & above.

See https://developer.android.com/training/articles/security-config#CleartextTrafficPermitted for more information about this newly required config, specifically:

Changelog:

[ANDROID] [Template] add Network Security Config file to allow access to packager via cleartext requests in Android API 28 and above. (fixes #22375)

Test Plan:

Before change:

After change:

[pull-bot]

	Warnings
⚠️	📋 Missing Summary - Can you add a Summary? To do so, add a "## Summary" section to your PR description. This is a good place to explain the motivation for making this change.
⚠️	📋 Missing Test Plan - Can you add a Test Plan? To do so, add a "## Test Plan" section to your PR description. A Test Plan lets us know how these changes were tested.
⚠️	📋 Missing Changelog - Can you add a Changelog? To do so, add a "## Changelog" section to your PR description. A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

Generated by 🚫 dangerJS

[hey99xx]

@Salakar thanks for making progress and also mentioning me.

Now what you do is use android:networkSecurityConfig="@xml/rn_network_security_config" in ReactAndroid/src/debug/AndroidManifest.xml. This is not necessarily wrong, but I don't think it will work as you intended.

This is because when you install react-native from NPM, it comes with only one AAR file, the one named react-native-0.56.1.aar somewhere inside in node_modules/react-native. It is possible to release two separate AARs for debug and release but it's usually cumbersome for consumers and it sometimes creates issues with transitive dependencies.

I'm not going to suggest editing template/android/app/src/main/AndroidManifest.xml either, because that applies to both release and debug APKs and we wouldn't want to expose localhost in production.

Instead you can move

• ReactAndroid/src/debug/AndroidManifest.xml into template/android/app/src/debug/AndroidManifest.xml and
• ReactAndroid/src/main/res/devsupport/xml/network_security_config.xml into template/android/app/src/debug/res/xml/network_security_config.xml

Basically we do the same thing described in https://stackoverflow.com/a/46321866 Note that this solution obviously does not work for brownfield apps that wasn't started from the helloworld template in this repo, so they would have to manually add those 2 files. I think that's acceptable, but I'm not authority here :)

[Salakar]

Ah damn, @hey99xx you're right again 🙈 this change would only apply when using RN from source like I was locally, completely forgot it gets distributed as an AAR - which this change won't apply to the bundled AAR that gets publish alongside on NPM >.>

I can apply it to the template but like you said this only works for apps that began with this template after the change.

I may have an alternative though; I can move it to the main manifest and wrap the security config in a <debug-overrides> block: https://developer.android.com/training/articles/security-config#debug-overrides - so it'll only apply to debug still

[hey99xx]

I can move it to the main manifest and wrap the security config in a <debug-overrides> block

You can try, but as far as I recall it won't work, I've tried it myself in the past 🙃. <debug-overrides> can only be used to override <trust-anchors> block inside. I wish it worked according to the common intuition like you have.

Maybe you can ask in the Discord channel for alternative solutions (such as releasing debug and release AARs, but I feel it's too much trouble for this issue). Unfortunately I don't know a better way. As long as the solution is mentioned in Android tab at https://facebook.github.io/react-native/docs/integration-with-existing-apps I personally think it's ok.

[Salakar]

😭 right, ok. I will ask about the AARs but I also feel it's far too much trouble.

I think this may just be a case of applying this to the template project & tests project only and then pair with some documentation for non-template based apps.

[cpojer]

@Salakar do you have any update on this PR?

[Salakar]

@cpojer sorry for the delay getting back to you, 🙈I've been been held up with prior OSS commitments this week - I've not forgotten I owe you a writeup for this as per Discord.

Will get this done by Friday latest 👍

[Salakar]

Hey @cpojer;

I've updated this to apply only to the template project. As discussed with @hey99xx above, this can't be applied to RN itself as RN is distributed as a single aar and distributing multiple aars (e.g. debug/release variants) is probably a bit OTT for this issue and could potentially cause more problems than it's worth.

I've also updated the PR description to reflect the above + added testing screenshots.

I will also follow up with a separate docs PR to https://facebook.github.io/react-native/docs/integration-with-existing-apps - to document this for existing apps that aren't using the template.

[dulmandakh]

how about having single config file, but use debug-overrides in it as described in https://developer.android.com/training/articles/security-config. so it'll be empty for non-debug builds.

[Salakar]

@dulmandakh thanks for the suggestion, see the comments above, specifically;

I can move it to the main manifest and wrap the security config in a <debug-overrides> block

You can try, but as far as I recall it won't work, I've tried it myself in the past 🙃. can only be used to override <trust-anchors> block inside. I wish it worked according to the common intuition like you have.

[cpojer]

Thanks for making this change, and yeah it seems like we need to make sure this is applied to new templates going forward.

[facebook-github-bot]

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[react-native-bot]

@Salakar merged commit 84572c4 into facebook:master.