diff --git a/.changeset/thirty-news-smell.md b/.changeset/thirty-news-smell.md new file mode 100644 index 00000000000..ee227adc9f7 --- /dev/null +++ b/.changeset/thirty-news-smell.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': patch +--- + +Fix Cart rate limiting issue 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 38434b60856..36693898e52 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, vi, afterEach} from 'vitest' +import {describe, test, expect} from 'vitest' import {createEvent} from 'h3' import {IncomingMessage, ServerResponse} from 'node:http' @@ -403,116 +403,5 @@ 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 57ab7a60354..debfc8e6d94 100644 --- a/packages/theme/src/cli/utilities/theme-environment/proxy.ts +++ b/packages/theme/src/cli/utilities/theme-environment/proxy.ts @@ -16,9 +16,6 @@ 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}`) @@ -117,18 +114,6 @@ 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, '\\.') } @@ -341,9 +326,8 @@ export function proxyStorefrontRequest(event: H3Event, ctx: DevServerContext): P Cookie: buildCookies(ctx.session, {headers}), } - // 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)) { + // Only include Authorization for theme dev, not theme-extensions + if (ctx.type === 'theme') { baseHeaders.Authorization = `Bearer ${ctx.session.storefrontToken}` }