fixes 500 in /api/auth/activate/{verification_key} API

When an `email_code` is present in the Contact db record,
`VerifyCompleteMixin.prefetch_contact_info` would add it to `initial_data`,
which then would propagate that `email_code` to `clean_data` and trigger
a call to `finalize_email_verification`.
As `VerifyCompleteMixin.create_user` is called after `auth_check_mfa`,
at the point we attempt to `activate_user`, there would be no record
with the verification_key URL argument in the database.

signup/mixins.py

@@ -547,17 +547,35 @@ class VerifyCompleteMixin(AuthMixin):
     """
     key_url_kwarg = 'verification_key'
 
+    @property
+    def contact(self):
+        if not hasattr(self, '_contact'):
+            #pylint:disable=attribute-defined-outside-init
+            verification_key = self.kwargs.get(self.key_url_kwarg)
+            self._contact = Contact.objects.get_token(verification_key)
+            if not self._contact:
+                raise Http404("Contact could not be found from"\
+                    " verification_key '%s'" % str())
+        return self._contact
+
     def prefetch_contact_info(self):
         data = {}
-        verification_key = self.kwargs.get(self.key_url_kwarg)
-        contact = Contact.objects.get_token(verification_key)
+        contact = self.contact
         if contact:
             fields = []
             if self.form_class is not None:
                 fields = six.iterkeys(self.get_initial())
             elif self.serializer_class is not None and hasattr(
                     self.serializer_class, 'Meta'):
-                fields = self.serializer_class.Meta.fields
+                # We want to pre-populate all fields except write-only
+                # fields obviously (ex: email_code), or read-only fields
+                # (ex: 'created_at').
+                #pylint:disable=protected-access
+                fields = [field_name
+                    for field_name, field_obj in six.iteritems(
+                        self.serializer_class._declared_fields) if not (
+                        field_obj.write_only or field_obj.read_only)]
+                LOGGER.debug("[prefetch_contact_info] fields=%s", str(fields))
             for field_name in fields:
                 field_value = None
                 if contact.user:
@@ -569,8 +587,7 @@ def prefetch_contact_info(self):
         return data
 
     def find_candidate(self, **cleaned_data):
-        verification_key = self.kwargs.get(self.key_url_kwarg)
-        contact = Contact.objects.get_token(verification_key)
+        contact = self.contact
         if not contact:
             raise serializers.ValidationError({
                 'detail': _("verification key not found")})
@@ -595,8 +612,7 @@ def check_password(self, user, **cleaned_data):
                 verification_key, self.request.user)
             try:
                 # There shouldn't be any `User` created, but in case.
-                user, _unused = Contact.objects.activate_user(
-                    verification_key)
+                user, _unused = self.contact.activate_user(verification_key)
             except IntegrityError as err:
                 handle_uniq_error(err)
 
@@ -620,10 +636,15 @@ def create_user(self, **cleaned_data):
                 'detail': _("Update of credentials (password, etc.)"\
                             " has been disabled.")})
 
-        # If we don't save the ``User`` model here,
-        # we won't be able to authenticate later.
+        # It is important to keep the same reference (`self.contact`
+        # cached property) before and after `auth_check_mfa` is called
+        # otherwise a `finalize_*_verification` will have reset verification
+        # keys and we wouldn't be able to get the contact back here
+        # from the URL path argument.
         try:
-            user, previously_inactive = Contact.objects.activate_user(
+            # If we don't save the ``User`` model here,
+            # we won't be able to authenticate later.
+            user, previously_inactive = self.contact.activate_user(
                 verification_key,
                 username=cleaned_data.get('username'),
                 password=cleaned_data.get('new_password'),
@@ -646,16 +667,17 @@ def create_user(self, **cleaned_data):
                     user.get_full_name(), user.email, user,
                     extra={'event': 'activate', 'user': user})
                 signals.user_activated.send(sender=__name__, user=user,
-                    verification_key=self.kwargs.get(self.key_url_kwarg),
+                    verification_key=verification_key,
                     request=self.request)
             else:
                 LOGGER.info("%s password reset", user)
                 signals.user_reset_password.send(
                     sender=__name__, user=user, request=self.request)
 
-        # Bypassing authentication here, we are doing frictionless registration
-        # the first time around.
-        user.backend = self.backend_path
+            # Bypassing authentication here, we are doing frictionless
+            # registration the first time around.
+            user.backend = self.backend_path
+
         return user
 
 

signup/models.py

@@ -450,6 +450,7 @@ def finalize_email_verification(self, email, email_code, at_time=None):
         Checks the email_code matches the code that was sent
         to the email address.
         """
