diff options
| -rw-r--r-- | .flake8 | 1 | ||||
| -rw-r--r-- | TODO.txt | 4 | ||||
| -rw-r--r-- | docs/api/request.rst | 18 | ||||
| -rw-r--r-- | docs/glossary.rst | 4 | ||||
| -rw-r--r-- | docs/narr/security.rst | 70 | ||||
| -rw-r--r-- | docs/quick_tutorial/authentication.rst | 2 | ||||
| -rw-r--r-- | docs/whatsnew-2.0.rst | 10 | ||||
| -rw-r--r-- | src/pyramid/authentication.py | 6 | ||||
| -rw-r--r-- | src/pyramid/config/testing.py | 8 | ||||
| -rw-r--r-- | src/pyramid/interfaces.py | 17 | ||||
| -rw-r--r-- | src/pyramid/security.py | 56 | ||||
| -rw-r--r-- | src/pyramid/testing.py | 9 | ||||
| -rw-r--r-- | src/pyramid/viewderivers.py | 9 | ||||
| -rw-r--r-- | tests/pkgs/securityapp/__init__.py | 8 | ||||
| -rw-r--r-- | tests/test_authorization.py | 3 | ||||
| -rw-r--r-- | tests/test_security.py | 14 | ||||
| -rw-r--r-- | tests/test_testing.py | 2 | ||||
| -rw-r--r-- | tests/test_viewderivers.py | 5 |
18 files changed, 140 insertions, 106 deletions
@@ -9,7 +9,6 @@ ignore = # W504: line break after binary operator (flake8 is not PEP8 compliant) W504 exclude = - src/pyramid/compat.py tests/fixtures tests/pkgs tests/test_config/pkgs @@ -116,10 +116,6 @@ Probably Bad Ideas - Add functionality that mocks the behavior of ``repoze.browserid``. -- Consider implementing the API outlined in - http://plope.com/pyramid_auth_design_api_postmortem, phasing out the - current auth-n-auth abstractions in a backwards compatible way. - - Maybe add ``add_renderer_globals`` method to Configurator. - Supply ``X-Vhm-Host`` support (probably better to do what paste#prefix diff --git a/docs/api/request.rst b/docs/api/request.rst index 8e0f77b87..fb4b5caee 100644 --- a/docs/api/request.rst +++ b/docs/api/request.rst @@ -166,11 +166,11 @@ .. attribute:: authenticated_userid - .. deprecated:: 2.0 + .. versionchanged:: 2.0 - ``authenticated_userid`` has been replaced by - :attr:`authenticated_identity` in the new security system. See - :ref:`upgrading_auth` for more information. + ``authenticated_userid`` uses security policy or authn pol + see also :attr:`authenticated_identity` and + :ref:`upgrading_auth` for more information. A property which returns the :term:`userid` of the currently authenticated user or ``None`` if there is no :term:`authentication @@ -184,9 +184,9 @@ .. deprecated:: 2.0 - ``unauthenticated_userid`` has been replaced by - :attr:`authenticated_identity` in the new security system. See - :ref:`upgrading_auth` for more information. + ``unauthenticated_userid`` has been replaced by + :attr:`authenticated_identity` in the new security system. See + :ref:`upgrading_auth` for more information. A property which returns a value which represents the *claimed* (not verified) :term:`userid` of the credentials present in the @@ -203,8 +203,8 @@ .. deprecated:: 2.0 - The new security policy has removed the concept of principals. See - :ref:`upgrading_auth` for more information. + The new security policy has removed the concept of principals. See + :ref:`upgrading_auth` for more information. A property which returns the list of 'effective' :term:`principal` identifiers for this request. This list typically includes the diff --git a/docs/glossary.rst b/docs/glossary.rst index ac60ebd24..5edc4eeab 100644 --- a/docs/glossary.rst +++ b/docs/glossary.rst @@ -304,8 +304,8 @@ Glossary identity An identity is an object identifying the user associated with the - current request. The identity can be any object, but should implement a - ``__str__`` method that outputs a corresponding :term:`userid`. + current request. The identity can be any object, but security policies + should ensure that it represents a valid user (not deleted or deactivated). security policy A security policy in :app:`Pyramid` terms is an object implementing the diff --git a/docs/narr/security.rst b/docs/narr/security.rst index f1bb37c69..a71b9abd9 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -72,12 +72,19 @@ A simple security policy might look like the following: from pyramid.security import Allowed, Denied class SessionSecurityPolicy: - def identify(self, request): + def authenticated_userid(self, request): """ Return the user ID stored in the session. """ return request.session.get('userid') - def permits(self, request, context, identity, permission): + def identify(self, request): + """ Return app-specific user object. """ + userid = self.authenticated_userid(request) + if userid is not None: + return models.Users.get(id=userid) + + def permits(self, request, context, permission): """ Allow access to everything if signed in. """ + identity = self.identify(request) if identity is not None: return Allowed('User is signed in.') else: @@ -87,7 +94,7 @@ A simple security policy might look like the following: request.session['userid'] = userid return [] - def forget(request): + def forget(request, **kw): del request.session['userid'] return [] @@ -136,12 +143,16 @@ For example, our above security policy can leverage these helpers like so: def __init__(self): self.helper = SessionAuthenticationHelper() + def authenticated_userid(self, request): + # XXX add code + ... + def identify(self, request): - """ Return the user ID stored in the session. """ return self.helper.identify(request) - def permits(self, request, context, identity, permission): + def permits(self, request, context, permission): """ Allow access to everything if signed in. """ + identity = self.identify(request) if identity is not None: return Allowed('User is signed in.') else: @@ -150,8 +161,8 @@ For example, our above security policy can leverage these helpers like so: def remember(request, userid, **kw): return self.helper.remember(request, userid, **kw) - def forget(request): - return self.helper.forget(request) + def forget(request, **kw): + return self.helper.forget(request, **kw) Helpers are intended to be used with application-specific code, so perhaps your authentication also queries the database to ensure the identity is valid. @@ -159,13 +170,13 @@ authentication also queries the database to ensure the identity is valid. .. code-block:: python :linenos: - def identify(self, request): - """ Return the user ID stored in the session. """ - user_id = self.helper.identify(request) - if validate_user_id(user_id): - return user_id - else: - return None + def identify(self, request): + # XXX review: use authenticated_userid below or identify? + user_id = self.helper.identify(request) + if validate_user_id(user_id): + return user_id + else: + return None .. index:: single: permissions @@ -237,7 +248,9 @@ might look like so: from pyramid.security import Allowed, Denied class SecurityPolicy: - def permits(self, request, context, identity, permission): + def permits(self, request, context, permission): + identity = self.identify(request) + if identity is None: return Denied('User is not signed in.') if identity.role == 'admin': @@ -246,6 +259,7 @@ might look like so: allowed = ['read', 'write'] else: allowed = ['read'] + if permission in allowed: return Allowed( 'Access granted for user %s with role %s.', @@ -326,7 +340,7 @@ object. An implementation might look like this: from pyramid.authorization import ACLHelper class SecurityPolicy: - def permits(self, request, context, identity, permission): + def permits(self, request, context, permission): principals = [Everyone] if identity is not None: principals.append(Authenticated) @@ -352,7 +366,7 @@ For example, an ACL might be attached to the resource for a blog via its class: (Allow, Everyone, 'view'), (Allow, 'group:editors', 'add'), (Allow, 'group:editors', 'edit'), - ] + ] Or, if your resources are persistent, an ACL might be specified via the ``__acl__`` attribute of an *instance* of a resource: @@ -369,10 +383,10 @@ Or, if your resources are persistent, an ACL might be specified via the blog = Blog() blog.__acl__ = [ - (Allow, Everyone, 'view'), - (Allow, 'group:editors', 'add'), - (Allow, 'group:editors', 'edit'), - ] + (Allow, Everyone, 'view'), + (Allow, 'group:editors', 'add'), + (Allow, 'group:editors', 'edit'), + ] Whether an ACL is attached to a resource's class or an instance of the resource itself, the effect is the same. It is useful to decorate individual resource @@ -425,10 +439,10 @@ Here's an example ACL: from pyramid.security import Everyone __acl__ = [ - (Allow, Everyone, 'view'), - (Allow, 'group:editors', 'add'), - (Allow, 'group:editors', 'edit'), - ] + (Allow, Everyone, 'view'), + (Allow, 'group:editors', 'add'), + (Allow, 'group:editors', 'edit'), + ] The example ACL indicates that the :data:`pyramid.security.Everyone` principal—a special system-defined principal indicating, literally, everyone—is @@ -460,7 +474,7 @@ dictated by the ACL*. So if you have an ACL like this: __acl__ = [ (Allow, Everyone, 'view'), (Deny, Everyone, 'view'), - ] + ] The ACL helper will *allow* everyone the view permission, even though later in the ACL you have an ACE that denies everyone the view permission. On the other @@ -476,7 +490,7 @@ hand, if you have an ACL like this: __acl__ = [ (Deny, Everyone, 'view'), (Allow, Everyone, 'view'), - ] + ] The ACL helper will deny everyone the view permission, even though later in the ACL, there is an ACE that allows everyone. @@ -495,7 +509,7 @@ can collapse this into a single ACE, as below. __acl__ = [ (Allow, Everyone, 'view'), (Allow, 'group:editors', ('add', 'edit')), - ] + ] .. _special_principals: diff --git a/docs/quick_tutorial/authentication.rst b/docs/quick_tutorial/authentication.rst index cd038ea36..ccdd9b70b 100644 --- a/docs/quick_tutorial/authentication.rst +++ b/docs/quick_tutorial/authentication.rst @@ -104,6 +104,8 @@ Steps Analysis ======== +# TODO update + Unlike many web frameworks, Pyramid includes a built-in but optional security model for authentication and authorization. This security system is intended to be flexible and support many needs. In this security model, authentication (who diff --git a/docs/whatsnew-2.0.rst b/docs/whatsnew-2.0.rst index bf1554a27..4448e0f69 100644 --- a/docs/whatsnew-2.0.rst +++ b/docs/whatsnew-2.0.rst @@ -44,10 +44,12 @@ The new security policy adds the concept of an :term:`identity`, which is an object representing the user associated with the current request. The identity can be accessed via :attr:`pyramid.request.Request.authenticated_identity`. The object can be of any shape, such as a simple ID string or an ORM object, -but should implement a ``__str__`` method that returns a string identifying the -current user, e.g. the ID of the user object in a database. The string -representation is return as -:attr:`pyramid.request.Request.authenticated_userid`. +and should represent an active user. + +As in previous version, the property :attr:`pyramid.request.Request.authenticated_userid` +can be used to get a string identifying the current user, for example +the ID of the user object in a database. The value is obtained from the +security policy. (:attr:`pyramid.request.Request.unauthenticated_userid` has been deprecated.) The concept of :term:`principals <principal>` has been removed; the diff --git a/src/pyramid/authentication.py b/src/pyramid/authentication.py index de06fe955..500a84646 100644 --- a/src/pyramid/authentication.py +++ b/src/pyramid/authentication.py @@ -1110,7 +1110,7 @@ class SessionAuthenticationPolicy(CallbackAuthenticationPolicy): return self.helper.forget(request) def unauthenticated_userid(self, request): - return self.helper.identify(request) + return self.helper.authenticated_userid(request) class SessionAuthenticationHelper: @@ -1134,13 +1134,13 @@ class SessionAuthenticationHelper: request.session[self.userid_key] = userid return [] - def forget(self, request): + def forget(self, request, **kw): """ Remove the stored userid from the session.""" if self.userid_key in request.session: del request.session[self.userid_key] return [] - def identify(self, request): + def authenticated_userid(self, request): """ Return the stored userid.""" return request.session.get(self.userid_key) diff --git a/src/pyramid/config/testing.py b/src/pyramid/config/testing.py index 21c622656..83a8db552 100644 --- a/src/pyramid/config/testing.py +++ b/src/pyramid/config/testing.py @@ -13,6 +13,7 @@ class TestingConfiguratorMixin(object): # testing API def testing_securitypolicy( self, + userid=None, identity=None, permissive=True, remember_result=None, @@ -39,6 +40,7 @@ class TestingConfiguratorMixin(object): not provided (or it is provided, and is ``None``), the default value ``[]`` (the empty list) will be returned by ``forget``. + XXX rewrite The behavior of the registered :term:`authentication policy` depends on the values provided for the ``userid`` and ``groupids`` argument. The authentication policy will return @@ -51,7 +53,6 @@ class TestingConfiguratorMixin(object): This function is most useful when testing code that uses the APIs named :meth:`pyramid.request.Request.has_permission`, :attr:`pyramid.request.Request.authenticated_userid`, - :attr:`pyramid.request.Request.effective_principals`, and :func:`pyramid.security.principals_allowed_by_permission`. .. versionadded:: 1.4 @@ -59,11 +60,14 @@ class TestingConfiguratorMixin(object): .. versionadded:: 1.4 The ``forget_result`` argument. + + .. versionchanged:: 2.0 + Removed ``groupids`` argument and doc about effective principals. """ from pyramid.testing import DummySecurityPolicy policy = DummySecurityPolicy( - identity, permissive, remember_result, forget_result + userid, identity, permissive, remember_result, forget_result ) self.registry.registerUtility(policy, ISecurityPolicy) return policy diff --git a/src/pyramid/interfaces.py b/src/pyramid/interfaces.py index 688293509..11b794e2b 100644 --- a/src/pyramid/interfaces.py +++ b/src/pyramid/interfaces.py @@ -484,18 +484,21 @@ class IViewMapperFactory(Interface): class ISecurityPolicy(Interface): def identify(request): - """ Return an object identifying a trusted and verified user. This - object may be anything, but should implement a ``__str__`` method that - outputs a corresponding :term:`userid`. + """ Return an object identifying a trusted and verified user. + The object may be anything. """ - def permits(request, context, identity, permission): + def authenticated_userid(request, identity): + """ Return a :term:`userid` string identifying the trusted and + verified user, or ``None`` if unauthenticated. + """ + + def permits(request, context, permission): """ Return an instance of :class:`pyramid.security.Allowed` if a user of the given identity is allowed the ``permission`` in the current ``context``, else return an instance of :class:`pyramid.security.Denied`. - """ def remember(request, userid, **kw): @@ -503,13 +506,11 @@ class ISecurityPolicy(Interface): :term:`userid` named ``userid`` when set in a response. An individual authentication policy and its consumers can decide on the composition and meaning of ``**kw``. - """ - def forget(request): + def forget(request, **kw): """ Return a set of headers suitable for 'forgetting' the current user on subsequent requests. - """ diff --git a/src/pyramid/security.py b/src/pyramid/security.py index 08c36b457..053ff5818 100644 --- a/src/pyramid/security.py +++ b/src/pyramid/security.py @@ -82,7 +82,7 @@ def remember(request, userid, **kw): return policy.remember(request, userid, **kw) -def forget(request): +def forget(request, **kw): """ Return a sequence of header tuples (e.g. ``[('Set-Cookie', 'foo=abc')]``) suitable for 'forgetting' the set of credentials @@ -104,7 +104,7 @@ def forget(request): policy = _get_security_policy(request) if policy is None: return [] - return policy.forget(request) + return policy.forget(request, **kw) def principals_allowed_by_permission(context, permission): @@ -293,7 +293,9 @@ class ACLAllowed(ACLPermitsResult, Allowed): """ -class SecurityAPIMixin(object): +class SecurityAPIMixin: + """ Mixin for Request class providing auth-related properties. """ + @property def authenticated_identity(self): """ @@ -315,18 +317,14 @@ class SecurityAPIMixin(object): .. versionchanged:: 2.0 - When using the new security system, this property outputs the - string representation of the :term:`identity`. + This property delegates to the effective :term:`security policy`, + ignoring old-style :term:`authentication policy`. """ - authn = _get_authentication_policy(self) - security = _get_security_policy(self) - if authn is not None: - return authn.authenticated_userid(self) - elif security is not None: - return str(security.identify(self)) - else: + policy = _get_security_policy(self) + if policy is None: return None + return policy.authenticated_userid(self) def has_permission(self, permission, context=None): """ Given a permission and an optional context, returns an instance of @@ -353,11 +351,12 @@ class SecurityAPIMixin(object): policy = _get_security_policy(self) if policy is None: return Allowed('No security policy in use.') - identity = policy.identify(self) - return policy.permits(self, context, identity, permission) + return policy.permits(self, context, permission) class AuthenticationAPIMixin(object): + """ Mixin for Request class providing compatibility properties. """ + @property def unauthenticated_userid(self): """ @@ -365,8 +364,8 @@ class AuthenticationAPIMixin(object): ``unauthenticated_userid`` does not have an equivalent in the new security system. Use :attr:`.authenticated_userid` or - :attr:`.identity` instead. See :ref:`upgrading_auth` for more - information. + :attr:`.authenticated_identity` instead. + See :ref:`upgrading_auth` for more information. Return an object which represents the *claimed* (not verified) user id of the credentials present in the request. ``None`` if there is no @@ -377,14 +376,18 @@ class AuthenticationAPIMixin(object): associated with the userid exists in persistent storage. """ - authn = _get_authentication_policy(self) - security = _get_security_policy(self) - if authn is not None: - return authn.unauthenticated_userid(self) - elif security is not None: - return str(security.identify(self)) - else: + policy = _get_security_policy(self) + if policy is None: return None + return policy.authenticated_userid(self) + + unauthenticated_userid = deprecated( + unauthenticated_userid, + 'The new security policy has removed the concept of unauthenticated ' + 'userid. See https://docs.pylonsproject.org/projects/pyramid/en/latest' + '/whatsnew-2.0.html#upgrading-authentication-authorization ' + 'for more information.', + ) @property def effective_principals(self): @@ -428,7 +431,7 @@ class LegacySecurityPolicy: def _get_authz_policy(self, request): return request.registry.getUtility(IAuthorizationPolicy) - def identify(self, request): + def authenticated_userid(self, request): authn = self._get_authn_policy(request) return authn.authenticated_userid(request) @@ -436,11 +439,12 @@ class LegacySecurityPolicy: authn = self._get_authn_policy(request) return authn.remember(request, userid, **kw) - def forget(self, request): + def forget(self, request, **kw): authn = self._get_authn_policy(request) + # XXX log warning if varkwargs were passed? return authn.forget(request) - def permits(self, request, context, identity, permission): + def permits(self, request, context, permission): authn = self._get_authn_policy(request) authz = self._get_authz_policy(request) principals = authn.effective_principals(request) diff --git a/src/pyramid/testing.py b/src/pyramid/testing.py index 3bf3f1898..316e0bd15 100644 --- a/src/pyramid/testing.py +++ b/src/pyramid/testing.py @@ -42,11 +42,13 @@ class DummySecurityPolicy(object): def __init__( self, + userid=None, identity=None, permissive=True, remember_result=None, forget_result=None, ): + self.userid = None self.identity = identity self.permissive = permissive if remember_result is None: @@ -59,14 +61,17 @@ class DummySecurityPolicy(object): def identify(self, request): return self.identity - def permits(self, request, context, identity, permission): + def authenticated_userid(self, request): + return self.userid + + def permits(self, request, context, permission): return self.permissive def remember(self, request, userid, **kw): self.remembered = userid return self.remember_result - def forget(self, request): + def forget(self, request, **kw): self.forgotten = True return self.forget_result diff --git a/src/pyramid/viewderivers.py b/src/pyramid/viewderivers.py index 35f9a08d2..7c28cbf85 100644 --- a/src/pyramid/viewderivers.py +++ b/src/pyramid/viewderivers.py @@ -316,8 +316,7 @@ def _secured_view(view, info): if policy and (permission is not None): def permitted(context, request): - identity = policy.identify(request) - return policy.permits(request, context, identity, permission) + return policy.permits(request, context, permission) def secured_view(context, request): result = permitted(context, request) @@ -363,10 +362,8 @@ def _authdebug_view(view, info): elif permission is None: msg = 'Allowed (no permission registered)' else: - identity = policy.identify(request) - msg = str( - policy.permits(request, context, identity, permission) - ) + result = policy.permits(request, context, permission) + msg = str(result) else: msg = 'Allowed (no security policy in use)' diff --git a/tests/pkgs/securityapp/__init__.py b/tests/pkgs/securityapp/__init__.py index 6ddba585b..b869ab541 100644 --- a/tests/pkgs/securityapp/__init__.py +++ b/tests/pkgs/securityapp/__init__.py @@ -4,9 +4,13 @@ from pyramid.security import Allowed, Denied class SecurityPolicy: def identify(self, request): + ... + + def authenticated_userid(self, request): return request.environ.get('REMOTE_USER') - def permits(self, request, context, identity, permission): + def permits(self, request, context, permission): + identity = self.identify(request) if identity and permission == 'foo': return Allowed('') else: @@ -15,7 +19,7 @@ class SecurityPolicy: def remember(self, request, userid, **kw): raise NotImplementedError() # pragma: no cover - def forget(self, request): + def forget(self, request, **kw): raise NotImplementedError() # pragma: no cover diff --git a/tests/test_authorization.py b/tests/test_authorization.py index 399b3da60..b50c1ae3b 100644 --- a/tests/test_authorization.py +++ b/tests/test_authorization.py @@ -3,6 +3,9 @@ import unittest from pyramid.testing import cleanUp +# XXX fix all tests to add request, remove principals + + class TestACLAuthorizationPolicy(unittest.TestCase): def setUp(self): cleanUp() diff --git a/tests/test_security.py b/tests/test_security.py index 2a8847f3b..726fbbbf3 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -350,7 +350,7 @@ class TestAuthenticatedUserId(unittest.TestCase): request = _makeRequest() _registerAuthenticationPolicy(request.registry, 'yo') _registerSecurityPolicy(request.registry, 'wat') - self.assertEqual(request.authenticated_userid, 'yo') + self.assertEqual(request.authenticated_userid, 'wat') def test_with_security_policy(self): request = _makeRequest() @@ -511,10 +511,7 @@ class TestLegacySecurityPolicy(unittest.TestCase): _registerAuthenticationPolicy(request.registry, ['p1', 'p2']) _registerAuthorizationPolicy(request.registry, True) - self.assertIs( - policy.permits(request, request.context, 'userid', 'permission'), - True, - ) + self.assertTrue(policy.permits(request, request.context, 'permission')) _TEST_HEADER = 'X-Pyramid-Test' @@ -532,7 +529,10 @@ class DummySecurityPolicy: def identify(self, request): return self.result - def permits(self, request, context, identity, permission): + def authenticated_userid(self, request): + return self.result + + def permits(self, request, context, permission): return self.result def remember(self, request, userid, **kw): @@ -540,7 +540,7 @@ class DummySecurityPolicy: self._header_remembered = headers[0] return headers - def forget(self, request): + def forget(self, request, **kw): headers = [(_TEST_HEADER, 'logout')] self._header_forgotten = headers[0] return headers diff --git a/tests/test_testing.py b/tests/test_testing.py index ebeafe21d..d0f23a116 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -33,7 +33,7 @@ class TestDummySecurityPolicy(unittest.TestCase): def test_permits(self): policy = self._makeOne() - self.assertEqual(policy.permits(None, None, None, None), True) + self.assertTrue(policy.permits(None, None, None)) def test_forget(self): policy = self._makeOne() diff --git a/tests/test_viewderivers.py b/tests/test_viewderivers.py index e47296b50..f1aa00e5b 100644 --- a/tests/test_viewderivers.py +++ b/tests/test_viewderivers.py @@ -2086,7 +2086,10 @@ class DummySecurityPolicy: def identify(self, request): return 123 - def permits(self, request, context, identity, permission): + def authenticated_userid(self, request): + return 123 + + def permits(self, request, context, permission): return self.permitted |