kallithea Changeset - b537babcf966

Changeset - b537babcf966

Parent rev.

Child rev.

[Not reviewed]

stable

0 6 0

Søren Løvborg - 10 years ago 2015-09-18 13:57:49
sorenl@unity3d.com

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.)

6 files changed with 24 insertions and 21 deletions:

kallithea/controllers/login.py

kallithea/lib/auth.py

kallithea/templates/base/base.html

kallithea/templates/changeset/changeset_file_comment.html

kallithea/templates/login.html

kallithea/tests/functional/test_login.py

0 comments (0 inline, 0 general)

kallithea/controllers/login.py

➞

Show inline comments

@@ @@ -41,54 +41,54 @@ from kallithea.lib.auth import AuthUser, @@
 from kallithea.lib.base import BaseController, log_in_user, 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, PasswordResetRequestForm, PasswordResetConfirmationForm
 from kallithea.model.user import UserModel
 from kallithea.model.meta import Session
 log = logging.getLogger(__name__)
 class LoginController(BaseController):
     def __before__(self):
         super(LoginController, self).__before__()
     def _validate_came_from(self, came_from):
         """Return True if came_from is valid and can and should be used"""
         url = urlparse.urlsplit(came_from)
         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')
         not_default = self.authuser.username != User.DEFAULT_USER
         ip_allowed = AuthUser.check_ip_allowed(self.authuser, self.ip_addr)
         # redirect if already logged in
         if self.authuser.is_authenticated and not_default and ip_allowed:
             raise HTTPFound(location=came_from)
         if request.POST:
             # import Login Form validator class
             login_form = LoginForm()
             try:
                 c.form_result = login_form.to_python(dict(request.POST))
                 # form checks for username/password, now we're authenticated
                 username = c.form_result['username']
                 user = User.get_by_username(username, case_insensitive=True)
             except formencode.Invalid as errors:
                 defaults = errors.value
                 # remove password from filling in form again
                 del defaults['password']
                 return htmlfill.render(
                     render('/login.html'),

kallithea/lib/auth.py

➞

Show inline comments

@@ @@ -691,53 +691,54 @@ def set_available_permissions(config): @@
     """
     This function will propagate pylons globals with all available defined
     permission given in db. We don't want to check each time from db for new
     permissions since adding a new permission also requires application restart
     ie. to decorate new views with the newly created permission
     :param config: current pylons config instance
     """
     log.info('getting information about all available permissions')
     try:
         sa = meta.Session
         all_perms = sa.query(Permission).all()
         config['available_permissions'] = [x.permission_name for x in all_perms]
     finally:
         meta.Session.remove()
 #==============================================================================
 # CHECK DECORATORS
 #==============================================================================
 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):
     """
     Must be logged in to execute this function else
     redirect to login page
     :param api_access: if enabled this checks only for valid auth token
         and grants access based on valid token
     """
     def __init__(self, api_access=False):
         self.api_access = api_access
     def __call__(self, func):
         return decorator(self.__wrapper, func)
     def __wrapper(self, func, *fargs, **fkwargs):
         controller = fargs[0]
         user = controller.authuser
         loc = "%s:%s" % (controller.__class__.__name__, func.__name__)
         log.debug('Checking access for user %s @ %s', user, loc)
         if not AuthUser.check_ip_allowed(user, controller.ip_addr):
             return redirect_to_login(_('IP %s not allowed') % controller.ip_addr)

kallithea/templates/base/base.html

➞

Show inline comments

@@ @@ -273,49 +273,49 @@ @@
         %endif
       </a>
     </li>
     ## USER MENU
     <li>
       <a class="menu_link childs" id="quick_login_link">
           <span class="icon">
             ${h.gravatar(c.authuser.email, size=20)}
           </span>
           %if c.authuser.username != 'default':
             <span class="menu_link_user">${c.authuser.username}</span>
             %if c.unread_notifications != 0:
               <span class="menu_link_notifications">${c.unread_notifications}</span>
             %endif
           %else:
               <span>${_('Not Logged In')}</span>
           %endif
       </a>
       <div class="user-menu">
         <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">
                         <div class="label">
                             <label for="username">${_('Username')}:</label>
                         </div>
                         <div class="input">
                             ${h.text('username',class_='focus')}
                         </div>
                     </div>
                     <div class="field">
                         <div class="label">
                             <label for="password">${_('Password')}:</label>
                         </div>
                         <div class="input">
                             ${h.password('password',class_='focus')}
                         </div>
                     </div>
                     <div class="buttons">
                         <div class="password_forgoten">${h.link_to(_('Forgot password ?'),h.url('reset_password'))}</div>
                         <div class="register">
                         %if h.HasPermissionAny('hg.admin', 'hg.register.auto_activate', 'hg.register.manual_activate')():

kallithea/templates/changeset/changeset_file_comment.html

➞

Show inline comments

