

My first Flask extension: Flask-Bcrypt - llambda
https://github.com/maxcountryman/flask-bcrypt

======
callahad
Congrats on writing your first extension!

Out of curiosity, why did you choose to pass around the salt explicitly?
According to the docs at <http://www.mindrot.org/projects/py-bcrypt/>, you can
test a password directly against the output bcrypt.hashpw, without explicitly
separating out the salt:

    
    
      import bcrypt
      password = "my_password"
      hashed = bcrypt.hashpw(password, bcrypt.gensalt())
      if bcrypt.hashpw(password, hashed) == hashed:
          print "Success!"
    

I'm also a bit curious as to what deficiencies you're trying to address in the
existing py-bcrypt library? Similarly, how did you decide to make this a Flask
extension, versus a standalone Python module? It doesn't look like you're
actually doing anything Flask-specific. Do you have broader plans for the
project?

Edit: Also, I think your docstring for generate_password_hash is incorrect.
You state that "Ints exceeding 31 will become the default value of 12."
Instead, py-bcrypt has a floor of 4 rounds and a ceiling of 31. Integers
exceeding 31 will be treated as 31, etc. For reference, here's line 32 of py-
bcrypt's __init__.py:

    
    
      return encode_salt(os.urandom(16), min(max(log_rounds, 4), 31))
    

Link: [http://code.google.com/p/py-
bcrypt/source/browse/bcrypt/__in...](http://code.google.com/p/py-
bcrypt/source/browse/bcrypt/__init__.py#32)

~~~
llambda
_I'm also a bit curious as to what deficiencies you're trying to address in
the existing py-bcrypt library? Similarly, how did you decide to make this a
Flask extension, versus a standalone Python module? It doesn't look like
you're actually doing anything Flask-specific. Do you have broader plans for
the project?_

I don't think there's any deficiencies regarding py-bcrypt that I'm attempting
to address; the idea here is to provide some convenience functions for Flask
apps so that they can import an extension and have a drop-in replacement for
werkzeug.security's password hashing utilities (which do not support bcrypt at
this time). Hopefully this can happen without having to edit any existing
code, beyond adding the import. This extension is similar to the Django bcrypt
module although I understand they provide some other functionality as well.

I like the idea about adding rounds to the app.config. I definitely will plan
on adding this.

And I've made some edits regarding your suggestions above, currently fixing
the tests and then I'll push some updates.

Thanks for the feedback! :)

Edit: I tried to merge your changes but failed. I'm sure I was doing something
wrong but I've only been able to do it when someone has made a pull request. I
tried to add your fork as a remote and then pull from the remote and merge but
somehow the conflicts were saved without being merged and then pushed. Anyway
I appreciate you taking the time to make the changes you pointed out and I
would gladly have used them directly if I were a little more finessed with
git.

------
sirn
I feel this Flask extension is too small to worth using over py-bcrypt at all;
the example in this extension's README is no more different than:

    
    
        import bcrypt
        pw_hash = bcrypt.hashpw('secret', bcrypt.gensalt())
        pw_hash == bcrypt.hashpw('secret', pw_hash)
    

I also don't understand why `generate_password_hash` returns a tuple, not just
string; the salt is always in the password hash so there's no need to return
the salt. My suggestions:

1\. Make the extension (optionally) read `log_rounds` from `app.config`.

2\. Support for anything other than py-bcrypt, e.g. cryptacular or bcryptor.

~~~
llambda
Both of your suggestions are great!

I'll work on adding them. Thanks.

