Re: [Django] #14354: Check password is not None in User.check_password

[Django]

#14354: Check password is not None in User.check_password
-+--
  Reporter:  berryp  | Owner:  laurentluce
Status:  closed  | Milestone:  1.3
 Component:  Authentication  |   Version:  1.2
Resolution:  fixed   |  Keywords: 
 Stage:  Accepted| Has_patch:  1  
Needs_docs:  0   |   Needs_tests:  0  
Needs_better_patch:  0   |  
-+--
Comment (by russellm):

 Laurent - thanks for the revised patch. For your benefit next time, I made
 a couple of minor tweaks before pushing to trunk:
  * Line length in the unit tests. Where practical and legible, keep lines
 under 80 chars
  * A trailing newline on the 'superuser created' message
  * A couple of explanatory comments and newline breaks in the tests so
 it's easy to follow what's going on.

 Other than that, thanks for the contribution!

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14354: Check password is not None in User.check_password

[Django]

#14354: Check password is not None in User.check_password
-+--
  Reporter:  berryp  | Owner:  laurentluce
Status:  assigned| Milestone:  1.3
 Component:  Authentication  |   Version:  1.2
Resolution:  |  Keywords: 
 Stage:  Accepted| Has_patch:  1  
Needs_docs:  0   |   Needs_tests:  0  
Needs_better_patch:  0   |  
-+--
Changes (by laurentluce):

  * needs_better_patch:  1 => 0

Comment:

 New patch attached:[[BR]]
  * 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

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14354: Check password is not None in User.check_password

[Django]

#14354: Check password is not None in User.check_password
-+--
  Reporter:  berryp  | Owner:  laurentluce
Status:  assigned| Milestone:  1.3
 Component:  Authentication  |   Version:  1.2
Resolution:  |  Keywords: 
 Stage:  Accepted| Has_patch:  1  
Needs_docs:  0   |   Needs_tests:  0  
Needs_better_patch:  1   |  
-+--
Changes (by russellm):

  * needs_better_patch:  0 => 1

Comment:

 Looks good, Laurent. Some minor review comments:

  * As discussed on the list, the check for invalidity should allow empty
 string, and use None to mark an unusable password.

  * Empty setup/teardown methods aren't needed. There's no need to formally
 declare a no-op.

  * The tests for the createsuperuser management commands output to stdout
 - tests shouldn't have any visible manifestation. As part of the migration
 to use unittests, we're modifying the admin commands to take stdout/stderr
 arguments, and writing output to those streams. See loaddata/dumpdata for
 examples. This allows the test commands (or any other programmatic usage)
 to provide a stream that will be used for output (which provides a second
 way to fix this test problem -- invoke the test with verbosity=0).

  * While you're in that areas, the "superuser created" text should also be
 covered by a verbosity check; verbosity=0 should output nothing.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14354: Check password is not None in User.check_password

[Django]

#14354: Check password is not None in User.check_password
-+--
  Reporter:  berryp  | Owner:  laurentluce
Status:  assigned| Milestone:  1.3
 Component:  Authentication  |   Version:  1.2
Resolution:  |  Keywords: 
 Stage:  Accepted| Has_patch:  1  
Needs_docs:  0   |   Needs_tests:  0  
Needs_better_patch:  0   |  
-+--
Comment (by laurentluce):

 Reformat the description.

 I attached a patch with the following changes:

 - in set_password(), check for raw_password and if None or , call
 set_unusable_password(), otherwise same as before[[BR]]

 - in has_usable_password(), return True only if password is not None, or
 '!'[[BR]]

 - 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 and
 branch out.[[BR]]

 - basic.py tests are now unittests and not doctests

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14354: Check password is not None in User.check_password

[Django]

#14354: Check password is not None in User.check_password
-+--
  Reporter:  berryp  | Owner:  laurentluce
Status:  assigned| Milestone:  1.3
 Component:  Authentication  |   Version:  1.2
Resolution:  |  Keywords: 
 Stage:  Accepted| Has_patch:  1  
Needs_docs:  0   |   Needs_tests:  0  
Needs_better_patch:  0   |  
-+--
Changes (by laurentluce):

  * has_patch:  0 => 1

Comment:

 I attached a patch with the following changes:

 - in set_password(), check for raw_password and if None or '', call
 set_unusable_password(), otherwise same as before
 - in has_usable_password(), return True only if password is not None, ''
 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 and
 branch out.
 - basic.py tests are now unittests and not doctests

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14354: Check password is not None in User.check_password

[Django]

#14354: Check password is not None in User.check_password
-+--
  Reporter:  berryp  | Owner:  laurentluce
Status:  assigned| Milestone:  1.3
 Component:  Authentication  |   Version:  1.2
Resolution:  |  Keywords: 
 Stage:  Accepted| Has_patch:  0  
Needs_docs:  0   |   Needs_tests:  0  
Needs_better_patch:  0   |  
-+--
Changes (by laurentluce):

  * status:  new => assigned

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14354: Check password is not None in User.check_password

[Django]

#14354: Check password is not None in User.check_password
-+--
  Reporter:  berryp  | Owner:  laurentluce
Status:  new | Milestone:  1.3
 Component:  Authentication  |   Version:  1.2
Resolution:  |  Keywords: 
 Stage:  Accepted| Has_patch:  0  
Needs_docs:  0   |   Needs_tests:  0  
Needs_better_patch:  0   |  
-+--
Changes (by laurentluce):

  * owner:  nobody => laurentluce
  * milestone:  => 1.3

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14354: Check password is not None in User.check_password

[Django]

#14354: Check password is not None in User.check_password
-+--
  Reporter:  berryp  | Owner:  nobody
Status:  new | Milestone:
 Component:  Authentication  |   Version:  1.2   
Resolution:  |  Keywords:
 Stage:  Accepted| Has_patch:  0 
Needs_docs:  0   |   Needs_tests:  0 
Needs_better_patch:  0   |  
-+--
Changes (by gabrielhurley):

  * needs_better_patch:  => 0
  * stage:  Unreviewed => Accepted
  * needs_tests:  => 0
  * needs_docs:  => 0

Comment:

 Seems like a valid point to me. I think it might be a little cleaner to
 alter `has_usable_password` to check for whatever cases are invalid
 (UNUSABLE_PASSWORD, None, ''), and then call `has_usable_password`
 internally.

 Also, this should definitely have a test case. For bonus points, since
 Django is slowly trying to eliminate doctests, you could convert the
 contrib.auth.tests.basic test case into a unit test (it's not that many
 lines).

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

[Django] #14354: Check password is not None in User.check_password

[Django]

#14354: Check password is not None in User.check_password
+---
 Reporter:  berryp  |   Owner:  nobody
   Status:  new |   Milestone:
Component:  Authentication  | Version:  1.2   
 Keywords:  |   Stage:  Unreviewed
Has_patch:  0   |  
+---
 I recently had an unexpected situation where users with no passwords would
 receive an error when trying to login. This is due to the fact that the
 User.check_password method does not check for missing passwords before
 calling get_hexdigest.

 It could be argued that all users should either have a password or an
 unusable password "!". However, as I am authenticating against a database
 that belongs to another system it is not an option to go and change all
 empty passwords to unusable ones. I would not expect authentication to
 raise an exception in this occasion.

 To get around this problem I simply inserted the following two lines at
 the top of the check_password function:

 {{{
 if self.password is None:
 return False
 }}}

 Additionally, would it not be a good idea to check that the password is
 not UNUSABLE_PASSWORD before trying to execute the code that checks the
 password? This would be a lot more elegant than executing code that is
 ultimately going to fail.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.