Repository: pennersr/django-allauth
Pull request #3346 (2023-07-12 14:05:52)

feat(account): Add `ACCOUNT_CHANGE_EMAIL` flow
With these changes, when `ACCOUNT_MAX_EMAIL_ADRESSES=1`, the user can still change his email. The user is allowed to add an extra email, and when that email is confirmed, the former email is removed. So, the user is allowed to go over the maximum of 1 for the purpose of changing his email.


coveralls: 
[![Coverage Status](https://coveralls.io/builds/61297956/badge)](https://coveralls.io/builds/61297956)

coverage: 91.274% (-0.02%) from 91.29% when pulling **b4acaa7f43139cfbd61a2fd0dcdd5e06824c463b on feat-change-email** into **d628c2ddcb8d6aee23ade1ea2d94c6ca334bce61 on main**.

pennersr: @9mido and @pktiuk -- does this solve your use cases?
e2489: This still seems overly complicated and potentially confusing for users. Having to add an extra email in order to change your current email is an odd way to do things, and will probably not make sense to the average person.

I get that that's how it has to work on the back end, and that the end result is the same, but from the user's perspective, they should simply be able to change their email. That's the way every site has done things for decades now, and this shouldn't be tied into the 'multiple emails' feature, which offers something different, and is quite nonstandard these days.
pennersr: @e2489 Valid remark, but isn't this solely a presentation concern? With a few changes to the templates, you turn "Add" into "Change" and instead of showing the user a manageable list of email addresses you can show the user just his current one and the one he is trying to change to, if any.
pennersr: Changes:
- Added an explicit setting `ACCOUNT_CHANGE_EMAIL` for this, instead of hiding it behind `ACCOUNT_MAX_EMAIL_ADDRESSES=1`.
- Template tweaks -- "Change email" instead of "Add email".
pennersr: There is now a dedicated setting for this and a separate template addressing the presentation concerns. Any other feedback?

Patch:
diff --git a/.editorconfig b/.editorconfig
index d29698dc..b455f76e 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -15,6 +15,9 @@ indent_size = 4
 [*.py]
 indent_size = 4
 
+[*.html]
+indent_size = 4
+
 [Makefile]
 indent_style = tab
 indent_size = 8
diff --git a/ChangeLog.rst b/ChangeLog.rst
index 46c9d891..64ec40ed 100644
--- a/ChangeLog.rst
+++ b/ChangeLog.rst
@@ -16,6 +16,11 @@ Note worthy changes
 - It is now possible to manage OpenID Connect providers via the Django
   admin. Simply add a `SocialApp` for each OpenID Connect provider.
 
+- There is now a new flow for changing the email address. When enabled
+  (``ACCOUNT_CHANGE_EMAIL``), users are limited to having exactly one email
+  address that they can change by adding a temporary second email address that,
+  when verified, replaces the current email address.
+
 
 Security notice
 ---------------
diff --git a/allauth/account/adapter.py b/allauth/account/adapter.py
index 871d581b..cd609957 100644
--- a/allauth/account/adapter.py
+++ b/allauth/account/adapter.py
@@ -474,10 +474,29 @@ class DefaultAccountAdapter(object):
         """
         Marks the email address as confirmed on the db
         """
+        from allauth.account.models import EmailAddress
+
+        from_email_address = (
+            EmailAddress.objects.filter(user_id=email_address.user_id)
+            .exclude(pk=email_address.pk)
+            .first()
+        )
         if not email_address.set_verified(commit=False):
             return False
-        email_address.set_as_primary(conditional=True)
+        email_address.set_as_primary(conditional=(not app_settings.CHANGE_EMAIL))
         email_address.save(update_fields=["verified", "primary"])
+        if app_settings.CHANGE_EMAIL:
+            for instance in EmailAddress.objects.filter(
+                user_id=email_address.user_id
+            ).exclude(pk=email_address.pk):
+                instance.remove()
+            signals.email_changed.send(
+                sender=get_user_model(),
+                request=request,
+                user=email_address.user,
+                from_email_address=from_email_address,
+                to_email_address=email_address,
+            )
         return True
 
     def set_password(self, user, password):
diff --git a/allauth/account/app_settings.py b/allauth/account/app_settings.py
index 4e053866..1246e952 100644
--- a/allauth/account/app_settings.py
+++ b/allauth/account/app_settings.py
@@ -1,3 +1,6 @@
+from django.core.exceptions import ImproperlyConfigured
+
+
 class AppSettings(object):
     class AuthenticationMethod:
         USERNAME = "username"
@@ -35,6 +38,11 @@ class AppSettings(object):
             )
         if self.MAX_EMAIL_ADDRESSES is not None:
             assert self.MAX_EMAIL_ADDRESSES > 0
+        if self.CHANGE_EMAIL:
+            if self.MAX_EMAIL_ADDRESSES is not None and self.MAX_EMAIL_ADDRESSES != 2:
+                raise ImproperlyConfigured(
+                    "Invalid combination of ACCOUNT_CHANGE_EMAIL and ACCOUNT_MAX_EMAIL_ADDRESSES"
+                )
 
     def _setting(self, name, dflt):
         from django.conf import settings
@@ -119,6 +127,10 @@ class AppSettings(object):
     def MAX_EMAIL_ADDRESSES(self):
         return self._setting("MAX_EMAIL_ADDRESSES", None)
 
+    @property
+    def CHANGE_EMAIL(self):
+        return self._setting("CHANGE_EMAIL", False)
+
     @property
     def AUTHENTICATION_METHOD(self):
         ret = self._setting("AUTHENTICATION_METHOD", self.AuthenticationMethod.USERNAME)
diff --git a/allauth/account/forms.py b/allauth/account/forms.py
index 497fa3fc..222bda95 100644
--- a/allauth/account/forms.py
+++ b/allauth/account/forms.py
@@ -499,6 +499,10 @@ class AddEmailForm(UserForm):
         return value
 
     def save(self, request):
+        if app_settings.CHANGE_EMAIL:
+            return EmailAddress.objects.add_new_email(
+                request, self.user, self.cleaned_data["email"]
+            )
         return EmailAddress.objects.add_email(
             request, self.user, self.cleaned_data["email"], confirm=True
         )
diff --git a/allauth/account/managers.py b/allauth/account/managers.py
index 1e7ea0ec..74638c49 100644
--- a/allauth/account/managers.py
+++ b/allauth/account/managers.py
@@ -10,11 +10,44 @@ from . import app_settings
 class EmailAddressManager(models.Manager):
     def can_add_email(self, user):
         ret = True
-        if app_settings.MAX_EMAIL_ADDRESSES:
+        if app_settings.CHANGE_EMAIL:
+            #  We always allow adding an email in this case, regardless of
+            # `MAX_EMAIL_ADDRESSES`, as adding actually adds a temporary email
+            # that the user wants to change to.
+            return True
+        elif app_settings.MAX_EMAIL_ADDRESSES:
             count = self.filter(user=user).count()
             ret = count < app_settings.MAX_EMAIL_ADDRESSES
         return ret
 
+    def get_new(self, user):
+        """
+        Returns the email address the user is in the process of changing to, if any.
+        """
+        assert app_settings.CHANGE_EMAIL
+        return (
+            self.model.objects.filter(user=user, verified=False).order_by("pk").last()
+        )
+
+    def add_new_email(self, request, user, email):
+        """
+        Adds an email address the user wishes to change to, replacing his
+        current email address once confirmed.
+        """
+        assert app_settings.CHANGE_EMAIL
+        instance = self.get_new(user)
+        if not instance:
+            instance = self.model.objects.create(user=user, email=email)
+        else:
+            # Apparently, the user was already in the process of changing his
+            # email.  Reuse that temporary email address.
+            instance.email = email
+            instance.verified = False
+            instance.primary = False
+            instance.save()
+        instance.send_confirmation(request)
+        return instance
+
     def add_email(self, request, user, email, confirm=False, signup=False):
         email_address, created = self.get_or_create(
             user=user, email__iexact=email, defaults={"email": email}
@@ -25,6 +58,9 @@ class EmailAddressManager(models.Manager):
 
         return email_address
 
+    def get_verified(self, user):
+        return self.filter(user=user, verified=True).order_by("-primary", "pk").first()
+
     def get_primary(self, user):
         try:
             return self.get(user=user, primary=True)
diff --git a/allauth/account/models.py b/allauth/account/models.py
index c8aaa301..27d5b487 100644
--- a/allauth/account/models.py
+++ b/allauth/account/models.py
@@ -67,6 +67,9 @@ class EmailAddress(models.Model):
         return self.verified
 
     def set_as_primary(self, conditional=False):
+        """Marks the email address as primary. In case of `conditional`, it is
+        only marked as primary if there is no other primary email address set.
+        """
         old_primary = EmailAddress.objects.get_primary(self.user)
         if old_primary:
             if conditional:
@@ -75,9 +78,7 @@ class EmailAddress(models.Model):
             old_primary.save()
         self.primary = True
         self.save()
-        user_email(self.user, self.email)
-        if app_settings.USER_MODEL_EMAIL_FIELD:
-            self.user.save(update_fields=[app_settings.USER_MODEL_EMAIL_FIELD])
+        user_email(self.user, self.email, commit=True)
         return True
 
     def send_confirmation(self, request=None, signup=False):
@@ -102,7 +103,30 @@ class EmailAddress(models.Model):
             user_email(self.user, alt_email, commit=True)
 
 
-class EmailConfirmation(models.Model):
+class EmailConfirmationMixin:
+    def confirm(self, request):
+        email_address = self.email_address
+        if not email_address.verified:
+            confirmed = get_adapter(request).confirm_email(request, email_address)
+            if confirmed:
+                signals.email_confirmed.send(
+                    sender=self.__class__,
+                    request=request,
+                    email_address=email_address,
+                )
+                return email_address
+
+    def send(self, request=None, signup=False):
+        get_adapter(request).send_confirmation_mail(request, self, signup)
+        signals.email_confirmation_sent.send(
+            sender=self.__class__,
+            request=request,
+            confirmation=self,
+            signup=signup,
+        )
+
+
+class EmailConfirmation(EmailConfirmationMixin, models.Model):
     email_address = models.ForeignKey(
         EmailAddress,
         verbose_name=_("e-mail address"),
@@ -135,30 +159,16 @@ class EmailConfirmation(models.Model):
     key_expired.boolean = True
 
     def confirm(self, request):
-        if not self.key_expired() and not self.email_address.verified:
-            email_address = self.email_address
-            confirmed = get_adapter(request).confirm_email(request, email_address)
-            if confirmed:
-                signals.email_confirmed.send(
-                    sender=self.__class__,
-                    request=request,
-                    email_address=email_address,
-                )
-                return email_address
+        if not self.key_expired():
+            return super().confirm(request)
 
     def send(self, request=None, signup=False):
-        get_adapter(request).send_confirmation_mail(request, self, signup)
+        super().send(request=request, signup=signup)
         self.sent = timezone.now()
         self.save()
-        signals.email_confirmation_sent.send(
-            sender=self.__class__,
-            request=request,
-            confirmation=self,
-            signup=signup,
-        )
 
 
-class EmailConfirmationHMAC:
+class EmailConfirmationHMAC(EmailConfirmationMixin, object):
     def __init__(self, email_address):
         self.email_address = email_address
 
@@ -179,24 +189,3 @@ class EmailConfirmationHMAC:
         ):
             ret = None
         return ret
-
-    def confirm(self, request):
-        if not self.email_address.verified:
-            email_address = self.email_address
-            confirmed = get_adapter(request).confirm_email(request, email_address)
-            if confirmed:
-                signals.email_confirmed.send(
-                    sender=self.__class__,
-                    request=request,
-                    email_address=email_address,
-                )
-                return email_address
-
-    def send(self, request=None, signup=False):
-        get_adapter(request).send_confirmation_mail(request, self, signup)
-        signals.email_confirmation_sent.send(
-            sender=self.__class__,
-            request=request,
-            confirmation=self,
-            signup=signup,
-        )
diff --git a/allauth/account/views.py b/allauth/account/views.py
index 19bb921d..6f3e7abb 100644
--- a/allauth/account/views.py
+++ b/allauth/account/views.py
@@ -428,7 +428,9 @@ confirm_email = ConfirmEmailView.as_view()
 
 @method_decorator(rate_limit(action="manage_email"), name="dispatch")
 class EmailView(AjaxCapableProcessFormViewMixin, FormView):
-    template_name = "account/email." + app_settings.TEMPLATE_EXTENSION
+    template_name = (
+        "account/email_change." if app_settings.CHANGE_EMAIL else "account/email."
+    ) + app_settings.TEMPLATE_EXTENSION
     form_class = AddEmailForm
     success_url = reverse_lazy("account_email")
 
@@ -565,10 +567,26 @@ class EmailView(AjaxCapableProcessFormViewMixin, FormView):
 
     def get_context_data(self, **kwargs):
         ret = super(EmailView, self).get_context_data(**kwargs)
-        # NOTE: For backwards compatibility
-        ret["add_email_form"] = ret.get("form")
-        # (end NOTE)
-        ret["can_add_email"] = EmailAddress.objects.can_add_email(self.request.user)
+        ret.update(
+            {
+                "emailaddresses": list(
+                    EmailAddress.objects.filter(user=self.request.user).order_by(
+                        "email"
+                    )
+                ),
+                "add_email_form": ret.get("form"),
+                "can_add_email": EmailAddress.objects.can_add_email(self.request.user),
+            }
+        )
+        if app_settings.CHANGE_EMAIL:
+            ret.update(
+                {
+                    "new_emailaddress": EmailAddress.objects.get_new(self.request.user),
+                    "current_emailaddress": EmailAddress.objects.get_verified(
+                        self.request.user
+                    ),
+                }
+            )
         return ret
 
     def get_ajax_data(self):
diff --git a/allauth/templates/account/email.html b/allauth/templates/account/email.html
index 373cd669..46c9f49b 100644
--- a/allauth/templates/account/email.html
+++ b/allauth/templates/account/email.html
@@ -6,18 +6,18 @@
 
 {% block content %}
     <h1>{% trans "E-mail Addresses" %}</h1>
-{% if user.emailaddress_set.all %}
+{% if emailaddresses %}
 <p>{% trans 'The following e-mail addresses are associated with your account:' %}</p>
 
 <form action="{% url 'account_email' %}" class="email_list" method="post">
 {% csrf_token %}
 <fieldset class="blockLabels">
 
-  {% for emailaddress in user.emailaddress_set.all %}
+  {% for emailaddress in emailaddresses %}
 <div class="ctrlHolder">
       <label for="email_radio_{{forloop.counter}}" class="{% if emailaddress.primary %}primary_email{%endif%}">
 
-      <input id="email_radio_{{forloop.counter}}" type="radio" name="email" {% if emailaddress.primary or user.emailaddress_set.count == 1 %}checked="checked"{%endif %} value="{{emailaddress.email}}"/>
+      <input id="email_radio_{{forloop.counter}}" type="radio" name="email" {% if emailaddress.primary or emailaddresses|length == 1 %}checked="checked"{%endif %} value="{{emailaddress.email}}"/>
 
 {{ emailaddress.email }}
     {% if emailaddress.verified %}
@@ -40,8 +40,7 @@
 </form>
 
 {% else %}
-<p><strong>{% trans 'Warning:'%}</strong> {% trans "You currently do not have any e-mail address set up. You should really add an e-mail address so you can receive notifications, reset your password, etc." %}</p>
-
+{% include "account/snippets/warn_no_email.html" %}
 {% endif %}
 
   {% if can_add_email %}
diff --git a/allauth/templates/account/email_change.html b/allauth/templates/account/email_change.html
new file mode 100644
index 00000000..262838cf
--- /dev/null
+++ b/allauth/templates/account/email_change.html
@@ -0,0 +1,33 @@
+{% extends "account/base.html" %}
+{% load i18n %}
+{% block head_title %}
+    {% trans "E-mail Address" %}
+{% endblock %}
+{% block content %}
+    <h1>{% trans "E-mail Address" %}</h1>
+    {% if emailaddresses %}
+        {% if current_emailaddress %}
+            <p>
+                {% trans 'The following e-mail address is associated with your account:' %} <a href="mailto:{{ current_emailaddress.email }}">{{ current_emailaddress.email }}</a>
+            </p>
+        {% endif %}
+        {% if new_emailaddress %}
+            <p>
+                {% trans 'Your e-mail address is still pending verification:' %} <a href="mailto:{{ new_emailaddress.email }}">{{ new_emailaddress.email }}</a>
+            </p>
+            <form method="post" action="{% url 'account_email' %}">
+                {% csrf_token %}
+                <input type="hidden" name="email" value="{{ new_emailaddress.email }}">
+                <button type="submit" name="action_send">{% trans 'Re-send Verification' %}</button>
+            </form>
+        {% endif %}
+    {% else %}
+        {% include "account/snippets/warn_no_email.html" %}
+    {% endif %}
+    <h2>{% trans "Change E-mail Address" %}</h2>
+    <form method="post" action="{% url 'account_email' %}">
+        {% csrf_token %}
+        {{ form.as_p }}
+        <button name="action_add" type="submit">{% trans "Change E-mail" %}</button>
+    </form>
+{% endblock %}
diff --git a/allauth/templates/account/snippets/warn_no_email.html b/allauth/templates/account/snippets/warn_no_email.html
new file mode 100644
index 00000000..f5f72ccd
--- /dev/null
+++ b/allauth/templates/account/snippets/warn_no_email.html
@@ -0,0 +1,4 @@
+{% load i18n %}
+<p>
+    <strong>{% trans 'Warning:' %}</strong> {% trans "You currently do not have any e-mail address set up. You should really add an e-mail address so you can receive notifications, reset your password, etc." %}
+</p>
diff --git a/docs/configuration.rst b/docs/configuration.rst
index 8fa6c4a8..9a37259d 100644
--- a/docs/configuration.rst
+++ b/docs/configuration.rst
@@ -19,6 +19,13 @@ ACCOUNT_AUTHENTICATION_METHOD (="username" | "email" | "username_email")
   entering their username, e-mail address, or either one of both.
   Setting this to "email" requires ACCOUNT_EMAIL_REQUIRED=True
 
+ACCOUNT_CHANGE_EMAIL (=False)
+  When disabled (``False``), users can add one or more email addresses (up to a
+  maximum of ``ACCOUNT_MAX_EMAIL_ADDRESSES``) to their account and freely manage
+  those email addresses. When enabled (``True``), users are limited to having
+  exactly one email address that they can change by adding a temporary second
+  email address that, when verified, replaces the current email address.
+
 ACCOUNT_CONFIRM_EMAIL_ON_GET (=False)
   Determines whether or not an e-mail address is automatically confirmed by
   a GET request. `GET is not designed to modify the server state
@@ -94,9 +101,7 @@ ACCOUNT_MAX_EMAIL_ADDRESSES(=None)
   is safe to change this setting for an already running project -- it will not
   negatively affect users that already exceed the allowed amount. Note that if
   you set the maximum to 1, users will not be able to change their email address
-  as they are unable to add the new address, followed by removing the old
-  address.
-
+  unless you enable ``ACCOUNT_CHANGE_EMAIL``.
 
 ACCOUNT_FORMS (={})
   Used to override forms, for example:


Test patch:
diff --git a/allauth/account/tests/test_change_email.py b/allauth/account/tests/test_change_email.py
index 486c1e00..9861d4b6 100644
--- a/allauth/account/tests/test_change_email.py
+++ b/allauth/account/tests/test_change_email.py
@@ -1,11 +1,12 @@
 from __future__ import absolute_import
 
 import json
+from unittest.mock import patch
 
 from django.test.utils import override_settings
 from django.urls import reverse
 
-from allauth.account.models import EmailAddress
+from allauth.account.models import EmailAddress, EmailConfirmationHMAC
 from allauth.account.utils import user_email
 from allauth.tests import TestCase
 from allauth.utils import get_user_model
@@ -224,7 +225,7 @@ def test_delete_email_changes_user_email(user_factory, client, email_factory):
     assert user_email(user) == other_verified_email.email
 
 
-def test_delete_email_wipes_user_email(user_factory, client, email_factory):
+def test_delete_email_wipes_user_email(user_factory, client):
     user = user_factory(email_verified=False)
     client.force_login(user)
     first_email = EmailAddress.objects.get(user=user)
@@ -238,3 +239,28 @@ def test_delete_email_wipes_user_email(user_factory, client, email_factory):
     assert resp.status_code == 302
     user.refresh_from_db()
     assert user_email(user) == ""
+
+
+def test_change_email(user_factory, client, settings):
+    settings.ACCOUNT_CHANGE_EMAIL = True
+    settings.ACCOUNT_EMAIL_CONFIRMATION_HMAC = True
+
+    user = user_factory(email_verified=True)
+    client.force_login(user)
+    current_email = EmailAddress.objects.get(user=user)
+    resp = client.post(
+        reverse("account_email"),
+        {"action_add": "", "email": "change-to@this.org"},
+    )
+    assert resp.status_code == 302
+    new_email = EmailAddress.objects.get(email="change-to@this.org")
+    key = EmailConfirmationHMAC(new_email).key
+    with patch("allauth.account.signals.email_changed.send") as email_changed_mock:
+        resp = client.post(reverse("account_confirm_email", args=[key]))
+    assert resp.status_code == 302
+    assert not EmailAddress.objects.filter(pk=current_email.pk).exists()
+    assert EmailAddress.objects.filter(user=user).count() == 1
+    new_email.refresh_from_db()
+    assert new_email.verified
+    assert new_email.primary
+    assert email_changed_mock.called