feat(api): require POST for authenticated action links

api/desecapi/tests/test_user_management.py

@@ -74,7 +74,7 @@ class UserManagementClient(APIClient):
         return self.get(reverse('v1:account'), HTTP_AUTHORIZATION='Token {}'.format(token))
 
     def verify(self, url, data=None, **kwargs):
-        return self.post(url, data, **kwargs) if data else self.get(url, **kwargs)
+        return self.post(url, data, **kwargs)
 
     def obtain_captcha(self, **kwargs):
         return self.post(reverse('v1:captcha'))
@@ -164,6 +164,13 @@ class UserManagementTestCase(DesecTestCase, PublicSuffixMockMixin):
             mail.outbox = []
         return body if not pattern else re.search(pattern, body).group(1)
 
+    def assertConfirmationLinkRedirect(self, confirmation_link):
+        response = self.client.get(confirmation_link)
+        self.assertResponse(response, status.HTTP_406_NOT_ACCEPTABLE)
+        response = self.client.get(confirmation_link, HTTP_ACCEPT='text/html')
+        self.assertResponse(response, status.HTTP_302_FOUND)
+        self.assertNoEmailSent()
+
     def assertRegistrationEmail(self, recipient, reset=True):
         return self.assertEmailSent(
             subject_contains='deSEC',
@@ -377,6 +384,7 @@ class UserManagementTestCase(DesecTestCase, PublicSuffixMockMixin):
         self.assertFalse(User.objects.get(email=email).is_active)
         self.assertPassword(email, password)
         confirmation_link = self.assertRegistrationEmail(email)
+        self.assertConfirmationLinkRedirect(confirmation_link)
         self.assertRegistrationVerificationSuccessResponse(self.client.verify(confirmation_link))
         self.assertTrue(User.objects.get(email=email).is_active)
         self.assertPassword(email, password)
@@ -435,6 +443,7 @@ class UserManagementTestCase(DesecTestCase, PublicSuffixMockMixin):
         new_password = new_password or self.random_password()
         self.assertResetPasswordSuccessResponse(self.reset_password(email))
         confirmation_link = self.assertResetPasswordEmail(email)
+        self.assertConfirmationLinkRedirect(confirmation_link)
         self.assertResetPasswordVerificationSuccessResponse(
             self.client.verify(confirmation_link, data={'new_password': new_password}, **kwargs))
         self.assertPassword(email, new_password)
@@ -446,6 +455,7 @@ class UserManagementTestCase(DesecTestCase, PublicSuffixMockMixin):
         self.assertChangeEmailSuccessResponse(self.change_email(new_email))
         new_email = new_email.strip()
         confirmation_link = self.assertChangeEmailVerificationEmail(new_email)
+        self.assertConfirmationLinkRedirect(confirmation_link)
         self.assertChangeEmailVerificationSuccessResponse(self.client.verify(confirmation_link), new_email)
         self.assertChangeEmailNotificationEmail(old_email)
         self.assertUserExists(new_email)
@@ -456,6 +466,7 @@ class UserManagementTestCase(DesecTestCase, PublicSuffixMockMixin):
     def _test_delete_account(self, email, password):
         self.assertDeleteAccountSuccessResponse(self.delete_account(email, password))
         confirmation_link = self.assertDeleteAccountEmail(email)
+        self.assertConfirmationLinkRedirect(confirmation_link)
         self.assertDeleteAccountVerificationSuccessResponse(self.client.verify(confirmation_link))
         self.assertUserDoesNotExist(email)
 
@@ -664,14 +675,12 @@ class HasUserAccountTestCase(UserManagementTestCase):
         self.assertUserDoesNotExist(new_email)
         self.assertPassword(self.email, new_password)
 
-    def test_reset_password_via_get(self):
+    def test_reset_password_without_new_password(self):
         confirmation_link = self._start_reset_password()
         response = self.client.verify(confirmation_link)
-        self.assertResponse(response, status.HTTP_406_NOT_ACCEPTABLE)
-
-        confirmation_link = self._start_reset_password()
-        response = self.client.verify(confirmation_link, HTTP_ACCEPT='text/html')
-        self.assertResponse(response, status.HTTP_302_FOUND)
+        self.assertResponse(response, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(response.data['new_password'][0], 'This field is required.')
+        self.assertNoEmailSent()
 
     def test_reset_password_validation_unknown_user(self):
         confirmation_link = self._start_reset_password()
@@ -717,13 +726,6 @@ class HasUserAccountTestCase(UserManagementTestCase):
         self.assertUserExists(new_email)
         self.assertPassword(new_email, new_password)
 
-    def test_change_email_verification_change_password(self):
-        new_email = self.random_username()
-        self.assertChangeEmailSuccessResponse(self.change_email(new_email))
-        confirmation_link = self.assertChangeEmailVerificationEmail(new_email)
-        response = self.client.verify(confirmation_link, data={'new_password': self.random_password()})
-        self.assertStatus(response, status.HTTP_405_METHOD_NOT_ALLOWED)
-
     def test_change_email_same_email(self):
         self.assertChangeEmailFailureSameAddressResponse(
             response=self.change_email(self.email)

api/desecapi/views.py

@@ -524,6 +524,7 @@ class AuthenticatedActionView(generics.GenericAPIView):
     authentication_classes = (auth.AuthenticatedActionAuthentication,)
     authentication_exception = ValidationError
     html_url = None
+    http_method_names = ['get', 'post']  # GET is for redirect only
     renderer_classes = [JSONRenderer, StaticHTMLRenderer]
 
     def perform_authentication(self, request):
@@ -532,35 +533,25 @@ class AuthenticatedActionView(generics.GenericAPIView):
         pass
 
     def get(self, request, *args, **kwargs):
-        is_redirect = (request.accepted_renderer.format == 'html') and self.html_url
-
-        # For POST-type actions, only allow GET for the purpose of returning a frontend redirect to a browser
-        if 'post' in self.http_method_names:
-            if not is_redirect:
-                raise NotAcceptable
-
         # Redirect browsers to frontend if available
+        is_redirect = (request.accepted_renderer.format == 'html') and self.html_url
         if is_redirect:
             # Careful: This can generally lead to an open redirect if values contain slashes!
             # However, it cannot happen for Django view kwargs.
             return redirect(self.html_url.format(**kwargs))
-
-        return self.take_action()
+        else:
+            raise NotAcceptable
 
     def post(self, request, *args, **kwargs):
-        return self.take_action()
+        self.request.auth.act()  # execute the action (triggers authentication if not yet done)
+        return self.finalize()
 
     def finalize(self):
         raise NotImplementedError
 
-    def take_action(self):
-        self.request.auth.act()  # execute the action (triggers authentication if not yet done)
-        return self.finalize()
-
 
 class AuthenticatedActivateUserActionView(AuthenticatedActionView):
     html_url = '/confirm/activate-account/{code}/'
-    http_method_names = ['get']
     serializer_class = serializers.AuthenticatedActivateUserActionSerializer
 
     def finalize(self):
@@ -619,7 +610,6 @@ class AuthenticatedActivateUserActionView(AuthenticatedActionView):
 
 class AuthenticatedChangeEmailUserActionView(AuthenticatedActionView):
     html_url = '/confirm/change-email/{code}/'
-    http_method_names = ['get']
     serializer_class = serializers.AuthenticatedChangeEmailUserActionSerializer
 
     def finalize(self):
@@ -630,7 +620,6 @@ class AuthenticatedChangeEmailUserActionView(AuthenticatedActionView):
 
 class AuthenticatedResetPasswordUserActionView(AuthenticatedActionView):
     html_url = '/confirm/reset-password/{code}/'
-    http_method_names = ['get', 'post']  # GET is for redirect only
     serializer_class = serializers.AuthenticatedResetPasswordUserActionSerializer
 
     def finalize(self):
@@ -640,13 +629,12 @@ class AuthenticatedResetPasswordUserActionView(AuthenticatedActionView):
 
 class AuthenticatedDeleteUserActionView(AuthenticatedActionView):
     html_url = '/confirm/delete-account/{code}/'
-    http_method_names = ['get']
     serializer_class = serializers.AuthenticatedDeleteUserActionSerializer
 
-    def take_action(self):
+    def post(self, request, *args, **kwargs):
         if self.request.user.domains.exists():
             return AccountDeleteView.response_still_has_domains
-        return super().take_action()
+        return super().post(request, *args, **kwargs)
 
     def finalize(self):
         return Response({'detail': 'All your data has been deleted. Bye bye, see you soon! <3'})

docs/authentication.rst

@@ -73,8 +73,8 @@ Accepted``. In case there already is an account for that email address,
 nothing else will be done. Otherwise, you will receive an email with a
 verification link of the form
 ``https://desec.io/api/v1/v/activate-account/<code>/``. To activate your
-account, send a ``GET`` request using this link (i.e., you can simply click
-it). The link expires after 12 hours.
+account, click on that link (which will direct you to our frontend) or send a
+``POST`` request on the command line. The link expires after 12 hours.
 
 If there is a problem with your email address, your password, or the proposed
 captcha solution, the server will reply with ``400 Bad Request`` and give a
@@ -207,7 +207,8 @@ The server will reply with ``202 Accepted``. If there is no account associated
 with this email address, nothing else will be done. Otherwise, you will receive
 an email with a URL of the form
 ``https://desec.io/api/v1/v/reset-password/<code>/``. To perform the actual
-password reset, send a ``POST`` request to this URL, with the new password in
+password reset, click on that link (which will direct you to our frontend) or
+send a ``POST`` request to this URL, with the new password in
 the payload::
 
     curl -X POST https://desec.io/api/v1/v/reset-password/<code>/ \
@@ -247,8 +248,8 @@ Accepted``. In case there already is an account for the email address given in
 the ``new_email`` field, nothing else will be done. Otherwise, we will send
 an email to the new email address for verification purposes. It will contain a
 link of the form ``https://desec.io/api/v1/v/change-email/<code>/``. To perform
-the actual change, send a ``GET`` request using this link (i.e., you can simply
-click the link).
+the actual change, click on that link (which will direct you to our frontend)
+or send a ``POST`` request on the command line.
 
 The link expires after 12 hours. It is also invalidated by certain other
 account-related activities, such as changing your password.
@@ -273,7 +274,8 @@ address and password to the ``/auth/account/delete/`` endpoint::
 If the correct password has been provided, the server will reply with ``202
 Accepted`` and send you an email with a link of the form
 ``https://desec.io/api/v1/v/delete-account/<code>/``. To finish the deletion,
-send a ``GET`` request using this link (i.e., you can simply click the link).
+click on that link (which will direct you to our frontend) or send a ``POST``
+request on the command line.
 
 The link expires after 12 hours. It is also invalidated by certain other
 account-related activities, such as changing your email address or password.

docs/endpoint-reference.rst

@@ -31,13 +31,13 @@ for `User Registration and Management`_.
 +------------------------------------------------+------------+---------------------------------------------+
 | ...\ ``/captcha/``                             | ``POST``   | Obtain captcha                              |
 +------------------------------------------------+------------+---------------------------------------------+
-| ...\ ``/v/activate-account/:code/``            | ``GET``    | Confirm email address for new account       |
+| ...\ ``/v/activate-account/:code/``            | ``POST``   | Confirm email address for new account       |
 +------------------------------------------------+------------+---------------------------------------------+
 | ...\ ``/v/reset-password/:code/``              | ``POST``   | Confirm password reset                      |
 +------------------------------------------------+------------+---------------------------------------------+
-| ...\ ``/v/change-email/:code/``                | ``GET``    | Confirm email address change                |
+| ...\ ``/v/change-email/:code/``                | ``POST``   | Confirm email address change                |
 +------------------------------------------------+------------+---------------------------------------------+
-| ...\ ``/v/delete-account/:code/``              | ``GET``    | Confirm account deletion                    |
+| ...\ ``/v/delete-account/:code/``              | ``POST``   | Confirm account deletion                    |
 +------------------------------------------------+------------+---------------------------------------------+
 
 The following table summarizes basic information about the deSEC API endpoints used