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

[DEPR]: Symmetric JWTs #83

Open
robrap opened this issue May 27, 2022 · 3 comments
Open

[DEPR]: Symmetric JWTs #83

robrap opened this issue May 27, 2022 · 3 comments
Labels
depr Proposal for deprecation & removal per OEP-21

Comments

@robrap
Copy link

robrap commented May 27, 2022

Proposal Date

2022-12-08

Target Ticket Acceptance Date

2022-12-23

Earliest Open edX Named Release Without This Functionality

Quince - 2023-10

Rationale

The decision to deprecate symmetric JWTs in favor of asymmetric JWTs was documented and accepted in this edx-platform ADR in (2018-07-23).

The ADR also details deprecation of JWT_ISSUERS, since the LMS should be the lone issuer of the new asymmetric JWTs.

Since the writing of the ADR, asymmetric JWTs have been used for JWT cookies for MFEs, and many (if not all) IDAs now include this configuration for accepting asymmetric JWTs. The one exception may be ecomworker, which still may be generating its own JWTs? In edx.org PROD, it seems to have its own JWT_ISSUER config, and does not include the JWT_AUTH config included for the other IDAs.

Removal

Before clean-up, we need to switch to create asymmetric JWTs only. We could put this behind a rollout toggle to ensure it is ok. When the toggle is enabled, we could force use_asymmetric_key to be True in this code. This change should be noted for the named release, because all IDAs using JWTs will require JWK config, which must have been added for any IDAs backing an MFE.

Once we are only using asymmetric JWTs, there is a long list of clean-up possible.

edx-drf-extensions jwt_decode_handler:

LMS JWT creation:

Related setting clean-up:

  • DEFAULT_JWT_ISSUER: Retired?, but the configs were never fully removed. Confirm and delete.
  • JWT_ISSUER: Used by the LMS when encoding JWTs? Should be confirmed and documented, and should not exist in other services.
  • JWT_AUTH['JWT_ISSUER']: This is would be the new "replacement" for the issuer. (Decided to leave JWT_ISSUERS list in case of future changes.)
  • JWT_AUTH['JWT_ISSUERS']: This should be removed from all services except ecommerce (see below). (Decided to leave as-is, except ensuring it only has one entry for now.) JWT_AUTH['JWT_ISSUERS'][0] entries should exist at top-level of JWT_AUTH (minus the removed JWT_SECRET_KEY).
  • JWT_AUTH['JWT_SECRET_KEY']: This should be removed from all services except ecommerce (see below).
  • JWT_AUTH['JWT_ISSUERS'][0]['JWT_SECRET_KEY']: This should be removed from all services.
  • Note: The contract phase of this clean-up should be possible after each service is only using asymmetric JWTs.

Ecommerce outlier notes:

  • Update: It is possible that the DEPR of the old rest client means that ecommerce is no longer creating its own JWTs. That needs to be confirmed. If it isn't, then it doesn't need any exceptional handling.
  • The ecommerce service has a custom jwt_decode_handler that uses the edx-drf-extensions version and adds its own support for symmetric JWTs (which it issues).
  • We must ensure we don't break the ecommerce service, and it should continue to use symmetric JWTs.
  • We would have wanted the ecommerce service to also drop support of asymmetric JWTs, but:
    • This is lower risk once the other services could no longer accept these symmetric JWTs, and
    • Ecommerce is already being deprecated/removed, so no need to put in this extra maintenance work.

Replacement

This will be replaced by the already existing support for asymmetric JWTs.

Deprecation

This was already completed.

Migration

No response

Additional Info

Again, this needs to be undertaken in steps very carefully. There may be additional details documented in the epic ARCHBOM-1202 and its tasks. However, it also assumed that ecommerce would ultimately need to be fixed as well.

For 2U, there are additional configs for JWT_ISSUERS in https://github.com/edx/edx-internal/blob/master/ansible/vars/edx.yml, in addition to remote config.

@robrap robrap added the depr Proposal for deprecation & removal per OEP-21 label May 27, 2022
@robrap
Copy link
Author

robrap commented Jun 7, 2022

It's unclear if #48 means that ecommerce no longer is creating/using its own JWTs. That requires some discovery work.

