-
Notifications
You must be signed in to change notification settings - Fork 482
Patch JwtPayload in order to correctly parse Checkout UI extensions session tokens #1346
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
Conversation
…okens, missing "iss" and "sid" keys (see: https://shopify.dev/docs/api/checkout-ui-extensions/2024-10/apis/session-token)
|
I have signed the CLA! |
lizkenyon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution good find that these tokens are inconsistent.
Could you add a test case to jwt_payload_test.rb to prevent regressions.
Could you also add to the CHANGELOG.md
| @iss = T.let(payload_hash["iss"], T.nilable(String)) | ||
| @dest = T.let(payload_hash["dest"], String) | ||
| @aud = T.let(payload_hash["aud"], String) | ||
| @sub = T.let(payload_hash["sub"], String) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my testing I found that the sub claim could also be nil, if the customer is not logged in.
| sig { returns(Integer) } | ||
| def shopify_user_id | ||
| @sub.to_i | ||
| @sub.tr("^0-9", "").to_i |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why you are extracting the ID?
|
Any news here? Should I open another PR so we fix this as soon as possible? |
|
@BaggioGiacomo |
|
Hi @lizkenyon! The Here's an example of a generated
When adding this token to the fetch call, I get this error on the console: This row is the one that throw the exception |
|
@BaggioGiacomo Thank you! Yes! If you make a PR setting the |
|
@lizkenyon We can close this since we've merged #1369 |

Checkout UI session tokens are actually missing "iss" and "sid" keys (see: https://shopify.dev/docs/api/checkout-ui-extensions/2024-10/apis/session-token). The current implementation makes them mandatory, generating exceptions while verifying JWT tokens (see the new
ShopifyApp::WithShopifyIdTokenconcern, and the legacyShopifyApp::JWTMiddlewaremiddleware).Description
The PR is for:
ShopifyAPI::Auth::JwtPayloadconstructor (line 30) when the JWT Token is missing the "iss" and "sid" keys (both optional in Checkout UI session tokens)ShopifyAppengine fails, raising an obscure exceptionshopify_user_idfrom the "sid" key, even when it's formatted asgid://shopify/Customer/<customerId>How has this been tested?
Develop a basic frontend Checkout UI extension and make cors authenticated calls to a Rails backend developed using
shopify_appgem (v22.4). Any request fails without reaching the corresponding controller, since an exception is raised in theShopifyApp::JWTMiddlewaremiddleware injected by theShopifyAppengine (seeengine.rb, row 20).After applying the patch, requests are nomally executed without raising any exception.
Checklist: