search

vertexcover-io / falcon-auth

A falcon middleware + authentication backends that adds authentication layer to you app/api service.
MIT License
103 stars 31 forks source link

Sending JWT headers to user_loader function #37

Open rythos42 opened 4 years ago

rythos42 commented 4 years ago

Trying to fix #36 .

I moved the get_header out of _decode_jwt_token and into authenticate, so I could use the token a second time. Then called jwt.get_unverified_header, documented here https://pyjwt.readthedocs.io/en/latest/usage.html#reading-headers-without-validation.

I send the headers as a second parameter to user_loader, and to maintain backwards compatibility I catch the TypeError and call the old version of user_loader in the catch block.

No tests because I haven't figured out Python unit testing yet. >.> (I'm primarily a C# developer...) Let me know if you think they need to be added, I'll figure them out soon enough for my project.

coveralls commented 4 years ago

Coverage Status

Coverage decreased (-0.9%) to 89.96% when pulling caa8d18f30410752bf7b939b68c070c66132421a on rythos42:master into 718fb0a3f209a8a7511dddbeab8a6eaecfe49529 on loanzen:master.

rythos42 commented 4 years ago

Added a new parameter to JWTAuthBackend to take a key discovery URL. This URL returns a JSON array of public keys. Each key is iterated over until one is found that matches the kid (key id) of the incoming JWT, and that key is used to verify the incoming JWT.

This scheme is used by Azure authentication.

Because this scheme is still a "JWT auth", I think it belongs in this class. But the class has a hard dependency on being given a string public key in the constructor, which isn't needed for the key discovery workflow. I decided not to remove this parameter because I'm not sure I want to be responsible for asking for breaking changes. :) But done properly, I'd do some refactoring to make it or key_discovery_url required, but not both.

rythos42 commented 4 years ago

It appears the travis-ci builds are failing because Python 2.0 and 3.3 don't exist there?

I'll look into increasing the test coverage for my PR...

rythos42 commented 4 years ago

I wrote a test that I feel covers what I need it to, but I'm struggling with getting the two checks to pass. Could a more experienced developer provide advice? I'm not sure what's wrong with Python 2.7 in the travis-ci build, and I feel that trying to get another 0.9% test coverage in my changes will lead me in a direction where I'm testing that urllib works? Thanks!