@staff_member_required handling of non-staff users

[Tim Graham]

We received a ticket [1] noting that when a non-staff user tries to access an admin page, they will be redirected to the admin login page with no explanation. A pull request [2] proposes to add this message to login page if a user is authenticated, "While you are authenticated as {{ username }}, you are unfortunately not authorized to access this page -- would you care to re-login?" -- the thinking being that any authenticated user viewing the login page will have reached it via such a redirect. (Logged in staff users cannot view the login page because they are redirected back to the admin index page if they try to access it).

I think it's a bit odd for the @staff_member_required to redirect non-staff users to the login page (which is a side effect of using the user_passes_test() function) as if many users have two separate "staff" and "non-staff" accounts. Instead I propose to raise PermissionDenied so that developers can use handler403 to customize the behavior.

What are your thoughts on this issue?

[1] https://code.djangoproject.com/ticket/25163
[2] https://github.com/django/django/pull/5044

[Carl Meyer]

On 07/28/2015 12:36 PM, Tim Graham wrote:
> We received a ticket [1] noting that when a non-staff user tries to
> access an admin page, they will be redirected to the admin login page
> with no explanation. A pull request [2] proposes to add this message to
> login page if a user is authenticated, "While you are authenticated as
> {{ username }}, you are unfortunately not authorized to access this page
> -- would you care to re-login?" -- the thinking being that any
> authenticated user viewing the login page will have reached it via such
> a redirect. (Logged in staff users cannot view the login page because
> they are redirected back to the admin index page if they try to access it).

This solution seems reasonable to me.

> I think it's a bit odd for the @staff_member_required to redirect
> non-staff users to the login page (which is a side effect of using the
> user_passes_test() function) as if many users have two separate "staff"
> and "non-staff" accounts.

I think that probably varies quite a bit by project. There are certainly
projects (especially on staging sites where a lot of testing occurs that
sometimes requires multiple accounts) where I do have multiple accounts,
one staff and others not, and I've used the re-login ability.

> Instead I propose to raise PermissionDenied so
> that developers can use handler403 to customize the behavior.

I guess this should be somewhat practical now that we have
https://github.com/django/django/commit/70779d9c1cab77791c73b72e8a63f60184d8f2b0
-- without that it would be hard to distinguish one type of 403 from
another.

It feels a bit ugly to me in the cases where you want to return
something other than a 403 (e.g. the 302 that is returned currently)
that you'd have to do that via `handler403`. And you'd still have the
`got_request_exception` signal sent, even though you are choosing to
handle it in a non-exceptional fashion.

> What are your thoughts on this issue?

I think the proposed PR for a UX hint is a good improvement on the
status quo, regardless.

In theory I like the idea of being able to customize what happens when a
`staff_member_required` check fails. For that matter, I'd like it if
`user_passes_test` itself were more customizable, so it could be
configured to return either a redirect, a 403, or a 404.

I am not convinced that `handler403` is the right place to do this
customization. To me that seems like the place to customize your 403
responses, not to make policy decisions that may result in returning
something that isn't a 403 at all.

The obvious place for this type of customization to happen is in an
optional parameter to `@staff_member_required`. The problem in the admin
is that the decorator is applied internally, not by the developer.
Perhaps an `AdminSite` attribute?

Carl

[Collin Anderson]

I like the behavior of showing the login screen if they don't have permission. It's simple and pretty friendly. The new hint is helpful too.

[Luiz Santos]

+1

Sent from my iPad

--
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-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/84d74c82-3328-478f-8210-c5be481a5be8%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.