Improve CORS configuration

The goal of this commit is to allow access to our resources from
different hosts:

* everything coming from `*.readthedocs.io`
* everything hitting any URL under `/api/` if it's safe method, no
matter the host where the request comes from

Compared with the previous implementation, this reduces the number of
queries to our database since we are not checking that the host where
the request was made is a known Domain for us.

Besides, this opens access to `/api/v3` as well.

Author: humitos

readthedocs/core/signals.py

@@ -18,13 +18,6 @@
 
 log = logging.getLogger(__name__)
 
-WHITELIST_URLS = [
-    '/api/v2/footer_html',
-    '/api/v2/search',
-    '/api/v2/docsearch',
-    '/api/v2/sustainability',
-]
-
 webhook_github = Signal(providing_args=['project', 'data', 'event'])
 webhook_gitlab = Signal(providing_args=['project', 'data', 'event'])
 webhook_bitbucket = Signal(providing_args=['project', 'data', 'event'])
@@ -34,52 +27,14 @@ def decide_if_cors(sender, request, **kwargs):  # pylint: disable=unused-argumen
     """
     Decide whether a request should be given CORS access.
 
-    This checks that:
-    * The URL is whitelisted against our CORS-allowed domains
-    * The Domain exists in our database, and belongs to the project being queried.
+    This checks that the URL is under ``/api/`` and it's a safe method.
 
-    Returns True when a request should be given CORS access.
+    Returns ``True`` when a request should be given CORS access.
     """
-    if 'HTTP_ORIGIN' not in request.META:
-        return False
-    host = urlparse(request.META['HTTP_ORIGIN']).netloc.split(':')[0]
-
-    # Don't do domain checking for this API for now
-    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):
-            valid_url = True
-            break
-
-    if valid_url:
-        project_slug = request.GET.get('project', None)
-        try:
-            project = Project.objects.get(slug=project_slug)
-        except Project.DoesNotExist:
-            log.warning(
-                'Invalid project passed to domain. [%s:%s]',
-                project_slug,
-                host,
-            )
-            return False
-
-        domain = Domain.objects.filter(
-            Q(domain__icontains=host),
-            Q(project=project) | Q(project__subprojects__child=project),
-        )
-        if domain.exists():
-            return True
-
-    return False
+    return all([
+        request.method in SAFE_METHODS,
+        request.path_info.startswith('/api/'),
+    ])
 
 
 @receiver(pre_delete, sender=settings.AUTH_USER_MODEL)

readthedocs/rtd_tests/tests/test_middleware.py

@@ -185,7 +185,7 @@ def setUp(self):
         )
         self.domain = get(Domain, domain='my.valid.domain', project=self.project)
 
-    def test_proper_domain(self):
+    def test_known_domain(self):
         request = self.factory.get(
             self.url,
             {'project': self.project.slug},
@@ -194,11 +194,24 @@ def test_proper_domain(self):
         resp = self.middleware.process_response(request, {})
         self.assertIn('Access-Control-Allow-Origin', resp)
 
-    def test_invalid_domain(self):
+    def test_unknown_domain(self):
+        """
+        Test accessing the API from a domain that it's not registered in our DB.
+
+        Only the endpoints under ``/api/`` should work.
+        """
         request = self.factory.get(
             self.url,
             {'project': self.project.slug},
-            HTTP_ORIGIN='http://invalid.domain',
+            HTTP_ORIGIN='http://unknown.domain',
+        )
+        resp = self.middleware.process_response(request, {})
+        self.assertIn('Access-Control-Allow-Origin', resp)
+
+        request = self.factory.get(
+            '/dashboard/',
+            {'project': self.project.slug},
+            HTTP_ORIGIN='http://unknown.domain',
         )
         resp = self.middleware.process_response(request, {})
         self.assertNotIn('Access-Control-Allow-Origin', resp)
@@ -227,15 +240,15 @@ def test_apiv2_endpoint_allowed(self):
         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)
+        self.assertIn('Access-Control-Allow-Origin', resp)
 
+    def test_apiv2_method_not_allowed(self):
         # POST is not allowed
         request = self.factory.post(
             '/api/v2/version/',

readthedocs/settings/base.py

@@ -403,16 +403,19 @@ def USE_PROMOS(self):  # noqa
         r'^http://(.+)\.readthedocs\.io$',
         r'^https://(.+)\.readthedocs\.io$',
     )
-    # So people can post to their accounts
+
+    # We need to allow credentials (cookies) to identify the user and remove ads
+    # if it's Gold member. Note: in Django 2.1 the ``SESSION_COOKIE_SAMESITE``
+    # setting was added, set to 'Lax' by default, which will prevent Django's
+    # session cookie being sent cross-domain. Change it to None to bypass this
+    # security restriction.
     CORS_ALLOW_CREDENTIALS = True
-    CORS_ALLOW_HEADERS = (
-        'x-requested-with',
-        'content-type',
-        'accept',
-        'origin',
-        'authorization',
-        'x-csrftoken'
-    )
+
+    CORS_ALLOW_METHODS = [
+        'GET',
+        'OPTIONS',
+        'HEAD',
+    ]
 
     # RTD Settings
     REPO_LOCK_SECONDS = 30