Merge pull request #3545 from luhn/authenticated-userid

Security policy changes

9 files changed, 116 insertions, 101 deletions

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/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/testing.py b/src/pyramid/config/testing.py
index 21c622656..58b239232 100644
--- a/src/pyramid/config/testing.py
+++ b/src/pyramid/config/testing.py
@@ -13,57 +13,59 @@ class TestingConfiguratorMixin(object):
     # testing API
     def testing_securitypolicy(
         self,
+        userid=None,
         identity=None,
         permissive=True,
         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``.
-
-        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`,
-        :attr:`pyramid.request.Request.effective_principals`, and
-        :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.
+            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.
+
         """
         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/config/views.py b/src/pyramid/config/views.py
index 3071de1e5..324462d1a 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/interfaces.py b/src/pyramid/interfaces.py
index 688293509..c4160cc2b 100644
--- a/src/pyramid/interfaces.py
+++ b/src/pyramid/interfaces.py
@@ -483,33 +483,34 @@ 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`.
+    def authenticated_userid(request):
+        """ 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, 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
         :class:`pyramid.security.Denied`.
-
         """
 
     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):
+    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``.
         """
 
 
diff --git a/src/pyramid/predicates.py b/src/pyramid/predicates.py
index a09933253..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,14 +269,6 @@ 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)
@@ -299,15 +289,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
diff --git a/src/pyramid/security.py b/src/pyramid/security.py
index 08c36b457..e3a978c52 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
@@ -382,7 +381,7 @@ class AuthenticationAPIMixin(object):
         if authn is not None:
             return authn.unauthenticated_userid(self)
         elif security is not None:
-            return str(security.identify(self))
+            return security.authenticated_userid(self)
         else:
             return None
 
@@ -429,6 +428,9 @@ class LegacySecurityPolicy:
         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)
 
@@ -436,11 +438,16 @@ class LegacySecurityPolicy:
         authn = self._get_authn_policy(request)
         return authn.remember(request, userid, **kw)
 
-    def forget(self, request):
+    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)
         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..a92bb5d03 100644
--- a/src/pyramid/testing.py
+++ b/src/pyramid/testing.py
@@ -38,15 +38,17 @@ class DummyRootFactory(object):
 
 
 class DummySecurityPolicy(object):
-    """ A standin for a security policy"""
+    """ A standin for a :term:`security policy`."""
 
     def __init__(
         self,
+        userid=None,
         identity=None,
         permissive=True,
         remember_result=None,
         forget_result=None,
     ):
+        self.userid = userid
         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)'