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

Replace jose with pyjwt #819

Merged
merged 2 commits into from
Sep 12, 2023

Conversation

sevdog
Copy link
Contributor

@sevdog sevdog commented Aug 3, 2023

Proposed changes

Replace python-jose with pyjwt (see #503).

Types of changes

Please check the type of change your PR introduces:

  • Release (new release request)
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (PEP8, lint, formatting, renaming, etc)
  • Refactoring (no functional changes, no api changes)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Build related changes (build process, tests runner, etc)
  • Other (please describe):

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask. We're here to
help! This is simply a reminder of what we are going to look for before merging
your code.

  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works

Other information

As discussed in #503 some changes was done to perform the migration:

  • add calc_at_hash static method to OpenIdConnectAuth since PyJWT does not support validating claims which are not part of JWT RFCs (see Add support for the OIDC at_hash claim jpadilla/pyjwt#296)
  • manual add "at_hash" claim in OpenIdConnectTestMixin.prepare_access_token_body (see above)
  • add "aud" claim in Auth0OAuth2Test.access_token_body because PyJWT behaves different from jose: it does not allow this claim to not be present if is should be validated
  • implement in Auth0OAuth2 the logic which tries every found key since PyJWT does not implement it while jose does.
  • update PyJWT minimum requirement to 2.7.0 which is the minimum version which provides the full API to handle OIDC

@nijel
Copy link
Member

nijel commented Aug 4, 2023

Thanks! Due to changed dependencies, this needs increasing required Python version. I've already created a pull request at #817, but it is waiting on @omab to adjust branch protection rules (or to give me permissions to make such changes myself).

@sevdog
Copy link
Contributor Author

sevdog commented Aug 4, 2023

Thank you @nijel, my fault to forgot testing locally against EOL python versions (I am too much addicted to remove EOL interpreters from my device).

@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Patch coverage: 70.58% and project coverage change: -0.04% ⚠️

Comparison is base (875b7bd) 77.80% compared to head (a4f7d73) 77.77%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #819      +/-   ##
==========================================
- Coverage   77.80%   77.77%   -0.04%     
==========================================
  Files         330      330              
  Lines       10097    10119      +22     
  Branches      681      685       +4     
==========================================
+ Hits         7856     7870      +14     
- Misses       2087     2093       +6     
- Partials      154      156       +2     
Flag Coverage Δ
unittests 77.77% <70.58%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
social_core/backends/auth0.py 82.35% <53.84%> (-17.65%) ⬇️
social_core/backends/open_id_connect.py 90.84% <77.77%> (-0.76%) ⬇️
social_core/tests/backends/test_auth0.py 100.00% <100.00%> (ø)
social_core/tests/backends/test_open_id_connect.py 99.09% <100.00%> (+<0.01%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nijel nijel linked an issue Sep 12, 2023 that may be closed by this pull request
@nijel nijel merged commit 013d27d into python-social-auth:master Sep 12, 2023
5 of 7 checks passed
@nijel
Copy link
Member

nijel commented Sep 12, 2023

Merged, thanks for your contribution!

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

Successfully merging this pull request may close these issues.

Use a single JWT library
2 participants