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

Let user remove their account by themselves #4162

Open
Tracked by #1382 ...
teolemon opened this issue Jun 15, 2023 · 26 comments
Open
Tracked by #1382 ...

Let user remove their account by themselves #4162

teolemon opened this issue Jun 15, 2023 · 26 comments
Labels
blocked on server ✨ enhancement New feature or request 🎯 P1 👥 User management Account login, signup, signout

Comments

@teolemon
Copy link
Member

teolemon commented Jun 15, 2023

What

Part of

@teolemon teolemon added ✨ enhancement New feature or request 👥 User management Account login, signup, signout labels Jun 15, 2023
@teolemon teolemon changed the title Account deletion from the app Let user remove their account by themselves Jun 27, 2023
@teolemon
Copy link
Member Author

@M123-dev @monsieurtanuki @g123k this one is really useful, since answering manually to deletion requests is a lot of recurring work

@monsieurtanuki
Copy link
Contributor

@teolemon The thing is that I don't think we have such thing as "check that the user is logged in" in off-dart.
That would mean anybody could delete any account.
In short, I don't have clear thoughts on that issue, and I'm afraid there are tons of potential blunders here.

@teolemon
Copy link
Member Author

What we could do is change the url of the webview to the deletion page on the website @monsieurtanuki
This will make the process truly self service, and going native can be done later

@monsieurtanuki
Copy link
Contributor

@teolemon Currently we go to this page:

    final Uri uri = Uri(
        scheme: 'https',
        host: 'blog.openfoodfacts.org',
        pathSegments: <String>[
          'en',
          'account-deletion',
        ],
        queryParameters: <String, String>{
          'your-subject': subject,
          if (userId != null && userId.isEmail)
            'your-mail': userId
          else if (userId != null)
            'your-name': userId
        });

What should the URL be instead?

@teolemon
Copy link
Member Author

https://world.openfoodfacts.org/cgi/user.pl?type=edit&userid=teolemon (replace teolemon by user-id)

@monsieurtanuki
Copy link
Contributor

I land on an "edit your profile" page, in English, and cannot see a "delete" button.

@teolemon
Copy link
Member Author

@alexgarel @stephanegigandet will deploy that soon I believe. It should be live on .net, but I don't see it with my regular account, as opposed to my superadmin one

@monsieurtanuki
Copy link
Contributor

Ping when it's available on .org.

@teolemon
Copy link
Member Author

@monsieurtanuki it is live

@monsieurtanuki
Copy link
Contributor

@teolemon It's not that easy.

If you're not connected before on the website, you land on an obscure "Error" page
https://world.openfoodfacts.org/cgi/user.pl?type=edit&userid=teolemon

If I remove the "type=edit" parameter, I land on a "Register" page (with an improbable pessimistic "Delete account" button)
https://world.openfoodfacts.org/cgi/user.pl?userid=monsieurtanuki

Is there a URL to the sign in page?

@teolemon
Copy link
Member Author

@monsieurtanuki
Copy link
Contributor

@john-gom Just checking: anybody can call /cgi/user.pl and delete anybody, right?

  • userid=xxx
  • type=delete
  • action=process
  • delete=on

It would be much safer if you also asked for the password. I mean, in the context you developed (website) it's not very important (already connected), but it is for an API.
Correct me if I'm wrong.

@teolemon
Copy link
Member Author

teolemon commented Jul 2, 2023

  • nope, you can't delete other people @monsieurtanuki :-)
  • only admins can potentially do that kind of things (which is somewhat dangerous, since admins are also logged into the app)

@monsieurtanuki
Copy link
Contributor

@teolemon I didn't mean that it was a desired feature: my limited knowledge of perl made me ask that while trying to reverse engineer /cgi/user.pl.
As I have also a limited experience of curl there's probably something wrong in my syntax, but if I run the following statement I get a localized html page answer like "Permission denied".
Perl/curl help needed!

curl
  -X POST https://fr.openfoodfacts.org/cgi/user.pl -H "Content-Type: application/x-www-form-urlencoded"
  -d "userid=test-del-20230703-1&type=edit&action=process&delete=on&password=test-del-20230703-1"

@john-gom
Copy link

john-gom commented Jul 3, 2023

You won't be able to call the API directly like that. You would need a session cookie for the user before it would work.

@monsieurtanuki
Copy link
Contributor

Then is that possible only for the website or also for flutter? I don't know how it would work.

@alexgarel
Copy link
Member

Sorry, it seems we may have to add an API point !

@monsieurtanuki
Copy link
Contributor

@alexgarel It definitely looks so.
We need a "password" parameter, in order to prevent someone to delete accidentally - or on purpose - other accounts.

Maybe it's not even enough, as someone could erase all users in bruteforce attack. I don't know how deleting a user works here:

  1. do you send an email and a link to confirm?
  2. do you remove the user or just flag it as "deleted", with a possibility to reactivate?

@stephanegigandet
Copy link
Contributor

Users can only delete their own user account. To work, the request needs to be authenticated: either with a session cookie, or with userid + password.

@monsieurtanuki
Copy link
Contributor

@stephanegigandet That's what I tried unsuccessfully with the code I mentioned earlier:

curl
  -X POST https://fr.openfoodfacts.org/cgi/user.pl -H "Content-Type: application/x-www-form-urlencoded"
  -d "userid=test-del-20230703-1&type=edit&action=process&delete=on&password=test-del-20230703-1"

Is there some typo in my request?

@teolemon
Copy link
Member Author

A test is currently being written

https://github.com/openfoodfacts/openfoodfacts-server/pull/8723/files

@teolemon
Copy link
Member Author

my %delete_form = (
name => 'Test',
email => 'bob@test.com',
password => '',
confirm_password => '',
delete => 'on',
action => 'process',
type => 'edit',
userid => 'tests'
);

@teolemon
Copy link
Member Author

@monsieurtanuki I've successfully deleted the account in two steps from my browser, login in, and then deleting using the url you tried. It seems you can't do both at the same time.
Curious how we do other user management operations

@monsieurtanuki
Copy link
Contributor

@monsieurtanuki I've successfully deleted the account in two steps from my browser, login in, and then deleting using the url you tried. It seems you can't do both at the same time. Curious how we do other user management operations

@teolemon Actually we don't have that much methods in off-dart regarding users:

  • login / login2, which is a "check password + get email, name and id" method
  • register
  • resetPassword, that doesn't use the password (sends a "reset password" email from the email or userId)

That said, there's nothing for the moment on the server side that would delete a user in just one command if you're not already connected to the website.
Smoothie issue is stalled then.

@monsieurtanuki
Copy link
Contributor

I believe we had this issue specifically for iOS, and we'll soon have the same issue for android:

December 7th, 2023
User Data policy – Account Deletion requirement
Watch RePlay episode 2 to learn more about the new data deletion policies

@M123-dev
Copy link
Member

M123-dev commented Sep 2, 2023

I've just created openfoodfacts/openfoodfacts-server#8940. Please add any corrections if I am wrong @monsieurtanuki

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked on server ✨ enhancement New feature or request 🎯 P1 👥 User management Account login, signup, signout
Projects
Status: 💬 To discuss and validate
Development

Successfully merging a pull request may close this issue.

6 participants