# HG changeset patch # User Søren Løvborg # Date 1442577469 -7200 # Node ID b537babcf9660eb5c90800433deb59639fe2bbed # Parent a0a9ae753cc4ffa60367de026d66366a67783c27 login: include query parameters in came_from The login controller uses the came_from query argument to determine the page to continue to after login. Previously, came_from specified only the URL path (obtained using h.url.current), and any URL query parameters were passed along as separate (additional) URL query parameters; to obtain the final redirect target, h.url was used to combine came_from with the request.GET. As of this changeset, came_from specifies both the URL path and query string (obtained using request.path_qs), which means that came_from can be used directly as the redirect target (as always, WebOb handles the task of expanding the server relative path to a fully qualified URL). The mangling of request.GET can also be removed. The login code appended arbitrary, user-supplied query parameters to URLs by calling the Routes URLGenerator (h.url) with user-supplied keyword arguments. This construct is unfortunate, since url only appends _unknown_ keyword arguments as query parameters, and the parameter names could overlap with known keyword arguments, possibly affecting the generated URL in various ways. This changeset removes this usage from the login code, but other instances remain. (In practice, the damage is apparently limited to causing an Internal Server Error when going to e.g. "/_admin/login?host=foo", since WebOb returns Unicode strings and URLGenerator only allows byte strings for these keyword arguments.) diff -r a0a9ae753cc4 -r b537babcf966 kallithea/controllers/login.py --- a/kallithea/controllers/login.py Fri Sep 18 13:57:49 2015 +0200 +++ b/kallithea/controllers/login.py Fri Sep 18 13:57:49 2015 +0200 @@ -62,12 +62,12 @@ return not url.scheme and not url.netloc def index(self): - c.came_from = safe_str(request.GET.pop('came_from', '')) + c.came_from = safe_str(request.GET.get('came_from', '')) if c.came_from: if not self._validate_came_from(c.came_from): log.error('Invalid came_from (not server-relative): %r', c.came_from) raise HTTPBadRequest() - came_from = url(c.came_from, **request.GET) + came_from = url(c.came_from) else: c.came_from = came_from = url('home') diff -r a0a9ae753cc4 -r b537babcf966 kallithea/lib/auth.py --- a/kallithea/lib/auth.py Fri Sep 18 13:57:49 2015 +0200 +++ b/kallithea/lib/auth.py Fri Sep 18 13:57:49 2015 +0200 @@ -712,11 +712,12 @@ def redirect_to_login(message=None): from kallithea.lib import helpers as h - p = url.current() + p = request.path_qs if message: h.flash(h.literal(message), category='warning') log.debug('Redirecting to login page, origin: %s', p) - return redirect(url('login_home', came_from=p, **request.GET)) + return redirect(url('login_home', came_from=p)) + class LoginRequired(object): """ diff -r a0a9ae753cc4 -r b537babcf966 kallithea/templates/base/base.html --- a/kallithea/templates/base/base.html Fri Sep 18 13:57:49 2015 +0200 +++ b/kallithea/templates/base/base.html Fri Sep 18 13:57:49 2015 +0200 @@ -294,7 +294,7 @@
%if c.authuser.username == 'default' or c.authuser.user_id is None:

${_('Login to Your Account')}

