Add Request.is_authenticated and is_authenticated predicate #3598

merwok · 2020-06-22T20:35:26Z

src/pyramid/interfaces.py

src/pyramid/security.py

tests/test_config/test_predicates.py

stevepiercy · 2020-06-22T21:11:41Z

@merwok fyi, thanks for the offer to update What's New, but it is easier to do a single copy-pasta job at release time. See https://github.com/Pylons/pyramid/blob/master/RELEASING.txt#L49-L58 for process.

mmerickel · 2020-07-01T23:19:21Z

This configuration will not work because the two view configs have the same specificity:

They have the same view weighting but because they are properly mutually exclusive, the correct one will be selected for each request. This configuration will work.

There could definitely be issues with effective_principals because you could setup configurations that were not mutually exclusive. For example Everyone vs Authenticated is ambiguous, because authenticated users also contain the Everyone principal.

src/pyramid/predicates.py

docs/narr/viewconfig.rst

stevepiercy · 2020-07-02T08:32:30Z

docs/narr/viewconfig.rst

+  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.


If it is specified and its value is ``True``, only a request from an authenticated user, as determined by the :term:`security policy` in use, will satisfy this predicate. If it is specified and its value is ``False``, only a request from a user who is not authenticated will satisfy this predicate.

The is_authenticated predicate is applicable to both routes and views, and this version makes it copy-pasteable into route config. I like less maintenance!

Also I don't think it is correct to say that a "view callable will match", but maybe "predicate will match" or "predicate is satisfied". I took a shot at it, but maybe @mmerickel can wordsmith it better. Whatever is correct in Pyramid terminology.

The sentences should be merged into one paragraph. I put them separately for easier comprehension.

Oops, that should have been the view config will match! (not correct to say the view callable will be invoked, there are other predicates) (edit: but the other predicate docs do say that 🙁)

I have kept the doc as it was (not copy-pastable between route and view doc), to follow the existing style.

Yeah, the predicate docs are suboptimal. There's an open issue that describes the current state, identifies the problems, and offers suggestions to improve the situation. I'd appreciate input on the best direction forward, then I'll create a PR for it.

I’m not sure about my wording the view config will match, it seems a little imprecise.
(the route will match feels correct, if a little dry — maybe route definition?)

I agree now that the predicate will match is simple, correct and reusable so I will edit the new docs.

src/pyramid/config/routes.py

src/pyramid/config/views.py

src/pyramid/interfaces.py

src/pyramid/security.py

docs/narr/viewconfig.rst

mmerickel

This looks great. No major changes, just a few little things.

CHANGES.rst

src/pyramid/predicates.py

mmerickel

Great work, thank you @merwok. I'm gonna wait for @stevepiercy since he also marked the PR for changes, but things look good to me.

merwok · 2020-07-02T19:48:42Z

Oh also let me validate the whole thing with a tiny app! (can only trust unit tests so far)

merwok · 2020-07-02T20:08:42Z

I have added a bunch of routes and views to a minimal app and it looks good.

Python

def hello_world(request):
    return Response("Hello World!")


def hello_member(request):
    return Response("For members only.")


def hello_stranger(request):
    return Response("You are not authenticated :)")


def includeme(config):
    config.add_route("members-only", "/cool-zone", is_authenticated=True)
    config.add_view(hello_member, route_name="members-only")

    config.add_route("home", "/home")
    config.add_route("welcome", "/home", is_authenticated=False)
    config.add_view(hello_member, route_name="home")
    config.add_view(hello_stranger, route_name="welcome")

    config.add_route("history", "/history")
    config.add_view(hello_stranger, route_name="history")
    config.add_view(hello_member, route_name="history", is_authenticated=True)

    config.add_route("profile", "/profile")
    config.add_view(hello_stranger, route_name="profile", is_authenticated=False)
    config.add_view(hello_member, route_name="profile", is_authenticated=True)

proutes

