Opened 3 years ago

Closed 21 months ago

#30348 closed New feature (wontfix)

Add superuser_required decorator

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

Description (last modified by Sultan Iman)

Create a new decorator superuser_required and SuperuserRequiredMixin which has use cases when only super users can access certain views.
Github PR is at https://github.com/django/django/pull/10640

Change History (7)

comment:1 Changed 3 years ago by Tobias Kunze

Summing up the discussion on the GitHub PR here:

On the plus side, Django does provide very similar decorators and mixins, so it is surprising that superuser_required is not already a part of Django.

On the other hand, adding a decorator like this is trivial with user_passes_test. We could add a decorator like this to the user_passes_test documentation, to make sure searching for this (fairly reasonable) requirement yields helpful information.

comment:2 Changed 3 years ago by Sultan Iman

Description: modified (diff)

comment:3 in reply to:  1 Changed 3 years ago by Sultan Iman

Replying to Tobias Kunze:

Summing up the discussion on the GitHub PR here:

On the plus side, Django does provide very similar decorators and mixins, so it is surprising that superuser_required is not already a part of Django.

On the other hand, adding a decorator like this is trivial with user_passes_test. We could add a decorator like this to the user_passes_test documentation, to make sure searching for this (fairly reasonable) requirement yields helpful information.

Hi Tobias,

Thanks for reviewing! Also agree that it is easily achievable. However I believe providing these out of the box is a good developer experience as well as convenience.

---
Kind regards,
Sultan.

comment:4 Changed 3 years ago by Carlton Gibson

Triage Stage: UnreviewedAccepted

Given the discussion on the PR, I'm happy to accept this to at least push it forward for review.
(If objections do arise we can switch to the documentation example...)

comment:5 Changed 3 years ago by David Foster

I'm not sure adding a @superuser_required is a good idea: It effectively creates a special permission that only superusers have, which might encourage users to be given the superuser bit. Unnecessarily giving a superuser bit seems questionable for security. I don't think we should encourage going down this route out-of-the-box.

comment:8 Changed 21 months ago by Jacob Walls

Has patch: set
Owner: set to Sultan Iman
Status: newassigned

comment:9 Changed 21 months ago by Carlton Gibson

Resolution: wontfix
Status: assignedclosed

Hi all. On review I think we should close this as wontfix.

I agree with David's comment:5. It's questionable whether you should this at all: to the extent that it's possible, you should avoid creating and having superusers — use the permissions system.

Then, if you really do want this, it's a one-liner with user_passes_test(). (Given the previous point, I'm not inclined to add that example to the docs. Folks who want it will work it out.)

I hope that makes sense.

Note: See TracTickets for help on using tickets.
Back to Top