feat(api): remove expiration from domain renewal code, closes #422

Links are still invalidated when significant domain attributes change,
such as the domain owner, or the renewal timestamp.  Using the link
thus renders it invalid.

api/desecapi/management/commands/scavenge-unused.py

@@ -1,6 +1,4 @@
 import datetime
-from functools import reduce
-import operator
 
 from django.conf import settings
 from django.core.mail import get_connection, mail_admins

api/desecapi/models.py

@@ -166,8 +166,6 @@ class User(ExportModelOperationsMixin('User'), AbstractBaseUser):
             raise ValueError(f'Cannot send email to user {self.pk} without a good reason: {reason}')
 
         context = context or {}
-        context.setdefault('link_expiration_hours',
-                           settings.VALIDITY_PERIOD_VERIFICATION_SIGNATURE // timedelta(hours=1))
         content = get_template(f'emails/{reason}/content.txt').render(context)
         content += f'\nSupport Reference: user_id = {self.pk}\n'
         footer = get_template('emails/footer.txt').render()

api/desecapi/serializers.py

@@ -638,6 +638,7 @@ class CustomFieldNameUniqueValidator(UniqueValidator):
 
 class AuthenticatedActionSerializer(serializers.ModelSerializer):
     state = serializers.CharField()  # serializer read-write, but model read-only field
+    validity_period = settings.VALIDITY_PERIOD_VERIFICATION_SIGNATURE
 
     class Meta:
         model = models.AuthenticatedAction
@@ -650,11 +651,10 @@ class AuthenticatedActionSerializer(serializers.ModelSerializer):
         return urlsafe_b64encode(payload_enc).decode()
 
     @classmethod
-    def _unpack_code(cls, code):
+    def _unpack_code(cls, code, *, ttl):
         try:
             payload_enc = urlsafe_b64decode(code.encode())
-            payload = crypto.decrypt(payload_enc, context='desecapi.serializers.AuthenticatedActionSerializer',
-                                     ttl=settings.VALIDITY_PERIOD_VERIFICATION_SIGNATURE.total_seconds())
+            payload = crypto.decrypt(payload_enc, context='desecapi.serializers.AuthenticatedActionSerializer', ttl=ttl)
             return json.loads(payload.decode())
         except (TypeError, UnicodeDecodeError, UnicodeEncodeError, json.JSONDecodeError, binascii.Error):
             raise ValueError
@@ -668,16 +668,25 @@ class AuthenticatedActionSerializer(serializers.ModelSerializer):
 
     def to_internal_value(self, data):
         data = data.copy()  # avoid side effect from .pop
+
+        # calculate code TTL
+        validity_period = self.context.get('validity_period', self.validity_period)
+        try:
+            ttl = validity_period.total_seconds()
+        except AttributeError:
+            ttl = None  # infinite
+
+        # decode from single string
         try:
-            # decode from single string
-            unpacked_data = self._unpack_code(self.context['code'])
+            unpacked_data = self._unpack_code(self.context['code'], ttl=ttl)
         except KeyError:
             raise serializers.ValidationError({'code': ['This field is required.']})
         except ValueError:
-            validity = settings.VALIDITY_PERIOD_VERIFICATION_SIGNATURE
-            raise serializers.ValidationError({
-                'code': [f'This code is invalid, most likely because it expired (validity: {validity}).']
-            })
+            if ttl is None:
+                msg = 'This code is invalid.'
+            else:
+                msg = f'This code is invalid, possibly because it expired (validity: {validity_period}).'
+            raise serializers.ValidationError({api_settings.NON_FIELD_ERRORS_KEY: msg, 'code': 'invalid_code'})
 
         # add extra fields added by the user
         unpacked_data.update(**data)
@@ -758,6 +767,7 @@ class AuthenticatedDomainBasicUserActionSerializer(AuthenticatedBasicUserActionS
 
 
 class AuthenticatedRenewDomainBasicUserActionSerializer(AuthenticatedDomainBasicUserActionSerializer):
+    validity_period = None
 
     class Meta(AuthenticatedDomainBasicUserActionSerializer.Meta):
         model = models.AuthenticatedRenewDomainBasicUserAction

api/desecapi/templates/emails/renew-domain/content.txt

@@ -19,7 +19,7 @@ The above domain name(s) are scheduled for deletion on
 your account, we will also delete your account on this date.
 
 To retain your domain name (and account), either change a DNS record
-before that date, or click the following link(s) (valid for {{ link_expiration_hours }} hours):
+before that date, or click the following link(s):
 
 {% for domain in domains %}
   * {{ domain.name }}

api/desecapi/tests/test_user_management.py

@@ -384,7 +384,7 @@ class UserManagementTestCase(DesecTestCase, PublicSuffixMockMixin):
     def assertVerificationFailureExpiredCodeResponse(self, response):
         return self.assertContains(
             response=response,
-            text="This code is invalid, most likely because it expired (validity: ",
+            text="This code is invalid, possibly because it expired (validity: ",
             status_code=status.HTTP_400_BAD_REQUEST
         )
 
@@ -427,7 +427,7 @@ class UserManagementTestCase(DesecTestCase, PublicSuffixMockMixin):
         if tampered_domain is not None:
             path = urlparse(confirmation_link).path
             code = resolve(path).kwargs.get('code')
-            data = AuthenticatedActionSerializer._unpack_code(code)
+            data = AuthenticatedActionSerializer._unpack_code(code, ttl=None)
             data['domain'] = tampered_domain
             tampered_code = AuthenticatedActionSerializer._pack_code(data)
             confirmation_link = confirmation_link.replace(code, tampered_code)
@@ -959,9 +959,12 @@ class RenewTestCase(UserManagementTestCase, DomainOwnerTestCase):
         pattern = r'following link[^:]*:\s+\* ' + domain.name.replace('.', r'\.') + r'\s+([^\s]*)'
         confirmation_link = self.assertRenewDomainEmail(domain.owner.email, body_contains, pattern)
         self.assertConfirmationLinkRedirect(confirmation_link)
-        self.assertRenewDomainVerificationSuccessResponse(self.client.verify(confirmation_link))
+
+        # Use link after 14 days
+        with mock.patch('time.time', return_value=time.time() + 86400*14):
+            self.assertRenewDomainVerificationSuccessResponse(self.client.verify(confirmation_link))
+            self.assertLess(timezone.now() - Domain.objects.get(pk=domain.pk).renewal_changed, timedelta(seconds=1))
         self.assertEqual(Domain.objects.get(pk=domain.pk).renewal_state, Domain.RenewalState.FRESH)
-        self.assertLess(timezone.now() - Domain.objects.get(pk=domain.pk).renewal_changed, timedelta(seconds=1))
 
         # Check that other domains aren't affected
         self.assertGreater(len(self.my_domains), 1)
@@ -984,9 +987,12 @@ class RenewTestCase(UserManagementTestCase, DomainOwnerTestCase):
         pattern = r'following link[^:]*:\s+\* ' + domain.name.replace('.', r'\.') + r'\s+([^\s]*)'
         confirmation_link = self.assertRenewDomainEmail(domain.owner.email, body_contains, pattern)
         self.assertConfirmationLinkRedirect(confirmation_link)
-        self.assertRenewDomainVerificationSuccessResponse(self.client.verify(confirmation_link))
+
+        # Use link after 6 days
+        with mock.patch('time.time', return_value=time.time() + 86400*6):
+            self.assertRenewDomainVerificationSuccessResponse(self.client.verify(confirmation_link))
+            self.assertLess(timezone.now() - Domain.objects.get(pk=domain.pk).renewal_changed, timedelta(seconds=1))
         self.assertEqual(Domain.objects.get(pk=domain.pk).renewal_state, Domain.RenewalState.FRESH)
-        self.assertLess(timezone.now() - Domain.objects.get(pk=domain.pk).renewal_changed, timedelta(seconds=1))
 
         # Check that other domains aren't affected
         self.assertGreater(len(self.my_domains), 1)

api/desecapi/views.py

@@ -1,5 +1,6 @@
 import base64
 import binascii
+from datetime import timedelta
 
 from django.conf import settings
 from django.contrib.auth import user_logged_in
@@ -27,6 +28,13 @@ from desecapi.permissions import IsDomainOwner, IsOwner, IsVPNClient, WithinDoma
 from desecapi.renderers import PlainTextRenderer
 
 
+def generate_confirmation_link(request, action_serializer, viewname, **kwargs):
+    action = action_serializer.Meta.model(**kwargs)
+    action_data = action_serializer(action).data
+    confirmation_link = reverse(viewname, request=request, args=[action_data['code']])
+    return confirmation_link, action_serializer.validity_period
+
+
 class EmptyPayloadMixin:
     def initialize_request(self, request, *args, **kwargs):
         request = super().initialize_request(request, *args, **kwargs)
@@ -447,10 +455,12 @@ class AccountCreateView(generics.CreateAPIView):
             # send email if needed
             domain = serializer.validated_data.get('domain')
             if domain or activation_required:
-                action = models.AuthenticatedActivateUserAction(user=user, domain=domain)
-                verification_code = serializers.AuthenticatedActivateUserActionSerializer(action).data['code']
+                link, validity_period = generate_confirmation_link(request,
+                                                                   serializers.AuthenticatedActivateUserActionSerializer,
+                                                                   'confirm-activate-account', user=user, domain=domain)
                 user.send_email('activate-with-domain' if domain else 'activate', context={
-                    'confirmation_link': reverse('confirm-activate-account', request=request, args=[verification_code]),
+                    'confirmation_link': link,
+                    'link_expiration_hours': validity_period // timedelta(hours=1),
                     'domain': domain,
                 })
 
@@ -480,10 +490,12 @@ class AccountDeleteView(generics.GenericAPIView):
     def post(self, request, *args, **kwargs):
         if self.request.user.domains.exists():
             return self.response_still_has_domains
-        action = models.AuthenticatedDeleteUserAction(user=self.request.user)
-        verification_code = serializers.AuthenticatedDeleteUserActionSerializer(action).data['code']
+        link, validity_period = generate_confirmation_link(request,
+                                                           serializers.AuthenticatedDeleteUserActionSerializer,
+                                                           'confirm-delete-account', user=self.request.user)
         request.user.send_email('delete-user', context={
-            'confirmation_link': reverse('confirm-delete-account', request=request, args=[verification_code])
+            'confirmation_link': link,
+            'link_expiration_hours': validity_period // timedelta(hours=1),
         })
 
         return Response(data={'detail': 'Please check your mailbox for further account deletion instructions.'},
@@ -530,10 +542,12 @@ class AccountChangeEmailView(generics.GenericAPIView):
         serializer.is_valid(raise_exception=True)
         new_email = serializer.validated_data['new_email']
 
-        action = models.AuthenticatedChangeEmailUserAction(user=request.user, new_email=new_email)
-        verification_code = serializers.AuthenticatedChangeEmailUserActionSerializer(action).data['code']
+        link, validity_period = generate_confirmation_link(request,
+                                                           serializers.AuthenticatedChangeEmailUserActionSerializer,
+                                                           'confirm-change-email', user=request.user, new_email=new_email)
         request.user.send_email('change-email', recipient=new_email, context={
-            'confirmation_link': reverse('confirm-change-email', request=request, args=[verification_code]),
+            'confirmation_link': link,
+            'link_expiration_hours': validity_period // timedelta(hours=1),
             'old_email': request.user.email,
             'new_email': new_email,
         })
@@ -565,10 +579,12 @@ class AccountResetPasswordView(generics.GenericAPIView):
 
     @staticmethod
     def send_reset_token(user, request):
-        action = models.AuthenticatedResetPasswordUserAction(user=user)
-        verification_code = serializers.AuthenticatedResetPasswordUserActionSerializer(action).data['code']
+        link, validity_period = generate_confirmation_link(request,
+                                                           serializers.AuthenticatedResetPasswordUserActionSerializer,
+                                                           'confirm-reset-password', user=user)
         user.send_email('reset-password', context={
-            'confirmation_link': reverse('confirm-reset-password', request=request, args=[verification_code])
+            'confirmation_link': link,
+            'link_expiration_hours': validity_period // timedelta(hours=1),
         })
 
 
@@ -588,7 +604,11 @@ class AuthenticatedActionView(generics.GenericAPIView):
         return 'account_management_passive' if self.request.method in SAFE_METHODS else 'account_management_active'
 
     def get_serializer_context(self):
-        return {**super().get_serializer_context(), 'code': self.kwargs['code']}
+        return {
+            **super().get_serializer_context(),
+            'code': self.kwargs['code'],
+            'validity_period': self.get_serializer_class().validity_period,
+        }
 
     def perform_authentication(self, request):
         # Delay authentication until request.auth or request.user is first accessed.