Skip to content

Render App Bridge instead of login redirect when shop/host params are missing in embedded apps#3216

Merged
byrichardpowell merged 3 commits into
mainfrom
rpowell/fix-hmr-login-redirect
Jun 24, 2026
Merged

Render App Bridge instead of login redirect when shop/host params are missing in embedded apps#3216
byrichardpowell merged 3 commits into
mainfrom
rpowell/fix-hmr-login-redirect

Conversation

@byrichardpowell

@byrichardpowell byrichardpowell commented May 15, 2026

Copy link
Copy Markdown
Contributor

Problem

During local development of embedded Shopify apps, there's an annoying workflow where the login screen appears inappropriately:

  1. Navigate to a new URL within the app (the id_token / shop / host query params are lost from the URL)
  2. Make a code change
  3. Vite triggers a full page reload (this happens intermittently — not all changes trigger HMR, some cause a full reload)
  4. The server receives a document request with no shop or host params and no Authorization header
  5. validateShopAndHostParams redirects to the login page

This is incorrect because the app is still running inside the Shopify admin iframe — it just lost the URL search params during SPA navigation + reload.

Solution

In validateShopAndHostParams, when shop or host params are missing/invalid, we now render a minimal App Bridge page instead of redirecting to login.

When App Bridge loads inside the Shopify admin iframe, it:

  1. Detects it's embedded in the admin
  2. Retrieves the session token from the parent frame
  3. Re-authenticates the app seamlessly

This is the same mechanism used by the existing "bounce page" flow (/auth/session-token), just applied earlier in the auth pipeline.

The previous code had a throw redirect(config.auth.loginPath) fallback for non-embedded apps, but isEmbeddedApp is hardcoded to true in shopify-app.ts for all apps using this library (the ShopifyAdmin distribution is already excluded earlier in the function). This made the login redirect dead code. This PR removes that dead code — no public APIs are changed.

Note: this is a production behavior change, not just a dev improvement. Any request to an embedded app without shop/host params (e.g., bookmarked deep links, direct URL access) will now get the App Bridge page instead of a login redirect. This is correct behavior — App Bridge will handle re-embedding in the admin.

What changed

  • validate-shop-and-host-params.ts: Replaced the redirectToLoginPath helper with renderAppBridgeOrError. Instead of redirecting to the login page, renders the App Bridge page so it can recover the session. Removed the unused redirect import from react-router and the dead login redirect code path. The login path guard (500 error when authenticate.admin() is called from the login route) is preserved.
  • doc-request-path.test.ts: Updated the 4 test cases for missing/invalid shop/host params to expect an App Bridge page (200 with App Bridge script tag) instead of a login redirect (302).
  • reject-bot-request.test.ts: Updated the "allowed Shopify agents" assertions to expect 200 (App Bridge page) instead of 302 (login redirect), confirming the request passes the bot check and proceeds to auth validation.
  • expect-login-redirect.ts: Removed — no longer used by any test.
  • __test-helpers/index.ts: Removed the expect-login-redirect export.

@github-actions github-actions Bot added the devtools-gardener Post the issue or PR to Slack for the gardener label May 15, 2026
@byrichardpowell byrichardpowell force-pushed the rpowell/fix-hmr-login-redirect branch 3 times, most recently from a58ca7e to 036e3f7 Compare May 19, 2026 13:11
@byrichardpowell byrichardpowell requested a review from lizkenyon May 19, 2026 14:34
… missing in embedded apps

When an embedded app receives a document request without shop or host
query parameters (e.g. after a full page reload during Vite HMR), the
server would redirect to the login page. This is incorrect because the
app is still running inside the Shopify admin iframe — it just lost the
URL search params during navigation/reload.

Instead of redirecting to login, we now render a minimal App Bridge page.
App Bridge detects it's in the admin iframe, retrieves the session token
from the parent frame, and re-authenticates the app seamlessly.

This fixes the annoying workflow during local development where:
1. Navigate to a new URL (loses the id_token param)
2. Make a code change
3. Vite triggers a full page reload (not HMR)
4. Server sees no identifiers and incorrectly shows login

Non-embedded apps (ShopifyAdmin distribution) are unaffected and still
redirect to login as before.
Addresses review feedback from @lizkenyon: when renderAppBridge is reached
with an invalid (but present) shop query param, the raw attacker-controlled
value was previously being interpolated into the Content-Security-Policy
frame-ancestors directive and the Link preconnect header.

Fix it once in renderAppBridge by running api.utils.sanitizeShop on the
query param before passing it to addDocumentResponseHeaders, mirroring what
addDocumentResponseHeadersFactory already does. This protects every caller
of renderAppBridge, not just the new validate-shop-and-host-params path.

Added a regression test that asserts a request with ?shop=evil.com does not
leak the attacker-controlled value into CSP or Link response headers.

Assisted-By: devx/39f88235-ad95-45b0-9ec3-b210adfe8abb
@byrichardpowell byrichardpowell force-pushed the rpowell/fix-hmr-login-redirect branch from 9c75a09 to 8c17765 Compare June 24, 2026 15:57
@byrichardpowell byrichardpowell merged commit 6d913f9 into main Jun 24, 2026
14 checks passed
@byrichardpowell byrichardpowell deleted the rpowell/fix-hmr-login-redirect branch June 24, 2026 17:16
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants