search

okta / okta-sdk-python

Apache License 2.0
240 stars 142 forks source link

Remove ecdsa dependency #403

Closed agburch closed 6 months ago

agburch commented 6 months ago

This PR proposes to remove the ecdsa dependency completely by refactoring to use jwcrypto and pyjwt. This change is motivated by the recent CVE that demonstrated the potential vulnerabilities in ecdsa. But regardless of any specific vuln, the maintainers of the ecdsa library have commented to suggest that it is not intended for "production use": https://github.com/tlsfuzzer/python-ecdsa?tab=readme-ov-file#Security

By removing python-jose this PR does remove functionality from the jwt module (e.g. specifying alternate hash algorithms or JWT_OPTIONS). But as of today, these features appear to be unused. However please let me know if I'm mistaken here. As far as this PR is concerned though, the output of JWT.create_token() has not changed.

I believe this PR extends the work done in PR #398 by fully removing the package in question. It should also address: Issues #395 Issues #388

agburch commented 6 months ago

@justinabrokwah-okta - I'm curious to get your thoughts here, I think you might be the most familiar with the current implementation.

justinabrokwah-okta commented 6 months ago

@justinabrokwah-okta - I'm curious to get your thoughts here, I think you might be the most familiar with the current implementation.

Thanks so much for this PR, sorry for the delay in response. I'll try to take a look at this today but skimming through, it looks like an overdue change to make. Also tagging @bryanapellanes-okta for visibility

bryanapellanes-okta commented 6 months ago

@agburch Thank you so much for your contribution! I'll review this week and consult with @justinabrokwah-okta . If no issues or concerns arise, I'll plan to merge and publish a new release by the end of next week.