From 6f30e12cd7f0f7f270f6461d4f17b10177530b50 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Thu, 25 Jun 2020 15:52:07 -0700 Subject: [PATCH] views: Drop use of private Django utility We are currently using django.utils.http.is_safe_url which is a private method and may break API anytime. Replace it with similar but limited implementation. Tests: - Unit tests. - Dismiss a notification and the redirect to the same page happens properly. - Logout, goto to home page or login page. Change the language and it will redirect back to home page or login page appropriately. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/tests/test_views.py | 31 +++++++++++++++++++++++++++++++ plinth/views.py | 28 ++++++++++++++++++++++++++-- 2 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 plinth/tests/test_views.py diff --git a/plinth/tests/test_views.py b/plinth/tests/test_views.py new file mode 100644 index 000000000..f4971a36b --- /dev/null +++ b/plinth/tests/test_views.py @@ -0,0 +1,31 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +""" +Tests for common FreedomBox views. +""" + +import pytest + +from plinth.views import is_safe_url + + +@pytest.mark.parametrize('url', [ + '/plinth/login/', + '/', + 'safe', +]) +def test_is_safe_url_valid_url(url): + """Test valid URLs for safe URL checks.""" + assert is_safe_url(url) + + +@pytest.mark.parametrize('url', [ + '', + None, + '\\plinth', + '///plinth', + 'https://example.com/plinth/login/', + 'https:///plinth/login', +]) +def test_is_safe_url_invalid_url(url): + """Test invalid URLs for safe URL checks.""" + assert not is_safe_url(url) diff --git a/plinth/views.py b/plinth/views.py index 8e4fa5b70..1117cb83c 100644 --- a/plinth/views.py +++ b/plinth/views.py @@ -4,13 +4,13 @@ Main FreedomBox views. """ import time +import urllib.parse from django.contrib import messages from django.core.exceptions import ImproperlyConfigured from django.http import Http404, HttpResponseBadRequest, HttpResponseRedirect from django.template.response import TemplateResponse from django.urls import reverse -from django.utils.http import is_safe_url from django.utils.translation import ugettext as _ from django.views.generic import TemplateView from django.views.generic.edit import FormView @@ -26,11 +26,35 @@ from . import forms, frontpage REDIRECT_FIELD_NAME = 'next' +def is_safe_url(url): + """Check if the URL is safe to redirect to. + + Based on Django internal utility. + + """ + if url is not None: + url = url.strip() + + if not url: + return False + + if '\\' in url or url.startswith('///'): + return False + + result = urllib.parse.urlparse(url) + + # Only accept URLs to the same site and scheme. + if result.scheme or result.netloc: + return False + + return True + + def _get_redirect_url_from_param(request): """Return the redirect URL from 'next' GET/POST param.""" redirect_to = request.GET.get(REDIRECT_FIELD_NAME, '') redirect_to = request.POST.get(REDIRECT_FIELD_NAME, redirect_to) - if is_safe_url(url=redirect_to, allowed_hosts={request.get_host()}): + if is_safe_url(redirect_to): return redirect_to return reverse('index')