Commit 88c14ab4 authored by Pierre-Elliott Bécue's avatar Pierre-Elliott Bécue

Implement a safe_redirect method

This method checks if the redirect url is safe, and if not, redirects to
the provided fallback url.

The fallback url IS NOT checked!!
parent 34895d2e
......@@ -11,8 +11,12 @@
from django.conf import settings
from django.core.exceptions import ValidationError
from django.db.models import Prefetch
from django.http import Http404, HttpResponseBadRequest, HttpResponseForbidden
from django.shortcuts import get_object_or_404, redirect, render
from django.http import (
Http404,
HttpResponseBadRequest,
HttpResponseForbidden
)
from django.shortcuts import get_object_or_404, render, resolve_url
from django.urls import reverse_lazy
from django.utils.html import format_html
from django.views.generic.base import View
......@@ -28,7 +32,9 @@ from distro_tracker.core.utils import (
distro_tracker_render_to_string,
render_to_json_response
)
from distro_tracker.core.utils.http import (
safe_redirect
)
from django_email_accounts import views as email_accounts_views
from django_email_accounts.views import LoginRequiredMixin
......@@ -265,10 +271,11 @@ class UnsubscribeUserView(LoginRequiredMixin, View):
'status': 'ok',
})
else:
if 'next' in request.POST:
return redirect(request.POST['next'])
else:
return redirect('dtracker-package-page', package_name=package)
_next = request.POST.get('next', None)
return safe_redirect(
_next,
resolve_url('dtracker-package-page', package_name=package),
)
class UnsubscribeAllView(LoginRequiredMixin, View):
......@@ -293,10 +300,11 @@ class UnsubscribeAllView(LoginRequiredMixin, View):
'status': 'ok',
})
else:
if 'next' in request.POST:
return redirect(request.POST['next'])
else:
return redirect('dtracker-index')
_next = request.POST.get('next', None)
return safe_redirect(
_next,
resolve_url('dtracker-index'),
)
class ChooseSubscriptionEmailView(LoginRequiredMixin, View):
......@@ -368,10 +376,11 @@ class ModifyKeywordsView(LoginRequiredMixin, View):
'status': 'ok',
})
else:
if 'next' in self.request.POST:
return redirect(self.request.POST['next'])
else:
return redirect('dtracker-index')
_next = self.request.POST.get('next', None)
return safe_redirect(
_next,
resolve_url('dtracker-index'),
)
def post(self, request):
if 'email' not in request.POST or 'keyword[]' not in request.POST:
......
......@@ -27,6 +27,7 @@ from unittest import mock
from debian import deb822
from django.core import mail
from django.http.response import HttpResponseRedirectBase
from django.test.utils import override_settings
from django.urls import reverse
from django.utils.functional import curry
......@@ -56,7 +57,8 @@ from distro_tracker.core.utils.email_messages import (
from distro_tracker.core.utils.http import (
HttpCache,
get_resource_content,
get_resource_text
get_resource_text,
safe_redirect
)
from distro_tracker.core.utils.linkify import (
LinkifyCVELinks,
......@@ -1576,6 +1578,53 @@ class UtilsTests(TestCase):
})
self.assertEqual(checksum, '99914b932bd37a50b983c5e7c90ae93b')
def test_safe_redirect_works(self):
"""Tests the default safe_url"""
_ret = safe_redirect(
"/pkg/dummy",
"/",
)
self.assertIsInstance(_ret, HttpResponseRedirectBase)
self.assertEqual(_ret.url, "/pkg/dummy")
def test_safe_redirect_fallbacks_properly(self):
"""Tests the default safe_url"""
_ret = safe_redirect(
"https://example.com",
"/",
allowed_hosts=None,
)
self.assertIsInstance(_ret, HttpResponseRedirectBase)
self.assertEqual(_ret.url, "/")
def test_safe_redirect_fallbacks_properly_again(self):
"""Tests the default safe_url"""
_ret = safe_redirect(
"https://example.com",
"/",
allowed_hosts=[],
)
self.assertIsInstance(_ret, HttpResponseRedirectBase)
self.assertEqual(_ret.url, "/")
def test_safe_redirect_works_with_allowed_hosts(self):
"""Tests the default safe_url"""
_ret = safe_redirect(
"https://example.com",
"/",
allowed_hosts=["example.com"],
)
self.assertIsInstance(_ret, HttpResponseRedirectBase)
self.assertEqual(_ret.url, "https://example.com")
class CallMethodsTests(TestCase):
def setUp(self):
......
......@@ -18,7 +18,8 @@ from hashlib import md5
from django.conf import settings
from django.utils import timezone
from django.utils.http import parse_http_date
from django.utils.http import parse_http_date, is_safe_url
from django.shortcuts import redirect
import requests
from requests.structures import CaseInsensitiveDict
......@@ -258,3 +259,31 @@ def get_resource_text(*args, **kwargs):
if content is not None:
return content.decode(encoding)
def safe_redirect(to, fallback, allowed_hosts=None):
"""Implements a safe redirection to `to` provided that it's safe. Else,
goes to `fallback`. `allowed_hosts` describes the list of valid hosts for
the call to :func:`django.utils.http.is_safe_url`.
:param to: The URL that one should be returned to.
:type to: str or None
:param fallback: A safe URL to fall back on if `to` isn't safe. WARNING!
This url is NOT checked! The developer is advised to put only an url he
knows to be safe!
:type fallback: str
:param allowed_hosts: A list of "safe" hosts. If `None`, relies on the
default behaviour of :func:`django.utils.http.is_safe_url`.
:type allowed_hosts: list of str
:returns: A ResponseRedirect instance containing the appropriate intel for
the redirection.
:rtype: :class:`django.http.HttpResponseRedirectBase`
"""
if to and is_safe_url(to, allowed_hosts=allowed_hosts):
return redirect(to)
return redirect(fallback)
......@@ -50,6 +50,9 @@ from distro_tracker.core.utils import (
get_or_none,
render_to_json_response
)
from distro_tracker.core.utils.http import (
safe_redirect
)
def package_page(request, package_name):
......@@ -580,10 +583,8 @@ class SetMuteTeamView(LoginRequiredMixin, View):
membership.muted = mute
membership.save()
if 'next' in request.POST:
return redirect(request.POST['next'])
else:
return redirect(team)
_next = request.POST.get('next', None)
return safe_redirect(_next, team)
class SetMembershipKeywords(LoginRequiredMixin, View):
......@@ -596,10 +597,8 @@ class SetMembershipKeywords(LoginRequiredMixin, View):
return render_to_json_response({
'status': 'ok',
})
if 'next' in self.request.POST:
return redirect(self.request.POST['next'])
else:
return redirect(self.team)
_next = self.request.POST.get('next', None)
return safe_redirect(_next, self.team)
def post(self, request, slug):
self.request = request
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment