Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate from python-jose to PyJWT #194

Merged
merged 3 commits into from
May 30, 2024
Merged

Migrate from python-jose to PyJWT #194

merged 3 commits into from
May 30, 2024

Conversation

dvdalilue
Copy link
Contributor

Hi, I was dealing with some vulnerabilities in a project, and I realized that this package uses python-jose. It hasn't been updated since 2021, so makes sense to use another implementation like PyJWT.

You can find the vulnerability here: GHSA-6c5p-j8vq-pqhj.

In the FastAPI security section of the documentation they were using python-jose yet it was updated a few days ago: fastapi/fastapi#11589.

This PR only adapts the PyJWT package where python-jose was used. All tests passed, and I have tested it in my own project. Any feedback is welcomed and hopefully we can include this change in future releases.

@JonasKs
Copy link
Member

JonasKs commented May 23, 2024

Hi! Thank you so much!
I've wanted to do this for a while, but not found the time. (#178 (comment))

I'm out for the week, but I'll review as soon as I get back home.
I see that some pipelines (lint) fail, could you look into those?

Copy link

codecov bot commented May 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.0%. Comparing base (562882b) to head (2b1298c).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #194   +/-   ##
=======================================
  Coverage   100.0%   100.0%           
=======================================
  Files           6        6           
  Lines         248      263   +15     
=======================================
+ Hits          248      263   +15     
Files Coverage Δ
fastapi_azure_auth/auth.py 100.0% <100.0%> (ø)
fastapi_azure_auth/openid_config.py 100.0% <100.0%> (ø)
fastapi_azure_auth/utils.py 100.0% <100.0%> (ø)

@dvdalilue dvdalilue force-pushed the main branch 3 times, most recently from 60603fb to d1b2dcd Compare May 23, 2024 20:44
@dvdalilue
Copy link
Contributor Author

Hi! Thank you so much! I've wanted to do this for a while, but not found the time. (#178 (comment))

I'm out for the week, but I'll review as soon as I get back home. I see that some pipelines (lint) fail, could you look into those?

Yes, I was missing some checks but seems to be fine now

@JonasKs
Copy link
Member

JonasKs commented May 23, 2024

Awesome! Thanks again, I really appreciate it😊

I'll get this reviewed/merged early next week.

fastapi_azure_auth/auth.py Outdated Show resolved Hide resolved
@JonasKs JonasKs merged commit 2f20d55 into intility:main May 30, 2024
11 checks passed
@JonasKs
Copy link
Member

JonasKs commented May 30, 2024

I've merged and released, thank you again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants