planetary-social / nos

nos.social social media for all of us, using nostr
https://nos.social
Mozilla Public License 2.0
122 stars 14 forks source link

[Account Deletion] Support account deletion #80

Open mplorentz opened 1 year ago

mplorentz commented 1 year ago

Overview : In order to launch in the app store we need to support account deletion. https://developer.apple.com/support/offering-account-deletion-in-your-app/

Given that I am a user of Nos When I view the Settings screen and scroll to the bottom Then I should see a Delete my account button

Designs: https://www.figma.com/design/s0qf4VmyQygydP8MIQazZc/Nos?node-id=8661-17604&t=fukd9IBDjjRQAu7Y-1

Given that I am a user of Nos When I click Delete my account Then a confirmation dialogue opens with the following content:

Permanently Delete Account Deleting your account will delete all your data from Nos servers and ask all your relays to do the same. Cancel | Delete

Designs https://www.figma.com/design/s0qf4VmyQygydP8MIQazZc/Nos?node-id=8661-17604&t=fukd9IBDjjRQAu7Y-1

Given that I am a user of Nos When I click delete Then I am prompted to type "DELETE ACCOUNT" before my account is permanently deleted. And Nos sends a message to the Nostr network to delete all the notes associated with this account.

Given that I am a user of Nos When I type "DELETE ACCOUNT" and press the Delete button Then Nos sends a message to the Nostr network to delete all the notes associated with this account, erases my bio, publishes an empty follow list, changes my name to "Account Deleted", deletes my data from the local database, and takes me back to onboarding.

Given that I am a user of Nos When I click cancel on the delete dialogue Then i am returned to the settings page.

setch-l commented 10 months ago

@mplorentz @dcadenas - @dcadenas is working on deleting nIP05 ID - will that be enough or do we need some other deletion mechanism beyond that?

mplorentz commented 10 months ago

We'll need more than that. Probably a button in the Nos settings that will publish a delete message for all your existing notes.

mplorentz commented 2 months ago

Vitor has proposed a NIP for this: https://github.com/nostr-protocol/nips/pull/1256

bryanmontz commented 1 month ago

@mplorentz @dcadenas

  1. We will publish a deletion request event for each note we have from the user. What if this amounts to hundreds or thousands of events?
  2. Will there be an internal API to call for information we store aside from Nostr events?
  3. Should we go ahead and implement the "request to vanish" as well, or do we need to wait until Vitor's proposal is merged?
mplorentz commented 1 month ago

@bryanmontz @setch-l

I see two sane ways to approach this:

  1. The Damus way: which IIRC basically adds a deleted: true flag to the profile metadata and changes the user's name to "account deleted" but doesn't delete any notes.
  2. The proposed NIP-62 "right to vanish" NIP where you publish one event that is a signal you want anyone who sees it to drop all your data. (probably should also implement #1 in this case)

Trying to fetch all the user's notes and publish individual delete requests for each would take several minutes or hours (due to relay rate limits) and doesn't seem viable.

Option #1 feels like it isn't what users want, and probably only passed App Store review because the reviewer didn't look closely. But it has the advantage of being easy to implement.

Option #2 feels like the right way but AFAIK it hasn't been implemented in any relays. We would at least need to add support for this NIP to strfry and possibly nostr-rs-relay and our other services like followers and push notifications. I think if we implement this Vitor will too and we'll have no problem getting the NIP merged.

@setch-l I'll leave it up to you which option we go with. @bryanmontz I think this answers all your questions but if not let me know.

setch-l commented 1 month ago

@bryanmontz @mplorentz - We should go with option 2 and implement NIP62. I assume this also involves work for Daniel as well as we need to implement across our services?

dcadenas commented 1 month ago

I just want to clarify that for the option of a slow per note deletion of all their notes, we could have a service that uses NIP26 to ask for short lived authorization. If the user accepts, we, as a server, can remove their notes in the background and not done through the iOS app. Still not ideal but just to note the possibility exists.

For #2, even if relays don't implement it directly, we can have a service living next to the relay with access to the DB that listens for NIP62 events, if it find one, it uses direct DB access to delete the user events. I would need to explore this further but it seems possible

mplorentz commented 1 month ago

@Chardot @setch-l I think we talked in design review about having the user type something before deleting all their data. Or maybe we just add a "Are you really sure" dialog after the first one. Two taps to delete everything seems way too easy to accidentally do.

Is that design change going to happen or are we going forward with a single button?

mplorentz commented 1 month ago

@bryanmontz I just thought of another TODO. Let's make sure the NIP-62 event is always published to relay.nos.social - even if it isn't in the user's relay list. This will ensure that our servers see the request and can delete their data across all our web services: the relay, our push notification database, the follow database, etc.

bryanmontz commented 1 month ago

@setch-l Matt gave some suggestions in one of my PRs related to this task that we could use your input on:

I think we should overwrite the user's kind 0 and kind 3. Maybe we should check with Linda on this one, but I think publishing an empty profile that says "Account deleted" in the name fields or something would go a long way towards signaling the user's intentions as much as possible, especially for relays and clients that don't support NIP-62. We could also publish an empty kind 3.

setch-l commented 1 month ago

@bryanmontz @mplorentz - Yes I agree we should show Account Deleted in the profile.

Chardot commented 1 month ago

@Chardot @setch-l I think we talked in design review about having the user type something before deleting all their data. Or maybe we just add a "Are you really sure" dialog after the first one. Two taps to delete everything seems way too easy to accidentally do.

Is that design change going to happen or are we going forward with a single button?

Yes, I'm updating the design by adding the text input to the confirmation dialog. I'll post it in a few minutes

Chardot commented 1 month ago

@mplorentz @setch-l Here's the updated design with a text input in the confirmation dialog: https://www.figma.com/design/s0qf4VmyQygydP8MIQazZc/Nos?node-id=8661-17533&t=u5zw7bg5eUGMXsT1-1

It's split in two flows:

  1. User deletes her account Screenshot 2024-09-20 at 20 29 33

  2. Failed to delete account Screenshot 2024-09-20 at 20 29 41

@mplorentz let me know if you see something that wouldn't be challenging to implement. I'm not sure what are the possibilities for customizing a native alert like this.

mplorentz commented 1 month ago

@Chardot thanks, this looks good!

mplorentz commented 3 weeks ago

We need to make sure we invalidate the APNS token when deleting the account (and on logout). I'll do that when I'm completing this ticket.

This is more work than I thought. I have planned it out and filed https://github.com/planetary-social/nos/issues/1613 and https://github.com/planetary-social/nos-notification-service-go/issues/70 to complete it separately.

mplorentz commented 3 weeks ago

@setch-l I updated the ticket description to reflect the latest designs. Please tweak it if you see anything off.

@pelumy this has been mostly implemented. The main change we need you to make is to modify the confirmation dialog for deleting and account. Currently you can delete your account in two taps, but we want to add a step where the user types the string "DELETE ACCOUNT" as shown in Figma.

I have an outstanding PR for this ticket that removes the feature flag. I'll be merging that soon so we can submit to app store review. You probably want to branch off of that branch if you start work on this before it is merged.