Proxito: handle http to https redirects for all requests (#10199)

stsewd · web-flow · commit 121fa70ce06f · 2023-04-19T13:37:50.000-05:00
In order to be able to cache the redirects, I had to refactor some of our code (and kind of a little in the direction of #10124). Closes #10144
diff --git a/readthedocs/api/mixins.py b/readthedocs/api/mixins.py
@@ -7,6 +7,7 @@
 from readthedocs.core.unresolver import UnresolverError, unresolve
 from readthedocs.core.utils import get_cache_tag
 from readthedocs.projects.models import Project
+from readthedocs.proxito.cache import add_cache_tags
 
 log = structlog.get_logger(__name__)
 
@@ -31,7 +32,7 @@ def dispatch(self, request, *args, **kwargs):
         response = super().dispatch(request, *args, **kwargs)
         cache_tags = self._get_cache_tags()
         if cache_tags:
-            response["Cache-Tag"] = ",".join(cache_tags)
+            add_cache_tags(response, cache_tags)
         return response
 
     def _get_cache_tags(self):
diff --git a/readthedocs/core/mixins.py b/readthedocs/core/mixins.py
@@ -2,6 +2,8 @@
 from django.contrib.auth.mixins import LoginRequiredMixin
 from vanilla import ListView
 
+from readthedocs.proxito.cache import cache_response, private_response
+
 
 class ListViewWithForm(ListView):
 
@@ -53,11 +55,12 @@ class CDNCacheControlMixin:
 
     def dispatch(self, request, *args, **kwargs):
         response = super().dispatch(request, *args, **kwargs)
-        cache_response = self.can_be_cached(request)
-        if cache_response is not None:
-            response.headers["CDN-Cache-Control"] = (
-                "public" if cache_response else "private"
-            )
+        can_be_cached = self.can_be_cached(request)
+        if can_be_cached is not None:
+            if can_be_cached:
+                cache_response(response)
+            else:
+                private_response(response)
         return response
 
     def can_be_cached(self, request):
diff --git a/readthedocs/proxito/cache.py b/readthedocs/proxito/cache.py
@@ -0,0 +1,67 @@
+import structlog
+
+log = structlog.get_logger(__name__)
+
+CACHE_TAG_HEADER = "Cache-Tag"
+CDN_CACHE_CONTROL_HEADER = "CDN-Cache-Control"
+
+
+def add_cache_tags(response, cache_tags):
+    """
+    Add cache tags to the response.
+
+    New cache tags will be appended to the existing ones.
+
+    :param response: The response to add cache tags to.
+    :param cache_tags: A list of cache tags to add to the response.
+    """
+    cache_tags = cache_tags.copy()
+    current_cache_tag = response.headers.get(CACHE_TAG_HEADER)
+    if current_cache_tag:
+        cache_tags.append(current_cache_tag)
+
+    response.headers[CACHE_TAG_HEADER] = ",".join(cache_tags)
+
+
+def cache_response(response, cache_tags=None, force=True):
+    """
+    Cache the response at the CDN level.
+
+    We add the ``Cache-Tag`` header to the response, to be able to purge
+    the cache by a given tag. And we set the ``CDN-Cache-Control: public`` header
+    to cache the response at the CDN level only. This doesn't
+    affect caching at the browser level (``Cache-Control``).
+
+    See:
+
+    - https://developers.cloudflare.com/cache/how-to/purge-cache/#cache-tags-enterprise-only
+    - https://developers.cloudflare.com/cache/about/cdn-cache-control.
+
+    :param response: The response to cache.
+    :param cache_tags: A list of cache tags to add to the response.
+    :param force: If ``True``, the header will be set to public even if it
+     was already set to private.
+    """
+    if cache_tags:
+        add_cache_tags(response, cache_tags)
+    if force or CDN_CACHE_CONTROL_HEADER not in response.headers:
+        if not response.headers.get(CACHE_TAG_HEADER):
+            # Caching a response at the CDN level without cache tags is
+            # incorrect, since we won't be able to purge it otherwise.
+            log.warning("Caching response without cache tags.")
+
+        response.headers[CDN_CACHE_CONTROL_HEADER] = "public"
+
+
+def private_response(response, force=True):
+    """
+    Prevent the response from being cached at the CDN level.
+
+    We do this by explicitly setting the ``CDN-Cache-Control`` header to private.
+
+    :param response: The response to mark as private.
+    :param force: If ``True``, the header will be set to private even if it
+     was already set to public.
+    """
+    if force or CDN_CACHE_CONTROL_HEADER not in response.headers:
+        response.headers[CDN_CACHE_CONTROL_HEADER] = "private"
diff --git a/readthedocs/proxito/middleware.py b/readthedocs/proxito/middleware.py
@@ -27,6 +27,8 @@
 )
 from readthedocs.core.utils import get_cache_tag
 from readthedocs.projects.models import Feature, Project
+from readthedocs.proxito.cache import add_cache_tags, cache_response, private_response
+from readthedocs.proxito.redirects import redirect_to_https
 
 log = structlog.get_logger(__name__)
 
@@ -63,15 +65,14 @@ def add_proxito_headers(self, request, response):
             response['X-RTD-Path'] = path
 
         # Include the project & project-version so we can do larger purges if needed
-        cache_tag = response.get('Cache-Tag')
-        cache_tags = [cache_tag] if cache_tag else []
+        cache_tags = []
         if project_slug:
             cache_tags.append(project_slug)
         if version_slug:
             cache_tags.append(get_cache_tag(project_slug, version_slug))
 
         if cache_tags:
-            response['Cache-Tag'] = ','.join(cache_tags)
+            add_cache_tags(response, cache_tags)
 
         unresolved_domain = request.unresolved_domain
         if unresolved_domain:
@@ -167,22 +168,20 @@ def add_cache_headers(self, request, response):
 
         See https://developers.cloudflare.com/cache/about/cdn-cache-control.
         """
-        cdn_cache_header = "CDN-Cache-Control"
         unresolved_domain = request.unresolved_domain
         # Never trust projects resolving from the X-RTD-Slug header,
         # we don't want to cache their content on domains from other
         # projects, see GHSA-mp38-vprc-7hf5.
         if unresolved_domain and unresolved_domain.is_from_http_header:
-            response.headers[cdn_cache_header] = "private"
+            private_response(response, force=True)
             # SECURITY: Return early, we never want to cache this response.
             return
 
-        # Set the key only if it hasn't already been set by the view.
-        if cdn_cache_header not in response.headers:
-            default_cache_level = (
-                "private" if settings.ALLOW_PRIVATE_REPOS else "public"
-            )
-            response.headers[cdn_cache_header] = default_cache_level
+        # Mark the response as private or cache it, if it hasn't been marked as so already.
+        if settings.ALLOW_PRIVATE_REPOS:
+            private_response(response, force=False)
+        else:
+            cache_response(response, force=False)
 
     def _set_request_attributes(self, request, unresolved_domain):
         """
@@ -233,6 +232,10 @@ def process_request(self, request):  # noqa
 
         self._set_request_attributes(request, unresolved_domain)
 
+        response = self._get_https_redirect(request)
+        if response:
+            return response
+
         # Remove multiple slashes from URL's
         if '//' in request.path:
             url_parsed = urlparse(request.get_full_path())
@@ -250,7 +253,9 @@ def process_request(self, request):  # noqa
                 from_url=request.get_full_path(),
                 to_url=final_url,
             )
