diff --git a/readthedocs/rtd_tests/tests/test_middleware.py b/readthedocs/rtd_tests/tests/test_middleware.py index 208cd7b1113..bffb40d6e61 100644 --- a/readthedocs/rtd_tests/tests/test_middleware.py +++ b/readthedocs/rtd_tests/tests/test_middleware.py @@ -1,7 +1,6 @@ from corsheaders.middleware import ( ACCESS_CONTROL_ALLOW_CREDENTIALS, ACCESS_CONTROL_ALLOW_ORIGIN, - CorsMiddleware, ) from django.conf import settings from django.http import HttpResponse @@ -25,8 +24,6 @@ class TestCORSMiddleware(TestCase): def setUp(self): - self.factory = RequestFactory() - self.middleware = CorsMiddleware() self.url = '/api/v2/search' self.owner = create_user(username='owner', password='test') self.project = get( @@ -69,66 +66,60 @@ def setUp(self): ) def test_allow_linked_domain_from_public_version(self): - request = self.factory.get( + resp = self.client.get( self.url, {'project': self.project.slug, 'version': self.version.slug}, HTTP_ORIGIN='http://my.valid.domain', ) - resp = self.middleware.process_response(request, {}) - self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp) - self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp) + self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp.headers) + self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp.headers) def test_linked_domain_from_private_version(self): self.version.privacy_level = PRIVATE self.version.save() - request = self.factory.get( + resp = self.client.get( self.url, {'project': self.project.slug, 'version': self.version.slug}, HTTP_ORIGIN='http://my.valid.domain', ) - resp = self.middleware.process_response(request, {}) - self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp) - self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp) + self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp.headers) + self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp.headers) def test_allowed_api_public_version_from_another_domain(self): - request = self.factory.get( + resp = self.client.get( self.url, {'project': self.project.slug, 'version': self.version.slug}, HTTP_ORIGIN='http://docs.another.domain', ) - resp = self.middleware.process_response(request, {}) - self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp) - self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp) + self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp.headers) + self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp.headers) - request = self.factory.get( + resp = self.client.get( self.url, {'project': self.project.slug, 'version': self.version.slug}, HTTP_ORIGIN='http://another.valid.domain', ) - resp = self.middleware.process_response(request, {}) - self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp) - self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp) + self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp.headers) + self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp.headers) def test_api_private_version_from_another_domain(self): self.version.privacy_level = PRIVATE self.version.save() - request = self.factory.get( + resp = self.client.get( self.url, {'project': self.project.slug, 'version': self.version.slug}, HTTP_ORIGIN='http://docs.another.domain', ) - resp = self.middleware.process_response(request, {}) - self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp) - self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp) + self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp.headers) + self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp.headers) - request = self.factory.get( + resp = self.client.get( self.url, {'project': self.project.slug, 'version': self.version.slug}, HTTP_ORIGIN='http://another.valid.domain', ) - resp = self.middleware.process_response(request, {}) - self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp) - self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp) + self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp.headers) + self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp.headers) def test_valid_subproject(self): self.assertTrue( @@ -137,102 +128,92 @@ def test_valid_subproject(self): subprojects__child=self.subproject, ).exists(), ) - request = self.factory.get( + resp = self.client.get( self.url, {'project': self.project.slug, 'version': self.version.slug}, HTTP_ORIGIN='http://my.valid.domain', ) - resp = self.middleware.process_response(request, {}) - self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp) - self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp) + self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp.headers) + self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp.headers) def test_embed_api_private_version_linked_domain(self): self.version.privacy_level = PRIVATE self.version.save() - request = self.factory.get( + resp = self.client.get( '/api/v2/embed/', {'project': self.project.slug, 'version': self.version.slug}, HTTP_ORIGIN='http://my.valid.domain', ) - resp = self.middleware.process_response(request, {}) - self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp) - self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp) + self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp.headers) + self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp.headers) def test_embed_api_external_url(self): - request = self.factory.get( + resp = self.client.get( "/api/v2/embed/", {"url": "https://pip.readthedocs.io/en/latest/index.hml"}, HTTP_ORIGIN="http://my.valid.domain", ) - resp = self.middleware.process_response(request, {}) - self.assertIn("Access-Control-Allow-Origin", resp) + self.assertIn("Access-Control-Allow-Origin", resp.headers) - request = self.factory.get( + resp = self.client.get( "/api/v2/embed/", {"url": "https://docs.example.com/en/latest/index.hml"}, HTTP_ORIGIN="http://my.valid.domain", ) - resp = self.middleware.process_response(request, {}) - self.assertIn("Access-Control-Allow-Origin", resp) + self.assertIn("Access-Control-Allow-Origin", resp.headers) def test_sustainability_endpoint_allways_allowed(self): - request = self.factory.get( + resp = self.client.get( '/api/v2/sustainability/', {'project': self.project.slug, 'active': True, 'version': self.version.slug}, HTTP_ORIGIN='http://invalid.domain', ) - resp = self.middleware.process_response(request, {}) - self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp) - self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp) + self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp.headers) + self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp.headers) - request = self.factory.get( + resp = self.client.get( '/api/v2/sustainability/', {'project': self.project.slug, 'active': True, 'version': self.version.slug}, HTTP_ORIGIN='http://my.valid.domain', ) - resp = self.middleware.process_response(request, {}) - self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp) - self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp) + self.assertIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp.headers) + self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp.headers) def test_apiv2_endpoint_not_allowed(self): - request = self.factory.get( + resp = self.client.get( '/api/v2/version/', {'project': self.project.slug, 'active': True, 'version': self.version.slug}, HTTP_ORIGIN='http://invalid.domain', ) - resp = self.middleware.process_response(request, {}) - self.assertNotIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp) - self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp) + self.assertNotIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp.headers) + self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp.headers) # This also doesn't work on registered domains. - request = self.factory.get( + resp = self.client.get( '/api/v2/version/', {'project': self.project.slug, 'active': True, 'version': self.version.slug}, HTTP_ORIGIN='http://my.valid.domain', ) - resp = self.middleware.process_response(request, {}) - self.assertNotIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp) - self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp) + self.assertNotIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp.headers) + self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp.headers) # Or from our public domain. - request = self.factory.get( + resp = self.client.get( '/api/v2/version/', {'project': self.project.slug, 'active': True, 'version': self.version.slug}, HTTP_ORIGIN='http://docs.readthedocs.io/', ) - resp = self.middleware.process_response(request, {}) - self.assertNotIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp) - self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp) + self.assertNotIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp.headers) + self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp.headers) # POST is not allowed - request = self.factory.post( + resp = self.client.post( '/api/v2/version/', {'project': self.project.slug, 'active': True, 'version': self.version.slug}, HTTP_ORIGIN='http://my.valid.domain', ) - resp = self.middleware.process_response(request, {}) - self.assertNotIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp) - self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp) + self.assertNotIn(ACCESS_CONTROL_ALLOW_ORIGIN, resp.headers) + self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp.headers) class TestSessionMiddleware(TestCase): diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index 5e1a3df61d6..c24d64c548b 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -1,6 +1,7 @@ # pylint: disable=missing-docstring import os +import re import subprocess import socket @@ -9,6 +10,7 @@ from celery.schedules import crontab from readthedocs.core.logs import shared_processors +from corsheaders.defaults import default_headers from readthedocs.core.settings import Settings @@ -277,6 +279,7 @@ def MIDDLEWARE(self): 'readthedocs.core.middleware.NullCharactersMiddleware', 'readthedocs.core.middleware.ReadTheDocsSessionMiddleware', 'django.middleware.locale.LocaleMiddleware', + 'corsheaders.middleware.CorsMiddleware', 'django.middleware.common.CommonMiddleware', 'django.middleware.security.SecurityMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', @@ -284,7 +287,6 @@ def MIDDLEWARE(self): 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'dj_pagination.middleware.PaginationMiddleware', - 'corsheaders.middleware.CorsMiddleware', 'csp.middleware.CSPMiddleware', 'readthedocs.core.middleware.ReferrerPolicyMiddleware', 'simple_history.middleware.HistoryRequestMiddleware', @@ -734,15 +736,17 @@ def DOCKER_LIMITS(self): # users to CSRF attacks. The sustainability API is the only view that requires # cookies to be send cross-site, we override that for that view only. CORS_ALLOW_CREDENTIALS = False - CORS_ALLOW_HEADERS = ( - 'x-requested-with', - 'content-type', - 'accept', - 'origin', - 'authorization', + + # Allow cross-site requests from any origin, + # all information from our allowed endpoits is public. + # + # NOTE: We don't use `CORS_ALLOW_ALL_ORIGINS=True`, + # since that will set the `Access-Control-Allow-Origin` header to `*`, + # we won't be able to pass credentials fo the sustainability API with that value. + CORS_ALLOWED_ORIGIN_REGEXES = [re.compile(".+")] + CORS_ALLOW_HEADERS = list(default_headers) + [ 'x-hoverxref-version', - 'x-csrftoken' - ) + ] # Additional protection to allow only idempotent methods. CORS_ALLOW_METHODS = [ 'GET', @@ -751,14 +755,19 @@ def DOCKER_LIMITS(self): ] # URLs to allow CORS to read from unauthed. - CORS_URLS_ALLOW_ALL_REGEX = [ - r"^/api/v2/footer_html", - r"^/api/v2/search", - r"^/api/v2/docsearch", - r"^/api/v2/embed", - r"^/api/v3/embed", - r"^/api/v2/sustainability", - ] + CORS_URLS_REGEX = re.compile( + r""" + ^( + /api/v2/footer_html + |/api/v2/search + |/api/v2/docsearch + |/api/v2/embed + |/api/v3/embed + |/api/v2/sustainability + ) + """, + re.VERBOSE, + ) # RTD Settings ALLOW_PRIVATE_REPOS = False diff --git a/readthedocs/settings/proxito/base.py b/readthedocs/settings/proxito/base.py index aedf2578f4a..0f0a82677e0 100644 --- a/readthedocs/settings/proxito/base.py +++ b/readthedocs/settings/proxito/base.py @@ -20,9 +20,6 @@ class CommunityProxitoSettingsMixin: # As 'Lax' breaks when the page is embedded in an iframe. SESSION_COOKIE_SAMESITE = None - # We don't need or want to allow cross site requests in proxito. - CORS_URLS_ALLOW_ALL_REGEX = [] - @property def DATABASES(self): # This keeps connections to the DB alive, diff --git a/requirements/deploy.txt b/requirements/deploy.txt index 8eeca3b749e..e8d8896a71a 100644 --- a/requirements/deploy.txt +++ b/requirements/deploy.txt @@ -28,11 +28,11 @@ billiard==3.6.4.0 # via # -r requirements/pip.txt # celery -boto3==1.26.64 +boto3==1.26.65 # via # -r requirements/pip.txt # django-storages -botocore==1.29.64 +botocore==1.29.65 # via # -r requirements/pip.txt # boto3 @@ -104,6 +104,7 @@ django==3.2.17 # dj-stripe # django-allauth # django-annoying + # django-cors-headers # django-csp # django-debug-toolbar # django-extensions @@ -121,7 +122,7 @@ django-annoying==0.10.6 # via -r requirements/pip.txt django-autoslug==1.9.8 # via -r requirements/pip.txt -django-cors-middleware==1.4.0 +django-cors-headers==3.13.0 # via -r requirements/pip.txt django-crispy-forms==1.14.0 # via -r requirements/pip.txt @@ -338,7 +339,7 @@ s3transfer==0.6.0 # boto3 selectolax==0.3.12 # via -r requirements/pip.txt -sentry-sdk==1.14.0 +sentry-sdk==1.15.0 # via structlog-sentry six==1.16.0 # via @@ -438,7 +439,7 @@ vine==5.0.0 # amqp # celery # kombu -virtualenv==20.17.1 +virtualenv==20.18.0 # via -r requirements/pip.txt wcwidth==0.2.6 # via diff --git a/requirements/docker.txt b/requirements/docker.txt index ace374e3005..0670c9b4ca8 100644 --- a/requirements/docker.txt +++ b/requirements/docker.txt @@ -28,11 +28,11 @@ billiard==3.6.4.0 # via # -r requirements/pip.txt # celery -boto3==1.26.64 +boto3==1.26.65 # via # -r requirements/pip.txt # django-storages -botocore==1.29.64 +botocore==1.29.65 # via # -r requirements/pip.txt # boto3 @@ -113,6 +113,7 @@ django==3.2.17 # dj-stripe # django-allauth # django-annoying + # django-cors-headers # django-csp # django-debug-toolbar # django-extensions @@ -130,7 +131,7 @@ django-annoying==0.10.6 # via -r requirements/pip.txt django-autoslug==1.9.8 # via -r requirements/pip.txt -django-cors-middleware==1.4.0 +django-cors-headers==3.13.0 # via -r requirements/pip.txt django-crispy-forms==1.14.0 # via -r requirements/pip.txt @@ -439,7 +440,7 @@ tomli==2.0.1 # ipdb # pyproject-api # tox -tox==4.4.4 +tox==4.4.5 # via -r requirements/docker.in traitlets==5.9.0 # via @@ -470,7 +471,7 @@ vine==5.0.0 # amqp # celery # kombu -virtualenv==20.17.1 +virtualenv==20.18.0 # via # -r requirements/pip.txt # tox diff --git a/requirements/docs.txt b/requirements/docs.txt index bea8cf63dcc..e0dd004c933 100644 --- a/requirements/docs.txt +++ b/requirements/docs.txt @@ -57,7 +57,7 @@ pygments==2.14.0 # sphinx-tabs pytz==2022.7.1 # via babel -pyyaml==5.4.1 +pyyaml==6.0 # via myst-parser readthedocs-sphinx-search==0.1.2 # via -r requirements/docs.in diff --git a/requirements/lint.txt b/requirements/lint.txt index f1c1ae22c14..10a55af7f79 100644 --- a/requirements/lint.txt +++ b/requirements/lint.txt @@ -31,11 +31,11 @@ billiard==3.6.4.0 # via # -r requirements/pip.txt # celery -boto3==1.26.64 +boto3==1.26.65 # via # -r requirements/pip.txt # django-storages -botocore==1.29.64 +botocore==1.29.65 # via # -r requirements/pip.txt # boto3 @@ -104,6 +104,7 @@ django==3.2.17 # dj-stripe # django-allauth # django-annoying + # django-cors-headers # django-csp # django-debug-toolbar # django-extensions @@ -121,7 +122,7 @@ django-annoying==0.10.6 # via -r requirements/pip.txt django-autoslug==1.9.8 # via -r requirements/pip.txt -django-cors-middleware==1.4.0 +django-cors-headers==3.13.0 # via -r requirements/pip.txt django-crispy-forms==1.14.0 # via -r requirements/pip.txt @@ -454,7 +455,7 @@ vine==5.0.0 # amqp # celery # kombu -virtualenv==20.17.1 +virtualenv==20.18.0 # via -r requirements/pip.txt wcwidth==0.2.6 # via diff --git a/requirements/pip.in b/requirements/pip.in index 4c26517080f..5fb25d329f7 100644 --- a/requirements/pip.in +++ b/requirements/pip.in @@ -107,9 +107,8 @@ dj-pagination # Version comparison stuff packaging -# django-cors-middleware==1.5.0 fails with -# AttributeError: 'dict' object has no attribute 'has_header' -django-cors-middleware==1.4.0 +# Allow cross-site requests to some of our APIs. +django-cors-headers # User agent parsing - used for analytics purposes user-agents diff --git a/requirements/pip.txt b/requirements/pip.txt index e926752ba80..81c7ec95630 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -14,9 +14,9 @@ babel==2.11.0 # via sphinx billiard==3.6.4.0 # via celery -boto3==1.26.64 +boto3==1.26.65 # via django-storages -botocore==1.29.64 +botocore==1.29.65 # via # boto3 # s3transfer @@ -62,6 +62,7 @@ django==3.2.17 # dj-stripe # django-allauth # django-annoying + # django-cors-headers # django-csp # django-debug-toolbar # django-extensions @@ -79,7 +80,7 @@ django-annoying==0.10.6 # via -r requirements/pip.in django-autoslug==1.9.8 # via -r requirements/pip.in -django-cors-middleware==1.4.0 +django-cors-headers==3.13.0 # via -r requirements/pip.in django-crispy-forms==1.14.0 # via -r requirements/pip.in @@ -294,7 +295,7 @@ vine==5.0.0 # amqp # celery # kombu -virtualenv==20.17.1 +virtualenv==20.18.0 # via -r requirements/pip.in wcwidth==0.2.6 # via prompt-toolkit diff --git a/requirements/testing.txt b/requirements/testing.txt index 9be85321678..5c66a534dff 100644 --- a/requirements/testing.txt +++ b/requirements/testing.txt @@ -26,11 +26,11 @@ billiard==3.6.4.0 # via # -r requirements/pip.txt # celery -boto3==1.26.64 +boto3==1.26.65 # via # -r requirements/pip.txt # django-storages -botocore==1.29.64 +botocore==1.29.65 # via # -r requirements/pip.txt # boto3 @@ -101,6 +101,7 @@ django==3.2.17 # dj-stripe # django-allauth # django-annoying + # django-cors-headers # django-csp # django-debug-toolbar # django-extensions @@ -118,7 +119,7 @@ django-annoying==0.10.6 # via -r requirements/pip.txt django-autoslug==1.9.8 # via -r requirements/pip.txt -django-cors-middleware==1.4.0 +django-cors-headers==3.13.0 # via -r requirements/pip.txt django-crispy-forms==1.14.0 # via -r requirements/pip.txt @@ -429,7 +430,7 @@ vine==5.0.0 # amqp # celery # kombu -virtualenv==20.17.1 +virtualenv==20.18.0 # via -r requirements/pip.txt wcwidth==0.2.6 # via