@robrap robrap moved this from Proposed to Communicated in DEPR: Deprecation & Removal Dec 9, 2022
@dianakhuang dianakhuang moved this from Communicated to Accepted in DEPR: Deprecation & Removal Jan 12, 2023
robrap added a commit to openedx/edx-platform that referenced this issue Apr 10, 2023
Add a temporary feature toggle to force the LMS to
only produce asymmetric JWTs. This is a part of
DEPR of Symmetric JWTs:
openedx/public-engineering#83
robrap added a commit to openedx/edx-platform that referenced this issue Apr 10, 2023
Add a temporary feature toggle to force the LMS to
only produce asymmetric JWTs. This is a part of
DEPR of Symmetric JWTs:
openedx/public-engineering#83
robrap added a commit to openedx-unsupported/ecommerce that referenced this issue Apr 10, 2023
Reordered the JWT decoders to first use the standard
library version, and then use the custom ecommerce
decoder which uses multiple issuers. In this way, we
can see if any JWTs cannot be decoded by that standard
library version, and when and if we are ready to retire
the custom JWT decoding code.

See DEPR openedx/public-engineering#83
robrap added a commit to openedx-unsupported/ecommerce that referenced this issue Apr 11, 2023
Reordered the JWT decoders to first use the standard
library version, and then use the custom ecommerce
decoder which uses multiple issuers. In this way, we
can see if any JWTs cannot be decoded by that standard
library version, and when and if we are ready to retire
the custom JWT decoding code.

See DEPR openedx/public-engineering#83
robrap added a commit to openedx/edx-drf-extensions that referenced this issue Apr 11, 2023
Adds monitoring around JWT decoding and symmetric keys, to
help with the eventual deprecation and removal of the
symmetric keys.

See DEPR: Symmetric JWTs:
openedx/public-engineering#83
@robrap robrap moved this from Accepted to Removing in DEPR: Deprecation & Removal Apr 11, 2023
robrap added a commit to openedx/edx-drf-extensions that referenced this issue Apr 12, 2023
Adds monitoring around JWT decoding and symmetric keys, to
help with the eventual deprecation and removal of the
symmetric keys.

See DEPR: Symmetric JWTs:
openedx/public-engineering#83
@robrap
Copy link
Author

robrap commented Apr 13, 2023

Updates:

@robrap
Copy link
Author

robrap commented Apr 17, 2023

Update: We needed to rollback at edx.org. See openedx/edx-drf-extensions#333 for details on how we could move forward again. Moving this back to announced.

@robrap robrap moved this from Removing to Accepted in DEPR: Deprecation & Removal Apr 17, 2023
grmartin added a commit to openedx-unsupported/ecommerce that referenced this issue Aug 29, 2023
* build: Creating a missing workflow file `self-assign-issue.yml`.

The .github/workflows/self-assign-issue.yml workflow is missing or needs an update to stay in
sync with the current standard for this workflow as defined in the
`.github` repo of the `openedx` GitHub org.

* build: Creating a missing workflow file `add-remove-label-on-comment.yml`.

The .github/workflows/add-remove-label-on-comment.yml workflow is missing or needs an update to stay in
sync with the current standard for this workflow as defined in the
`.github` repo of the `openedx` GitHub org.

* build: Updating a missing workflow file `add-depr-ticket-to-depr-board.yml`.

The .github/workflows/add-depr-ticket-to-depr-board.yml workflow is missing or needs an update to stay in
sync with the current standard for this workflow as defined in the
`.github` repo of the `openedx` GitHub org.

* docs: Remove repo specific CONTRIBUTING.rst

We now have a org wide CONTRIBUTING.md that points to our correct
general contributing guidelines.  We don't need repo specific ones that
forward to other contributing docs.

