Proxito: fix https and canonical redirects

readthedocs/proxito/constants.py

@@ -0,0 +1,3 @@
+REDIRECT_HTTPS = 'https'
+REDIRECT_CANONICAL_CNAME = 'canonical-cname'
+REDIRECT_SUBPROJECT_MAIN_DOMAIN = 'subproject-main-domain'

readthedocs/proxito/middleware.py

@@ -6,16 +6,17 @@
 Additional processing is done to get the project from the URL in the ``views.py`` as well.
 """
 import logging
-import sys
 import re
+import sys
 from urllib.parse import urlparse
 
 from django.conf import settings
-from django.shortcuts import render, redirect
-from django.utils.deprecation import MiddlewareMixin
+from django.shortcuts import redirect, render
 from django.urls import reverse
+from django.utils.deprecation import MiddlewareMixin
 
-from readthedocs.projects.models import Domain, Project
+from readthedocs.projects.models import Domain, Project, ProjectRelationship
+from readthedocs.proxito import constants
 
 log = logging.getLogger(__name__)  # noqa
 
@@ -65,7 +66,13 @@ def map_host_to_project_slug(request):  # pylint: disable=too-many-return-statem
                 https=True,
             ).exists():
                 log.debug('Proxito Public Domain -> Canonical Domain Redirect: host=%s', host)
-                request.canonicalize = 'canonical-cname'
+                request.canonicalize = constants.REDIRECT_CANONICAL_CNAME
+            elif (
+                ProjectRelationship.objects.
+                filter(child__slug=project_slug).exists()
+            ):
+                log.debug('Proxito Public Domain -> Subproject Main Domain Redirect: host=%s', host)
+                request.canonicalize = constants.REDIRECT_SUBPROJECT_MAIN_DOMAIN
             return project_slug
 
         # TODO: This can catch some possibly valid domains (docs.readthedocs.io.com) for example
@@ -101,7 +108,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)
-            request.canonicalize = 'https'
+            request.canonicalize = constants.REDIRECT_HTTPS
 
         # NOTE: consider redirecting non-canonical custom domains to the canonical one
         # Whether that is another custom domain or the public domain

readthedocs/proxito/tests/test_middleware.py

@@ -73,6 +73,42 @@ def test_canonical_cname_redirect(self):
             self.assertTrue(hasattr(request, 'canonicalize'))
             self.assertEqual(request.canonicalize, 'canonical-cname')
 
+    def test_subproject_redirect(self):
+        """Requests to a subproject should redirect to the domain of the main project."""
+        subproject = get(
+            Project,
+            name='subproject',
+            slug='subproject',
+            users=[self.owner],
+            privacy_level=PUBLIC,
+        )
+        subproject.versions.update(privacy_level=PUBLIC)
+        get(
+            ProjectRelationship,
+            parent=self.pip,
+            child=subproject,
+        )
+
+        for url in (self.url, '/subdir/', '/en/latest/'):
+            request = self.request(url, HTTP_HOST='subproject.dev.readthedocs.io')
+            res = self.run_middleware(request)
+            self.assertIsNone(res)
+            self.assertEqual(getattr(request, 'canonicalize', None), 'subproject-main-domain')
+
+        # Using a custom domain in a subproject isn't supported (or shouldn't be!).
+        cname = 'docs.random.com'
+        domain = get(
+            Domain,
+            project=subproject,
+            domain=cname,
+            canonical=True,
+            https=True,
+        )
+        request = self.request(self.url, HTTP_HOST='subproject.dev.readthedocs.io')
+        res = self.run_middleware(request)
+        self.assertIsNone(res)
+        self.assertEqual(getattr(request, 'canonicalize', None), 'canonical-cname')
+
     # We are not canonicalizing custom domains -> public domain for now
     @pytest.mark.xfail(strict=True)
     def test_canonical_cname_redirect_public_domain(self):

readthedocs/proxito/tests/test_redirects.py

@@ -26,6 +26,28 @@ def test_root_url(self):
             r['Location'], 'https://project.dev.readthedocs.io/en/latest/',
         )
 
+    def test_custom_domain_root_url(self):
+        self.domain.canonical = True
+        self.domain.save()
+
+        r = self.client.get('/', HTTP_HOST=self.domain.domain, secure=True)
+        self.assertEqual(r.status_code, 302)
+        self.assertEqual(
+            r['Location'], f'https://{self.domain.domain}/en/latest/',
+        )
+        self.assertEqual(r['X-RTD-Redirect'], 'system')
+
+    def test_custom_domain_root_url_no_slash(self):
+        self.domain.canonical = True
+        self.domain.save()
+
+        r = self.client.get('', HTTP_HOST=self.domain.domain, secure=True)
+        self.assertEqual(r.status_code, 302)
+        self.assertEqual(
+            r['Location'], f'https://{self.domain.domain}/en/latest/',
+        )
+        self.assertEqual(r['X-RTD-Redirect'], 'system')
+
     def test_single_version_root_url_doesnt_redirect(self):
         self.project.single_version = True
         self.project.save()
@@ -55,6 +77,57 @@ def test_single_version_subproject_root_url_no_slash(self):
             r['Location'], 'https://project.dev.readthedocs.io/projects/subproject/',
         )
 
+    def test_subproject_redirect(self):
+        r = self.client.get('/', HTTP_HOST='subproject.dev.readthedocs.io')
+        self.assertEqual(r.status_code, 302)
+        self.assertEqual(
+            r['Location'], 'https://project.dev.readthedocs.io/projects/subproject/en/latest/',
+        )
+
+        r = self.client.get('/en/latest/', HTTP_HOST='subproject.dev.readthedocs.io')
+        self.assertEqual(r.status_code, 302)
+        self.assertEqual(
+            r['Location'], 'https://project.dev.readthedocs.io/projects/subproject/en/latest/',
+        )
+
+        r = self.client.get('/en/latest/foo/bar', HTTP_HOST='subproject.dev.readthedocs.io')
+        self.assertEqual(r.status_code, 302)
+        self.assertEqual(
+            r['Location'], 'https://project.dev.readthedocs.io/projects/subproject/en/latest/foo/bar',
+        )
+
+        self.domain.canonical = True
+        self.domain.save()
+        r = self.client.get('/en/latest/foo/bar', HTTP_HOST='subproject.dev.readthedocs.io')
+        self.assertEqual(r.status_code, 302)
+        self.assertEqual(
+            r['Location'], 'https://docs1.example.com/projects/subproject/en/latest/foo/bar',
+        )
+
+    def test_single_version_subproject_redirect(self):
+        self.subproject.single_version = True
+        self.subproject.save()
+
+        r = self.client.get('/', HTTP_HOST='subproject.dev.readthedocs.io')
+        self.assertEqual(r.status_code, 302)
+        self.assertEqual(
+            r['Location'], 'https://project.dev.readthedocs.io/projects/subproject/',
+        )
+
+        r = self.client.get('/foo/bar/', HTTP_HOST='subproject.dev.readthedocs.io')
+        self.assertEqual(r.status_code, 302)
+        self.assertEqual(
+            r['Location'], 'https://project.dev.readthedocs.io/projects/subproject/foo/bar/',
+        )
+
+        self.domain.canonical = True
+        self.domain.save()
+        r = self.client.get('/foo/bar', HTTP_HOST='subproject.dev.readthedocs.io')
+        self.assertEqual(r.status_code, 302)
+        self.assertEqual(
+            r['Location'], 'https://docs1.example.com/projects/subproject/foo/bar',
+        )
+
     def test_root_redirect_with_query_params(self):
         r = self.client.get('/?foo=bar', HTTP_HOST='project.dev.readthedocs.io')
         self.assertEqual(r.status_code, 302)
@@ -70,7 +143,7 @@ def test_canonicalize_https_redirect(self):
         r = self.client.get('/', HTTP_HOST=self.domain.domain)
         self.assertEqual(r.status_code, 302)
         self.assertEqual(
-            r['Location'], f'https://{self.domain.domain}/en/latest/',
+            r['Location'], f'https://{self.domain.domain}/',
         )
         self.assertEqual(r['X-RTD-Redirect'], 'https')
 
@@ -90,7 +163,7 @@ def test_canonicalize_public_domain_to_cname_redirect(self):
         r = self.client.get('/', HTTP_HOST='project.dev.readthedocs.io')
         self.assertEqual(r.status_code, 302)
         self.assertEqual(
-            r['Location'], f'https://{self.domain.domain}/en/latest/',
+            r['Location'], f'https://{self.domain.domain}/',
         )
         self.assertEqual(r['X-RTD-Redirect'], 'canonical-cname')
 
@@ -102,6 +175,22 @@ def test_canonicalize_public_domain_to_cname_redirect(self):
         )
         self.assertEqual(r['X-RTD-Redirect'], 'canonical-cname')
 
+    def test_translation_redirect(self):
+        r = self.client.get('/', HTTP_HOST='translation.dev.readthedocs.io')
+        self.assertEqual(r.status_code, 302)
+        self.assertEqual(
+            r['Location'], f'https://project.dev.readthedocs.io/es/latest/',
+        )
+        self.assertEqual(r['X-RTD-Redirect'], 'system')
+
+    def test_translation_secure_redirect(self):
+        r = self.client.get('/', HTTP_HOST='translation.dev.readthedocs.io', secure=True)
+        self.assertEqual(r.status_code, 302)
+        self.assertEqual(
+            r['Location'], f'https://project.dev.readthedocs.io/es/latest/',
+        )
+        self.assertEqual(r['X-RTD-Redirect'], 'system')
+
     # We are not canonicalizing custom domains -> public domain for now
     @pytest.mark.xfail(strict=True)
     def test_canonicalize_cname_to_public_domain_redirect(self):

readthedocs/proxito/views/mixins.py

@@ -2,20 +2,24 @@
 import logging
 import mimetypes
 from urllib.parse import urlparse, urlunparse
-from slugify import slugify as unicode_slugify
 
 from django.conf import settings
 from django.http import (
     HttpResponse,
-    HttpResponseRedirect,
     HttpResponsePermanentRedirect,
+    HttpResponseRedirect,
 )
 from django.shortcuts import render
 from django.utils.encoding import iri_to_uri
 from django.views.static import serve
+from slugify import slugify as unicode_slugify
 
 from readthedocs.builds.constants import EXTERNAL, INTERNAL
 from readthedocs.core.resolver import resolve
+from readthedocs.proxito.constants import (
+    REDIRECT_CANONICAL_CNAME,
+    REDIRECT_HTTPS,
+)
 from readthedocs.redirects.exceptions import InfiniteRedirectException
 from readthedocs.storage import build_media_storage
 
@@ -169,19 +173,40 @@ def canonical_redirect(self, request, final_project, version_slug, filename):
         """
         Return a redirect to the canonical domain including scheme.
 