- ${h.form(h.url('login_home',came_from=h.url.current()))} + ${h.form(h.url('login_home', came_from=request.path_qs))}
diff -r a0a9ae753cc4 -r b537babcf966 kallithea/templates/changeset/changeset_file_comment.html --- a/kallithea/templates/changeset/changeset_file_comment.html Fri Sep 18 13:57:49 2015 +0200 +++ b/kallithea/templates/changeset/changeset_file_comment.html Fri Sep 18 13:57:49 2015 +0200 @@ -87,7 +87,7 @@ ${h.form('')}
- ${_('You need to be logged in to comment.')} ${_('Login now')} + ${_('You need to be logged in to comment.')} ${_('Login now')}
diff -r a0a9ae753cc4 -r b537babcf966 kallithea/templates/login.html --- a/kallithea/templates/login.html Fri Sep 18 13:57:49 2015 +0200 +++ b/kallithea/templates/login.html Fri Sep 18 13:57:49 2015 +0200 @@ -16,7 +16,7 @@ %endif
- ${h.form(h.url.current(came_from=c.came_from, **request.GET))} + ${h.form(url('login_home', came_from=c.came_from))}
diff -r a0a9ae753cc4 -r b537babcf966 kallithea/tests/functional/test_login.py --- a/kallithea/tests/functional/test_login.py Fri Sep 18 13:57:49 2015 +0200 +++ b/kallithea/tests/functional/test_login.py Fri Sep 18 13:57:49 2015 +0200 @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import re import time +import urlparse import mock @@ -132,9 +133,9 @@ # verify that get arguments are correctly passed along login redirection @parameterized.expand([ - ({'foo':'one', 'bar':'two'}, ('foo=one', 'bar=two')), + ({'foo':'one', 'bar':'two'}, (('foo', 'one'), ('bar', 'two'))), ({'blue': u'blå'.encode('utf-8'), 'green':u'grøn'}, - ('blue=bl%C3%A5', 'green=gr%C3%B8n')), + (('blue', u'blå'.encode('utf-8')), ('green', u'grøn'.encode('utf-8')))), ]) def test_redirection_to_login_form_preserves_get_args(self, args, args_encoded): with fixture.anon_access(False): @@ -142,30 +143,31 @@ repo_name=HG_REPO, **args)) self.assertEqual(response.status, '302 Found') + came_from = urlparse.parse_qs(urlparse.urlparse(response.location).query)['came_from'][0] + came_from_qs = urlparse.parse_qsl(urlparse.urlparse(came_from).query) for encoded in args_encoded: - self.assertIn(encoded, response.location) + self.assertIn(encoded, came_from_qs) @parameterized.expand([ ({'foo':'one', 'bar':'two'}, ('foo=one', 'bar=two')), - ({'blue': u'blå'.encode('utf-8'), 'green':u'grøn'}, + ({'blue': u'blå', 'green':u'grøn'}, ('blue=bl%C3%A5', 'green=gr%C3%B8n')), ]) def test_login_form_preserves_get_args(self, args, args_encoded): response = self.app.get(url(controller='login', action='index', - came_from = '/_admin/users', - **args)) + came_from=url('/_admin/users', **args))) + came_from = urlparse.parse_qs(urlparse.urlparse(response.form.action).query)['came_from'][0] for encoded in args_encoded: - self.assertIn(encoded, response.form.action) + self.assertIn(encoded, came_from) @parameterized.expand([ ({'foo':'one', 'bar':'two'}, ('foo=one', 'bar=two')), - ({'blue': u'blå'.encode('utf-8'), 'green':u'grøn'}, + ({'blue': u'blå', 'green':u'grøn'}, ('blue=bl%C3%A5', 'green=gr%C3%B8n')), ]) def test_redirection_after_successful_login_preserves_get_args(self, args, args_encoded): response = self.app.post(url(controller='login', action='index', - came_from = '/_admin/users', - **args), + came_from = url('/_admin/users', **args)), {'username': TEST_USER_ADMIN_LOGIN, 'password': TEST_USER_ADMIN_PASS}) self.assertEqual(response.status, '302 Found') @@ -174,19 +176,19 @@ @parameterized.expand([ ({'foo':'one', 'bar':'two'}, ('foo=one', 'bar=two')), - ({'blue': u'blå'.encode('utf-8'), 'green':u'grøn'}, + ({'blue': u'blå', 'green':u'grøn'}, ('blue=bl%C3%A5', 'green=gr%C3%B8n')), ]) def test_login_form_after_incorrect_login_preserves_get_args(self, args, args_encoded): response = self.app.post(url(controller='login', action='index', - came_from = '/_admin/users', - **args), + came_from=url('/_admin/users', **args)), {'username': 'error', 'password': 'test12'}) response.mustcontain('Invalid username or password') + came_from = urlparse.parse_qs(urlparse.urlparse(response.form.action).query)['came_from'][0] for encoded in args_encoded: - self.assertIn(encoded, response.form.action) + self.assertIn(encoded, came_from) #========================================================================== # REGISTRATIONS