changeset 5166:4c5c59b96adc

login: preserve GET arguments throughout login redirection (issue #104) When redirecting a user to the login page and while handling this login and redirecting to the original page, the GET arguments passed to the original URL are lost through the login redirection process. For example, when creating a pull request for a specific revision from the repository changelog, there are rev_start and rev_end arguments passed in the URL. Through the login redirection, they are lost. Fix the issue by passing along the GET arguments to the login page, in the login form action, and when redirecting back to the original page. Tests are added to cover these cases, including tests with unicode GET arguments (in URL encoding).
author Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
date Tue, 19 May 2015 21:50:35 +0200
parents 6ec5fd198084
children 8b35ec087464
files kallithea/controllers/login.py kallithea/lib/auth.py kallithea/templates/login.html kallithea/tests/functional/test_login.py
diffstat 4 files changed, 75 insertions(+), 7 deletions(-) [+]
line wrap: on
line diff
--- a/kallithea/controllers/login.py	Sun May 10 21:51:44 2015 +0200
+++ b/kallithea/controllers/login.py	Tue May 19 21:50:35 2015 +0200
@@ -42,6 +42,7 @@
 from kallithea.lib.auth_modules import importplugin
 from kallithea.lib.base import BaseController, render
 from kallithea.lib.exceptions import UserCreationError
+from kallithea.lib.utils2 import safe_str
 from kallithea.model.db import User, Setting
 from kallithea.model.forms import LoginForm, RegisterForm, PasswordResetForm
 from kallithea.model.user import UserModel
@@ -102,9 +103,14 @@
             came_from = url('home')
         return came_from
 
+    def _redirect_to_origin(self, origin, headers=None):
+        '''redirect to the original page, preserving any get arguments given'''
+        request.GET.pop('came_from', None)
+        raise HTTPFound(location=url(origin, **request.GET), headers=headers)
+
     def index(self):
         _default_came_from = url('home')
-        came_from = self._validate_came_from(request.GET.get('came_from'))
+        came_from = self._validate_came_from(safe_str(request.GET.get('came_from', '')))
         c.came_from = came_from or _default_came_from
 
         not_default = self.authuser.username != User.DEFAULT_USER
@@ -112,7 +118,7 @@
 
         # redirect if already logged in
         if self.authuser.is_authenticated and not_default and ip_allowed:
-            raise HTTPFound(location=c.came_from)
+            return self._redirect_to_origin(c.came_from)
 
         if request.POST:
             # import Login Form validator class
@@ -124,7 +130,8 @@
                 headers = self._store_user_in_session(
                                         username=c.form_result['username'],
                                         remember=c.form_result['remember'])
-                raise HTTPFound(location=c.came_from, headers=headers)
+                return self._redirect_to_origin(c.came_from, headers)
+
             except formencode.Invalid, errors:
                 defaults = errors.value
                 # remove password from filling in form again
@@ -157,7 +164,8 @@
 
             if auth_info:
                 headers = self._store_user_in_session(auth_info.get('username'))
-                raise HTTPFound(location=c.came_from, headers=headers)
+                return self._redirect_to_origin(c.came_from, headers)
+
         return render('/login.html')
 
     @HasPermissionAnyDecorator('hg.admin', 'hg.register.auto_activate',
--- a/kallithea/lib/auth.py	Sun May 10 21:51:44 2015 +0200
+++ b/kallithea/lib/auth.py	Tue May 19 21:50:35 2015 +0200
@@ -711,7 +711,7 @@
     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))
+    return redirect(url('login_home', came_from=p, **request.GET))
 
 class LoginRequired(object):
     """
--- a/kallithea/templates/login.html	Sun May 10 21:51:44 2015 +0200
+++ b/kallithea/templates/login.html	Tue May 19 21:50:35 2015 +0200
@@ -16,7 +16,7 @@
         %endif
     </div>
     <div class="panel-body inner">
-        ${h.form(h.url.current(came_from=c.came_from))}
+        ${h.form(h.url.current(**request.GET))}
         <div class="form">
             <i class="icon-lock"></i>
             <!-- fields -->
--- a/kallithea/tests/functional/test_login.py	Sun May 10 21:51:44 2015 +0200
+++ b/kallithea/tests/functional/test_login.py	Tue May 19 21:50:35 2015 +0200
@@ -66,7 +66,7 @@
           ('mailto:test@example.com',),
           ('file:///etc/passwd',),
           ('ftp://some.ftp.server',),
-          ('http://other.domain',),
+          ('http://other.domain/bl%C3%A5b%C3%A6rgr%C3%B8d',),
     ])
     def test_login_bad_came_froms(self, url_came_from):
         response = self.app.post(url(controller='login', action='index',
@@ -96,6 +96,66 @@
         response.mustcontain('invalid user name')
         response.mustcontain('invalid password')
 
+    # verify that get arguments are correctly passed along login redirection
+
+    @parameterized.expand([
+        ({'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')),
+    ])
+    def test_redirection_to_login_form_preserves_get_args(self, args, args_encoded):
+        with fixture.anon_access(False):
+            response = self.app.get(url(controller='summary', action='index',
+                                        repo_name=HG_REPO,
+                                        **args))
+            self.assertEqual(response.status, '302 Found')
+            for encoded in args_encoded:
+                self.assertIn(encoded, response.location)
+
+    @parameterized.expand([
+        ({'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')),
+    ])
+    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))
+        for encoded in args_encoded:
+            self.assertIn(encoded, response.form.action)
+
+    @parameterized.expand([
+        ({'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')),
+    ])
+    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),
+                                 {'username': 'test_admin',
+                                  'password': 'test12'})
+        self.assertEqual(response.status, '302 Found')
+        for encoded in args_encoded:
+            self.assertIn(encoded, response.location)
+
+    @parameterized.expand([
+        ({'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')),
+    ])
+    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),
+                                 {'username': 'error',
+                                  'password': 'test12'})
+
+        response.mustcontain('invalid user name')
+        response.mustcontain('invalid password')
+        for encoded in args_encoded:
+            self.assertIn(encoded, response.form.action)
+
     #==========================================================================
     # REGISTRATIONS
     #==========================================================================