I think deprecating it is a good idea. I think it's actually a small footgun - I've seen it used to restrict access to non-admin functionality, when `user_passes_test` would be appropriate. In the case I found it, it made adding tests trickier than it should have been.
Ian On Tue, 9 Mar 2021 at 21:51, Timothy McCurrach <tim.mccurr...@gmail.com> wrote: > > It seems like a good idea. However, it leads us to another question. Do > we really need the is_staff field on the User Model if it is deprecated? > > User.is_staff is still used, just from Adminsite.has_permission instead: > > https://github.com/django/django/blob/main/django/contrib/admin/sites.py#L196 > > On Tue, Mar 9, 2021 at 9:45 PM Matthew Pava <matthew.p...@iss.com> wrote: > >> > Thoughts on deprecating it? >> >> It seems like a good idea. However, it leads us to another question. Do >> we really need the is_staff field on the User Model if it is deprecated? >> >> >> >> *From:* django-developers@googlegroups.com < >> django-developers@googlegroups.com> *On Behalf Of *Timothy McCurrach >> *Sent:* Tuesday, March 9, 2021 3:22 PM >> *To:* django-developers@googlegroups.com >> *Subject:* Re: Difference between AdminSite.admin_view and >> staff_member_required? >> >> >> >> staff_member_required feels like a bit of a relic from the past: >> >> - It looks like before a massive refactor of the admin that happened 12 >> years ago ( >> https://github.com/django/django/commit/a19ed8aea395e8e07164ff7d85bd7dff2f24edca >> <https://us-east-2.protection.sophos.com?d=github.com&u=aHR0cHM6Ly9naXRodWIuY29tL2RqYW5nby9kamFuZ28vY29tbWl0L2ExOWVkOGFlYTM5NWU4ZTA3MTY0ZmY3ZDg1YmQ3ZGZmMmYyNGVkY2E=&i=NWVjN2YxNzUxNGEyNzMxNmMyMGRkZGU1&t=YkNVTDFoOEQ4dEdQdU1PYW9WWlJ0R0R0Y2ZGN0hRYkRiT0FkNTRkaU1Ocz0=&h=64eb5f1d05c6477aa783f14819c9eebe>) >> staff_member_required played a similar role to what Adminsite.admin_view >> does now (checks permissions and redirects to login if appropriate). Before >> the refactor, all of the relevant views were wrapped with >> staff_member_required. >> - The above commit introduced AdminSite which has its own >> 'has_permissions' method. Adminsite.root (which was the old get_urls) >> called self.has_permissions before calling any other views, so >> staff_member_required was now no longer really required. >> - staff_member_required did however continue to be used in the admindocs >> app (which was now split off into its own app). >> - Adminsite.root was eventually replaced with get_urls and the call to >> has_permissions was moved into `Adminsite.admin_view` (which also does a >> few other things: adds csrf protection, and deals with caching) >> - Over time staff_member_required became simpler and simpler as things >> like user_passes_test were introduced, and there was no need to repeat >> logic. >> - It's now just a very specific version of `user_passes_test` and is only >> used internally one place - the admindocs app. >> >> >> Tobias - In terms of permissions and redirecting; the behaviour of >> admin_view and `staff_member_required` are very similar. The differences >> are that: >> i) admin_view defers to Adminsite.has_permission to do the actual >> checking of permissions (so if that method is overwritten the behaviour >> will change) >> ii) admin_view does a whole load of other things that all admin views >> are going to need (mentioned above). >> iii) They have slightly different arguments you can pass in to modify >> the behaviour >> >> If you are decorating a view of ModelAdmin, or AdminSite you should use >> admin_view - as described in the docs under get_urls. >> >> Since staff_member_required is more or less a very specific version of >> user_passes_test designed for admin views, it's use-case seems pretty >> niche. If you were making an app like admindocs which behaves like an >> extension of the admin app, without actually being part of the admin app >> you could use it then, but I'm struggling to think of any other time you >> would want to use it though. >> >> I think there's an argument to be made for deprecating it completely: >> - It's only used in one place internally, and the wider use case seems >> pretty niche. >> - user_passes_test can do the same thing and seems to be the standard >> way of doing this kind of thing anyway. >> - user_passes_test with a suitably named test argument would also be >> more explicit, since the login_url and redirect_field_name would need to be >> explicitly stated. >> >> Definitely the documentation should be updated though. Thoughts on >> deprecating it? >> >> Tim >> >> >> >> On Tue, Mar 9, 2021 at 7:08 PM Tobias Bengfort <tobias.bengf...@posteo.de> >> wrote: >> >> On 09/03/2021 18.03, Carlton Gibson wrote: >> > Do you want to open a PR with suggested changes as a focus for >> discussion? >> >> I would open a PR that improves the docs, but I still don't understand >> the difference, so I wouldn't know what to write. >> >> The other alternative would be remove one of them, but at this point >> that is probably too drastic. >> >> 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. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/django-developers/d10799d4-8a42-6caa-8e13-c2ce93256521%40posteo.de >> <https://us-east-2.protection.sophos.com?d=google.com&u=aHR0cHM6Ly9ncm91cHMuZ29vZ2xlLmNvbS9kL21zZ2lkL2RqYW5nby1kZXZlbG9wZXJzL2QxMDc5OWQ0LThhNDItNmNhYS04ZTEzLWMyY2U5MzI1NjUyMSU0MHBvc3Rlby5kZQ==&i=NWVjN2YxNzUxNGEyNzMxNmMyMGRkZGU1&t=MDN1T0ZiM2oxaW9lVUVKMzFTSDhqTFpqQUVDbkw5VTVWaCtzb0JEYlp3bz0=&h=64eb5f1d05c6477aa783f14819c9eebe> >> . >> >> -- >> 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 view this discussion on the web visit >> https://groups.google.com/d/msgid/django-developers/CAFBPPP0bM2cr9etcMQwmprK1zX8gfKZ2wmEa7BW2CyiCrxTUog%40mail.gmail.com >> <https://us-east-2.protection.sophos.com?d=google.com&u=aHR0cHM6Ly9ncm91cHMuZ29vZ2xlLmNvbS9kL21zZ2lkL2RqYW5nby1kZXZlbG9wZXJzL0NBRkJQUFAwYk0yY3I5ZXRjTVF3bXBySzF6WDhnZktaMndtRWE3QlcyQ3lpQ3J4VFVvZyU0MG1haWwuZ21haWwuY29tP3V0bV9tZWRpdW09ZW1haWwmdXRtX3NvdXJjZT1mb290ZXI=&i=NWVjN2YxNzUxNGEyNzMxNmMyMGRkZGU1&t=d1F0bm5MRWYxTDcvRE53NFI5MXFpc216U2t2cUYydE93c0ZkWi9TeEszTT0=&h=64eb5f1d05c6477aa783f14819c9eebe> >> . >> >> -- >> 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 view this discussion on the web visit >> https://groups.google.com/d/msgid/django-developers/0158dd682e26429ea4e5b12ad2a548c4%40iss2.ISS.LOCAL >> <https://groups.google.com/d/msgid/django-developers/0158dd682e26429ea4e5b12ad2a548c4%40iss2.ISS.LOCAL?utm_medium=email&utm_source=footer> >> . >> > -- > 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 view this discussion on the web visit > https://groups.google.com/d/msgid/django-developers/CAFBPPP0rBhf1vfYa7p9RWY_bkmUCM0w4o2DyTwyUZYhSN_S0vQ%40mail.gmail.com > <https://groups.google.com/d/msgid/django-developers/CAFBPPP0rBhf1vfYa7p9RWY_bkmUCM0w4o2DyTwyUZYhSN_S0vQ%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > -- 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 view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAFv-zfJAiEqqX_X8FZK2w47JcCLhKqQ2VwJ%2Bgs%3D4fjzfAs-PeA%40mail.gmail.com.
