From 07c9ee0ec96eb664974fe314a46389ed59390520 Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Fri, 2 Nov 2012 21:05:46 -0400 Subject: - 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. --- CHANGES.txt | 8 +++ pyramid/authentication.py | 89 +++++++++++++++++++++++++++++++-- pyramid/tests/test_authentication.py | 97 ++++++++++++++++++++++++++++++++++++ 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() -- cgit v1.2.3