Skip to content
This repository has been archived by the owner on May 6, 2024. It is now read-only.

fix: remove JWT_DECODE_HANDLER override #6921

Merged
merged 1 commit into from
Apr 24, 2023

Conversation

robrap
Copy link
Contributor

@robrap robrap commented Apr 21, 2023

Remove the override of JWT_DECODE_HANDLER in the settings files, because it already matches the default set in base.py, and when we wish to remove this custom
jwt_decode_handler, it is possible that this setting will unnecessarily cause an error in the future. See
https://github.com/openedx/ecommerce/blob/b5fa2e55e3d4f5f8224d381317177cf4086b35a6/ecommerce/settings/base.py#L449

The JWT_AUTH dictionary should not be replaced as a whole, but should only replace keys that are declared, according to this:
https://github.com/openedx/ecommerce/blob/b5fa2e55e3d4f5f8224d381317177cf4086b35a6/ecommerce/settings/production.py#L51-L53

Note: This change is very similar to https://github.com/edx/edx-internal/pull/8163 which already landed.

Configuration Pull Request

Make sure that the following steps are done before merging:

  • A SRE team member has approved the PR if it is code shared across multiple services and you don't own all of the services.
  • Are you adding any new default values that need to be overridden when this change goes live? If so:
    • Update the appropriate internal repo (be sure to update for all our environments)
    • If you are updating a secure value rather than an internal one, file a SRE ticket with details.
    • Add an entry to the CHANGELOG.
  • If you are making a complicated change, have you performed the proper testing specified on the Ops Ansible Testing Checklist? Adding a new variable does not require the full list (although testing on a sandbox is a great idea to ensure it links with your downstream code changes).
  • Think about how this change will affect Open edX operators. Have you updated the wiki page for the next Open edX release?

Remove the override of JWT_DECODE_HANDLER in the settings
files, because it already matches the default set in
base.py, and when we wish to remove this custom
jwt_decode_handler, it is possible that this setting will
unnecessarily cause an error in the future. See
https://github.com/openedx/ecommerce/blob/b5fa2e55e3d4f5f8224d381317177cf4086b35a6/ecommerce/settings/base.py#L449

The JWT_AUTH dictionary should not be replaced as a whole,
but should only replace keys that are declared, according
to this:
https://github.com/openedx/ecommerce/blob/b5fa2e55e3d4f5f8224d381317177cf4086b35a6/ecommerce/settings/production.py#L51-L53
Copy link
Contributor

@pshiu pshiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @robrap!

@robrap robrap merged commit 628a5db into master Apr 24, 2023
3 checks passed
@robrap robrap deleted the robrap/remove-ecom-jwt-decode-handler branch April 24, 2023 15:59
@pshiu
Copy link
Contributor

pshiu commented May 1, 2023

Note: We are having a temporary issue with the Jenkins job that builds the ecommerce Docker image for devstack.

Until this PR can be incorporated into devstack via that Jenkins job, developers experiencing issues like:

ImportError: Could not import 'ecommerce.extensions.api.handlers.jwt_decode_handler' for API setting 'JWT_DECODE_HANDLER'. ModuleNotFoundError: No module named 'ecommerce.extensions.api.handlers'.

after bringing up a new container of the ecommerce devstack can make it work by temporarily adding:

'JWT_DECODE_HANDLER': 'edx_rest_framework_extensions.auth.jwt.decoder.jwt_decode_handler',

to this dictionary on their local. (Apologies for the inconvenience.)

robrap added a commit to openedx/ecommerce that referenced this pull request May 1, 2023
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
@robrap
Copy link
Contributor Author

robrap commented May 1, 2023

@pshiu: Will this PR temporarily fix the issue for everyone without special instructions? openedx/ecommerce#3958. I did not test.

@pshiu
Copy link
Contributor

pshiu commented May 1, 2023

Great idea, thanks @robrap! Just gave a thumb and added an AC to edx/upgrades#109.

@robrap
Copy link
Contributor Author

robrap commented May 1, 2023

Great @pshiu. As I noted, this is untested, so just make sure it resolves your issue. Thanks.

pshiu pushed a commit to openedx/ecommerce that referenced this pull request May 1, 2023
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
@pshiu
Copy link
Contributor

pshiu commented May 2, 2023

@robrap, yep, @jsnwesson and I had tested an almost identical patch right before I saw your comment.

grmartin added a commit to openedx/ecommerce that referenced this pull request 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 <irfanahmad@arbisoft.com>

* 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 <syedzubairtahir12@gmail.com>

* 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 <syedzubairtahir12@gmail.com>

* 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 <syedzubairtahir12@gmail.com>

* 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 <feanil@tcril.org>
Co-authored-by: Adam Stankiewicz <agstanki@gmail.com>
Co-authored-by: jawad khan <jawadkhan444@gmail.com>
Co-authored-by: Moeez Zahid <moeezzahid1996@gmail.com>
Co-authored-by: Robert Raposa <rraposa@edx.org>
Co-authored-by: irfanuddinahmad <34648393+irfanuddinahmad@users.noreply.github.com>
Co-authored-by: IrfanUddinAhmad <irfanahmad@arbisoft.com>
Co-authored-by: Kshitij Sobti <kshitij@sobti.in>
Co-authored-by: Mohammad Ahtasham ul Hassan <60315450+aht007@users.noreply.github.com>
Co-authored-by: Alex Dusenbery <adusenbery@edx.org>
Co-authored-by: Muhammad Zubair <syedzubairtahir12@gmail.com>
Co-authored-by: Soban Javed <iamsobanjaved@gmail.com>
Co-authored-by: Jade Olivier <jadeolivier95@gmail.com>
Co-authored-by: Saleem Latif <saleem-latif@users.noreply.github.com>
Co-authored-by: Shahroz Ahmad <97090106+ishahroz@users.noreply.github.com>
Co-authored-by: Phillip Shiu <pshiu@users.noreply.github.com>
Co-authored-by: Phillip Shiu <pshiu@edx.org>
Co-authored-by: Hamzah Ullah <hamzahullah@yahoo.com>
Co-authored-by: jawad khan <jawad.khan@arbisoft.com>
Co-authored-by: Chris Pappas <christopappas@users.noreply.github.com>
Co-authored-by: Usama Sadiq <usama7274@gmail.com>
jdmulloy pushed a commit that referenced this pull request Sep 14, 2023
Remove the override of JWT_DECODE_HANDLER in the settings
files, because it already matches the default set in
base.py, and when we wish to remove this custom
jwt_decode_handler, it is possible that this setting will
unnecessarily cause an error in the future. See
https://github.com/openedx/ecommerce/blob/b5fa2e55e3d4f5f8224d381317177cf4086b35a6/ecommerce/settings/base.py#L449

The JWT_AUTH dictionary should not be replaced as a whole,
but should only replace keys that are declared, according
to this:
https://github.com/openedx/ecommerce/blob/b5fa2e55e3d4f5f8224d381317177cf4086b35a6/ecommerce/settings/production.py#L51-L53
christopappas pushed a commit to openedx/ecommerce that referenced this pull request Dec 4, 2023
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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants