Fix: respond to CORS preflight OPTIONS requests without authenticating (shopify-app-express)#3238
Open
morgan-coded wants to merge 2 commits into
Open
Fix: respond to CORS preflight OPTIONS requests without authenticating (shopify-app-express)#3238morgan-coded wants to merge 2 commits into
morgan-coded wants to merge 2 commits into
Conversation
…y-app-express) The validateAuthenticatedSession middleware ran every request, including CORS preflight OPTIONS requests, through the full authentication flow. Preflight requests never carry a session token or Authorization header, so they always failed authentication and were redirected to /auth, which then failed CORS. This broke authenticated fetches from admin extensions to an app's own backend. Detect OPTIONS preflight requests at the top of the middleware, set the appropriate CORS headers, and respond with 204 instead of authenticating. This mirrors the existing respondToOptionsRequest helper already used by the shopify-app-remix and shopify-app-react-router packages. Adds unit tests covering the OPTIONS path: the preflight now returns 204 with CORS headers and never invokes session lookup or redirects. Fixes Shopify#1420
github-actions
Bot
added
cla-needed
devtools-gardener
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds explicit handling for CORS preflight OPTIONS requests so validateAuthenticatedSession doesn’t attempt session authentication/redirects for preflights (notably for admin extensions).
Changes:
- Short-circuit
validateAuthenticatedSessionforOPTIONSrequests by responding with CORS headers and204. - Introduce
respondToOptionsRequestmiddleware helper to send the preflight response. - Add Jest/Supertest coverage for the new preflight behavior and publish a patch changeset.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/apps/shopify-app-express/src/middlewares/validate-authenticated-session.ts | Calls a new helper to intercept OPTIONS requests before auth logic runs |
| packages/apps/shopify-app-express/src/middlewares/respond-to-options-request.ts | New helper that returns a 204 and sets CORS-related headers |
| packages/apps/shopify-app-express/src/middlewares/tests/validate-authenticated-session.test.ts | Adds tests ensuring preflights don’t authenticate and return expected CORS headers |
| .changeset/express-options-preflight-cors.md | Patch note describing the preflight behavior change |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+20
to
+22
| if (req.method !== 'OPTIONS') { | ||
| return false; | ||
| } |
Comment on lines
+29
to
+30
| res.header('Access-Control-Allow-Origin', '*'); | ||
| res.header('Access-Control-Allow-Headers', 'Authorization, Content-Type'); |
| 'X-Shopify-Api-Request-Failure-Reauthorize', | ||
| 'X-Shopify-Api-Request-Failure-Reauthorize-Url', | ||
| ]); | ||
| res.header('Access-Control-Max-Age', '7200'); |
Author
|
I have signed the CLA! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WHY are these changes introduced?
Fixes #1420
The
validateAuthenticatedSessionmiddleware in@shopify/shopify-app-expressruns every request — including CORS preflight
OPTIONSrequests — through thefull authentication flow. A preflight request never carries a session cookie or
Authorization: Bearerheader, so it always fails authentication and isredirected to
/auth(HTTP 302). The browser's CORS preflight then fails on theredirect, so the real authenticated request from an admin extension to the app's
own backend never fires.
This is the behavior the maintainer confirmed in #1420: for
OPTIONSwe shouldjust set the CORS headers and respond, instead of trying to authenticate.
WHAT is this pull request doing?
respondToOptionsRequesthelper to the Express package that, forOPTIONSrequests, sets the CORS headers (Access-Control-Allow-Origin,Access-Control-Allow-Headers,Access-Control-Expose-Headers,Access-Control-Max-Age) and responds204without authenticating. Thisadapts the preflight handling already used by
shopify-app-remixandshopify-app-react-routerto the Express middleware path.validateAuthenticatedSession, before any sessionlookup, returning early for preflight requests.
OPTIONSpath: the preflight now returns204with CORS headers, never invokes session lookup, never redirects, and still
handles preflights without an
Originheader.No behavior change for any non-
OPTIONSrequest.How was this verified?
jestforvalidate-authenticated-session.test.ts: 18/18 testspass, including the hardened preflight coverage.
expected 204, got 302— reproducing the reported redirect — and pass oncerestored.
eslint .andtsc --noEmitfor the package: both clean.Type of change
Checklist
pnpm changesetto create a draft changelog entry (do NOT update theCHANGELOG.mdfiles manually)