Skip to content

Commit dc3cb4d

Browse files
EvilGenius13karreiro
authored andcommitted
Revert "Fix 401 Unauthorized on cart AJAX endpoints during shopify theme dev"
1 parent c835661 commit dc3cb4d

2 files changed

Lines changed: 3 additions & 130 deletions

File tree

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

Lines changed: 1 addition & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
patchRenderingResponse,
66
proxyStorefrontRequest,
77
} from './proxy.js'
8-
import {describe, test, expect, vi, afterEach} from 'vitest'
8+
import {describe, test, expect} from 'vitest'
99
import {createEvent} from 'h3'
1010
import {IncomingMessage, ServerResponse} from 'node:http'
1111

@@ -403,116 +403,5 @@ describe('dev proxy', () => {
403403
'Request failed: Hostname mismatch. Expected host: cdn.shopify.com. Resulting URL hostname: evil.com',
404404
)
405405
})
406-
407-
const themeCtx = {
408-
session: {
409-
storeFqdn: 'my-store.myshopify.com',
410-
storefrontToken: 'test-sfr-token',
411-
storefrontPassword: '',
412-
sessionCookies: {},
413-
},
414-
options: {host: 'localhost', port: '9292'},
415-
localThemeFileSystem: {files: new Map()},
416-
localThemeExtensionFileSystem: {files: new Map()},
417-
type: 'theme',
418-
} as unknown as DevServerContext
419-
420-
const extensionCtx = {
421-
...themeCtx,
422-
type: 'theme-extension',
423-
} as unknown as DevServerContext
424-
425-
let fetchMock: ReturnType<typeof vi.fn>
426-
427-
afterEach(() => {
428-
vi.unstubAllGlobals()
429-
})
430-
431-
function stubFetchAndProxy(method: string, path: string, context: DevServerContext) {
432-
fetchMock = vi.fn().mockResolvedValue(new Response('ok'))
433-
vi.stubGlobal('fetch', fetchMock)
434-
const event = createH3Event(method, path)
435-
return proxyStorefrontRequest(event, context)
436-
}
437-
438-
function getAuthHeader(): string | undefined {
439-
const headers = fetchMock.mock.calls[0]![1].headers as {[key: string]: string}
440-
return headers.Authorization ?? headers.authorization
441-
}
442-
443-
test('excludes Bearer token for POST /cart/add.js', async () => {
444-
await stubFetchAndProxy('POST', '/cart/add.js', themeCtx)
445-
expect(getAuthHeader()).toBeUndefined()
446-
})
447-
448-
test('excludes Bearer token for POST /cart/update.js', async () => {
449-
await stubFetchAndProxy('POST', '/cart/update.js', themeCtx)
450-
expect(getAuthHeader()).toBeUndefined()
451-
})
452-
453-
test('excludes Bearer token for GET /cart.js', async () => {
454-
await stubFetchAndProxy('GET', '/cart.js', themeCtx)
455-
expect(getAuthHeader()).toBeUndefined()
456-
})
457-
458-
test('excludes Bearer token for GET /cart.json', async () => {
459-
await stubFetchAndProxy('GET', '/cart.json', themeCtx)
460-
expect(getAuthHeader()).toBeUndefined()
461-
})
462-
463-
test('excludes Bearer token for POST /cart (bare)', async () => {
464-
await stubFetchAndProxy('POST', '/cart', themeCtx)
465-
expect(getAuthHeader()).toBeUndefined()
466-
})
467-
468-
test('excludes Bearer token for POST /cart/change.js', async () => {
469-
await stubFetchAndProxy('POST', '/cart/change.js', themeCtx)
470-
expect(getAuthHeader()).toBeUndefined()
471-
})
472-
473-
test('excludes Bearer token for GET /checkouts/abc123', async () => {
474-
await stubFetchAndProxy('GET', '/checkouts/abc123', themeCtx)
475-
expect(getAuthHeader()).toBeUndefined()
476-
})
477-
478-
test('excludes Bearer token for GET /account/logout', async () => {
479-
await stubFetchAndProxy('GET', '/account/logout', themeCtx)
480-
expect(getAuthHeader()).toBeUndefined()
481-
})
482-
483-
test('excludes Bearer token for GET /cart.js?sections=header (query string)', async () => {
484-
await stubFetchAndProxy('GET', '/cart.js?sections=header', themeCtx)
485-
expect(getAuthHeader()).toBeUndefined()
486-
})
487-
488-
test('excludes Bearer token for POST /cart/add.js?sections=main-cart-items (query string)', async () => {
489-
await stubFetchAndProxy('POST', '/cart/add.js?sections=main-cart-items', themeCtx)
490-
expect(getAuthHeader()).toBeUndefined()
491-
})
492-
493-
test('excludes Bearer token for GET /account?foo=bar (query string with $ anchor)', async () => {
494-
await stubFetchAndProxy('GET', '/account?foo=bar', themeCtx)
495-
expect(getAuthHeader()).toBeUndefined()
496-
})
497-
498-
test('includes Bearer token for GET /cdn/shop/t/10/assets/theme.css', async () => {
499-
await stubFetchAndProxy('GET', '/cdn/shop/t/10/assets/theme.css', themeCtx)
500-
expect(getAuthHeader()).toBe('Bearer test-sfr-token')
501-
})
502-
503-
test('includes Bearer token for GET /some-path.js', async () => {
504-
await stubFetchAndProxy('GET', '/some-path.js', themeCtx)
505-
expect(getAuthHeader()).toBe('Bearer test-sfr-token')
506-
})
507-
508-
test('includes Bearer token for GET /checkouts/internal/something (negative lookahead)', async () => {
509-
await stubFetchAndProxy('GET', '/checkouts/internal/something', themeCtx)
510-
expect(getAuthHeader()).toBe('Bearer test-sfr-token')
511-
})
512-
513-
test('excludes Bearer token for theme-extension context', async () => {
514-
await stubFetchAndProxy('GET', '/cdn/shop/t/10/assets/theme.css', extensionCtx)
515-
expect(getAuthHeader()).toBeUndefined()
516-
})
517406
})
518407
})

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

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@ export const EXTENSION_CDN_PREFIX = '/ext/cdn/'
1616

