diff --git a/termsandconditions/tests.py b/termsandconditions/tests.py index 08cb6ca6..99c65a42 100644 --- a/termsandconditions/tests.py +++ b/termsandconditions/tests.py @@ -242,6 +242,32 @@ def test_accept(self): ) self.assertContains(accept_version_post_response, "Secure") + def test_accept_redirect_safe(self): + # Pre-accept terms 2 and 3 + UserTermsAndConditions.objects.create(user=self.user1, terms=self.terms2) + UserTermsAndConditions.objects.create(user=self.user1, terms=self.terms3) + + LOGGER.debug("Test user1 login for test_accept_redirect") + login_response = self.client.login(username="user1", password="user1password") + self.assertTrue(login_response) + + LOGGER.debug("Test /terms/accept/site-terms/1/ post") + accept_response = self.client.post( + "/terms/accept/", {"terms": 1, "returnTo": "/secure/"}, follow=True + ) + self.assertRedirects(accept_response, "/secure/") + + def test_accept_redirect_unsafe(self): + # Pre-accept terms 2 and 3 + UserTermsAndConditions.objects.create(user=self.user1, terms=self.terms2) + UserTermsAndConditions.objects.create(user=self.user1, terms=self.terms3) + + LOGGER.debug("Test /terms/accept/contrib-terms/3/ post") + accept_response = self.client.post( + "/terms/accept/", {"terms": 3, "returnTo": "http://attacker/"}, follow=False + ) + self.assertRedirects(accept_response, "/") + def test_accept_store_ip_address(self): """Test with IP address storage setting true (default)""" self.client.login(username="user1", password="user1password") diff --git a/termsandconditions/views.py b/termsandconditions/views.py index cf9c4ec5..1eb96881 100644 --- a/termsandconditions/views.py +++ b/termsandconditions/views.py @@ -1,4 +1,5 @@ """Django Views for the termsandconditions module""" +from urllib.parse import urlparse # pylint: disable=E1120,R0901,R0904 from django.contrib.auth.models import User @@ -79,6 +80,11 @@ def post(self, request, *args, **kwargs): return_url = request.POST.get("returnTo", "/") terms_ids = request.POST.getlist("terms") + parsed = urlparse(return_url) + if parsed.hostname and parsed.hostname not in settings.ALLOWED_HOSTS: + # Make sure the return url is a relative path or a trusted hostname + return_url = '/' + if not terms_ids: # pragma: nocover return HttpResponseRedirect(return_url)