+        assert email and email_code
         if not at_time:
             at_time = datetime_or_now()
         email_filter = models.Q(email__iexact=email)
@@ -461,15 +462,17 @@ def finalize_email_verification(self, email, email_code, at_time=None):
         contact = queryset.get()
         contact.email_verification_key = None
         contact.email_verified_at = at_time
-        contact.email_code = generate_random_code()
+        contact.email_code = None
         contact.save()
         return contact
 
+
     def finalize_phone_verification(self, phone, phone_code, at_time=None):
         """
         Checks the phone_code matches the code that was sent
         to the phone number.
         """
+        assert phone and phone_code
         if not at_time:
             at_time = datetime_or_now()
         phone_filter = models.Q(phone=phone)
@@ -480,79 +483,10 @@ def finalize_phone_verification(self, phone, phone_code, at_time=None):
         contact = self.filter(phone_filter).select_related('user').get()
         contact.phone_verification_key = None
         contact.phone_verified_at = at_time
-        contact.phone_code = generate_random_code()
+        contact.phone_code = None
         contact.save()
         return contact
 
-    def activate_user(self, verification_key,
-                      username=None, password=None, full_name=None):
-        """
-        Activate a user whose email address has been verified.
-        """
-        #pylint:disable=too-many-arguments
-        at_time = datetime_or_now()
-        try:
-            token = self.get_token(verification_key=verification_key)
-            if token:
-                token_username = (
-                    token.user.username if token.user else token.slug)
-                LOGGER.info('active user %s through code: %s ...',
-                    token_username, verification_key,
-                    extra={'event': 'activate', 'username': token_username,
-                        'verification_key': verification_key,
-                        'email_verification_key': token.email_verification_key,
-                        'phone_verification_key': token.phone_verification_key})
-                user_model = get_user_model()
-                with transaction.atomic(using=self._db):
-                    if token.email_verification_key == verification_key:
-                        token.email_verification_key = None
-                        token.email_verified_at = at_time
-                    elif token.phone_verification_key == verification_key:
-                        token.phone_verification_key = None
-                        token.phone_verified_at = at_time
-                    if not token.user:
-                        try:
-                            token.user = user_model.objects.get(
-                                email__iexact=token.email)
-                        except user_model.DoesNotExist:
-                            token.user = user_model.objects.create_user(
-                                username, email=token.email,
-                                password=password)
-                    token.save()
-                    previously_inactive = has_invalid_password(token.user)
-                    needs_save = False
-                    if full_name:
-                        token.full_name = full_name
-                        #pylint:disable=unused-variable
-                        first_name, mid_name, last_name = \
-                            full_name_natural_split(full_name)
-                        token.user.first_name = first_name
-                        token.user.last_name = last_name
-                        LOGGER.info('%s (first_name, last_name) needs '\
-                            'to be saved as ("%s", "%s")', verification_key,
-                            token.user.first_name, token.user.last_name)
-                        needs_save = True
-                    if username:
-                        token.user.username = username
-                        LOGGER.info('%s username needs to be saved as "%s"',
-                            verification_key, token.user.username)
-                        needs_save = True
-                    if password:
-                        token.user.set_password(password)
-                        LOGGER.info('%s password needs to be saved',
-                            verification_key)
-                        needs_save = True
-                    if not token.user.is_active:
-                        token.user.is_active = True
-                        LOGGER.info('%s user needs to be activated',
-                            verification_key)
-                        needs_save = True
-                    if needs_save:
-                        token.user.save()
-                return token.user, previously_inactive
-        except Contact.DoesNotExist:
-            pass # We return None instead here.
-        return None, None
 
     def is_reachable_by_email(self, user, at_time=None):
         """
@@ -565,6 +499,7 @@ def is_reachable_by_email(self, user, at_time=None):
                 - settings.VERIFICATION_LIFETIME)
         return self.filter(user=user).exclude(verified_filter).exists()
 
+
     def is_reachable_by_phone(self, user, at_time=None):
         """
         Returns True if the user is reachable by phone.
