From 73e73c9d25eb8dcbe3e83a37bd9b9efaefd4e8c9 Mon Sep 17 00:00:00 2001 From: Baptiste Mispelon Date: Fri, 29 Nov 2024 13:43:59 +0100 Subject: [PATCH] Fixed #1782 -- Added page to delete one's user account --- accounts/forms.py | 51 +++++++++++++++++++ accounts/tests.py | 51 ++++++++++++++++++- accounts/urls.py | 10 ++++ accounts/views.py | 40 ++++++++++++++- .../templates/accounts/delete_profile.html | 40 +++++++++++++++ .../accounts/delete_profile_success.html | 17 +++++++ .../templates/accounts/edit_profile.html | 6 +++ 7 files changed, 213 insertions(+), 2 deletions(-) create mode 100644 djangoproject/templates/accounts/delete_profile.html create mode 100644 djangoproject/templates/accounts/delete_profile_success.html diff --git a/accounts/forms.py b/accounts/forms.py index 39547d3b1..a63c3116f 100644 --- a/accounts/forms.py +++ b/accounts/forms.py @@ -1,4 +1,6 @@ from django import forms +from django.db import transaction +from django.db.models import ProtectedError from django.utils.translation import gettext_lazy as _ from .models import Profile @@ -36,3 +38,52 @@ def save(self, commit=True): if commit: instance.user.save() return instance + + +class DeleteProfileForm(forms.Form): + """ + A form for delete the request's user and their associated data. + + This form has no fields, it's used as a container for validation and deltion + logic. + """ + + class InvalidFormError(Exception): + pass + + def __init__(self, *args, user=None, **kwargs): + if user.is_anonymous: + raise TypeError("DeleteProfileForm only accepts actual User instances") + self.user = user + super().__init__(*args, **kwargs) + + def clean(self): + cleaned_data = super().clean() + if self.user.is_staff: + # Prevent potentially deleting some important history (admin.LogEntry) + raise forms.ValidationError(_("Staff users cannot be deleted")) + return cleaned_data + + def add_errors_from_protectederror(self, exception): + """ + Convert the given ProtectedError exception object into validation + errors on the instance. + """ + self.add_error(None, _("User has protected data and cannot be deleted")) + + @transaction.atomic() + def delete(self): + """ + Delete the form's user (self.instance). + """ + if not self.is_valid(): + raise self.InvalidFormError( + "DeleteProfileForm.delete() can only be called on valid forms" + ) + + try: + self.user.delete() + except ProtectedError as e: + self.add_errors_from_protectederror(e) + return None + return self.user diff --git a/accounts/tests.py b/accounts/tests.py index bc6c48e50..1c84ce44f 100644 --- a/accounts/tests.py +++ b/accounts/tests.py @@ -1,7 +1,9 @@ -from django.contrib.auth.models import User +from django.contrib.auth.models import AnonymousUser, User from django.test import TestCase, override_settings from django_hosts.resolvers import reverse +from accounts.forms import DeleteProfileForm +from foundation import models as foundationmodels from tracdb.models import Revision, Ticket, TicketChange from tracdb.testutils import TracDBCreateDatabaseMixin @@ -169,3 +171,50 @@ def test_profile_view_reversal(self): """ for username in ["asdf", "@asdf", "asd-f", "as.df", "as+df"]: reverse("user_profile", host="www", args=[username]) + + +class UserDeletionTestCase(TestCase): + def create_user_and_form(self, bound=True, **userkwargs): + userkwargs.setdefault("username", "test") + userkwargs.setdefault("email", "test@example.com") + userkwargs.setdefault("password", "password") + + formkwargs = {"user": User.objects.create_user(**userkwargs)} + if bound: + formkwargs["data"] = {} + + return DeleteProfileForm(**formkwargs) + + def test_deletion(self): + form = self.create_user_and_form() + self.assertFormError(form, None, []) + form.delete() + self.assertQuerySetEqual(User.objects.all(), []) + + def test_anonymous_user_error(self): + self.assertRaises(TypeError, DeleteProfileForm, user=AnonymousUser) + + def test_deletion_staff_forbidden(self): + form = self.create_user_and_form(is_staff=True) + self.assertFormError(form, None, ["Staff users cannot be deleted"]) + + def test_user_with_protected_data(self): + form = self.create_user_and_form() + form.user.boardmember_set.create( + office=foundationmodels.Office.objects.create(name="test"), + term=foundationmodels.Term.objects.create(year=2000), + ) + form.delete() + self.assertFormError( + form, None, ["User has protected data and cannot be deleted"] + ) + + def test_form_delete_method_requires_valid_form(self): + form = self.create_user_and_form(is_staff=True) + self.assertRaises(form.InvalidFormError, form.delete) + + def test_view_deletion_also_logs_out(self): + user = self.create_user_and_form().user + self.client.force_login(user) + self.client.post(reverse("delete_profile")) + self.assertEqual(self.client.cookies["sessionid"].value, "") diff --git a/accounts/urls.py b/accounts/urls.py index 5558d130e..0e535c756 100644 --- a/accounts/urls.py +++ b/accounts/urls.py @@ -15,6 +15,16 @@ account_views.edit_profile, name="edit_profile", ), + path( + "delete/", + account_views.delete_profile, + name="delete_profile", + ), + path( + "delete/success/", + account_views.delete_profile_success, + name="delete_profile_success", + ), path("", include("django.contrib.auth.urls")), path("", include("registration.backends.default.urls")), ] diff --git a/accounts/views.py b/accounts/views.py index bbdf888db..8c3c7dd0f 100644 --- a/accounts/views.py +++ b/accounts/views.py @@ -1,5 +1,7 @@ import hashlib +from urllib.parse import urlencode +from django.contrib.auth import logout from django.contrib.auth.decorators import login_required from django.contrib.auth.models import User from django.core.cache import caches @@ -7,7 +9,7 @@ from tracdb import stats as trac_stats -from .forms import ProfileForm +from .forms import DeleteProfileForm, ProfileForm from .models import Profile @@ -34,6 +36,42 @@ def edit_profile(request): return render(request, "accounts/edit_profile.html", {"form": form}) +@login_required +def delete_profile(request): + if request.method == "POST": + form = DeleteProfileForm(data=request.POST, user=request.user) + if form.is_valid(): + if form.delete(): + logout(request) + return redirect("delete_profile_success") + else: + form = DeleteProfileForm(user=request.user) + + context = { + "form": form, + # Strings are left translated on purpose (ops prefer english :D) + "OPS_EMAIL_PRESETS": urlencode( + { + "subject": "[djangoproject.com] Manual account deletion", + "body": ( + "Hello lovely Django Ops,\n\n" + "I would like to delete my djangoproject.com user account (" + f"username {request.user.username}) but the system is not letting " + "me do it myself. Could you help me out please?\n\n" + "Thanks in advance,\n" + "You're amazing\n" + f"{request.user.get_full_name() or request.user.username}" + ), + } + ), + } + return render(request, "accounts/delete_profile.html", context) + + +def delete_profile_success(request): + return render(request, "accounts/delete_profile_success.html") + + def get_user_stats(user): c = caches["default"] username = user.username.encode("ascii", "ignore") diff --git a/djangoproject/templates/accounts/delete_profile.html b/djangoproject/templates/accounts/delete_profile.html new file mode 100644 index 000000000..48bfc4cfd --- /dev/null +++ b/djangoproject/templates/accounts/delete_profile.html @@ -0,0 +1,40 @@ +{% extends "base.html" %} +{% load i18n %} + +{% block title %}{% translate "Confirmation: delete your profile" %}{% endblock %} + +{% block content %} + {% if form.errors %} +

