react-native-keychain icon indicating copy to clipboard operation
react-native-keychain copied to clipboard

[Android] Replace `SharedPreferences` with `DataStore` Preferences

Open ovitrif opened this issue 1 year ago • 2 comments

Closes #628

This PR replaces the usage of SharedPreferences with DataStore.

Why

Google is recommending in their official documentation to migrate to DataStore from SharedPreferences, and they also provided a very easy-to-use auto-migration tool that will move the data to the new DataStore, then clean up the entries from SharedPreferences.

Their official guide for saving simple data with SharedPreferences displays an alert about this: Google Chrome 2024-03-14 001629

For security concerns as well, it is recommended to migrate to DataStore.

How

Since DataStore APIs are tailored for Kotlin, the code for using it is also done in Kotlin, thus introducing Kotlin to the project.

Calls to read and write to DataStore are forced to run synchronously based on the example provided by the Android team in Use DataStore in synchronous code.

Data Migration

SharedPreferencesMigration is used to migrate the entries stored in the RN_KEYCHAIN shared prefs file to data store. Upon completion, the SharedPreferences get removed.

Testing

There's an unit test added to check this, and I also tested it and it proved to work seamlessly in a React-Native app that is using this library, resulting in no data loss for the user or any issues when this gets upgraded.

Result

Before - SharedPreferences After - DataStore
Android Studio 2024-03-14 001625 Android Studio 2024-03-14 001625

ovitrif avatar Mar 14 '24 14:03 ovitrif

@oblador Any idea why the error about compileSdkVersion is not specified. Please add it to build.gradle in the unit tests?

Could it be a side-effect of adding Kotlin?! 🤷🏻

ovitrif avatar Mar 27 '24 16:03 ovitrif

@ovitrif Can you update this PR?

DorianMazur avatar Oct 21 '24 09:10 DorianMazur

@ovitrif Can you update this PR?

Updated sir, also now I could make use of the coroutineScope you introduced with the Kotlin migration, and hot reload works again 🎉 (was actually broken)

ovitrif avatar Nov 05 '24 15:11 ovitrif

@DorianMazur Please let me know if you're suggesting other updates 👍🏻

ovitrif avatar Nov 05 '24 15:11 ovitrif

FYI I found an issue that is not related to the datastore update but highlighted by it:

Unknown error with alias: pin, error: Cannot update the same operation concurrently. (Ask Gemini)
com.oblador.keychain.exceptions.CryptoFailedException: Unknown error with alias: pin, error: Cannot update the same operation concurrently.
	at com.oblador.keychain.cipherStorage.CipherStorageKeystoreAesCbc.encrypt(CipherStorageKeystoreAesCbc.kt:101)
	at com.oblador.keychain.KeychainModule$setGenericPassword$1.invokeSuspend(KeychainModule.kt:210)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:108)
	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:793)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:697)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:684)
Caused by: java.lang.IllegalThreadStateException: Cannot update the same operation concurrently.
	at android.security.KeyStoreOperation.handleExceptions(KeyStoreOperation.java:74)
	at android.security.KeyStoreOperation.update(KeyStoreOperation.java:118)
	at android.security.keystore2.KeyStoreCryptoOperationChunkedStreamer$MainDataStream.update(KeyStoreCryptoOperationChunkedStreamer.java:222)
	at android.security.keystore2.KeyStoreCryptoOperationChunkedStreamer.update(KeyStoreCryptoOperationChunkedStreamer.java:156)
	at android.security.keystore2.AndroidKeyStoreCipherSpiBase.engineUpdate(AndroidKeyStoreCipherSpiBase.java:436)
	at javax.crypto.Cipher.update(Cipher.java:1741)
	at javax.crypto.CipherOutputStream.write(CipherOutputStream.java:158)
	at javax.crypto.CipherOutputStream.write(CipherOutputStream.java:144)
	at com.oblador.keychain.cipherStorage.CipherStorageBase.encryptString(CipherStorageBase.kt:369)
	at com.oblador.keychain.cipherStorage.CipherStorageKeystoreAesCbc.encryptString(CipherStorageKeystoreAesCbc.kt:238)
	at com.oblador.keychain.cipherStorage.CipherStorageKeystoreAesCbc.encrypt(CipherStorageKeystoreAesCbc.kt:97)
	at com.oblador.keychain.KeychainModule$setGenericPassword$1.invokeSuspend(KeychainModule.kt:210) 
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33) 
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:108) 
	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584) 
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:793) 
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:697) 
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:684) 
	Suppressed: java.lang.IllegalThreadStateException: Cannot update the same operation concurrently.
		at android.security.KeyStoreOperation.handleExceptions(KeyStoreOperation.java:74)
		at android.security.KeyStoreOperation.finish(KeyStoreOperation.java:132)
		at android.security.keystore2.KeyStoreCryptoOperationChunkedStreamer$MainDataStream.finish(KeyStoreCryptoOperationChunkedStreamer.java:228)
		at android.security.keystore2.KeyStoreCryptoOperationChunkedStreamer.doFinal(KeyStoreCryptoOperationChunkedStreamer.java:181)
		at android.security.keystore2.AndroidKeyStoreCipherSpiBase.engineDoFinal(AndroidKeyStoreCipherSpiBase.java:626)
		at javax.crypto.Cipher.doFinal(Cipher.java:1957)
		at javax.crypto.CipherOutputStream.close(CipherOutputStream.java:210)

~~Working on a fix.~~

Fixed in: eb6df3b

ovitrif avatar Nov 05 '24 17:11 ovitrif

Great work @ovitrif Thank you! I'll check out that PR and merge it, probably this week.

DorianMazur avatar Nov 05 '24 19:11 DorianMazur