From 1d1161be578fed98707d5d7ced96b89dfcff74ee Mon Sep 17 00:00:00 2001
From: Santos Gallegos <stsewd@proton.me>
Date: Thu, 4 Jan 2024 08:53:50 -0500
Subject: [PATCH] Integrations: hardcode deprecation date for incoming webhooks
 without a secret (#10993)

Ref https://blog.readthedocs.com/security-update-on-incoming-webhooks/
---
 readthedocs/api/v2/views/integrations.py | 32 ++++++++++++++++++++++--
 readthedocs/rtd_tests/tests/test_api.py  | 27 ++++++++++++++++++++
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/readthedocs/api/v2/views/integrations.py b/readthedocs/api/v2/views/integrations.py
index 011bec245d2..809e0f10cef 100644
--- a/readthedocs/api/v2/views/integrations.py
+++ b/readthedocs/api/v2/views/integrations.py
@@ -1,5 +1,6 @@
 """Endpoints integrating with Github, Bitbucket, and other webhooks."""
 
+import datetime
 import hashlib
 import hmac
 import json
@@ -9,6 +10,7 @@
 
 import structlog
 from django.shortcuts import get_object_or_404
+from django.utils import timezone
 from django.utils.crypto import constant_time_compare
 from rest_framework import permissions, status
 from rest_framework.exceptions import NotFound, ParseError
@@ -74,9 +76,17 @@ class WebhookMixin:
     invalid_payload_msg = 'Payload not valid'
     missing_secret_for_pr_events_msg = dedent(
         """
-        The webhook doesn't have a secret configured.
+        This webhook doesn't have a secret configured.
         For security reasons, webhooks without a secret can't process pull/merge request events.
-        You can read more information about this in our blog post: https://blog.readthedocs.com/security-update-on-incoming-webhooks/.
+        For more information, read our blog post: https://blog.readthedocs.com/security-update-on-incoming-webhooks/.
+        """
+    ).strip()
+
+    missing_secret_deprecated_msg = dedent(
+        """
+        This webhook doesn't have a secret configured.
+        For security reasons, webhooks without a secret are no longer permitted.
+        For more information, read our blog post: https://blog.readthedocs.com/security-update-on-incoming-webhooks/.
         """
     ).strip()
 
@@ -105,6 +115,18 @@ def post(self, request, project_slug):
                 return Response(resp, status=status.HTTP_406_NOT_ACCEPTABLE)
         except Project.DoesNotExist as exc:
             raise NotFound("Project not found") from exc
+
+        # Deprecate webhooks without a secret
+        # https://blog.readthedocs.com/security-update-on-incoming-webhooks/.
+        now = timezone.now()
+        deprecation_date = datetime.datetime(2024, 1, 31, tzinfo=datetime.timezone.utc)
+        is_deprecated = now >= deprecation_date
+        if is_deprecated and not self.has_secret():
+            return Response(
+                {"detail": self.missing_secret_deprecated_msg},
+                status=HTTP_400_BAD_REQUEST,
+            )
+
         if not self.is_payload_valid():
             log.warning('Invalid payload for project and integration.')
             return Response(
@@ -122,6 +144,12 @@ def post(self, request, project_slug):
             return resp
         return Response(resp)
 
+    def has_secret(self):
+        integration = self.get_integration()
+        if hasattr(integration, "token"):
+            return bool(integration.token)
+        return bool(integration.secret)
+
     def get_project(self, **kwargs):
         return Project.objects.get(**kwargs)
 
diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py
index c6e0812070f..824c1c2a2ba 100644
--- a/readthedocs/rtd_tests/tests/test_api.py
+++ b/readthedocs/rtd_tests/tests/test_api.py
@@ -3239,6 +3239,33 @@ def test_webhook_build_another_branch(self, trigger_build):
         self.assertTrue(resp.data['build_triggered'])
         self.assertEqual(resp.data['versions'], ['v1.0'])
 
+    @mock.patch("readthedocs.api.v2.views.integrations.timezone.now")
+    def test_deprecate_webhooks_without_a_secret(self, now, trigger_build):
+        now.return_value = datetime.datetime(2024, 1, 31, tzinfo=datetime.timezone.utc)
+        client = APIClient()
+
+        Integration.objects.filter(pk=self.github_integration.pk).update(secret=None)
+        resp = client.post(
+            f"/api/v2/webhook/github/{self.project.slug}/",
+            self.github_payload,
+            format="json",
+            headers={GITHUB_SIGNATURE_HEADER: "skip"},
+        )
+        self.assertContains(
+            resp, "This webhook doesn't have a secret configured.", status_code=400
+        )
+
+        self.generic_integration.provider_data = {"token": None}
+        self.generic_integration.save()
+        resp = client.post(
+            f"/api/v2/webhook/{self.project.slug}/{self.generic_integration.pk}/",
+            {"token": "skip"},
+            format="json",
+        )
+        # For generic webhooks, we first check if the secret matches,
+        # and return a 400 if it doesn't.
+        self.assertEqual(resp.status_code, 404)
+
 
 @override_settings(PUBLIC_DOMAIN="readthedocs.io")
 class APIVersionTests(TestCase):