mCodex / react-native-sensitive-info

Save sensitive data into Android's Shared Preferences with keystore encryption/iOS's Keychain for React Native
https://mcodex.dev/react-native-sensitive-info/
MIT License
979 stars 216 forks source link

[v6.0.0] Discussions and proposals #196

Open mCodex opened 4 years ago

mCodex commented 4 years ago

Since v6.0.0 the lib should use Android's Keystore to encrypt data for better security purposes before saving it into shared preferences.

The problem

Data stored using RNSInfo's versions > 6 was saved in Android's shared preferences without keystore encryption then releasing v6.0 these data could be lost?

On the other hand, data stored using keystore's branch wouldn't be affected by v6

Checklist

kneza23 commented 4 years ago

Official RN docs included your library to the Security page https://reactnative.dev/docs/security. It would be amazing if eventually we get Android's Keystore as default storage :)

mCodex commented 4 years ago

@kneza23 thanks for pointing it out! I didn't know 😱 Yeah, I think it is time to do this migration

mCodex commented 4 years ago

This afternoon I will create a new branch from master and I'll merge keystore into that branch. So, we gonna have a starting point for this 👏 👏 👏

--- Edit ---

I've merged everything into feature/keystoreAsDefault.

mCodex commented 4 years ago

There is a new release available in npm which includes keystore by default. For more information, please read here.

juanapp commented 4 years ago

Is there a solution for data migration? I would love to integrate this version but I'm also afraid that it will cause quite some troubles to users that have data saved..

mCodex commented 4 years ago

Is there a solution for data migration? I would love to integrate this version but I'm also afraid that it will cause quite some troubles to users that have data saved..

Unfortunately there is not. Maybe we need an extra method to read from sharedPreferences without using keystore something like getAllUncryptedDataFromSharedPreferences and then every user should handle saving all these infos again using setItem and now everything will be encrypted in Android.

juanapp commented 4 years ago

@mCodex That would be ideal... I think what we need is to have a way to trigger the migration from everything that is stored on SharedPreferences to the KeyStore, keeping the same structure and then deleting it from SharedPreferences. That would save the hassle of the first time run after upgrading.

mCodex commented 4 years ago

@mCodex That would be ideal... I think what we need is to have a way to trigger the migration from everything that is stored on SharedPreferences to the KeyStore, keeping the same structure and then deleting it from SharedPreferences. That would save the hassle of the first time run after upgrading.

Yeah, I couldn't agree more. This is why v6.0 is yet in alpha release. I don't want to developers updating RNInfo and their users losing data. On the other hand, there is a work to do to handle this scenario but no one is taking care of it right now. I'll see if I can code something to speed up this release.

kgsachinthaudara commented 4 years ago

@mCodex when are your hope to release v6, we are waiting for great news ❤️

idanlevi1 commented 3 years ago

@mCodex Can you write the changes between alpha versions? and the alpha version is stable to use in production new?

Marvedog commented 3 years ago

@mCodex That would be ideal... I think what we need is to have a way to trigger the migration from everything that is stored on SharedPreferences to the KeyStore, keeping the same structure and then deleting it from SharedPreferences. That would save the hassle of the first time run after upgrading.

Yeah, I couldn't agree more. This is why v6.0 is yet in alpha release. I don't want to developers updating RNInfo and their users losing data. On the other hand, there is a work to do to handle this scenario but no one is taking care of it right now. I'll see if I can code something to speed up this release.

Do you need any help with this?

@mCodex

mCodex commented 3 years ago

@mCodex That would be ideal... I think what we need is to have a way to trigger the migration from everything that is stored on SharedPreferences to the KeyStore, keeping the same structure and then deleting it from SharedPreferences. That would save the hassle of the first time run after upgrading.

Yeah, I couldn't agree more. This is why v6.0 is yet in alpha release. I don't want to developers updating RNInfo and their users losing data. On the other hand, there is a work to do to handle this scenario but no one is taking care of it right now. I'll see if I can code something to speed up this release.

Do you need any help with this?

@mCodex

Hi Marcus, of course! Being honest I still haven't looked into this yet.

PRs are always welcome.

Marvedog commented 3 years ago

@mCodex I'll take a look then

aarondail commented 3 years ago

In lieu of some kinda wholesale migration to deal with old unencrypted values in shared preferences, we have cooked up something super simple for our app: when we call getItem() we wrap it in a try/catch and (if it fails) then call getItem() again with a { dontDecrypt: true } option we patched into the library.

The patch is just a one line change to RNSensitiveInfoModule's getItem() method:

    @ReactMethod
    public void getItem(String key, ReadableMap options, Promise pm) {
        // ...omitted...
        if (value != null && options.hasKey("touchID") && options.getBoolean("touchID")) {
            // ...omitted...
//****
// The following line is the only one we changed (it used to be just else if (value !== null) {)
        } else if (value != null && !(options.hasKey("dontDecrypt") && options.getBoolean("dontDecrypt"))) {
//****
            try {
                pm.resolve(decrypt(value));
            } catch (Exception e) {
                pm.reject(e);
            }
        } else {
            pm.resolve(value);
        }
    }

Again: super simple. Maybe too simple... and it does require the caller to do some work so still a breaking change. This could be made completely transparent (to callers) by dropping the dontDecrypt option we added and instead changing pm.reject(e); to pm.resolve(value) in the function I pasted above... though that may be dissatisfying too. Definitely not as satisfying as a real wholesale migration. But if this or some variant of it seems like something you'd like to have in the library I am happy to submit a PR.

kneza23 commented 3 years ago

Hi guys. Any news or estimates on this?

kam89 commented 3 years ago

Hi, is it possible to include fail attempt for each biometric authentication for both Android and iOS?

fedeerbes commented 3 years ago

Hi, is it possible to include #264 as well? That will greatly improve the user experience.

MaxChenMC commented 3 years ago

Hi. May I know whether 6.0.0 supports Face Recognition for Android?

JeffreyLeeDave commented 3 years ago

I would like to propose a delete all function

ShivamJoker commented 2 years ago

Is macOS support added ?

davux commented 2 years ago

Are there any news on the data migration part?

Version 6.x is documented as stable but it looks like it's actually still considered alpha because of the data migration. So I think many projects are stuck with 5.x until there is either a silent migration or at least some workaround to ensure no data is lost.

mikeschoneveld commented 2 years ago

Are there any news on the data migration part?

Version 6.x is documented as stable but it looks like it's actually still considered alpha because of the data migration. So I think many projects are stuck with 5.x until there is either a silent migration or at least some workaround to ensure no data is lost.

Any update on the issue? The data migration is a very important part for us because we don't want all our users to login again.