{% translate "Could not delete account" %}

+ +

{% blocktranslate trimmed %} + Sorry, something went wrong when trying to delete your account. + That means there's probably some protected data still associated + with your account. + Please contact + the operations team + and we'll sort it out for you. + {% endblocktranslate %}

+ {% else %} +

{% translate "Are you sure?" %}

+ +

{% blocktranslate trimmed with username=request.user.username %} + ⚠️ You are about to delete all data associated with the username + {{ username}}. + {% endblocktranslate %}

+ +

{% blocktranslate trimmed %} + Deleting your account is permanent and cannot be reversed. + Are you sure you want to continue? + {% endblocktranslate %}

+
+ {% csrf_token %} +
+ + + {% translate "No, cancel and go back" %} + +
+
+ {% endif %} +{% endblock %} diff --git a/djangoproject/templates/accounts/delete_profile_success.html b/djangoproject/templates/accounts/delete_profile_success.html new file mode 100644 index 000000000..411b3fcf7 --- /dev/null +++ b/djangoproject/templates/accounts/delete_profile_success.html @@ -0,0 +1,17 @@ +{% extends "base.html" %} + +{% load i18n %} + +{% block content %} +

{% translate "Account deleted" %}

+

+ {% translate "Your account and its data was successfully deleted and you've been logged out." %} +

+

+ {% url "community-index" as community_index_url %} + {% blocktranslate trimmed %} + Thanks for spending your time with us, we hope we'll still see you + around on our various community spaces, online and off. + {% endblocktranslate %} +

+{% endblock %} diff --git a/djangoproject/templates/accounts/edit_profile.html b/djangoproject/templates/accounts/edit_profile.html index 536ac5bdd..be41208a4 100644 --- a/djangoproject/templates/accounts/edit_profile.html +++ b/djangoproject/templates/accounts/edit_profile.html @@ -50,5 +50,11 @@

{% translate "Help" %}

emails. We'll also use this email to try to fetch a Gravatar. You can change the image for this email at Gravatar.{% endblocktranslate %}

+ +

+ + {% translate "Want to delete your account?" %} + +

{% endblock %}