Proxito: pass unresolved domain in request (#9982)

Make use of the unresolved domain from the request instead of each of the attributes we are setting in the request from the middleware, we aren't using any of those on .com, so we are safe removing them here.

This is on top of #9974.

Author: stsewd

readthedocs/audit/models.py

@@ -7,7 +7,6 @@
 
 from readthedocs.acl.utils import get_auth_backend
 from readthedocs.analytics.utils import get_client_ip
-from readthedocs.projects.models import Project
 
 
 class AuditLogManager(models.Manager):
@@ -50,9 +49,9 @@ def new(self, action, user=None, request=None, **kwargs):
 
             # Fill the project from the request if available.
             # This is frequently on actions generated from a subdomain.
-            project_slug = getattr(request, 'host_project_slug', None)
-            if 'project' not in kwargs and project_slug:
-                kwargs['project'] = Project.objects.filter(slug=project_slug).first()
+            unresolved_domain = getattr(request, "unresolved_domain", None)
+            if "project" not in kwargs and unresolved_domain:
+                kwargs["project"] = unresolved_domain.project
 
         return self.create(
             user=user,

readthedocs/core/unresolver.py

@@ -81,6 +81,22 @@ class UnresolvedDomain:
     domain: Domain = None
     external_version_slug: str = None
 
+    @property
+    def is_from_custom_domain(self):
+        return self.source == DomainSourceType.custom_domain
+
+    @property
+    def is_from_public_domain(self):
+        return self.source == DomainSourceType.public_domain
+
+    @property
+    def is_from_http_header(self):
+        return self.source == DomainSourceType.http_header
+
+    @property
+    def is_from_external_domain(self):
+        return self.source == DomainSourceType.external_domain
+
 
 class Unresolver:
     # This pattern matches:
@@ -136,7 +152,7 @@ def unresolve(self, url, append_indexhtml=True):
         )
 
         # Make sure we are serving the external version from the subdomain.
-        if unresolved_domain.source == DomainSourceType.external_domain and version:
+        if unresolved_domain.is_from_external_domain and version:
             if unresolved_domain.external_version_slug != version.slug:
                 log.warning(
                     "Invalid version for external domain.",
@@ -166,7 +182,7 @@ def unresolve(self, url, append_indexhtml=True):
             filename=filename,
             parsed_url=parsed,
             domain=unresolved_domain.domain,
-            external=unresolved_domain.source == DomainSourceType.external_domain,
+            external=unresolved_domain.is_from_external_domain,
         )
 
     @staticmethod

readthedocs/proxito/middleware.py

@@ -18,7 +18,6 @@
 from django.utils.deprecation import MiddlewareMixin
 
 from readthedocs.core.unresolver import (
-    DomainSourceType,
     InvalidCustomDomainError,
     InvalidExternalDomainError,
     InvalidSubdomainError,
@@ -75,17 +74,13 @@ def add_proxito_headers(self, request, response):
         if cache_tags:
             response['Cache-Tag'] = ','.join(cache_tags)
 
-        if hasattr(request, 'rtdheader'):
-            response['X-RTD-Project-Method'] = 'rtdheader'
-        elif hasattr(request, 'subdomain'):
-            response['X-RTD-Project-Method'] = 'subdomain'
-        elif hasattr(request, 'cname'):
-            response['X-RTD-Project-Method'] = 'cname'
-
-        if hasattr(request, 'external_domain'):
-            response['X-RTD-Version-Method'] = 'domain'
-        else:
-            response['X-RTD-Version-Method'] = 'path'
+        unresolved_domain = request.unresolved_domain
+        if unresolved_domain:
+            response["X-RTD-Project-Method"] = unresolved_domain.source.name
+            if unresolved_domain.is_from_external_domain:
+                response["X-RTD-Version-Method"] = "domain"
+            else:
+                response["X-RTD-Version-Method"] = "path"
 
     def add_user_headers(self, request, response):
         """
@@ -94,19 +89,21 @@ def add_user_headers(self, request, response):
         The headers added come from ``projects.models.HTTPHeader`` associated
         with the ``Domain`` object.
         """
-        if hasattr(request, 'domain'):
+        unresolved_domain = request.unresolved_domain
+        if unresolved_domain and unresolved_domain.is_from_custom_domain:
             response_headers = [header.lower() for header in response.headers.keys()]
-            for http_header in request.domain.http_headers.all():
+            domain = unresolved_domain.domain
+            for http_header in domain.http_headers.all():
                 if http_header.name.lower() in response_headers:
                     log.error(
                         'Overriding an existing response HTTP header.',
                         http_header=http_header.name,
-                        domain=request.domain.domain,
+                        domain=domain.domain,
                     )
                 log.info(
                     'Adding custom response HTTP header.',
                     http_header=http_header.name,
-                    domain=request.domain.domain,
+                    domain=domain.domain,
                 )
 
                 if http_header.only_if_secure_request and not request.is_secure():
@@ -129,17 +126,20 @@ def add_hsts_headers(self, request, response):
             # Only set the HSTS header if the request is over HTTPS
             return response
 
-        host = request.get_host().lower().split(':')[0]
-        public_domain = settings.PUBLIC_DOMAIN.lower().split(':')[0]
         hsts_header_values = []
-        if settings.PUBLIC_DOMAIN_USES_HTTPS and public_domain in host:
+        unresolved_domain = request.unresolved_domain
+        if (
+            settings.PUBLIC_DOMAIN_USES_HTTPS
+            and unresolved_domain
+            and unresolved_domain.is_from_public_domain
+        ):
             hsts_header_values = [
                 'max-age=31536000',
                 'includeSubDomains',
                 'preload',
             ]
-        elif hasattr(request, 'domain'):
-            domain = request.domain
+        elif unresolved_domain and unresolved_domain.is_from_custom_domain:
+            domain = unresolved_domain.domain
             # TODO: migrate Domains with HSTS set using these fields to
             # ``HTTPHeader`` and remove this chunk of code from here.
             if domain.hsts_max_age:
@@ -169,10 +169,11 @@ def add_cache_headers(self, request, response):
         See https://developers.cloudflare.com/cache/about/cdn-cache-control.
         """
         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 hasattr(request, "rtdheader"):
+        if unresolved_domain and unresolved_domain.is_from_http_header:
             response.headers[header] = "private"
 
         if settings.ALLOW_PRIVATE_REPOS:
@@ -183,32 +184,18 @@ def _set_request_attributes(self, request, unresolved_domain):
         """
         Set attributes in the request from the unresolved domain.
 
-        - If the project was extracted from the ``X-RTD-Slug`` header,
-          we set ``request.rtdheader`` to `True`.
-        - If the project was extracted from the public domain,
-          we set ``request.subdomain`` to `True`.
-        - If the project was extracted from a custom domain,
-          we set ``request.cname`` to `True`.
+        - Set ``request.unresolved_domain`` to the unresolved domain.
         - If the domain needs to redirect, set the canonicalize attribute accordingly.
         """
-        # TODO: Set the unresolved domain in the request instead of each of these attributes.
-        source = unresolved_domain.source
+        request.unresolved_domain = unresolved_domain
         project = unresolved_domain.project
-        if source == DomainSourceType.http_header:
-            request.rtdheader = True
-        elif source == DomainSourceType.custom_domain:
+        if unresolved_domain.is_from_custom_domain:
             domain = unresolved_domain.domain
-            request.cname = True
-            request.domain = domain
             if domain.https and not request.is_secure():
                 # Redirect HTTP -> HTTPS (302) for this custom domain.
                 log.debug("Proxito CNAME HTTPS Redirect.", domain=domain.domain)
                 request.canonicalize = constants.REDIRECT_HTTPS
-        elif source == DomainSourceType.external_domain:
-            request.external_domain = True
-            request.host_version_slug = unresolved_domain.external_version_slug
-        elif source == DomainSourceType.public_domain:
-            request.subdomain = True
+        elif unresolved_domain.is_from_public_domain:
             canonical_domain = (
                 Domain.objects.filter(project=project)
                 .filter(canonical=True, https=True)
@@ -226,10 +213,11 @@ def _set_request_attributes(self, request, unresolved_domain):
                     project_slug=project.slug,
                 )
                 request.canonicalize = constants.REDIRECT_SUBPROJECT_MAIN_DOMAIN
-        else:
-            raise NotImplementedError
 
     def process_request(self, request):  # noqa
+        # Initialize our custom request attributes.
+        request.unresolved_domain = None
+
         skip = any(
             request.path.startswith(reverse(view))
             for view in self.skip_views
@@ -292,9 +280,6 @@ def process_request(self, request):  # noqa
             project_slug=project.slug,
         )
 
-        # Otherwise set the slug on the request
-        request.host_project_slug = request.slug = project.slug
-
         # This is hacky because Django wants a module for the URLConf,
         # instead of also accepting string
         if project.urlconf:

readthedocs/proxito/tests/test_headers.py

@@ -21,23 +21,25 @@ def test_redirect_headers(self):
         self.assertEqual(
             r['Location'], 'https://project.dev.readthedocs.io/en/latest/',
         )
-        self.assertEqual(r['Cache-Tag'], 'project')
-        self.assertEqual(r['X-RTD-Project'], 'project')
-        self.assertEqual(r['X-RTD-Project-Method'], 'subdomain')
-        self.assertEqual(r['X-RTD-Domain'], 'project.dev.readthedocs.io')
-        self.assertIsNone(r.get('X-RTD-Version'))
-        self.assertIsNone(r.get('X-RTD-Path'))
+        self.assertEqual(r["Cache-Tag"], "project")
+        self.assertEqual(r["X-RTD-Project"], "project")
+        self.assertEqual(r["X-RTD-Project-Method"], "public_domain")
+        self.assertEqual(r["X-RTD-Domain"], "project.dev.readthedocs.io")
+        self.assertIsNone(r.get("X-RTD-Version"))
+        self.assertIsNone(r.get("X-RTD-Path"))
 
     def test_serve_headers(self):
         r = self.client.get('/en/latest/', HTTP_HOST='project.dev.readthedocs.io')
         self.assertEqual(r.status_code, 200)
-        self.assertEqual(r['Cache-Tag'], 'project,project:latest')
-        self.assertEqual(r['X-RTD-Domain'], 'project.dev.readthedocs.io')
-        self.assertEqual(r['X-RTD-Project'], 'project')
-        self.assertEqual(r['X-RTD-Project-Method'], 'subdomain')
-        self.assertEqual(r['X-RTD-Version'], 'latest')
-        self.assertEqual(r['X-RTD-version-Method'], 'path')
-        self.assertEqual(r['X-RTD-Path'], '/proxito/media/html/project/latest/index.html')
+        self.assertEqual(r["Cache-Tag"], "project,project:latest")
+        self.assertEqual(r["X-RTD-Domain"], "project.dev.readthedocs.io")
+        self.assertEqual(r["X-RTD-Project"], "project")
+        self.assertEqual(r["X-RTD-Project-Method"], "public_domain")
+        self.assertEqual(r["X-RTD-Version"], "latest")
+        self.assertEqual(r["X-RTD-version-Method"], "path")
+        self.assertEqual(
+            r["X-RTD-Path"], "/proxito/media/html/project/latest/index.html"
+        )
 
     def test_subproject_serve_headers(self):
         r = self.client.get('/projects/subproject/en/latest/', HTTP_HOST='project.dev.readthedocs.io')
@@ -49,7 +51,7 @@ def test_subproject_serve_headers(self):
         # I think it's not accurate saying that it's `subdomain` the method
         # that we use to get the project slug here, since it was in fact the
         # URL's path but we don't have that feature built
-        self.assertEqual(r['X-RTD-Project-Method'], 'subdomain')
+        self.assertEqual(r["X-RTD-Project-Method"], "public_domain")
 
         self.assertEqual(r['X-RTD-Version'], 'latest')
         self.assertEqual(r['X-RTD-version-Method'], 'path')
@@ -58,13 +60,13 @@ def test_subproject_serve_headers(self):
     def test_404_headers(self):
         r = self.client.get('/foo/bar.html', HTTP_HOST='project.dev.readthedocs.io')
         self.assertEqual(r.status_code, 404)
-        self.assertEqual(r['Cache-Tag'], 'project')
-        self.assertEqual(r['X-RTD-Domain'], 'project.dev.readthedocs.io')
-        self.assertEqual(r['X-RTD-Project'], 'project')
-        self.assertEqual(r['X-RTD-Project-Method'], 'subdomain')
-        self.assertEqual(r['X-RTD-version-Method'], 'path')
-        self.assertIsNone(r.get('X-RTD-Version'))
-        self.assertIsNone(r.get('X-RTD-Path'))
+        self.assertEqual(r["Cache-Tag"], "project")
+        self.assertEqual(r["X-RTD-Domain"], "project.dev.readthedocs.io")
+        self.assertEqual(r["X-RTD-Project"], "project")
+        self.assertEqual(r["X-RTD-Project-Method"], "public_domain")
+        self.assertEqual(r["X-RTD-version-Method"], "path")
+        self.assertIsNone(r.get("X-RTD-Version"))
+        self.assertIsNone(r.get("X-RTD-Path"))
 
     def test_custom_domain_headers(self):
         hostname = 'docs.random.com'
@@ -75,13 +77,15 @@ def test_custom_domain_headers(self):
         )
         r = self.client.get("/en/latest/", HTTP_HOST=hostname)
         self.assertEqual(r.status_code, 200)
-        self.assertEqual(r['Cache-Tag'], 'project,project:latest')
-        self.assertEqual(r['X-RTD-Domain'], self.domain.domain)
-        self.assertEqual(r['X-RTD-Project'], self.project.slug)
-        self.assertEqual(r['X-RTD-Project-Method'], 'cname')
-        self.assertEqual(r['X-RTD-Version'], 'latest')
-        self.assertEqual(r['X-RTD-version-Method'], 'path')
-        self.assertEqual(r['X-RTD-Path'], '/proxito/media/html/project/latest/index.html')
+        self.assertEqual(r["Cache-Tag"], "project,project:latest")
+        self.assertEqual(r["X-RTD-Domain"], self.domain.domain)
+        self.assertEqual(r["X-RTD-Project"], self.project.slug)
+        self.assertEqual(r["X-RTD-Project-Method"], "custom_domain")
+        self.assertEqual(r["X-RTD-Version"], "latest")
+        self.assertEqual(r["X-RTD-version-Method"], "path")
+        self.assertEqual(
+            r["X-RTD-Path"], "/proxito/media/html/project/latest/index.html"
+        )
 
     def test_footer_headers(self):
         version = self.project.versions.get(slug=LATEST)

readthedocs/proxito/tests/test_middleware.py

@@ -41,8 +41,8 @@ def test_proper_cname(self):
         request = self.request(method='get', path=self.url, HTTP_HOST=domain)
         res = self.run_middleware(request)
         self.assertIsNone(res)
-        self.assertEqual(request.cname, True)
-        self.assertEqual(request.host_project_slug, 'pip')
+        self.assertTrue(request.unresolved_domain.is_from_custom_domain)
+        self.assertEqual(request.unresolved_domain.project, self.pip)
 
     def test_proper_cname_https_upgrade(self):
         cname = 'docs.random.com'
@@ -138,8 +138,8 @@ def test_proper_cname_uppercase(self):
         get(Domain, project=self.pip, domain='docs.random.com')
         request = self.request(method='get', path=self.url, HTTP_HOST='docs.RANDOM.COM')
         self.run_middleware(request)
-        self.assertEqual(request.cname, True)
-        self.assertEqual(request.host_project_slug, 'pip')
+        self.assertTrue(request.unresolved_domain.is_from_custom_domain)
+        self.assertEqual(request.unresolved_domain.project, self.pip)
 
     def test_invalid_cname(self):
         self.assertFalse(Domain.objects.filter(domain='my.host.com').exists())
@@ -151,17 +151,17 @@ def test_invalid_cname(self):
     def test_proper_subdomain(self):
         request = self.request(method='get', path=self.url, HTTP_HOST='pip.dev.readthedocs.io')
         self.run_middleware(request)
-        self.assertEqual(request.subdomain, True)
-        self.assertEqual(request.host_project_slug, 'pip')
+        self.assertTrue(request.unresolved_domain.is_from_public_domain)
+        self.assertEqual(request.unresolved_domain.project, self.pip)
 
     @override_settings(PUBLIC_DOMAIN='foo.bar.readthedocs.io')
     def test_subdomain_different_length(self):
         request = self.request(
             method='get', path=self.url, HTTP_HOST='pip.foo.bar.readthedocs.io'
         )
         self.run_middleware(request)
-        self.assertEqual(request.subdomain, True)
-        self.assertEqual(request.host_project_slug, 'pip')
+        self.assertTrue(request.unresolved_domain.is_from_public_domain)
+        self.assertEqual(request.unresolved_domain.project, self.pip)
 
     def test_request_header(self):
         get(
@@ -171,8 +171,8 @@ def test_request_header(self):
             method='get', path=self.url, HTTP_HOST='some.random.com', HTTP_X_RTD_SLUG='pip'
         )
         self.run_middleware(request)
-        self.assertEqual(request.rtdheader, True)
-        self.assertEqual(request.host_project_slug, 'pip')
+        self.assertTrue(request.unresolved_domain.is_from_http_header)
+        self.assertEqual(request.unresolved_domain.project, self.pip)
 
     def test_request_header_uppercase(self):
         get(
@@ -183,8 +183,8 @@ def test_request_header_uppercase(self):
         )
         self.run_middleware(request)
 
-        self.assertEqual(request.rtdheader, True)
-        self.assertEqual(request.host_project_slug, 'pip')
+        self.assertTrue(request.unresolved_domain.is_from_http_header)
+        self.assertEqual(request.unresolved_domain.project, self.pip)
 
     def test_request_header_not_allowed(self):
         request = self.request(