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

Return 403 in non-embedded XHR requests #639

Merged
merged 2 commits into from
Feb 13, 2024
Merged

Conversation

paulomarg
Copy link
Contributor

WHY are these changes introduced?

Closes #470

As pointed out in the issue, we're returning a 302 that runs into CORS errors, making the validateAuthenticatedSession middleware essentially broken for non-embedded apps.

WHAT is this pull request doing?

Returning the same 403 response headers we return for XHR requests for embedded apps. It'll be up to the app to catch and handle those requests, but it'll be a valid response, instead of a 302 that will always fail.

Type of change

  • Patch: Bug (non-breaking change which fixes an issue)

Checklist

  • I have used yarn changeset to create a draft changelog entry (do NOT update the CHANGELOG.md files manually)
  • I have added/updated tests for this change
  • I have documented new APIs/updated the documentation for modified APIs (for public APIs)

@paulomarg paulomarg requested a review from a team as a code owner February 9, 2024 15:06
Copy link
Contributor

@matteodepalo matteodepalo left a comment

Choose a reason for hiding this comment

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

Looking good! Just a minor comment.

packages/shopify-app-express/src/redirect-out-of-app.ts Outdated Show resolved Hide resolved
@paulomarg paulomarg force-pushed the return_403_for_xhr_requests branch from 2b80081 to d2c6674 Compare February 13, 2024 13:33
@paulomarg paulomarg merged commit b74209c into main Feb 13, 2024
10 checks passed
@paulomarg paulomarg deleted the return_403_for_xhr_requests branch February 13, 2024 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[shopify-app-express]: validateAuthenticatedSession for non-embedded apps that make XHR requests for data?
2 participants