-            return redirect(final_url)
+            response = redirect(final_url)
+            cache_response(response, cache_tags=[unresolved_domain.project.slug])
+            return response
 
         project = unresolved_domain.project
         log.debug(
@@ -286,6 +291,37 @@ def add_hosting_integrations_headers(self, request, response):
             if project.has_feature(Feature.HOSTING_INTEGRATIONS):
                 response["X-RTD-Hosting-Integrations"] = "true"
 
+    def _get_https_redirect(self, request):
+        """
+        Get a redirect response if the request should be redirected to HTTPS.
+
+        A request should be redirected to HTTPS if any of the following conditions are met:
+
+        - It's from a custom domain and the domain has HTTPS enabled.
+        - It's from a public domain, and the public domain uses HTTPS.
+        """
+        if request.is_secure():
+            # The request is already HTTPS, so we skip redirecting it.
+            return None
+
+        unresolved_domain = request.unresolved_domain
+
+        # HTTPS redirect for custom domains.
+        if unresolved_domain.is_from_custom_domain:
+            domain = unresolved_domain.domain
+            if domain.https:
+                return redirect_to_https(request, project=unresolved_domain.project)
+            return None
+
+        # HTTPS redirect for public domains.
+        if (
+            unresolved_domain.is_from_public_domain
+            or unresolved_domain.is_from_external_domain
+        ) and settings.PUBLIC_DOMAIN_USES_HTTPS:
+            return redirect_to_https(request, project=unresolved_domain.project)
+
+        return None
+
     def process_response(self, request, response):  # noqa
         self.add_proxito_headers(request, response)
         self.add_cache_headers(request, response)
diff --git a/readthedocs/proxito/redirects.py b/readthedocs/proxito/redirects.py
@@ -0,0 +1,91 @@
+from urllib.parse import urlparse
+
+import structlog
+from django.http import HttpResponseRedirect
+
+from readthedocs.core.resolver import resolver
+from readthedocs.core.utils.url import unsafe_join_url_path
+from readthedocs.proxito.cache import cache_response
+from readthedocs.proxito.constants import RedirectType
+from readthedocs.redirects.exceptions import InfiniteRedirectException
+
+log = structlog.get_logger(__name__)
+
+
+def redirect_to_https(request, project):
+    """
+    Shortcut to get a https redirect.
+
+    :param request: Request object.
+    :param project: The current project being served
+    """
+    return canonical_redirect(request, project, RedirectType.http_to_https)
+
+
+def canonical_redirect(request, project, redirect_type, external_version_slug=None):
+    """
+    Return a canonical redirect response.
+
+    All redirects are cached, since the final URL will be checked for authorization.
+
+    The following cases are covered:
+
+    - Redirect a custom domain from http to https
+        http://project.rtd.io/ -> https://project.rtd.io/
+    - Redirect a domain to a canonical domain (http or https).
+        http://project.rtd.io/ -> https://docs.test.com/
+        http://project.rtd.io/foo/bar/ -> https://docs.test.com/foo/bar/
+    - Redirect from a subproject domain to the main domain
+        https://subproject.rtd.io/en/latest/foo -> https://main.rtd.io/projects/subproject/en/latest/foo  # noqa
+        https://subproject.rtd.io/en/latest/foo -> https://docs.test.com/projects/subproject/en/latest/foo  # noqa
+
+    Raises ``InfiniteRedirectException`` if the redirect is the same as the current URL.
+
+    :param request: Request object.
+    :param project: The current project being served
+    :param redirect_type: The type of canonical redirect (https, canonical-cname, subproject-main-domain)
+    :param external_version_slug: The version slug if the request is from a pull request preview.
+    """
+    from_url = request.build_absolute_uri()
+    parsed_from = urlparse(from_url)
+
+    if redirect_type == RedirectType.http_to_https:
+        # We only need to change the protocol.
+        to = parsed_from._replace(scheme="https").geturl()
+    elif redirect_type == RedirectType.to_canonical_domain:
+        # We need to change the domain and protocol.
+        canonical_domain = project.get_canonical_custom_domain()
+        protocol = "https" if canonical_domain.https else "http"
+        to = parsed_from._replace(
+            scheme=protocol, netloc=canonical_domain.domain
+        ).geturl()
+    elif redirect_type == RedirectType.subproject_to_main_domain:
+        # We need to get the subproject root in the domain of the main
+        # project, and append the current path.
+        project_doc_prefix = resolver.get_url_prefix(
+            project=project,
+            external_version_slug=external_version_slug,
+        )
+        parsed_doc_prefix = urlparse(project_doc_prefix)
+        to = parsed_doc_prefix._replace(
+            path=unsafe_join_url_path(parsed_doc_prefix.path, parsed_from.path),
+            query=parsed_from.query,
+        ).geturl()
+    else:
+        raise NotImplementedError
+
+    if from_url == to:
+        # check that we do have a response and avoid infinite redirect
+        log.warning(
+            "Infinite Redirect: FROM URL is the same than TO URL.",
+            url=to,
+        )
+        raise InfiniteRedirectException()
+
+    log.info(
+        "Canonical Redirect.", host=request.get_host(), from_url=from_url, to_url=to
+    )
+    resp = HttpResponseRedirect(to)
+    resp["X-RTD-Redirect"] = redirect_type.name
+    cache_response(resp, cache_tags=[project.slug])
+    return resp
diff --git a/readthedocs/proxito/tests/test_full.py b/readthedocs/proxito/tests/test_full.py
@@ -1572,14 +1572,12 @@ def _test_cache_control_header_project(self, expected_value, host=None):
             resp.headers["Cache-Tag"], "project,project:rtd-staticfiles,rtd-staticfiles"
         )
 
