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

Remove dependency on jwcrypto and cryptography #617

Closed
3 tasks done
angela-tran opened this issue May 23, 2022 · 1 comment · Fixed by #624
Closed
3 tasks done

Remove dependency on jwcrypto and cryptography #617

angela-tran opened this issue May 23, 2022 · 1 comment · Fixed by #624
Assignees
Labels
security Changes to improve or maintain the availability and resilience of the app

Comments

@angela-tran
Copy link
Member

angela-tran commented May 23, 2022

This task is to make the code changes necessary for removing jwcrypto from our requirements.txt file. (cryptography is a transitive dependency of jwcrypto, so we can remove that as well.) Some of our Django models currently return jwk.JWK instances in some of their methods.

Acceptance Criteria

  • The requirements.txt file does not contain jwcrypto or any of its dependencies
  • The Benefits app still works as expected
  • Unit tests still pass

Additional context

#141 mentions that a goal is to consolidate dependency management, which means benefits should not be directly depending on packages that are only needed for the Eligibility API. #596 pulls out most of the usage of jwcrypto, except for the ones left in core/models.py and tests/pytests/eligibility/test_views.py.

@angela-tran angela-tran added this to the Eligibility API refactor milestone May 23, 2022
@thekaveman thekaveman added the security Changes to improve or maintain the availability and resilience of the app label May 24, 2022
@thekaveman
Copy link
Member

thekaveman commented May 24, 2022

Commented on cal-itp/eligibility-api#31, which I think does more than necessary to solve this.

My thinking for solving this issue was actually more to simplify the benefits side - why do the models even need to return that JWK type? They are just passing directly into this API - it should fully internalize the logic and deal with the raw underlying type used in the model (e.g. the text or the bytes).

We don't really need a special class here.

@thekaveman thekaveman moved this to This Sprint (Month) in Digital Services May 24, 2022
@thekaveman thekaveman moved this from This Sprint (Month) to In Progress in Digital Services May 24, 2022
@angela-tran angela-tran self-assigned this May 26, 2022
Repository owner moved this from In Progress to Done in Digital Services May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security Changes to improve or maintain the availability and resilience of the app
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants