damus-io / damus

iOS nostr client
GNU General Public License v3.0
1.95k stars 290 forks source link

TBC: Prompt user to delete key from keystore on delete account #2185

Open alltheseas opened 3 weeks ago

alltheseas commented 3 weeks ago

what happens

When creating keys on Damus, keys are prompted to be saved on iOS passwords/keystore, on e.g. Bitwarden etc.

If the user decides to "delete" their account, and if the user has keys saved to a keystore, Damus does not currently delete keys in the keystore, and e.g. in bitwarden. It is unclear if damus can clear keys from keystores outside damus.

consider

Consider prompting the user to delete their keys from iOS keystore, other keystore apps as part of account deletion flow.

@ericholguin @robagreda how do yall think we might approach this?

How big of a problem is this? Is it a non-issue?

alltheseas commented 3 weeks ago

Discovered by @kernelkind

alltheseas commented 3 weeks ago

question

Should there be some warning / notification upon keystore save during Damus onboarding - e.g. "If you choose to store keys using iOS passwords, or another passwords app, Damus can not delete your nsec from such a keystore. If you would like to delete your keys, you will have to delete them in both 1) damus, and 2) the keystore app."

jb55 commented 3 weeks ago

On Wed, Apr 24, 2024 at 01:16:59PM GMT, alltheseas wrote:

what happens

When creating keys on Damus, keys are prompted to be saved on iOS passwords/keystore, on e.g. Bitwarden etc.

If the user decides to "delete" their account, Damus does not currently delete keys in the keystore, and e.g. in bitwarden. It is unclear if this damus can clear keys from keystores outside damus.

consider

Consider prompting the user to delete their keys from iOS keystore, other keystore apps as part of account deletion flow.

@ericholguin @robagreda how do yall think we might approach this?

How big of a problem is this? Is it a non-issue?

does anything else do this? usually your keys stay after the app is gone in case you reinstall later... I would find this behavior surprising.

alltheseas commented 3 weeks ago

does anything else do this? usually your keys stay after the app is gone in case you reinstall later... I would find this behavior surprising.

For comparison:

Twitter flow:

  1. save password to keystore
  2. hit account delete
  3. notification of 30 day permanent account deletion (in case user changes their mind)
  4. after 30 days twitter auto deletes account (i.e. data + keys)

comment: even if the twitter password is stored in keystore after 30 days, there is no longer any twitter account

Damus flow:

  1. save nsec to keystore
  2. hit "account delete"

Comment: 1) damus provides no notification, nor prompt regarding possibility of restoring the "account" using the stored nsec 2) if nsec is stored in keystore, the "account" can be restored, which does not imply successful "account deletion"

jb55 commented 3 weeks ago

On Wed, Apr 24, 2024 at 02:14:58PM GMT, alltheseas wrote:

does anything else do this? usually your keys stay after the app is gone in case you reinstall later... I would find this behavior surprising.

For comparison:

Twitter flow:

  1. save password to keystore
  2. hit account delete
  3. notification of 30 day permanent account deletion (in case user changes their mind)
  4. after 30 days twitter auto deletes account (i.e. data + keys)

comment: even if the twitter password is stored in keystore after 30 days, there is no longer any twitter account

Damus flow:

  1. save nsec to keystore
  2. hit "account delete"

Comment: 1) damus provides no notification, nor prompt regarding possibility of restoring the "account" using the stored nsec 2) if nsec is stored in keystore, the "account" can be restored, which does not imply successful "account deletion"

oh I see, that makes sense for deletion. This is one of those things where if someone wants to fix this they can, but I don't see it anywhere near the top of our priority list.

alltheseas commented 3 weeks ago

don't see it anywhere near the top of our priority list.

agreed on priority.

I capture here so we dont lose track of this issue.