Conversation
gasman
changed the title
|
This would be a fantastic feature to have! |
|
|
||
| ### Front-end behaviour | ||
|
|
||
| Subclasses of `PageViewRestriction` should implement a method `check(self, request, page)` which will be called just before serving a page which is covered by that view restriction, passing the `HttpRequest` and the `Page` instance. This method shall return either `None` (to allow the page to be served as normal) or an `HttpResponse` (which is served immediately to the user, cancelling the rest of the page serve workflow). This differs from the current implementation, where the `accept_request` method returns a boolean and the calling code (the `before_serve_page` hook in `wagtail.core.wagtail_hooks`) has to generate a suitable response in the case of `False` being returned. |
There was a problem hiding this comment.
Would there be a way to filter a page QuerySet to only contain pages that the request can view? This is what accept_request was originally developed for.
There was a problem hiding this comment.
Good point! The problem of filtering a queryset to the pages accessible to a given visitor came up recently here: https://stackoverflow.com/questions/54629835/wagtail-filter-page-childs-elements-on-the-basis-of-logged-in-users-permissions/54655495#54655495
I wonder if we'll be creating problems down the line by tying the filtering logic to a request object, though... it's possible that we'll want to apply the filtering in a situation where a request isn't available, or where the active request isn't representative of the request for the actual page. For example, a headless site might make an API request to fetch a listing of pages accessible to the current user, and we can't assume that the HTTP request corresponds to an active session. (Judging from https://www.django-rest-framework.org/api-guide/authentication/ we probably can rely on request.user to be set appropriately, but not request.session['passed_page_view_restrictions'] as used by password-based restrictions.) It seems to me that we might want to introduce a formal concept of "credentials", i.e. variables that might have a bearing on the "can this visitor view page X or not" decision, and pass those to the filtering / decision methods rather than letting them dig around in a request object. Might get a bit over-engineered though...
On a separate note, it occurs to me that user-based auth is probably going to be the most common use-case here - and if the *ViewRestriction API is going to grow like this, then there'll almost certainly be some recurring patterns ("does the user satisfy condition X? If not, redirect them to the login page, unless they are logged in, in which case send them to a dead end you-don't-have-permission page") that would be best handled in a new abstract class, say UserPageViewRestriction. Hopefully, a developer subclassing UserPageViewRestriction would then only have to provide the minimal model-level logic, and leave all the request/redirection stuff to Wagtail.
4 participants
Rendered