diff --git a/.changeset/pretty-donkeys-dream.md b/.changeset/pretty-donkeys-dream.md new file mode 100644 index 0000000000..63be209c90 --- /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 49da135dce..6727cdfd59 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') @@ -93,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', () => { @@ -195,7 +200,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 32cc452dae..b40fd26d60 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,10 @@ 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 + const redirectLocation = new URL(response.url) + return redirectLocation.pathname.endsWith('/password') } /**