Skip to content

Conversation

@mattlennon3
Copy link

This MR makes it possible to store an array of items in the keychain.

Reason for MR

I am raising this MR because I wanted to migrate our project from using react-native-sensitive-info to using react-native-keychain. This repo is actually maintained and has a larger community using it. Also the sensitive info library seems to stop working after upgrading RN 0.76.1+ mCodex/react-native-sensitive-info#424 (comment). So, time to move on!

However, during testing I found that this library only returns a single object from the keychain. Which seems to be intended looking at comments here: #36 (comment), I should be serialising my data structure and storing it as a string in this single object.

This would be fine if I was starting a new project, but losing access to data already in the keychain was not acceptable as we would lose important login keys etc. So I decided to convert the functionality over to this library. Hopefully this will help others convert to this library and add some nice new functionality for existing users.

Changes

I have added these functions: getItemForKey getAllItems setItemForKey removeItemForKey clearItems.

Of course a requirement for me was to mirror as best as possible the behaviour of the functions in react-native-sensitive-info. So I can preserve my data after migrating to this library.

I feel like the iOS implementation fits in nicely with the existing code. But I will admit that the Android side was more complex, you guys have a DataStorePrefsStorage class that I wasn't sure how exactly to integrate with, so I played it safe and kept to a simple implementation within the main KeychainModule. Would appreciate any pointers for this.

Thanks and any feedback is welcome 🙏

@DorianMazur
Copy link
Collaborator

DorianMazur commented Aug 27, 2025

Hey @mattlennon3
Thank you for the PR.
I don't understand one thing, why do we need this? It's already possible to save multiple items, just JSON.stringify them.
Instead of mirroring react-native-sensitive-info and adding unnecessary methods, I would create some migration script.

@RomanSytnyk
Copy link

It can be very useful for migration from mCodex/react-native-sensitive-info to this library

@mattlennon3
Copy link
Author

mattlennon3 commented Sep 24, 2025

Hi @DorianMazur , thanks for the response. Sorry I've not added any README changes or other documentation yet, I've had to put my upgrade on hold to fix some other issues. But I'm ramping back up now.

To your question, because of the limited retrieval functionality I believe a migration step is not possible, as I alluded to in my description:

However, during testing I found that this library only returns a single object from the keychain.

I am referring to getGenericPassword here, which only returns the first item in the array of saved passwords. If this library had a getAllPasswords type function, I could write a migration step, but it doesn't.

There is also no period where we can have both react-native-sensitive-info and this library installed. We have to take a step from the old library, to this new one. Because of this:

the sensitive info library seems to stop working after upgrading RN 0.76.1+

So if you want, I could refactor this PR to have the ability to return the array of passwords, but not set them. Then a migration step is possible. Or we can keep the full functionality of updating the array of passwords, which I do think is valid as it prevents this arguably "heavy" serialising/deserialising of data.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants