Merge pull request #4880 from rtfd/humitos/cors/allow-api-known-domains

Allow all /api/v2/ CORS if the Domain is known
readthedocs · Dec 11, 2018 · 5907064 · 5907064
2 parents 04cc5ac + 4e5f747
commit 5907064
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 12 deletions.
diff --git a/readthedocs/core/signals.py b/readthedocs/core/signals.py
@@ -13,6 +13,7 @@
 from django.db.models import Q, Count
 from django.dispatch import receiver
 from future.backports.urllib.parse import urlparse
+from rest_framework.permissions import SAFE_METHODS
 
 from readthedocs.oauth.models import RemoteOrganization
 from readthedocs.projects.models import Project, Domain
@@ -49,6 +50,12 @@ def decide_if_cors(sender, request, **kwargs):  # pylint: disable=unused-argumen
     if request.path_info.startswith('/api/v2/sustainability'):
         return True
 
+    # Don't do domain checking for APIv2 when the Domain is known
+    if request.path_info.startswith('/api/v2/') and request.method in SAFE_METHODS:
+        domain = Domain.objects.filter(domain__icontains=host)
+        if domain.exists():
+            return True
+
     valid_url = False
     for url in WHITELIST_URLS:
         if request.path_info.startswith(url):
@@ -69,7 +76,7 @@ def decide_if_cors(sender, request, **kwargs):  # pylint: disable=unused-argumen
 
         domain = Domain.objects.filter(
             Q(domain__icontains=host),
-            Q(project=project) | Q(project__subprojects__child=project)
+            Q(project=project) | Q(project__subprojects__child=project),
         )
         if domain.exists():
             return True

diff --git a/readthedocs/rtd_tests/tests/test_middleware.py b/readthedocs/rtd_tests/tests/test_middleware.py
@@ -185,21 +185,12 @@ def test_invalid_domain(self):
         resp = self.middleware.process_response(request, {})
         self.assertNotIn('Access-Control-Allow-Origin', resp)
 
-    def test_invalid_project(self):
-        request = self.factory.get(
-            self.url,
-            {'project': 'foo'},
-            HTTP_ORIGIN='http://my.valid.domain',
-        )
-        resp = self.middleware.process_response(request, {})
-        self.assertNotIn('Access-Control-Allow-Origin', resp)
-
     def test_valid_subproject(self):
         self.assertTrue(
             Project.objects.filter(
                 pk=self.project.pk,
-                subprojects__child=self.subproject
-            ).exists()
+                subprojects__child=self.subproject,
+            ).exists(),
         )
         request = self.factory.get(
             self.url,
@@ -208,3 +199,30 @@ def test_valid_subproject(self):
         )
         resp = self.middleware.process_response(request, {})
         self.assertIn('Access-Control-Allow-Origin', resp)
+
+    def test_apiv2_endpoint_allowed(self):
+        request = self.factory.get(
+            '/api/v2/version/',
+            {'project__slug': self.project.slug, 'active': True},
+            HTTP_ORIGIN='http://my.valid.domain',
+        )
+        resp = self.middleware.process_response(request, {})
+        self.assertIn('Access-Control-Allow-Origin', resp)
+
+    def test_apiv2_endpoint_not_allowed(self):
+        request = self.factory.get(
+            '/api/v2/version/',
+            {'project__slug': self.project.slug, 'active': True},
+            HTTP_ORIGIN='http://invalid.domain',
+        )
+        resp = self.middleware.process_response(request, {})
+        self.assertNotIn('Access-Control-Allow-Origin', resp)
+
+        # POST is not allowed
+        request = self.factory.post(
+            '/api/v2/version/',
+            {'project__slug': self.project.slug, 'active': True},
+            HTTP_ORIGIN='http://my.valid.domain',
+        )
+        resp = self.middleware.process_response(request, {})
+        self.assertNotIn('Access-Control-Allow-Origin', resp)