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

Upgrade pyjwt #2131

Closed
3 of 4 tasks
kalbfled opened this issue Nov 18, 2024 · 3 comments
Closed
3 of 4 tasks

Upgrade pyjwt #2131

kalbfled opened this issue Nov 18, 2024 · 3 comments

Comments

@kalbfled
Copy link
Member

kalbfled commented Nov 18, 2024

User Story - Business Need

  • Ticket is understood, and QA has been contacted (if the ticket has a QA label).

User Story(ies)

As a Notify stakeholder,
I want to use the most up to date Python pyjwt package
So that we don't have bit rot or vulnerabilities.

Additional Info and Resources

During the routine upgrades for #2103, upgrading pyjwt from 2.9.0 to 2.10.0 caused 80 test failures in Github that probably all result from an unannounced breaking change. The tests did not fail locally for the person who did the upgrade. 🤷

Comment to bypass this.

Acceptance Criteria

  • All tests pass with pyjwt >= 2.10.1
  • Makefile removes the 74429 ignore and associated comment This was in PR @k-macmillan was working on and not merged into main.
  • This work is added to the sprint review slide deck (key win bullet point and demo slide)

QA Considerations

Potential Dependencies

@MackHalliday
Copy link

MackHalliday commented Jan 7, 2025

  • Thread on new error in open source repo
  • Only seeing errors locally if running test file individually. All test pass if ran all at once.
    • example tests/app/authentication/test_authentication.py
  • Common errors
    • jwt.exceptions.InvalidSubjectError: Subject must be a string

@MackHalliday
Copy link

MackHalliday commented Jan 8, 2025

Image

Traced source of error for tests in tests/app/authentication/test_authentication.py

Method requires_user_in_service_or_admin passed in None for arg required_permission. This results in error when calling validate()

This only seems to be an issue when called the decorator requires_user_in_service_or_admin. requires_user_in_service_or_admin called in code passes in required_permission='manage_templates'. See app/template/rest.py::245.

@MackHalliday
Copy link

Yesterday

  • Issues with docker
  • Implement fix JWT_VERIFY_SUB = False
  • Reading on fix to update configs to JWT_VERIFY_SUB = False

Today

  • PR is ready

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

No branches or pull requests

5 participants