martinkasa / capacitor-secure-storage-plugin

Capacitor plugin for storing string values securly on iOS and Android.
MIT License
153 stars 55 forks source link

Item with given key does not exist #39

Open t4tapas opened 3 years ago

t4tapas commented 3 years ago

Previously I created this issue for the following reason.

This plugin is getting the following error when minifyEnabled is set to true in Android.

This error is actually coming always whether or not minifyEnabled is true.

E/Capacitor: Unable to read file at path public/plugins
E/Capacitor/Plugin: Item with given key does not exist
    java.lang.Exception: Item with given key does not exist
        at com.whitestein.securestorage.SecureStoragePlugin.Z(:95)
        at com.whitestein.securestorage.SecureStoragePlugin.get(:45)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.getcapacitor.t0.h(:121)
        at com.getcapacitor.a0.D(:584)
        at com.getcapacitor.a0.E(Unknown Source:0)
        at com.getcapacitor.a.run(Unknown Source:8)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:246)
        at android.os.HandlerThread.run(HandlerThread.java:67)

Could you please share the ProGuard rule for this plugin which is needed for proguard-rules.pro file?

martinkasa commented 2 years ago

@t4tapas can you try it with the most recent version? I rebased the plugin with latest plugin template from Ionic, maybe they fixed it. I do not use ProGuard, so I can not tell.

Saqib92 commented 1 year ago

@t4tapas can you try it with the most recent version? I rebased the plugin with latest plugin template from Ionic, maybe they fixed it. I do not use ProGuard, so I can not tell.

Still happening on latest plugin v0.8.1.

rgbaman commented 1 year ago

Got the same issue here using the latest npm. Even catching the error with a console log breaks the entire app.

wellwoller commented 1 year ago

Is there any advancement on this issue?

NillDk commented 1 year ago

We experience the same error in our Angular Ionic/Capacitor App. Works on some phones (e.g. Samsung Galaxy A40) - fails on other phones of the same brand. For us, the issue seems to come from decryption which throws javax.crypto.IllegalBlockSizeException. We're using v0.8.1

W/System.err: javax.crypto.IllegalBlockSizeException
W/System.err:     at android.security.keystore.AndroidKeyStoreCipherSpiBase.engineDoFinal(AndroidKeyStoreCipherSpiBase.java:519)
W/System.err:     at javax.crypto.Cipher.doFinal(Cipher.java:2113)
W/System.err:     at com.whitestein.securestorage.PasswordStorageHelper$PasswordStorageHelper_SDK18.decrypt(PasswordStorageHelper.java:363)
W/System.err:     at com.whitestein.securestorage.PasswordStorageHelper$PasswordStorageHelper_SDK18.getData(PasswordStorageHelper.java:280)
W/System.err:     at com.whitestein.securestorage.PasswordStorageHelper.getData(PasswordStorageHelper.java:76)
W/System.err:     at com.whitestein.securestorage.SecureStoragePluginPlugin._get(SecureStoragePluginPlugin.java:105)
W/System.err:     at com.whitestein.securestorage.SecureStoragePluginPlugin.get(SecureStoragePluginPlugin.java:48)
W/System.err:     at java.lang.reflect.Method.invoke(Native Method)
W/System.err:     at com.getcapacitor.PluginHandle.invoke(PluginHandle.java:138)
W/System.err:     at com.getcapacitor.Bridge.lambda$callPluginMethod$0$com-getcapacitor-Bridge(Bridge.java:763)
W/System.err:     at com.getcapacitor.Bridge$$ExternalSyntheticLambda5.run(Unknown Source:8)
W/System.err:     at android.os.Handler.handleCallback(Handler.java:938)
W/System.err:     at android.os.Handler.dispatchMessage(Handler.java:99)
W/System.err:     at android.os.Looper.loop(Looper.java:246)
W/System.err:     at android.os.HandlerThread.run(HandlerThread.java:67)
W/System.err: Caused by: android.security.KeyStoreException: Unknown error
W/System.err:     at android.security.KeyStore.getKeyStoreException(KeyStore.java:1461)
W/System.err:     at android.security.keystore.KeyStoreCryptoOperationChunkedStreamer.doFinal(KeyStoreCryptoOperationChunkedStreamer.java:186)
W/System.err:     at android.security.keystore.AndroidKeyStoreCipherSpiBase.engineDoFinal(AndroidKeyStoreCipherSpiBase.java:506)
W/System.err:   ... 14 more
E/Capacitor/Plugin: Item with given key does not exist
    java.lang.Exception: Item with given key does not exist
        at com.whitestein.securestorage.SecureStoragePluginPlugin._get(SecureStoragePluginPlugin.java:112)
        at com.whitestein.securestorage.SecureStoragePluginPlugin.get(SecureStoragePluginPlugin.java:48)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.getcapacitor.PluginHandle.invoke(PluginHandle.java:138)
        at com.getcapacitor.Bridge.lambda$callPluginMethod$0$com-getcapacitor-Bridge(Bridge.java:763)
        at com.getcapacitor.Bridge$$ExternalSyntheticLambda5.run(Unknown Source:8)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:246)
        at android.os.HandlerThread.run(HandlerThread.java:67)
