Skip to content

Add shop validation#1443

Open
gonzaloriestra wants to merge 1 commit intomainfrom
river/use-jwt-dest-for-token-exchange
Open

Add shop validation#1443
gonzaloriestra wants to merge 1 commit intomainfrom
river/use-jwt-dest-for-token-exchange

Conversation

@gonzaloriestra
Copy link
Copy Markdown

@gonzaloriestra gonzaloriestra commented Apr 9, 2026

Description

https://docs.google.com/document/d/1c0iXhKBpm-yhvff0iSgq1XwBSPL39fTBVajIJ3svaxU/edit?usp=sharing

Checklist:

  • My commit message follow the pattern described in here
  • I have performed a self-review of my own code.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have updated the project documentation.
  • I have added a changelog line.

@gonzaloriestra gonzaloriestra force-pushed the river/use-jwt-dest-for-token-exchange branch from aaf2cc7 to 7992177 Compare April 9, 2026 11:38
@gonzaloriestra gonzaloriestra marked this pull request as ready for review April 9, 2026 11:59
@lizkenyon lizkenyon requested a review from zzooeeyy April 9, 2026 14:18

module ShopifyAPI
module Utils
class ShopValidator
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's also a similar sanitize method from shopify_app gem, I wonder if the logic can be extracted so we don't have to maintain 2 packages for validating Shopify URLS

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I've finally copied the code from there. Once we release a new version, we could use it from shopify_app, as it's already using shopify-api-ruby. Makes sense?

@gonzaloriestra gonzaloriestra force-pushed the river/use-jwt-dest-for-token-exchange branch from 7992177 to 66cb7f4 Compare April 14, 2026 12:46
@gonzaloriestra gonzaloriestra force-pushed the river/use-jwt-dest-for-token-exchange branch from 66cb7f4 to d51f23a Compare April 14, 2026 12:57
requested_token_type: RequestedTokenType,
).returns(ShopifyAPI::Auth::Session)
end
def exchange_token(shop:, session_token:, requested_token_type:)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would prefer we don't remove this in this PR. But rather log a warning that this will be remove in a future version. i.e we should deprecate now remove later.

This will allow us to group our breaking changes together.

  ShopifyAPI::Logger.deprecated(
    "The `shop` parameter for `exchange_token` is deprecated and will be removed in v17. " \
    "The shop is now always taken from the session token's `dest` claim.",
    "17.0.0"
  )

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Oh, sorry, I misread your previous comment. Updated!

@gonzaloriestra gonzaloriestra force-pushed the river/use-jwt-dest-for-token-exchange branch from d51f23a to 89b2da5 Compare April 15, 2026 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants