From 17e661641ddaf8266e7430d83cfb2039abc55df7 Mon Sep 17 00:00:00 2001 From: Tobias McNulty Date: Fri, 3 Jun 2016 15:02:38 -0700 Subject: [PATCH] Refs #26666 -- Added ALLOWED_HOSTS validation when running tests. Also used ALLOWED_HOSTS to check for external hosts in assertRedirects(). --- django/test/testcases.py | 17 +++++-- django/test/utils.py | 3 +- docs/ref/settings.txt | 10 ++-- docs/releases/1.11.txt | 5 ++ docs/spelling_wordlist | 1 + docs/topics/testing/advanced.txt | 55 ++++++++++++++++++++++ tests/cache/tests.py | 1 + tests/sites_tests/tests.py | 5 +- tests/staticfiles_tests/test_liveserver.py | 2 +- tests/test_client/tests.py | 8 +++- tests/test_client_regress/tests.py | 10 +++- 11 files changed, 105 insertions(+), 12 deletions(-) diff --git a/django/test/testcases.py b/django/test/testcases.py index ba7a9ccd6c..c1e9379d55 100644 --- a/django/test/testcases.py +++ b/django/test/testcases.py @@ -29,6 +29,7 @@ from django.core.servers.basehttp import WSGIRequestHandler, WSGIServer from django.db import DEFAULT_DB_ALIAS, connection, connections, transaction from django.forms.fields import CharField from django.http import QueryDict +from django.http.request import split_domain_port, validate_host from django.test.client import Client from django.test.html import HTMLParseError, parse_html from django.test.signals import setting_changed, template_rendered @@ -306,10 +307,13 @@ class SimpleTestCase(unittest.TestCase): # netloc might be empty, or in cases where Django tests the # HTTP scheme, the convention is for netloc to be 'testserver'. # Trust both as "internal" URLs here. - if netloc and netloc != 'testserver': + domain, port = split_domain_port(netloc) + if domain and not validate_host(domain, settings.ALLOWED_HOSTS): raise ValueError( - "The Django test client is unable to fetch remote URLs (got %s). " - "Use assertRedirects(..., fetch_redirect_response=False) instead." % url + "The test client is unable to fetch remote URLs (got %s). " + "If the host is served by Django, add '%s' to ALLOWED_HOSTS. " + "Otherwise, use assertRedirects(..., fetch_redirect_response=False)." + % (url, domain) ) redirect_response = response.client.get(path, QueryDict(query), secure=(scheme == 'https')) @@ -1324,9 +1328,12 @@ class LiveServerTestCase(TransactionTestCase): conn.allow_thread_sharing = True connections_override[conn.alias] = conn - # Launch the live server's thread specified_address = os.environ.get( 'DJANGO_LIVE_TEST_SERVER_ADDRESS', 'localhost:8081-8179') + cls._live_server_modified_settings = modify_settings( + ALLOWED_HOSTS={'append': specified_address.split(':')[0]}, + ) + cls._live_server_modified_settings.enable() # The specified ports may be of the form '8000-8010,8080,9200-9300' # i.e. a comma-separated list of ports or ranges of ports, so we break @@ -1348,6 +1355,7 @@ class LiveServerTestCase(TransactionTestCase): except Exception: msg = 'Invalid address ("%s") for live server.' % specified_address six.reraise(ImproperlyConfigured, ImproperlyConfigured(msg), sys.exc_info()[2]) + # Launch the live server's thread cls.server_thread = cls._create_server_thread(host, possible_ports, connections_override) cls.server_thread.daemon = True cls.server_thread.start() @@ -1386,6 +1394,7 @@ class LiveServerTestCase(TransactionTestCase): @classmethod def tearDownClass(cls): cls._tearDownClassInternal() + cls._live_server_modified_settings.disable() super(LiveServerTestCase, cls).tearDownClass() diff --git a/django/test/utils.py b/django/test/utils.py index e0b2b0da6c..ce235a5cea 100644 --- a/django/test/utils.py +++ b/django/test/utils.py @@ -111,7 +111,8 @@ def setup_test_environment(): settings.EMAIL_BACKEND = 'django.core.mail.backends.locmem.EmailBackend' request._original_allowed_hosts = settings.ALLOWED_HOSTS - settings.ALLOWED_HOSTS = ['*'] + # Add the default host of the test client. + settings.ALLOWED_HOSTS = settings.ALLOWED_HOSTS + ['testserver'] mail.outbox = [] diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt index 01cecd44a4..7b9c8e89fb 100644 --- a/docs/ref/settings.txt +++ b/docs/ref/settings.txt @@ -90,14 +90,18 @@ If the ``Host`` header (or ``X-Forwarded-Host`` if list, the :meth:`django.http.HttpRequest.get_host()` method will raise :exc:`~django.core.exceptions.SuspiciousOperation`. -When :setting:`DEBUG` is ``True`` or when running tests, host validation is -disabled; any host will be accepted. Thus it's usually only necessary to set it -in production. +When :setting:`DEBUG` is ``True``, host validation is disabled; any host will +be accepted. ``ALLOWED_HOSTS`` is :ref:`checked when running tests +`. This validation only applies via :meth:`~django.http.HttpRequest.get_host()`; if your code accesses the ``Host`` header directly from ``request.META`` you are bypassing this security protection. +.. versionchanged:: 1.11 + + In older versions, ``ALLOWED_HOSTS`` wasn't checked when running tests. + .. setting:: APPEND_SLASH ``APPEND_SLASH`` diff --git a/docs/releases/1.11.txt b/docs/releases/1.11.txt index 019b52f0f8..9a1cc81d8d 100644 --- a/docs/releases/1.11.txt +++ b/docs/releases/1.11.txt @@ -262,6 +262,11 @@ Miscellaneous * CSRF failures are logged to the ``django.security.csrf ``` logger instead of ``django.request``. +* :setting:`ALLOWED_HOSTS` validation is no longer disabled when running tests. + If your application includes tests with custom host names, you must include + those host names in :setting:`ALLOWED_HOSTS`. See + :ref:`topics-testing-advanced-multiple-hosts`. + * Using a foreign key's id (e.g. ``'field_id'``) in ``ModelAdmin.list_display`` displays the related object's ID. Remove the ``_id`` suffix if you want the old behavior of the string representation of the object. diff --git a/docs/spelling_wordlist b/docs/spelling_wordlist index 4355bbd065..557d139a3e 100644 --- a/docs/spelling_wordlist +++ b/docs/spelling_wordlist @@ -498,6 +498,7 @@ multiline multilinestring multipart multipolygon +multitenancy multithreaded multithreading Mumbai diff --git a/docs/topics/testing/advanced.txt b/docs/topics/testing/advanced.txt index 754904e1d0..d69e95e5a8 100644 --- a/docs/topics/testing/advanced.txt +++ b/docs/topics/testing/advanced.txt @@ -67,6 +67,61 @@ The following is a simple unit test using the request factory:: response = MyView.as_view()(request) self.assertEqual(response.status_code, 200) +.. _topics-testing-advanced-multiple-hosts: + +Tests and multiple host names +============================= + +The :setting:`ALLOWED_HOSTS` setting is validated when running tests. This +allows the test client to differentiate between internal and external URLs. + +Projects that support multitenancy or otherwise alter business logic based on +the request's host and use custom host names in tests must include those hosts +in :setting:`ALLOWED_HOSTS`. + +The first and simplest option to do so is to add the hosts to your settings +file. For example, the test suite for docs.djangoproject.com includes the +following:: + + from django.test import TestCase + + class SearchFormTestCase(TestCase): + def test_empty_get(self): + response = self.client.get('/en/dev/search/', HTTP_HOST='docs.djangoproject.dev:8000') + self.assertEqual(response.status_code, 200) + +and the settings file includes a list of the domains supported by the project:: + + ALLOWED_HOSTS = [ + 'www.djangoproject.dev', + 'docs.djangoproject.dev', + ... + ] + +Another option is to add the required hosts to :setting:`ALLOWED_HOSTS` using +:meth:`~django.test.override_settings()` or +:attr:`~django.test.SimpleTestCase.modify_settings()`. This option may be +preferable in standalone apps that can't package their own settings file or +for projects where the list of domains is not static (e.g., subdomains for +multitenancy). For example, you could write a test for the domain +``http://otherserver/`` as follows:: + + from django.test import TestCase, override_settings + + class MultiDomainTestCase(TestCase): + @override_settings(ALLOWED_HOSTS=['otherserver']) + def test_other_domain(self): + response = self.client.get('http://otherserver/foo/bar/') + +Disabling :setting:`ALLOWED_HOSTS` checking (``ALLOWED_HOSTS = ['*']``) when +running tests prevents the test client from raising a helpful error message if +you follow a redirect to an external URL. + +.. versionchanged:: 1.11 + + Older versions didn't validate ``ALLOWED_HOSTS`` while testing so these + techniques weren't necessary. + .. _topics-testing-advanced-multidb: Tests and multiple databases diff --git a/tests/cache/tests.py b/tests/cache/tests.py index 9224434dfe..9099402f43 100644 --- a/tests/cache/tests.py +++ b/tests/cache/tests.py @@ -1393,6 +1393,7 @@ class DefaultNonExpiringCacheKeyTests(SimpleTestCase): }, }, USE_I18N=False, + ALLOWED_HOSTS=['.example.com'], ) class CacheUtils(SimpleTestCase): """TestCase for django.utils.cache functions.""" diff --git a/tests/sites_tests/tests.py b/tests/sites_tests/tests.py index 7a4215cc4a..8c540f82ea 100644 --- a/tests/sites_tests/tests.py +++ b/tests/sites_tests/tests.py @@ -142,6 +142,7 @@ class SitesFrameworkTests(TestCase): with self.assertRaises(ValidationError): site.full_clean() + @override_settings(ALLOWED_HOSTS=['example.com']) def test_clear_site_cache(self): request = HttpRequest() request.META = { @@ -162,7 +163,7 @@ class SitesFrameworkTests(TestCase): clear_site_cache(Site, instance=self.site, using='default') self.assertEqual(models.SITE_CACHE, {}) - @override_settings(SITE_ID='') + @override_settings(SITE_ID='', ALLOWED_HOSTS=['example2.com']) def test_clear_site_cache_domain(self): site = Site.objects.create(name='example2.com', domain='example2.com') request = HttpRequest() @@ -191,6 +192,7 @@ class SitesFrameworkTests(TestCase): self.assertEqual(Site.objects.get_by_natural_key(self.site.domain), self.site) self.assertEqual(self.site.natural_key(), (self.site.domain,)) + @override_settings(ALLOWED_HOSTS=['example.com']) def test_requestsite_save_notimplemented_msg(self): # Test response msg for RequestSite.save NotImplementedError request = HttpRequest() @@ -201,6 +203,7 @@ class SitesFrameworkTests(TestCase): with self.assertRaisesMessage(NotImplementedError, msg): RequestSite(request).save() + @override_settings(ALLOWED_HOSTS=['example.com']) def test_requestsite_delete_notimplemented_msg(self): # Test response msg for RequestSite.delete NotImplementedError request = HttpRequest() diff --git a/tests/staticfiles_tests/test_liveserver.py b/tests/staticfiles_tests/test_liveserver.py index ceabaaa5d9..2c9d7b71db 100644 --- a/tests/staticfiles_tests/test_liveserver.py +++ b/tests/staticfiles_tests/test_liveserver.py @@ -35,9 +35,9 @@ class LiveServerBase(StaticLiveServerTestCase): @classmethod def tearDownClass(cls): + super(LiveServerBase, cls).tearDownClass() # Restore original settings cls.settings_override.disable() - super(LiveServerBase, cls).tearDownClass() class StaticLiveServerChecks(LiveServerBase): diff --git a/tests/test_client/tests.py b/tests/test_client/tests.py index 1c43a2957e..3db42cb1c2 100644 --- a/tests/test_client/tests.py +++ b/tests/test_client/tests.py @@ -601,7 +601,13 @@ class ClientTest(TestCase): a relevant ValueError rather than a non-descript AssertionError. """ response = self.client.get('/django_project_redirect/') - with self.assertRaisesMessage(ValueError, 'unable to fetch'): + msg = ( + "The test client is unable to fetch remote URLs (got " + "https://www.djangoproject.com/). If the host is served by Django, " + "add 'www.djangoproject.com' to ALLOWED_HOSTS. " + "Otherwise, use assertRedirects(..., fetch_redirect_response=False)." + ) + with self.assertRaisesMessage(ValueError, msg): self.assertRedirects(response, 'https://www.djangoproject.com/') def test_session_modifying_view(self): diff --git a/tests/test_client_regress/tests.py b/tests/test_client_regress/tests.py index ea33d66a02..d03b2d14e9 100644 --- a/tests/test_client_regress/tests.py +++ b/tests/test_client_regress/tests.py @@ -15,7 +15,8 @@ from django.template import ( ) from django.template.response import SimpleTemplateResponse from django.test import ( - Client, SimpleTestCase, TestCase, ignore_warnings, override_settings, + Client, SimpleTestCase, TestCase, ignore_warnings, modify_settings, + override_settings, ) from django.test.client import RedirectCycleError, RequestFactory, encode_file from django.test.utils import ContextList, str_prefix @@ -455,6 +456,7 @@ class AssertRedirectsTests(SimpleTestCase): self.assertRedirects(response, '/no_template_view/', 302, 200) self.assertEqual(len(response.redirect_chain), 3) + @modify_settings(ALLOWED_HOSTS={'append': 'otherserver'}) def test_redirect_to_different_host(self): "The test client will preserve scheme, host and port changes" response = self.client.get('/redirect_other_host/', follow=True) @@ -467,6 +469,12 @@ class AssertRedirectsTests(SimpleTestCase): self.assertEqual(response.request.get('wsgi.url_scheme'), 'https') self.assertEqual(response.request.get('SERVER_NAME'), 'otherserver') self.assertEqual(response.request.get('SERVER_PORT'), '8443') + # assertRedirects() can follow redirect to 'otherserver' too. + response = self.client.get('/redirect_other_host/', follow=False) + self.assertRedirects( + response, 'https://otherserver:8443/no_template_view/', + status_code=302, target_status_code=200 + ) def test_redirect_chain_on_non_redirect_page(self): "An assertion is raised if the original page couldn't be retrieved as expected"