- Do not allow the userid returned from the ``authenticated_userid`` or the

  userid that is one of the list of principals returned by
  ``effective_principals`` to be either of the strings ``system.Everyone`` or
  ``system.Authenticated`` when any of the built-in authorization policies that
  live in ``pyramid.authentication`` are in use.  These two strings are
  reserved for internal usage by Pyramid and they will not be accepted as valid
  userids.

3 files changed, 191 insertions, 3 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 740de0f17..291795da3 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -6,6 +6,14 @@ Features
 
 - Added an ``effective_principals`` route and view predicate.
 
+- Do not allow the userid returned from the ``authenticated_userid`` or the
+  userid that is one of the list of principals returned by
+  ``effective_principals`` to be either of the strings ``system.Everyone`` or
+  ``system.Authenticated`` when any of the built-in authorization policies that
+  live in ``pyramid.authentication`` are in use.  These two strings are
+  reserved for internal usage by Pyramid and they will not be accepted as valid
+  userids.
+
 1.4a3 (2012-10-26)
 ==================
 
diff --git a/pyramid/authentication.py b/pyramid/authentication.py
index 1bae20937..8be34cc0a 100644
--- a/pyramid/authentication.py
+++ b/pyramid/authentication.py
@@ -47,6 +47,11 @@ class CallbackAuthenticationPolicy(object):
             methodname = classname + '.' + methodname
             logger.debug(methodname + ': ' + msg)
 
+    def _clean_principal(self, princid):
+        if princid in (Authenticated, Everyone):
+            princid = None
+        return princid
+
     def authenticated_userid(self, request):
         """ Return the authenticated userid or ``None``.
 
@@ -65,6 +70,14 @@ class CallbackAuthenticationPolicy(object):
                 'authenticated_userid',
                 request)
             return None
+        if self._clean_principal(userid) is None:
+            debug and self._log(
+                ('use of userid %r is disallowed by any built-in Pyramid '
+                 'security policy, returning None' % userid),
+                'authenticated_userid' ,
+                request)
+            return None
+            
         if self.callback is None:
             debug and self._log(
                 'there was no groupfinder callback; returning %r' % (userid,),
@@ -112,6 +125,7 @@ class CallbackAuthenticationPolicy(object):
         debug = self.debug
         effective_principals = [Everyone]
         userid = self.unauthenticated_userid(request)
+
         if userid is None:
             debug and self._log(
                 'unauthenticated_userid returned %r; returning %r' % (
@@ -120,6 +134,16 @@ class CallbackAuthenticationPolicy(object):
                 request
                 )
             return effective_principals
+
+        if self._clean_principal(userid) is None:
+            debug and self._log(
+                ('unauthenticated_userid returned disallowed %r; returning %r '
+                 'as if it was None' % (userid, effective_principals)),
+                'effective_principals',
+                request
+                )
+            return effective_principals
+            
         if self.callback is None:
             debug and self._log(
                 'groupfinder callback is None, so groups is []',
@@ -132,6 +156,7 @@ class CallbackAuthenticationPolicy(object):
                 'groupfinder callback returned %r as groups' % (groups,),
                 'effective_principals',
                 request)
+
         if groups is None: # is None!
             debug and self._log(
                 'returning effective principals: %r' % (
@@ -204,12 +229,36 @@ class RepozeWho1AuthenticationPolicy(CallbackAuthenticationPolicy):
 
         """
         identity = self._get_identity(request)
+
         if identity is None:
+            self.debug and self._log(
+                'repoze.who identity is None, returning None',
+                'authenticated_userid',
+                request)
             return None
+
+        userid = identity['repoze.who.userid']
+
+        if userid is None:
+            self.debug and self._log(
+                'repoze.who.userid is None, returning None' % userid,
+                'authenticated_userid',
+                request)
+            return None
+            
+        if self._clean_principal(userid) is None:
+            self.debug and self._log(
+                ('use of userid %r is disallowed by any built-in Pyramid '
+                 'security policy, returning None' % userid),
+                'authenticated_userid',
+                request)
+            return None
+
         if self.callback is None:
