Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

chore: set storage not 'migrable' #672

Merged
merged 1 commit into from
Jun 16, 2021
Merged

chore: set storage not 'migrable' #672

merged 1 commit into from
Jun 16, 2021

Conversation

patogallaiovlabs
Copy link
Collaborator

@patogallaiovlabs patogallaiovlabs commented Apr 29, 2021

Issue
When the users resets/change phone, all data is migrated to the new phone, when for security reasons it shouldn't. The private data should 'live' only on the scope of the current device.

Tech description
rWallet uses lib rn-secure-storage for saving data in the device, from their docs they have some ways to set how to save data.
To fix it lets change to:

  • WHEN_UNLOCKED_THIS_DEVICE_ONLY: The data in the keychain item can be accessed only while the device is unlocked by the user. Items with this attribute do not migrate to a new device.

What do we store?

  • Passcode
  • Mnemonic phrase
  • Private key
  • User password

How to test it

  • Install the app in a new device with apple account set.
  • Create a new wallet with new seed.
  • Create a new device.
  • Import apple account.
  • When opening the app, we shouldn't get any wallet already created.

@patogallaiovlabs patogallaiovlabs marked this pull request as ready for review May 17, 2021 21:29
@patogallaiovlabs patogallaiovlabs modified the milestones: v1.4.4, v2.0.0 May 17, 2021
Copy link
Contributor

@martinmedina martinmedina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look good, definitely a security improvement to what we had before.
I tested on a device the following cases

  1. install rwallet local build, create a wallet, backup seed, delete app, reinstall app and no data was available on the new install.
  2. install rwallet local build, create a wallet, backup seed, backup full device on icloud, wipe device, restore backup from icloud on the same device and data was available on the new install.
    This case worked as expected since the data is available if the backup is restored on the same device.

A test that could not be done is case 2 but restoring the backup on a new device. For that case, the data should not be available.

As mentioned before, code looks good and since there is no way to do the remaining test on the short term I'm approving this PR.

@patogallaiovlabs patogallaiovlabs merged commit ea7421a into master Jun 16, 2021
@patogallaiovlabs patogallaiovlabs deleted the storage_fix branch June 16, 2021 12:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants