Repository: repoze/repoze.who

Issue:
AuthTicket should not allow tokens with embedded separators
The Apache `mod_auth_tkt` module docs are canonical for the [cookie format](https://github.com/gavincarr/mod_auth_tkt#cookie-format).  Because there is no mechanism specified for escaping characters, we should disallow token values containing `","`.  We should also disallow `"!"` in the userid, token values, and user data.

Discussion:

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index ae441cd..be3d9b0 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -4,10 +4,12 @@ repoze.who Changelog
 2.4.1 (unreleased)
 ------------------
 
+- Disallow separators in AuthTicket component values.  Closes #37.
+
 - Handle bytes / string correctly in 'repoze.who.plugins.htpasswd.sha1_check'.
   Closes #28.
 
-- Switch to use ``pytest`` as the testrunner.
+- Switch to use ``pytest`` as the testrunner.  Closes #34.
 
 2.4 (2020-06-03)
 ----------------
diff --git a/repoze/who/_auth_tkt.py b/repoze/who/_auth_tkt.py
index b088064..313f900 100644
--- a/repoze/who/_auth_tkt.py
+++ b/repoze/who/_auth_tkt.py
@@ -47,6 +47,15 @@ from repoze.who._compat import url_unquote
 DEFAULT_DIGEST = hashlib.md5
 
 
+def _exclude_separator(separator, value, fieldname):
+    if isinstance(value, bytes):
+        separator = separator.encode("ascii")
+
+    if separator in value:
+        raise ValueError(
+            "{} may not contain '{}'".format(fieldname, separator)
+        )
+
 class AuthTicket(object):
 
     """
@@ -88,14 +97,26 @@ class AuthTicket(object):
                  time=None, cookie_name='auth_tkt',
                  secure=False, digest_algo=DEFAULT_DIGEST):
         self.secret = secret
+
+        _exclude_separator('!', userid, "'userid'")
         self.userid = userid
+
         self.ip = ip
+
+        for token in tokens:
+            _exclude_separator(',', token, "'token' values")
+            _exclude_separator('!', token, "'token' values")
+
         self.tokens = ','.join(tokens)
+
+        _exclude_separator('!', user_data, "'user_data'")
         self.user_data = user_data
+
         if time is None:
             self.time = time_mod.time()
         else:
             self.time = time
+
         self.cookie_name = cookie_name
         self.secure = secure
         if isinstance(digest_algo, str):


Test patch:
diff --git a/repoze/who/tests/test__auth_tkt.py b/repoze/who/tests/test__auth_tkt.py
index a5745d5..6d8ee90 100644
--- a/repoze/who/tests/test__auth_tkt.py
+++ b/repoze/who/tests/test__auth_tkt.py
@@ -25,6 +25,42 @@ class AuthTicketTests(unittest.TestCase):
         self.assertEqual(tkt.secure, False)
         self.assertEqual(tkt.digest_algo, hashlib.md5)
 
+    def test_ctor_w_userid_w_embedded_bang(self):
+        tokens = ('a,b',)  # cannot be safely round-tripped
+
+        with self.assertRaises(ValueError) as exc:
+            self._makeOne('SEEKRIT', 'USER!ID', '1.2.3.4')
+
+        self.assertEqual(str(exc.exception), "'userid' may not contain '!'")
+
+    def test_ctor_w_token_w_embedded_bang(self):
+        tokens = ('a!b',)  # cannot be safely round-tripped
+
+        with self.assertRaises(ValueError) as exc:
+            self._makeOne('SEEKRIT', 'USERID', '1.2.3.4', tokens=tokens)
+
+        self.assertEqual(
+            str(exc.exception), "'token' values may not contain '!'"
+        )
+
+    def test_ctor_w_token_w_embedded_comma(self):
+        tokens = ('a,b',)  # cannot be safely round-tripped
+
+        with self.assertRaises(ValueError) as exc:
+            self._makeOne('SEEKRIT', 'USERID', '1.2.3.4', tokens=tokens)
+
+        self.assertEqual(
+            str(exc.exception), "'token' values may not contain ','"
+        )
+
+    def test_ctor_w_user_data_w_embedded_bang(self):
+        user_data = 'DATA!HERE' # cannot be safely round-tripped
+
+        with self.assertRaises(ValueError) as exc:
+            self._makeOne('SEEKRIT', 'USERID', '1.2.3.4', user_data=user_data)
+
+        self.assertEqual(str(exc.exception), "'user_data' may not contain '!'")
+
     def test_ctor_explicit(self):
         import hashlib
         tkt = self._makeOne('SEEKRIT', 'USERID', '1.2.3.4', tokens=('a', 'b'),