login_required decorator and user.is_active (yes, again)

256 views
Skip to first unread message

ptone

unread,
Oct 5, 2011, 3:59:13 PM10/5/11
to Django developers

13125 was wontfixed by Jacob at the end of the summer, this relates to
the login_required decorator not checking for user.is_active

I had opened a duplicate ticket 16996 before catching it as a dupe

I'm going to dredge this one back up.

At a minimum, the current, counter-intuitive behavior of
login_required needs to be documented, but before I open a doc ticket
for that, I wanted to give this another shot here.

On Apr 15 2010, 5:55 pm, Russell Keith-Magee <freakboy3...@gmail.com>
wrote:
> On Thu, Apr 15, 2010 at 11:20 PM, subs...@gmail.com <subs...@gmail.com> wrote:
> > Thanks for breaking it down.
>
> > On Mar 17, 7:45 am, Russell Keith-Magee <freakboy3...@gmail.com>
> > wrote:
>
> >>  1) Don't touch the code. It's an annoying edge case, but it can be
> >> caught by shortening session timeouts and making more use of
> >> permissions. However, we should document the edge case with
> >>login_required, as it is certainly contrary to obvious usage.

Note, this is not yet documented in trunk.

> As I indicated previously in this thread, there are two use cases that
> need to be handled:
>
>  1) Normal Django authentication, honoring the is_activeflag. For
> this use case, you are completely correct - the current behavior is
> wrong.
>
>  2) Custom authentication backends that *don't* honor the is_active
> flag. This is entirely legal, and documented as such.

However, as of 1.3, the expectation is that backends will "handle" an
inactive user.

It is unclear from the current docs, what exactly the expectation is,
but it implies that any custom auth backend needs to support the
is_active attr on the user_obj.

"Django 1.5 will assume that every backend supports inactive users
being passed to the authorization methods."

Right now, it is confusing as to how the is_active flag is treated by
contrib.auth. While the auth system involves several layers, there is
basically two types of implementation: "the built in default" and
"custom" (anything other than the builtin). It is inconssistant to
have one part of the built in defaults (the login form) check this
attribute, but have other parts not check it (the decorator). I know
that the defaults aren't explicitly a "set" in this way, none the
less, I think generally that people consider the out of box experience
to be coherent.

I think that there would be pretty universal agreement about the
expectation of what should happen if you make a previously active user
inactive with regard to their access to @login_required protected
views. Shortening session times, seems like a really unpalatable
workaround for sites that have chosen longer/default session
expiration times.

During the deprecation, the decorator could check
user.backend.supports_inactive_user, as unlike at the time of the
above thread, the user object is not annotated with the backend.

-Preston

Brian Neal

unread,
Oct 5, 2011, 4:39:28 PM10/5/11
to Django developers
On Oct 5, 2:59 pm, ptone <pres...@ptone.com> wrote:
> 13125 was wontfixed by Jacob at the end of the summer, this relates to
> the login_required decorator not checking for user.is_active
>
> I had opened a duplicate ticket 16996 before catching it as a dupe
>
> I'm going to dredge this one back up.

...

> I think that there would be pretty universal agreement about the
> expectation of what should happen if you make a previously active user
> inactive with regard to their access to @login_required protected
> views.  Shortening session times, seems like a really unpalatable
> workaround for sites that have chosen longer/default session
> expiration times.

If we are talking about work-arounds, have you seen this middleware?
This does the job perfectly for me and it is so simple:

http://djangosnippets.org/snippets/1105/

Regards,
BN

Reply all
Reply to author
Forward
0 new messages