views: require authentication for deleting account without a verified email (#28853)

src/authentic2/authenticators.py

@@ -36,6 +36,8 @@ logger = logging.getLogger(__name__)
 
 
 class BaseAuthenticator:
+    how = ()
+
     def __init__(self, show_condition=None, **kwargs):
         self.show_condition = show_condition
 
@@ -60,6 +62,7 @@ class BaseAuthenticator:
 
 class LoginPasswordAuthenticator(BaseAuthenticator):
     id = 'password'
+    how = ['password', 'password-on-https']
     submit_name = 'login-password-submit'
     priority = 0
 
@@ -119,11 +122,16 @@ class LoginPasswordAuthenticator(BaseAuthenticator):
         form = authentication_forms.AuthenticationForm(
             request=request, data=data, initial=initial, preferred_ous=preferred_ous
         )
+        if request.user.is_authenticated and request.login_token.get('action'):
+            form.initial['username'] = request.user.username or request.user.email
+            form.fields['username'].widget.attrs['readonly'] = True
+            form.fields['password'].widget.attrs['autofocus'] = True
+        else:
+            form.fields['username'].widget.attrs['autofocus'] = not (bool(context.get('block_index')))
         if app_settings.A2_ACCEPT_EMAIL_AUTHENTICATION:
             form.fields['username'].label = _('Username or email')
         if app_settings.A2_USERNAME_LABEL:
             form.fields['username'].label = app_settings.A2_USERNAME_LABEL
-        form.fields['username'].widget.attrs['autofocus'] = not (bool(context.get('block_index')))
         is_secure = request.is_secure
         context['submit_name'] = self.submit_name
         if is_post:

src/authentic2/forms/profile.py

@@ -17,7 +17,6 @@
 
 from django import forms
 from django.forms.models import modelform_factory as dj_modelform_factory
-from django.utils.translation import ugettext
 from django.utils.translation import ugettext_lazy as _
 
 from authentic2 import app_settings, models
@@ -28,20 +27,6 @@ from .mixins import LockedFieldFormMixin
 from .utils import NextUrlFormMixin
 
 
-class DeleteAccountForm(forms.Form):
-    password = forms.CharField(widget=forms.PasswordInput, label=_("Password"))
-
-    def __init__(self, *args, **kwargs):
-        self.user = kwargs.pop('user')
-        super().__init__(*args, **kwargs)
-
-    def clean_password(self):
-        password = self.cleaned_data.get('password')
-        if password and not self.user.check_password(password):
-            raise forms.ValidationError(ugettext('Password is invalid'))
-        return password
-
-
 class EmailChangeFormNoPassword(forms.Form):
     email = ValidatedEmailField(label=_('New email'))
 

src/authentic2/templates/authentic2/accounts_delete_request.html

@@ -20,13 +20,15 @@
   Do you really want to delete your account?
   {% endblocktrans %}
   </p>
+  {% if user.email_verified %}
   <p>
   {% blocktrans trimmed %}
   A validation message will be sent to {{ email }}. You will have to visit the
   link in this email in order to complete the deletion process.
   {% endblocktrans %}
   </p>
-  <button class="submit-button" name="submit">{% trans "Send message" %}</button>
+  {% endif %}
+  <button class="submit-button" name="submit">{% if user.email_verified %}{% trans "Send message" %}{% else %}{% trans "Delete account" %}{% endif %}</button>
   <button class="cancel-button" name="cancel" formnovalidate>{% trans "Cancel" %}</button>
 </form>
 {% endblock %}

src/authentic2/views.py

@@ -17,6 +17,7 @@
 import collections
 import logging
 import re
+import time
 from email.utils import parseaddr
 
 from django import shortcuts
@@ -294,6 +295,15 @@ email_change_verify = EmailChangeVerifyView.as_view()
 def login(request, template_name='authentic2/login.html', redirect_field_name=REDIRECT_FIELD_NAME):
     """Displays the login form and handles the login action."""
 
+    request.login_token = token = {}
+    if 'token' in request.GET:
+        try:
+            token.update(crypto.loads(request.GET['token']))
+            logger.debug('login: got token %s', token)
+        except (crypto.SignatureExpired, crypto.BadSignature, ValueError):
+            logger.warning('login: bad token')
+    methods = token.get('methods', [])
+
     # redirect user to homepage if already connected, if setting
     # A2_LOGIN_REDIRECT_AUTHENTICATED_USERS_TO_HOMEPAGE is True
     if request.user.is_authenticated and app_settings.A2_LOGIN_REDIRECT_AUTHENTICATED_USERS_TO_HOMEPAGE:
@@ -330,6 +340,8 @@ def login(request, template_name='authentic2/login.html', redirect_field_name=RE
 
     # Create blocks
     for authenticator in authenticators:
+        if methods and not set(authenticator.how) & set(methods):
+            continue
         # Legacy API
         if not hasattr(authenticator, 'login'):
             fid = authenticator.id
@@ -390,6 +402,8 @@ def login(request, template_name='authentic2/login.html', redirect_field_name=RE
         block = blocks[0]
         authenticator = block['authenticator']
         if hasattr(authenticator, 'autorun'):
+            if 'message' in token:
+                messages.info(request, token['message'])
             return authenticator.autorun(request, block['id'])
 
     # Old frontends API
@@ -416,6 +430,8 @@ def login(request, template_name='authentic2/login.html', redirect_field_name=RE
             redirect_field_name: redirect_to,
         }
     )
+    if 'message' in token:
+        messages.info(request, token['message'])
     return render(request, template_name, context)
 
 
@@ -1323,17 +1339,42 @@ registration_completion = RegistrationCompletionView.as_view()
 class AccountDeleteView(HomeURLMixin, TemplateView):
     template_name = 'authentic2/accounts_delete_request.html'
     title = _('Request account deletion')
+    last_authentication_max_age = 600  # 10 minutes
 
     def dispatch(self, request, *args, **kwargs):
         if not app_settings.A2_REGISTRATION_CAN_DELETE_ACCOUNT:
             return utils_misc.redirect(request, '..')
+        if not self.request.user.email_verified and not self.has_recent_authentication():
+            methods = [event['how'] for event in utils_misc.get_authentication_events(request)]
+            return utils_misc.login_require(
+                request,
+                token={
+                    'action': 'account-delete',
+                    'message': _('You must re-authenticate to delete your account.'),
+                    'methods': methods,
+                },
+            )
         return super().dispatch(request, *args, **kwargs)
 
+    def has_recent_authentication(self):
+        age = time.time() - utils_misc.last_authentication_event(request=self.request)['when']
+        return age < self.last_authentication_max_age
+
     def post(self, request, *args, **kwargs):
         if 'cancel' in request.POST:
             return utils_misc.redirect(request, 'account_management')
-        utils_misc.send_account_deletion_code(self.request, self.request.user)
-        messages.info(request, _("An account deletion validation email has been sent to your email address."))
+        if self.request.user.email_verified:
+            utils_misc.send_account_deletion_code(self.request, self.request.user)
+            messages.info(
+                request, _("An account deletion validation email has been sent to your email address.")
+            )
+        else:
+            deletion_url = utils_misc.build_deletion_url(request, prompt=False)
+            return logout(
+                request,
+                next_url=deletion_url,
+                check_referer=False,
+            )
         return utils_misc.redirect(request, 'account_management')
 
     def get_context_data(self, **kwargs):
@@ -1346,12 +1387,14 @@ class ValidateDeletionView(TemplateView):
     template_name = 'authentic2/accounts_delete_validation.html'
     title = _('Confirm account deletion')
     user = None
+    prompt = True
 
     def dispatch(self, request, *args, **kwargs):
         try:
             deletion_token = crypto.loads(
                 kwargs['deletion_token'], max_age=app_settings.A2_DELETION_REQUEST_LIFETIME
             )
+            self.prompt = deletion_token.get('prompt', self.prompt)
             user_pk = deletion_token['user_pk']
             self.user = get_user_model().objects.get(pk=user_pk)
             # A user account wont be deactived twice
@@ -1373,20 +1416,29 @@ class ValidateDeletionView(TemplateView):
         messages.error(request, error)
         return utils_misc.redirect(request, 'auth_homepage')
 
+    def get(self, request, *args, **kwargs):
+        if not self.prompt:
+            return self.delete_account(request)
+        return super().get(request, *args, **kwargs)
+
     def post(self, request, *args, **kwargs):
         if 'cancel' not in request.POST:
-            utils_misc.send_account_deletion_mail(self.request, self.user)
-            logger.info('deletion of account %s performed', self.user)
-            hooks.call_hooks('event', name='delete-account', user=self.user)
-            request.journal.record('user.deletion', user=self.user)
-            is_deleted_user_logged = self.user == request.user
-            self.user.delete()
-            messages.info(request, _('Deletion performed.'))
-            # No real use for cancel_url or next_url here, assuming the link
-            # has been received by email. We instead redirect the user to the
-            # homepage.
-            if is_deleted_user_logged:
-                return logout(request, check_referer=False)
+            return self.delete_account(request)
+        return utils_misc.redirect(request, 'auth_homepage')
+
+    def delete_account(self, request):
+        utils_misc.send_account_deletion_mail(self.request, self.user)
+        logger.info('deletion of account %s performed', self.user)
+        hooks.call_hooks('event', name='delete-account', user=self.user)
+        request.journal.record('user.deletion', user=self.user)
+        is_deleted_user_logged = self.user == request.user
+        self.user.delete()
+        messages.info(request, _('Deletion performed.'))
+        # No real use for cancel_url or next_url here, assuming the link
+        # has been received by email. We instead redirect the user to the
+        # homepage.
+        if is_deleted_user_logged:
+            return logout(request, check_referer=False)
         return utils_misc.redirect(request, 'auth_homepage')
 
     def get_context_data(self, **kwargs):

src/authentic2_auth_fc/authenticators.py

@@ -28,6 +28,7 @@ from . import app_settings
 
 class FcAuthenticator(BaseAuthenticator):
     id = 'fc'
+    how = ['france-connect']
     priority = -1
 
     def enabled(self):

src/authentic2_auth_oidc/authenticators.py

@@ -26,6 +26,7 @@ from .models import OIDCProvider
 
 class OIDCAuthenticator(BaseAuthenticator):
     id = 'oidc'
+    how = ['oidc']
     priority = 2
 
     def enabled(self):

src/authentic2_auth_saml/authenticators.py

@@ -28,6 +28,7 @@ from . import app_settings
 class SAMLAuthenticator(BaseAuthenticator):
     id = 'saml'
     priority = 3
+    how = ['saml']
 
     def enabled(self):
         return app_settings.enable and list(get_idps())

tests/conftest.py

@@ -126,7 +126,11 @@ def create_user(**kwargs):
 @pytest.fixture
 def simple_user(db, ou1):
     return create_user(
-        username='user', first_name='Jôhn', last_name='Dôe', email='user@example.net', ou=get_default_ou()
+        username='user',
+        first_name='Jôhn',
+        last_name='Dôe',
+        email='user@example.net',
+        ou=get_default_ou(),
     )
 
 

tests/test_views.py

@@ -62,115 +62,154 @@ def test_well_known_password_change(app):
     assert resp.location == '/accounts/password/change/'
 
 
-def test_account_delete(app, simple_user, mailoutbox):
-    assert simple_user.is_active
-    assert len(mailoutbox) == 0
-    page = login(app, simple_user, path=reverse('delete_account'))
-    assert simple_user.email in page.text
-    page.form.submit(name='submit').follow()
-    assert len(mailoutbox) == 1
-    link = get_link_from_mail(mailoutbox[0])
-    assert mailoutbox[0].subject == 'Validate account deletion request on testserver'
-    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='delete')
-    assert '_auth_user_id' not in app.session
-    assert User.objects.filter(id=simple_user.id).count() == 0
-    assert DeletedUser.objects.filter(old_user_id=simple_user.id).count() == 1
-    assert len(mailoutbox) == 2
-    assert mailoutbox[1].subject == 'Account deletion on testserver'
-    assert mailoutbox[0].to == [simple_user.email]
-    assert "Deletion performed" in str(response)  # Set-Cookie: messages..
-    assert urlparse(response.location).path == '/'
+class TestDeleteAccountEmailVerified:
+    @pytest.fixture
+    def simple_user(self, simple_user):
+        simple_user.email_verified = True
+        simple_user.save()
+        return simple_user
+
+    def test_account_delete(self, app, simple_user, mailoutbox):
+        assert simple_user.is_active
+        assert len(mailoutbox) == 0
+        page = login(app, simple_user, path=reverse('delete_account'))
+        assert simple_user.email in page.text
+        page.form.submit(name='submit').follow()
+        assert len(mailoutbox) == 1
+        link = get_link_from_mail(mailoutbox[0])
+        assert mailoutbox[0].subject == 'Validate account deletion request on testserver'
+        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='delete')
+        assert '_auth_user_id' not in app.session
+        assert User.objects.filter(id=simple_user.id).count() == 0
+        assert DeletedUser.objects.filter(old_user_id=simple_user.id).count() == 1
+        assert len(mailoutbox) == 2
+        assert mailoutbox[1].subject == 'Account deletion on testserver'
+        assert mailoutbox[0].to == [simple_user.email]
+        assert "Deletion performed" in str(response)  # Set-Cookie: messages..
+        assert urlparse(response.location).path == '/'
+
+    def test_account_delete_when_logged_out(self, app, simple_user, mailoutbox):
+        assert simple_user.is_active
+        assert len(mailoutbox) == 0
+        page = login(app, simple_user, path=reverse('delete_account'))
+        page.form.submit(name='submit').follow()
+        assert len(mailoutbox) == 1
+        link = get_link_from_mail(mailoutbox[0])
+        logout(app)
+        page = app.get(link)
+        assert (
+            'You are about to delete the account of <strong>%s</strong>.'
+            % escape(simple_user.get_full_name())
+            in page.text
+        )
+        response = page.form.submit(name='delete')
+        assert User.objects.filter(id=simple_user.id).count() == 0
+        assert DeletedUser.objects.filter(old_user_id=simple_user.id).count() == 1
+        assert len(mailoutbox) == 2
+        assert mailoutbox[1].subject == 'Account deletion on testserver'
+        assert mailoutbox[0].to == [simple_user.email]
+        assert "Deletion performed" in str(response)  # Set-Cookie: messages..
+        assert urlparse(response.location).path == '/'
+
+    def test_account_delete_by_other_user(self, app, simple_user, user_ou1, mailoutbox):
+        assert simple_user.is_active
+        assert user_ou1.is_active
+        assert len(mailoutbox) == 0
+        page = login(app, simple_user, path=reverse('delete_account'))
+        page.form.submit(name='submit').follow()
+        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 (
+            'You are about to delete the account of <strong>%s</strong>.'
+            % escape(simple_user.get_full_name())
+            in page.text
+        )
+        response = page.form.submit(name='delete')
+        assert app.session['_auth_user_id'] == str(user_ou1.id)
+        assert User.objects.filter(id=simple_user.id).count() == 0
+        assert DeletedUser.objects.filter(old_user_id=simple_user.id).count() == 1
+        assert len(mailoutbox) == 2
+        assert mailoutbox[1].subject == 'Account deletion on testserver'
+        assert mailoutbox[0].to == [simple_user.email]
+        assert "Deletion performed" in str(response)  # Set-Cookie: messages..
+        assert urlparse(response.location).path == '/'
+
+    def test_account_delete_fake_token(self, app, simple_user, mailoutbox):
+        response = (
+            app.get(reverse('validate_deletion', kwargs={'deletion_token': 'thisismostlikelynotavalidtoken'}))
+            .follow()
+            .follow()
+        )
+        assert "The account deletion request is invalid, try again" in response.text
+
+    def test_account_delete_expired_token(self, app, simple_user, mailoutbox, freezer):
+        freezer.move_to('2019-08-01')
+        page = login(app, simple_user, path=reverse('delete_account'))
+        page.form.submit(name='submit').follow()
+        freezer.move_to('2019-08-04')  # Too late...
+        link = get_link_from_mail(mailoutbox[0])
+        response = app.get(link).follow()
+        assert "The account deletion request is too old, try again" in response.text
+
+    def test_account_delete_valid_token_unexistent_user(self, app, simple_user, mailoutbox):
+        page = login(app, simple_user, path=reverse('delete_account'))
+        page.form.submit(name='submit').follow()
+        link = get_link_from_mail(mailoutbox[0])
+        simple_user.delete()
+        response = app.get(link).follow().follow()
+        assert 'This account has previously been deleted.' in response.text
+
+    def test_account_delete_valid_token_inactive_user(self, app, simple_user, mailoutbox):
+        page = login(app, simple_user, path=reverse('delete_account'))
+        page.form.submit(name='submit').follow()
+        link = get_link_from_mail(mailoutbox[0])
+        simple_user.is_active = False
+        simple_user.save()
+        response = app.get(link).maybe_follow()
+        assert 'This account is inactive, it cannot be deleted.' in response.text
 
 
-def test_account_delete_when_logged_out(app, simple_user, mailoutbox):
-    assert simple_user.is_active
-    assert len(mailoutbox) == 0
-    page = login(app, simple_user, path=reverse('delete_account'))
-    page.form.submit(name='submit').follow()
-    assert len(mailoutbox) == 1
-    link = get_link_from_mail(mailoutbox[0])
-    logout(app)
-    page = app.get(link)
-    assert (
-        'You are about to delete the account of <strong>%s</strong>.' % escape(simple_user.get_full_name())
-        in page.text
-    )
-    response = page.form.submit(name='delete')
-    assert User.objects.filter(id=simple_user.id).count() == 0
-    assert DeletedUser.objects.filter(old_user_id=simple_user.id).count() == 1
-    assert len(mailoutbox) == 2
-    assert mailoutbox[1].subject == 'Account deletion on testserver'
-    assert mailoutbox[0].to == [simple_user.email]
-    assert "Deletion performed" in str(response)  # Set-Cookie: messages..
-    assert urlparse(response.location).path == '/'
+class TestDeleteAccountEmailNotVerified:
+    def test_account_delete(self, app, simple_user, mailoutbox):
+        assert simple_user.is_active
+        assert len(mailoutbox) == 0
+        page = login(app, simple_user, path=reverse('delete_account'))
+        response = page.form.submit(name='submit').follow()
+        assert '_auth_user_id' not in app.session
+        assert User.objects.filter(id=simple_user.id).count() == 0
+        assert DeletedUser.objects.filter(old_user_id=simple_user.id).count() == 1
+        assert len(mailoutbox) == 1
+        assert mailoutbox[0].subject == 'Account deletion on testserver'
+        assert mailoutbox[0].to == [simple_user.email]
+        assert "Deletion performed" in str(response)  # Set-Cookie: messages..
+        assert urlparse(response.location).path == '/'
 
