Cookies: set samesite: Lax by default

The django's default is `Lax`, so I'm removing the setting.
I'm overriding only for .org in our ops repos.

We also are only allowing cross site requests
from public versions only.
Even if the domain is known,
if the version is private we don't set the CORS header.

Author: stsewd

readthedocs/core/middleware.py

@@ -2,22 +2,16 @@
 import time
 
 from django.conf import settings
-from django.contrib.sessions.backends.base import SessionBase
-from django.contrib.sessions.backends.base import UpdateError
+from django.contrib.sessions.backends.base import SessionBase, UpdateError
 from django.contrib.sessions.middleware import SessionMiddleware
-from django.core.exceptions import SuspiciousOperation
-from django.core.exceptions import ImproperlyConfigured, MiddlewareNotUsed
-from django.core.exceptions import MultipleObjectsReturned, ObjectDoesNotExist
-from django.http import Http404, HttpResponseBadRequest
-from django.urls.base import set_urlconf
+from django.core.exceptions import (
+    ImproperlyConfigured,
+    MiddlewareNotUsed,
+    SuspiciousOperation,
+)
 from django.utils.cache import patch_vary_headers
-from django.utils.deprecation import MiddlewareMixin
 from django.utils.http import http_date
 from django.utils.translation import ugettext_lazy as _
-from django.shortcuts import render
-
-from readthedocs.projects.models import Domain, Project
-
 
 log = logging.getLogger(__name__)
 

readthedocs/core/signals.py

@@ -1,18 +1,17 @@
 """Signal handling for core app."""
 
 import logging
-from urllib.parse import urlparse
 
 from corsheaders import signals
 from django.conf import settings
-from django.db.models import Count, Q
+from django.db.models import Count
 from django.db.models.signals import pre_delete
 from django.dispatch import Signal, receiver
 from rest_framework.permissions import SAFE_METHODS
 
 from readthedocs.builds.models import Version
 from readthedocs.core.unresolver import unresolve
-from readthedocs.projects.models import Domain, Project
+from readthedocs.projects.models import Project
 
 log = logging.getLogger(__name__)
 
@@ -49,8 +48,7 @@ def decide_if_cors(sender, request, **kwargs):  # pylint: disable=unused-argumen
     * It's a safe HTTP method
     * The origin is in ALLOWED_URLS
     * The URL is owned by the project that they are requesting data from
-    * The version is public or the domain is linked to the project
-      (except for the embed API).
+    * The version is public
 
     .. note::
 
@@ -62,8 +60,6 @@ def decide_if_cors(sender, request, **kwargs):  # pylint: disable=unused-argumen
     if 'HTTP_ORIGIN' not in request.META or request.method not in SAFE_METHODS:
         return False
 
-    host = urlparse(request.META['HTTP_ORIGIN']).netloc.split(':')[0]
-
     # Always allow the sustainability API,
     # it's used only on .org to check for ad-free users.
     if _has_donate_app() and request.path_info.startswith('/api/v2/sustainability'):
@@ -103,20 +99,6 @@ def decide_if_cors(sender, request, **kwargs):  # pylint: disable=unused-argumen
             if is_public:
                 return True
 
-            # Don't check for known domains for the embed api.
-            # It gives a lot of information,
-            # we should use a list of trusted domains from the user.
-            if valid_url == '/api/v2/embed':
-                return False
-
-            # Or allow if they have a registered domain
-            # linked to that project.
-            domain = Domain.objects.filter(
-                Q(domain__iexact=host),
-                Q(project=project) | Q(project__subprojects__child=project),
-            )
-            if domain.exists():
-                return True
     return False
 
 

readthedocs/rtd_tests/tests/test_middleware.py

@@ -72,7 +72,7 @@ def test_allow_linked_domain_from_public_version(self):
         resp = self.middleware.process_response(request, {})
         self.assertIn('Access-Control-Allow-Origin', resp)
 
-    def test_allow_linked_domain_from_private_version(self):
+    def test_dont_allow_linked_domain_from_private_version(self):
         self.version.privacy_level = PRIVATE
         self.version.save()
         request = self.factory.get(
@@ -81,7 +81,7 @@ def test_allow_linked_domain_from_private_version(self):
             HTTP_ORIGIN='http://my.valid.domain',
         )
         resp = self.middleware.process_response(request, {})
-        self.assertIn('Access-Control-Allow-Origin', resp)
+        self.assertNotIn('Access-Control-Allow-Origin', resp)
 
     def test_allowed_api_public_version_from_another_domain(self):
         request = self.factory.get(
@@ -228,6 +228,7 @@ def setUp(self):
 
         self.user = create_user(username='owner', password='test')
 
+    @override_settings(SESSION_COOKIE_SAMESITE=None)
     def test_fallback_cookie(self):
         request = self.factory.get('/')
         response = HttpResponse()
@@ -238,6 +239,7 @@ def test_fallback_cookie(self):
         self.assertTrue(settings.SESSION_COOKIE_NAME in response.cookies)
         self.assertTrue(self.middleware.cookie_name_fallback in response.cookies)
 
+    @override_settings(SESSION_COOKIE_SAMESITE=None)
     def test_main_cookie_samesite_none(self):
         request = self.factory.get('/')
         response = HttpResponse()
@@ -247,3 +249,13 @@ def test_main_cookie_samesite_none(self):
 
         self.assertEqual(response.cookies[settings.SESSION_COOKIE_NAME]['samesite'], 'None')
         self.assertEqual(response.cookies[self.middleware.cookie_name_fallback]['samesite'], '')
+
+    def test_main_cookie_samesite_lax(self):
+        request = self.factory.get('/')
+        response = HttpResponse()
+        self.middleware.process_request(request)
+        request.session['test'] = 'value'
+        response = self.middleware.process_response(request, response)
+
+        self.assertEqual(response.cookies[settings.SESSION_COOKIE_NAME]['samesite'], 'Lax')
+        self.assertTrue(self.test_main_cookie_samesite_none not in response.cookies)

readthedocs/settings/base.py

@@ -73,8 +73,6 @@ class CommunityBaseSettings(Settings):
     SESSION_COOKIE_HTTPONLY = True
     SESSION_COOKIE_AGE = 30 * 24 * 60 * 60  # 30 days
     SESSION_SAVE_EVERY_REQUEST = True
-    # This cookie is used in cross-origin API requests from *.readthedocs.io to readthedocs.org
-    SESSION_COOKIE_SAMESITE = None
 
     # CSRF
     CSRF_COOKIE_HTTPONLY = True