Re: plea for re-opening ticket 13125 marked as won't fix
Here's an idea that might work better than a decorator: Create a setting called ALLOW_DEACTIVATED_LOGINS. Then modify auth.login() to enforce this as well as changing ModelBackend.get_user() to logout users whose accounts are disabled. Make the setting True by default in 1.4 and announce it'll be set to False in either 1.5 or 1.6. This way is_active can be secure by default, not require users to implement its functionality themselves while also following the principle of least astonishment. The only confusing this about this solution is that users might not understand it doesn't affect the admin login form and auth.views.login On Mon, Sep 12, 2011 at 3:43 AM, Florian Apollonerwrote: > Probably yeah, on the other hand the docs tell you that is_active doesn't > neccessarily have to be checked by backends, so if a backend allows to login > inactive users it makes no sense to check that flag in login_required… I > guess what I am proposing is that the login_required flag checks via the > auth backends whether or not the user should be allowed to pass, that way > all the neccessary checks stay in one place… > > -- > You received this message because you are subscribed to the Google Groups > "Django developers" group. > To view this discussion on the web visit > https://groups.google.com/d/msg/django-developers/-/A922lTjpZc8J. > > To post to this group, send email to django-developers@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. > -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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.
Re: plea for re-opening ticket 13125 marked as won't fix
Probably yeah, on the other hand the docs tell you that is_active doesn't neccessarily have to be checked by backends, so if a backend allows to login inactive users it makes no sense to check that flag in login_required… I guess what I am proposing is that the login_required flag checks via the auth backends whether or not the user should be allowed to pass, that way all the neccessary checks stay in one place… -- You received this message because you are subscribed to the Google Groups "Django developers" group. To view this discussion on the web visit https://groups.google.com/d/msg/django-developers/-/A922lTjpZc8J. To post to this group, send email to django-developers@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.
Re: plea for re-opening ticket 13125 marked as won't fix
Hi Florian, Then again, the default behaviour now is as you describe. That's why I would call it a security leak. Unfortunately, it is not only my system, it is the system of any unaware Django programmer. Wim On Sep 11, 10:24 pm, Florian Apollonerwrote: > On Sunday, September 11, 2011 8:52:03 PM UTC+2, Wim Feijen wrote: > > > 3. Because the user is still logged in, (maybe for two weeks, or for > > whatever expiration time the developer has set) > > Imo in that case the developer should write a middleware that logs the user > out on the next request. I see your problem, but imo your system needs a bit > of tweaking if you allow inactive users to browse your site till their > session expires (which with SAVE_EVERY_REQUEST + a high timeout) could as > well be never… -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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.
Re: plea for re-opening ticket 13125 marked as won't fix
On Sunday, September 11, 2011 8:52:03 PM UTC+2, Wim Feijen wrote: > > 3. Because the user is still logged in, (maybe for two weeks, or for > whatever expiration time the developer has set) > Imo in that case the developer should write a middleware that logs the user out on the next request. I see your problem, but imo your system needs a bit of tweaking if you allow inactive users to browse your site till their session expires (which with SAVE_EVERY_REQUEST + a high timeout) could as well be never… -- You received this message because you are subscribed to the Google Groups "Django developers" group. To view this discussion on the web visit https://groups.google.com/d/msg/django-developers/-/gXvWPvnJThgJ. To post to this group, send email to django-developers@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.
Re: plea for re-opening ticket 13125 marked as won't fix
Paul, On further thinking, I do believe that the current behaviour of login_required should be considered a bug. Wim On 10 sep, 02:03, Paul McMillanwrote: > No. Django makes an incredibly strong promise about backwards > compatibility to its users. Security releases are the ONLY reason we > modify behavior in backwards incompatible fashions, and we try very > hard to avoid that. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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.
Re: plea for re-opening ticket 13125 marked as won't fix
The case is as follows: 1. An active user is logged in and has a valid session. 2. The user is inactivated by a system admin, who wants to disable the user. 3. Because the user is still logged in, (maybe for two weeks, or for whatever expiration time the developer has set), he passes the login_required decorator, and still can see those pages which we naively thought were being protected by the login_required decorator, because that decorator doesn't check for is_active. This patch is a patch for that problem. Wim On 10 sep, 23:09, Florian Apollonerwrote: > Stupid question, but why do you let inactive users login at all? I mean is > this really a problem of the decorator and not of the login system you use?! -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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.
Re: plea for re-opening ticket 13125 marked as won't fix
Stupid question, but why do you let inactive users login at all? I mean is this really a problem of the decorator and not of the login system you use?! -- You received this message because you are subscribed to the Google Groups "Django developers" group. To view this discussion on the web visit https://groups.google.com/d/msg/django-developers/-/vLiHd62iAJ0J. To post to this group, send email to django-developers@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.
Re: plea for re-opening ticket 13125 marked as won't fix
I added an active_login_required decorator. See: https://code.djangoproject.com/ticket/16797 Is it a good name? It is a good patch? Or is it stupid? Thanks, Wim On 10 sep, 02:27, Wim Feijenwrote: > Thanks Paul, > > I like the idea of the additional decorator! Let's do that. > > Wim > > On 10 sep, 02:03, Paul McMillan wrote: > > > > > > > > > > I'd like to make a case to re-open ticket 13125. > > > Thanks for taking this to the mailing list rather than arguing in trac. > > > > I understand that changing the current behaviour is backwards- > > > incompatible and therefor very unwanted. But, I'd say the current > > > implementation is forward-incompatible: meaning that current and > > > future users will stumble on something counter-intuitive and be amazed > > > that an inactive user can pass a login_required. > > > No. Django makes an incredibly strong promise about backwards > > compatibility to its users. Security releases are the ONLY reason we > > modify behavior in backwards incompatible fashions, and we try very > > hard to avoid that. > > > > For me, the current behaviour is contrary to most peoples expectation, > > > and my proposal would be to make the backwards-incompatible change to > > > make django more consistent (I might even say: more logical), which I > > > think is a good thing. > > > Yeah, I agree that the current behavior is counter intuitive. It is an > > oddity and a wart that exists. > > > > My proposal is also to add an active_or_inactive_login_required > > > decorator (a better name is welcome) which just checks whether a user > > > is authenticated; and then people could import that as login_required. > > > I wouldn't be opposed to an additional decorator which makes better > > grammatical sense and does explicitly what you want. We just can't > > change the behavior of the current one. If you can come up with two > > new ones that make better sense there might be an argument for slowly > > deprecating the existing one. > > > > The consequence is that some people would need to make a change to > > > keep their code working in Django 1.4 , but it is my belief that this > > > is only a small part of the Django population who have the skills to > > > adapt and that it will have a benificial effect to most current and > > > all future users. > > > No. We do not do this. Otherwise every release would end up stuffed > > full of dozens of "tiny easy changes" which means nobody would bother > > updating. > > > Regards, > > -Paul -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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.
Re: plea for re-opening ticket 13125 marked as won't fix
> I'd like to make a case to re-open ticket 13125. Thanks for taking this to the mailing list rather than arguing in trac. > I understand that changing the current behaviour is backwards- > incompatible and therefor very unwanted. But, I'd say the current > implementation is forward-incompatible: meaning that current and > future users will stumble on something counter-intuitive and be amazed > that an inactive user can pass a login_required. No. Django makes an incredibly strong promise about backwards compatibility to its users. Security releases are the ONLY reason we modify behavior in backwards incompatible fashions, and we try very hard to avoid that. > For me, the current behaviour is contrary to most peoples expectation, > and my proposal would be to make the backwards-incompatible change to > make django more consistent (I might even say: more logical), which I > think is a good thing. Yeah, I agree that the current behavior is counter intuitive. It is an oddity and a wart that exists. > My proposal is also to add an active_or_inactive_login_required > decorator (a better name is welcome) which just checks whether a user > is authenticated; and then people could import that as login_required. I wouldn't be opposed to an additional decorator which makes better grammatical sense and does explicitly what you want. We just can't change the behavior of the current one. If you can come up with two new ones that make better sense there might be an argument for slowly deprecating the existing one. > The consequence is that some people would need to make a change to > keep their code working in Django 1.4 , but it is my belief that this > is only a small part of the Django population who have the skills to > adapt and that it will have a benificial effect to most current and > all future users. No. We do not do this. Otherwise every release would end up stuffed full of dozens of "tiny easy changes" which means nobody would bother updating. Regards, -Paul -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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.
Re: plea for re-opening ticket 13125 marked as won't fix
> There are numerous counter arguments to the idea: Unintended > consequences. There is a possibility for race conditions, which would > then be security issues. Action at distance. I don't know if this is > possible to implement for all session backends. It's impossible to implement for cookie-based session backends. I'd probably be opposed to a behavior like that which worked with some backends and not others (though that backend is a weird special case). -Paul -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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.
Re: plea for re-opening ticket 13125 marked as won't fix
One approach would be to invalidate the sessions of the user when is_active is changed from True to False. This way the current registration method would work, and there would not be surprising "can use site as long as session is open" situations, because there would not be any open sessions. There are numerous counter arguments to the idea: Unintended consequences. There is a possibility for race conditions, which would then be security issues. Action at distance. I don't know if this is possible to implement for all session backends. Just an idea maybe worth discussing. - Anssi On Sep 10, 12:03 am, Wim Feijenwrote: > Jakob, thanks for looking into 13125 and taking action on it. > > I'd like to make a case to re-open ticket 13125. > > I understand that changing the current behaviour is backwards- > incompatible and therefor very unwanted. But, I'd say the current > implementation is forward-incompatible: meaning that current and > future users will stumble on something counter-intuitive and be amazed > that an inactive user can pass a login_required. > > For me, the current behaviour is contrary to most peoples expectation, > and my proposal would be to make the backwards-incompatible change to > make django more consistent (I might even say: more logical), which I > think is a good thing. > > My proposal is also to add an active_or_inactive_login_required > decorator (a better name is welcome) which just checks whether a user > is authenticated; and then people could import that as login_required. > > The consequence is that some people would need to make a change to > keep their code working in Django 1.4 , but it is my belief that this > is only a small part of the Django population who have the skills to > adapt and that it will have a benificial effect to most current and > all future users. > > Sorry that I raise this question again, but it is my strongest belief > that it will make Django better. > > Wim -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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.
plea for re-opening ticket 13125 marked as won't fix
Jakob, thanks for looking into 13125 and taking action on it. I'd like to make a case to re-open ticket 13125. I understand that changing the current behaviour is backwards- incompatible and therefor very unwanted. But, I'd say the current implementation is forward-incompatible: meaning that current and future users will stumble on something counter-intuitive and be amazed that an inactive user can pass a login_required. For me, the current behaviour is contrary to most peoples expectation, and my proposal would be to make the backwards-incompatible change to make django more consistent (I might even say: more logical), which I think is a good thing. My proposal is also to add an active_or_inactive_login_required decorator (a better name is welcome) which just checks whether a user is authenticated; and then people could import that as login_required. The consequence is that some people would need to make a change to keep their code working in Django 1.4 , but it is my belief that this is only a small part of the Django population who have the skills to adapt and that it will have a benificial effect to most current and all future users. Sorry that I raise this question again, but it is my strongest belief that it will make Django better. Wim -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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.