diff --git a/readthedocs/proxito/constants.py b/readthedocs/proxito/constants.py new file mode 100644 index 00000000000..b2022b44707 --- /dev/null +++ b/readthedocs/proxito/constants.py @@ -0,0 +1,3 @@ +REDIRECT_HTTPS = 'https' +REDIRECT_CANONICAL_CNAME = 'canonical-cname' +REDIRECT_SUBPROJECT_MAIN_DOMAIN = 'subproject-main-domain' diff --git a/readthedocs/proxito/middleware.py b/readthedocs/proxito/middleware.py index badba3eff29..eef46eab8dc 100644 --- a/readthedocs/proxito/middleware.py +++ b/readthedocs/proxito/middleware.py @@ -16,6 +16,7 @@ from django.utils.deprecation import MiddlewareMixin from readthedocs.projects.models import Domain, Project, ProjectRelationship +from readthedocs.proxito import constants log = logging.getLogger(__name__) # noqa @@ -65,13 +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 = 'subproject-main-domain' + request.canonicalize = constants.REDIRECT_SUBPROJECT_MAIN_DOMAIN return project_slug # TODO: This can catch some possibly valid domains (docs.readthedocs.io.com) for example @@ -107,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 diff --git a/readthedocs/proxito/tests/test_redirects.py b/readthedocs/proxito/tests/test_redirects.py index 16ef51689d5..d5e3cbec81c 100644 --- a/readthedocs/proxito/tests/test_redirects.py +++ b/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() @@ -121,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') @@ -141,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') @@ -153,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): diff --git a/readthedocs/proxito/views/mixins.py b/readthedocs/proxito/views/mixins.py index ee95dbffca3..152e397e8cc 100644 --- a/readthedocs/proxito/views/mixins.py +++ b/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 domain 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: + parsed_to = urlparse(to) + to = parsed_from._replace( + scheme=parsed_to.scheme, + netloc=parsed_to.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',