search

hydra-newmedia / passport-headerapikey

Api key authentication strategy for Passport, which only handles headers (not body fields).
28 stars 6 forks source link

Missing api key should return 401 instead of 400 #4

Open gnuton opened 5 years ago

gnuton commented 5 years ago

Hi there, I have just found out something which is not consistent with HTTP status errors usage.

The issue is quite simple. If the API key is missing the strategy should return a 403 instead of a 400, since the access to the resource is forbidden due to invalid (missing) authentication. Same issue for the line 45 https://github.com/hydra-newmedia/passport-headerapikey/blob/develop/src/Strategy.ts#L38

guischdi commented 5 years ago

Hey @gnuton, sorry for the late response. Was out of office until yesterday. I think it should rather be 401, since the requester is unauthenticated due to invalid/missing api key. 403 means 'credentials valid, but not enough permissions for the resource'.

To be honest, I do not even know how it works as of today: There is no .fail method in the Strategy class of the passport-strategy npm package (as far as I can see here https://github.com/jaredhanson/passport-strategy/blob/master/lib/strategy.js). The typings tell there is a .fail method however. Regarding to those definitions, the second parameter should be the response code.

I do not know how to test this any more, as I have not been using this module for years. But feel free to open a Pull Request with the regarding changes and tests. I will then try to run the tests and verify the code.

gnuton commented 5 years ago

Hi, Thanks a lot for your reply. I think you are right it should be 401. I will take a look and try to raise a PR. Cheers