From 9b6820a4ea2a5de8ea43f0c1d9e636176d3ea510 Mon Sep 17 00:00:00 2001 From: Sebastien Mirolo Date: Thu, 1 Aug 2024 15:14:41 -0700 Subject: [PATCH] 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 | 50 ++++++++--- signup/models.py | 144 +++++++++++++++--------------- signup/views/auth.py | 8 -- testsite/fixtures/default-db.json | 10 +++ 4 files changed, 119 insertions(+), 93 deletions(-) diff --git a/signup/mixins.py b/signup/mixins.py index df3ae10..0c2cd13 100644 --- a/signup/mixins.py +++ b/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 diff --git a/signup/models.py b/signup/models.py index e41ee00..eb5b62c 100644 --- a/signup/models.py +++ b/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): """ diff --git a/signup/views/auth.py b/signup/views/auth.py index 1f1de18..6b84d67 100644 --- a/signup/views/auth.py +++ b/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}) diff --git a/testsite/fixtures/default-db.json b/testsite/fixtures/default-db.json index 2981cf0..52b478f 100644 --- a/testsite/fixtures/default-db.json +++ b/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",