Accessing requires from within Jinja

[susodapop]

Been playing around with this tonight but I've hit a wall. I want to hide certain menu links from users that don't have permission to access those links. In theory, I'll replace url_for in many of my templates with a macro called url_for_allowed backed by a custom template filter.

Something like this:

@app.template_filter('ok')
def check_if_allowed(route_name):
...check if g.user can access this route_name

With a macro like this:

{% macro url_for_allows(route, link_text) %}
    {% if route | ok %}
        <a href="{{ url_for(route) }}">link_text</a>
    {% endif %}
{% endmacro %}

I thought maybe there would be a permissions map similar to Flask's route map. Thoughts?

[justanr]

The way we addressed this in FlaskBB was to register individual requirements as template filters. I regret doing that now -- it was early in Allow's life and I didn't really understand what I'd created. Instead I'd recommend using the Permission construct and registering that and any requirements needed into the jinja namespace instead of doing filters, or at the very least create a filter using Permission rather than following what we did.

[susodapop]

If I understand correctly, doing it that way I’d need to explicitly state my requirement logic in the Jinja template. So if the permissions for a given route changed from is_admin to is_account_holder I’d update the route decorator and every reference to the route anywhere in Jinja. If I missed one, the template wouldn’t show the link unless is_admin passed - but the route permission would be different.

Is there any way around this? I’d like to register permissions in one central place in keeping with DRY. Maybe something similar to Flasks’s route decorator versus add_url_rule construct.

This is my first project of this kind so I wonder if I’m using the wrong design pattern.

[justanr]

Hm, that does sound painful. You have an interesting use case -- being able to retrieve requirements from endpoints. Currently there's nothing that handles that, however I'd be open to considering it as it seems very useful.

My first thoughts on this matter:

Re-exposing the requirements as an attribute on the endpoint, so you could do something like:

func = app.view_functions[endpoint]
func.__requirements__  # (is_admin, Has('hello'))

This exposing is straight forward for endpoints that are functions and CBV that have their entire access governed by a single requires. There are two complications that come to mind, the first is very hard to deal with:

@app.route('/')
@some_other_deco
@requires(...)
def some_handler():
    pass

requires and app.route are well behaved decorators, but is some_other_deco well behaved -- does it use functool.wraps -- even if it's on Py2.7 it might not matter because there was a bug with wraps that wasn't fixed until 3.3 that would result in clobbering. So that's an issue. The only way to be 100% to avoid it is to place requires second to last, only after app.route so this discipline falls to the end user.

The other complication is more subtle but actually easier to deal with from a library point of view:

class SomeView(MethodView):
    decorators = [requires(...)]

    @requires(...)
    def get(self):
        pass

There are now two sets of requirements that are logically AND'd together, however since the endpoint class is exposed by the as_view constructor, a helper method can be made to retrieve and assemble this into a coherent set of requirements.

