From d02b14766c6b53167e09092a358858c63f0e8e9f Mon Sep 17 00:00:00 2001
From: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Thu, 18 Jul 2019 15:09:30 +0200
Subject: [PATCH] accounts: send validation email before self-triggered account
 deletion (#27823)

---
 src/authentic2/app_settings.py                |  3 +
 .../account_delete_notification_subject.txt   |  2 +-
 .../account_deletion_code_body.html           | 13 +++
 .../authentic2/account_deletion_code_body.txt |  8 ++
 .../account_deletion_code_subject.txt         |  1 +
 ...e.html => accounts_delete_validation.html} |  8 +-
 .../authentic2/accounts_plain_message.html    | 10 ++
 src/authentic2/templatetags/__init__.py       |  0
 src/authentic2/templatetags/authentic2.py     |  7 ++
 src/authentic2/urls.py                        |  3 +
 src/authentic2/utils.py                       | 29 ++++++
 src/authentic2/views.py                       | 91 +++++++++++++------
 tests/test_views.py                           | 83 ++++++++++++++++-
 13 files changed, 223 insertions(+), 35 deletions(-)
 create mode 100644 src/authentic2/templates/authentic2/account_deletion_code_body.html
 create mode 100644 src/authentic2/templates/authentic2/account_deletion_code_body.txt
 create mode 100644 src/authentic2/templates/authentic2/account_deletion_code_subject.txt
 rename src/authentic2/templates/authentic2/{accounts_delete.html => accounts_delete_validation.html} (66%)
 create mode 100644 src/authentic2/templates/authentic2/accounts_plain_message.html
 create mode 100644 src/authentic2/templatetags/__init__.py
 create mode 100644 src/authentic2/templatetags/authentic2.py

