From a0a2b26c74e45b0944fff28b7abb88630baad3e4 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Mon, 6 Aug 2018 15:15:00 -0700 Subject: [PATCH 1/3] Allow enforcing HTTPS for custom domains --- readthedocs/core/resolver.py | 28 +++++++++++++++----- readthedocs/projects/admin.py | 2 +- readthedocs/projects/forms.py | 2 +- readthedocs/projects/models.py | 2 +- readthedocs/rtd_tests/tests/test_resolver.py | 8 +++--- 5 files changed, 28 insertions(+), 14 deletions(-) diff --git a/readthedocs/core/resolver.py b/readthedocs/core/resolver.py index 77e8c6a85e6..57308be821b 100644 --- a/readthedocs/core/resolver.py +++ b/readthedocs/core/resolver.py @@ -129,7 +129,7 @@ def resolve_path(self, project, filename='', version_slug=None, def resolve_domain(self, project, private=None): # pylint: disable=unused-argument canonical_project = self._get_canonical_project(project) - domain = canonical_project.domains.filter(canonical=True).first() + domain = self._get_project_custom_domain(canonical_project) if domain: return domain.domain elif self._use_subdomain(): @@ -144,13 +144,24 @@ def resolve(self, project, protocol='http', filename='', private=None, version_slug = project.get_default_version() private = self._get_private(project, version_slug) - domain = self.resolve_domain(project, private=private) + canonical_project = self._get_canonical_project(project) + custom_domain = self._get_project_custom_domain(canonical_project) - # Use HTTPS if settings specify - public_domain = getattr(settings, 'PUBLIC_DOMAIN', None) - use_https = getattr(settings, 'PUBLIC_DOMAIN_USES_HTTPS', False) - if use_https and public_domain and public_domain in domain: - protocol = 'https' + if custom_domain: + domain = custom_domain.domain + elif self._use_subdomain(): + domain = self._get_project_subdomain(canonical_project) + else: + domain = getattr(settings, 'PRODUCTION_DOMAIN') + + if custom_domain: + protocol = 'https' if custom_domain.https else 'http' + else: + # Use HTTPS if settings specify + public_domain = getattr(settings, 'PUBLIC_DOMAIN', None) + use_https = getattr(settings, 'PUBLIC_DOMAIN_USES_HTTPS', False) + if use_https and public_domain and public_domain in domain: + protocol = 'https' return '{protocol}://{domain}{path}'.format( protocol=protocol, @@ -196,6 +207,9 @@ def _get_project_subdomain(self, project): subdomain_slug = project.slug.replace('_', '-') return "%s.%s" % (subdomain_slug, public_domain) + def _get_project_custom_domain(self, project): + return project.domains.filter(canonical=True).first() + def _get_private(self, project, version_slug): from readthedocs.builds.models import Version try: diff --git a/readthedocs/projects/admin.py b/readthedocs/projects/admin.py index 3b57d8f81f3..ded6ca10632 100644 --- a/readthedocs/projects/admin.py +++ b/readthedocs/projects/admin.py @@ -184,7 +184,7 @@ class DomainAdmin(admin.ModelAdmin): list_display = ('domain', 'project', 'https', 'count') search_fields = ('domain', 'project__slug') raw_id_fields = ('project',) - list_filter = ('canonical',) + list_filter = ('canonical', 'https') model = Domain diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index 98955711eff..6b3201458f4 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -634,7 +634,7 @@ class DomainForm(forms.ModelForm): class Meta(object): model = Domain - exclude = ['machine', 'cname', 'count', 'https'] + exclude = ['machine', 'cname', 'count'] def __init__(self, *args, **kwargs): self.project = kwargs.pop('project', None) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index a4f7619e9ce..83f198fb5d5 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -985,7 +985,7 @@ class Domain(models.Model): https = models.BooleanField( _('Use HTTPS'), default=False, - help_text=_('SSL is enabled for this domain') + help_text=_('Always use HTTPS for this domain') ) count = models.IntegerField(default=0, help_text=_( 'Number of times this domain has been hit'),) diff --git a/readthedocs/rtd_tests/tests/test_resolver.py b/readthedocs/rtd_tests/tests/test_resolver.py index 30de7844dcd..b925497e01e 100644 --- a/readthedocs/rtd_tests/tests/test_resolver.py +++ b/readthedocs/rtd_tests/tests/test_resolver.py @@ -745,16 +745,16 @@ def test_subproject_with_translation_with_custom_domain(self): ) url = resolve(self.superproject_en, filename='') - self.assertEqual(url, 'http://docs.example.com/en/latest/') + self.assertEqual(url, 'https://docs.example.com/en/latest/') url = resolve(self.superproject_es, filename='') - self.assertEqual(url, 'http://docs.example.com/es/latest/') + self.assertEqual(url, 'https://docs.example.com/es/latest/') # yapf: disable url = resolve(self.subproject_en, filename='') self.assertEqual( url, - ('http://docs.example.com/projects/' + ('https://docs.example.com/projects/' '{subproject.slug}/en/latest/').format( subproject=self.subproject_en, ), @@ -763,7 +763,7 @@ def test_subproject_with_translation_with_custom_domain(self): url = resolve(self.subproject_es, filename='') self.assertEqual( url, - ('http://docs.example.com/projects/' + ('https://docs.example.com/projects/' '{subproject.slug}/es/latest/').format( subproject=self.subproject_en, ), From 6d67a0e35eb7b405121abfdd8e072b5c0c81b690 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Wed, 8 Aug 2018 10:12:42 -0700 Subject: [PATCH 2/3] Updates based on feedback - add resolve HTTPS/HTTP test - add comment detailing duplication --- readthedocs/core/resolver.py | 2 ++ readthedocs/rtd_tests/tests/test_resolver.py | 26 ++++++++++++++++---- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/readthedocs/core/resolver.py b/readthedocs/core/resolver.py index 57308be821b..e2da852ebbe 100644 --- a/readthedocs/core/resolver.py +++ b/readthedocs/core/resolver.py @@ -147,6 +147,8 @@ def resolve(self, project, protocol='http', filename='', private=None, canonical_project = self._get_canonical_project(project) custom_domain = self._get_project_custom_domain(canonical_project) + # This duplication from resolve_domain is for performance purposes + # in order to check whether a custom domain should be HTTPS if custom_domain: domain = custom_domain.domain elif self._use_subdomain(): diff --git a/readthedocs/rtd_tests/tests/test_resolver.py b/readthedocs/rtd_tests/tests/test_resolver.py index b925497e01e..1ef55d564ca 100644 --- a/readthedocs/rtd_tests/tests/test_resolver.py +++ b/readthedocs/rtd_tests/tests/test_resolver.py @@ -471,6 +471,22 @@ def test_resolver_domain(self): url = resolve(project=self.pip) self.assertEqual(url, 'http://docs.foobar.com/en/latest/') + @override_settings(PRODUCTION_DOMAIN='readthedocs.org') + def test_resolver_domain_https(self): + self.domain = fixture.get( + Domain, + domain='docs.foobar.com', + project=self.pip, + https=True, + canonical=True, + ) + with override_settings(USE_SUBDOMAIN=False): + url = resolve(project=self.pip) + self.assertEqual(url, 'https://docs.foobar.com/en/latest/') + with override_settings(USE_SUBDOMAIN=True): + url = resolve(project=self.pip) + self.assertEqual(url, 'https://docs.foobar.com/en/latest/') + @override_settings(PRODUCTION_DOMAIN='readthedocs.org') def test_resolver_subproject(self): with override_settings(USE_SUBDOMAIN=False): @@ -740,21 +756,21 @@ def test_subproject_with_translation_with_custom_domain(self): domain='docs.example.com', canonical=True, cname=True, - https=True, + https=False, project=self.superproject_en, ) url = resolve(self.superproject_en, filename='') - self.assertEqual(url, 'https://docs.example.com/en/latest/') + self.assertEqual(url, 'http://docs.example.com/en/latest/') url = resolve(self.superproject_es, filename='') - self.assertEqual(url, 'https://docs.example.com/es/latest/') + self.assertEqual(url, 'http://docs.example.com/es/latest/') # yapf: disable url = resolve(self.subproject_en, filename='') self.assertEqual( url, - ('https://docs.example.com/projects/' + ('http://docs.example.com/projects/' '{subproject.slug}/en/latest/').format( subproject=self.subproject_en, ), @@ -763,7 +779,7 @@ def test_subproject_with_translation_with_custom_domain(self): url = resolve(self.subproject_es, filename='') self.assertEqual( url, - ('https://docs.example.com/projects/' + ('http://docs.example.com/projects/' '{subproject.slug}/es/latest/').format( subproject=self.subproject_en, ), From 570ce61f4c8534e794abb14fb176612f9f6acbe8 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Wed, 8 Aug 2018 10:18:56 -0700 Subject: [PATCH 3/3] Show certificate issue status --- readthedocs/templates/projects/domain_form.html | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/readthedocs/templates/projects/domain_form.html b/readthedocs/templates/projects/domain_form.html index 48a1ac8a1b2..307e089a39a 100644 --- a/readthedocs/templates/projects/domain_form.html +++ b/readthedocs/templates/projects/domain_form.html @@ -21,6 +21,14 @@ {% endblock %} {% block project_edit_content %} + {% if domain.domainssl %} + {# This references optional code to get the SSL certificate status #} +

+ HTTPS status: + {{ domain.domainssl.status }} +

+ {% endif %} + {% if domain %} {% url 'projects_domains_edit' project.slug domain.pk as action_url %} {% else %}