diff options
| -rw-r--r-- | CHANGES.txt | 21 | ||||
| -rw-r--r-- | TODO.txt | 3 | ||||
| -rw-r--r-- | docs/api/authentication.rst | 10 | ||||
| -rw-r--r-- | docs/narr/renderers.rst | 2 | ||||
| -rw-r--r-- | docs/narr/sessions.rst | 13 | ||||
| -rw-r--r-- | pyramid/authentication.py | 172 | ||||
| -rw-r--r-- | pyramid/tests/test_authentication.py | 97 |
7 files changed, 299 insertions, 19 deletions
diff --git a/CHANGES.txt b/CHANGES.txt index 25d2dc75c..563851e74 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -6,13 +6,20 @@ Features - Added an ``effective_principals`` route and view predicate. -Bug Fixes ---------- - -- :func:`pyramid.security.view_execution_permitted` would return `True` if - no view could be found. This case now raises an exception as it doesn't - make sense make an assertion about a non-existant view. See - https://github.com/Pylons/pyramid/issues/299. +- 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. + +- Slightly better debug logging from RepozeWho1AuthenticationPolicy. + +- ``pyramid.security.view_execution_permitted`` used to return `True` if no + view could be found. It now raises a ``TypeError`` exception in that case, as + it doesn't make sense to assert that a nonexistent view is + execution-permitted. See https://github.com/Pylons/pyramid/issues/299. 1.4a3 (2012-10-26) ================== @@ -13,9 +13,6 @@ Nice-to-Have - Have action methods return their discriminators. -- Add docs about upgrading between Pyramid versions (e.g. how to see - deprecation warnings). - - Fix renderers chapter to better document system values passed to template renderers. diff --git a/docs/api/authentication.rst b/docs/api/authentication.rst index 587026a3b..19d08618b 100644 --- a/docs/api/authentication.rst +++ b/docs/api/authentication.rst @@ -9,14 +9,24 @@ Authentication Policies .. automodule:: pyramid.authentication .. autoclass:: AuthTktAuthenticationPolicy + :members: + :inherited-members: .. autoclass:: RemoteUserAuthenticationPolicy + :members: + :inherited-members: .. autoclass:: SessionAuthenticationPolicy + :members: + :inherited-members: .. autoclass:: BasicAuthAuthenticationPolicy + :members: + :inherited-members: .. autoclass:: RepozeWho1AuthenticationPolicy + :members: + :inherited-members: Helper Classes ~~~~~~~~~~~~~~ diff --git a/docs/narr/renderers.rst b/docs/narr/renderers.rst index 63287e2cd..1158d2225 100644 --- a/docs/narr/renderers.rst +++ b/docs/narr/renderers.rst @@ -329,7 +329,7 @@ time "by hand". Configure a JSONP renderer using the Once this renderer is registered via :meth:`~pyramid.config.Configurator.add_renderer` as above, you can use ``jsonp`` as the ``renderer=`` parameter to ``@view_config`` or -:meth:`pyramid.config.Configurator.add_view``: +:meth:`pyramid.config.Configurator.add_view`: .. code-block:: python diff --git a/docs/narr/sessions.rst b/docs/narr/sessions.rst index 1aa1b6341..f7da7838e 100644 --- a/docs/narr/sessions.rst +++ b/docs/narr/sessions.rst @@ -63,10 +63,15 @@ application by using the ``session_factory`` argument to the this implementation is, by default, *unencrypted*. You should not use it when you keep sensitive information in the session object, as the information can be easily read by both users of your application and third - parties who have access to your users' network traffic. Use a different - session factory implementation (preferably one which keeps session data on - the server) for anything but the most basic of applications where "session - security doesn't matter". + parties who have access to your users' network traffic. And if you use this + sessioning implementation, and you inadvertently create a cross-site + scripting vulnerability in your application, because the session data is + stored unencrypted in a cookie, it will also be easier for evildoers to + obtain the current user's cross-site scripting token. In short, use a + different session factory implementation (preferably one which keeps session + data on the server) for anything but the most basic of applications where + "session security doesn't matter", and you are sure your application has no + cross-site scripting vulnerabilities. .. index:: single: session object diff --git a/pyramid/authentication.py b/pyramid/authentication.py index d4fd7ab8b..8be34cc0a 100644 --- a/pyramid/authentication.py +++ b/pyramid/authentication.py @@ -47,7 +47,21 @@ 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``. + + If no callback is registered, this will be the same as + ``unauthenticated_userid``. + + If a ``callback`` is registered, this will return the userid if + and only if the callback returns a value that is not ``None``. + + """ debug = self.debug userid = self.unauthenticated_userid(request) if userid is None: @@ -56,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,), @@ -78,9 +100,32 @@ class CallbackAuthenticationPolicy(object): ) def effective_principals(self, request): + """ A list of effective principals derived from request. + + This will return a list of principals including, at least, + :data:`pyramid.security.Everyone`. If there is no authenticated + userid, or the ``callback`` returns ``None``, this will be the + only principal: + + .. code-block:: python + + return [Everyone] + + If the ``callback`` does not return ``None`` and an authenticated + userid is found, then the principals will include + :data:`pyramid.security.Authenticated`, the ``authenticated_userid`` + and the list of principals returned by the ``callback``: + + .. code-block:: python + + extra_principals = callback(userid, request) + return [Everyone, Authenticated, userid] + extra_principals + + """ 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' % ( @@ -89,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 []', @@ -101,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' % ( @@ -163,39 +219,120 @@ class RepozeWho1AuthenticationPolicy(CallbackAuthenticationPolicy): return identifier def authenticated_userid(self, request): + """ Return the authenticated userid or ``None``. + + If no callback is registered, this will be the same as + ``unauthenticated_userid``. + + If a ``callback`` is registered, this will return the userid if + and only if the callback returns a value that is not ``None``. + + """ 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.""" identity = self._get_identity(request) if identity is None: return None return identity['repoze.who.userid'] def effective_principals(self, request): + """ A list of effective principals derived from the identity. + + This will return a list of principals including, at least, + :data:`pyramid.security.Everyone`. If there is no identity, or + the ``callback`` returns ``None``, this will be the only principal. + + If the ``callback`` does not return ``None`` and an identity is + found, then the principals will include + :data:`pyramid.security.Authenticated`, the ``authenticated_userid`` + and the list of principals returned by the ``callback``. + + """ 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): + """ Store the ``principal`` as ``repoze.who.userid``.""" identifier = self._get_identifier(request) if identifier is None: return [] @@ -204,6 +341,12 @@ class RepozeWho1AuthenticationPolicy(CallbackAuthenticationPolicy): return identifier.remember(environ, identity) def forget(self, request): + """ Forget the current authenticated user. + + Return headers that, if included in a response, will delete the + cookie responsible for tracking the current user. + + """ identifier = self._get_identifier(request) if identifier is None: return [] @@ -247,12 +390,19 @@ class RemoteUserAuthenticationPolicy(CallbackAuthenticationPolicy): self.debug = debug def unauthenticated_userid(self, request): + """ The ``REMOTE_USER`` value found within the ``environ``.""" return request.environ.get(self.environ_key) def remember(self, request, principal, **kw): + """ A no-op. The ``REMOTE_USER`` does not provide a protocol for + remembering the user. This will be application-specific and can + be done somewhere else or in a subclass.""" return [] def forget(self, request): + """ A no-op. The ``REMOTE_USER`` does not provide a protocol for + forgetting the user. This will be application-specific and can + be done somewhere else or in a subclass.""" return [] @implementer(IAuthenticationPolicy) @@ -388,16 +538,23 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy): self.debug = debug def unauthenticated_userid(self, request): + """ The userid key within the auth_tkt cookie.""" result = self.cookie.identify(request) if result: return result['userid'] def remember(self, request, principal, **kw): """ Accepts the following kw args: ``max_age=<int-seconds>, - ``tokens=<sequence-of-ascii-strings>``""" + ``tokens=<sequence-of-ascii-strings>``. + + Return a list of headers which will set appropriate cookies on + the response. + + """ return self.cookie.remember(request, principal, **kw) def forget(self, request): + """ A list of headers which will delete appropriate cookies.""" return self.cookie.forget(request) def b64encode(v): @@ -860,14 +1017,21 @@ class BasicAuthAuthenticationPolicy(CallbackAuthenticationPolicy): self.debug = debug def unauthenticated_userid(self, request): + """ The userid parsed from the ``Authorization`` request header.""" credentials = self._get_credentials(request) if credentials: return credentials[0] def remember(self, request, principal, **kw): + """ A no-op. Basic authentication does not provide a protocol for + remembering the user. Credentials are sent on every request. + + """ return [] def forget(self, request): + """ Returns challenge headers. This should be attached to a response + to indicate that credentials are required.""" return [('WWW-Authenticate', 'Basic realm="%s"' % self.realm)] def callback(self, username, request): 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() |