Hello Tobias, And sorry for the late answer!
Yes, this is a good idea. https://docs.djangoproject.com/en/2.1/topics/auth/customizing/#handling-authorization-in-custom-backends <https://docs.djangoproject.com/en/2.1/topics/auth/customizing/#handling-authorization-in-custom-backends> reads more like "let's document the existing APIs to help users handle permissions in custom backends" than like "we spent a lot of time designing the most user friendly API for handling permissions in custom backends". I think you're on the right track for defining: A. which permission-related APIs custom authentication / authorization backends are expected to provide B. which other public APIs have a reasonable implementation that usually doesn't need overriding Regarding your questions: > - Either add get_user_permissions() or remove get_group_permissions() The path of least resistance is to make `get_user_permissions()`, which already exists, a public API. I'm not finding a sufficiently good reason to remove `get_group_permissions()`. > - Add default implementations for get_all_permissions() and > has_perm(), either in PermissionMixin or in a new BaseBackend class. Did you mean has_perms()? Also, `PermissionsMixin` is a mixin for a user model class; it isn't the right target here. Moving generic parts of ModelBackend into a BaseBackend class would be good. This is where you start sorting APIs between my categories A and B. Writing a good description of what changes you're planning to make and why in a Trac ticket should be enough — especially if you can divide changes in several steps, with one ticket per step. I don't think a DEP is necessary. Cheers, -- Aymeric. > On 12 Nov 2018, at 17:57, Collin Anderson <cmawebs...@gmail.com> wrote: > > > > Add default implementations for get_all_permissions() and has_perm(), > > either in PermissionMixin or in a new BaseBackend class. > On a first glance, I think that makes sense to me. > > > Also note that the separation between user and group permissions may not be > > applicable with custom backends. > That also makes sense to me. It does seems like kind of an implementation > detail as to where the permission is actually coming from, though I think > that information could be useful in some cases, so somehow making it optional > would be nice. > > On Sat, Nov 10, 2018 at 3:07 AM Tobias Bengfort <tobias.bengf...@posteo.de > <mailto:tobias.bengf...@posteo.de>> wrote: > I feel like the interface for authentication backends is unnecessarily > complex: Basically, you only need authenticate() and has_perm(), but > currently the interface also includes get_group_permissions(), > get_all_permissions(), and has_module_perms(). > > The architecture is like this: User inherits from PermissionMixin which > implements the methods get_group_permissions(), get_all_permissions(), > has_perm(), has_perms(), and has_module_perms(). > > All of these methods basically just call the corresponding methods on > the authentication backends, if available. has_perms() is the only > exception, as it is just a shortcut to call has_perm() multiple times. > > I believe that has_perm() is vastly more important than > get_*_permissions(). Still, I can understand that the latter exist. What > I find confusing is that there is no get_user_permissions() in this > interface. Whithout its counterpart, get_group_permissions() seems > pretty much useless to me. Also note that the separation between user > and group permissions may not be applicable with custom backends. > > Another issue is that developers can easily end up with inconsistent > backends: There is no guarantee that get_all_permissions() will return a > superset of get_group_permissions(). There is also no guarantee that > has_perm() will be equivalent to `perm in get_all_permissions()`. > > Then there is the issue of has_module_perms(). As far as I understand, > this should have been called has_app_perms(). It is used in > contrib.admin where it makes a lot of sense, but I don't really see how > it could be useful anywhere else. > > I looked at some popular backends to see how they are implemented: > > The default ModelBackend implements get_user_permissions() and > get_group_permissions(). get_all_permissions() joins their results and > has_perm() and has_module_perms() call get_all_permissions(). The > results of get_all_permissions() are cached. > > django-guardian works pretty much the same. However, even though the > mechanism is the same, get_group_permissions() is not exposed in the > interface. has_module_perms() is missing. > > django-rules only implements has_perm(). get_*_permissions() are > missing. has_module_perms() exists, but it is just an alias to > has_perm(). > > I guess the benefits of simplifying the interface do not justify > breaking changes for most of these issues. However, I think there are > two changes that could significantly improve the situation: > > - Either add get_user_permissions() or remove get_group_permissions() > - Add default implementations for get_all_permissions() and > has_perm(), either in PermissionMixin or in a new BaseBackend class. > > Any thoughts? > > tobias > > -- > You received this message because you are subscribed to the Google Groups > "Django developers (Contributions to Django itself)" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to django-developers+unsubscr...@googlegroups.com > <mailto:django-developers%2bunsubscr...@googlegroups.com>. > To post to this group, send email to django-developers@googlegroups.com > <mailto:django-developers@googlegroups.com>. > Visit this group at https://groups.google.com/group/django-developers > <https://groups.google.com/group/django-developers>. > To view this discussion on the web visit > https://groups.google.com/d/msgid/django-developers/9eb25389-d59e-1f6c-4b50-d1c7a986f923%40posteo.de > > <https://groups.google.com/d/msgid/django-developers/9eb25389-d59e-1f6c-4b50-d1c7a986f923%40posteo.de>. > For more options, visit https://groups.google.com/d/optout > <https://groups.google.com/d/optout>. > > -- > You received this message because you are subscribed to the Google Groups > "Django developers (Contributions to Django itself)" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to django-developers+unsubscr...@googlegroups.com > <mailto:django-developers+unsubscr...@googlegroups.com>. > To post to this group, send email to django-developers@googlegroups.com > <mailto:django-developers@googlegroups.com>. > Visit this group at https://groups.google.com/group/django-developers > <https://groups.google.com/group/django-developers>. > To view this discussion on the web visit > https://groups.google.com/d/msgid/django-developers/CAFO84S4SKCkN3X_-Hw4gWhVd2AHd8hZN6X2phEy0RriiiuMV8Q%40mail.gmail.com > > <https://groups.google.com/d/msgid/django-developers/CAFO84S4SKCkN3X_-Hw4gWhVd2AHd8hZN6X2phEy0RriiiuMV8Q%40mail.gmail.com?utm_medium=email&utm_source=footer>. > For more options, visit https://groups.google.com/d/optout > <https://groups.google.com/d/optout>. -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/250E388D-2D71-49F9-AE15-3839D89B2C82%40polytechnique.org. For more options, visit https://groups.google.com/d/optout.
