I think the problem isn't the login_required, but it simply does what it says it does: Check if the user is logged in.
For me a user with is_active set to false shouldn't be allowed to login, they either just created an account and still need to verify it or they indicated that they wanted their account "removed", in which case it's marked inactive so it doesn't cascade delete all their related items too. As the login view and the authentication middleware both use the backend to get the user, this is is the place to implement the check or not. Which isn't that hard, the quick fix is to simply point out that the is_active doesn't check this and if people want this they should simply extend the ModelBackend's authenticate and get_user functions and add the checks there. On Apr 15, 5: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. > > > 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). > > -Steve -- 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.
