artsy / eigen

The Art World in Your Pocket or Your Trendy Tech Company's Tote, Artsy's mobile app.
MIT License
3.57k stars 576 forks source link

[RFC] Add react-native-config (forked) dependency #3724

Closed ashfurrow closed 4 years ago

ashfurrow commented 4 years ago

New Dependency

Name: react-native-config (forked, for now)

URL: https://github.com/artsy/react-native-config

Motivation

Currently, Eigen uses cocoapods-keys to store environment configuration for the app (the list of used keys is here). CocoaPods only works for iOS apps, and we are planning in MX-489 to migrate to a cross-platform solution, to support upcoming Android work.

react-native-config is a good solution. However, it lacks one important feature from cocoapods-keys: key obfuscation. Please note that it is impossible to ship a binary to users that guarantees security of the keys, but key obfuscation makes it much more difficult to extract them. Unobfuscated keys are present as plaintext in the app binary, which is not acceptable. We have forked react-native-config and ported the obfuscation code from cocoapods-keys in this PR. Eventually, we'll need to recreate that logic for the Android app too.

A big upside to using react-native-config over cocoapods-keys is that we will migrate to storing local keys in a .env file, just like most other apps at Artsy. This will reduce the overhead for new contributors to the app. We will need to change our beta deploys to write the environment variables to a .env file prior to deploying.

Check List

Alternatives

We chatted about alternatives in MX Knowledge Share yesterday, and nothing came up.

admbtlr commented 4 years ago

react-native-config is nice, so def 👍 on that.

Are you planning to create a PR with the obfuscation code on the main repo? It would be good not to have to maintain a fork. Ah, just read the PR in the forked repo. Nice!

pvinis commented 4 years ago

nice touch with the obfuscation.

ashfurrow commented 4 years ago

Resolution

We decided to adopt the dependency.

Level of Support

1: Overwhelming positive feedback.

Additional Context:

None

Next Steps

Let's merge https://github.com/artsy/eigen/pull/3725 and then announce.

Exceptions

None.