-            return identity['repoze.who.userid']
+            return userid
+
         if self.callback(identity, request) is not None: # is not None!
-            return identity['repoze.who.userid']
+            return userid
 
     def unauthenticated_userid(self, request):
         """ Return the ``repoze.who.userid`` key from the detected identity."""
@@ -233,19 +282,53 @@ class RepozeWho1AuthenticationPolicy(CallbackAuthenticationPolicy):
         """
         effective_principals = [Everyone]
         identity = self._get_identity(request)
+
         if identity is None:
+            self.debug and self._log(
+                ('repoze.who identity was None; returning %r' %
+                 effective_principals),
+                'effective_principals',
+                request
+                )
             return effective_principals
+
         if self.callback is None:
             groups = []
         else:
             groups = self.callback(identity, request)
+
         if groups is None: # is None!
+            self.debug and self._log(
+                ('security policy groups callback returned None; returning %r' %
+                 effective_principals),
+                'effective_principals',
+                request
+                )
             return effective_principals
+
         userid = identity['repoze.who.userid']
+
+        if userid is None:
+            self.debug and self._log(
+                ('repoze.who.userid was None; returning %r' %
+                 effective_principals),
+                'effective_principals',
+                request
+                )
+            return effective_principals
+
+        if self._clean_principal(userid) is None:
+            self.debug and self._log(
+                ('unauthenticated_userid returned disallowed %r; returning %r '
+                 'as if it was None' % (userid, effective_principals)),
+                'effective_principals',
+                request
+                )
+            return effective_principals
+
         effective_principals.append(Authenticated)
         effective_principals.append(userid)
         effective_principals.extend(groups)
-
         return effective_principals
 
     def remember(self, request, principal, **kw):
diff --git a/pyramid/tests/test_authentication.py b/pyramid/tests/test_authentication.py
index dfe3cf0b0..2b7a770c1 100644
--- a/pyramid/tests/test_authentication.py
+++ b/pyramid/tests/test_authentication.py
@@ -76,6 +76,30 @@ class TestCallbackAuthenticationPolicyDebugging(unittest.TestCase):
             "authenticated_userid: groupfinder callback returned []; "
             "returning 'fred'")
 
+    def test_authenticated_userid_fails_cleaning_as_Authenticated(self):
+        request = DummyRequest(registry=self.config.registry)
+        policy = self._makeOne(userid='system.Authenticated')
+        self.assertEqual(policy.authenticated_userid(request), None)
+        self.assertEqual(len(self.messages), 1)
+        self.assertEqual(
+            self.messages[0],
+            "pyramid.tests.test_authentication.MyAuthenticationPolicy."
+            "authenticated_userid: use of userid 'system.Authenticated' is "
+            "disallowed by any built-in Pyramid security policy, returning "
+            "None")
+
+    def test_authenticated_userid_fails_cleaning_as_Everyone(self):
+        request = DummyRequest(registry=self.config.registry)
+        policy = self._makeOne(userid='system.Everyone')
+        self.assertEqual(policy.authenticated_userid(request), None)
+        self.assertEqual(len(self.messages), 1)
+        self.assertEqual(
+            self.messages[0],
+            "pyramid.tests.test_authentication.MyAuthenticationPolicy."
+            "authenticated_userid: use of userid 'system.Everyone' is "
+            "disallowed by any built-in Pyramid security policy, returning "
+            "None")
+
     def test_effective_principals_no_unauthenticated_userid(self):
         request = DummyRequest(registry=self.config.registry)
         policy = self._makeOne()
@@ -144,6 +168,34 @@ class TestCallbackAuthenticationPolicyDebugging(unittest.TestCase):
             "effective_principals: returning effective principals: "
             "['system.Everyone', 'system.Authenticated', 'fred']")
 
+    def test_effective_principals_with_unclean_principal_Authenticated(self):
+        request = DummyRequest(registry=self.config.registry)
+        policy = self._makeOne(userid='system.Authenticated')
+        self.assertEqual(
+            policy.effective_principals(request),
+            ['system.Everyone'])
+        self.assertEqual(len(self.messages), 1)
+        self.assertEqual(
+            self.messages[0],
+            "pyramid.tests.test_authentication.MyAuthenticationPolicy."
+            "effective_principals: unauthenticated_userid returned disallowed "
+            "'system.Authenticated'; returning ['system.Everyone'] as if it "
+            "was None")
+
+    def test_effective_principals_with_unclean_principal_Everyone(self):
+        request = DummyRequest(registry=self.config.registry)
+        policy = self._makeOne(userid='system.Everyone')
+        self.assertEqual(
+            policy.effective_principals(request),
+            ['system.Everyone'])
+        self.assertEqual(len(self.messages), 1)
+        self.assertEqual(
+            self.messages[0],
+            "pyramid.tests.test_authentication.MyAuthenticationPolicy."
+            "effective_principals: unauthenticated_userid returned disallowed "
+            "'system.Everyone'; returning ['system.Everyone'] as if it "
+            "was None")
+
 class TestRepozeWho1AuthenticationPolicy(unittest.TestCase):
     def _getTargetClass(self):
         from pyramid.authentication import RepozeWho1AuthenticationPolicy
@@ -184,6 +236,12 @@ class TestRepozeWho1AuthenticationPolicy(unittest.TestCase):
         policy = self._makeOne()
         self.assertEqual(policy.authenticated_userid(request), 'fred')
 
+    def test_authenticated_userid_repoze_who_userid_is_None(self):
+        request = DummyRequest(
+            {'repoze.who.identity':{'repoze.who.userid':None}})
+        policy = self._makeOne()
+        self.assertEqual(policy.authenticated_userid(request), None)
+
     def test_authenticated_userid_with_callback_returns_None(self):
         request = DummyRequest(
             {'repoze.who.identity':{'repoze.who.userid':'fred'}})
@@ -200,6 +258,20 @@ class TestRepozeWho1AuthenticationPolicy(unittest.TestCase):
         policy = self._makeOne(callback=callback)
         self.assertEqual(policy.authenticated_userid(request), 'fred')
 
+    def test_authenticated_userid_unclean_principal_Authenticated(self):
+        request = DummyRequest(
+            {'repoze.who.identity':{'repoze.who.userid':'system.Authenticated'}}
+            )
+        policy = self._makeOne()
+        self.assertEqual(policy.authenticated_userid(request), None)
+
+    def test_authenticated_userid_unclean_principal_Everyone(self):
+        request = DummyRequest(
+            {'repoze.who.identity':{'repoze.who.userid':'system.Everyone'}}
+            )
+        policy = self._makeOne()
+        self.assertEqual(policy.authenticated_userid(request), None)
+
     def test_effective_principals_None(self):
         from pyramid.security import Everyone
         request = DummyRequest({})
@@ -237,6 +309,31 @@ class TestRepozeWho1AuthenticationPolicy(unittest.TestCase):
         policy = self._makeOne(callback=callback)
         self.assertEqual(policy.effective_principals(request), [Everyone])
 
+    def test_effective_principals_repoze_who_userid_is_None(self):
+        from pyramid.security import Everyone
+        request = DummyRequest(
+            {'repoze.who.identity':{'repoze.who.userid':None}}
+            )
+        policy = self._makeOne()
+        self.assertEqual(policy.effective_principals(request), [Everyone])
+
+    def test_effective_principals_repoze_who_userid_is_unclean_Everyone(self):
+        from pyramid.security import Everyone
+        request = DummyRequest(
+            {'repoze.who.identity':{'repoze.who.userid':'system.Everyone'}}
+            )
+        policy = self._makeOne()
+        self.assertEqual(policy.effective_principals(request), [Everyone])
+
+    def test_effective_principals_repoze_who_userid_is_unclean_Authenticated(
+        self):
+        from pyramid.security import Everyone
+        request = DummyRequest(
+            {'repoze.who.identity':{'repoze.who.userid':'system.Authenticated'}}
+            )
+        policy = self._makeOne()
+        self.assertEqual(policy.effective_principals(request), [Everyone])
+
     def test_remember_no_plugins(self):
         request = DummyRequest({})
         policy = self._makeOne()