-
-def test_account_delete_by_other_user(app, simple_user, user_ou1, mailoutbox):
-    assert simple_user.is_active
-    assert user_ou1.is_active
-    assert len(mailoutbox) == 0
-    page = login(app, simple_user, path=reverse('delete_account'))
-    page.form.submit(name='submit').follow()
-    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 (
-        'You are about to delete the account of <strong>%s</strong>.' % escape(simple_user.get_full_name())
-        in page.text
-    )
-    response = page.form.submit(name='delete')
-    assert app.session['_auth_user_id'] == str(user_ou1.id)
-    assert User.objects.filter(id=simple_user.id).count() == 0
-    assert DeletedUser.objects.filter(old_user_id=simple_user.id).count() == 1
-    assert len(mailoutbox) == 2
-    assert mailoutbox[1].subject == 'Account deletion on testserver'
-    assert mailoutbox[0].to == [simple_user.email]
-    assert "Deletion performed" in str(response)  # Set-Cookie: messages..
-    assert urlparse(response.location).path == '/'
-
-
-def test_account_delete_fake_token(app, simple_user, mailoutbox):
-    response = (
-        app.get(reverse('validate_deletion', kwargs={'deletion_token': 'thisismostlikelynotavalidtoken'}))
-        .follow()
-        .follow()
-    )
-    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'))
-    page.form.submit(name='submit').follow()
-    freezer.move_to('2019-08-04')  # Too late...
-    link = get_link_from_mail(mailoutbox[0])
-    response = app.get(link).follow()
-    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'))
-    page.form.submit(name='submit').follow()
-    link = get_link_from_mail(mailoutbox[0])
-    simple_user.delete()
-    response = app.get(link).follow().follow()
-    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'))
-    page.form.submit(name='submit').follow()
-    link = get_link_from_mail(mailoutbox[0])
-    simple_user.is_active = False
-    simple_user.save()
-    response = app.get(link).maybe_follow()
-    assert 'This account is inactive, it cannot be deleted.' in response.text
+    def test_account_delete_old_authentication(self, app, simple_user, mailoutbox, freezer):
+        assert simple_user.is_active
+        assert len(mailoutbox) == 0
+        login(app, simple_user)
+        freezer.move_to(datetime.timedelta(hours=1))
+        redirect = app.get('/accounts/delete/')
+        login_page = redirect.follow()
+        assert 'You must re-authenticate' in login_page
+        login_page.form.set('password', simple_user.username)
+        page = login_page.form.submit(name='login-password-submit').follow()
+        response = page.form.submit(name='submit').follow()
+        assert '_auth_user_id' not in app.session
+        assert User.objects.filter(id=simple_user.id).count() == 0
+        assert DeletedUser.objects.filter(old_user_id=simple_user.id).count() == 1
+        assert len(mailoutbox) == 1
+        assert mailoutbox[0].subject == 'Account deletion on testserver'
+        assert mailoutbox[0].to == [simple_user.email]
+        assert "Deletion performed" in str(response)  # Set-Cookie: messages..
+        assert urlparse(response.location).path == '/'
 
 
 def test_login_invalid_next(app):