Skip to content

Harden token exchange shop context handling#2081

Closed
lizkenyon wants to merge 1 commit into
mainfrom
liz/split-authenticated-requested-shop
Closed

Harden token exchange shop context handling#2081
lizkenyon wants to merge 1 commit into
mainfrom
liz/split-authenticated-requested-shop

Conversation

@lizkenyon

Copy link
Copy Markdown
Contributor

What this PR does

  • Hardens token-exchange shop context handling.
  • Separates requested and authenticated shop domain helpers.
  • Aligns authenticated controller helpers with verified token/session context.
  • Updates tests and related docs.

Reviewer's guide to testing

Validated locally with:

bundle exec ruby -Itest test/shopify_app/controller_concerns/token_exchange_test.rb
bundle exec ruby -Itest test/shopify_app/controller_concerns/login_protection_test.rb
bundle exec rake test
bundle exec rubocop lib/shopify_app/controller_concerns/token_exchange.rb test/shopify_app/controller_concerns/token_exchange_test.rb
git diff --check

Things to focus on

  1. lib/shopify_app/controller_concerns/token_exchange.rb
  2. Token-exchange helper and session-activation coverage
  3. Docs/changelog wording

Checklist

  • Update CHANGELOG.md if the changes would impact users
  • Update README.md, if appropriate.
  • Update any relevant pages in /docs, if necessary
  • For security fixes, the Disclosure Policy must be followed.

@github-actions github-actions Bot added the devtools-gardener Post the issue or PR to Slack for the gardener label Jun 18, 2026
@lizkenyon lizkenyon force-pushed the liz/split-authenticated-requested-shop branch from 9b924ba to 1f65f90 Compare June 18, 2026 14:08
)
end

def requested_shopify_domain

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

AI:

This split keeps requested shop context available for bootstrap and routing, while aligning authenticated helpers with verified token/session context.

return false if requested_domain.blank?

authenticated_domain = current_shopify_session&.shop || jwt_shopify_domain
return false if authenticated_domain.blank? || authenticated_domain == requested_domain

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

AI:

This keeps the added validation scoped to cases where both contexts are available, so existing missing/invalid token bounce and retry behavior remains unchanged.

@lizkenyon lizkenyon force-pushed the liz/split-authenticated-requested-shop branch 3 times, most recently from e758b18 to 0e5e485 Compare June 18, 2026 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devtools-gardener Post the issue or PR to Slack for the gardener Hackerone security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant