Merge pull request #3545 from luhn/authenticated-userid

Security policy changes

23 files changed, 251 insertions, 247 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..9e9c70d3a 100644
--- a/docs/api/request.rst
+++ b/docs/api/request.rst
@@ -166,27 +166,17 @@
 
    .. attribute:: authenticated_userid
 
-      .. deprecated:: 2.0
-
-          ``authenticated_userid`` has been replaced by
-          :attr:`authenticated_identity` in the new security system.  See
-          :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/glossary.rst b/docs/glossary.rst
index 81358e688..5a33ff39d 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 should implement a
-     ``__str__`` method that outputs a corresponding :term:`userid`.
+      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 f1bb37c69..ac64cba0a 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:
 
@@ -73,11 +70,22 @@ A simple security policy might look like the following:
 
     class SessionSecurityPolicy:
         def identify(self, request):
-            """ Return the user ID stored in the session. """
-            return request.session.get('userid')
+            """ Return app-specific user object. """
+            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 = self.identify(request)
+            if identity is None:
+                return None
+            return string(identity.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:
@@ -87,7 +95,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 []
 
@@ -137,11 +145,22 @@ For example, our above security policy can leverage these helpers like so:
             self.helper = SessionAuthenticationHelper()
 
         def identify(self, request):
-            """ Return the user ID stored in the session. """
-            return self.helper.identify(request)
+            """ 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 = self.identify(request)
+            if identity is None:
+                return None
+            return str(identity.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:
@@ -150,22 +169,14 @@ 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.
-
-.. 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
+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
@@ -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/whatsnew-2.0.rst b/docs/whatsnew-2.0.rst
index ec506894e..d5f825c43 100644
--- a/docs/whatsnew-2.0.rst
+++ b/docs/whatsnew-2.0.rst
@@ -40,15 +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,
-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`.
-(: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 <principal>` has been removed; the
 ``permits`` method is passed an identity object.  This change gives much more
@@ -94,10 +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.unauthenticated_userid` and
-:attr:`pyramid.request.Request.authenticated_userid` will both return the
-string representation of the :term:`identity`.
-: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.
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)'
 
diff --git a/tests/pkgs/securityapp/__init__.py b/tests/pkgs/securityapp/__init__.py
index 6ddba585b..6c9025e7d 100644
--- a/tests/pkgs/securityapp/__init__.py
+++ b/tests/pkgs/securityapp/__init__.py
@@ -4,10 +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, identity, permission):
-        if identity and permission == 'foo':
+    def permits(self, request, context, permission):
+        userid = self.authenticated_userid(request)
+        if userid and permission == 'foo':
             return Allowed('')
         else:
             return Denied('')
@@ -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_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()
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_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_config/test_views.py b/tests/test_config/test_views.py
index baa87dd6b..d133aedbd 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
@@ -2041,14 +2042,9 @@ class TestViewsConfigurationMixin(unittest.TestCase):
         outerself = self
 
         class DummyPolicy(object):
-            def identify(self, r):
-                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
 
@@ -2066,14 +2062,9 @@ class TestViewsConfigurationMixin(unittest.TestCase):
         outerself = self
 
         class DummyPolicy(object):
-            def identify(self, r):
-                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
 
@@ -2935,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):
diff --git a/tests/test_security.py b/tests/test_security.py
index 2a8847f3b..f39e3c730 100644
--- a/tests/test_security.py
+++ b/tests/test_security.py
@@ -350,23 +350,13 @@ 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()
-        # Ensure the identity is stringified.
-        _registerSecurityPolicy(request.registry, 123)
+        _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 +380,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 +399,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):
@@ -503,6 +475,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
 
@@ -511,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'
@@ -532,7 +507,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 +518,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 d0e974a58..be519cd15 100644
--- a/tests/test_testing.py
+++ b/tests/test_testing.py
@@ -23,17 +23,21 @@ 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, 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()
-        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..3b5349094 100644
--- a/tests/test_viewderivers.py
+++ b/tests/test_viewderivers.py
@@ -2083,10 +2083,7 @@ class DummySecurityPolicy:
     def __init__(self, permitted=True):
         self.permitted = permitted
 
-    def identify(self, request):
-        return 123
-
-    def permits(self, request, context, identity, permission):
+    def permits(self, request, context, permission):
         return self.permitted