Proxito: pass unresolved domain in request

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(