misc: return 400 when an improrer next parameter is given to login (#33082)

combo/public/views.py

@@ -60,8 +60,11 @@ def login(request, *args, **kwargs):
     if any(get_idps()):
         if not 'next' in request.GET:
             return HttpResponseRedirect(resolve_url('mellon_login'))
-        return HttpResponseRedirect(resolve_url('mellon_login') + '?next='
-                                    + urllib.quote(request.GET.get('next')))
+        try:
+            quoted_next_url = urllib.quote(request.GET.get('next'))
+        except KeyError:
+            return HttpResponseBadRequest('invalid value for "next" parameter')
+        return HttpResponseRedirect(resolve_url('mellon_login') + '?next=' + quoted_next_url)
     return auth_views.login(request, *args, **kwargs)
 
 def logout(request, next_page=None):

tests/test_public.py

@@ -16,6 +16,11 @@ from django.utils.six.moves.urllib import parse as urlparse
 from django.test import override_settings
 from django.test.utils import CaptureQueriesContext
 
+try:
+    import mellon
+except ImportError:
+    mellon = None
+
 from combo.wsgi import application
 from combo.data.models import (Page, CellBase, TextCell, ParentContentCell,
         FeedCell, LinkCell, ConfigJsonCell, Redirect, JsonCell)
@@ -73,6 +78,16 @@ def test_page_contents_unlogged_only(app, admin_user):
     resp = app.get('/', status=200)
     assert not 'Foobar' in resp.text
 
+@pytest.mark.skipif('mellon is None')
+def test_mellon_login(app):
+    with mock.patch('combo.public.views.get_idps') as get_idps:
+        get_idps.return_value = ['xxx']
+        resp = app.get('/login/')
+        assert urlparse.urlparse(resp.location).path == '/accounts/mellon/login/'
+        resp = app.get('/login/?next=whatever')
+        assert urlparse.urlparse(resp.location).query == 'next=whatever'
+        resp = app.get('/login/?next=%e0%40', status=400)
+
 def test_page_contents_group_presence(app, normal_user):
     group = Group(name='plop')
     group.save()