search

kumar303 / hawkrest

Hawk HTTP Authorization for Django Rest Framework
BSD 3-Clause "New" or "Revised" License
19 stars 8 forks source link

Skip hawkrest authentication if it's not detected and not mandatory. #6

Closed maurodoglio closed 9 years ago

maurodoglio commented 9 years ago

To allow hawkrest to play well with other authentication schemes it should return None if a proper hawk HTTP_AUTHORIZATION header is not detected. To avoid breaking backward compatibility I hid the detection behind a HAWK_IS_MANDATORY setting which defaults to True.

kumar303 commented 9 years ago

This type of bypass has a way of introducing lots of subtle security bugs (JWT algorithm='none' comes to mind!) but I see the issue that you're trying to fix. I made a comment about disallowing missing auth headers but otherwise it looks good. Thanks!

kumar303 commented 9 years ago

btw, I switched to container builds in https://github.com/kumar303/hawkrest/pull/7 which will speed up getting the TravisCI result on future pulls.

maurodoglio commented 9 years ago

good idea, the travis run took 1 min 25 sec this time

maurodoglio commented 9 years ago

Patch updated. Thanks for your feedback!

kumar303 commented 9 years ago

looks good, thanks for the patch.

kumar303 commented 9 years ago

@maurodoglio oh hey! I did another read through the DRF docs and source and I realize I just had the authenticator implemented wrong. Returning None is the thing to do whenever no authentication is performed. I fixed that up in https://github.com/kumar303/hawkrest/pull/8 which means you do not need the HAWK_IS_MANDATORY setting. It should work for you out of the box now. I pushed 0.0.6 to PyPI, care to try it out?

maurodoglio commented 9 years ago

That's awesome news, it solves more than a problem for me :smiley: . I'm working on a patch for treeherder to add hawk as an api authentication scheme and being able to support both the legacy scheme and hawk makes it easier to migrate old clients. I'll let you know how it goes.