From 0168300b0da3c79e05ec87aa777e04674a86cebb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Araujo?= Date: Sat, 14 Dec 2019 13:32:07 -0500 Subject: start reworking security policy --- .flake8 | 1 - TODO.txt | 4 -- docs/api/request.rst | 18 ++++----- docs/glossary.rst | 4 +- docs/narr/security.rst | 70 ++++++++++++++++++++-------------- docs/quick_tutorial/authentication.rst | 2 + docs/whatsnew-2.0.rst | 10 +++-- src/pyramid/authentication.py | 6 +-- src/pyramid/config/testing.py | 8 +++- src/pyramid/interfaces.py | 17 +++++---- src/pyramid/security.py | 56 ++++++++++++++------------- src/pyramid/testing.py | 9 ++++- src/pyramid/viewderivers.py | 9 ++--- tests/pkgs/securityapp/__init__.py | 8 +++- tests/test_authorization.py | 3 ++ tests/test_security.py | 14 +++---- tests/test_testing.py | 2 +- tests/test_viewderivers.py | 5 ++- 18 files changed, 140 insertions(+), 106 deletions(-) diff --git a/.flake8 b/.flake8 index 998f6ffec..39d486b3a 100644 --- a/.flake8 +++ b/.flake8 @@ -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 diff --git a/TODO.txt b/TODO.txt index 318171931..c66583ba8 100644 --- a/TODO.txt +++ b/TODO.txt @@ -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 ` 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 -- cgit v1.2.3 From 279fbb2524586d8a0ed1d5fa28ca1aced79eb126 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sat, 14 Dec 2019 17:49:44 -0600 Subject: Update docs from Configurator.testing_securitypolicy. --- src/pyramid/config/testing.py | 69 +++++++++++++++++++++---------------------- src/pyramid/testing.py | 2 +- 2 files changed, 34 insertions(+), 37 deletions(-) diff --git a/src/pyramid/config/testing.py b/src/pyramid/config/testing.py index 83a8db552..2cde8fc59 100644 --- a/src/pyramid/config/testing.py +++ b/src/pyramid/config/testing.py @@ -19,41 +19,38 @@ class TestingConfiguratorMixin(object): remember_result=None, forget_result=None, ): - """Unit/integration testing helper: Registers a pair of faux - :app:`Pyramid` security policies: a :term:`authentication - policy` and a :term:`authorization policy`. - - The behavior of the registered :term:`authorization policy` - depends on the ``permissive`` argument. If ``permissive`` is - true, a permissive :term:`authorization policy` is registered; - this policy allows all access. If ``permissive`` is false, a - nonpermissive :term:`authorization policy` is registered; this - policy denies all access. - - ``remember_result``, if provided, should be the result returned by - the ``remember`` method of the faux authentication policy. If it is - not provided (or it is provided, and is ``None``), the default value - ``[]`` (the empty list) will be returned by ``remember``. - - ``forget_result``, if provided, should be the result returned by - the ``forget`` method of the faux authentication policy. If it is - 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 - the userid identifier implied by the ``userid`` argument and - the group ids implied by the ``groupids`` argument when the - :attr:`pyramid.request.Request.authenticated_userid` or - :attr:`pyramid.request.Request.effective_principals` APIs are - used. - - 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`, - :func:`pyramid.security.principals_allowed_by_permission`. + """Unit/integration testing helper. Registers a faux :term:`security + policy`. + + This function is most useful when testing code that uses the security + APIs, such as :meth:`pyramid.request.Request.identity`, + :attr:`pyramid.request.Request.authenticated_userid`, or + :meth:`pyramid.request.Request.has_permission`, + + The behavior of the registered :term:`security policy` depends on the + arguments passed to this method. + + :param userid: If provided, the policy's ``authenticated_userid`` + method will return this value. As a result, + :attr:`pyramid.request.Request.authenticated_userid` will have this + value as well. + :type userid: str + :param identity: If provided, the policy's ``identify`` method will + return this value. As a result, + :attr:`pyramid.request.Request.authenticated_identity`` will have + this value. + :type identity: object + :param permissive: If true, the policy will allow access to any user + for any permission. If false, the policy will deny all access. + :type permissive: bool + :param remember_result: If provided, the policy's ``remember`` method + will return this value. Otherwise, ``remember`` will return an + empty list. + :type remember_result: list + :param forget_result: If provided, the policy's ``forget`` method will + return this value. Otherwise, ``forget`` will return an empty + list. + :type forget_result: list .. versionadded:: 1.4 The ``remember_result`` argument. @@ -62,7 +59,7 @@ class TestingConfiguratorMixin(object): The ``forget_result`` argument. .. versionchanged:: 2.0 - Removed ``groupids`` argument and doc about effective principals. + Removed ``groupids`` argument and add `identity` argument. """ from pyramid.testing import DummySecurityPolicy diff --git a/src/pyramid/testing.py b/src/pyramid/testing.py index 316e0bd15..a1870874e 100644 --- a/src/pyramid/testing.py +++ b/src/pyramid/testing.py @@ -38,7 +38,7 @@ class DummyRootFactory(object): class DummySecurityPolicy(object): - """ A standin for a security policy""" + """ A standin for a :term:`security policy`.""" def __init__( self, -- cgit v1.2.3 From 4af6a51e7edfac4ec88b01f5be97d966742a8757 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sat, 14 Dec 2019 18:00:20 -0600 Subject: Fix tests for `SesssionAuthenticationHelper` --- tests/test_authentication.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_authentication.py b/tests/test_authentication.py index cb2a0a035..e0f5a7963 100644 --- a/tests/test_authentication.py +++ b/tests/test_authentication.py @@ -1706,20 +1706,20 @@ class TestSessionAuthenticationHelper(unittest.TestCase): return SessionAuthenticationHelper(prefix=prefix) - def test_identify(self): + def test_authenticated_userid(self): request = self._makeRequest({'userid': 'fred'}) helper = self._makeOne() - self.assertEqual(helper.identify(request), 'fred') + self.assertEqual(helper.authenticated_userid(request), 'fred') - def test_identify_with_prefix(self): + def test_authenticated_userid_with_prefix(self): request = self._makeRequest({'foo.userid': 'fred'}) helper = self._makeOne(prefix='foo.') - self.assertEqual(helper.identify(request), 'fred') + self.assertEqual(helper.authenticated_userid(request), 'fred') - def test_identify_none(self): + def test_authenticated_userid_none(self): request = self._makeRequest() helper = self._makeOne() - self.assertEqual(helper.identify(request), None) + self.assertEqual(helper.authenticated_userid(request), None) def test_remember(self): request = self._makeRequest() -- cgit v1.2.3 From a7692dbc47a86c8fbf763d095bf567d7e28ab3ff Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sat, 14 Dec 2019 18:02:31 -0600 Subject: Fix security policy integration tests. --- tests/pkgs/securityapp/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/pkgs/securityapp/__init__.py b/tests/pkgs/securityapp/__init__.py index b869ab541..6c9025e7d 100644 --- a/tests/pkgs/securityapp/__init__.py +++ b/tests/pkgs/securityapp/__init__.py @@ -4,14 +4,14 @@ from pyramid.security import Allowed, Denied class SecurityPolicy: def identify(self, request): - ... + raise NotImplementedError() # pragma: no cover def authenticated_userid(self, request): return request.environ.get('REMOTE_USER') def permits(self, request, context, permission): - identity = self.identify(request) - if identity and permission == 'foo': + userid = self.authenticated_userid(request) + if userid and permission == 'foo': return Allowed('') else: return Denied('') -- cgit v1.2.3 From 0f1ef0d4885ab2fd99d1cf2ccc92886c5519f651 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sat, 14 Dec 2019 18:45:21 -0600 Subject: Remove failing tests using threadlocal request. It shoud be okay to remove because threadlocal support was removed from the security implementation. However, *I don't understand why they started failing.* In master, `get_current_registry` returns a registry object, which DummyRequest will fall back on, causing the tests to pass and rendering them useless. On this branch, it returns `None`, causing the tests to fail. I can't find any reason in the diff why this would change. This makes me nervous. --- tests/test_security.py | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/tests/test_security.py b/tests/test_security.py index 726fbbbf3..ca28ec190 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -165,15 +165,6 @@ class TestPrincipalsAllowedByPermission(unittest.TestCase): result = self._callFUT(context, 'view') self.assertEqual(result, [Everyone]) - def test_with_authorization_policy(self): - from pyramid.threadlocal import get_current_registry - - registry = get_current_registry() - _registerAuthorizationPolicy(registry, 'yo') - context = DummyContext() - result = self._callFUT(context, 'view') - self.assertEqual(result, 'yo') - class TestRemember(unittest.TestCase): def setUp(self): @@ -358,15 +349,6 @@ class TestAuthenticatedUserId(unittest.TestCase): _registerSecurityPolicy(request.registry, 123) self.assertEqual(request.authenticated_userid, '123') - def test_with_authentication_policy_no_reg_on_request(self): - from pyramid.threadlocal import get_current_registry - - registry = get_current_registry() - request = _makeRequest() - del request.registry - _registerAuthenticationPolicy(registry, 'yo') - self.assertEqual(request.authenticated_userid, 'yo') - class TestUnAuthenticatedUserId(unittest.TestCase): def setUp(self): @@ -390,15 +372,6 @@ class TestUnAuthenticatedUserId(unittest.TestCase): _registerSecurityPolicy(request.registry, 'yo') self.assertEqual(request.unauthenticated_userid, 'yo') - def test_with_authentication_policy_no_reg_on_request(self): - from pyramid.threadlocal import get_current_registry - - registry = get_current_registry() - request = _makeRequest() - del request.registry - _registerAuthenticationPolicy(registry, 'yo') - self.assertEqual(request.unauthenticated_userid, 'yo') - class TestEffectivePrincipals(unittest.TestCase): def setUp(self): @@ -418,15 +391,6 @@ class TestEffectivePrincipals(unittest.TestCase): _registerAuthenticationPolicy(request.registry, 'yo') self.assertEqual(request.effective_principals, 'yo') - def test_with_authentication_policy_no_reg_on_request(self): - from pyramid.threadlocal import get_current_registry - - registry = get_current_registry() - request = _makeRequest() - del request.registry - _registerAuthenticationPolicy(registry, 'yo') - self.assertEqual(request.effective_principals, 'yo') - class TestHasPermission(unittest.TestCase): def setUp(self): -- cgit v1.2.3 From 495da107e3da765649957a0c03561648ea75e26b Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sat, 14 Dec 2019 18:57:03 -0600 Subject: Correct implementation of Request.unauthenticated_userid. New implementation was not backwards compatible. This brings back the old implementation, except changing to pull from ISecurityPolicy.authenticated_userid when applicable. Also undeprecated the method again. --- src/pyramid/security.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/pyramid/security.py b/src/pyramid/security.py index 053ff5818..838bcebbd 100644 --- a/src/pyramid/security.py +++ b/src/pyramid/security.py @@ -376,18 +376,14 @@ class AuthenticationAPIMixin(object): associated with the userid exists in persistent storage. """ - policy = _get_security_policy(self) - if policy is None: + 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 security.authenticated_userid(self) + else: 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): -- cgit v1.2.3 From fe637b6953eb95137844a4c1153ccc33ab8136e3 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sat, 14 Dec 2019 19:05:36 -0600 Subject: Bring back `identify` to `LegacySecurityPolicy`. --- src/pyramid/security.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/pyramid/security.py b/src/pyramid/security.py index 838bcebbd..f07bc0bfe 100644 --- a/src/pyramid/security.py +++ b/src/pyramid/security.py @@ -427,6 +427,9 @@ class LegacySecurityPolicy: def _get_authz_policy(self, request): return request.registry.getUtility(IAuthorizationPolicy) + def identify(self, request): + return self.authenticated_userid(request) + def authenticated_userid(self, request): authn = self._get_authn_policy(request) return authn.authenticated_userid(request) -- cgit v1.2.3 From eda8787d00b31dc90164e5c233bfb1cc1f94eaed Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sat, 14 Dec 2019 19:05:47 -0600 Subject: Don't test request.authenticated_userid stringifies the result. --- tests/test_security.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_security.py b/tests/test_security.py index ca28ec190..715d99804 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -345,8 +345,7 @@ class TestAuthenticatedUserId(unittest.TestCase): def test_with_security_policy(self): request = _makeRequest() - # Ensure the identity is stringified. - _registerSecurityPolicy(request.registry, 123) + _registerSecurityPolicy(request.registry, '123') self.assertEqual(request.authenticated_userid, '123') -- cgit v1.2.3 From d699c9ace8028bf74e1c6c3ea085fdf9beeb2586 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sat, 14 Dec 2019 19:12:01 -0600 Subject: Raise error on kwargs in `LegacySecurityPolicy.forget`. --- src/pyramid/security.py | 6 +++++- tests/test_security.py | 6 ++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/pyramid/security.py b/src/pyramid/security.py index f07bc0bfe..e3a978c52 100644 --- a/src/pyramid/security.py +++ b/src/pyramid/security.py @@ -439,8 +439,12 @@ class LegacySecurityPolicy: return authn.remember(request, userid, **kw) def forget(self, request, **kw): + if kw: + raise ValueError( + 'Legacy authentication policies do not support keyword ' + 'arguments for `forget`' + ) authn = self._get_authn_policy(request) - # XXX log warning if varkwargs were passed? return authn.forget(request) def permits(self, request, context, permission): diff --git a/tests/test_security.py b/tests/test_security.py index 715d99804..1c969e305 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -466,6 +466,12 @@ class TestLegacySecurityPolicy(unittest.TestCase): policy.forget(request), [('X-Pyramid-Test', 'logout')] ) + def test_forget_with_kwargs(self): + from pyramid.security import LegacySecurityPolicy + + policy = LegacySecurityPolicy() + self.assertRaises(ValueError, lambda: policy.forget(None, foo='bar')) + def test_permits(self): from pyramid.security import LegacySecurityPolicy -- cgit v1.2.3 From 638125171a311b9a6ba7c02da844ca601a2b0e0e Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sat, 14 Dec 2019 19:16:58 -0600 Subject: Fix tests for `DummySecurityPolicy`. --- src/pyramid/testing.py | 2 +- tests/test_config/test_testing.py | 5 +++-- tests/test_testing.py | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/pyramid/testing.py b/src/pyramid/testing.py index a1870874e..a92bb5d03 100644 --- a/src/pyramid/testing.py +++ b/src/pyramid/testing.py @@ -48,7 +48,7 @@ class DummySecurityPolicy(object): remember_result=None, forget_result=None, ): - self.userid = None + self.userid = userid self.identity = identity self.permissive = permissive if remember_result is None: diff --git a/tests/test_config/test_testing.py b/tests/test_config/test_testing.py index 500aedeae..efbe28f66 100644 --- a/tests/test_config/test_testing.py +++ b/tests/test_config/test_testing.py @@ -17,12 +17,13 @@ class TestingConfiguratorMixinTests(unittest.TestCase): from pyramid.testing import DummySecurityPolicy config = self._makeOne(autocommit=True) - config.testing_securitypolicy('user', permissive=False) + config.testing_securitypolicy('userid', 'identity', permissive=False) from pyramid.interfaces import ISecurityPolicy policy = config.registry.getUtility(ISecurityPolicy) self.assertTrue(isinstance(policy, DummySecurityPolicy)) - self.assertEqual(policy.identity, 'user') + self.assertEqual(policy.userid, 'userid') + self.assertEqual(policy.identity, 'identity') self.assertEqual(policy.permissive, False) def test_testing_securitypolicy_remember_result(self): diff --git a/tests/test_testing.py b/tests/test_testing.py index ad1fca1d9..6eb474f65 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -25,7 +25,7 @@ class TestDummySecurityPolicy(unittest.TestCase): def _makeOne(self, identity=None, permissive=True): klass = self._getTargetClass() - return klass(identity, permissive) + return klass(identity, identity, permissive) def test_identify(self): policy = self._makeOne('user') -- cgit v1.2.3 From 6213507e80d1b77ffd314b88c23ff7eca99aaee4 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sat, 14 Dec 2019 19:24:40 -0600 Subject: Fix couple final view tests. --- tests/test_config/test_views.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/test_config/test_views.py b/tests/test_config/test_views.py index a1e975756..a474d3754 100644 --- a/tests/test_config/test_views.py +++ b/tests/test_config/test_views.py @@ -2045,10 +2045,9 @@ class TestViewsConfigurationMixin(unittest.TestCase): outerself.assertEqual(r, request) return 123 - def permits(self, r, context, identity, permission): + def permits(self, r, context, permission): outerself.assertEqual(r, request) outerself.assertEqual(context, None) - outerself.assertEqual(identity, 123) outerself.assertEqual(permission, 'view') return True @@ -2070,10 +2069,9 @@ class TestViewsConfigurationMixin(unittest.TestCase): outerself.assertEqual(r, request) return 123 - def permits(self, r, context, identity, permission): + def permits(self, r, context, permission): outerself.assertEqual(r, request) outerself.assertEqual(context, None) - outerself.assertEqual(identity, 123) outerself.assertEqual(permission, 'view') return True -- cgit v1.2.3 From 491e6e15edb6ca35a33a133c323ba1db2664c05f Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sat, 14 Dec 2019 19:26:42 -0600 Subject: Remove unnecessary TODO statement. --- tests/test_authorization.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/test_authorization.py b/tests/test_authorization.py index b50c1ae3b..399b3da60 100644 --- a/tests/test_authorization.py +++ b/tests/test_authorization.py @@ -3,9 +3,6 @@ import unittest from pyramid.testing import cleanUp -# XXX fix all tests to add request, remove principals - - class TestACLAuthorizationPolicy(unittest.TestCase): def setUp(self): cleanUp() -- cgit v1.2.3 From 2cbb91b80438e6f5ec98d004eb5ac8c1650ad176 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sat, 14 Dec 2019 19:30:21 -0600 Subject: Remove TODO for authentication tutorial. It should be done, but not as part of this PR. I'll open an issue. --- docs/quick_tutorial/authentication.rst | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/quick_tutorial/authentication.rst b/docs/quick_tutorial/authentication.rst index ccdd9b70b..cd038ea36 100644 --- a/docs/quick_tutorial/authentication.rst +++ b/docs/quick_tutorial/authentication.rst @@ -104,8 +104,6 @@ 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 -- cgit v1.2.3 From cd0b92d10bfbb38068c216ce44dde9732fa127a8 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sat, 14 Dec 2019 20:27:30 -0600 Subject: Update docs. --- docs/api/request.rst | 20 +++++------------- docs/narr/security.rst | 54 +++++++++++++++++++---------------------------- docs/whatsnew-2.0.rst | 19 +++++++---------- src/pyramid/interfaces.py | 6 +++--- 4 files changed, 38 insertions(+), 61 deletions(-) diff --git a/docs/api/request.rst b/docs/api/request.rst index fb4b5caee..50703ff63 100644 --- a/docs/api/request.rst +++ b/docs/api/request.rst @@ -166,27 +166,17 @@ .. attribute:: authenticated_userid - .. versionchanged:: 2.0 - - ``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 - policy` in effect or there is no currently authenticated user. This - differs from :attr:`~pyramid.request.Request.unauthenticated_userid`, - because the effective authentication policy will have ensured that a - record associated with the :term:`userid` exists in persistent storage; - if it has not, this value will be ``None``. + authenticated user or ``None`` if there is no :term:`security policy` in + effect or there is no currently authenticated user. .. attribute:: unauthenticated_userid .. 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 deprecated in version 2.0. Use + :attr:`authenticated_userid` or :attr:`authenticated_identity` + instead. 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 diff --git a/docs/narr/security.rst b/docs/narr/security.rst index a71b9abd9..b01bec903 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -32,14 +32,11 @@ how it works at a high level: - A :term:`view callable` is located by :term:`view lookup` using the context as well as other attributes of the request. -- If a :term:`security policy` is in effect, it is passed the request and - returns the :term:`identity` of the current user. - - If a :term:`security policy` is in effect and the :term:`view configuration` associated with the view callable that was found has a - :term:`permission` associated with it, the policy is passed the - :term:`context`, the current :term:`identity`, and the :term:`permission` - associated with the view; it will allow or deny access. + :term:`permission` associated with it, the policy is passed :term:`request`, + the :term:`context`, and the :term:`permission` associated with the view; it + will allow or deny access. - If the security policy allows access, the view callable is invoked. @@ -62,7 +59,7 @@ Writing a Security Policy accessible by completely anonymous users. In order to begin protecting views from execution based on security settings, you need to write a security policy. -Security policies are simple classes implementing a +Security policies are simple classes implementing :class:`pyramid.interfaces.ISecurityPolicy`. A simple security policy might look like the following: @@ -72,15 +69,16 @@ A simple security policy might look like the following: from pyramid.security import Allowed, Denied class SessionSecurityPolicy: - def authenticated_userid(self, request): - """ Return the user ID stored in the session. """ - return request.session.get('userid') - 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) + userid = request.session.get('userid') + if userid is None: + return None + return load_identity_from_db(request, userid) + + def authenticated_userid(self, request): + """ Return a string ID for the user. """ + return self.identify(request).id def permits(self, request, context, permission): """ Allow access to everything if signed in. """ @@ -143,12 +141,12 @@ 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 self.helper.identify(request) + userid = self.helper.authenticated_userid(request) + return load_identity_from_db(request, userid) + + def authenticated_userid(self, request): + return self.identify(request).id def permits(self, request, context, permission): """ Allow access to everything if signed in. """ @@ -164,19 +162,11 @@ For example, our above security policy can leverage these helpers like so: 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. - -.. code-block:: python - :linenos: - - 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 +Helpers are intended to be used with application-specific code. Notice how the +above code takes the userid from the helper and uses it to load the +:term:`identity` from the database. ``authenticated_userid`` pulls the +:term:`userid` from the :term:`identity` in order to guarantee that the user ID +stored in the session exists in the database ("authenticated"). .. index:: single: permissions diff --git a/docs/whatsnew-2.0.rst b/docs/whatsnew-2.0.rst index b5f349166..6b3261284 100644 --- a/docs/whatsnew-2.0.rst +++ b/docs/whatsnew-2.0.rst @@ -40,17 +40,15 @@ The new security policy should implement ``security_policy`` argument of :class:`pyramid.config.Configurator` or :meth:`pyramid.config.Configurator.set_security_policy`. +The policy contains ``authenticated_userid`` and ``remember``, +with the same method signatures as in the legacy authentication policy. It +also contains ``forget``, but now with keyword arguments in the method +signature. + 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, -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 object can be of any shape, such as a simple ID string or an ORM object. The concept of :term:`principals ` has been removed; the ``permits`` method is passed an identity object. This change gives much more @@ -97,9 +95,8 @@ The new :attr:`pyramid.request.Request.authenticated_identity` property will output the same result as :attr:`pyramid.request.Request.authenticated_userid`. If using a security policy, -:attr:`pyramid.request.Request.unauthenticated_userid` and -:attr:`pyramid.request.Request.authenticated_userid` will both return the -string representation of the :term:`identity`. +:attr:`pyramid.request.Request.authenticated_userid` will return the same value +as :attr:`pyramid.request.Request.authenticated_userid`. :attr:`pyramid.request.Request.effective_principals` will always return a one-element list containing the :data:`pyramid.security.Everyone` principal, as there is no equivalent in the new security policy. diff --git a/src/pyramid/interfaces.py b/src/pyramid/interfaces.py index 11b794e2b..891b851ee 100644 --- a/src/pyramid/interfaces.py +++ b/src/pyramid/interfaces.py @@ -484,9 +484,9 @@ class IViewMapperFactory(Interface): class ISecurityPolicy(Interface): def identify(request): - """ Return an object identifying a trusted and verified user. - - The object may be anything. + """ Return an object identifying a trusted and verified user for the + current request. The object can be of any shape, such as a simple ID + string or an ORM object. """ def authenticated_userid(request, identity): -- cgit v1.2.3 From 2e06fa414412688dc3b7e0b422b0fc0b96ec882f Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sat, 14 Dec 2019 20:17:36 -0800 Subject: Bring back identity into permits. --- docs/narr/security.rst | 12 ++++-------- src/pyramid/interfaces.py | 2 +- src/pyramid/security.py | 6 ++++-- src/pyramid/testing.py | 2 +- src/pyramid/viewderivers.py | 9 ++++++--- tests/pkgs/securityapp/__init__.py | 4 ++-- tests/test_config/test_views.py | 6 ++++-- tests/test_security.py | 7 +++++-- tests/test_testing.py | 2 +- tests/test_viewderivers.py | 2 +- 10 files changed, 29 insertions(+), 23 deletions(-) diff --git a/docs/narr/security.rst b/docs/narr/security.rst index b01bec903..07b7fe825 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -80,9 +80,8 @@ A simple security policy might look like the following: """ Return a string ID for the user. """ return self.identify(request).id - def permits(self, request, context, permission): + def permits(self, request, context, identity, permission): """ Allow access to everything if signed in. """ - identity = self.identify(request) if identity is not None: return Allowed('User is signed in.') else: @@ -148,9 +147,8 @@ For example, our above security policy can leverage these helpers like so: def authenticated_userid(self, request): return self.identify(request).id - def permits(self, request, context, permission): + def permits(self, request, context, identity, permission): """ Allow access to everything if signed in. """ - identity = self.identify(request) if identity is not None: return Allowed('User is signed in.') else: @@ -238,9 +236,7 @@ might look like so: from pyramid.security import Allowed, Denied class SecurityPolicy: - def permits(self, request, context, permission): - identity = self.identify(request) - + def permits(self, request, context, identity, permission): if identity is None: return Denied('User is not signed in.') if identity.role == 'admin': @@ -330,7 +326,7 @@ object. An implementation might look like this: from pyramid.authorization import ACLHelper class SecurityPolicy: - def permits(self, request, context, permission): + def permits(self, request, context, identity, permission): principals = [Everyone] if identity is not None: principals.append(Authenticated) diff --git a/src/pyramid/interfaces.py b/src/pyramid/interfaces.py index 891b851ee..d20401028 100644 --- a/src/pyramid/interfaces.py +++ b/src/pyramid/interfaces.py @@ -494,7 +494,7 @@ class ISecurityPolicy(Interface): verified user, or ``None`` if unauthenticated. """ - def permits(request, context, permission): + def permits(request, context, identity, 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 diff --git a/src/pyramid/security.py b/src/pyramid/security.py index e3a978c52..d6af69e51 100644 --- a/src/pyramid/security.py +++ b/src/pyramid/security.py @@ -351,7 +351,9 @@ class SecurityAPIMixin: policy = _get_security_policy(self) if policy is None: return Allowed('No security policy in use.') - return policy.permits(self, context, permission) + return policy.permits( + self, context, self.authenticated_identity, permission + ) class AuthenticationAPIMixin(object): @@ -447,7 +449,7 @@ class LegacySecurityPolicy: authn = self._get_authn_policy(request) return authn.forget(request) - def permits(self, request, context, permission): + def permits(self, request, context, identity, 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 a92bb5d03..f550156dd 100644 --- a/src/pyramid/testing.py +++ b/src/pyramid/testing.py @@ -64,7 +64,7 @@ class DummySecurityPolicy(object): def authenticated_userid(self, request): return self.userid - def permits(self, request, context, permission): + def permits(self, request, context, identity, permission): return self.permissive def remember(self, request, userid, **kw): diff --git a/src/pyramid/viewderivers.py b/src/pyramid/viewderivers.py index 7c28cbf85..35f9a08d2 100644 --- a/src/pyramid/viewderivers.py +++ b/src/pyramid/viewderivers.py @@ -316,7 +316,8 @@ def _secured_view(view, info): if policy and (permission is not None): def permitted(context, request): - return policy.permits(request, context, permission) + identity = policy.identify(request) + return policy.permits(request, context, identity, permission) def secured_view(context, request): result = permitted(context, request) @@ -362,8 +363,10 @@ def _authdebug_view(view, info): elif permission is None: msg = 'Allowed (no permission registered)' else: - result = policy.permits(request, context, permission) - msg = str(result) + identity = policy.identify(request) + msg = str( + policy.permits(request, context, identity, permission) + ) else: msg = 'Allowed (no security policy in use)' diff --git a/tests/pkgs/securityapp/__init__.py b/tests/pkgs/securityapp/__init__.py index 6c9025e7d..caf65ad4c 100644 --- a/tests/pkgs/securityapp/__init__.py +++ b/tests/pkgs/securityapp/__init__.py @@ -4,12 +4,12 @@ from pyramid.security import Allowed, Denied class SecurityPolicy: def identify(self, request): - raise NotImplementedError() # pragma: no cover + return self.authenticated_userid(request) def authenticated_userid(self, request): return request.environ.get('REMOTE_USER') - def permits(self, request, context, permission): + def permits(self, request, context, identity, permission): userid = self.authenticated_userid(request) if userid and permission == 'foo': return Allowed('') diff --git a/tests/test_config/test_views.py b/tests/test_config/test_views.py index a474d3754..a1e975756 100644 --- a/tests/test_config/test_views.py +++ b/tests/test_config/test_views.py @@ -2045,9 +2045,10 @@ class TestViewsConfigurationMixin(unittest.TestCase): outerself.assertEqual(r, request) return 123 - def permits(self, r, context, permission): + def permits(self, r, context, identity, permission): outerself.assertEqual(r, request) outerself.assertEqual(context, None) + outerself.assertEqual(identity, 123) outerself.assertEqual(permission, 'view') return True @@ -2069,9 +2070,10 @@ class TestViewsConfigurationMixin(unittest.TestCase): outerself.assertEqual(r, request) return 123 - def permits(self, r, context, permission): + def permits(self, r, context, identity, permission): outerself.assertEqual(r, request) outerself.assertEqual(context, None) + outerself.assertEqual(identity, 123) outerself.assertEqual(permission, 'view') return True diff --git a/tests/test_security.py b/tests/test_security.py index 1c969e305..3896e008d 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -480,7 +480,10 @@ class TestLegacySecurityPolicy(unittest.TestCase): _registerAuthenticationPolicy(request.registry, ['p1', 'p2']) _registerAuthorizationPolicy(request.registry, True) - self.assertTrue(policy.permits(request, request.context, 'permission')) + self.assertIs( + policy.permits(request, request.context, 'userid', 'permission'), + True, + ) _TEST_HEADER = 'X-Pyramid-Test' @@ -501,7 +504,7 @@ class DummySecurityPolicy: def authenticated_userid(self, request): return self.result - def permits(self, request, context, permission): + def permits(self, request, context, identity, permission): return self.result def remember(self, request, userid, **kw): diff --git a/tests/test_testing.py b/tests/test_testing.py index 6eb474f65..a329b0a04 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.assertTrue(policy.permits(None, None, None)) + self.assertEqual(policy.permits(None, None, None, None), True) def test_forget(self): policy = self._makeOne() diff --git a/tests/test_viewderivers.py b/tests/test_viewderivers.py index f1aa00e5b..48a564c7b 100644 --- a/tests/test_viewderivers.py +++ b/tests/test_viewderivers.py @@ -2089,7 +2089,7 @@ class DummySecurityPolicy: def authenticated_userid(self, request): return 123 - def permits(self, request, context, permission): + def permits(self, request, context, identity, permission): return self.permitted -- cgit v1.2.3 From dc4241edd6d433224f62aece153741f7ea63569a Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sat, 14 Dec 2019 22:46:51 -0800 Subject: Fix coverage. --- tests/test_security.py | 9 +++++++++ tests/test_viewderivers.py | 3 --- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/test_security.py b/tests/test_security.py index 3896e008d..a555fd7f6 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -165,6 +165,15 @@ class TestPrincipalsAllowedByPermission(unittest.TestCase): result = self._callFUT(context, 'view') self.assertEqual(result, [Everyone]) + def test_with_authorization_policy(self): + from pyramid.threadlocal import get_current_registry + + registry = get_current_registry() + _registerAuthorizationPolicy(registry, 'yo') + context = DummyContext() + result = self._callFUT(context, 'view') + self.assertEqual(result, 'yo') + class TestRemember(unittest.TestCase): def setUp(self): diff --git a/tests/test_viewderivers.py b/tests/test_viewderivers.py index 48a564c7b..e47296b50 100644 --- a/tests/test_viewderivers.py +++ b/tests/test_viewderivers.py @@ -2086,9 +2086,6 @@ class DummySecurityPolicy: def identify(self, request): return 123 - def authenticated_userid(self, request): - return 123 - def permits(self, request, context, identity, permission): return self.permitted -- cgit v1.2.3 From 2e55311fec0a539dab87c2ee4b0d7781055dd98a Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sat, 14 Dec 2019 23:21:25 -0800 Subject: Fix coverage. --- tests/test_testing.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/test_testing.py b/tests/test_testing.py index a329b0a04..22bc7332b 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -23,13 +23,17 @@ class TestDummySecurityPolicy(unittest.TestCase): return DummySecurityPolicy - def _makeOne(self, identity=None, permissive=True): + def _makeOne(self, userid=None, identity=None, permissive=True): klass = self._getTargetClass() - return klass(identity, identity, permissive) + return klass(userid, identity, permissive) def test_identify(self): + policy = self._makeOne('user', 'identity') + self.assertEqual(policy.identify(None), 'identity') + + def test_authenticated_userid(self): policy = self._makeOne('user') - self.assertEqual(policy.identify(None), 'user') + self.assertEqual(policy.authenticated_userid(None), 'user') def test_permits(self): policy = self._makeOne() -- cgit v1.2.3 From 8a93507edf5032334b597f47c4e520a2ef08b2d8 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sun, 15 Dec 2019 08:53:04 -0800 Subject: Update docs/narr/security.rst Co-Authored-By: Steve Piercy --- docs/narr/security.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/narr/security.rst b/docs/narr/security.rst index 07b7fe825..aac9eeb7b 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -35,7 +35,7 @@ how it works at a high level: - If a :term:`security policy` is in effect and the :term:`view configuration` associated with the view callable that was found has a :term:`permission` associated with it, the policy is passed :term:`request`, - the :term:`context`, and the :term:`permission` associated with the view; it + the :term:`context`, and the :term:`permission` associated with the view; it will allow or deny access. - If the security policy allows access, the view callable is invoked. -- cgit v1.2.3 From 7b74e97fd156bef6b8f347d7d38615d5bea6c967 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sun, 15 Dec 2019 09:15:12 -0800 Subject: Four spaces of indentation. --- docs/api/request.rst | 10 +++++----- src/pyramid/config/testing.py | 7 ++++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/api/request.rst b/docs/api/request.rst index 50703ff63..9e9c70d3a 100644 --- a/docs/api/request.rst +++ b/docs/api/request.rst @@ -174,9 +174,9 @@ .. deprecated:: 2.0 - ``unauthenticated_userid`` has been deprecated in version 2.0. Use - :attr:`authenticated_userid` or :attr:`authenticated_identity` - instead. See :ref:`upgrading_auth` for more information. + ``unauthenticated_userid`` has been deprecated in version 2.0. Use + :attr:`authenticated_userid` or :attr:`authenticated_identity` + instead. 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 @@ -193,8 +193,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/src/pyramid/config/testing.py b/src/pyramid/config/testing.py index 2cde8fc59..58b239232 100644 --- a/src/pyramid/config/testing.py +++ b/src/pyramid/config/testing.py @@ -53,13 +53,14 @@ class TestingConfiguratorMixin(object): :type forget_result: list .. versionadded:: 1.4 - The ``remember_result`` argument. + The ``remember_result`` argument. .. versionadded:: 1.4 - The ``forget_result`` argument. + The ``forget_result`` argument. .. versionchanged:: 2.0 - Removed ``groupids`` argument and add `identity` argument. + Removed ``groupids`` argument and add `identity` argument. + """ from pyramid.testing import DummySecurityPolicy -- cgit v1.2.3 From 32bf9b3669f2ba0c4a0aaf35f4e2cdad8f9314f0 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sun, 15 Dec 2019 19:55:10 -0800 Subject: Revert "Bring back identity into permits." This reverts commit 2e06fa414412688dc3b7e0b422b0fc0b96ec882f. --- docs/narr/security.rst | 12 ++++++++---- src/pyramid/interfaces.py | 2 +- src/pyramid/security.py | 6 ++---- src/pyramid/testing.py | 2 +- src/pyramid/viewderivers.py | 9 +++------ tests/pkgs/securityapp/__init__.py | 4 ++-- tests/test_config/test_views.py | 6 ++---- tests/test_security.py | 7 ++----- tests/test_testing.py | 2 +- tests/test_viewderivers.py | 2 +- 10 files changed, 23 insertions(+), 29 deletions(-) diff --git a/docs/narr/security.rst b/docs/narr/security.rst index aac9eeb7b..cdc16b6a1 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -80,8 +80,9 @@ A simple security policy might look like the following: """ Return a string ID for the user. """ return self.identify(request).id - 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: @@ -147,8 +148,9 @@ For example, our above security policy can leverage these helpers like so: def authenticated_userid(self, request): return self.identify(request).id - 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: @@ -236,7 +238,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': @@ -326,7 +330,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) diff --git a/src/pyramid/interfaces.py b/src/pyramid/interfaces.py index d20401028..891b851ee 100644 --- a/src/pyramid/interfaces.py +++ b/src/pyramid/interfaces.py @@ -494,7 +494,7 @@ class ISecurityPolicy(Interface): verified user, or ``None`` if unauthenticated. """ - def permits(request, context, identity, permission): + 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 diff --git a/src/pyramid/security.py b/src/pyramid/security.py index d6af69e51..e3a978c52 100644 --- a/src/pyramid/security.py +++ b/src/pyramid/security.py @@ -351,9 +351,7 @@ class SecurityAPIMixin: policy = _get_security_policy(self) if policy is None: return Allowed('No security policy in use.') - return policy.permits( - self, context, self.authenticated_identity, permission - ) + return policy.permits(self, context, permission) class AuthenticationAPIMixin(object): @@ -449,7 +447,7 @@ class LegacySecurityPolicy: authn = self._get_authn_policy(request) 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 f550156dd..a92bb5d03 100644 --- a/src/pyramid/testing.py +++ b/src/pyramid/testing.py @@ -64,7 +64,7 @@ class DummySecurityPolicy(object): def authenticated_userid(self, request): return self.userid - def permits(self, request, context, identity, permission): + def permits(self, request, context, permission): return self.permissive def remember(self, request, userid, **kw): 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 caf65ad4c..6c9025e7d 100644 --- a/tests/pkgs/securityapp/__init__.py +++ b/tests/pkgs/securityapp/__init__.py @@ -4,12 +4,12 @@ from pyramid.security import Allowed, Denied class SecurityPolicy: def identify(self, request): - return self.authenticated_userid(request) + raise NotImplementedError() # pragma: no cover def authenticated_userid(self, request): return request.environ.get('REMOTE_USER') - def permits(self, request, context, identity, permission): + def permits(self, request, context, permission): userid = self.authenticated_userid(request) if userid and permission == 'foo': return Allowed('') diff --git a/tests/test_config/test_views.py b/tests/test_config/test_views.py index a1e975756..a474d3754 100644 --- a/tests/test_config/test_views.py +++ b/tests/test_config/test_views.py @@ -2045,10 +2045,9 @@ class TestViewsConfigurationMixin(unittest.TestCase): outerself.assertEqual(r, request) return 123 - def permits(self, r, context, identity, permission): + def permits(self, r, context, permission): outerself.assertEqual(r, request) outerself.assertEqual(context, None) - outerself.assertEqual(identity, 123) outerself.assertEqual(permission, 'view') return True @@ -2070,10 +2069,9 @@ class TestViewsConfigurationMixin(unittest.TestCase): outerself.assertEqual(r, request) return 123 - def permits(self, r, context, identity, permission): + def permits(self, r, context, permission): outerself.assertEqual(r, request) outerself.assertEqual(context, None) - outerself.assertEqual(identity, 123) outerself.assertEqual(permission, 'view') return True diff --git a/tests/test_security.py b/tests/test_security.py index a555fd7f6..f39e3c730 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -489,10 +489,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' @@ -513,7 +510,7 @@ class DummySecurityPolicy: def authenticated_userid(self, request): return self.result - def permits(self, request, context, identity, permission): + def permits(self, request, context, permission): return self.result def remember(self, request, userid, **kw): diff --git a/tests/test_testing.py b/tests/test_testing.py index 22bc7332b..be519cd15 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -37,7 +37,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..ba10eeaac 100644 --- a/tests/test_viewderivers.py +++ b/tests/test_viewderivers.py @@ -2086,7 +2086,7 @@ class DummySecurityPolicy: def identify(self, request): return 123 - def permits(self, request, context, identity, permission): + def permits(self, request, context, permission): return self.permitted -- cgit v1.2.3 From 11b3c1c7dea42f8290b6a91d5ab292a27d974f3b Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sun, 15 Dec 2019 20:02:36 -0800 Subject: Fix whatsnew. --- docs/whatsnew-2.0.rst | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/docs/whatsnew-2.0.rst b/docs/whatsnew-2.0.rst index 6b3261284..d5f825c43 100644 --- a/docs/whatsnew-2.0.rst +++ b/docs/whatsnew-2.0.rst @@ -94,9 +94,5 @@ normal, as well as all related :class:`pyramid.request.Request` properties. The new :attr:`pyramid.request.Request.authenticated_identity` property will output the same result as :attr:`pyramid.request.Request.authenticated_userid`. -If using a security policy, -:attr:`pyramid.request.Request.authenticated_userid` will return the same value -as :attr:`pyramid.request.Request.authenticated_userid`. -:attr:`pyramid.request.Request.effective_principals` will always return a -one-element list containing the :data:`pyramid.security.Everyone` principal, as -there is no equivalent in the new security policy. +If using a security policy, :attr:`pyramid.request.Request.unauthenticated_userid` will return the same value as :attr:`pyramid.request.Request.authenticated_userid`. +:attr:`pyramid.request.Request.effective_principals` will always return a one-element list containing the :data:`pyramid.security.Everyone` principal, as there is no equivalent in the new security policy. -- cgit v1.2.3 From d79e1dfa0f0f52dbce8ec4a9b08c6ef7740f6dea Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sun, 15 Dec 2019 20:25:02 -0800 Subject: Fix coverage. --- tests/test_viewderivers.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/test_viewderivers.py b/tests/test_viewderivers.py index ba10eeaac..3b5349094 100644 --- a/tests/test_viewderivers.py +++ b/tests/test_viewderivers.py @@ -2083,9 +2083,6 @@ class DummySecurityPolicy: def __init__(self, permitted=True): self.permitted = permitted - def identify(self, request): - return 123 - def permits(self, request, context, permission): return self.permitted -- cgit v1.2.3 From 5f6f7184a997cb2dfa341eef53259d4254a242e8 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sun, 15 Dec 2019 20:27:10 -0800 Subject: Remove requirement that identity is validated. --- docs/glossary.rst | 5 ++--- docs/narr/security.rst | 28 +++++++++++++++++++--------- src/pyramid/interfaces.py | 11 +++++------ 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/docs/glossary.rst b/docs/glossary.rst index 5edc4eeab..8152c7b96 100644 --- a/docs/glossary.rst +++ b/docs/glossary.rst @@ -303,9 +303,8 @@ Glossary request. Oftentimes this is the ID of the user object in a database. identity - An identity is an object identifying the user associated with the - current request. The identity can be any object, but security policies - should ensure that it represents a valid user (not deleted or deactivated). + An identity is an object identifying the user associated with the current request. + The object can be of any shape, such as a simple ID string or an ORM object. 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 cdc16b6a1..60be067bf 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -69,17 +69,21 @@ A simple security policy might look like the following: from pyramid.security import Allowed, Denied class SessionSecurityPolicy: + def authenticated_userid(self, request): + """ Return a string ID for the user. """ + userid = self.identify(request).id + if validate_userid(request, userid): + return userid + else: + return None + def identify(self, request): """ Return app-specific user object. """ - userid = request.session.get('userid') + userid = self.authenticated_userid if userid is None: return None return load_identity_from_db(request, userid) - def authenticated_userid(self, request): - """ Return a string ID for the user. """ - return self.identify(request).id - def permits(self, request, context, permission): """ Allow access to everything if signed in. """ identity = self.identify(request) @@ -141,12 +145,18 @@ For example, our above security policy can leverage these helpers like so: def __init__(self): self.helper = SessionAuthenticationHelper() - def identify(self, request): + def authenticated_userid(self, request): userid = self.helper.authenticated_userid(request) - return load_identity_from_db(request, userid) + if validate_userid(request, userid): + return userid + else: + return None - def authenticated_userid(self, request): - return self.identify(request).id + def identify(self, request): + userid = self.authenticated_userid + if userid is None: + return None + return load_identity_from_db(request, userid) def permits(self, request, context, permission): """ Allow access to everything if signed in. """ diff --git a/src/pyramid/interfaces.py b/src/pyramid/interfaces.py index 891b851ee..c0ff317a4 100644 --- a/src/pyramid/interfaces.py +++ b/src/pyramid/interfaces.py @@ -483,17 +483,16 @@ class IViewMapperFactory(Interface): class ISecurityPolicy(Interface): - def identify(request): - """ Return an object identifying a trusted and verified user for the - current request. The object can be of any shape, such as a simple ID - string or an ORM object. - """ - def authenticated_userid(request, identity): """ Return a :term:`userid` string identifying the trusted and verified user, or ``None`` if unauthenticated. """ + def identify(request): + """ Return the :term:`identity` of the current user. The object can be + of any shape, such as a simple ID string or an ORM object. + """ + 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 -- cgit v1.2.3 From 1b127af1412ce9ae24cc993c15162b48283d76e9 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Sun, 15 Dec 2019 20:42:49 -0800 Subject: Fix coverage. --- tests/test_config/test_views.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/test_config/test_views.py b/tests/test_config/test_views.py index a474d3754..d6b08c2f7 100644 --- a/tests/test_config/test_views.py +++ b/tests/test_config/test_views.py @@ -2041,10 +2041,6 @@ class TestViewsConfigurationMixin(unittest.TestCase): outerself = self class DummyPolicy(object): - def identify(self, r): - outerself.assertEqual(r, request) - return 123 - def permits(self, r, context, permission): outerself.assertEqual(r, request) outerself.assertEqual(context, None) @@ -2065,10 +2061,6 @@ class TestViewsConfigurationMixin(unittest.TestCase): outerself = self class DummyPolicy(object): - def identify(self, r): - outerself.assertEqual(r, request) - return 123 - def permits(self, r, context, permission): outerself.assertEqual(r, request) outerself.assertEqual(context, None) -- cgit v1.2.3 From 03069ff0845c1b85c21119985e8157d54e7ce71c Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Mon, 16 Dec 2019 11:37:45 -0800 Subject: Fix EffectivePrincipalsPredicate deprecation warning. Fired upon registering, not upon use. --- src/pyramid/predicates.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/pyramid/predicates.py b/src/pyramid/predicates.py index a09933253..83f7f3295 100644 --- a/src/pyramid/predicates.py +++ b/src/pyramid/predicates.py @@ -285,6 +285,14 @@ class EffectivePrincipalsPredicate(object): else: self.val = set((val,)) + __init__ = deprecated( + __init__, + 'The new security policy has removed the concept of principals. See ' + 'https://docs.pylonsproject.org/projects/pyramid/en/latest' + '/whatsnew-2.0.html#upgrading-authentication-authorization ' + 'for more information.', + ) + def text(self): return 'effective_principals = %s' % sorted(list(self.val)) @@ -299,15 +307,6 @@ class EffectivePrincipalsPredicate(object): return False -deprecated( - 'EffectivePrincipalsPredicate', - 'The new security policy has removed the concept of principals. See ' - 'https://docs.pylonsproject.org/projects/pyramid/en/latest' - '/whatsnew-2.0.html#upgrading-authentication-authorization ' - 'for more information.', -) - - class Notted(object): def __init__(self, predicate): self.predicate = predicate -- cgit v1.2.3 From 918155824ec9bdd8f7a08c1b0a3e0c56720e9f41 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Mon, 16 Dec 2019 17:30:43 -0800 Subject: Update docs/narr/security.rst code examples. --- docs/narr/security.rst | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/docs/narr/security.rst b/docs/narr/security.rst index 60be067bf..50eeab27b 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -69,21 +69,20 @@ A simple security policy might look like the following: from pyramid.security import Allowed, Denied class SessionSecurityPolicy: - def authenticated_userid(self, request): - """ Return a string ID for the user. """ - userid = self.identify(request).id - if validate_userid(request, userid): - return userid - else: - return None - def identify(self, request): """ Return app-specific user object. """ - userid = self.authenticated_userid + userid = request.session.get('userid') if userid is None: return None return load_identity_from_db(request, userid) + def authenticated_userid(self, request): + """ Return a string ID for the user. """ + identity = request.authenticated_identity + if identity is None: + return None + return string(identity.id) + def permits(self, request, context, permission): """ Allow access to everything if signed in. """ identity = self.identify(request) @@ -145,19 +144,20 @@ For example, our above security policy can leverage these helpers like so: def __init__(self): self.helper = SessionAuthenticationHelper() - def authenticated_userid(self, request): - userid = self.helper.authenticated_userid(request) - if validate_userid(request, userid): - return userid - else: - return None - def identify(self, request): - userid = self.authenticated_userid + """ Return app-specific user object. """ + userid = self.helper.authenticated_userid(request) if userid is None: return None return load_identity_from_db(request, userid) + def authenticated_userid(self, request): + """ Return a string ID for the user. """ + identity = request.authenticated_identity + if identity is None: + return None + return str(identity.id) + def permits(self, request, context, permission): """ Allow access to everything if signed in. """ identity = self.identify(request) -- cgit v1.2.3 From 965dd8295fc9fade649ca61b899811492a0dd2f6 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Tue, 17 Dec 2019 11:28:51 -0800 Subject: Remove `identity` from authenticated_userid interface. --- src/pyramid/interfaces.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pyramid/interfaces.py b/src/pyramid/interfaces.py index c0ff317a4..af3c61503 100644 --- a/src/pyramid/interfaces.py +++ b/src/pyramid/interfaces.py @@ -483,7 +483,7 @@ class IViewMapperFactory(Interface): class ISecurityPolicy(Interface): - def authenticated_userid(request, identity): + def authenticated_userid(request): """ Return a :term:`userid` string identifying the trusted and verified user, or ``None`` if unauthenticated. """ -- cgit v1.2.3 From 3b34a67aa3916a766369220a0185b648b9513489 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Tue, 17 Dec 2019 11:32:16 -0800 Subject: Improve docs for remember/forget. --- src/pyramid/interfaces.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/pyramid/interfaces.py b/src/pyramid/interfaces.py index af3c61503..c4160cc2b 100644 --- a/src/pyramid/interfaces.py +++ b/src/pyramid/interfaces.py @@ -502,14 +502,15 @@ class ISecurityPolicy(Interface): def remember(request, userid, **kw): """ Return a set of headers suitable for 'remembering' the - :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``. + :term:`userid` named ``userid`` when set in a response. An individual + security policy and its consumers can decide on the composition and + meaning of ``**kw``. """ def forget(request, **kw): """ Return a set of headers suitable for 'forgetting' the - current user on subsequent requests. + current user on subsequent requests. An individual security policy and + its consumers can decide on the composition and meaning of ``**kw``. """ -- cgit v1.2.3 From 014b5ecedec76fd7bbade6c954adce94954bd171 Mon Sep 17 00:00:00 2001 From: Theron Luhn Date: Tue, 17 Dec 2019 11:33:18 -0800 Subject: Use `self.identify` instead of `request.authenticated_identity` --- docs/narr/security.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/narr/security.rst b/docs/narr/security.rst index 50eeab27b..ac64cba0a 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -78,7 +78,7 @@ A simple security policy might look like the following: def authenticated_userid(self, request): """ Return a string ID for the user. """ - identity = request.authenticated_identity + identity = self.identify(request) if identity is None: return None return string(identity.id) @@ -153,7 +153,7 @@ For example, our above security policy can leverage these helpers like so: def authenticated_userid(self, request): """ Return a string ID for the user. """ - identity = request.authenticated_identity + identity = self.identify(request) if identity is None: return None return str(identity.id) -- cgit v1.2.3 From c52cfec057a9e22b794da655eac1708dfba8bef7 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 23 Dec 2019 13:31:54 -0600 Subject: modify deprecation warning --- src/pyramid/config/routes.py | 11 +++++++++++ src/pyramid/config/views.py | 11 +++++++++++ src/pyramid/predicates.py | 18 ------------------ 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/pyramid/config/routes.py b/src/pyramid/config/routes.py index 4b26b7481..daef8e9f2 100644 --- a/src/pyramid/config/routes.py +++ b/src/pyramid/config/routes.py @@ -332,6 +332,17 @@ class RoutesConfiguratorMixin(object): stacklevel=3, ) + if 'effective_principals' in predicates: + warnings.warn( + ( + 'The new security policy has removed the concept of ' + 'principals. See "Upgrading Authentication/Authorization" ' + 'in "What\'s New in Pyramid 2.0" for more information.' + ), + DeprecationWarning, + stacklevel=3, + ) + if accept is not None: if not is_nonstr_iter(accept): accept = [accept] diff --git a/src/pyramid/config/views.py b/src/pyramid/config/views.py index bc0b05a08..1c17d2343 100644 --- a/src/pyramid/config/views.py +++ b/src/pyramid/config/views.py @@ -791,6 +791,17 @@ class ViewsConfiguratorMixin(object): stacklevel=4, ) + if 'effective_principals' in view_options: + warnings.warn( + ( + 'The new security policy has removed the concept of ' + 'principals. See "Upgrading Authentication/Authorization" ' + 'in "What\'s New in Pyramid 2.0" for more information.' + ), + DeprecationWarning, + stacklevel=4, + ) + if accept is not None: if is_nonstr_iter(accept): raise ConfigurationError( diff --git a/src/pyramid/predicates.py b/src/pyramid/predicates.py index 83f7f3295..32c6a4089 100644 --- a/src/pyramid/predicates.py +++ b/src/pyramid/predicates.py @@ -1,7 +1,5 @@ import re -from zope.deprecation import deprecated - from pyramid.exceptions import ConfigurationError from pyramid.traversal import ( @@ -271,28 +269,12 @@ class PhysicalPathPredicate(object): class EffectivePrincipalsPredicate(object): - """ - .. deprecated:: 2.0 - - The new security system has removed the concept of principals. See - :ref:`upgrading_auth` for more information. - - """ - def __init__(self, val, config): if is_nonstr_iter(val): self.val = set(val) else: self.val = set((val,)) - __init__ = deprecated( - __init__, - 'The new security policy has removed the concept of principals. See ' - 'https://docs.pylonsproject.org/projects/pyramid/en/latest' - '/whatsnew-2.0.html#upgrading-authentication-authorization ' - 'for more information.', - ) - def text(self): return 'effective_principals = %s' % sorted(list(self.val)) -- cgit v1.2.3 From 570243fcf3f9bb7b3da78404b0598011791ac882 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 23 Dec 2019 13:57:14 -0600 Subject: add coverage tests for deprecation warnings --- tests/test_config/test_routes.py | 11 +++++++++++ tests/test_config/test_views.py | 11 +++++++++++ 2 files changed, 22 insertions(+) diff --git a/tests/test_config/test_routes.py b/tests/test_config/test_routes.py index 4ff67cf66..423da5834 100644 --- a/tests/test_config/test_routes.py +++ b/tests/test_config/test_routes.py @@ -1,4 +1,5 @@ import unittest +import warnings from . import dummyfactory from . import DummyContext @@ -308,6 +309,16 @@ class RoutesConfiguratorMixinTests(unittest.TestCase): else: # pragma: no cover raise AssertionError + def test_add_route_effective_principals_deprecated(self): + config = self._makeOne(autocommit=True) + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter('always', DeprecationWarning) + config.add_route('foo', '/bar', effective_principals=['any']) + self.assertIn( + 'removed the concept of principals', str(w[-1].message) + ) + class DummyRequest: subpath = () diff --git a/tests/test_config/test_views.py b/tests/test_config/test_views.py index d6b08c2f7..dcea02b1d 100644 --- a/tests/test_config/test_views.py +++ b/tests/test_config/test_views.py @@ -1,5 +1,6 @@ import os import unittest +import warnings from zope.interface import implementer from pyramid import testing @@ -2925,6 +2926,16 @@ class TestViewsConfigurationMixin(unittest.TestCase): weighs_more_than='text/plain;charset=utf8', ) + def test_effective_principals_deprecated(self): + config = self._makeOne(autocommit=True) + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter('always', DeprecationWarning) + config.add_view(lambda: None, effective_principals=['any']) + self.assertIn( + 'removed the concept of principals', str(w[-1].message) + ) + class Test_runtime_exc_view(unittest.TestCase): def _makeOne(self, view1, view2): -- cgit v1.2.3