The final issue that I don't think is able to be handled as Allows is right now is when a Requirement internally switches its actions based on the request's HTTP action. However, if that's being done its probably troublesome in other ways as well and to be frank, I don't think multiplexing is a good move with either requirements or endpoints (as a consequence, I'm a big supporter of CBVs).

Anyways, those are fiddly points that would need to be addressed. As for gathering the requirements and checking them, I think something along these lines would be up to the task:

def requirements_for_endpoint(endpoint):
    handler = current_app.view_functions[endpoint]
    requirements = getattr(handler, '__requirements__`, ())
    return requirements

And running them might look like:

def allowed_endpoint(endpoint):
    return bool(Permission(*requirements_for_endpoint(endpoint)))

However you'd only ever be able to use in situations where there is an active application context, but I think that's a small price to pay.

[justanr]

You'd need to be careful with requirements that hit the database or do other expensive work if the requirement was hit multiple times, caching results or something similar will save you here.

[justanr]

I'm actually going to co-opt this issue to make it the tracking issue for this sort of thing. I think it's a great addition and I can think of places where I'd use in side projects and in FlaskBB. Proposals I'm putting forth:

1. Expose requirements out via an attribute like __requirements__ (I know dunders are reserved for Python itself, but exposing it as _requirements may clash as that's a perfectly reasonable attribute name and __requirements would cause issues all its own because of name mangling).

2. Create requirements_for_endpoint and allowed_endpoint helpers such as above (but requirements_for_endpoint needs to consider multiple @requires registrations on a single endpoint as well as allow for specifying an HTTP method for coordinating with MethodView -- all of these should be AND'd together as that's how they'd be treated in practice). If guarding blueprints makes it into this library (highly likely) we'd need some way of detecting those as well -- right now the current proposal is to use a before_request handler which would render them effectively invisible, nevermind that we need some way of getting the blueprint from the endpoint name.

3. Update documentation to highly recommend that @requires be the last decorator before @route when dealing with function views and LAST when dealing with CBVs (this would ensure even if another decorator isn't well behaved we can still retrieve what we need).

4. Must be compatible with the planned override/additional permission proposal that I have planned for 0.6

I probably won't have time to do this until after 0.6 (which I already have little time for right now anyways), so I can plan this for 0.7. However, I'd also be happy to review a PR to get this into 0.6 as well.

[justanr]

I'll also consider adding a factory function for creating a jinja filter but I think that might be too specialized.

[susodapop]

Regarding number 2 above and blueprints: when using url_for, calls to blueprinted view functions are made by prepending <blueprint>.to the function name. This makes it easy to see in a Jinja template if a given link is part of a blueprint or not. Could Allows rely on that same logic?

We could regex for ., split if it’s present, and then lookup its Requirements.

[justanr]

After doing some research last night, I think keeping the before request handler for running the permissions and also dropping the dunder requirements field on it will be the best solution.

Since nested blueprints aren't a thing right now, str.partition(endpoint, '.', 1) will get us what we need but we'd need to check that '.' appears in the endpoint first (otherwise the first element will be '' and that's ambiguous because it could be routing in the same blueprint). From there, it's a lookup into app blueprints

[justanr]

Here's a complication. We're at some endpoint, however one of the endpoints we're checking against uses request to pull some url args or query parameters as part of the requirement check. The route we're at now uses different url args or query params.

How do handle this? Push a new request context when checking? This becomes more complicated when we start considering HATEOAS and going using this beyond GET (e. g. an api endpoint that says "if you want to update this resource, send a put here")..

I'm fine not considering cross http method magic right now (though in my mind it doesn't seem difficult to implement), but the request thing is a bigger issue that deserves some serious contemplation.

[susodapop]

I haven’t sat down to noodle this one. But on face value pushing a request context seems like the right idea. Considering what Jinja is doing (deciding whether or not to display a link or html snippet to the current user), we can assume that Jinja knows what it’s going to display. If that’s a URL with arguments, Jinja must have those available or else it couldn’t print the whole URL.

[susodapop]

It occurs to me that one (dumb) way to solve this issue is to actually try a GET request with the current user’s credentials and only print the block in Jinja if the response has code 201.

Obviously this would generate needless strain on the application (imagine permissioning dozens of links on a page). But perhaps a similar construct could do the trick. Like building a flag into the Allows wrapper that says “this is a permissions check”. Allows could return yes or no without executing the wrapped function.

[justanr]

The more I dig into flask the bleaker pushing a fake context looks. :/ Pushing a fake context has almost the same consequences as doing a full request.

I'll poke around a bit more, but it might require a change in flask itself to support this.

[justanr]

I dug in a bit more, and here's what I think:

It's not impossible. There's Flask.request_context which can push a new context -- that would entail copying the current wsgi environ and altering it to match what the target endpoint would expect. After that, any request context bound objects (e.g. flask-login's current_user proxy) would need to be loaded.

The difficulty there is know what things can be safely copied from one request context to another. After that, we'd potentially need to figure out which (if any) before_request handlers needed to be run.

Then we can gather all the requirements and check them.

Now we need to unwind what we did by running any after_request handlers that need to teardown something a before_request setup.

Then and only then do we have our answer: can the user enter that endpoint?

Other complications arise:

1. What happens if we encounter an exception during all of this? Since we're doing something we're not suppose to be doing it's up to use to handle it, relying on Flask.errorhandler could have very bad results.

2. This requires completely stateless requirements. The implication is that they should be already since class based requirements are one instance per application lifetime (at least when registered on views).

3. Requirements can't have any visible side effects -- e.g. querying the database is fine, but get_or_404 is not, this would route the user away from the current page and all of our hard work is for not.

4. We have to be 100% sure that no matter what happens short of the process dying that we teardown our fake request context.

There are also performance considerations. I have no doubt in my mind that this approach results in all of the n+1 queries on every page that uses this because there's no good way to share that information across all of this fake requests.

Consider FlaskBB, where this project took shape: We'd likely want to use this in the topic view to determine what things a user is allowed to do a post:

Here I have permissions to quote, reply, edit, delete and report this post. Quote+reply share one set of permissions, edit, delete and report each have separate sets. If we used endpoint resolution for these, that's 1+(n*5) queries -- one to gather all the posts and data and then an individual query for every set of options, this gets worse in later version of FlaskBB where other options are found there as well. Some of this would not result in database queries when users have admin or super mod privileges, but after that queries need to happen because we need other details than what the route alone provides.

I'm still not opposed to the idea in general, but I think this particular line of thought leads to a doable but incredibly fragile and inefficient dead end.