* fix: account for refunds in exec ed 2u redemption flow (#3920)

* chore: add logging to include fulfillment details upon GEAG allocation exception

* chore: quality

* fix: Pick the right purchase from ios response (#3921)

* fix: Pick the right purchase from ios response

iOS response contain multiple purchases, instead of picking the first purchase,
pick the one which have given product id and latest date.
LEARNER-9261

* feat: Added Android refund api (#3922)

* feat: Added Android refund api

Like Apple android doesn't have callback for every refund. Therefore we have created an endpoint  which we will hit daily through ecommerce worker.
Learner-9149

* feat: Error if products in basket are already purchased (#3929)

* feat: Error if products in basket are already purchased

* refactor: Add tests, Improve error message

* refactor: Update docstring

* test: Increase coverage

* chore: add logging to debug ent-6954 (#3931)

* fix: Fix error in checkout api for mobile (#3934)

* fix: Fix error in checkout api for mobile

* fix: Return error in case of duplicate transaction_id for mobile (#3936)

* fix: Return error in case of duplicate transactionID for mobile

* refactor: Review feedback, add documentation

* feat: Added course and expires field in product form on ecommerce dashboard (#3938)

Forked catalogue app from oscar and added course and expire field in ProductForm. This change will enable to
add Android sku from a same dashboard page.

* fix: reorder JWT decoders (#3941)

Reordered the JWT decoders to first use the standard
library version, and then use the custom ecommerce
decoder which uses multiple issuers. In this way, we
can see if any JWTs cannot be decoded by that standard
library version, and when and if we are ready to retire
the custom JWT decoding code.

See DEPR openedx/public-engineering#83

* fix: cached monitoring (#3942)

Monitoring features such as use of the increment
method, to increment a custom attribute, requires
the CachedCustomMonitoringMiddleware. This has been
added so the earlier calls to increment will function.

* feat: add discount_jwt monitoring (#3944)

Add monitoring for the discount JWT.

* feat: Added data_share_consent field to order fullfillment notes (#3939)

Co-authored-by: IrfanUddinAhmad <[email protected]>

* chore: Switch from edx-sphinx-theme to sphinx-book-theme

The edx-sphinx theme is being deprecated, and replaced with sphinx-book-theme.
This removes references to the deprecated theme and replaces them with the new
standard theme for the platform.

* test: Add tests for Mobile IAP (#3937)

* test: Add tests for mobile In-app purchases

This reverts commit 54ea975.

* fix: fix codecov error

Codecov PyPI package was removed on 12 April and the recommended step is to migrate to codecov Github Action instead.

* fix: add an exec ex 2u max application check to the checkout flow. ENT-7059

Also removes codecov from ci.yml workflow.

* feat: add product entitlement info api (#3945)

* fix: Updated format for data_share_consent field

* docs: Update the contributing guidelines link.

We're moving towards a single set of guidelines org-wide.

* feat!: remove custom JWT decoding (#3943)

* feat!: remove custom JWT decoding

Removes the ecommerce custom JWT decoding, and replaces
with the simple decoding from the edx-drf-extensions
library.

* fix: drop constraints and make upgrade

* fix: handle major upgrade of django-crispy-forms

The major upgrade of django-crispy-forms called for
some changes related to bootstrap3 and dependencies.
See https://github.com/django-crispy-forms/django-crispy-forms/blob/main/CHANGELOG.md#major-changes-and-migration-guide

* fix: code coverage reporting

Codecov no longer exists on PyPI, so switch to github
action to run coverage report.

---------

Co-authored-by: Muhammad Zubair <[email protected]>

* fix: Course to have multiple seats with certificate_type attribute (#3950)

* fix: Course to have multiple seats with certificate_type attribute
* refactor: Modify SKU generation hash, add tests
* test: Modify tests

* temp: update JWT_DECODE_HANDER in devstack.py

Jenkins job for building devstack images is temporarily broken.
This should fix the devstack settings until this configuration
change lands in an updated image:
openedx-unsupported/configuration#6921

* feat: add native Dockerfile to create ansible free image

* feat: add additional fields to EnterpriseLearnerOfferApiSerializer (#3963)

* refactor: add logging to mobile IAP (#3962)

* refactor: Improve exception handling for mobile IAP (#3969)

* refactor: Improve exception handling for mobile IAP
* refactor: pylint fixes

* feat: Fix capture_context error on Payment MFE (#3965)

* feat: Fix capture_context error on Payment MFE

* feat: removed whitespace

* feat: removed whitespace

* feat: modified test case

* feat: modified test case

---------

Co-authored-by: Muhammad Zubair <[email protected]>

* feat: Add enterprise_customer_name in the event metadata for offer usage braze emails. (#3972)

* feat: Embargo check for subscription Programs (#3960)

* fix: Enable TrackingMiddleware for Mobile IAP basket (#3977)

* chore: updated Python requirements (edx-ecommerce-worker to version 3.3.3) (#3968)

Co-authored-by: Muhammad Zubair <[email protected]>

* fix: schedule upgrade-python-requirements monthly

* fix: add edx-revenue-tasks to user_reviewers & remove team_reviewers

Will create a Jira ticket instead of tagging all the members of
@openedx/revenue-squad.

* feat: add sf line item field to enterprise offers

ENT-7013

* feat: Added ios refund callback (#3967)

* feat: add SDN endpoints (#3985)

* feat: add endpoint to run SDN check and return counts

* feat: add SDNCheckFailure REST APi

* fix: fix 500 on SDN for subscriptions (#3989)

* fix: fix 500 on SDN for subscriptions

* fix: pytest-selenium, pytest-variables, pyjwkest dependency issues (#3987)

* feat: add coupon sf opp line item attribute

* feat: Store price and currency for Mobile IAP (#3992)

* feat: Store price and currency for mobile IAP

* fix: return 200 on embargo failure to prevent downstream error (#3993)

* feat: Make mobile IAP execute/ API atomic (#3995)

* chore: added CODEOWNERS file (#3970)

* refactor: Add logging to mobile IAP checkout/ API (#4000)

* chore: django security patch 3.2.20 upgrade (#3999)

* feat: Updates opportunity line item regex and tests (#3996)

* feat: unenroll refunded android users daily (#4015)

* feat: unenroll refunded android users daily

Django management command to un-enroll refunded android users. This command will be run by Jenkins job daily.

* feat: mail mobile team for a mobile course change in publisher (#4014)

* feat: mail mobile team for a mobile course change in publisher

This will fix any unknown change from publisher to a course having mobile seats.
After this fix mobile team will see mail and adjust price of the course on playstore or appstore.
In the longer run we want to replace this solution by changing the course price directly using mobile platform apis.

LEARNER-9377

* fix: fixed coverage issue

---------

Co-authored-by: Feanil Patel <[email protected]>
Co-authored-by: Adam Stankiewicz <[email protected]>
Co-authored-by: jawad khan <[email protected]>
Co-authored-by: Moeez Zahid <[email protected]>
Co-authored-by: Robert Raposa <[email protected]>
Co-authored-by: irfanuddinahmad <[email protected]>
Co-authored-by: IrfanUddinAhmad <[email protected]>
Co-authored-by: Kshitij Sobti <[email protected]>
Co-authored-by: Mohammad Ahtasham ul Hassan <[email protected]>
Co-authored-by: Alex Dusenbery <[email protected]>
Co-authored-by: Muhammad Zubair <[email protected]>
Co-authored-by: Soban Javed <[email protected]>
Co-authored-by: Jade Olivier <[email protected]>
Co-authored-by: Saleem Latif <[email protected]>
Co-authored-by: Shahroz Ahmad <[email protected]>
Co-authored-by: Phillip Shiu <[email protected]>
Co-authored-by: Phillip Shiu <[email protected]>
Co-authored-by: Hamzah Ullah <[email protected]>
Co-authored-by: jawad khan <[email protected]>
Co-authored-by: Chris Pappas <[email protected]>
Co-authored-by: Usama Sadiq <[email protected]>
christopappas pushed a commit to openedx-unsupported/ecommerce that referenced this issue Dec 4, 2023
Reordered the JWT decoders to first use the standard
library version, and then use the custom ecommerce
decoder which uses multiple issuers. In this way, we
can see if any JWTs cannot be decoded by that standard
library version, and when and if we are ready to retire
the custom JWT decoding code.

See DEPR openedx/public-engineering#83
@dianakhuang dianakhuang moved this from Accepted to Proposed in DEPR: Deprecation & Removal Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
depr Proposal for deprecation & removal per OEP-21
Projects
Status: Proposed
Development

No branches or pull requests

1 participant