search

tasitlabs / tasit-apps

Native mobile Ethereum dapps for mainstream users
https://tasit.io/
MIT License
36 stars 9 forks source link

Keep ephemeral account after app removal #289

Open marcelomorgado opened 5 years ago

marcelomorgado commented 5 years ago

Issue related by @pcowgill :

For some reason even after uninstalling the app on my iOS device and reinstalling it from TestFlight, it still was using the same account I had been using before, so I wasn’t able to demo the account setup steps on my physical device. I’m not sure why that would be. We should probably create an issue for this.

pcowgill commented 5 years ago

@marcelomorgado Can you reproduce this on Android?

marcelomorgado commented 5 years ago

I've trying to do that right now and just saw that the land for sale doesn't appear on the last version of the app. I guess that could be happening because of the config files. Maybe because of the build was made without current.js file generated?

pcowgill commented 5 years ago

@marcelomorgado Are you sure? There are a few estates showing up for me

marcelomorgado commented 5 years ago

@marcelomorgado Are you sure? There are a few estates showing up for me

I'll reinstall the app again.

pcowgill commented 5 years ago

@marcelomorgado Oh weird, after reloading the app they don’t appear anymore. Did you run a build or publish script recently? We should try to fix this as a top priority. I guess we should move this discussion outside of this issue since it’s not related.

marcelomorgado commented 5 years ago

Just finished the account persistence test. The account was deleted after reinstalling the app from Google Play.

pcowgill commented 5 years ago

Just finished the account persistence test. The account was deleted after reinstalling the app from Google Play.

Okay good, that’s as expected. I’ll try it again on iOS

pcowgill commented 5 years ago

Weird, the same thing happened again. I wonder if it’s an iOS 12 change. Maybe something specifically about SecureStore?

marcelomorgado commented 5 years ago

Weird, the same thing happened again. I wonder if it’s an iOS 12 change. Maybe something specifically about SecureStore?

Yes, seems that it's related to the key chain (refs) I'm looking for a way to solve it.

pcowgill commented 5 years ago

Weird, the same thing happened again. I wonder if it’s an iOS 12 change. Maybe something specifically about SecureStore?

Yes, seems that it's related to the key chain (refs) I'm looking for a way to solve it.

@marcelomorgado Thinking on this a little more, in a way it's actually nice if I don't lose my private keys just from uninstalling the app if I were using this in "production". So maybe we want to get Android to work this way too and then just offer an option to manually purge the account from within the app?

marcelomorgado commented 5 years ago

1) A solution to clear the account on iOS is: Since AsyncStore is clear on app removal, we can use it to store a flag to know when the app is running for the first time and delete pre-existent account from ScureStore.

2) Hmmm, it makes sense. I'll take a look at how we can do that. Note: Since some part of the state (e.g.: account creation status/actions) we'll need to recreate the AsyncStore on the first app use with an account already exists.

marcelomorgado commented 5 years ago

Here are what I've found until now:

pcowgill commented 5 years ago 
1. A solution to clear the account on iOS is: Since `AsyncStore` is clear on app removal, we can use it to store a flag to know when the app is running for the first time and delete pre-existent account from `SecureStore`.

Yep, that would work if we want to go this route!

2. Hmmm, it makes sense. I'll take a look at how we can do that.
   Note: Since some part of the state (e.g.: account creation status/actions) we'll need to recreate the `AsyncStore` on the first app use with an account already exists.

Hmm yes, this would be a bigger effort then - so maybe we go with option 1 and keep this as an separate issue? We'd have to get fancy about querying for all past transactions involving that account to rebuild the actions state OR keep everything in SecureStore, but that might not scale well.

pcowgill commented 5 years ago

Here are what I've found until now:

* Since Android 6, [some app data are also stored on the cloud](https://developer.android.com/guide/topics/data/autobackup.html) (allowBackup option on the manifest);

Hmm, we'll need to decide whether most users would want us to have this disabled or not.

* Seems that `Expo` has no way to set up the android backup but seems that is enabled by default ([refs](https://forums.expo.io/t/clear-storage-on-app-uninstall-android/5733));

https://expo.canny.io/feature-requests?search=clear+storage+on+app+uninstall

* Seems that the `SecureStore` are also backup-ed because it is stored on [`SharedPreferences`](https://docs.expo.io/versions/latest/sdk/securestore/) files.

Hmm, we'll need to decide whether most users would want us to have this disabled or not. In the meantime all we need to do is find any way for testers to be able to go through the onboarding flow again if they want.

* A way to go from here is to know better how the backup works and see if the account will be stored.

Yep, sounds good

marcelomorgado commented 5 years ago

Hmm yes, this would be a bigger effort then - so maybe we go with option 1 and keep this as an separate issue?

Yes, I agree. Extracted to: https://github.com/tasitlabs/tasit/issues/295 Renaming this issue (and moving to 0.2.0)

pcowgill commented 5 years ago

Thanks. Also noting that whenever we work on this issue, we'd want an in-app button to clear the account and start over.