diff --git a/modoboa/core/tests/test_authentication.py b/modoboa/core/tests/test_authentication.py index 07248bd73..0575a88aa 100644 --- a/modoboa/core/tests/test_authentication.py +++ b/modoboa/core/tests/test_authentication.py @@ -45,6 +45,24 @@ class AuthenticationTestCase(ModoTestCase): self.assertEqual(response.status_code, 302) self.assertTrue(response.url.endswith(reverse("core:dashboard"))) + def test_open_redirect(self): + """Check that open redirect is not allowed.""" + self.client.logout() + data = {"username": "admin", "password": "password"} + + # 1. Check valid redirection + url = "{}?next=/admin/".format(reverse("core:login")) + response = self.client.post(url, data) + self.assertEqual(response.status_code, 302) + self.assertTrue(response.url.endswith(reverse("admin:index"))) + self.client.logout() + + # 2. Check bad redirection + url = "{}?next=http://www.evil.com".format(reverse("core:login")) + response = self.client.post(url, data) + self.assertEqual(response.status_code, 302) + self.assertTrue(response.url.endswith(reverse("core:dashboard"))) + class PasswordResetTestCase(ModoTestCase): """Test password reset service.""" diff --git a/modoboa/core/views/base.py b/modoboa/core/views/base.py index 2470adfa4..8a934ba65 100644 --- a/modoboa/core/views/base.py +++ b/modoboa/core/views/base.py @@ -3,6 +3,7 @@ from __future__ import unicode_literals from django.core.urlresolvers import reverse +from django.utils.http import is_safe_url from django.views import generic from django.contrib.auth import mixins as auth_mixins @@ -15,18 +16,23 @@ def find_nextlocation(request, user): if not user.last_login: # Redirect to profile on first login return reverse("core:user_index") - nextlocation = request.POST.get("next", None) - if nextlocation is None or nextlocation == "None": - if request.user.role == "SimpleUsers": - topredir = request.localconfig.parameters.get_value( - "default_top_redirection") - if topredir != "user": - infos = exts_pool.get_extension_infos(topredir) - nextlocation = infos["topredirection_url"] - else: - nextlocation = reverse("core:user_index") + nextlocation = request.POST.get("next", request.GET.get("next")) + condition = ( + nextlocation is not None and nextlocation != "" and + is_safe_url(nextlocation, host=request.get_host()) + ) + if condition: + return nextlocation + if request.user.role == "SimpleUsers": + topredir = request.localconfig.parameters.get_value( + "default_top_redirection") + if topredir != "user": + infos = exts_pool.get_extension_infos(topredir) + nextlocation = infos["topredirection_url"] else: - nextlocation = reverse("core:dashboard") + nextlocation = reverse("core:user_index") + else: + nextlocation = reverse("core:dashboard") return nextlocation