search

elliotpeele / pyramid_oauth2_provider

An Oauth2 provider built on pyramid
MIT License
31 stars 16 forks source link

token salting/hashing in DB, docs, changes #16

Open secynic opened 7 years ago

secynic commented 7 years ago

I want to thank you for this library, it has saved me quite some time.

I would like to work on some changes:

Let me know if you are open to this.

P.S. I confirmed the PRs from @tonthon are working.

elliotpeele commented 7 years ago

Feel free. I’m happy to review any pull requests you send my way.

On Mar 10, 2017, at 2:26 PM, Philip Hane notifications@github.com wrote:

I want to thank you for this library, it has saved me quite some time.

I would like to work on some changes:

hash/salt of client secret, access_token and refresh_token in the DB - the actual secret/tokens should be provided once at generation time, and then regenerated if forgotten (will have new requirement - cryptacular.bcrypt) Sphinx configuration and more docs (need to setup readthedocs and add/fix docstrings) Changes file Let me know if you are open to this.

P.S. I confirmed the PRs from @tonthon https://github.com/tonthon are working.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/elliotpeele/pyramid_oauth2_provider/issues/16, or mute the thread https://github.com/notifications/unsubscribe-auth/AADYjp_RL0xFsvQ8EVlDHDjM1_gmOAhyks5rkaP_gaJpZM4MZxQu.

secynic commented 7 years ago

^ This should be good aside from Sphinx/docs

Only minor issue is cryptacular and bcrypt libraries in general require a C compiler. Those instructions can be added into the readme easily, and in my opinion, outweigh not having any security on the DB secrets. -EDIT Going to try scrypt (cryptography) instead so there are no build issues.

I could not add that security to the access_token and refresh_token due to how Bearer works. I think it would be good to later add AES 256 encryption (PyCrypto + hashlib) on those columns, with a random encryption key stored in the config ini. I may write a new helper library for that since I already wrote the code for a private project.

Let me know if you are ok with these changes, and I will submit a PR. I also included #10 as that script was failing otherwise.

elliotpeele commented 7 years ago

I'd like to see the changes split out into a few different patches.

secynic commented 7 years ago

I will work on this. You can merge #13.

-EDIT: Some of the Python 2vs3 encoding changes will need to be bundled with the crypto PR. I would rather bundle all of them so tests pass. I can still split out the formatting fixes, deprecations, and other fixes. https://github.com/elliotpeele/pyramid_oauth2_provider/compare/master...secynic:ref16 is almost ready to be split up.