-        # Slash redirect is done at the middleware level.
-        # So, it doesn't take into consideration the privacy level of the
-        # version, and always defaults to private.
+        # Slash redirects can always be cached.
         url = '/en//latest//'
         resp = self.client.get(url, secure=True, HTTP_HOST=host)
-        self.assertEqual(resp['Location'], '/en/latest/', url)
-        self.assertEqual(resp.headers['CDN-Cache-Control'], 'private', url)
-        self.assertNotIn('Cache-Tag', resp.headers, url)
+        self.assertEqual(resp["Location"], "/en/latest/", url)
+        self.assertEqual(resp.headers["CDN-Cache-Control"], "public", url)
+        self.assertEqual(resp.headers["Cache-Tag"], "project")
 
         # Forced redirects will be cached only if the version is public.
         get(
@@ -1637,14 +1635,12 @@ def _test_cache_control_header_subproject(self, expected_value, host=None):
             resp.headers["Cache-Tag"], "project,project:rtd-staticfiles,rtd-staticfiles"
         )
 
-        # Slash redirect is done at the middleware level.
-        # So, it doesn't take into consideration the privacy level of the
-        # version, and always defaults to private.
+        # Slash redirects can always be cached.
         url = '/projects//subproject//'
         resp = self.client.get(url, secure=True, HTTP_HOST=host)
