From 67ff3b9ce4bcaceeee90f2aa4ae900d9220cac8b Mon Sep 17 00:00:00 2001 From: RamiC Date: Wed, 28 Oct 2015 21:38:22 +0200 Subject: [PATCH 1/9] Convert timeout argument to int values when applicable --- CONTRIBUTORS.txt | 2 ++ pyramid/session.py | 2 +- pyramid/tests/test_session.py | 16 ++++++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 2ef07af757..4edf1b4e93 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -254,3 +254,5 @@ Contributors - Jesse Dhillon, 2015/10/07 - Amos Latteier, 2015/10/22 + +- Rami Chousein, 2015/10/28 diff --git a/pyramid/session.py b/pyramid/session.py index c4cfc19491..525636688f 100644 --- a/pyramid/session.py +++ b/pyramid/session.py @@ -244,7 +244,7 @@ class CookieSession(dict): _cookie_secure = secure _cookie_httponly = httponly _cookie_on_exception = set_on_exception - _timeout = timeout + _timeout = timeout if timeout is None else int(timeout) _reissue_time = reissue_time # dirty flag diff --git a/pyramid/tests/test_session.py b/pyramid/tests/test_session.py index eac6593d97..c3a1f30552 100644 --- a/pyramid/tests/test_session.py +++ b/pyramid/tests/test_session.py @@ -62,6 +62,22 @@ def test_timeout_never(self): session = self._makeOne(request, timeout=None) self.assertEqual(dict(session), {'state': 1}) + def test_timeout_str(self): + import time + request = testing.DummyRequest() + cookieval = self._serialize((time.time() - 5, 0, {'state': 1})) + request.cookies['session'] = cookieval + session = self._makeOne(request, timeout='1') + self.assertEqual(dict(session), {}) + + def test_timeout_invalid(self): + import time + request = testing.DummyRequest() + cookieval = self._serialize((time.time() - 5, 0, {'state': 1})) + request.cookies['session'] = cookieval + self.assertRaises(ValueError, self._makeOne, request, timeout='error') + + def test_changed(self): request = testing.DummyRequest() session = self._makeOne(request) From a43abd25858d3e60f525c12da5a271e30a99c4fb Mon Sep 17 00:00:00 2001 From: RamiC Date: Thu, 29 Oct 2015 19:32:58 +0200 Subject: [PATCH 2/9] Convert reissue_time argument to int values when applicable --- pyramid/session.py | 3 ++- pyramid/tests/test_session.py | 27 ++++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/pyramid/session.py b/pyramid/session.py index 525636688f..56140b4c78 100644 --- a/pyramid/session.py +++ b/pyramid/session.py @@ -245,7 +245,7 @@ class CookieSession(dict): _cookie_httponly = httponly _cookie_on_exception = set_on_exception _timeout = timeout if timeout is None else int(timeout) - _reissue_time = reissue_time + _reissue_time = reissue_time if reissue_time is None else int(reissue_time) # dirty flag _dirty = False @@ -390,6 +390,7 @@ def _set_cookie(self, response): def UnencryptedCookieSessionFactoryConfig( secret, timeout=1200, + reissue_time=0, cookie_name='session', cookie_max_age=None, cookie_path='/', diff --git a/pyramid/tests/test_session.py b/pyramid/tests/test_session.py index c3a1f30552..147ce7fa15 100644 --- a/pyramid/tests/test_session.py +++ b/pyramid/tests/test_session.py @@ -77,7 +77,6 @@ def test_timeout_invalid(self): request.cookies['session'] = cookieval self.assertRaises(ValueError, self._makeOne, request, timeout='error') - def test_changed(self): request = testing.DummyRequest() session = self._makeOne(request) @@ -313,6 +312,19 @@ def test_reissue_never(self): self.assertEqual(session['state'], 1) self.assertFalse(session._dirty) + def test_reissue_str_triggered(self): + import time + request = testing.DummyRequest() + cookieval = self._serialize((time.time() - 2, 0, {'state': 1})) + request.cookies['session'] = cookieval + session = self._makeOne(request, reissue_time='0') + self.assertEqual(session['state'], 1) + self.assertTrue(session._dirty) + + def test_reissue_invalid(self): + request = testing.DummyRequest() + self.assertRaises(ValueError, self._makeOne, request, reissue_time='invalid value') + class TestSignedCookieSession(SharedCookieSessionTests, unittest.TestCase): def _makeOne(self, request, **kw): from pyramid.session import SignedCookieSessionFactory @@ -347,6 +359,19 @@ def test_reissue_never(self): self.assertEqual(session['state'], 1) self.assertFalse(session._dirty) + def test_reissue_str_triggered(self): + import time + request = testing.DummyRequest() + cookieval = self._serialize((time.time() - 2, 0, {'state': 1})) + request.cookies['session'] = cookieval + session = self._makeOne(request, reissue_time='0') + self.assertEqual(session['state'], 1) + self.assertTrue(session._dirty) + + def test_reissue_invalid(self): + request = testing.DummyRequest() + self.assertRaises(ValueError, self._makeOne, request, reissue_time='invalid value') + def test_custom_salt(self): import time request = testing.DummyRequest() From 7ca2b205cd2079f4e5c9b03aec82ac75cfa8fe88 Mon Sep 17 00:00:00 2001 From: RamiC Date: Thu, 29 Oct 2015 19:38:45 +0200 Subject: [PATCH 3/9] Remove unnecessary code --- pyramid/tests/test_session.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pyramid/tests/test_session.py b/pyramid/tests/test_session.py index 147ce7fa15..68741b265e 100644 --- a/pyramid/tests/test_session.py +++ b/pyramid/tests/test_session.py @@ -71,11 +71,8 @@ def test_timeout_str(self): self.assertEqual(dict(session), {}) def test_timeout_invalid(self): - import time request = testing.DummyRequest() - cookieval = self._serialize((time.time() - 5, 0, {'state': 1})) - request.cookies['session'] = cookieval - self.assertRaises(ValueError, self._makeOne, request, timeout='error') + self.assertRaises(ValueError, self._makeOne, request, timeout='Invalid value') def test_changed(self): request = testing.DummyRequest() From b6f0be8bba239fbc1a3104c530ec996d8a5ee62e Mon Sep 17 00:00:00 2001 From: RamiC Date: Thu, 29 Oct 2015 19:46:51 +0200 Subject: [PATCH 4/9] Remove a leaked line from a local test --- pyramid/session.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pyramid/session.py b/pyramid/session.py index 56140b4c78..18aa41a0f0 100644 --- a/pyramid/session.py +++ b/pyramid/session.py @@ -390,7 +390,6 @@ def _set_cookie(self, response): def UnencryptedCookieSessionFactoryConfig( secret, timeout=1200, - reissue_time=0, cookie_name='session', cookie_max_age=None, cookie_path='/', From fa7886f89b99b52bcbed6b7d3577e7f1caace3df Mon Sep 17 00:00:00 2001 From: RamiC Date: Sat, 31 Oct 2015 09:56:28 +0200 Subject: [PATCH 5/9] Convert max_age argument to int when applicable --- pyramid/session.py | 2 +- pyramid/tests/test_session.py | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/pyramid/session.py b/pyramid/session.py index 18aa41a0f0..fa85fe69cb 100644 --- a/pyramid/session.py +++ b/pyramid/session.py @@ -238,7 +238,7 @@ class CookieSession(dict): # configuration parameters _cookie_name = cookie_name - _cookie_max_age = max_age + _cookie_max_age = max_age if max_age is None else int(max_age) _cookie_path = path _cookie_domain = domain _cookie_secure = secure diff --git a/pyramid/tests/test_session.py b/pyramid/tests/test_session.py index 68741b265e..82e4fb0012 100644 --- a/pyramid/tests/test_session.py +++ b/pyramid/tests/test_session.py @@ -322,6 +322,10 @@ def test_reissue_invalid(self): request = testing.DummyRequest() self.assertRaises(ValueError, self._makeOne, request, reissue_time='invalid value') + def test_cookie_max_age_invalid(self): + request = testing.DummyRequest() + self.assertRaises(ValueError, self._makeOne, request, max_age='invalid value') + class TestSignedCookieSession(SharedCookieSessionTests, unittest.TestCase): def _makeOne(self, request, **kw): from pyramid.session import SignedCookieSessionFactory @@ -369,6 +373,10 @@ def test_reissue_invalid(self): request = testing.DummyRequest() self.assertRaises(ValueError, self._makeOne, request, reissue_time='invalid value') + def test_cookie_max_age_invalid(self): + request = testing.DummyRequest() + self.assertRaises(ValueError, self._makeOne, request, max_age='invalid value') + def test_custom_salt(self): import time request = testing.DummyRequest() From e2519d64b3dd1f53555a184eb38b5138ea9bc2f6 Mon Sep 17 00:00:00 2001 From: RamiC Date: Tue, 3 Nov 2015 10:20:43 +0200 Subject: [PATCH 6/9] Convert AuthTktCookieHelper time related parameters to int when applicable --- pyramid/authentication.py | 9 +++--- pyramid/tests/test_authentication.py | 46 ++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/pyramid/authentication.py b/pyramid/authentication.py index 0924b59013..9bf1de62e6 100644 --- a/pyramid/authentication.py +++ b/pyramid/authentication.py @@ -855,9 +855,9 @@ def __init__(self, secret, cookie_name='auth_tkt', secure=False, self.cookie_name = cookie_name self.secure = secure self.include_ip = include_ip - self.timeout = timeout - self.reissue_time = reissue_time - self.max_age = max_age + self.timeout = timeout if timeout is None else int(timeout) + self.reissue_time = reissue_time if reissue_time is None else int(reissue_time) + self.max_age = max_age if max_age is None else int(max_age) self.wild_domain = wild_domain self.parent_domain = parent_domain self.domain = domain @@ -977,8 +977,7 @@ def remember(self, request, userid, max_age=None, tokens=()): Tokens are available in the returned identity when an auth_tkt is found in the request and unpacked. Default: ``()``. """ - if max_age is None: - max_age = self.max_age + max_age = self.max_age if max_age is None else int(max_age) environ = request.environ diff --git a/pyramid/tests/test_authentication.py b/pyramid/tests/test_authentication.py index c7fc1c2110..595a0eac85 100644 --- a/pyramid/tests/test_authentication.py +++ b/pyramid/tests/test_authentication.py @@ -600,6 +600,15 @@ def _parseCookie(self, cookie): cookies.load(cookie) return cookies.get('auth_tkt') + def test_init_cookie_str_reissue_invalid(self): + self.assertRaises(ValueError, self._makeOne, 'secret', reissue_time='invalid value') + + def test_init_cookie_str_timeout_invalid(self): + self.assertRaises(ValueError, self._makeOne, 'secret', timeout='invalid value') + + def test_init_cookie_str_max_age_invalid(self): + self.assertRaises(ValueError, self._makeOne, 'secret', max_age='invalid value') + def test_identify_nocookie(self): helper = self._makeOne('secret') request = self._makeRequest() @@ -758,6 +767,12 @@ def test_identify_cookie_timed_out(self): result = helper.identify(request) self.assertEqual(result, None) + def test_identify_cookie_str_timeout(self): + helper = self._makeOne('secret', timeout='1') + request = self._makeRequest({'HTTP_COOKIE':'auth_tkt=bogus'}) + result = helper.identify(request) + self.assertEqual(result, None) + def test_identify_cookie_reissue(self): import time helper = self._makeOne('secret', timeout=10, reissue_time=0) @@ -774,6 +789,22 @@ def test_identify_cookie_reissue(self): self.assertEqual(len(response.headerlist), 3) self.assertEqual(response.headerlist[0][0], 'Set-Cookie') + def test_identify_cookie_str_reissue(self): + import time + helper = self._makeOne('secret', timeout=10, reissue_time='0') + now = time.time() + helper.auth_tkt.timestamp = now + helper.now = now + 1 + helper.auth_tkt.tokens = (text_('a'), ) + request = self._makeRequest('bogus') + result = helper.identify(request) + self.assertTrue(result) + self.assertEqual(len(request.callbacks), 1) + response = DummyResponse() + request.callbacks[0](request, response) + self.assertEqual(len(response.headerlist), 3) + self.assertEqual(response.headerlist[0][0], 'Set-Cookie') + def test_identify_cookie_reissue_already_reissued_this_request(self): import time helper = self._makeOne('secret', timeout=10, reissue_time=0) @@ -1058,6 +1089,16 @@ def test_remember_insane_userid(self): self.assertTrue('userid' in value.value) def test_remember_max_age(self): + helper = self._makeOne('secret') + request = self._makeRequest() + result = helper.remember(request, 'userid', max_age=500) + values = self._parseHeaders(result) + self.assertEqual(len(result), 3) + + self.assertEqual(values[0]['max-age'], '500') + self.assertTrue(values[0]['expires']) + + def test_remember_str_max_age(self): helper = self._makeOne('secret') request = self._makeRequest() result = helper.remember(request, 'userid', max_age='500') @@ -1067,6 +1108,11 @@ def test_remember_max_age(self): self.assertEqual(values[0]['max-age'], '500') self.assertTrue(values[0]['expires']) + def test_remember_str_max_age_invalid(self): + helper = self._makeOne('secret') + request = self._makeRequest() + self.assertRaises(ValueError, helper.remember, request, 'userid', max_age='invalid value') + def test_remember_tokens(self): helper = self._makeOne('secret') request = self._makeRequest() From 81cd59826279dab5959a3e198eee75ab09910872 Mon Sep 17 00:00:00 2001 From: RamiC Date: Tue, 3 Nov 2015 10:22:07 +0200 Subject: [PATCH 7/9] Include CHANGES entry for converting time related parameters of AuthTktCookieHelper and CookieSession --- CHANGES.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index 8b63cf8471..aefb166dc7 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -232,6 +232,11 @@ Bug Fixes shell a little more straightfoward. See https://github.com/Pylons/pyramid/pull/1883 +- Fix an issue when user passes unparsed strings to ``pyramid.session.CookieSession`` + and ``pyramid.authentication.AuthTktCookieHelper`` for time related parameters + ``timeout``, ``reissue_time``, ``max_age`` that expect an integer value. + See https://github.com/Pylons/pyramid/pull/2050 + Deprecations ------------ From b084d76c8941609caca5b8fcd451ce516c1b3611 Mon Sep 17 00:00:00 2001 From: Bert JW Regeer Date: Thu, 12 Nov 2015 21:50:15 -0700 Subject: [PATCH 8/9] Simplify tests --- pyramid/tests/test_authentication.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/pyramid/tests/test_authentication.py b/pyramid/tests/test_authentication.py index 1a367fd156..4a6525af29 100644 --- a/pyramid/tests/test_authentication.py +++ b/pyramid/tests/test_authentication.py @@ -761,17 +761,13 @@ def test_identify_bad_cookie(self): result = helper.identify(request) self.assertEqual(result, None) - def test_identify_cookie_timed_out(self): + def test_identify_cookie_timeout(self): helper = self._makeOne('secret', timeout=1) - request = self._makeRequest({'HTTP_COOKIE':'auth_tkt=bogus'}) - result = helper.identify(request) - self.assertEqual(result, None) + self.assertEqual(helper.timeout, 1) def test_identify_cookie_str_timeout(self): helper = self._makeOne('secret', timeout='1') - request = self._makeRequest({'HTTP_COOKIE':'auth_tkt=bogus'}) - result = helper.identify(request) - self.assertEqual(result, None) + self.assertEqual(helper.timeout, 1) def test_identify_cookie_reissue(self): import time From 203cf3accd0bec0cc08eab8e736f26cd0e711d8b Mon Sep 17 00:00:00 2001 From: Bert JW Regeer Date: Thu, 12 Nov 2015 22:15:51 -0700 Subject: [PATCH 9/9] Add AuthTkt test to test ticket timeouts Previous tests did not actually test the code, other than by chance, which meant future changes could regress, which is bad. --- pyramid/tests/test_authentication.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pyramid/tests/test_authentication.py b/pyramid/tests/test_authentication.py index 4a6525af29..0a22e59652 100644 --- a/pyramid/tests/test_authentication.py +++ b/pyramid/tests/test_authentication.py @@ -769,6 +769,17 @@ def test_identify_cookie_str_timeout(self): helper = self._makeOne('secret', timeout='1') self.assertEqual(helper.timeout, 1) + def test_identify_cookie_timeout_aged(self): + import time + helper = self._makeOne('secret', timeout=10) + now = time.time() + helper.auth_tkt.timestamp = now - 1 + helper.now = now + 10 + helper.auth_tkt.tokens = (text_('a'), ) + request = self._makeRequest('bogus') + result = helper.identify(request) + self.assertFalse(result) + def test_identify_cookie_reissue(self): import time helper = self._makeOne('secret', timeout=10, reissue_time=0)