From 5ba0636b83277c24c48e81427a176541266cd30e Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Wed, 3 Feb 2010 02:51:24 +0000 Subject: Bug Fixes --------- - Ensure that ``secure`` flag for AuthTktAuthenticationPolicy constructor does what it's documented to do (merge Daniel Holth's fancy-cookies-2 branch). New Features ------------ - Add ``path`` and ``http_only`` options to AuthTktAuthenticationPolicy constructor (merge Daniel Holth's fancy-cookies-2 branch). --- CHANGES.txt | 20 ++++++++++++ docs/narr/security.rst | 2 ++ repoze/bfg/authentication.py | 46 +++++++++++++++++++++----- repoze/bfg/tests/test_authentication.py | 57 +++++++++++++++++++++++++++++++++ repoze/bfg/tests/test_zcml.py | 7 ++-- repoze/bfg/zcml.py | 10 ++++-- 6 files changed, 130 insertions(+), 12 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 52d158138..c02ca46d0 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,6 +1,23 @@ Next release ============ +Bug Fixes +--------- + +- Ensure that ``secure`` flag for AuthTktAuthenticationPolicy + constructor does what it's documented to do (merge Daniel Holth's + fancy-cookies-2 branch). + +New Features +------------ + +- Add ``path`` and ``http_only`` options to + AuthTktAuthenticationPolicy constructor (merge Daniel Holth's + fancy-cookies-2 branch). + +Backwards Incompatibilities +--------------------------- + - Remove ``view_header``, ``view_accept``, ``view_xhr``, ``view_path_info``, ``view_request_method``, ``view_request_param``, and ``view_containment`` predicate arguments from the @@ -17,6 +34,9 @@ Next release with a route using the ``route_name`` attribute of the ``view`` ZCML directive instead. +Dependencies +------------ + - Remove dependency on ``sourcecodegen`` (not depended upon by Chameleon 1.1.1+). diff --git a/docs/narr/security.rst b/docs/narr/security.rst index 67f6342df..5178a7c54 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -595,6 +595,8 @@ An example of its usage, with all attributes fully expanded: timeout="86400" reissue_time="600" max_age="31536000" + path="/" + http_only="False" /> See :ref:`authtktauthenticationpolicy_directive` for details about diff --git a/repoze/bfg/authentication.py b/repoze/bfg/authentication.py index ca8a26eec..4444bb33b 100644 --- a/repoze/bfg/authentication.py +++ b/repoze/bfg/authentication.py @@ -225,6 +225,18 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy): to set this to a value that is lower than ``timeout`` or ``reissue_time``, although it is not explicitly prevented. Optional. + + ``path`` + + Default: ``/``. The path for which the auth_tkt cookie is valid. + May be desirable if the application only serves part of a domain. + Optional. + + ``http_only`` + + Default: ``False``. Hide cookie from JavaScript by setting the + HttpOnly flag. Not honored by all browsers. + Optional. """ implements(IAuthenticationPolicy) def __init__(self, @@ -235,7 +247,10 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy): include_ip=False, timeout=None, reissue_time=None, - max_age=None): + max_age=None, + path="/", + http_only=False, + ): self.cookie = AuthTktCookieHelper( secret, cookie_name=cookie_name, @@ -244,6 +259,8 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy): timeout=timeout, reissue_time=reissue_time, max_age=max_age, + http_only=http_only, + path=path, ) self.callback = callback @@ -286,7 +303,7 @@ class AuthTktCookieHelper(object): def __init__(self, secret, cookie_name='auth_tkt', secure=False, include_ip=False, timeout=None, reissue_time=None, - max_age=None): + max_age=None, http_only=False, path="/"): self.secret = secret self.cookie_name = cookie_name self.include_ip = include_ip @@ -297,6 +314,15 @@ class AuthTktCookieHelper(object): raise ValueError('reissue_time must be lower than timeout') self.reissue_time = reissue_time self.max_age = max_age + self.http_only = http_only + self.path = path + + static_flags = [] + if self.secure: + static_flags.append('; Secure') + if self.http_only: + static_flags.append('; HttpOnly') + self.static_flags = "".join(static_flags) def _get_cookies(self, environ, value, max_age=None): if max_age is EXPIRE: @@ -314,14 +340,18 @@ class AuthTktCookieHelper(object): cur_domain = environ.get('HTTP_HOST', environ.get('SERVER_NAME')) wild_domain = '.' + cur_domain + cookies = [ - ('Set-Cookie', '%s="%s"; Path=/%s' % ( - self.cookie_name, value, max_age)), - ('Set-Cookie', '%s="%s"; Path=/; Domain=%s%s' % ( - self.cookie_name, value, cur_domain, max_age)), - ('Set-Cookie', '%s="%s"; Path=/; Domain=%s%s' % ( - self.cookie_name, value, wild_domain, max_age)) + ('Set-Cookie', '%s="%s"; Path=%s%s%s' % ( + self.cookie_name, value, self.path, max_age, self.static_flags)), + ('Set-Cookie', '%s="%s"; Path=%s; Domain=%s%s%s' % ( + self.cookie_name, value, self.path, cur_domain, max_age, + self.static_flags)), + ('Set-Cookie', '%s="%s"; Path=%s; Domain=%s%s%s' % ( + self.cookie_name, value, self.path, wild_domain, max_age, + self.static_flags)) ] + return cookies def identify(self, request): diff --git a/repoze/bfg/tests/test_authentication.py b/repoze/bfg/tests/test_authentication.py index bd4be6641..a6f34970f 100644 --- a/repoze/bfg/tests/test_authentication.py +++ b/repoze/bfg/tests/test_authentication.py @@ -456,6 +456,63 @@ class TestAuthTktCookieHelper(unittest.TestCase): self.failUnless(result[2][1].endswith('; Path=/; Domain=.localhost')) self.failUnless(result[2][1].startswith('auth_tkt=')) + def test_remember_path(self): + plugin = self._makeOne('secret', include_ip=True, + path="/cgi-bin/bfg.cgi/") + request = self._makeRequest() + result = plugin.remember(request, 'other') + self.assertEqual(len(result), 3) + + self.assertEqual(result[0][0], 'Set-Cookie') + self.failUnless(result[0][1].endswith('; Path=/cgi-bin/bfg.cgi/')) + self.failUnless(result[0][1].startswith('auth_tkt=')) + + self.assertEqual(result[1][0], 'Set-Cookie') + self.failUnless(result[1][1].endswith( + '; Path=/cgi-bin/bfg.cgi/; Domain=localhost')) + self.failUnless(result[1][1].startswith('auth_tkt=')) + + self.assertEqual(result[2][0], 'Set-Cookie') + self.failUnless(result[2][1].endswith( + '; Path=/cgi-bin/bfg.cgi/; Domain=.localhost')) + self.failUnless(result[2][1].startswith('auth_tkt=')) + + def test_remember_http_only(self): + plugin = self._makeOne('secret', include_ip=True, http_only=True) + request = self._makeRequest() + result = plugin.remember(request, 'other') + self.assertEqual(len(result), 3) + + self.assertEqual(result[0][0], 'Set-Cookie') + self.failUnless(result[0][1].endswith('; HttpOnly')) + self.failUnless(result[0][1].startswith('auth_tkt=')) + + self.assertEqual(result[1][0], 'Set-Cookie') + self.failUnless(result[1][1].endswith('; HttpOnly')) + self.failUnless(result[1][1].startswith('auth_tkt=')) + + self.assertEqual(result[2][0], 'Set-Cookie') + self.failUnless(result[2][1].endswith('; HttpOnly')) + self.failUnless(result[2][1].startswith('auth_tkt=')) + + def test_remember_secure(self): + plugin = self._makeOne('secret', include_ip=True, secure=True) + request = self._makeRequest() + result = plugin.remember(request, 'other') + self.assertEqual(len(result), 3) + + self.assertEqual(result[0][0], 'Set-Cookie') + self.failUnless('; Secure' in result[0][1]) + self.failUnless(result[0][1].startswith('auth_tkt=')) + + self.assertEqual(result[1][0], 'Set-Cookie') + self.failUnless('; Secure' in result[1][1]) + self.failUnless(result[1][1].startswith('auth_tkt=')) + + self.assertEqual(result[2][0], 'Set-Cookie') + self.failUnless('; Secure' in result[2][1]) + self.failUnless(result[2][1].startswith('auth_tkt=')) + def test_remember_string_userid(self): plugin = self._makeOne('secret') request = self._makeRequest() diff --git a/repoze/bfg/tests/test_zcml.py b/repoze/bfg/tests/test_zcml.py index 1784a6aa8..9148640ca 100644 --- a/repoze/bfg/tests/test_zcml.py +++ b/repoze/bfg/tests/test_zcml.py @@ -423,7 +423,7 @@ class TestAuthTktAuthenticationPolicyDirective(unittest.TestCase): self._callFUT(context, 'sosecret', callback=callback, cookie_name='repoze.bfg.auth_tkt', secure=True, include_ip=True, timeout=100, - reissue_time=60) + reissue_time=60, http_only=True, path="/sub/") actions = context.actions self.assertEqual(len(actions), 1) regadapt = actions[0] @@ -431,6 +431,8 @@ class TestAuthTktAuthenticationPolicyDirective(unittest.TestCase): self.assertEqual(regadapt['callable'], None) self.assertEqual(regadapt['args'], ()) policy = reg.getUtility(IAuthenticationPolicy) + self.assertEqual(policy.cookie.path, '/sub/') + self.assertEqual(policy.cookie.http_only, True) self.assertEqual(policy.cookie.secret, 'sosecret') self.assertEqual(policy.callback, callback) @@ -444,7 +446,8 @@ class TestAuthTktAuthenticationPolicyDirective(unittest.TestCase): context, 'sosecret', callback=callback, cookie_name='repoze.bfg.auth_tkt', secure=True, include_ip=True, timeout=100, - reissue_time=500) + reissue_time=500, http_only=True, + path="/cgi-bin/bfg.cgi/") class TestACLAuthorizationPolicyDirective(unittest.TestCase): def setUp(self): diff --git a/repoze/bfg/zcml.py b/repoze/bfg/zcml.py index 297a59434..fabc1a03c 100644 --- a/repoze/bfg/zcml.py +++ b/repoze/bfg/zcml.py @@ -445,6 +445,8 @@ class IAuthTktAuthenticationPolicyDirective(Interface): timeout = Int(title=u"timeout", required=False, default=None) reissue_time = Int(title=u"reissue_time", required=False, default=None) max_age = Int(title=u"max_age", required=False, default=None) + path = ASCIILine(title=u"path", required=False, default='/') + http_only = Bool(title=u"http_only", required=False, default=False) def authtktauthenticationpolicy(_context, secret, @@ -454,7 +456,9 @@ def authtktauthenticationpolicy(_context, include_ip=False, timeout=None, reissue_time=None, - max_age=None): + max_age=None, + http_only=False, + path='/'): try: policy = AuthTktAuthenticationPolicy(secret, callback=callback, @@ -463,7 +467,9 @@ def authtktauthenticationpolicy(_context, include_ip = include_ip, timeout = timeout, reissue_time = reissue_time, - max_age=max_age) + max_age=max_age, + http_only=http_only, + path=path) except ValueError, why: raise ConfigurationError(str(why)) # authentication policies must be registered eagerly so they can -- cgit v1.2.3