From 7017b9e31c8dca04bed008ae9064e67339e4afc8 Mon Sep 17 00:00:00 2001 From: James Meng Date: Wed, 27 Nov 2024 16:02:35 -0800 Subject: [PATCH 1/2] fix - Fix storefront password detection for password-protected shops with redirects --- .changeset/pretty-donkeys-dream.md | 5 ++++ .../storefront-session.test.ts | 29 ++++++++----------- .../theme-environment/storefront-session.ts | 5 +--- 3 files changed, 18 insertions(+), 21 deletions(-) create mode 100644 .changeset/pretty-donkeys-dream.md diff --git a/.changeset/pretty-donkeys-dream.md b/.changeset/pretty-donkeys-dream.md new file mode 100644 index 00000000000..63be209c903 --- /dev/null +++ b/.changeset/pretty-donkeys-dream.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': patch +--- + +Improve storefront password detection for password-protected shops with redirects diff --git a/packages/theme/src/cli/utilities/theme-environment/storefront-session.test.ts b/packages/theme/src/cli/utilities/theme-environment/storefront-session.test.ts index 5d5615b0d28..ff273175640 100644 --- a/packages/theme/src/cli/utilities/theme-environment/storefront-session.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/storefront-session.test.ts @@ -14,9 +14,7 @@ describe('Storefront API', () => { describe('isStorefrontPasswordProtected', () => { test('returns true when the request is redirected to the password page', async () => { // Given - vi.mocked(fetch).mockResolvedValue( - response({status: 302, headers: {location: 'https://store.myshopify.com/password'}}), - ) + vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.com/password'})) // When const isProtected = await isStorefrontPasswordProtected('store.myshopify.com') @@ -25,13 +23,12 @@ describe('Storefront API', () => { expect(isProtected).toBe(true) expect(fetch).toBeCalledWith('https://store.myshopify.com', { method: 'GET', - redirect: 'manual', }) }) test('returns false when request is not redirected', async () => { // Given - vi.mocked(fetch).mockResolvedValue(response({status: 200})) + vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.com'})) // When const isProtected = await isStorefrontPasswordProtected('store.myshopify.com') @@ -40,13 +37,12 @@ describe('Storefront API', () => { expect(isProtected).toBe(false) expect(fetch).toBeCalledWith('https://store.myshopify.com', { method: 'GET', - redirect: 'manual', }) }) test('returns false when store redirects to a different domain', async () => { // Given - vi.mocked(fetch).mockResolvedValue(response({status: 302, headers: {location: 'https://store.myshopify.se'}})) + vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.se'})) // When const isProtected = await isStorefrontPasswordProtected('store.myshopify.com') @@ -57,9 +53,7 @@ describe('Storefront API', () => { test('returns false when store redirects to a different URI', async () => { // Given - vi.mocked(fetch).mockResolvedValue( - response({status: 302, headers: {location: 'https://store.myshopify.com/random'}}), - ) + vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.com/random'})) // When const isProtected = await isStorefrontPasswordProtected('store.myshopify.com') @@ -70,9 +64,7 @@ describe('Storefront API', () => { test('return true when store redirects to //password', async () => { // Given - vi.mocked(fetch).mockResolvedValue( - response({status: 302, headers: {location: 'https://store.myshopify.com/fr-CA/password'}}), - ) + vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.com/fr-CA/password'})) // When const isProtected = await isStorefrontPasswordProtected('store.myshopify.com') @@ -83,9 +75,7 @@ describe('Storefront API', () => { test('returns false if response is not a 302', async () => { // Given - vi.mocked(fetch).mockResolvedValue( - response({status: 301, headers: {location: 'https://store.myshopify.com/random'}}), - ) + vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.com/random'})) // When const isProtected = await isStorefrontPasswordProtected('store.myshopify.com') @@ -195,7 +185,12 @@ describe('Storefront API', () => { // Tests rely on this function because the 'packages/theme' package cannot // directly access node-fetch and they use: new Response('OK', {status: 200}) - function response(mock: {status: number; headers?: {[key: string]: string}; text?: () => Promise}) { + function response(mock: { + status: number + url?: string + headers?: {[key: string]: string} + text?: () => Promise + }) { const setCookieHeader = (mock.headers ?? {})['set-cookie'] ?? '' const setCookieArray = [setCookieHeader] diff --git a/packages/theme/src/cli/utilities/theme-environment/storefront-session.ts b/packages/theme/src/cli/utilities/theme-environment/storefront-session.ts index 40c68e12e93..d8bbb15731c 100644 --- a/packages/theme/src/cli/utilities/theme-environment/storefront-session.ts +++ b/packages/theme/src/cli/utilities/theme-environment/storefront-session.ts @@ -9,12 +9,9 @@ export class ShopifyEssentialError extends Error {} export async function isStorefrontPasswordProtected(storeURL: string): Promise { const response = await fetch(prependHttps(storeURL), { method: 'GET', - redirect: 'manual', }) - if (response.status !== 302) return false - - return response.headers.get('location')?.endsWith('/password') ?? false + return response.url.endsWith('/password') } /** From 115718cdf5d258a1e9ac3310c6f6b8c69a8d5f1a Mon Sep 17 00:00:00 2001 From: James Meng Date: Thu, 28 Nov 2024 08:56:41 -0800 Subject: [PATCH 2/2] Use URL constructor to grab redirect pathname --- .../theme-environment/storefront-session.test.ts | 15 +++++++++++++++ .../theme-environment/storefront-session.ts | 3 ++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/theme/src/cli/utilities/theme-environment/storefront-session.test.ts b/packages/theme/src/cli/utilities/theme-environment/storefront-session.test.ts index ff273175640..5c25602acd1 100644 --- a/packages/theme/src/cli/utilities/theme-environment/storefront-session.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/storefront-session.test.ts @@ -83,6 +83,21 @@ describe('Storefront API', () => { // Then expect(isProtected).toBe(false) }) + + test('ignores query params', async () => { + // Given + vi.mocked(fetch) + .mockResolvedValueOnce(response({status: 200, url: 'https://store.myshopify.com/random?a=b'})) + .mockResolvedValueOnce(response({status: 200, url: 'https://store.myshopify.com/password?a=b'})) + + // When + const redirectToRandomPath = await isStorefrontPasswordProtected('store.myshopify.com') + const redirectToPasswordPath = await isStorefrontPasswordProtected('store.myshopify.com') + + // Then + expect(redirectToRandomPath).toBe(false) + expect(redirectToPasswordPath).toBe(true) + }) }) describe('getStorefrontSessionCookies', () => { diff --git a/packages/theme/src/cli/utilities/theme-environment/storefront-session.ts b/packages/theme/src/cli/utilities/theme-environment/storefront-session.ts index d8bbb15731c..b7510e0d799 100644 --- a/packages/theme/src/cli/utilities/theme-environment/storefront-session.ts +++ b/packages/theme/src/cli/utilities/theme-environment/storefront-session.ts @@ -11,7 +11,8 @@ export async function isStorefrontPasswordProtected(storeURL: string): Promise