search

kumar303 / hawkrest

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

Allow to override credential lookup. #5

Closed felipeota closed 9 years ago

kumar303 commented 9 years ago

Hi. Thanks for the patch. What you added looks useful for programmatic use but for regular Django usage it would be easier if one could configure a lookup function using a setting. For example:

HAWK_CREDENTIALS_LOOKUP = 'my_app.utils.hawk_credentials_lookup'

Can you try adding that? Actually, another person asked for this feature too.

You could implement it using Django's import_string.

felipeota commented 9 years ago

Ok. I've implemented it. Although I prefer the other way around because it allows to make a self contained module without needing to modify the settings file. It also makes it possible to use hawk in different apis inside the same app.

kumar303 commented 9 years ago

Looks great, I just made some minor stylistic comments. Do you have time to add a patch to the docs showing how this setting can be used? If not, I can file an issue to add that later. The new setting would need a mention underneath the HAWK_CREDENTIALS example. Here are instructions for how to build the docs locally while in development.

kumar303 commented 9 years ago

Although I prefer the other way around because it allows to make a self contained module without needing to modify the settings file. It also makes it possible to use hawk in different apis inside the same app.

Good point! Maybe you could expand on this patch where the Django setting can be used to set a lookup function but also where one could subclass the HawkAuthentication class for lower level usage?

felipeota commented 9 years ago

Alright, I added the aesthetic changes and the documentation. I will open a different pull request for the subclass way of extending lookup.

kumar303 commented 9 years ago

Awesome. I like the example in the docs. Thanks again.

kumar303 commented 9 years ago

I pushed a release with your feature to PyPI: https://pypi.python.org/pypi/hawkrest This new setting will be super helpful. By the way, if you wanted to follow up with another patch that added in your class approach (so that one could subclass it) then I think that would be fine.