Re: [PATCH 1 of 1 v2] login: preserve GET arguments throughout login redirection (issue #104)

2015-06-07 Thread Thomas De Schampheleire
On Sat, Jun 6, 2015 at 10:28 PM, Mads Kiilerich m...@kiilerich.com wrote:
 On 06/04/2015 07:40 PM, Thomas De Schampheleire wrote:

 On June 4, 2015 6:55:30 PM CEST, Mads Kiilerich m...@kiilerich.com
 wrote:

 On 05/31/2015 01:41 PM, Thomas De Schampheleire wrote:

 # HG changeset patch
 # User Thomas De Schampheleire thomas.de.schamphele...@gmail.com
 # Date 1432065035 -7200
 #  Tue May 19 21:50:35 2015 +0200
 # Node ID 20bf71e88042ca1f519c4ee711fa436564360755
 # Parent  579110ca5178f13254e7e4c7b6043767a11b92a2
 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.

 diff --git a/kallithea/controllers/login.py

 b/kallithea/controllers/login.py

 --- a/kallithea/controllers/login.py
 +++ b/kallithea/controllers/login.py
 @@ -102,6 +102,11 @@ class LoginController(BaseController):
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.encode('utf-8'),

 **request.GET), headers=headers)

 I wonder, why is it correct to hardcode utf-8 here?

 Do you mean to just call encode without argument? Or do you don't encode
 itself?

 Without it, it doesn't work, but I welcome any attempts to do it
 differently. The tests should cover everything so feel free to experiment...


 It seems like the answer is that anything in GET already is URL encoded and
 thus always ascii and it is more correct to use something like safestr. I
 applied something like that ... and some other minor tweaks.

Thanks for looking into it!
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general


Re: [PATCH 1 of 1 v2] login: preserve GET arguments throughout login redirection (issue #104)

2015-06-06 Thread Mads Kiilerich

On 06/04/2015 07:40 PM, Thomas De Schampheleire wrote:

On June 4, 2015 6:55:30 PM CEST, Mads Kiilerich m...@kiilerich.com wrote:

On 05/31/2015 01:41 PM, Thomas De Schampheleire wrote:

# HG changeset patch
# User Thomas De Schampheleire thomas.de.schamphele...@gmail.com
# Date 1432065035 -7200
#  Tue May 19 21:50:35 2015 +0200
# Node ID 20bf71e88042ca1f519c4ee711fa436564360755
# Parent  579110ca5178f13254e7e4c7b6043767a11b92a2
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.

diff --git a/kallithea/controllers/login.py

b/kallithea/controllers/login.py

--- a/kallithea/controllers/login.py
+++ b/kallithea/controllers/login.py
@@ -102,6 +102,11 @@ class LoginController(BaseController):
   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.encode('utf-8'),

**request.GET), headers=headers)

I wonder, why is it correct to hardcode utf-8 here?

Do you mean to just call encode without argument? Or do you don't encode itself?

Without it, it doesn't work, but I welcome any attempts to do it differently. 
The tests should cover everything so feel free to experiment...


It seems like the answer is that anything in GET already is URL encoded 
and thus always ascii and it is more correct to use something like 
safestr. I applied something like that ... and some other minor tweaks.


/Mads
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general


Re: [PATCH 1 of 1 v2] login: preserve GET arguments throughout login redirection (issue #104)

2015-06-04 Thread Thomas De Schampheleire
On June 4, 2015 6:55:30 PM CEST, Mads Kiilerich m...@kiilerich.com wrote:
On 05/31/2015 01:41 PM, Thomas De Schampheleire wrote:
 # HG changeset patch
 # User Thomas De Schampheleire thomas.de.schamphele...@gmail.com
 # Date 1432065035 -7200
 #  Tue May 19 21:50:35 2015 +0200
 # Node ID 20bf71e88042ca1f519c4ee711fa436564360755
 # Parent  579110ca5178f13254e7e4c7b6043767a11b92a2
 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.

 diff --git a/kallithea/controllers/login.py
b/kallithea/controllers/login.py
 --- a/kallithea/controllers/login.py
 +++ b/kallithea/controllers/login.py
 @@ -102,6 +102,11 @@ class LoginController(BaseController):
   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.encode('utf-8'),
**request.GET), headers=headers)

I wonder, why is it correct to hardcode utf-8 here?

Do you mean to just call encode without argument? Or do you don't encode itself?

Without it, it doesn't work, but I welcome any attempts to do it differently. 
The tests should cover everything so feel free to experiment...

Thanks,
Thomas

___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general


[PATCH 1 of 1 v2] login: preserve GET arguments throughout login redirection (issue #104)

2015-05-31 Thread Thomas De Schampheleire
# HG changeset patch
# User Thomas De Schampheleire thomas.de.schamphele...@gmail.com
# Date 1432065035 -7200
#  Tue May 19 21:50:35 2015 +0200
# Node ID 20bf71e88042ca1f519c4ee711fa436564360755
# Parent  579110ca5178f13254e7e4c7b6043767a11b92a2
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.

diff --git a/kallithea/controllers/login.py b/kallithea/controllers/login.py
--- a/kallithea/controllers/login.py
+++ b/kallithea/controllers/login.py
@@ -102,6 +102,11 @@ class LoginController(BaseController):
 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.encode('utf-8'), **request.GET), 
headers=headers)
+
 def index(self):
 _default_came_from = url('home')
 came_from = self._validate_came_from(request.GET.get('came_from'))
@@ -112,7 +117,7 @@ class LoginController(BaseController):
 
 # 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 +129,8 @@ class LoginController(BaseController):
 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 +163,8 @@ class LoginController(BaseController):
 
 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',
diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py
--- a/kallithea/lib/auth.py
+++ b/kallithea/lib/auth.py
@@ -711,7 +711,7 @@ def redirect_to_login(message=None):
 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):
 
diff --git a/kallithea/templates/login.html b/kallithea/templates/login.html
--- a/kallithea/templates/login.html
+++ b/kallithea/templates/login.html
@@ -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 --
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
@@ -96,6 +96,66 @@ class TestLoginController(TestController
 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'.encode('utf-8')},
+ ('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:
+