Merge pull request #3545 from luhn/authenticated-userid

Security policy changes

8 files changed, 63 insertions, 67 deletions

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