From a3c2de51c27cb596a2a0925073fd5fe1a54c7d0e Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 14 Sep 2023 14:01:37 +0200 Subject: [PATCH 1/4] Proxito: add CORS headers only on public versions Add `Access-Control-Allow-Origin: *` and `Access-Control-Allow-Methods: OPTIONS, GET` HTTP headers when serving a PUBLIC version. This is required for DocDiff addon to be able to download the original version of the page and compare the DOMs to show the visual differences. Closes https://github.com/readthedocs/addons/issues/93 --- dockerfiles/nginx/proxito.conf.template | 2 ++ readthedocs/proxito/middleware.py | 28 +++++++++++++++++++++++ readthedocs/proxito/tests/test_headers.py | 25 ++++++++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/dockerfiles/nginx/proxito.conf.template b/dockerfiles/nginx/proxito.conf.template index 44559b7d4da..38e463d1026 100644 --- a/dockerfiles/nginx/proxito.conf.template +++ b/dockerfiles/nginx/proxito.conf.template @@ -80,6 +80,8 @@ server { add_header Access-Control-Allow-Origin $access_control_allow_origin always; set $access_control_allow_headers $upstream_http_access_control_allow_headers; add_header Access-Control-Allow-Headers $access_control_allow_headers always; + set $access_control_allow_methods $upstream_http_access_control_allow_methods; + add_header Access-Control-Allow-Methods $access_control_allow_methods always; set $x_frame_options $upstream_http_x_frame_options; add_header X-Frame-Options $x_frame_options always; set $x_content_type_options $upstream_http_x_content_type_options; diff --git a/readthedocs/proxito/middleware.py b/readthedocs/proxito/middleware.py index d779972124a..08816d9b3b6 100644 --- a/readthedocs/proxito/middleware.py +++ b/readthedocs/proxito/middleware.py @@ -25,6 +25,7 @@ unresolver, ) from readthedocs.core.utils import get_cache_tag +from readthedocs.projects.constants import PUBLIC from readthedocs.proxito.cache import add_cache_tags, cache_response, private_response from readthedocs.proxito.redirects import redirect_to_https @@ -278,6 +279,32 @@ def add_hosting_integrations_headers(self, request, response): if addons: response["X-RTD-Hosting-Integrations"] = "true" + def add_cors_headers(self, request, response): + """ + Add CORS headers only on PUBLIC versions. + + DocDiff addons requires making a request from + ``RTD_EXTERNAL_VERSION_DOMAIN`` to ``PUBLIC_DOMAIN`` to be able to + compare both DOMs and show the visual differences. + + This request needs ``Access-Control-Allow-Origin`` HTTP headers to be + accepted by browsers. However, we cannot expose these headers for + documentation that's not PUBLIC. + """ + project_slug = getattr(request, "path_project_slug", "") + version_slug = getattr(request, "path_version_slug", "") + + if project_slug and version_slug: + allow_cors = Version.objects.filter( + project__slug=project_slug, + slug=version_slug, + privacy_level=PUBLIC, + ).exists() + if allow_cors: + response.headers["Access-Control-Allow-Origin"] = "*" + response.headers["Access-Control-Allow-Methods"] = "OPTIONS, GET" + return response + def _get_https_redirect(self, request): """ Get a redirect response if the request should be redirected to HTTPS. @@ -315,4 +342,5 @@ def process_response(self, request, response): # noqa self.add_hsts_headers(request, response) self.add_user_headers(request, response) self.add_hosting_integrations_headers(request, response) + self.add_cors_headers(request, response) return response diff --git a/readthedocs/proxito/tests/test_headers.py b/readthedocs/proxito/tests/test_headers.py index 2f8fd6626d6..3f1fd6564c6 100644 --- a/readthedocs/proxito/tests/test_headers.py +++ b/readthedocs/proxito/tests/test_headers.py @@ -3,6 +3,7 @@ from django.urls import reverse from readthedocs.builds.constants import LATEST +from readthedocs.projects.constants import PRIVATE, PUBLIC from readthedocs.projects.models import Domain, HTTPHeader from .base import BaseDocServing @@ -158,6 +159,30 @@ def test_hosting_integrations_header(self): self.assertIsNotNone(r.get("X-RTD-Hosting-Integrations")) self.assertEqual(r["X-RTD-Hosting-Integrations"], "true") + def test_cors_headers_private_version(self): + version = self.project.versions.get(slug=LATEST) + version.privacy_level = PRIVATE + version.save() + + r = self.client.get( + "/en/latest/", secure=True, headers={"host": "project.dev.readthedocs.io"} + ) + self.assertEqual(r.status_code, 200) + self.assertIsNone(r.get("Access-Control-Allow-Origin")) + self.assertIsNone(r.get("Access-Control-Allow-Methods")) + + def test_cors_headers_public_version(self): + version = self.project.versions.get(slug=LATEST) + version.privacy_level = PUBLIC + version.save() + + r = self.client.get( + "/en/latest/", secure=True, headers={"host": "project.dev.readthedocs.io"} + ) + self.assertEqual(r.status_code, 200) + self.assertEqual(r["Access-Control-Allow-Origin"], "*") + self.assertEqual(r["Access-Control-Allow-Methods"], "OPTIONS, GET") + @override_settings(ALLOW_PRIVATE_REPOS=False) def test_cache_headers_public_version_with_private_projects_not_allowed(self): r = self.client.get( From e13384ab5d20943ce66dcd511b9b77360cef1617 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 18 Sep 2023 20:02:41 +0200 Subject: [PATCH 2/4] Merge conflict fixed --- readthedocs/proxito/middleware.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/readthedocs/proxito/middleware.py b/readthedocs/proxito/middleware.py index 4bf73cfec06..76e38251550 100644 --- a/readthedocs/proxito/middleware.py +++ b/readthedocs/proxito/middleware.py @@ -25,7 +25,8 @@ unresolver, ) from readthedocs.core.utils import get_cache_tag -from readthedocs.projects.constants import PUBLIC, Project +from readthedocs.projects.constants import PUBLIC +from readthedocs.projects.models import Project from readthedocs.proxito.cache import add_cache_tags, cache_response, private_response from readthedocs.proxito.redirects import redirect_to_https From 38cbee08e49bc453616818096b2d83ac99bc99c5 Mon Sep 17 00:00:00 2001 From: Eric Holscher <25510+ericholscher@users.noreply.github.com> Date: Mon, 18 Sep 2023 15:23:23 -0700 Subject: [PATCH 3/4] Update readthedocs/proxito/middleware.py --- readthedocs/proxito/middleware.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/proxito/middleware.py b/readthedocs/proxito/middleware.py index 76e38251550..667f5ea5391 100644 --- a/readthedocs/proxito/middleware.py +++ b/readthedocs/proxito/middleware.py @@ -328,7 +328,7 @@ def add_cors_headers(self, request, response): privacy_level=PUBLIC, ).exists() if allow_cors: - response.headers["Access-Control-Allow-Origin"] = "*" + response.headers["Access-Control-Allow-Origin"] = "*.readthedocs.build" response.headers["Access-Control-Allow-Methods"] = "OPTIONS, GET" return response From cd1698506b720446265c6985fbe117ca53e16693 Mon Sep 17 00:00:00 2001 From: Eric Holscher <25510+ericholscher@users.noreply.github.com> Date: Mon, 18 Sep 2023 15:23:41 -0700 Subject: [PATCH 4/4] Update readthedocs/proxito/tests/test_headers.py --- readthedocs/proxito/tests/test_headers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/proxito/tests/test_headers.py b/readthedocs/proxito/tests/test_headers.py index 3f1fd6564c6..9f619c5655a 100644 --- a/readthedocs/proxito/tests/test_headers.py +++ b/readthedocs/proxito/tests/test_headers.py @@ -180,7 +180,7 @@ def test_cors_headers_public_version(self): "/en/latest/", secure=True, headers={"host": "project.dev.readthedocs.io"} ) self.assertEqual(r.status_code, 200) - self.assertEqual(r["Access-Control-Allow-Origin"], "*") + self.assertEqual(r["Access-Control-Allow-Origin"], "*.readthedocs.build") self.assertEqual(r["Access-Control-Allow-Methods"], "OPTIONS, GET") @override_settings(ALLOW_PRIVATE_REPOS=False)