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. > > This isn't appropriate in situations where HR wants to do a hard cut- > off of a user's access, in the event they are terminated. > >> 2) Add a specific "requires_active_user" decorator (and maybe a >> decorator that combines the login_required and requires_active_user >> check) > > Eh. > >> 3) Allow login_required to take an argument that customizes the >> 'active' check. However, the default value for this argument would >> need to be equivalent to the current behaviour; once you start down >> this path, there really isn't much difference between a call to >> login_required with an 'is_active' argument, and a call to >> user_passes_test that does the authentication and active check. > > Seems strange: "I want a user to be authenticated to use this view, > but I don't care if they're active." I'm not sure I agree that the > default should be the current behavior. I think in most cases this is > an oversight that most programmers simply don't account for in their > code--and closing this hatch is a favor and not an incompatibility > issue. Could be wrong.
As I indicated previously in this thread, there are two use cases that need to be handled: 1) Normal Django authentication, honoring the is_active flag. 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. Unfortunately, the simple solution (add an is_active check to the login_required decorator) is a completely valid and reasonable solution for (1), but breaks (2). That's the sticking point. >> 4) Work out if there is a way to refactor the login process so that >> the decision of whether is_active is checked is deferred. I don't have >> any great ideas how this could be done, though. > > Would it be too ugly or simplistic to have a callable > Backend.is_active() which these backends could manipulate to their > liking? Its flexible in the same way as get_profile(). Also, is it > possible we're expecting too much perfection given that, as you say, > there are legitimate proposals to refactor auth.User either way? (and > present solutions are more or less trivial). In principle, this sounds like the right approach. However, there are a few implementation kinks: 1) Backends aren't required to subclass a common base class, so we have to assume that there will be custom backends out there that *wont'* have an is_active check, even if we add one to ModelBackend. This isn't a huge problem - we can work around it with a hasattr check - but it's worth noting as something that will need to be addressed. 2) How do you get access to the right authentication backend in the login_required decorator? The only argument you are guaranteed to have access to is user, and the user doesn't (currently) have a way to get access to the backend on which they were authenticated. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