@@ @@ -66,49 +66,49 @@ @@
         </div>
         <div class="mentions-container" id="mentions_container_{1}"></div>
         <textarea id="text_{1}" name="text" class="comment-block-ta yui-ac-input"></textarea>
       </div>
       <div id="preview-container_{1}" class="clearfix" style="display:none">
         <div class="comment-help">
             ${_('Comment preview')}
         </div>
         <div id="preview-box_{1}" class="preview-box"></div>
       </div>
       <div class="comment-button">
         <div class="submitting-overlay">${_('Submitting ...')}</div>
         <input type="hidden" name="f_path" value="{0}">
         <input type="hidden" name="line" value="{1}">
         ${h.submit('save', _('Comment'), class_='btn btn-small save-inline-form')}
         ${h.reset('hide-inline-form', _('Cancel'), class_='btn btn-small hide-inline-form')}
         <div id="preview-btn_{1}" class="preview-btn btn btn-small">${_('Preview')}</div>
         <div id="edit-btn_{1}" class="edit-btn btn btn-small" style="display:none">${_('Edit')}</div>
       </div>
     ${h.end_form()}
   %else:
       ${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">
       ${h.reset('hide-inline-form', _('Hide'), class_='btn btn-small hide-inline-form')}
       </div>
       ${h.end_form()}
   %endif
   </div>
 </div>
 </%def>
 ## show comment count as "x comments (y inline, z general)"
 <%def name="comment_count(inline_cnt, general_cnt)">
     ${'%s (%s, %s)' % (
         ungettext("%d comment", "%d comments", inline_cnt + general_cnt) % (inline_cnt + general_cnt),
         ungettext("%d inline", "%d inline", inline_cnt) % inline_cnt,
         ungettext("%d general", "%d general", general_cnt) % general_cnt
     )}
     <span class="firstlink"></span>
 </%def>
 ## generate inline comments and the main ones

kallithea/templates/login.html

➞

Show inline comments

 ## -*- coding: utf-8 -*-
 <%inherit file="base/root.html"/>
 <%block name="title">
     ${_('Log In')}
 </%block>
 <div id="login" class="panel panel-default">
     <%include file="/base/flash_msg.html"/>
     <!-- login -->
     <div class="panel-heading title withlogo">
         %if c.site_name:
             <h5>${_('Log In to %s') % c.site_name}</h5>
         %else:
             <h5>${_('Log In')}</h5>
         %endif
     </div>
     <div class="panel-body inner">
-        ${h.form(h.url.current(came_from=c.came_from, **request.GET))}
+        ${h.form(url('login_home', came_from=c.came_from))}
         <div class="form">
             <i class="icon-lock"></i>
             <!-- fields -->
             <div class="form-horizontal">
                 <div class="form-group">
                     <label class="control-label col-sm-5" for="username">${_('Username')}:</label>
                     <div class="input col-sm-7">
                         ${h.text('username',class_='form-control focus large')}
                     </div>
                 </div>
                 <div class="form-group">
                     <label class="control-label col-sm-5" for="password">${_('Password')}:</label>
                     <div class="input col-sm-7">
                         ${h.password('password',class_='form-control focus large')}
                     </div>
                 </div>
                 <div class="form-group">
                     <div class="col-sm-offset-5 col-sm-7">
                         <div class="checkbox">
                             <label for="remember">
                                 <input type="checkbox" id="remember" name="remember"/>

kallithea/tests/functional/test_login.py

➞

Show inline comments

 # -*- coding: utf-8 -*-
 import re
 import time
 import urlparse
 import mock
 from kallithea.tests import *
 from kallithea.tests.fixture import Fixture
 from kallithea.lib.utils2 import generate_api_key
 from kallithea.lib.auth import check_password
 from kallithea.lib import helpers as h
 from kallithea.model.api_key import ApiKeyModel
 from kallithea.model import validators
 from kallithea.model.db import User, Notification
 from kallithea.model.meta import Session
 from kallithea.model.user import UserModel
 fixture = Fixture()
 class TestLoginController(TestController):
     def setUp(self):
         self.remove_all_notifications()
         self.assertEqual(Notification.query().all(), [])
     def test_index(self):
         response = self.app.get(url(controller='login', action='index'))
@@ @@ -111,103 +112,104 @@ class TestLoginController(TestController @@
         response = self.app.post(url(controller='login', action='index',
                                      came_from=url_came_from),
                                  {'username': TEST_USER_ADMIN_LOGIN,
                                   'password': TEST_USER_ADMIN_PASS},
                                  status=400)
     def test_login_short_password(self):
         response = self.app.post(url(controller='login', action='index'),
                                  {'username': TEST_USER_ADMIN_LOGIN,
                                   'password': 'as'})
         self.assertEqual(response.status, '200 OK')
         response.mustcontain('Enter 3 characters or more')
     def test_login_wrong_username_password(self):
         response = self.app.post(url(controller='login', action='index'),
                                  {'username': 'error',
                                   'password': 'test12'})
         response.mustcontain('Invalid username or password')
     # 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):
             response = self.app.get(url(controller='summary', action='index',
                                         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')
         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': 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
     #==========================================================================
     def test_register(self):
         response = self.app.get(url(controller='login', action='register'))
         response.mustcontain('Sign Up')
     def test_register_err_same_username(self):
         uname = TEST_USER_ADMIN_LOGIN
         response = self.app.post(url(controller='login', action='register'),
                                             {'username': uname,
                                              'password': 'test12',
                                              'password_confirmation': 'test12',
                                              'email': 'goodmail@example.com',
                                              'firstname': 'test',
                                              'lastname': 'test'})
         msg = validators.ValidUsername()._messages['username_exists']
         msg = h.html_escape(msg % {'username': uname})
         response.mustcontain(msg)
     def test_register_err_same_email(self):
         response = self.app.post(url(controller='login', action='register'),

0 comments (0 inline, 0 general)