Commit 7261a04b authored by Raphaël Hertzog's avatar Raphaël Hertzog

Merge branch 'fix_issue_36' into 'master'

Fix issue 36

Closes #36

See merge request qa/distro-tracker!54
parents 384781fa 683aa3c0
......@@ -17,3 +17,4 @@ docs/_build/
.coverage
.coverage.*
coverage.html/
venv/
......@@ -19,6 +19,7 @@ from distro_tracker.core.models import (
EmailSettings,
Keyword,
PackageName,
SourcePackageName,
Subscription
)
from distro_tracker.test import TestCase
......@@ -309,7 +310,7 @@ class SubscribeUserToPackageViewTests(TestCase):
self.password = 'asdf'
self.user = User.objects.create_user(
main_email='user@domain.com', password=self.password)
self.package = PackageName.objects.create(name='dummy-package')
self.package = SourcePackageName.objects.create(name='dummy-package')
def log_in_user(self):
self.client.login(username=self.user.main_email, password=self.password)
......
......@@ -11,18 +11,30 @@
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
from distro_tracker.accounts.models import UserEmail
from distro_tracker.core.models import EmailSettings, Keyword, Subscription
from distro_tracker.core.models import (
EmailSettings,
Keyword,
Subscription,
get_web_package
)
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
......@@ -192,26 +204,39 @@ class SubscribeUserToPackageView(LoginRequiredMixin, View):
if email not in users_emails:
return HttpResponseForbidden()
# Create the subscriptions
json_result = {'status': 'ok'}
try:
for email in emails:
Subscription.objects.create_for(
package_name=package,
email=email)
except ValidationError as e:
json_result['status'] = 'failed'
json_result['error'] = e.message
_pkg = get_web_package(package)
_err = None
if _pkg:
try:
for email in emails:
Subscription.objects.create_for(
package_name=package,
email=email)
except ValidationError as e:
_err = e.message
else:
_err = format_html(
"Package {pkg} does not exist.",
pkg=package,
)
if request.is_ajax():
json_result = {'status': 'ok'}
if _err is not None:
json_result = {
'status': 'failed',
'error': _err,
}
return render_to_json_response(json_result)
else:
if 'error' in json_result:
return HttpResponseBadRequest(json_result['error'])
next = request.POST.get('next', None)
if not next:
return redirect('dtracker-package-page', package_name=package)
return redirect(next)
if _err:
return HttpResponseBadRequest(_err)
_next = request.POST.get('next', None)
return safe_redirect(
_next,
resolve_url('dtracker-package-page', package_name=package),
)
class UnsubscribeUserView(LoginRequiredMixin, View):
......@@ -246,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):
......@@ -274,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):
......@@ -292,6 +319,9 @@ class ChooseSubscriptionEmailView(LoginRequiredMixin, View):
if 'package' not in request.GET:
raise Http404
if not get_web_package(request.GET['package']):
raise Http404
return render(request, self.template_name, {
'package': request.GET['package'],
'emails': request.user.emails.all(),
......@@ -346,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:
......
......@@ -33,7 +33,7 @@ from django.urls import reverse
from django.utils import timezone
from django.utils.encoding import force_text
from django.utils.functional import cached_property
from django.utils.html import escape
from django.utils.html import escape, format_html
from django.utils.safestring import mark_safe
from jsonfield import JSONField
......@@ -320,7 +320,10 @@ class PackageName(models.Model):
def save(self, *args, **kwargs):
if not re.match('[0-9a-z][-+.0-9a-z]+$', self.name):
raise ValidationError('Invalid package name: {}'.format(self.name))
raise ValidationError(format_html(
'Invalid package name: {}',
self.name,
))
models.Model.save(self, *args, **kwargs)
......
......@@ -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
......
......@@ -63,14 +63,17 @@ class LoginView(FormView):
return super().get(request, *args, **kwargs)
def form_valid(self, form):
self.success_url = self.request.GET.get(
redirect_url = self.request.GET.get(
self.redirect_parameter, self.success_url)
if not is_safe_url(url=self.success_url, host=self.request.get_host()):
self.success_url = '/'
if not is_safe_url(
redirect_url,
allowed_hosts=set(self.request.get_host())
):
redirect_url = self.success_url
login(self.request, form.get_user())
return redirect(self.success_url)
return redirect(redirect_url)
class LogoutView(View):
......@@ -80,7 +83,14 @@ class LogoutView(View):
def get(self, request):
user = request.user
logout(request)
next_url = request.GET.get(self.redirect_parameter, self.success_url)
if not is_safe_url(
next_url,
allowed_hosts=set(self.request.get_host())
):
next_url = self.success_url
redirect_url = run_hook('post-logout-redirect', request, user, next_url)
if redirect_url:
return redirect(redirect_url)
......
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