-        self.assertEqual(resp['Location'], '/projects/subproject/', url)
-        self.assertEqual(resp.headers['CDN-Cache-Control'], 'private', url)
-        self.assertNotIn('Cache-Tag', resp.headers, url)
+        self.assertEqual(resp["Location"], "/projects/subproject/", url)
+        self.assertEqual(resp.headers["CDN-Cache-Control"], "public", url)
+        self.assertEqual(resp.headers["Cache-Tag"], "project")
 
     def test_cache_on_private_versions(self):
         self.project.versions.update(privacy_level=PRIVATE)
diff --git a/readthedocs/proxito/tests/test_headers.py b/readthedocs/proxito/tests/test_headers.py
@@ -175,14 +175,18 @@ def test_cache_headers_robots_txt_with_private_projects_allowed(self):
 
     @override_settings(ALLOW_PRIVATE_REPOS=False)
     def test_cache_headers_robots_txt_with_private_projects_not_allowed(self):
-        r = self.client.get("/sitemap.xml", HTTP_HOST="project.dev.readthedocs.io")
+        r = self.client.get(
+            "/sitemap.xml", secure=True, HTTP_HOST="project.dev.readthedocs.io"
+        )
         self.assertEqual(r.status_code, 200)
         self.assertEqual(r["CDN-Cache-Control"], "public")
         self.assertEqual(r["Cache-Tag"], "project,project:sitemap.xml")
 
     @override_settings(ALLOW_PRIVATE_REPOS=True)
     def test_cache_headers_robots_txt_with_private_projects_allowed(self):
-        r = self.client.get("/sitemap.xml", HTTP_HOST="project.dev.readthedocs.io")
+        r = self.client.get(
+            "/sitemap.xml", secure=True, HTTP_HOST="project.dev.readthedocs.io"
+        )
         self.assertEqual(r.status_code, 200)
         self.assertEqual(r["CDN-Cache-Control"], "public")
         self.assertEqual(r["Cache-Tag"], "project,project:sitemap.xml")
diff --git a/readthedocs/proxito/tests/test_middleware.py b/readthedocs/proxito/tests/test_middleware.py
@@ -45,7 +45,9 @@ def run_middleware(self, request):
     def test_proper_cname(self):
         domain = 'docs.random.com'
         get(Domain, project=self.pip, domain=domain)
-        request = self.request(method='get', path=self.url, HTTP_HOST=domain)
+        request = self.request(
+            method="get", secure=True, path=self.url, HTTP_HOST=domain
+        )
         res = self.run_middleware(request)
         self.assertIsNone(res)
         self.assertTrue(request.unresolved_domain.is_from_custom_domain)
diff --git a/readthedocs/proxito/tests/test_redirects.py b/readthedocs/proxito/tests/test_redirects.py
diff --git a/readthedocs/proxito/views/mixins.py b/readthedocs/proxito/views/mixins.py
diff --git a/readthedocs/proxito/views/serve.py b/readthedocs/proxito/views/serve.py
