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

[v6.0.0] Discussions and proposals #196

Open
5 of 7 tasks
mCodex opened this issue Jun 11, 2020 · 22 comments
Open
5 of 7 tasks

[v6.0.0] Discussions and proposals #196

mCodex opened this issue Jun 11, 2020 · 22 comments

Comments

@mCodex
Copy link
Owner

mCodex commented Jun 11, 2020

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

  • Improve documentation
  • Create a branch which merges master and keystore
  • Release an alpha version
  • Add support for tvOS and MacOS in react-native-sensitive-info.podspec
  • Merge everything into master
  • Fix macOS build error MacOS build failing #222
  • Find a way to automatically migrate data from shared preferences to keystore? (should the library itself handle this?)
@mCodex mCodex pinned this issue Jun 23, 2020
@stale stale bot added the 🚧 stale label Jul 20, 2020
Repository owner deleted a comment from stale bot Jul 21, 2020
@stale stale bot removed the 🚧 stale label Jul 21, 2020
@kneza23
Copy link

kneza23 commented Jul 24, 2020

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
Copy link
Owner Author

mCodex commented Jul 28, 2020

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

@mCodex
Copy link
Owner Author

mCodex commented Jul 28, 2020

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
Copy link
Owner Author

mCodex commented Jul 28, 2020

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

@mCodex mCodex changed the title Use keystore as default secure storage for Android [v6.0.0] Discussions and proposals Jul 29, 2020
@juanapp
Copy link

juanapp commented Aug 18, 2020

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
Copy link
Owner Author

mCodex commented Aug 18, 2020

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
Copy link

juanapp commented Aug 18, 2020

@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
Copy link
Owner Author

mCodex commented Aug 18, 2020

@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
Copy link

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

@idanlevi1
Copy link

idanlevi1 commented Nov 12, 2020

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

@Marvedog
Copy link

Marvedog commented Feb 5, 2021

@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
Copy link
Owner Author

mCodex commented Feb 28, 2021

@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
Copy link

Marvedog commented Mar 1, 2021

@mCodex I'll take a look then

@aarondail
Copy link

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
Copy link

kneza23 commented Apr 22, 2021

Hi guys. Any news or estimates on this?

@kam89
Copy link

kam89 commented May 4, 2021

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

@fedeerbes
Copy link

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

@MaxChenMC
Copy link

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

@JeffreyLeeDave
Copy link

I would like to propose a delete all function

@ShivamJoker
Copy link

Is macOS support added ?

@davux
Copy link

davux commented Apr 1, 2022

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
Copy link

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.

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

No branches or pull requests