Opened 9 years ago

Last modified 9 years ago

#25142 closed Cleanup/optimization

Refactor access mixins to allow customisation — at Version 7

Reported by: Akis Kesoglou Owned by: Akis Kesoglou
Component: contrib.auth Version: dev
Severity: Normal Keywords:
Cc: akiskesoglou@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by Akis Kesoglou)

Extending from PermissionRequiredMixin to implement a mixin for checking for *object* permissions means that you can't call super in .dispatch because PermissionRequiredMixin will do another check for model-level permissions.

I propose to delegate the check for permissions in PermissionRequiredMixin.dispatch to an instance method. This way, we are able to provide a hook for code to check for object permissions without having to re-implement the whole class.

PR coming shortly.


I have went ahead and generalised the ticket and patch to refactor all access mixins.

Change History (7)

comment:1 by Akis Kesoglou, 9 years ago

Cc: akiskesoglou@… added
Has patch: set
Status: newassigned

comment:3 by Markus Holtermann, 9 years ago

Needs documentation: set
Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Does this actually solve the issue? You haven't called get_object() by the time dispatch() is called. You would now query for the object twice, one during permission check and once sometime later (depending on the CBV).

Tentively accepting based on the general idea.

comment:4 by Akis Kesoglou, 9 years ago

get_object() will get called twice -- I don't see a way around it, unless itself caches the result somewhere, but I haven't investigated what happens then with form mixins. Another way would be to delay permission checks by wrapping the handler methods themselves, but this seems fragile. I personally consider this a fair compromise, but suggestions are welcome.

comment:5 by Akis Kesoglou, 9 years ago

Just in case I was not clear in the ticket's description, the purpose is to allow subclassing the PermissionRequiredMixin to customise authorization. As it is now, PermissionRequiredMixin has model-level authorization hard-coded to dispatch -- there's no way to extend it -- one must implement a completely new mixin, duplicating most code from PermissionRequiredMixin.

comment:6 by Akis Kesoglou, 9 years ago

Needs documentation: unset
Patch needs improvement: unset

comment:7 by Akis Kesoglou, 9 years ago

Description: modified (diff)
Summary: Refactor dispatch method in PermissionRequiredMixin to allow customisationRefactor access mixins to allow customisation
Note: See TracTickets for help on using tickets.
Back to Top