Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-cart-ajax-401.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/theme': patch
---

Fix 401 Unauthorized errors on cart AJAX endpoints during `shopify theme dev`
113 changes: 112 additions & 1 deletion packages/theme/src/cli/utilities/theme-environment/proxy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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()
})
})
})
20 changes: 18 additions & 2 deletions packages/theme/src/cli/utilities/theme-environment/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`)
Expand Down Expand Up @@ -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, '\\.')
}
Expand Down Expand Up @@ -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}`
}

Expand Down
Loading