Fix a race condition in the "Change Email" flow

The race scenario was as follows:

* Alice initiates email address change to bob@example.org
* a verification link is sent to bob@example.org
* separately, somebody creates a new account for bob@example.org
* Alice clicks on the verification link

At this point,
- if the database has an uniqueness constraint on auth_user.email,
  Alice will receive a HTTP 500 error
- if there's no uniqueness constraint, the email change
  will succeed and the system will have two accounts with the
  same email address

The simple fix is to re-check the address availability just
before finalizing the email address change. Currently this is
not done in a transaction block, so the race condition still
exists in theory, but is much less likely to happen in practice.

CHANGELOG.md

@@ -15,6 +15,7 @@ All notable changes to this project will be documented in this file.
 
 ### Bug Fixes
 - Fix the display of ignored pings with non-zero exit status
+- Fix a race condition in the "Change Email" flow
 
 ## v2.2.1 - 2022-06-13
 

hc/accounts/tests/test_change_email_verify.py

@@ -1,6 +1,7 @@
 from unittest.mock import patch
 
 from django.contrib.auth.hashers import make_password
+from django.contrib.auth.models import User
 from django.core.signing import TimestampSigner
 from hc.test import BaseTestCase
 
@@ -11,7 +12,7 @@ class ChangeEmailVerifyTestCase(BaseTestCase):
         self.profile.token = make_password("secret-token", "login")
         self.profile.save()
 
-        self.checks_url = "/projects/%s/checks/" % self.project.code
+        self.checks_url = f"/projects/{self.project.code}/checks/"
 
     def _url(self, expired=False):
         payload = {
@@ -62,3 +63,15 @@ class ChangeEmailVerifyTestCase(BaseTestCase):
     def test_it_handles_bad_payload(self):
         r = self.client.post("/accounts/change_email/bad-payload/")
         self.assertContains(r, "The link you just used is incorrect.")
+
+    def test_it_handles_unavailable_email(self):
+        # Make the target address unavailable
+        User.objects.create(email="alice+new@example.org")
+
+        r = self.client.post(self._url(), follow=True)
+        self.assertRedirects(r, "/accounts/login/")
+        self.assertContains(r, "incorrect or expired")
+
+        # Alice's email should have *not* been updated
+        self.alice.refresh_from_db()
+        self.assertEqual(self.alice.email, "alice@example.org")

hc/accounts/views.py

@@ -230,6 +230,10 @@ def check_token(request, username, token, new_email=None):
     user = authenticate(username=username, token=token)
     if user is not None and user.is_active:
         if new_email:
+            if User.objects.filter(email=new_email).exists():
+                request.session["bad_link"] = True
+                return redirect("hc-login")
+
             user.email = new_email
             user.set_unusable_password()
             user.save()