diff --git a/.changeset/fix-cart-ajax-401.md b/.changeset/fix-cart-ajax-401.md new file mode 100644 index 0000000000..4e1ba41513 --- /dev/null +++ b/.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` diff --git a/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts b/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts index dd954934a9..3e6bb409c5 100644 --- a/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts +++ b/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 + + 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() + }) }) }) diff --git a/packages/theme/src/cli/utilities/theme-environment/proxy.ts b/packages/theme/src/cli/utilities/theme-environment/proxy.ts index 9f08555eb5..a2d2e81b6b 100644 --- a/packages/theme/src/cli/utilities/theme-environment/proxy.ts +++ b/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}` }