-        This is normally used HTTP -> HTTPS redirects or redirects to/from custom domains.
+        The following cases are covered:
+
+        - Redirect a custom from http to https (if supported)
+          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
         """
-        full_path = request.get_full_path()
-        urlparse_result = urlparse(full_path)
-        to = resolve(
-            project=final_project,
-            version_slug=version_slug,
-            filename=filename,
-            query_params=urlparse_result.query,
-            external=hasattr(request, 'external_domain'),
-        )
+        from_url = request.build_absolute_uri()
+        parsed_from = urlparse(from_url)
 
-        if full_path == to:
+        redirect_type = getattr(request, 'canonicalize', None)
+        if redirect_type == REDIRECT_HTTPS:
+            to = parsed_from._replace(scheme='https').geturl()
+        else:
+            to = resolve(
+                project=final_project,
+                version_slug=version_slug,
+                filename=filename,
+                query_params=parsed_from.query,
+                external=hasattr(request, 'external_domain'),
+            )
+            # When a canonical redirect is done, only change the domain.
+            if redirect_type == REDIRECT_CANONICAL_CNAME:
+                to_parsed = urlparse(to)
+                to = parsed_from._replace(
+                    scheme=to_parsed.scheme,
+                    netloc=to_parsed.netloc,
+                ).geturl()
+
+        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=%s',

readthedocs/rtd_tests/tests/test_footer.py

@@ -7,15 +7,12 @@
 from django_dynamic_fixture import get
 from rest_framework.test import APIRequestFactory
 
-from readthedocs.api.v2.views.footer_views import (
-    FooterHTML,
-    get_version_compare_data,
-)
+from readthedocs.api.v2.views.footer_views import get_version_compare_data
 from readthedocs.builds.constants import BRANCH, EXTERNAL, LATEST, TAG
 from readthedocs.builds.models import Version
 from readthedocs.core.middleware import ReadTheDocsSessionMiddleware
 from readthedocs.projects.constants import PUBLIC
-from readthedocs.projects.models import Feature, Project
+from readthedocs.projects.models import Project
 
 
 class BaseTestFooterHTML:
@@ -427,7 +424,7 @@ def test_highest_version_without_tags(self):
 class TestFooterPerformance(TestCase):
     # The expected number of queries for generating the footer
     # This shouldn't increase unless we modify the footer API
-    EXPECTED_QUERIES = 14
+    EXPECTED_QUERIES = 15
 
     def setUp(self):
         self.pip = get(
@@ -485,7 +482,6 @@ def test_domain_queries(self):
             canonical=True,
         )
 
-        # Setting up a custom domain increases only one query.
-        with self.assertNumQueries(self.EXPECTED_QUERIES + 1):
+        with self.assertNumQueries(self.EXPECTED_QUERIES):
             response = self.client.get(self.url, HTTP_HOST=domain)
             self.assertContains(response, domain)