Merge pull request #6839 from Shopify/kd-add-to-cart-bug

Fix 401 Unauthorized on cart AJAX endpoints during `shopify theme dev`

Author: kdaviduik

.changeset/fix-cart-ajax-401.md

@@ -0,0 +1,5 @@
+---
+'@shopify/theme': patch
+---
+
+Fix 401 Unauthorized errors on cart AJAX endpoints during `shopify theme dev`

packages/theme/src/cli/utilities/theme-environment/proxy.test.ts

@@ -5,7 +5,7 @@ import {
   patchRenderingResponse,
   proxyStorefrontRequest,
 } from './proxy.js'
-import {describe, test, expect} from 'vitest'
+import {describe, test, expect, vi, afterEach} from 'vitest'
 import {createEvent} from 'h3'
 import {IncomingMessage, ServerResponse} from 'node:http'
 import {Socket} from 'node:net'
@@ -401,5 +401,116 @@ describe('dev proxy', () => {
         'Request failed: Hostname mismatch. Expected host: cdn.shopify.com. Resulting URL hostname: evil.com',
       )
     })
+
+    const themeCtx = {
+      session: {
+        storeFqdn: 'my-store.myshopify.com',
+        storefrontToken: 'test-sfr-token',
+        storefrontPassword: '',
+        sessionCookies: {},
+      },
+      options: {host: 'localhost', port: '9292'},
+      localThemeFileSystem: {files: new Map()},
+      localThemeExtensionFileSystem: {files: new Map()},
+      type: 'theme',
+    } as unknown as DevServerContext
+
+    const extensionCtx = {
+      ...themeCtx,
+      type: 'theme-extension',
+    } as unknown as DevServerContext
+
+    let fetchMock: ReturnType<typeof vi.fn>
+
+    afterEach(() => {
+      vi.unstubAllGlobals()
+    })
+
+    function stubFetchAndProxy(method: string, path: string, context: DevServerContext) {
+      fetchMock = vi.fn().mockResolvedValue(new Response('ok'))
+      vi.stubGlobal('fetch', fetchMock)
+      const event = createH3Event(method, path)
+      return proxyStorefrontRequest(event, context)
+    }
+
+    function getAuthHeader(): string | undefined {
+      const headers = fetchMock.mock.calls[0]![1].headers as {[key: string]: string}
+      return headers.Authorization ?? headers.authorization
+    }
+
+    test('excludes Bearer token for POST /cart/add.js', async () => {
+      await stubFetchAndProxy('POST', '/cart/add.js', themeCtx)
+      expect(getAuthHeader()).toBeUndefined()
+    })
+
+    test('excludes Bearer token for POST /cart/update.js', async () => {
+      await stubFetchAndProxy('POST', '/cart/update.js', themeCtx)
+      expect(getAuthHeader()).toBeUndefined()
+    })
+
+    test('excludes Bearer token for GET /cart.js', async () => {
+      await stubFetchAndProxy('GET', '/cart.js', themeCtx)
+      expect(getAuthHeader()).toBeUndefined()
+    })
+
+    test('excludes Bearer token for GET /cart.json', async () => {
+      await stubFetchAndProxy('GET', '/cart.json', themeCtx)
+      expect(getAuthHeader()).toBeUndefined()
+    })
+
+    test('excludes Bearer token for POST /cart (bare)', async () => {
+      await stubFetchAndProxy('POST', '/cart', themeCtx)
+      expect(getAuthHeader()).toBeUndefined()
+    })
+
+    test('excludes Bearer token for POST /cart/change.js', async () => {
+      await stubFetchAndProxy('POST', '/cart/change.js', themeCtx)
+      expect(getAuthHeader()).toBeUndefined()
+    })
+
+    test('excludes Bearer token for GET /checkouts/abc123', async () => {
+      await stubFetchAndProxy('GET', '/checkouts/abc123', themeCtx)
+      expect(getAuthHeader()).toBeUndefined()
+    })
+
+    test('excludes Bearer token for GET /account/logout', async () => {
+      await stubFetchAndProxy('GET', '/account/logout', themeCtx)
+      expect(getAuthHeader()).toBeUndefined()
+    })
+
+    test('excludes Bearer token for GET /cart.js?sections=header (query string)', async () => {
+      await stubFetchAndProxy('GET', '/cart.js?sections=header', themeCtx)
+      expect(getAuthHeader()).toBeUndefined()
+    })
+
+    test('excludes Bearer token for POST /cart/add.js?sections=main-cart-items (query string)', async () => {
+      await stubFetchAndProxy('POST', '/cart/add.js?sections=main-cart-items', themeCtx)
+      expect(getAuthHeader()).toBeUndefined()
+    })
+
+    test('excludes Bearer token for GET /account?foo=bar (query string with $ anchor)', async () => {
+      await stubFetchAndProxy('GET', '/account?foo=bar', themeCtx)
+      expect(getAuthHeader()).toBeUndefined()
+    })
+
+    test('includes Bearer token for GET /cdn/shop/t/10/assets/theme.css', async () => {
+      await stubFetchAndProxy('GET', '/cdn/shop/t/10/assets/theme.css', themeCtx)
+      expect(getAuthHeader()).toBe('Bearer test-sfr-token')
+    })
+
+    test('includes Bearer token for GET /some-path.js', async () => {
+      await stubFetchAndProxy('GET', '/some-path.js', themeCtx)
+      expect(getAuthHeader()).toBe('Bearer test-sfr-token')
+    })
+
+    test('includes Bearer token for GET /checkouts/internal/something (negative lookahead)', async () => {
+      await stubFetchAndProxy('GET', '/checkouts/internal/something', themeCtx)
+      expect(getAuthHeader()).toBe('Bearer test-sfr-token')
+    })
+
+    test('excludes Bearer token for theme-extension context', async () => {
+      await stubFetchAndProxy('GET', '/cdn/shop/t/10/assets/theme.css', extensionCtx)
+      expect(getAuthHeader()).toBeUndefined()
+    })
   })
 })