@@ -582,6 +517,8 @@ class Contact(models.Model):
     """
     Used in workflow to verify the email address of a ``User``.
     """
+    #pylint:disable=too-many-instance-attributes
+
     objects = ContactManager()
 
     slug = models.SlugField(unique=True,
@@ -731,6 +668,71 @@ def save(self, force_insert=False, force_update=False,
                 % {'base': slug_base}})
 
 
+    def activate_user(self, verification_key, username=None, password=None,
+                      full_name=None, at_time=None):
+        """
+        Activate a user whose email address has been verified.
+        """
+        #pylint:disable=too-many-arguments
+        if not at_time:
+            at_time = datetime_or_now()
+        token_username = (self.user.username if self.user else self.slug)
+        LOGGER.info('active user %s through code: %s ...',
+            token_username, verification_key,
+            extra={'event': 'activate', 'username': token_username,
+                'verification_key': verification_key,
+                'email_verification_key': self.email_verification_key,
+                'phone_verification_key': self.phone_verification_key})
+        user_model = get_user_model()
+        with transaction.atomic(using=self._state.db):
+            if self.email_verification_key == verification_key:
+                self.email_verification_key = None
+                self.email_verified_at = at_time
+                self.email_code = None
+            elif self.phone_verification_key == verification_key:
+                self.phone_verification_key = None
+                self.phone_verified_at = at_time
+                self.phone_code = None
+            if not self.user:
+                try:
+                    self.user = user_model.objects.get(email__iexact=self.email)
+                except user_model.DoesNotExist:
+                    self.user = user_model.objects.create_user(
+                        username, email=self.email, password=password)
+            self.save()
+            previously_inactive = has_invalid_password(self.user)
+            needs_save = False
+            if full_name:
+                self.full_name = full_name
+                #pylint:disable=unused-variable
+                first_name, mid_name, last_name = full_name_natural_split(
+                    full_name)
+                self.user.first_name = first_name
+                self.user.last_name = last_name
+                LOGGER.info('%s (first_name, last_name) needs '\
+                    'to be saved as ("%s", "%s")', verification_key,
+                    self.user.first_name, self.user.last_name)
+                needs_save = True
+            if username:
+                self.user.username = username
+                LOGGER.info('%s username needs to be saved as "%s"',
+                    verification_key, self.user.username)
+                needs_save = True
+            if password:
+                self.user.set_password(password)
+                LOGGER.info('%s password needs to be saved',
+                    verification_key)
+                needs_save = True
+            if not self.user.is_active:
+                self.user.is_active = True
+                LOGGER.info('%s user needs to be activated',
+                    verification_key)
+                needs_save = True
+            if needs_save:
+                self.user.save()
+        return self.user, previously_inactive
+
+
 @python_2_unicode_compatible
 class Activity(models.Model):
     """

signup/views/auth.py

@@ -189,14 +189,6 @@ class ActivationView(VerifyCompleteMixin, AuthResponseMixin, View):
     form_class = ActivationForm
     template_name = 'accounts/activate/verification_key.html'
 
-    @property
-    def contact(self):
-        if not hasattr(self, '_contact'):
-            #pylint:disable=attribute-defined-outside-init
-            self._contact = Contact.objects.get_token(
-                self.kwargs.get(self.key_url_kwarg))
-        return self._contact
-
     def get_context_data(self, **kwargs):
         context = super(ActivationView, self).get_context_data(**kwargs)
         context.update({'object': self.contact})

testsite/fixtures/default-db.json

@@ -279,6 +279,16 @@
     },
     "model": "signup.Contact", "pk": 13
 },
+{
+    "fields": {
+      "slug": "activate-no-data",
+      "created_at": "2024-01-01T00:00:00-00:00",
+      "email": "xia+14@localhost.localdomain",
+      "email_verification_key": "2bfd21255cc906781afca502ec2e4f8074244f80",
+      "email_code": 21945
+    },
+    "model": "signup.Contact", "pk": 14
+},
 {
     "fields": {
       "date_joined": "2022-01-01T00:00:00+00:00",