firebase / FirebaseUI-Flutter

Apache License 2.0
101 stars 91 forks source link

[firebase_ui_auth] Delete account needs confirmation dialog. #40

Closed lesnitsky closed 9 months ago

lesnitsky commented 1 year ago

When tapping on "Delete account" button there's no confirmation dialog that probably should be present for such a destructive action

graemep-nz commented 1 year ago

I think some more thought is needed on what to do about this. Firstly I'm not sure that profile_screen.dart will be used very much as it stands because usually there is more to a person's profile than just a name - but I could be wrong. In my case I will want to use it as an "account admin" and email verification page rather than a profile page so I would need the ability to not have the avatar and username stuff at the top. I will probably end up customizing profile_screen.dart for my own use. So then, regarding "delete account", in my case I will want to warn the user about losing their data (firestore and cloud storage) but the best workflow is possibly

  1. Initial user confirmation of deletion (maybe not needed)
  2. Reauthenticate if necessary
  3. Execute a callback that allows people to display their own warning dialog - this might involve actually deleting stuff from firestore etc.
  4. Delete the account if the callback said to
lesnitsky commented 1 year ago

I'm not sure that profile_screen.dart will be used very much

Firebase UI allows usage of lower level widgets, so you can easily build your custom profile screen using exported widgets:

  1. Initial user confirmation of deletion (maybe not needed)

this is exactly what this issue is about

  1. Reauthenticate if necessary

this is a current behaviour

  1. Execute a callback that allows people to display their own warning dialog

This is planned. Firebase UI provides a default behaviour for most of the paths, but it is possible to override everything and provide custom handlers.

graemep-nz commented 1 year ago

Yes I know reauthentication is a current behaviour. I was describing a possible workflow. Deletion confirmation => reauthenticate => deletion callback => delete the user.

I don't think it's very easy to build your own "account admin" screen that allows adding sign-in methods and MFA - is that multi factor authentication"? What is that for?

Why do the source files say Copyright 2022, the Chromium project authors. Please see the AUTHORS file for details. Where is the AUTHORS file. Did this code come from the chromium project?

graemep-nz commented 1 year ago

FYI https://github.com/firebase/flutterfire/discussions/6978#discussioncomment-4035241

kwill39 commented 1 year ago

I like where you both are going with this. Here's my thoughts on what the process should look like:

  1. Optional callback that returns a boolean—true to continue to Step 2, false to stop the process (dev can decide what to do in the callback: show a confirmation dialog, etc.). The callback is optional. If nothing is provided, Firebase UI Auth would show a simple dialog with a title like "Delete account?" with "Cancel" and "Delete" options.
  2. Reauthenticate (I'm thinking there should be no exceptions here. Reauthentication would always be required, just to confirm the user's identity. I'm open to thoughts on this.)
  3. Optional callback where the dev can do any final logic they feel the need to just before Firebase UI Auth deletes the account
  4. Delete the account

What do you think?

kwill39 commented 1 year ago

@lesnitsky, just checking in to see what you think about my suggestion above. I would like to try to create a PR myself that follows that process and hopefully resolves this issue.

graemep-nz commented 1 year ago

The default confirmation dialog could say this "Your account, and all data associated with your account, will be deleted."

graemep-nz commented 1 year ago

Correction - need to show username and email address in the dialog

Your account, and all data associated with your account, will be deleted. Username : Jim Email : jim@gmail.com

lesnitsky commented 1 year ago

@kwilliams3 the flow described in this comment sounds reasonable to me.

I would like to try to create a PR myself that follows that process and hopefully resolves this issue.

Sure, go ahead!

graemep-nz commented 1 year ago

@kwilliams3 @lesnitsky How about adding a button for "change email address" on the profile screen. The user needs to do this before they lose access to their old email address. The system should ideally ask them to verify the new email address before associating it with their account, just in case they type it incorrectly.

If someone wants to change a federated login to a different login, a possible way to do it is to create an email/password login if they don't already have one, then login with email/password and delete the federated login on the profile page. Then add a new federated login. Hence the "reverse" of this is a possible way for someone to change their email address but a "change email address" button would be nicer. Also see this https://github.com/firebase/flutterfire/issues/11115

lesnitsky commented 1 year ago

@graemep-nz

How about adding a button for "change email address" on the profile screen.

This sounds like a feature request which is unrelated to this issue, please open another issue for this.