Re: password is None in check_password
I just want to summarize the password handling changes before and after this patch. before: - create_user() with empty password or None -> password set to '!' unusable - set_password() accepts everything - has_usable_password() returning False for password equal to '!' after: - create_user() with password None -> password set to '!' unusable - set_password() with None -> password set to '!' unusable - has_usable_password() returning False for password equal to '!' or None Laurent On Oct 8, 4:47 pm, Laurent Luce wrote: > Thanks for your feedback. I attached a new patch with the following > changes: > > * allow empty string in set_password() > * has_usable_password() returns false if password is '!' or None > * add unit test set_password(None) > * add verbosity option to createsuperuser command + unit test > * output msg to stdout in createsuperuser command + update unit tests > > Let me know if the patch still needs improvement. > > Laurent Luce > > On Oct 7, 11:17 pm, Russell Keith-Magee > wrote: > > > On Fri, Oct 8, 2010 at 11:28 AM, Laurent Luce > > wrote: > > > I noticed that create_user() is currently setting password to unusable > > > if it is None or empty. However, set_password() is accepting an empty > > > password. I decided to follow the first rule in the patch I submitted > > > but I am kind of confused now. Can someone indicate what we should > > > accept as a password? > > > An empty string isn't a *good* password, but then neither is a single > > character or a dictionary word, and we don't reject those. Policy > > decisions like this aren't the domain of a web framework. > > > I'm sure there are also people using an empty password as the "I don't > > actually want security" password. Plus, there's a history in the free > > software community of using empty passwords as a protest [1] :-) > > > We can use None to mark an unusable password. Absent of a good > > technical reason, I don't see why we should reject empty string. > > > As for the remainder of the patch on #14354 -- on the whole, it looks > > good. I've put some review comments on the ticket. > > > [1]http://en.wikipedia.org/wiki/Richard_Stallman#Early_years > > > 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.
Re: password is None in check_password
Thanks for your feedback. I attached a new patch with the following changes: * allow empty string in set_password() * has_usable_password() returns false if password is '!' or None * add unit test set_password(None) * add verbosity option to createsuperuser command + unit test * output msg to stdout in createsuperuser command + update unit tests Let me know if the patch still needs improvement. Laurent Luce On Oct 7, 11:17 pm, Russell Keith-Magee wrote: > On Fri, Oct 8, 2010 at 11:28 AM, Laurent Luce wrote: > > I noticed that create_user() is currently setting password to unusable > > if it is None or empty. However, set_password() is accepting an empty > > password. I decided to follow the first rule in the patch I submitted > > but I am kind of confused now. Can someone indicate what we should > > accept as a password? > > An empty string isn't a *good* password, but then neither is a single > character or a dictionary word, and we don't reject those. Policy > decisions like this aren't the domain of a web framework. > > I'm sure there are also people using an empty password as the "I don't > actually want security" password. Plus, there's a history in the free > software community of using empty passwords as a protest [1] :-) > > We can use None to mark an unusable password. Absent of a good > technical reason, I don't see why we should reject empty string. > > As for the remainder of the patch on #14354 -- on the whole, it looks > good. I've put some review comments on the ticket. > > [1]http://en.wikipedia.org/wiki/Richard_Stallman#Early_years > > 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.
Re: password is None in check_password
On Fri, Oct 8, 2010 at 11:28 AM, Laurent Luce wrote: > I noticed that create_user() is currently setting password to unusable > if it is None or empty. However, set_password() is accepting an empty > password. I decided to follow the first rule in the patch I submitted > but I am kind of confused now. Can someone indicate what we should > accept as a password? An empty string isn't a *good* password, but then neither is a single character or a dictionary word, and we don't reject those. Policy decisions like this aren't the domain of a web framework. I'm sure there are also people using an empty password as the "I don't actually want security" password. Plus, there's a history in the free software community of using empty passwords as a protest [1] :-) We can use None to mark an unusable password. Absent of a good technical reason, I don't see why we should reject empty string. As for the remainder of the patch on #14354 -- on the whole, it looks good. I've put some review comments on the ticket. [1] http://en.wikipedia.org/wiki/Richard_Stallman#Early_years 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.
Re: password is None in check_password
I noticed that create_user() is currently setting password to unusable if it is None or empty. However, set_password() is accepting an empty password. I decided to follow the first rule in the patch I submitted but I am kind of confused now. Can someone indicate what we should accept as a password? On Oct 7, 3:25 pm, Łukasz Rekucki wrote: > Hi, > > I started working on a somehow related ticket #14390. Adrian suggested > to create a utils module, so I wanted to put there all useful > password-related functions: check_password(), make_password(), > is_password_usable() and the UNUSABLE_PASSWORD constant. So I'm > interested in API that this functions (and thus User's methods) should > provide. > > On 7 October 2010 23:13, Laurent Luce wrote:> Hello, > > > Regarding the issue about password is None in check_password (http:// > > code.djangoproject.com/ticket/14354). I attached a patch with the > > following changes: > > > - in set_password(), check for raw_password and if None or empty, call > > set_unusable_password(), otherwise same as before > > In theory, someone could allow empty passwords which will get hashed properly. > > > > > - in has_usable_password(), return True only if password is not None, > > not empty, or '!' > > There are actually lot of other unusable values - at the moment, > anything that isn't a MD5 hash or a legitimate password string in > algo$salt$hash format. Is it ok, to only special case this three > values ? > > > > > - because of the 2 changes above, we can simplify a bit create_user() > > by just calling set_password() for all cases. No need to test password > > inside this function anymore. > > > - basic.py tests are now unittests and not doctests > > Good work :) > > -- > Łukasz Rekucki -- 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.
Re: password is None in check_password
Hi, I started working on a somehow related ticket #14390. Adrian suggested to create a utils module, so I wanted to put there all useful password-related functions: check_password(), make_password(), is_password_usable() and the UNUSABLE_PASSWORD constant. So I'm interested in API that this functions (and thus User's methods) should provide. On 7 October 2010 23:13, Laurent Luce wrote: > Hello, > > Regarding the issue about password is None in check_password (http:// > code.djangoproject.com/ticket/14354). I attached a patch with the > following changes: > > - in set_password(), check for raw_password and if None or empty, call > set_unusable_password(), otherwise same as before In theory, someone could allow empty passwords which will get hashed properly. > > - in has_usable_password(), return True only if password is not None, > not empty, or '!' There are actually lot of other unusable values - at the moment, anything that isn't a MD5 hash or a legitimate password string in algo$salt$hash format. Is it ok, to only special case this three values ? > > - because of the 2 changes above, we can simplify a bit create_user() > by just calling set_password() for all cases. No need to test password > inside this function anymore. > > - basic.py tests are now unittests and not doctests Good work :) -- Łukasz Rekucki -- 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.
password is None in check_password
Hello, Regarding the issue about password is None in check_password (http:// code.djangoproject.com/ticket/14354). I attached a patch with the following changes: - in set_password(), check for raw_password and if None or empty, call set_unusable_password(), otherwise same as before - in has_usable_password(), return True only if password is not None, not empty, or '!' - because of the 2 changes above, we can simplify a bit create_user() by just calling set_password() for all cases. No need to test password inside this function anymore. - basic.py tests are now unittests and not doctests I assumed that has_usable_password() will be called directly by the user code so I didn't modify check_password(). Please let me know if my changes match what is expected. Laurent http://www.laurentluce.com http://www.gourmious.com -- 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.