packages/theme/src/cli/utilities/theme-environment/proxy.ts

@@ -16,6 +16,9 @@ export const EXTENSION_CDN_PREFIX = '/ext/cdn/'
 
 const API_COLLECT_PATH = '/api/collect'
 const CART_PATTERN = /^\/cart\//
+// Broader than CART_PATTERN (routing) -- covers /cart, /cart.js, /cart.json for auth exclusion.
+// CART_PATTERN only matches /cart/... subpaths to avoid proxying GET /cart (the cart HTML page).
+const CART_SESSION_PATTERN = /^\/cart(\/|\.js|\.json|$)/
 const CHECKOUT_PATTERN = /^\/checkouts\/(?!internal\/)/
 const ACCOUNT_PATTERN = /^\/account(\/login\/multipass(\/[^/]+)?|\/logout)?\/?$/
 const VANITY_CDN_PATTERN = new RegExp(`^${VANITY_CDN_PREFIX}`)
@@ -114,6 +117,18 @@ export function canProxyRequest(event: H3Event) {
   return Boolean(extension) || acceptsType !== '*/*'
 }
 
+// Cart, checkout, and account endpoints use session-cookie auth, not Bearer tokens.
+// CHECKOUT_PATTERN and ACCOUNT_PATTERN are reused from routing because their
+// routing and auth scopes currently align; changes to those patterns should
+// consider the auth implications here.
+function needsBearerToken(path: string): boolean {
+  const [pathname] = path.split('?') as [string]
+  if (CART_SESSION_PATTERN.test(pathname)) return false
+  if (CHECKOUT_PATTERN.test(pathname)) return false
+  if (ACCOUNT_PATTERN.test(pathname)) return false
+  return true
+}
+
 function getStoreFqdnForRegEx(ctx: DevServerContext) {
   return ctx.session.storeFqdn.replace(/\\/g, '\\\\').replace(/\./g, '\\.')
 }
@@ -326,8 +341,9 @@ export function proxyStorefrontRequest(event: H3Event, ctx: DevServerContext): P
     Cookie: buildCookies(ctx.session, {headers}),
   }
 
-  // Only include Authorization for theme dev, not theme-extensions
-  if (ctx.type === 'theme') {
+  // Cart, checkout, and account endpoints use cookie-based auth.
+  // Sending a Bearer token causes SFR to select token auth, which lacks cart scopes.
+  if (ctx.type === 'theme' && needsBearerToken(event.path)) {
     baseHeaders.Authorization = `Bearer ${ctx.session.storefrontToken}`
   }