Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Per-actions Permissions. #5

Open
carltongibson opened this issue Feb 27, 2023 · 10 comments
Open

Per-actions Permissions. #5

carltongibson opened this issue Feb 27, 2023 · 10 comments
Assignees

Comments

@carltongibson
Copy link
Owner

Scenario:

  • Public access to List and Detail views.
  • Restricted access to edit views.

Needs a hook into ModelView.get_urls() since parsing the end-list to wrap the view callbacks isn't fun.
But the most vanilla approach is to wrap the view, rather than add a DRF-style check_permissions() hook on the view class itself.

@carltongibson
Copy link
Owner Author

carltongibson commented Feb 27, 2023

Decorators take various kwargs.

        staff_member_required(..., login_url="/", redirect_field_name=None),

🔨: Using partial when defining a map?

@carltongibson carltongibson self-assigned this Feb 27, 2023
@tissieres
Copy link

Hello Carlton,
This is a functionality that I need for my current project. I was planning to implement something that relies on the built-in model permissions. Do you prefer something more generic using decorators? Maybe I could propose a PR, but I will need your guidance on how to do that first 😉

@carltongibson
Copy link
Owner Author

Hi @tissieres.

I'm still working on the details of the desired API here.

Normally I'd be very grateful for input but I do want to handle this one myself.

A first pass would be to implement a check_permissions and call that at the appropriate place in the view.

We're still at the early stage here so there's no wrong answers yet.

I know that's not very specific but I hope it helps.

@tissieres
Copy link

No problem. I will test some approaches on my side and if I find something really elegant, I'll come back to you. Thanks for neapolitan, fun project!

@carltongibson
Copy link
Owner Author

Thanks! Let me know what you come up with!

@tissieres
Copy link

That was simple finally:

    @classonlymethod
    def get_urls(cls):
        verbose_name = cls.model._meta.model_name
        app_label = cls.model._meta.app_label
        urlpatterns = [
            path(
                f"{verbose_name}/",
                permission_required(f"{app_label}.view_{verbose_name}")(
                    cls.as_view(role=Role.LIST)
                ),
                name=f"{verbose_name}-list",
            ),
            path(
                f"{verbose_name}/new/",
                permission_required(f"{app_label}.add_{verbose_name}")(
                    cls.as_view(role=Role.CREATE)
                ),
                name=f"{verbose_name}-create",
            ),
...

I also added a check of the permissions in the object_list template tag to restrict the actions and in the object_list.html template to remove the "add button" depending on them.

@carltongibson
Copy link
Owner Author

Yes, wrapping the view callbacks in get_urls() is a very good way to go!

@radiac
Copy link

radiac commented Nov 2, 2024

Hi @carltongibson! We spoke at djangocon about this, and whether we could do something with django-fastview's permission system. (Documentation and source)

I've looked at it more closely and have some ideas, but how far do you want to go with permission support?

If we just want a hook for access control to be left to the user, we can use or clone Django's UserPassesTestMixin - by the point test_func gets called we already have self.role and self.request, so can accomplish quite a bit, albeit with a couple of gaps.

If you're up for more syntactic sugar and functionality, I'm thinking about refactoring fastview's permission stuff into a separate project, and am thinking about what it would look like. I would love to use it with Neapolitan - either by using hooks to write it as a mixin in mine, or building it in directly as a dependency.

As a user I'd like to end up with something like this:

class BookmarkView(CRUDView):
    model = Bookmark
    fields = ["url", "title", "note"]
    permissions = Login()

with a way to manage permissions for specific roles, perhaps:

class BookmarkView(CRUDView):
    permissions = {
        Role.LIST: Public(),
        None: Staff() | Owner("created_by")
    }

(Not entirely sure about None as the default there, but that general idea)

As I say, a lot of this is already achievable by subclassing, but we are missing a couple of bits for some nice-to-have features:

Support in action links

We would probably want to hide action links when they're not available. The action_links template tag already has a reference to the view, so should also be able to use the same test_func logic.

It may be worth having a separate check_action_permission(role) as an intermediate fn/property, so you could have different logic for the two - but it feels very edge-casey at this point.

Support for checking objects

I'd like to be able to check based on the object a user is trying to access etc - eg being able to say only the owner of the object can edit it.

This is possible now, but would result in an extra query or two. The neatest solution would be to call get_object or get_queryset from test_func, and stash the result for later.

We would probably also need to add Role.has_object and Role.has_object_list properties so it knows which to check. This could also be useful later (eg custom roles).

Filtering in list view

I would like to be able to control who can see what on the list view. It really needs to be separate to the main List permission (eg any logged in user can see the list view, but they can only see their data), something like:

class BookmarkView(CRUDView):
    row_permission = Staff() | Owner("created_by")

It would be a very simple change to get_queryset and nicely round out permission functionality - but equally it could be a howto.

@ryanhiebert
Copy link

I was working on a project with Neapolitan, and ended up abandoning neapolitan for that because I couldn't figure it out. Overriding get_urls() isn't too bad. I have some ideas on how I'd implement this, but they're different enough and not baked enough to be ready to justify just yet.

@carltongibson
Copy link
Owner Author

Thanks all. Yes, again, I have patterns in the WORK project that I need to pull out to advance here.

Essence if it is to apply view level permissions before dispatching the handler, table level permissions in or just after get_queryset and row level permissions in or just after get_object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants