- Don't explicitly prevent the ``timeout`` from being lower than the

  ``reissue_time`` when setting up an ``AuthTktAuthenticationPolicy``
  (previously such a configuration would raise a ``ValueError``, now it's
  allowed, although typically nonsensical).  Allowing the nonsensical
  configuration made the code more understandable and required fewer tests.

- Add a test for when the auth_tkt value is empty.

3 files changed, 45 insertions, 19 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 7f12b4d08..768a08b0a 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -43,6 +43,12 @@ Features
   eg. ``context.result.permission``, ``context.result.acl``, etc within the
   logic of the Forbidden exception view.
 
+- Don't explicitly prevent the ``timeout`` from being lower than the
+  ``reissue_time`` when setting up an ``AuthTktAuthenticationPolicy``
+  (previously such a configuration would raise a ``ValueError``, now it's
+  allowed, although typically nonsensical).  Allowing the nonsensical
+  configuration made the code more understandable and required fewer tests.
+
 Bug Fixes
 ---------
 
diff --git a/pyramid/authentication.py b/pyramid/authentication.py
index ad4ddf3ce..3d3139b7c 100644
--- a/pyramid/authentication.py
+++ b/pyramid/authentication.py
@@ -210,18 +210,22 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy):
 
     ``reissue_time``
 
-       Default: ``None``.  If this parameter is set, it represents the
-       number of seconds that must pass before an authentication token
-       cookie is reissued.  The duration is measured as the number of
-       seconds since the last auth_tkt cookie was issued and 'now'.
-       If the ``timeout`` value is ``None``, this parameter has no
-       effect.  If this parameter is provided, and the value of
-       ``timeout`` is not ``None``, the value of ``reissue_time`` must
-       be smaller than value of ``timeout``.  A good rule of thumb: if
-       you want auto-reissued cookies: set this to the ``timeout``
-       value divided by ten.  If this value is ``0``, a new ticket
-       cookie will be reissued on every request which needs
-       authentication. Optional.
+       Default: ``None``.  If this parameter is set, it represents the number
+       of seconds that must pass before an authentication token cookie is
+       automatically reissued as the result of a request which requires
+       authentication.  The duration is measured as the number of seconds
+       since the last auth_tkt cookie was issued and 'now'.  If this value is
+       ``0``, a new ticket cookie will be reissued on every request which
+       requires authentication.
+
+       A good rule of thumb: if you want auto-expired cookies based on
+       inactivity: set the ``timeout`` value to 1200 (20 mins) and set the
+       ``reissue_time`` value to perhaps a tenth of the ``timeout`` value
+       (120 or 2 mins).  It's nonsensical to set the ``timeout`` value lower
+       than the ``reissue_time`` value, as the ticket will never be reissued
+       if so.  However, such a configuration is not explicitly prevented.
+
+       Optional.
 
     ``max_age``
 
@@ -334,9 +338,6 @@ class AuthTktCookieHelper(object):
         self.include_ip = include_ip
         self.secure = secure
         self.timeout = timeout
-        if reissue_time is not None and timeout is not None:
-            if reissue_time > timeout:
-                raise ValueError('reissue_time must be lower than timeout')
         self.reissue_time = reissue_time
         self.max_age = max_age
         self.http_only = http_only
@@ -421,6 +422,7 @@ class AuthTktCookieHelper(object):
             now = time.time()
 
         if self.timeout and ( (timestamp + self.timeout) < now ):
+            # the auth_tkt data has expired
             return None
 
         userid_typename = 'userid_type:'
diff --git a/pyramid/tests/test_authentication.py b/pyramid/tests/test_authentication.py
index 84d2839c9..e16f53fdb 100644
--- a/pyramid/tests/test_authentication.py
+++ b/pyramid/tests/test_authentication.py
@@ -328,6 +328,12 @@ class TestAuthTktCookieHelper(unittest.TestCase):
         request = self._makeRequest()
         result = plugin.identify(request)
         self.assertEqual(result, None)
+
+    def test_identify_cookie_value_is_None(self):
+        plugin = self._makeOne('secret')
+        request = self._makeRequest({'HTTP_COOKIE':'auth_tkt='})
+        result = plugin.identify(request)
+        self.assertEqual(result, None)
         
     def test_identify_good_cookie_include_ip(self):
         plugin = self._makeOne('secret', include_ip=True)
@@ -379,6 +385,22 @@ class TestAuthTktCookieHelper(unittest.TestCase):
         self.assertEqual(environ['REMOTE_USER_DATA'],'userid_type:int')
         self.assertEqual(environ['AUTH_TYPE'],'cookie')
 
+    def test_identify_nonuseridtype_user_data(self):
+        plugin = self._makeOne('secret', include_ip=False)
+        plugin.auth_tkt.userid = '1'
+        plugin.auth_tkt.user_data = 'bogus:int'
+        request = self._makeRequest({'HTTP_COOKIE':'auth_tkt=ticket'})
+        result = plugin.identify(request)
+        self.assertEqual(len(result), 4)
+        self.assertEqual(result['tokens'], ())
+        self.assertEqual(result['userid'], '1')
+        self.assertEqual(result['userdata'], 'bogus:int')
+        self.assertEqual(result['timestamp'], 0)
+        environ = request.environ
+        self.assertEqual(environ['REMOTE_USER_TOKENS'], ())
+        self.assertEqual(environ['REMOTE_USER_DATA'],'bogus:int')
+        self.assertEqual(environ['AUTH_TYPE'],'cookie')
+
     def test_identify_good_cookie_unknown_useridtype(self):
         plugin = self._makeOne('secret', include_ip=False)
         plugin.auth_tkt.userid = 'abc'
@@ -696,10 +718,6 @@ class TestAuthTktCookieHelper(unittest.TestCase):
                          'auth_tkt=""; Path=/; Domain=.localhost; Max-Age=0; '
                          'Expires=Wed, 31-Dec-97 23:59:59 GMT')
 
-    def test_timeout_lower_than_reissue(self):
-        self.assertRaises(ValueError, self._makeOne, 'userid', timeout=1,
-                          reissue_time=2)
-
 class DummyContext:
     pass