Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent 2 multiaccounts on the same phone with same seed #8750

Closed
guylouis opened this issue Aug 14, 2019 · 7 comments
Closed

Prevent 2 multiaccounts on the same phone with same seed #8750

guylouis opened this issue Aug 14, 2019 · 7 comments

Comments

@guylouis
Copy link
Contributor

guylouis commented Aug 14, 2019

Relates to recovery #8687

Problem

A user can now recover a multiaccount on the phone or on a keycard.

For a given phone, a user can be in a situation where:

  • he has a multiaccount stored on keycard
  • then he tries to recover the same multiaccount on the phone

and the other way around:

  • he has a multiaccount stored on phone
  • then he tries to recover the same multiaccount on keycard

This situation does not make sense from a usage and security point of view. It defeats the purpose of Keycard.

Solution

At recovery stage, prevent the user to recover a multiaccount already in his list.

Next step

Design error messages and/or indication we want to provide to the user

The simplest thing to do is to just prevent it

We can also explain to the user what's going on and guide him through next steps depending on its intent:

  • if he had a keycard based multiaccount and wants to manage it without keycard, he must first reset his keycard multiaccount which is part of the settings for a keycard multiaccount. This deletes the multiaccount. [#7911 #7894] delete account and logout after keycard reset #7937
  • if he has a non-keycard based multiaccount. The user could either delete his multiaccount (does it exist in the UI ?) or use a "Migrate to Keycard" feature for non-Keycard multiaccount (it does not exist yet - but is in our backlog)

Question: can a user delete his non keycard multiaccount ?

cc @hesterbruikman @asemiankevich @dmitryn

@asemiankevich
Copy link
Contributor

i think we can't remove accounts now

@dmitryn
Copy link
Contributor

dmitryn commented Aug 14, 2019

Question: can a user delete his non keycard multiaccount ?

i think we can't remove accounts now

Yes, we don't support that afaik.

@hesterbruikman
Copy link
Contributor

When it comes to implementation it looks like this indeed relies on the option to Remove a non-keycard multiaccount which as @dmitryn added we don't support yet. Could this be an item for the Keycard backlog?

Like the idea of what we can offer when we detect a multiaccount is being recovered that already exists. We probably want to communicate the account already exists and offer to change storage.

I imagine this situation can occur in the following use cases:

  • user mixed up seed phrases and actually didn't want to 'duplicate' the account (I sure have more than enough lying around....)
  • user actually wants to select storage (in which case we probably want to offer this through the Account info screen and have recovery be a second entry point https://www.figma.com/file/XUehMnhyD1FGcWzvGz6SXqvh/Mobile-wallet?node-id=2559%3A33433
  • user wants to use the account with Keycard, but also wants to sign transactions with passcode alone. I can totally see this need becoming a thing once Keycard can be used at a POS and with Apple Card introducing this pay with/without card concept.

I understand the last use case is what we want to prevent because it defeates the security of Keycard. Still, I would say we probably want to look for ways to accommodate, e.g. by allowing the the account on both Phone and Keycard, With transactions on the phone having a transaction limit.

@guylouis
Copy link
Contributor Author

Ok thanks

The simplest v1 short term approach could be

  • remove non-keycard account : not possible (current implementation)
  • remove keycard account : possible (current implementation)
  • user tries to recover a multiaccount already on the phone - > we prevent that and show an error message
    i. if the multiaccount on the phone is a keycard one than display "You can't recover this key since it's already on this phone. If you want to use this key without a Keycard then first remove it in Keycard>Settings>Reset, and then recover it"
    ii. if the multiaccount on the phone is an non Keycard one "You can't recover this key since it's already on the phone"

And in v1 backlog, we'll add a "migrate to keycard' feature for non-keycard accounts. The message in ii, will then become "You can't recover this key since it's already on the phone. If you wish to migrate this key to your Keycard go to .../"Migrate account to Keycard"

@guylouis guylouis changed the title Several multiaccount on the same phone with same seed Prevent 2 multiaccounts on the same phone with same seed Aug 14, 2019
@rasom
Copy link
Contributor

rasom commented Nov 1, 2019

@hesterbruikman @guylouis @dmitryn @corpetty

i. if the multiaccount on the phone is a keycard one than display "You can't recover this key since it's already on this phone. If you want to use this key without a Keycard then first remove it in Keycard>Settings>Reset, and then recover it"

What if user forgot password or lost their card and still wants to use the app? I would suggest that we show an option to restore account but with removing all sensitive data (like chat history, contacts) if we consider that this data is sensitive. If user knows mnemonic they have control over acc anyway. Prohibition of recovering here doesn't really make sense as user still can re-install app and recover account without keycard. It will be inconvenient and annoying, but still doable.

Also there are at least four different scenarios related to this "recover existing account" thing, lets say keycard is KCA and on-device account is ODA, we have:

  • KCA -> KCA
  • KCA -> ODA
  • ODA -> KCA
  • ODA -> ODA

In each case it might be acceptable to allow the user to restore the existing account, although it is still a question if we can give access to accountэы data stored on the device. I think that it has to be removed before restoring, but probably I'm missing something.

The way how it works currently:

  1. KCA -> KCA
  • right after restoring acc the list of chats is empty (even if it wasn't :) )
  • after re-login with a new keycard with restored acc the user can see the chats list with all history and stuff (i can’t check if the old card is still working fine with that acc, because i have only one card already)

So my concern here is that we actually allow user to use a different card and get access to all data already existing on the device, when he hasn't provided neither old card nor password used before with this account. @yenda mentioned on discord that this is not an issue, but I'm not convinced yet. Though even if providing access to data is fine, we have to fix broken state after restoring, because everything works as expected only after-re-login.

  1. KCA -> ODA
  • we create a separate account definitely an issue

imo we can allow this if user lost access to their card, but we need to make sure that no extra data I leaked.

  1. ODA -> KCA
    Basically same as 2.. We also can propose user to move account to keycard (when such option will be implemented), but user would need to enter their password before doing this.

  2. ODA -> ODA

  • we open account with broken state
  • when user logins next time they need to enter the old password. A new one is not applied in any way.

Again, if user really wants to restore their acc, we should allow this (here it literally means changing password). But, no old password - no data.

@guylouis
Copy link
Contributor Author

guylouis commented Nov 4, 2019

My suggestion on what we need now (v1) and a way forward:

NOW (v1) the goal is to limit at the maximum the amount of work, while preserving our principles

  • we need to be able to remove a account from a keycard (it's a user right to control his secrets since he owns them, if he wants to delete it from this piece of hardware we have to allow him). I suggest that when the user does that, we remove the account on the phone too. Anyway, unless the user has a copy of its keycard, he cannot access it anymore. This issue is listed here Clean keycard and remove multi-account #9229
  • there is no way to remove a ODA

so it gives:
KCA->KCA: a user has lost his keycard and wants to create and wants to use his account again with a new keycard. The only thing he can do is to delete status, and start fresh with another card with his mnemonic
ODA->ODA: a user has lost his password on a ODA and wants to use his account. Not possible or the user needs to delete status totally and start fresh
KCA->ODA: a user is using a keycard but is fed-up and wants to use his account without keycard. User resets his keycard (needs #9229 to be implemented) and imports a new account with its mnemonic. Here there is no security issue since the previous account is deleted.
ODA->KCA: a user has a ODA buuys a keycard and wants to use his account with keycard. Not possible or user needs to delete status totally and start fresh

AFTER V1
We need UX team onboard to design the experience

A simple experience would be:m
KCA->KCA if user loses his card, he should be able to remove his account, and import it again from the mnemonic
ODA->ODA if user loses his ODA password, he could remove his ODA and import it again with the mnemonic. His data history could be retrieved if we implement database persistence. OR we need to implement a feature to recover a lost password of an ODA with the mnemonic.
KCA-> ODA same as now
ODA->KCA we can design an experience to the user to help him migrate his ODA to a keycard. This is issue #7134 (post v1)

@guylouis
Copy link
Contributor Author

guylouis commented Nov 6, 2019

after discussion with @rachelhamlin closing this one and following v1 approach described above
design work will be needed for post v1 approach

@guylouis guylouis closed this as completed Nov 6, 2019
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

No branches or pull requests

5 participants