From baa59284e1ee26284ecb68d957d5607e40ed786f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Araujo?= Date: Mon, 22 Jun 2020 16:33:13 -0400 Subject: [PATCH 1/3] first shot for #1602 --- docs/narr/advanced-features.rst | 2 +- docs/narr/viewconfig.rst | 8 ++++++ src/pyramid/config/routes.py | 7 +++++ src/pyramid/config/views.py | 7 +++++ src/pyramid/interfaces.py | 12 +++++++++ src/pyramid/predicates.py | 13 ++++++++++ src/pyramid/security.py | 5 ++++ tests/test_config/test_predicates.py | 38 ++++++++++++++++++++++++---- 8 files changed, 86 insertions(+), 6 deletions(-) diff --git a/docs/narr/advanced-features.rst b/docs/narr/advanced-features.rst index 6e819ff5be..6f3b4e11b8 100644 --- a/docs/narr/advanced-features.rst +++ b/docs/narr/advanced-features.rst @@ -34,7 +34,7 @@ For our example above, you can do this instead: .. code-block:: python :linenos: - @view_config(route_name="items", effective_principals=pyramid.authorization.Authenticated) + @view_config(route_name="items", is_authenticated=True) def auth_view(request): # do one thing diff --git a/docs/narr/viewconfig.rst b/docs/narr/viewconfig.rst index b43ebb93ee..c60b7b50d1 100644 --- a/docs/narr/viewconfig.rst +++ b/docs/narr/viewconfig.rst @@ -494,6 +494,12 @@ configured view. .. versionadded:: 1.4a3 +``is_authenticated`` + + XXX doc doc + + .. versionadded:: 2.0 + ``effective_principals`` If specified, this value should be a :term:`principal` identifier or a sequence of principal identifiers. If the @@ -505,6 +511,8 @@ configured view. .. versionadded:: 1.4a4 + .. deprecated:: TODO add + ``custom_predicates`` If ``custom_predicates`` is specified, it must be a sequence of references to custom predicate callables. Custom predicates can be combined with diff --git a/src/pyramid/config/routes.py b/src/pyramid/config/routes.py index a12e18fa84..0fbfcca0c3 100644 --- a/src/pyramid/config/routes.py +++ b/src/pyramid/config/routes.py @@ -268,6 +268,12 @@ def add_route( Removed support for media ranges. + is_authenticated + + XXX doc doc + + .. versionadded:: 2.0 + effective_principals If specified, this value should be a :term:`principal` identifier or @@ -537,6 +543,7 @@ def add_default_route_predicates(self): ('request_param', p.RequestParamPredicate), ('header', p.HeaderPredicate), ('accept', p.AcceptPredicate), + ('is_authenticated', p.IsAuthenticatedPredicate), ('effective_principals', p.EffectivePrincipalsPredicate), ('custom', p.CustomPredicate), ('traverse', p.TraversePredicate), diff --git a/src/pyramid/config/views.py b/src/pyramid/config/views.py index a064ebd05f..87f2cbcd70 100644 --- a/src/pyramid/config/views.py +++ b/src/pyramid/config/views.py @@ -712,6 +712,12 @@ def wrapper(context, request): .. versionadded:: 1.4a3 + is_authenticated + + XXX doc doc + + ..versionadded:: 2.0 + effective_principals If specified, this value should be a :term:`principal` identifier or @@ -1205,6 +1211,7 @@ def add_default_view_predicates(self): ('request_type', p.RequestTypePredicate), ('match_param', p.MatchParamPredicate), ('physical_path', p.PhysicalPathPredicate), + ('is_authenticated', p.IsAuthenticatedPredicate), ('effective_principals', p.EffectivePrincipalsPredicate), ('custom', p.CustomPredicate), ): diff --git a/src/pyramid/interfaces.py b/src/pyramid/interfaces.py index e92662f11d..85539c2f2f 100644 --- a/src/pyramid/interfaces.py +++ b/src/pyramid/interfaces.py @@ -113,6 +113,14 @@ def app_iter_range(start, stop): """ Return a new app_iter built from the response app_iter that serves up only the given start:stop range. """ + authenticated_identity = Attribute( + """XXX Doc doc""" + ) + + authenticated_userid = Attribute( + """XXX Doc doc""" + ) + body = Attribute( """The body of the response, as a str. This will read in the entire app_iter if necessary.""" @@ -233,6 +241,10 @@ def encode_content(encoding='gzip', lazy=False): headers = Attribute(""" The headers in a dictionary-like object """) + is_authenticated = Attribute( + """XXX doc doc""" + ) + last_modified = Attribute( """ Gets and sets and deletes the Last-Modified header. For more information on Last-Modified see RFC 2616 section 14.29. Converts diff --git a/src/pyramid/predicates.py b/src/pyramid/predicates.py index 576bbbce68..fe8bc228c8 100644 --- a/src/pyramid/predicates.py +++ b/src/pyramid/predicates.py @@ -276,6 +276,19 @@ def __call__(self, context, request): return False +class IsAuthenticatedPredicate: + def __init__(self, val, config): + self.val = val + + def text(self): + return "is_authenticated = %r" % (self.val,) + + phash = text + + def __call__(self, context, request): + return request.is_authenticated == self.val + + class EffectivePrincipalsPredicate: def __init__(self, val, config): if is_nonstr_iter(val): diff --git a/src/pyramid/security.py b/src/pyramid/security.py index 58bc721168..356286407c 100644 --- a/src/pyramid/security.py +++ b/src/pyramid/security.py @@ -244,6 +244,11 @@ def authenticated_userid(self): return None return policy.authenticated_userid(self) + @property + def is_authenticated(self): + """Return True if a user is authenticated for this request.""" + return self.authenticated_identity is not None + def has_permission(self, permission, context=None): """ Given a permission and an optional context, returns an instance of :data:`pyramid.security.Allowed` if the permission is granted to this diff --git a/tests/test_config/test_predicates.py b/tests/test_config/test_predicates.py index 8017fc8982..6797e71bcf 100644 --- a/tests/test_config/test_predicates.py +++ b/tests/test_config/test_predicates.py @@ -19,6 +19,7 @@ def _makeOne(self): ('containment', predicates.ContainmentPredicate), ('request_type', predicates.RequestTypePredicate), ('match_param', predicates.MatchParamPredicate), + ('is_authenticated', predicates.IsAuthenticatedPredicate), ('custom', predicates.CustomPredicate), ('traverse', predicates.TraversePredicate), ): @@ -38,6 +39,19 @@ def test_ordering_xhr_and_request_method_trump_only_containment(self): def test_ordering_number_of_predicates(self): from pyramid.config.predicates import predvalseq + order0, _, _ = self._callFUT( + xhr='xhr', + request_method='request_method', + path_info='path_info', + request_param='param', + match_param='foo=bar', + header='header', + accept='accept', + is_authenticated=True, + containment='containment', + request_type='request_type', + custom=predvalseq([DummyCustomPredicate()]), + ) order1, _, _ = self._callFUT( xhr='xhr', request_method='request_method', @@ -121,6 +135,7 @@ def test_ordering_number_of_predicates(self): ) order11, _, _ = self._callFUT(xhr='xhr') order12, _, _ = self._callFUT() + self.assertTrue(order1 > order0) self.assertEqual(order1, order2) self.assertTrue(order3 > order2) self.assertTrue(order4 > order3) @@ -131,7 +146,7 @@ def test_ordering_number_of_predicates(self): self.assertTrue(order9 > order8) self.assertTrue(order10 > order9) self.assertTrue(order11 > order10) - self.assertTrue(order12 > order10) + self.assertTrue(order12 > order11) def test_ordering_importance_of_predicates(self): from pyramid.config.predicates import predvalseq @@ -145,7 +160,8 @@ def test_ordering_importance_of_predicates(self): order7, _, _ = self._callFUT(containment='containment') order8, _, _ = self._callFUT(request_type='request_type') order9, _, _ = self._callFUT(match_param='foo=bar') - order10, _, _ = self._callFUT( + order10, _, _ = self._callFUT(is_authenticated=True) + order11, _, _ = self._callFUT( custom=predvalseq([DummyCustomPredicate()]) ) self.assertTrue(order1 > order2) @@ -157,6 +173,7 @@ def test_ordering_importance_of_predicates(self): self.assertTrue(order7 > order8) self.assertTrue(order8 > order9) self.assertTrue(order9 > order10) + self.assertTrue(order10 > order11) def test_ordering_importance_and_number(self): from pyramid.config.predicates import predvalseq @@ -296,6 +313,7 @@ def test_predicate_text_is_correct(self): ] ), match_param='foo=bar', + is_authenticated=False, ) self.assertEqual(predicates[0].text(), 'xhr = True') self.assertEqual( @@ -308,9 +326,10 @@ def test_predicate_text_is_correct(self): self.assertEqual(predicates[6].text(), 'containment = containment') self.assertEqual(predicates[7].text(), 'request_type = request_type') self.assertEqual(predicates[8].text(), "match_param foo=bar") - self.assertEqual(predicates[9].text(), 'custom predicate') - self.assertEqual(predicates[10].text(), 'classmethod predicate') - self.assertTrue(predicates[11].text().startswith('custom predicate')) + self.assertEqual(predicates[9].text(), "is_authenticated = False") + self.assertEqual(predicates[10].text(), 'custom predicate') + self.assertEqual(predicates[11].text(), 'classmethod predicate') + self.assertTrue(predicates[12].text().startswith('custom predicate')) def test_predicate_text_is_correct_when_multiple(self): _, predicates, _ = self._callFUT( @@ -434,6 +453,15 @@ def test_header_multiple_mixed_fails(self): request.headers = {'foo': 'nobar', 'spamme': 'ham'} self.assertFalse(predicates[0](Dummy(), request)) + def test_is_authenticated_true_matches(self): + ... + def test_is_authenticated_true_fails(self): + ... + def test_is_authenticated_false_matches(self): + ... + def test_is_authenticated_false_fails(self): + ... + def test_unknown_predicate(self): from pyramid.exceptions import ConfigurationError From 70a23ba6e872ae03988caa322f8dd2b03770515c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Araujo?= Date: Wed, 1 Jul 2020 18:33:12 -0400 Subject: [PATCH 2/3] add tests and docs --- CHANGES.rst | 5 +++- docs/narr/viewconfig.rst | 8 ++++-- src/pyramid/config/routes.py | 7 ++++- src/pyramid/config/views.py | 6 ++++- src/pyramid/interfaces.py | 9 ++++--- src/pyramid/security.py | 2 +- tests/test_config/test_predicates.py | 23 +++++++++++++--- tests/test_config/test_routes.py | 12 +++++++++ tests/test_config/test_views.py | 40 ++++++++++++++++++++++++++++ tests/test_security.py | 23 ++++++++++++++++ 10 files changed, 122 insertions(+), 13 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 6b6e1ebbb7..753997bf48 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -22,10 +22,13 @@ Features - ``pyramid.config.Configurator.set_security_policy``. - ``pyramid.interfaces.ISecurityPolicy`` - ``pyramid.request.Request.authenticated_identity``. + - ``pyramid.request.Request.is_authenticated`` - ``pyramid.authentication.SessionAuthenticationHelper`` - ``pyramid.authorization.ACLHelper`` + - ``is_authenticated=True/False`` predicate for route and view configs - See https://github.com/Pylons/pyramid/pull/3465 + See https://github.com/Pylons/pyramid/pull/3465 and + https://github.com/Pylons/pyramid/pull/3598 - Changed the default ``serializer`` on ``pyramid.session.SignedCookieSessionFactory`` to use diff --git a/docs/narr/viewconfig.rst b/docs/narr/viewconfig.rst index c60b7b50d1..2dd5bfb9e3 100644 --- a/docs/narr/viewconfig.rst +++ b/docs/narr/viewconfig.rst @@ -496,7 +496,11 @@ configured view. ``is_authenticated`` - XXX doc doc + This value, if specified, must be either ``True`` or ``False``. If it is + specified and is ``True``, the request must be for an authenticated user, + as determined by the :term:`security policy` in use. If it is specified and + ``False``, the associated view callable will be invoked only if the request + does not have an authenticated user. .. versionadded:: 2.0 @@ -511,7 +515,7 @@ configured view. .. versionadded:: 1.4a4 - .. deprecated:: TODO add + .. deprecated:: 2.0 ``custom_predicates`` If ``custom_predicates`` is specified, it must be a sequence of references to diff --git a/src/pyramid/config/routes.py b/src/pyramid/config/routes.py index 0fbfcca0c3..feb28c7a78 100644 --- a/src/pyramid/config/routes.py +++ b/src/pyramid/config/routes.py @@ -270,7 +270,12 @@ def add_route( is_authenticated - XXX doc doc + This value, if specified, should be either ``True`` or ``False``. + If it is specified and is ``True``, the route will only match if + the request has an authenticated user, as determined by the + :term:`security policy` in use. If it is specified and ``False``, + the route will only match if the request does not have an + authenticated user. .. versionadded:: 2.0 diff --git a/src/pyramid/config/views.py b/src/pyramid/config/views.py index 87f2cbcd70..4a5723a143 100644 --- a/src/pyramid/config/views.py +++ b/src/pyramid/config/views.py @@ -714,7 +714,11 @@ def wrapper(context, request): is_authenticated - XXX doc doc + This value, if specified, should be either ``True`` or ``False``. + If it is specified and is ``True``, the request must be for an + authenticated user, as determined by the :term:`security policy` in + use. If it is specified and ``False``, the associated view callable + will match only if the request does not have an authenticated user. ..versionadded:: 2.0 diff --git a/src/pyramid/interfaces.py b/src/pyramid/interfaces.py index 85539c2f2f..b8c8d06a9c 100644 --- a/src/pyramid/interfaces.py +++ b/src/pyramid/interfaces.py @@ -114,11 +114,13 @@ def app_iter_range(start, stop): serves up only the given start:stop range. """ authenticated_identity = Attribute( - """XXX Doc doc""" + """An object representing the authenticated user, as determined by + the security policy in use, or ``None`` for unauthenticated requests. + The object's class and meaning is defined by the security policy.""" ) authenticated_userid = Attribute( - """XXX Doc doc""" + """A string to identify the authenticated user or ``None``.""" ) body = Attribute( @@ -242,7 +244,8 @@ def encode_content(encoding='gzip', lazy=False): headers = Attribute(""" The headers in a dictionary-like object """) is_authenticated = Attribute( - """XXX doc doc""" + """A boolean indicating whether the request has an authenticated + user, as determined by the security policy in use.""" ) last_modified = Attribute( diff --git a/src/pyramid/security.py b/src/pyramid/security.py index 356286407c..2a1ef24bd2 100644 --- a/src/pyramid/security.py +++ b/src/pyramid/security.py @@ -246,7 +246,7 @@ def authenticated_userid(self): @property def is_authenticated(self): - """Return True if a user is authenticated for this request.""" + """Return ``True`` if a user is authenticated for this request.""" return self.authenticated_identity is not None def has_permission(self, permission, context=None): diff --git a/tests/test_config/test_predicates.py b/tests/test_config/test_predicates.py index 6797e71bcf..7e2f32786a 100644 --- a/tests/test_config/test_predicates.py +++ b/tests/test_config/test_predicates.py @@ -454,13 +454,28 @@ def test_header_multiple_mixed_fails(self): self.assertFalse(predicates[0](Dummy(), request)) def test_is_authenticated_true_matches(self): - ... + _, predicates, _ = self._callFUT(is_authenticated=True) + request = DummyRequest() + request.is_authenticated = True + self.assertTrue(predicates[0](Dummy(), request)) + def test_is_authenticated_true_fails(self): - ... + _, predicates, _ = self._callFUT(is_authenticated=True) + request = DummyRequest() + request.is_authenticated = False + self.assertFalse(predicates[0](Dummy(), request)) + def test_is_authenticated_false_matches(self): - ... + _, predicates, _ = self._callFUT(is_authenticated=False) + request = DummyRequest() + request.is_authenticated = False + self.assertTrue(predicates[0](Dummy(), request)) + def test_is_authenticated_false_fails(self): - ... + _, predicates, _ = self._callFUT(is_authenticated=False) + request = DummyRequest() + request.is_authenticated = True + self.assertFalse(predicates[0](Dummy(), request)) def test_unknown_predicate(self): from pyramid.exceptions import ConfigurationError diff --git a/tests/test_config/test_routes.py b/tests/test_config/test_routes.py index 8227784ec3..605f618577 100644 --- a/tests/test_config/test_routes.py +++ b/tests/test_config/test_routes.py @@ -184,6 +184,18 @@ def test_add_route_with_request_param(self): request.params = {} self.assertEqual(predicate(None, request), False) + def test_add_route_with_is_authenticated(self): + config = self._makeOne(autocommit=True) + config.add_route('name', 'path', is_authenticated=True) + route = self._assertRoute(config, 'name', 'path', 1) + predicate = route.predicates[0] + request = self._makeRequest(config) + request.is_authenticated = True + self.assertEqual(predicate(None, request), True) + request = self._makeRequest(config) + request.is_authenticated = False + self.assertEqual(predicate(None, request), False) + def test_add_route_with_custom_predicates(self): import warnings diff --git a/tests/test_config/test_views.py b/tests/test_config/test_views.py index 2a55ad45dc..09714d82ee 100644 --- a/tests/test_config/test_views.py +++ b/tests/test_config/test_views.py @@ -1742,6 +1742,46 @@ def test_add_view_with_xhr_false(self): request.is_xhr = False self._assertNotFound(wrapper, None, request) + def test_add_view_with_is_authenticated_true_matches(self): + from pyramid.renderers import null_renderer as nr + + view = lambda *arg: 'OK' + config = self._makeOne(autocommit=True) + config.add_view(view=view, is_authenticated=True, renderer=nr) + wrapper = self._getViewCallable(config) + request = self._makeRequest(config) + request.is_authenticated = True + self.assertEqual(wrapper(None, request), 'OK') + + def test_add_view_with_is_authenticated_true_no_match(self): + view = lambda *arg: 'OK' + config = self._makeOne(autocommit=True) + config.add_view(view=view, is_authenticated=True) + wrapper = self._getViewCallable(config) + request = self._makeRequest(config) + request.is_authenticated = False + self._assertNotFound(wrapper, None, request) + + def test_add_view_with_is_authenticated_false_matches(self): + from pyramid.renderers import null_renderer as nr + + view = lambda *arg: 'OK' + config = self._makeOne(autocommit=True) + config.add_view(view=view, is_authenticated=False, renderer=nr) + wrapper = self._getViewCallable(config) + request = self._makeRequest(config) + request.is_authenticated = False + self.assertEqual(wrapper(None, request), 'OK') + + def test_add_view_with_is_authenticated_false_no_match(self): + view = lambda *arg: 'OK' + config = self._makeOne(autocommit=True) + config.add_view(view=view, is_authenticated=False) + wrapper = self._getViewCallable(config) + request = self._makeRequest(config) + request.is_authenticated = True + self._assertNotFound(wrapper, None, request) + def test_add_view_with_header_badregex(self): view = lambda *arg: 'OK' config = self._makeOne() diff --git a/tests/test_security.py b/tests/test_security.py index bf29081005..72598f5706 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -393,6 +393,29 @@ def test_security_policy_trumps_authentication_policy(self): self.assertEqual(request.unauthenticated_userid, 'wat') +class TestIsAuthenticated(unittest.TestCase): + def setUp(self): + testing.setUp() + + def tearDown(self): + testing.tearDown() + + def test_no_security_policy(self): + request = _makeRequest() + self.assertIs(request.is_authenticated, False) + + def test_with_security_policy(self): + request = _makeRequest() + _registerSecurityPolicy(request.registry, '123') + self.assertIs(request.is_authenticated, True) + + def test_with_legacy_security_policy(self): + request = _makeRequest() + _registerAuthenticationPolicy(request.registry, 'yo') + _registerLegacySecurityPolicy(request.registry) + self.assertEqual(request.authenticated_userid, 'yo') + + class TestEffectivePrincipals(unittest.TestCase): def setUp(self): testing.setUp() From 5f37acda1c8af7cb288e792e2c82f728fe254818 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Araujo?= Date: Thu, 2 Jul 2020 17:06:02 -0400 Subject: [PATCH 3/3] improve doc Co-Authored-By: Steve Piercy --- docs/narr/viewconfig.rst | 12 ++++++------ src/pyramid/config/routes.py | 13 +++++++------ src/pyramid/config/views.py | 14 ++++++++------ 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/docs/narr/viewconfig.rst b/docs/narr/viewconfig.rst index 2dd5bfb9e3..de82fc0754 100644 --- a/docs/narr/viewconfig.rst +++ b/docs/narr/viewconfig.rst @@ -495,12 +495,11 @@ configured view. .. versionadded:: 1.4a3 ``is_authenticated`` - - This value, if specified, must be either ``True`` or ``False``. If it is - specified and is ``True``, the request must be for an authenticated user, - as determined by the :term:`security policy` in use. If it is specified and - ``False``, the associated view callable will be invoked only if the request - does not have an authenticated user. + This value, if specified, must be either ``True`` or ``False``. + If it is specified and ``True``, only a request from an authenticated user, as + determined by the :term:`security policy` in use, will satisfy the predicate. + If it is specified and ``False``, only a request from a user who is not + authenticated will satisfy the predicate. .. versionadded:: 2.0 @@ -516,6 +515,7 @@ configured view. .. versionadded:: 1.4a4 .. deprecated:: 2.0 + Use ``is_authenticated`` or a custom predicate. ``custom_predicates`` If ``custom_predicates`` is specified, it must be a sequence of references to diff --git a/src/pyramid/config/routes.py b/src/pyramid/config/routes.py index feb28c7a78..4f3440c406 100644 --- a/src/pyramid/config/routes.py +++ b/src/pyramid/config/routes.py @@ -270,12 +270,12 @@ def add_route( is_authenticated - This value, if specified, should be either ``True`` or ``False``. - If it is specified and is ``True``, the route will only match if - the request has an authenticated user, as determined by the - :term:`security policy` in use. If it is specified and ``False``, - the route will only match if the request does not have an - authenticated user. + This value, if specified, must be either ``True`` or ``False``. + If it is specified and ``True``, only a request from an authenticated + user, as determined by the :term:`security policy` in use, will + satisfy the predicate. + If it is specified and ``False``, only a request from a user who is + not authenticated will satisfy the predicate. .. versionadded:: 2.0 @@ -293,6 +293,7 @@ def add_route( .. versionadded:: 1.4a4 .. deprecated:: 2.0 + Use ``is_authenticated`` or a custom predicate. custom_predicates diff --git a/src/pyramid/config/views.py b/src/pyramid/config/views.py index 4a5723a143..26b69beb91 100644 --- a/src/pyramid/config/views.py +++ b/src/pyramid/config/views.py @@ -714,13 +714,14 @@ def wrapper(context, request): is_authenticated - This value, if specified, should be either ``True`` or ``False``. - If it is specified and is ``True``, the request must be for an - authenticated user, as determined by the :term:`security policy` in - use. If it is specified and ``False``, the associated view callable - will match only if the request does not have an authenticated user. + This value, if specified, must be either ``True`` or ``False``. + If it is specified and ``True``, only a request from an authenticated + user, as determined by the :term:`security policy` in use, will + satisfy the predicate. + If it is specified and ``False``, only a request from a user who is + not authenticated will satisfy the predicate. - ..versionadded:: 2.0 + .. versionadded:: 2.0 effective_principals @@ -736,6 +737,7 @@ def wrapper(context, request): .. versionadded:: 1.4a4 .. deprecated:: 2.0 + Use ``is_authenticated`` or a custom predicate. custom_predicates