[PATCH 3 of 4 prefix exploded] login: include query parameters in came_from

[Mads Kiilerich]

# HG changeset patch
# User Søren Løvborg <sorenl at unity3d.com>
# Date 1442577469 -7200
#      Fri Sep 18 13:57:49 2015 +0200
# Branch stable
# Node ID ba21aaf1e89a2bd63ac4df05dc4d0ab24e1791a0
# Parent  7a9470196cdc4377538fca0d389ad38719f250eb
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 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 --git a/kallithea/controllers/login.py b/kallithea/controllers/login.py
--- a/kallithea/controllers/login.py
+++ b/kallithea/controllers/login.py
@@ -67,7 +67,7 @@ class LoginController(BaseController):
             if not self._validate_came_from(came_from):
                 log.error('Invalid came_from (not server-relative): %r', came_from)
                 raise HTTPBadRequest()
-            c.came_from = url(came_from, **request.GET)
+            c.came_from = url(came_from)
         else:
             c.came_from = url('home')
 
diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py
--- a/kallithea/lib/auth.py
+++ b/kallithea/lib/auth.py
@@ -712,11 +712,12 @@ def set_available_permissions(config):
 
 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 --git a/kallithea/templates/base/base.html b/kallithea/templates/base/base.html
--- a/kallithea/templates/base/base.html
+++ b/kallithea/templates/base/base.html
@@ -294,7 +294,7 @@
         <div id="quick_login">
           %if c.authuser.username == 'default' or c.authuser.user_id is None:
             <h4>${_('Login to Your Account')}</h4>
-            ${h.form(h.url('login_home',came_from=h.url.current()))}
+            ${h.form(h.url('login_home', came_from=request.path_qs))}
             <div class="form">
                 <div class="fields">
                     <div class="field">
diff --git a/kallithea/templates/changeset/changeset_file_comment.html b/kallithea/templates/changeset/changeset_file_comment.html
--- a/kallithea/templates/changeset/changeset_file_comment.html
+++ b/kallithea/templates/changeset/changeset_file_comment.html
@@ -87,7 +87,7 @@
       ${h.form('')}
       <div class="clearfix">
           <div class="comment-help">
-            ${_('You need to be logged in to comment.')} <a href="${h.url('login_home',came_from=h.url.current())}">${_('Login now')}</a>
+            ${_('You need to be logged in to comment.')} <a href="${h.url('login_home', came_from=request.path_qs)}">${_('Login now')}</a>
           </div>
       </div>
       <div class="comment-button">
diff --git a/kallithea/tests/functional/test_login.py b/kallithea/tests/functional/test_login.py
--- a/kallithea/tests/functional/test_login.py
+++ b/kallithea/tests/functional/test_login.py
@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 import re
 import time
+import urlparse
 
 import mock
 
@@ -132,9 +133,9 @@ class TestLoginController(TestController
     # 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,8 +143,11 @@ class TestLoginController(TestController
                                         repo_name=HG_REPO,
                                         **args))
             self.assertEqual(response.status, '302 Found')
+            qs = urlparse.parse_qs(urlparse.urlparse(response.location).query)
+            came_from = qs['came_from'][0]
+            camefrom_qs = urlparse.parse_qsl(urlparse.urlparse(came_from).query)
             for encoded in args_encoded:
-                self.assertIn(encoded, response.location)
+                self.assertIn(encoded, camefrom_qs)
 
     @parameterized.expand([
         ({'foo':'one', 'bar':'two'}, ('foo=one', 'bar=two')),
@@ -159,13 +163,12 @@ class TestLoginController(TestController
 
     @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')