NillDk commented 1 year ago

Another interesting finding.

We have 2 values in our storage:

Case 1: We uninstall + reinstall the App, the OldKey persists and NewKey is gone.

Case 2: We call SecureStoragePlugin.clear() both keys/values are gone. We the kill the App and reopen it, both keys/values are still gone. However, if we then uninstall + reinstall the App. The OldKey reappears.

Case 3: The value of OldKey is 'AAA' (the one that throws). We call SecureStoragePlugin.set({ OldKey, 'BBB'}) We uninstall + reinstall the App. The value of OldKey is now 'AAA' (expected to be completely gone due to the uninstall/reinstall).

@martinkasa do you have any idea on what's going on here? Could the KeyStore be distributed to some external storage and read from there? Or do you have any other suggestions to, why the old value reappears?

Would you be interested in a screen sharing / remote debug session?

alexcroox commented 8 months ago

This issue still exists in the latest version. The only work around is to get the key first, which does correctly throw:

      try {
        await this.$native.secureStorage.get({ key: 'accessToken' })

        // This cannot be caught so we need to check above first
        this.$native.secureStorage.remove({ key: 'accessToken' })
      } catch (error) {
        // STUB
      }
GuillaumeUnice commented 8 months ago

Is there any enhancement here?

@alexcroox I don't get your point could explain to me?

I have the impression when we send multiple get in parallel there is an asynchronous issue otherwise seems to work.

alexcroox commented 8 months ago

@GuillaumeUnice previously I had just .remove in a try/catch in case the key didn't exist. But .remove doesn't throw an exception so I couldn't catch it and the app produced an error I couldn't silence.

However .get does throw an exception if the key doesn't exist, so you have to do .get first before .remove to avoid errors bubbling that you can't catch.

GuillaumeUnice commented 7 months ago

Ok just to sum up the problem because everything as diverged from the initial issue. The problem is related to the use of .get and .remove which throw item with given key does not exist when the key doesn't exist whereas the error should be on the catch of the .get or .remove promise because here we can't handle it and manage it at all (currently the throw is on the capacitor internal callback system).

The only solution to handle it for now is to use other method for instances use .keys to check before calling .get and remove but it's kinda workaround so whether someone can handle the .getand .remove error more properly it will be more suitable in use.

RichardScholtenAareonNL commented 3 days ago

We are running in to the same problem where an error is thrown if a key does not exist.

We do not call the methods like getItem setItem by ourselves, we pass the securestorage to a 3rd party package with the Storage interface. The external package stops working whenever an error is thrown, this is not in our control.

Throwing an error if a key does not exist is not compliant with the specs: https://html.spec.whatwg.org/multipage/webstorage.html#dom-storage-getitem