Name                         Pattern                              View                                                                     Method    
----                         -------                              ----                                                                     ------    
hello                        /                                    helloapp.hello_world                                                     *         
members-only                 /cool-zone                           helloapp.hello_member                                                    *         
home                         /home                                helloapp.hello_member                                                    *         
welcome                      /home                                helloapp.hello_stranger                                                  *         
history                      /history                             pyramid.config.views.<pyramid.config.views.MultiView object at 0x...>    *         
profile                      /profile                             pyramid.config.views.<pyramid.config.views.MultiView object at 0x...>    *         
debugtoolbar                 /_debug_toolbar/*subpath             <unknown>                                                                *         
__/_debug_toolbar/static/    /_debug_toolbar/static/*subpath      pyramid_debugtoolbar:static/                                             *

pviews

URL = /home

    context: <pyramid.traversal.DefaultRootFactory object at 0x...>
    view name: home

    Route:
    ------
    route name: home
    route pattern: /home
    route path: /home
    subpath: 

        View:
        -----
        helloapp.hello_member.

    Route:
    ------
    route name: welcome
    route pattern: /home
    route path: /home
    subpath: 
    route predicates (is_authenticated = False)

        View:
        -----
        helloapp.hello_stranger.


URL = /cool-zone

    context: <pyramid.traversal.DefaultRootFactory object at 0x...>
    view name: cool-zone

    Route:
    ------
    route name: members-only
    route pattern: /cool-zone
    route path: /cool-zone
    subpath: 
    route predicates (is_authenticated = True)

        View:
        -----
        helloapp.hello_member.


URL = /profile

    context: <pyramid.traversal.DefaultRootFactory object at 0x...>
    view name: profile

    Route:
    ------
    route name: profile
    route pattern: /profile
    route path: /profile
    subpath: 

        View:
        -----
        helloapp.hello_stranger
        view predicates (is_authenticated = False)

        View:
        -----
        helloapp.hello_member
        view predicates (is_authenticated = True)


URL = /history

    context: <pyramid.traversal.DefaultRootFactory object at 0x...>
    view name: history

    Route:
    ------
    route name: history
    route pattern: /history
    route path: /history
    subpath: 

        View:
        -----
        helloapp.hello_member
        view predicates (is_authenticated = True)

        View:
        -----
        helloapp.hello_stranger

I saw the expected text when opening the URLs with and without security policy (a simple class with 7 lines that permits everything)!

I also deleted the original IsAuthenticated predicate in my project, installed this Pyramid branch and my tests all pass.

stevepiercy

I'm OK with the changes as is, but would like both @merwok and @mmerickel thoughts on my comments about multiple predicates.

docs/narr/viewconfig.rst

src/pyramid/config/routes.py

src/pyramid/config/views.py

Co-Authored-By: Steve Piercy

merwok · 2020-07-03T21:07:06Z

@mmerickel this is ready!

digitalresistor · 2020-07-06T03:45:35Z

\o/ this is great, thank you @merwok!

first shot for Pylons#1602

baa5928

merwok commented Jun 22, 2020

View reviewed changes

src/pyramid/interfaces.py Show resolved Hide resolved

merwok commented Jun 22, 2020

View reviewed changes

src/pyramid/security.py Show resolved Hide resolved

merwok commented Jun 22, 2020

View reviewed changes

tests/test_config/test_predicates.py Show resolved Hide resolved

This comment has been minimized.

Sign in to view

merwok force-pushed the feature/is_authenticated branch from 28b4471 to 363b54c Compare July 1, 2020 23:43

merwok marked this pull request as ready for review July 1, 2020 23:43

stevepiercy requested changes Jul 2, 2020

View reviewed changes

merwok commented Jul 2, 2020

View reviewed changes

docs/narr/viewconfig.rst Outdated Show resolved Hide resolved

merwok requested review from mmerickel and stevepiercy July 2, 2020 13:49

mmerickel requested changes Jul 2, 2020

View reviewed changes

CHANGES.rst Outdated Show resolved Hide resolved

CHANGES.rst Outdated Show resolved Hide resolved

src/pyramid/predicates.py Outdated Show resolved Hide resolved

merwok force-pushed the feature/is_authenticated branch 3 times, most recently from cf34271 to 46c8308 Compare July 2, 2020 17:05

add tests and docs

70a23ba

merwok force-pushed the feature/is_authenticated branch from 46c8308 to 70a23ba Compare July 2, 2020 17:09

merwok requested a review from mmerickel July 2, 2020 18:12

mmerickel approved these changes Jul 2, 2020

View reviewed changes

stevepiercy reviewed Jul 2, 2020

View reviewed changes

docs/narr/viewconfig.rst Outdated Show resolved Hide resolved

src/pyramid/config/routes.py Show resolved Hide resolved

src/pyramid/config/views.py Show resolved Hide resolved

improve doc

5f37acd

Co-Authored-By: Steve Piercy

merwok requested a review from stevepiercy July 2, 2020 21:10

stevepiercy approved these changes Jul 2, 2020

View reviewed changes

digitalresistor merged commit 5269b28 into Pylons:master Jul 6, 2020

merwok deleted the feature/is_authenticated branch July 6, 2020 14:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Request.is_authenticated and is_authenticated predicate #3598

Add Request.is_authenticated and is_authenticated predicate #3598

merwok commented Jun 22, 2020 •

edited

Loading

stevepiercy commented Jun 22, 2020

This comment has been minimized.

mmerickel commented Jul 1, 2020 •

edited

Loading

This comment has been minimized.

stevepiercy Jul 2, 2020

merwok Jul 2, 2020 •

edited

Loading

merwok Jul 2, 2020

stevepiercy Jul 2, 2020

merwok Jul 2, 2020

mmerickel left a comment

mmerickel left a comment

merwok commented Jul 2, 2020

merwok commented Jul 2, 2020 •

edited

Loading

stevepiercy left a comment

merwok commented Jul 3, 2020

digitalresistor commented Jul 6, 2020

Add Request.is_authenticated and is_authenticated predicate #3598

Add Request.is_authenticated and is_authenticated predicate #3598

Conversation

merwok commented Jun 22, 2020 • edited Loading

stevepiercy commented Jun 22, 2020

This comment has been minimized.

mmerickel commented Jul 1, 2020 • edited Loading

This comment has been minimized.

stevepiercy Jul 2, 2020

Choose a reason for hiding this comment

merwok Jul 2, 2020 • edited Loading

Choose a reason for hiding this comment

merwok Jul 2, 2020

Choose a reason for hiding this comment

stevepiercy Jul 2, 2020

Choose a reason for hiding this comment

merwok Jul 2, 2020

Choose a reason for hiding this comment

mmerickel left a comment

Choose a reason for hiding this comment

mmerickel left a comment

Choose a reason for hiding this comment

merwok commented Jul 2, 2020

merwok commented Jul 2, 2020 • edited Loading

stevepiercy left a comment

Choose a reason for hiding this comment

merwok commented Jul 3, 2020

digitalresistor commented Jul 6, 2020

merwok commented Jun 22, 2020 •

edited

Loading

mmerickel commented Jul 1, 2020 •

edited

Loading

merwok Jul 2, 2020 •

edited

Loading

merwok commented Jul 2, 2020 •

edited

Loading