From 8ef115f92fcea285db93a946bfde1edd95dd0dbe Mon Sep 17 00:00:00 2001 From: David Fischer Date: Wed, 8 Apr 2020 17:07:45 -0700 Subject: [PATCH 1/3] Redirect HTTP -> HTTPS for custom domains - Only redirect if the appropriate flag is set on the Domain (default is not to redirect) - Use a 302 redirect for now. Ideally this is a 301 redirect, but redirecting other people's domains with a 301 is always a bit risky. --- readthedocs/proxito/middleware.py | 8 +++++++- readthedocs/proxito/tests/test_middleware.py | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/readthedocs/proxito/middleware.py b/readthedocs/proxito/middleware.py index e30dc12eb99..c29993061a4 100644 --- a/readthedocs/proxito/middleware.py +++ b/readthedocs/proxito/middleware.py @@ -8,7 +8,7 @@ import logging from django.conf import settings -from django.shortcuts import render +from django.shortcuts import render, redirect from django.utils.deprecation import MiddlewareMixin from readthedocs.projects.models import Domain, Project @@ -83,6 +83,12 @@ def map_host_to_project_slug(request): # pylint: disable=too-many-return-statem project_slug = domain.project.slug request.cname = True log.debug('Proxito CNAME: host=%s', host) + + if domain.https and not request.is_secure(): + # Redirect HTTP -> HTTPS (302) for this custom domain + log.debug('Proxito CNAME HTTPS Redirect: host=%s', host) + return redirect('https://%s%s' % (host, request.get_full_path())) + return project_slug # Some person is CNAMEing to us without configuring a domain - 404. diff --git a/readthedocs/proxito/tests/test_middleware.py b/readthedocs/proxito/tests/test_middleware.py index ffa7297b87e..66038c0edb6 100644 --- a/readthedocs/proxito/tests/test_middleware.py +++ b/readthedocs/proxito/tests/test_middleware.py @@ -38,6 +38,20 @@ def test_proper_cname(self): self.assertEqual(request.cname, True) self.assertEqual(request.host_project_slug, 'pip') + def test_proper_cname_https_upgrade(self): + domain = 'docs.random.com' + get(Domain, project=self.pip, domain=domain, https=True) + + for url in (self.url, '/subdir/'): + request = self.request(url, HTTP_HOST=domain) + res = self.run_middleware(request) + self.assertIsNotNone(res) + self.assertEqual(res.status_code, 302) + self.assertEqual( + res['Location'], + f'https://{domain}{url}', + ) + def test_proper_cname_uppercase(self): get(Domain, project=self.pip, domain='docs.random.com') request = self.request(self.url, HTTP_HOST='docs.RANDOM.COM') From 4b1f3f11e2a7f274ec7085e15de8c7f4ccc839d3 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Thu, 9 Apr 2020 16:25:39 -0700 Subject: [PATCH 2/3] Use a format string --- 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 c29993061a4..0da7d0b8930 100644 --- a/readthedocs/proxito/middleware.py +++ b/readthedocs/proxito/middleware.py @@ -87,7 +87,7 @@ def map_host_to_project_slug(request): # pylint: disable=too-many-return-statem if domain.https and not request.is_secure(): # Redirect HTTP -> HTTPS (302) for this custom domain log.debug('Proxito CNAME HTTPS Redirect: host=%s', host) - return redirect('https://%s%s' % (host, request.get_full_path())) + return redirect('https://{}{}'.format(host, request.get_full_path())) return project_slug From 60d8495ad2625124de305e310dda79081cf7290a Mon Sep 17 00:00:00 2001 From: David Fischer Date: Tue, 14 Apr 2020 10:57:00 -0700 Subject: [PATCH 3/3] Add a header for debugging redirects from the Proxito Middleware --- dockerfiles/nginx/proxito.conf | 2 ++ readthedocs/proxito/middleware.py | 4 +++- readthedocs/proxito/tests/test_middleware.py | 4 ++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/dockerfiles/nginx/proxito.conf b/dockerfiles/nginx/proxito.conf index 704404bfd7b..cd24b0ddbc3 100644 --- a/dockerfiles/nginx/proxito.conf +++ b/dockerfiles/nginx/proxito.conf @@ -58,6 +58,8 @@ server { add_header X-RTD-Domain $rtd_domain always; set $rtd_method $upstream_http_x_rtd_version_method; add_header X-RTD-Version-Method $rtd_method always; + set $rtd_redirect $upstream_http_x_rtd_redirect; + add_header X-RTD-Redirect $rtd_redirect always; } # Serve 404 pages here diff --git a/readthedocs/proxito/middleware.py b/readthedocs/proxito/middleware.py index 0da7d0b8930..cdbb6908997 100644 --- a/readthedocs/proxito/middleware.py +++ b/readthedocs/proxito/middleware.py @@ -87,7 +87,9 @@ def map_host_to_project_slug(request): # pylint: disable=too-many-return-statem if domain.https and not request.is_secure(): # Redirect HTTP -> HTTPS (302) for this custom domain log.debug('Proxito CNAME HTTPS Redirect: host=%s', host) - return redirect('https://{}{}'.format(host, request.get_full_path())) + resp = redirect('https://{}{}'.format(host, request.get_full_path())) + resp['X-RTD-Redirect'] = 'https' + return resp return project_slug diff --git a/readthedocs/proxito/tests/test_middleware.py b/readthedocs/proxito/tests/test_middleware.py index 66038c0edb6..02cf1040822 100644 --- a/readthedocs/proxito/tests/test_middleware.py +++ b/readthedocs/proxito/tests/test_middleware.py @@ -51,6 +51,10 @@ def test_proper_cname_https_upgrade(self): res['Location'], f'https://{domain}{url}', ) + self.assertEqual( + res['X-RTD-Redirect'], + 'https', + ) def test_proper_cname_uppercase(self): get(Domain, project=self.pip, domain='docs.random.com')