From de6096b91d295801f71c6cca4ad80a1beee271ed Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 24 Jan 2024 16:59:00 -0500 Subject: [PATCH 1/2] Custom domain: don't allow external domain We have a CNAME from our external domain to readthedocs.io, making these domains valid if they are added to a project. This isn't a security issue, since our code won't resolve those domains because custom domains are checked last, and we have this protection in place: https://github.com/readthedocs/readthedocs.org/blob/7daff3d6cc953898934cc80cba53403d6e0fa259/readthedocs/core/unresolver.py#L590-L594 --- readthedocs/projects/forms.py | 11 +++++++++-- readthedocs/rtd_tests/tests/test_domains.py | 12 ++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index 2c0822b1b5a..3287b847def 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -818,8 +818,15 @@ def clean_domain(self): domain_string = parsed.netloc - # Don't allow production or public domain to be set as custom domain - for invalid_domain in [settings.PRODUCTION_DOMAIN, settings.PUBLIC_DOMAIN]: + # Don't allow internal domains to be added, we have: + # - Dashboard domain + # - Public domain (from where documentation pages are served) + # - External version domain (from where PR previews are served) + for invalid_domain in [ + settings.PRODUCTION_DOMAIN, + settings.PUBLIC_DOMAIN, + settings.RTD_EXTERNAL_VERSION_DOMAIN, + ]: if invalid_domain and domain_string.endswith(invalid_domain): raise forms.ValidationError( f'{invalid_domain} is not a valid domain.' diff --git a/readthedocs/rtd_tests/tests/test_domains.py b/readthedocs/rtd_tests/tests/test_domains.py index 49c3843b16d..a7c0b039978 100644 --- a/readthedocs/rtd_tests/tests/test_domains.py +++ b/readthedocs/rtd_tests/tests/test_domains.py @@ -92,6 +92,18 @@ def test_public_domain_not_allowed(self): f"{settings.PUBLIC_DOMAIN} is not a valid domain.", ) + @override_settings(RTD_EXTERNAL_VERSION_DOMAIN="readthedocs.build") + def test_external_domain_not_allowed(self): + for domain in ["readthedocs.build", "test.readthedocs.build"]: + form = DomainForm( + {"domain": domain}, + project=self.project, + ) + self.assertFalse(form.is_valid()) + self.assertEqual( + form.errors["domain"][0], f"{domain} is not a valid domain." + ) + def test_domain_with_path(self): form = DomainForm( {"domain": "domain.com/foo/bar"}, From 10dbd3311feb6cd3c33d1b0bdabf62eb67f97d26 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 24 Jan 2024 19:25:01 -0500 Subject: [PATCH 2/2] Fix test --- readthedocs/rtd_tests/tests/test_domains.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_domains.py b/readthedocs/rtd_tests/tests/test_domains.py index a7c0b039978..2af9ab831f7 100644 --- a/readthedocs/rtd_tests/tests/test_domains.py +++ b/readthedocs/rtd_tests/tests/test_domains.py @@ -101,7 +101,7 @@ def test_external_domain_not_allowed(self): ) self.assertFalse(form.is_valid()) self.assertEqual( - form.errors["domain"][0], f"{domain} is not a valid domain." + form.errors["domain"][0], "readthedocs.build is not a valid domain." ) def test_domain_with_path(self):