1717
const API_COLLECT_PATH = '/api/collect'
1818
const CART_PATTERN = /^\/cart\//
19-
// Broader than CART_PATTERN (routing) -- covers /cart, /cart.js, /cart.json for auth exclusion.
20-
// CART_PATTERN only matches /cart/... subpaths to avoid proxying GET /cart (the cart HTML page).
21-
const CART_SESSION_PATTERN = /^\/cart(\/|\.js|\.json|$)/
2219
const CHECKOUT_PATTERN = /^\/checkouts\/(?!internal\/)/
2320
const ACCOUNT_PATTERN = /^\/account(\/login\/multipass(\/[^/]+)?|\/logout)?\/?$/
2421
const VANITY_CDN_PATTERN = new RegExp(`^${VANITY_CDN_PREFIX}`)
@@ -117,18 +114,6 @@ export function canProxyRequest(event: H3Event) {
117114
return Boolean(extension) || acceptsType !== '*/*'
118115
}
119116

120-
// Cart, checkout, and account endpoints use session-cookie auth, not Bearer tokens.
121-
// CHECKOUT_PATTERN and ACCOUNT_PATTERN are reused from routing because their
122-
// routing and auth scopes currently align; changes to those patterns should
123-
// consider the auth implications here.
124-
function needsBearerToken(path: string): boolean {
125-
const [pathname] = path.split('?') as [string]
126-
if (CART_SESSION_PATTERN.test(pathname)) return false
127-
if (CHECKOUT_PATTERN.test(pathname)) return false
128-
if (ACCOUNT_PATTERN.test(pathname)) return false
129-
return true
130-
}
131-
132117
function getStoreFqdnForRegEx(ctx: DevServerContext) {
133118
return ctx.session.storeFqdn.replace(/\\/g, '\\\\').replace(/\./g, '\\.')
134119
}
@@ -341,9 +326,8 @@ export function proxyStorefrontRequest(event: H3Event, ctx: DevServerContext): P
341326
Cookie: buildCookies(ctx.session, {headers}),
342327
}
343328

344-
// Cart, checkout, and account endpoints use cookie-based auth.
345-
// Sending a Bearer token causes SFR to select token auth, which lacks cart scopes.
346-
if (ctx.type === 'theme' && needsBearerToken(event.path)) {
329+
// Only include Authorization for theme dev, not theme-extensions
330+
if (ctx.type === 'theme') {
347331
baseHeaders.Authorization = `Bearer ${ctx.session.storefrontToken}`
348332
}
349333

0 commit comments

Comments
 (0)