Fixed #24253 -- Documented staff_member_required decorator

[akulakov]

Added staff_member_required section to auth/default.txt

[timgraham]

I don't think the docs belong in docs/topics/auth/default.txt -- that should be limited to features of contrib.auth in my opinion.

[akulakov]

I looked at the admin doc and auth doc and it seemed a more suitable place for a couple of reasons:

1. From the user's POV, it's very similar to but an extension of login_required. It might even make sense to shorten it into saying something like "just like login_required above but checks if it's an active staff user".

This is probably the first place most users will look for this decorator.

1. Auth default.txt doc already has a lot of admin-related stuff, even larger sections. I'm not sure why it was done like that, but probably because a lot of stuff in admin is related to users and auth but it would not be an obvious place to look for that, so auth docs currently mean "Things related to auth from various packages" rather than just auth package.

Here's an example of admin-related sections in auth doc:

Managing users in the admin

When you have both django.contrib.admin and django.contrib.auth installed, the admin provides a convenient way to view and manage users, groups, and permissions. Users can be created and deleted like any Django model. Groups can be created, and permissions can be assigned to users or groups. A log of user edits to models made within the admin is also stored and displayed.

Creating Users
You should see a link to “Users” in the “Auth” section of the main admin index page. The “Add user” admin page is different than standard admin pages in that it requires you to choose a username and password before allowing you to edit the rest of the user’s fields.

Also note: if you want a user account to be able to create users using the Django admin site, you’ll need to give them permission to add users and change users (i.e., the “Add user” and “Change user” permissions). If an account has permission to add users but not to change them, that account won’t be able to add users. Why? Because if you have permission to add users, you have the power to create superusers, which can then, in turn, change other users. So Django requires add and change permissions as a slight security measure.

[timgraham]

Okay, I see. I think the .. function:: staff_member_required bit should be in the reference document though with a description of its arguments, etc. Then its usage can be discussed further in the topic guide. Could you give that try?

[akulakov]

Good point, updated admin/index.txt..

[timgraham]

omit this

[timgraham]

How does this look:

diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt
index d638f36..0c0a1da 100644
--- a/docs/ref/contrib/admin/index.txt
+++ b/docs/ref/contrib/admin/index.txt
@@ -2717,3 +2717,28 @@ The action in the examples above match the last part of the URL names for
 :class:`ModelAdmin` instances described above. The ``opts`` variable can be any
 object which has an ``app_label`` and ``model_name`` attributes and is usually
 supplied by the admin views for the current model.
+
+.. currentmodule:: django.contrib.admin.views.decorators
+
+The ``staff_member_required`` decorator
+=======================================
+
+.. function:: staff_member_required([redirect_field_name=REDIRECT_FIELD_NAME, login_url='admin:login'])
+
+    A view decorated with this function will having the following behavior:
+
+    * If the user is logged in, is a staff member (``User.is_staff=True``), and
+      is active (``User.is_active=True``), execute the view normally.
+
+    * Otherwise, the request will be redirected to the URL specified by the
+      ``login_url`` parameter, with the originally requested path in a query
+      string variable specified by ``redirect_field_name``. For example:
+      ``/accounts/login/?next=/polls/3/``.
+
+    Example usage::
+
+        from django.contrib.admin.views.decorators import staff_member_required
+
+        @staff_member_required
+        def my_view(request):
+            ...
diff --git a/docs/topics/auth/default.txt b/docs/topics/auth/default.txt
index 156746e..1905b8e 100644
--- a/docs/topics/auth/default.txt
+++ b/docs/topics/auth/default.txt
@@ -485,7 +485,14 @@ The login_required decorator

 .. note::

-    The login_required decorator does NOT check the is_active flag on a user.
+    The ``login_required`` decorator does NOT check the ``is_active`` flag on a
+    user.
+
+.. seealso::
+
+    If you are writing custom views for Django's admin, you may find the
+    :func:`django.contrib.admin.views.decorators.staff_member_required`
+    decorator a useful alternative to ``login_required()``.

 Limiting access to logged-in users that pass a test

[akulakov]

I think it would be better to have a section in auth/default.txt for this decorator because it's consistent with how other decorators are listed there and it's more discoverable by users.

In other words, let's say I'm a user glancing through the topic list and through the body of the page, I will see the decorators listed and my conclusion will be that these are the auth-related decorators available (EDIT: and by implication no other auth-related decorators are available).

With your diff, the user has to actively seek out this decorator (and even then it may be easy to overlook the 'seealso').

I think it boils down to whether auth/default.txt is unusually badly structured compared to other django docs or more or less consistent with them, as it stands now it's not restricted to auth module but has items from other packages, and also has some reference-style sections.

Since this is a small addition, it might make sense to keep it consistent with this page for now [EDIT: and clean up the entire page in a consistent style at some point].

However I don't feel strongly about any of this because you can also argue that if user needs it, they will search for it either in page or google (and this particular decorator is highly searchable).

It's partly my personal preference for finding things through documentation tree structure / listing vs. searching, I know that there's a split between users who prefer one approach to the other.

[EDIT2: another consideration is that the section includes full doc for user_passes_test which
means they can easily implement this check themselves, which means it's not a big deal
if there's no separate section for staff_member_requried]

[akulakov]

Forgot to add:

If you are writing custom views for Django's admin, you may find the

It seems to me though that it's not restricted to custom views for django's admin app, it often may
be a view in other apps that should be restricted to privileged users, for example in a recent
app I worked on there was a view that allowed deletion of recently added records for testing,
I've added this decorator as an additional safety to make sure it would not be triggered by
normal users (although there's other safety checks as well).

[timgraham]

I guess why I'm hesitant to make heavy mention of it in the auth doc is that it's part of contrib.admin. Many of the admin features discussed in that doc are actually located in contrib.auth.admin. Maybe this is more a purity argument than a practical one. With the exception of the fact that the decorator uses login_url='admin:login', it could probably just as easily live in contrib.auth. Anyway, like you said, I think people who want this will find it without too much trouble no matter where it is, so I'll make an amendment based on your last comment and then commit this. Thanks!

[timgraham]

merged in e8a758e.

[akulakov]

Looks good, thanks!