Deferring this due to lack of response.

Chris, I actually wrote a mock (using import hooks) that would allow me to
simulate not being able to import a module, even if it's really available.  This
should allow us to get a higher coverage.  Unfortunately, without bcrypt
installed, we couldn't get full coverage...

As to backwards compatibility, the current implementation remains 100% backwards
compatibility, including an existing bug when using cleartext passwords.  (Using
cleartext passwords, it is possible for a user to enter a password that has the
same format as a hashed password, making it impossible to login).

Using cleartext passwords is almost always a bad idea, and if you're doing so
and you absolutely insist on keeping them that way, this code won't try to
prevent you from doing so, and even offers an alternative to fix this bug...
(Prefixing all cleartext passwords with {clear} will prevent the bug from
occurring)  The other possibility, not mentioned here is to hash all passwords
in the database, or as the users log in (since that gives you access to the
cleartext password, it is easy to change the hashes).

Ok, so that only covers the first half of your comments... I'll look at my code
for the rest...

Doug

Hi Douglas,

Sorry for not responding til now; the worthwhile patches always require more
thought than plain bugreports.  Thanks for the submission!

(FTR, I tried to apply the patch but it has a syntax error on line 267.  That
was easy to fix, just needed a tab char).

Anyway, it's a bit hard to tell from here: will this patch introduce any
backwards incompatibility when people upgrade to a version of who that contains
your patch?

If not (that's a bit of a deal-killer), a few things:

- To get this into who, we really need all the code covered by tests.  Here's
output of a run of "setup.py nosetests" of the package after applying your patch:

Name                           Stmts   Exec  Cover   Missing
------------------------------------------------------------
repoze.who                         0      0   100%   
repoze.who.classifiers            37     37   100%   
repoze.who.config                102    102   100%   
repoze.who.interfaces             17     17   100%   
repoze.who.middleware            255    255   100%   
repoze.who.plugins                 0      0   100%   
repoze.who.plugins.auth_tkt      104    104   100%   
repoze.who.plugins.basicauth      45     45   100%   
repoze.who.plugins.cookie         39     39   100%   
repoze.who.plugins.fixtures   coverage.CoverageException: No source for compiled
code
'/Users/chrism/projects/repoze/svn/repoze.who/trunk/repoze/who/plugins/fixtures/__init__.pyc'.
repoze.who.plugins.form          132    132   100%   
repoze.who.plugins.htpasswd       45     45   100%   
repoze.who.plugins.sql           192    163    84%   25-26, 51, 65-76, 84-85,
88-92, 105-108, 128, 138-139, 165, 172, 201, 213-216, 224-225
repoze.who.restrict               23     23   100%   
repoze.who.utils                   3      3   100%   
------------------------------------------------------------
TOTAL                            994    965    97%   

Without your patches, we get 100% code coverage, which is what we shoot for.  A
lot of the un-covered code is likely due to conditional imports.  But the
conditional imports themselves have a little bit of a smell, especially the ones
that try an import but then turn around and catch an ImportError exception and
reraise a different error to help.  We usually prefer to allow original
exceptions to propagate up even if it means a slightly more cryptic error
message, it makes the code easier to read and maintain.

A few style things (picaynue but also important, to us at least):

- In default_password_compare, instead of putting things into a big
if/else/else, etc, can we use a dict full of functions by name and dispatch on
the name?

- The r.who source is formatted at 79 char linebreaks. 

If fixing that stuff is too much work, I'd suggest forking the SQL plugin itself
into a separate package and releasing it by itself.  That'd be a reasonable
thing either way, actually.

Any comments for me?  Anything I need to do to get this accepted?  I think it's
pretty thorough, but I'm open to any criticism...

Hopefully, the last of the unit tests that don't work properly in Python 2.4

Whoops, bad unittest passed through because I was testing in Python 2.5

New version of the patch which also supports blowfish hashes when bcrypt is
installed, and uses pycrypto on python < 2.5 for sha256 support.  This patch
superseded the previous two patches.

Adding a version of the patch that uses base64 encoding, to be more standards
compliant.  The default comparator supports reading the older hex based encoding
as well...

The SQL Authenticator uses unsalted hashes by default which are susceptible to
attacks like Rainbow tables.  I'm including a patch to add support, with tests.
 In addition, it's useful to have a default implementation of the hash function,
so I've added that.