diff --git a/src/authentic2/app_settings.py b/src/authentic2/app_settings.py
index b8b37c821..6f5a71116 100644
--- a/src/authentic2/app_settings.py
+++ b/src/authentic2/app_settings.py
@@ -278,6 +278,9 @@ default_settings = dict(
     A2_EMAIL_CHANGE_TOKEN_LIFETIME=Setting(
         default=7200,
         definition='Lifetime in seconds of the token sent to verify email adresses'),
+    A2_DELETION_REQUEST_LIFETIME=Setting(
+        default=48*3600,
+        definition='Lifetime in seconds of the user account deletion request'),
     A2_REDIRECT_WHITELIST=Setting(
         default=(),
         definition='List of origins which are authorized to ask for redirection.'),
diff --git a/src/authentic2/templates/authentic2/account_delete_notification_subject.txt b/src/authentic2/templates/authentic2/account_delete_notification_subject.txt
index da0210842..0383ad143 100644
--- a/src/authentic2/templates/authentic2/account_delete_notification_subject.txt
+++ b/src/authentic2/templates/authentic2/account_delete_notification_subject.txt
@@ -1 +1 @@
-{% load i18n %}{% autoescape off %}{% blocktrans %}Account deletion request on {{ site }}{% endblocktrans %}{% endautoescape %}
+{% load i18n %}{% autoescape off %}{% blocktrans %}Account deletion on {{ site }}{% endblocktrans %}{% endautoescape %}
diff --git a/src/authentic2/templates/authentic2/account_deletion_code_body.html b/src/authentic2/templates/authentic2/account_deletion_code_body.html
new file mode 100644
index 000000000..f5bcbf851
--- /dev/null
+++ b/src/authentic2/templates/authentic2/account_deletion_code_body.html
@@ -0,0 +1,13 @@
+{% load i18n %}
+<html>
+  <body style="max-width: 90ex">
+        <p>{% blocktrans %}{{ full_name }},{% endblocktrans %}</p>
+        <p>
+{% blocktrans %}
+Please click on {{ deletion_url }} if you want to validate your account deletion request on {{ site }}.
+If so, all related data will be deleted in the next few hours.
+You won't be able to log in with this account anymore.
+{% endblocktrans %}
+        </p>
+  </body>
+</html>
diff --git a/src/authentic2/templates/authentic2/account_deletion_code_body.txt b/src/authentic2/templates/authentic2/account_deletion_code_body.txt
new file mode 100644
index 000000000..368862f14
--- /dev/null
+++ b/src/authentic2/templates/authentic2/account_deletion_code_body.txt
@@ -0,0 +1,8 @@
+{% load i18n %}{% autoescape off %}{% blocktrans %}{{ full_name }},{% endblocktrans %}
+
+{% blocktrans %}
+Please click on {{ deletion_url }} if you want to validate your account deletion request on {{ site }}.
+If so, all related data will be deleted in the next few hours.
+You won't be able to log in with this account anymore.
+{% endblocktrans %}
+{% endautoescape %}
diff --git a/src/authentic2/templates/authentic2/account_deletion_code_subject.txt b/src/authentic2/templates/authentic2/account_deletion_code_subject.txt
new file mode 100644
index 000000000..91226f19c
--- /dev/null
+++ b/src/authentic2/templates/authentic2/account_deletion_code_subject.txt
@@ -0,0 +1 @@
+{% load i18n %}{% autoescape off %}{% blocktrans %}Validate account deletion request on {{ site }}{% endblocktrans %}{% endautoescape %}
diff --git a/src/authentic2/templates/authentic2/accounts_delete.html b/src/authentic2/templates/authentic2/accounts_delete_validation.html
similarity index 66%
rename from src/authentic2/templates/authentic2/accounts_delete.html
rename to src/authentic2/templates/authentic2/accounts_delete_validation.html
index a5bbfba61..1b06d5df5 100644
--- a/src/authentic2/templates/authentic2/accounts_delete.html
+++ b/src/authentic2/templates/authentic2/accounts_delete_validation.html
@@ -1,5 +1,6 @@
 {% extends "authentic2/base-page.html" %}
 {% load i18n %}
+{% load authentic2 %}
 
 {% block page-title %}
   {{ block.super }} - {{ view.title }}
@@ -15,8 +16,11 @@
 {% block content %}
 <form method="post">
   {% csrf_token %}
-  <p>{% trans "Delete my account and all my personal datas ?" %}</p>
-  {{ form.as_p }}
+  <p>
+  {% blocktrans with first_name=user.first_name last_name=user.last_name possessive=user.last_name|endswith:"s"|yesno:"','s" %}
+  Delete {{ first_name }} {{ last_name }}{{ possessive }} account and all related personal data?
+  {% endblocktrans %}
+  </p>
   <button class="delete-button" name="submit">{% trans "Delete" %}</button>
   <button class="cancel-button" name="cancel" formnovalidate>{% trans "Cancel" %}</button>
 </form>
diff --git a/src/authentic2/templates/authentic2/accounts_plain_message.html b/src/authentic2/templates/authentic2/accounts_plain_message.html
new file mode 100644
index 000000000..a39b3e17f
--- /dev/null
+++ b/src/authentic2/templates/authentic2/accounts_plain_message.html
@@ -0,0 +1,10 @@
+{% extends "authentic2/base-page.html" %}
+{% load i18n %}
+
+{% block page-title %}
+  {{ block.super }} - {% trans "Account deletion" %}
+{% endblock %}
+
+{% block content %}
+  <p>{{ message }}</p>
+{% endblock %}
diff --git a/src/authentic2/templatetags/__init__.py b/src/authentic2/templatetags/__init__.py
new file mode 100644
index 000000000..e69de29bb
diff --git a/src/authentic2/templatetags/authentic2.py b/src/authentic2/templatetags/authentic2.py
new file mode 100644
index 000000000..a81424072
--- /dev/null
+++ b/src/authentic2/templatetags/authentic2.py
@@ -0,0 +1,7 @@
+from django import template
+
+register = template.Library()
+
+@register.filter
+def endswith(value, suffix):
+    return value.endswith(suffix)
diff --git a/src/authentic2/urls.py b/src/authentic2/urls.py
index 7c5d97549..b953046cd 100644
--- a/src/authentic2/urls.py
+++ b/src/authentic2/urls.py
@@ -44,6 +44,9 @@ accounts_urlpatterns = [
     url(r'^delete/$',
         login_required(views.DeleteView.as_view()),
         name='delete_account'),
+    url(r'validate-deletion/(?P<deletion_token>[\w: -]+)/$',
+        views.ValidateDeletionView.as_view(),
+        name='validate_deletion'),
     url(r'^logged-in/$',
         views.logged_in,
         name='logged-in'),
diff --git a/src/authentic2/utils.py b/src/authentic2/utils.py
index c0c9d9543..f737a3227 100644
--- a/src/authentic2/utils.py
+++ b/src/authentic2/utils.py
@@ -670,6 +670,15 @@ def build_activation_url(request, email, next_url=None, ou=None, **kwargs):
     return activate_url
 
 
+def build_deletion_url(request, **kwargs):
+    data = kwargs.copy()
+    data['user_pk'] = request.user.pk
+    deletion_token = signing.dumps(data)
+    delete_url = request.build_absolute_uri(
+            reverse('validate_deletion', kwargs={'deletion_token': deletion_token}))
+    return delete_url
+
+
 def send_registration_mail(request, email, ou, template_names=None, next_url=None, context=None,
                            **kwargs):
     '''Send a registration mail to an user. All given kwargs will be used
@@ -716,6 +725,26 @@ def send_registration_mail(request, email, ou, template_names=None, next_url=Non
                 registration_url)
 
 
+def send_account_deletion_code(request, user):
+    '''Send an account deletion notification code to a user.
+
+       Can raise an smtplib.SMTPException
+    '''
+    logger = logging.getLogger(__name__)
+    deletion_url = build_deletion_url(request)
+    context = {
+        'full_name': request.user.get_full_name(),
+        'user': request.user,
+        'site': request.get_host(),
+        'deletion_url': deletion_url}
+    template_names = [
+        'authentic2/account_deletion_code']
+    if user.ou:
+        template_names.insert(0, 'authentic2/account_deletion_code_%s' % user.ou.slug)
+    send_templated_mail(user.email, template_names, context, request=request)
+    logger.info(u'account deletion code sent to %s', user.email)
+
+
 def send_account_deletion_mail(request, user):
     '''Send an account deletion notification mail to a user.
 
diff --git a/src/authentic2/views.py b/src/authentic2/views.py
index df8a35529..4a473eff0 100644
--- a/src/authentic2/views.py
+++ b/src/authentic2/views.py
@@ -1060,45 +1060,78 @@ class RegistrationCompletionView(CreateView):
                                   request=self.request)
 
 
-class DeleteView(FormView):
-    template_name = 'authentic2/accounts_delete.html'
-    success_url = reverse_lazy('auth_logout')
-    title = _('Delete account')
+registration_completion = valid_token(RegistrationCompletionView.as_view())
+
 
+class DeleteView(View):
     def dispatch(self, request, *args, **kwargs):
         if not app_settings.A2_REGISTRATION_CAN_DELETE_ACCOUNT:
             return utils.redirect(request, '..')
         return super(DeleteView, self).dispatch(request, *args, **kwargs)
 
-    def post(self, request, *args, **kwargs):
-        if 'cancel' in request.POST:
-            return utils.redirect(request, 'account_management')
-        return super(DeleteView, self).post(request, *args, **kwargs)
+    def get(self, request, *args, **kwargs):
+        utils.send_account_deletion_code(self.request, self.request.user)
+        messages.info(request,
+                _("An account deletion validation email has been sent to your email address."))
+        return utils.redirect(request, 'account_management')
 
-    def get_form_class(self):
-        if self.request.user.has_usable_password():
-            return profile_forms.DeleteAccountForm
-        return Form
 
-    def get_form_kwargs(self, **kwargs):
-        kwargs = super(DeleteView, self).get_form_kwargs(**kwargs)
-        if self.request.user.has_usable_password():
-            kwargs['user'] = self.request.user
-        return kwargs
+class ValidateDeletionView(TemplateView):
+    template_name = 'authentic2/accounts_delete_validation.html'
+    title = _('Confirm account deletion')
+    user = None
 
-    def form_valid(self, form):
-        utils.send_account_deletion_mail(self.request, self.request.user)
-        models.DeletedUser.objects.delete_user(self.request.user)
-        self.request.user.email += '#%d' % random.randint(1, 10000000)
-        self.request.user.email_verified = False
-        self.request.user.save(update_fields=['email', 'email_verified'])
-        logger.info(u'deletion of account %s requested', self.request.user)
-        hooks.call_hooks('event', name='delete-account', user=self.request.user)
-        message_template = loader.get_template('authentic2/account_deletion_message.html')
-        messages.info(self.request, message_template.render(request=self.request))
-        return super(DeleteView, self).form_valid(form)
+    def dispatch(self, request, *args, **kwargs):
+        error = None
+        try:
+            deletion_token = signing.loads(kwargs['deletion_token'],
+                    max_age=app_settings.A2_DELETION_REQUEST_LIFETIME)
+            user_pk = deletion_token['user_pk']
+            self.user = get_user_model().objects.get(pk=user_pk)
+            # A user account wont be deactived twice
+            if not self.user.is_active:
+                raise ValidationError(
+                    _('This account had previously been deactivated and will be deleted soon.'))
+            logger.info('user %s confirmed the deletion of their own account', self.user)
+        except signing.SignatureExpired:
+            error = _('The account deletion request is too old, try again')
+        except signing.BadSignature:
+            error = _('The account deletion request is invalid, try again')
+        except ValueError:
+            error = _('The account deletion request was not on this site, try again')
+        except ValidationError as e:
+            error = e.message
+        except get_user_model().DoesNotExist:
+            error = _('This account has previously been deleted.')
+
+        if error:
+            return render(request, 'authentic2/accounts_plain_message.html',
+                    context={'message': error})
+        return super(ValidateDeletionView, self).dispatch(request, *args, **kwargs)
 
-registration_completion = valid_token(RegistrationCompletionView.as_view())
+    def post(self, request, *args, **kwargs):
+        if 'cancel' not in request.POST:
+            utils.send_account_deletion_mail(self.request, self.user)
+            models.DeletedUser.objects.delete_user(self.user)
+            self.user.email += '#%d' % random.randint(1, 10000000)
+            self.user.email_verified = False
+            self.user.save(update_fields=['email', 'email_verified'])
+            logger.info(u'deletion of account %s performed', self.user)
+            hooks.call_hooks('event', name='delete-account', user=self.user)
+            if self.user == request.user:
+                # No validation message displayed, as the user will surely
+                # notice their own account deletion...
+                return utils.redirect(request, 'auth_logout')
+            # No real use for cancel_url or next_url here, assuming the link
+            # has been received by email.
+            return render(request, 'authentic2/accounts_plain_message.html',
+                    context={'message': 'Deletion performed.'})
+        return utils.redirect(request, '/')
+
+    def get_context_data(self, **kwargs):
+        ctx = super(ValidateDeletionView, self).get_context_data(**kwargs)
+        ctx['user'] = self.user # Not necessarily the user in request
+        return ctx
 
 
 class RegistrationCompleteView(TemplateView):
diff --git a/tests/test_views.py b/tests/test_views.py
index 70757fb42..e8c4827bf 100644
--- a/tests/test_views.py
+++ b/tests/test_views.py
@@ -15,10 +15,11 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 # authentic2
 
-from utils import login
+from utils import login, logout, get_link_from_mail
 import pytest
 
 from django.core.urlresolvers import reverse
+from django.utils.html import escape
 from django.utils.six.moves.urllib.parse import urlparse
 
 from authentic2.custom_user.models import User
@@ -41,18 +42,94 @@ def test_account_delete(app, simple_user, mailoutbox):
     assert simple_user.is_active
     assert not len(mailoutbox)
     page = login(app, simple_user, path=reverse('delete_account'))
-    page.form.set('password', simple_user.username)
+    assert len(mailoutbox) == 1
+    link = get_link_from_mail(mailoutbox[0])
+    assert 'Validate account deletion request on testserver' == mailoutbox[0].subject
+    assert [simple_user.email] == mailoutbox[0].to
+    page = app.get(link)
     # FIXME: webtest does not set the Referer header, so the logout page will always ask for
     # confirmation under tests
     response = page.form.submit(name='submit').follow()
     response = response.form.submit()
-    assert len(mailoutbox) == 1
     assert not User.objects.get(pk=simple_user.pk).is_active
+    assert len(mailoutbox) == 2
+    assert 'Account deletion on testserver' == mailoutbox[1].subject
+    assert [simple_user.email] == mailoutbox[0].to
     assert urlparse(response.location).path == '/'
     response = response.follow().follow()
     assert response.request.url.endswith('/login/?next=/')
 
 
+def test_account_delete_when_logged_out(app, simple_user, mailoutbox):
+    assert simple_user.is_active
+    assert not len(mailoutbox)
+    page = login(app, simple_user, path=reverse('delete_account'))
+    assert len(mailoutbox) == 1
+    link = get_link_from_mail(mailoutbox[0])
+    logout(app)
+    page = app.get(link)
+    assert escape('Delete %s %s\'s account and all related personal data?' % (
+            simple_user.first_name, simple_user.last_name)) in page.text
+    response = page.form.submit(name='submit')
+    assert not User.objects.get(pk=simple_user.pk).is_active
+    assert len(mailoutbox) == 2
+    assert 'Account deletion on testserver' == mailoutbox[1].subject
+    assert [simple_user.email] == mailoutbox[0].to
+    assert "Deletion performed" in response.text
+
+
+def test_account_delete_by_other_user(app, simple_user, user_ou1, mailoutbox):
+    assert simple_user.is_active
+    assert user_ou1.is_active
+    assert not len(mailoutbox)
+    page = login(app, simple_user, path=reverse('delete_account'))
+    assert len(mailoutbox) == 1
+    link = get_link_from_mail(mailoutbox[0])
+    logout(app)
+    login(app, user_ou1, path=reverse('account_management'))
+    page = app.get(link)
+    assert escape('Delete %s %s\'s account and all related personal data?' % (
+            simple_user.first_name, simple_user.last_name)) in page.text
+    response = page.form.submit(name='submit')
+    assert not User.objects.get(pk=simple_user.pk).is_active
+    assert User.objects.get(pk=user_ou1.pk).is_active
+    assert "Deletion performed" in response.text
+    assert len(mailoutbox) == 2
+    assert 'Account deletion on testserver' == mailoutbox[1].subject
+    assert [simple_user.email] == mailoutbox[0].to
+
+
+def test_account_delete_fake_token(app, simple_user, mailoutbox):
+    response = app.get(reverse('validate_deletion', kwargs={'deletion_token': 'thisismostlikelynotavalidtoken'}))
+    assert "The account deletion request is invalid, try again" in response.text
+
+
+def test_account_delete_expired_token(app, simple_user, mailoutbox, freezer):
+    freezer.move_to('2019-08-01')
+    page = login(app, simple_user, path=reverse('delete_account'))
+    freezer.move_to('2019-08-04') # Too late...
+    link = get_link_from_mail(mailoutbox[0])
+    response = app.get(link)
+    assert "The account deletion request is too old, try again" in response.text
+
+
+def test_account_delete_valid_token_unexistent_user(app, simple_user, mailoutbox):
+    page = login(app, simple_user, path=reverse('delete_account'))
+    link = get_link_from_mail(mailoutbox[0])
+    simple_user.delete()
+    response = app.get(link)
+    assert 'This account has previously been deleted.' in response.text
+
+
+def test_account_delete_valid_token_inactive_user(app, simple_user, mailoutbox):
+    page = login(app, simple_user, path=reverse('delete_account'))
+    link = get_link_from_mail(mailoutbox[0])
+    simple_user.is_active = False
+    simple_user.save()
+    response = app.get(link)
+    assert "This account had previously been deactivated" in response.text
+
+
 def test_login_invalid_next(app):
     app.get(reverse('auth_login') + '?next=plop')
 
-- 
2.23.0.rc1

