Skip to content

Commit

Permalink
Replace isStorefrontPasswordProtected with API call
Browse files Browse the repository at this point in the history
We were previously determining whether or not a storefront was password
protected by making a HTTP request to the storefront and checking the
response status. This worked for us in the past but is a known fragile
piece.

We've now shipped the ability to query whether the storefront is
password protected directly in the Admin API so we no longer need to
guess. This replaces all instances of `isStorefrontPasswordProtected`
with the updated API call.
  • Loading branch information
graygilmore committed Jan 14, 2025
1 parent 271e28c commit 1afd8f6
Show file tree
Hide file tree
Showing 13 changed files with 92 additions and 129 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {describe, test, expect, beforeEach, vi} from 'vitest'
import {ensureAuthenticatedAdmin, ensureAuthenticatedStorefront} from '@shopify/cli-kit/node/session'
import {Config} from '@oclif/core'
import {getEnvironmentVariables} from '@shopify/cli-kit/node/environment'
import {isStorefrontPasswordProtected} from '@shopify/theme'
import {isPasswordProtected} from '@shopify/theme'
import {fetchTheme} from '@shopify/cli-kit/node/themes/api'

vi.mock('../../context/identifiers.js')
Expand All @@ -59,7 +59,7 @@ beforeEach(() => {
})
vi.mocked(ensureAuthenticatedStorefront).mockResolvedValue('storefront-token')
vi.mocked(getEnvironmentVariables).mockReturnValue({})
vi.mocked(isStorefrontPasswordProtected).mockResolvedValue(false)
vi.mocked(isPasswordProtected).mockResolvedValue(false)
vi.mocked(fetchTheme).mockResolvedValue({
id: 1,
name: 'Theme',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {fetchTheme} from '@shopify/cli-kit/node/themes/api'
import {AbortError} from '@shopify/cli-kit/node/error'
import {Theme} from '@shopify/cli-kit/node/themes/types'
import {renderInfo, renderTasks, Task} from '@shopify/cli-kit/node/ui'
import {initializeDevelopmentExtensionServer, ensureValidPassword, isStorefrontPasswordProtected} from '@shopify/theme'
import {initializeDevelopmentExtensionServer, ensureValidPassword, isPasswordProtected} from '@shopify/theme'
import {partnersFqdn} from '@shopify/cli-kit/node/context/fqdn'

interface ThemeAppExtensionServerOptions {
Expand Down Expand Up @@ -52,7 +52,7 @@ export async function setupPreviewThemeAppExtensionsProcess(
])

const storeFqdn = adminSession.storeFqdn
const storefrontPassword = (await isStorefrontPasswordProtected(storeFqdn))
const storefrontPassword = (await isPasswordProtected(adminSession))
? await ensureValidPassword('', storeFqdn)
: undefined

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/* eslint-disable @typescript-eslint/consistent-type-definitions */
import * as Types from './types.js'

import {TypedDocumentNode as DocumentNode} from '@graphql-typed-document-node/core'

export type OnlineStorePasswordProtectionQueryVariables = Types.Exact<{[key: string]: never}>

export type OnlineStorePasswordProtectionQuery = {onlineStore: {passwordProtection: {enabled: boolean}}}

export const OnlineStorePasswordProtection = {
kind: 'Document',
definitions: [
{
kind: 'OperationDefinition',
operation: 'query',
name: {kind: 'Name', value: 'OnlineStorePasswordProtection'},
selectionSet: {
kind: 'SelectionSet',
selections: [
{
kind: 'Field',
name: {kind: 'Name', value: 'onlineStore'},
selectionSet: {
kind: 'SelectionSet',
selections: [
{
kind: 'Field',
name: {kind: 'Name', value: 'passwordProtection'},
selectionSet: {
kind: 'SelectionSet',
selections: [
{kind: 'Field', name: {kind: 'Name', value: 'enabled'}},
{kind: 'Field', name: {kind: 'Name', value: '__typename'}},
],
},
},
{kind: 'Field', name: {kind: 'Name', value: '__typename'}},
],
},
},
],
},
},
],
} as unknown as DocumentNode<OnlineStorePasswordProtectionQuery, OnlineStorePasswordProtectionQueryVariables>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
query OnlineStorePasswordProtection {
onlineStore {
passwordProtection {
enabled
}
}
}
12 changes: 12 additions & 0 deletions packages/cli-kit/src/public/node/themes/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
import {MetafieldDefinitionsByOwnerType} from '../../../cli/api/graphql/admin/generated/metafield_definitions_by_owner_type.js'
import {GetThemes} from '../../../cli/api/graphql/admin/generated/get_themes.js'
import {GetTheme} from '../../../cli/api/graphql/admin/generated/get_theme.js'
import {OnlineStorePasswordProtection} from '../../../cli/api/graphql/admin/generated/online_store_password_protection.js'
import {restRequest, RestResponse, adminRequestDoc} from '@shopify/cli-kit/node/api/admin'
import {AdminSession} from '@shopify/cli-kit/node/session'
import {AbortError} from '@shopify/cli-kit/node/error'
Expand Down Expand Up @@ -353,6 +354,17 @@ export async function metafieldDefinitionsByOwnerType(type: MetafieldOwnerType,
}))
}

export async function passwordProtected(session: AdminSession): Promise<boolean> {
const {onlineStore} = await adminRequestDoc(OnlineStorePasswordProtection, session)
if (!onlineStore) {
unexpectedGraphQLError("Unable to get details about the storefront's password protection")
}

const {passwordProtection} = onlineStore

return passwordProtection.enabled
}

async function request<T>(
method: string,
path: string,
Expand Down
11 changes: 4 additions & 7 deletions packages/theme/src/cli/services/console.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import {ensureReplEnv} from './console.js'
import {
isStorefrontPasswordCorrect,
isStorefrontPasswordProtected,
} from '../utilities/theme-environment/storefront-session.js'
import {isStorefrontPasswordCorrect, isPasswordProtected} from '../utilities/theme-environment/storefront-session.js'
import {ensureValidPassword} from '../utilities/theme-environment/storefront-password-prompt.js'
import {beforeEach, describe, expect, test, vi} from 'vitest'
import {AdminSession} from '@shopify/cli-kit/node/session'
Expand Down Expand Up @@ -30,7 +27,7 @@ describe('ensureReplEnv', () => {

test('should prompt for password when storefront is password protected', async () => {
// Given
vi.mocked(isStorefrontPasswordProtected).mockResolvedValue(true)
vi.mocked(isPasswordProtected).mockResolvedValue(true)
vi.mocked(isStorefrontPasswordCorrect).mockResolvedValue(true)

// When
Expand All @@ -43,7 +40,7 @@ describe('ensureReplEnv', () => {

test('should skip prompt and return undefined for password when storefront is not password protected', async () => {
// Given
vi.mocked(isStorefrontPasswordProtected).mockResolvedValue(false)
vi.mocked(isPasswordProtected).mockResolvedValue(false)
vi.mocked(isStorefrontPasswordCorrect).mockResolvedValue(true)

// When
Expand All @@ -56,7 +53,7 @@ describe('ensureReplEnv', () => {

test('should return undefined for storePassword when password is provided but storefront is not password protected', async () => {
// Given
vi.mocked(isStorefrontPasswordProtected).mockResolvedValue(false)
vi.mocked(isPasswordProtected).mockResolvedValue(false)
vi.mocked(isStorefrontPasswordCorrect).mockResolvedValue(true)

// When
Expand Down
4 changes: 2 additions & 2 deletions packages/theme/src/cli/services/console.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {isStorefrontPasswordProtected} from '../utilities/theme-environment/storefront-session.js'
import {isPasswordProtected} from '../utilities/theme-environment/storefront-session.js'
import {REPLThemeManager} from '../utilities/repl/repl-theme-manager.js'
import {ensureValidPassword} from '../utilities/theme-environment/storefront-password-prompt.js'
import {replLoop} from '../utilities/repl/repl.js'
Expand All @@ -9,7 +9,7 @@ import {consoleLog} from '@shopify/cli-kit/node/output'
export async function ensureReplEnv(adminSession: AdminSession, storePasswordFlag?: string) {
const themeId = await findOrCreateReplTheme(adminSession)

const storePassword = (await isStorefrontPasswordProtected(adminSession.storeFqdn))
const storePassword = (await isPasswordProtected(adminSession))
? await ensureValidPassword(storePasswordFlag, adminSession.storeFqdn)
: undefined

Expand Down
4 changes: 2 additions & 2 deletions packages/theme/src/cli/services/dev.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {dev, DevOptions, openURLSafely, renderLinks} from './dev.js'
import {setupDevServer} from '../utilities/theme-environment/theme-environment.js'
import {mountThemeFileSystem} from '../utilities/theme-fs.js'
import {fakeThemeFileSystem} from '../utilities/theme-fs/theme-fs-mock-factory.js'
import {isStorefrontPasswordProtected} from '../utilities/theme-environment/storefront-session.js'
import {isPasswordProtected} from '../utilities/theme-environment/storefront-session.js'
import {ensureValidPassword} from '../utilities/theme-environment/storefront-password-prompt.js'
import {emptyThemeExtFileSystem} from '../utilities/theme-fs-empty.js'
import {initializeDevServerSession} from '../utilities/theme-environment/dev-server-session.js'
Expand Down Expand Up @@ -69,7 +69,7 @@ describe('dev', () => {
test('calls startDevServer with the correct arguments when the `legacy` option is false', async () => {
// Given
vi.mocked(initializeDevServerSession).mockResolvedValue(session)
vi.mocked(isStorefrontPasswordProtected).mockResolvedValue(true)
vi.mocked(isPasswordProtected).mockResolvedValue(true)
vi.mocked(ensureValidPassword).mockResolvedValue('valid-password')
vi.mocked(fetchChecksums).mockResolvedValue([])
vi.mocked(mountThemeFileSystem).mockReturnValue(localThemeFileSystem)
Expand Down
7 changes: 3 additions & 4 deletions packages/theme/src/cli/services/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {hasRequiredThemeDirectories, mountThemeFileSystem} from '../utilities/th
import {ensureDirectoryConfirmed} from '../utilities/theme-ui.js'
import {setupDevServer} from '../utilities/theme-environment/theme-environment.js'
import {DevServerContext, LiveReload} from '../utilities/theme-environment/types.js'
import {isStorefrontPasswordProtected} from '../utilities/theme-environment/storefront-session.js'
import {isPasswordProtected} from '../utilities/theme-environment/storefront-session.js'
import {ensureValidPassword} from '../utilities/theme-environment/storefront-password-prompt.js'
import {emptyThemeExtFileSystem} from '../utilities/theme-fs-empty.js'
import {initializeDevServerSession} from '../utilities/theme-environment/dev-server-session.js'
Expand Down Expand Up @@ -42,9 +42,8 @@ export async function dev(options: DevOptions) {
return
}

const storefrontPasswordPromise = isStorefrontPasswordProtected(options.adminSession.storeFqdn).then(
(needsPassword) =>
needsPassword ? ensureValidPassword(options.storePassword, options.adminSession.storeFqdn) : undefined,
const storefrontPasswordPromise = await isPasswordProtected(options.adminSession).then((needsPassword) =>
needsPassword ? ensureValidPassword(options.storePassword, options.adminSession.storeFqdn) : undefined,
)

const localThemeExtensionFileSystem = emptyThemeExtFileSystem()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {ensureValidPassword} from './storefront-password-prompt.js'
import {isStorefrontPasswordProtected, isStorefrontPasswordCorrect} from './storefront-session.js'
import {isPasswordProtected, isStorefrontPasswordCorrect} from './storefront-session.js'
import {
getStorefrontPassword,
getThemeStore,
Expand Down Expand Up @@ -33,7 +33,7 @@ describe('ensureValidPassword', () => {

test('should skip prompt for password when correct storefront password is provided', async () => {
// Given
vi.mocked(isStorefrontPasswordProtected).mockResolvedValue(true)
vi.mocked(isPasswordProtected).mockResolvedValue(true)
vi.mocked(isStorefrontPasswordCorrect).mockResolvedValue(true)

// When
Expand All @@ -45,7 +45,7 @@ describe('ensureValidPassword', () => {

test('should prompt for correct password when incorrect password is provided', async () => {
// Given
vi.mocked(isStorefrontPasswordProtected).mockResolvedValue(true)
vi.mocked(isPasswordProtected).mockResolvedValue(true)
vi.mocked(isStorefrontPasswordCorrect)
.mockResolvedValueOnce(false)
.mockResolvedValueOnce(false)
Expand All @@ -60,7 +60,7 @@ describe('ensureValidPassword', () => {

test('should read the password from local storage when no password is provided', async () => {
// Given
vi.mocked(isStorefrontPasswordProtected).mockResolvedValue(true)
vi.mocked(isPasswordProtected).mockResolvedValue(true)
vi.mocked(isStorefrontPasswordCorrect).mockResolvedValue(true)
vi.mocked(getStorefrontPassword).mockReturnValue('testPassword')

Expand All @@ -73,7 +73,7 @@ describe('ensureValidPassword', () => {

test('should set the password in local storage when a password is validated', async () => {
// Given
vi.mocked(isStorefrontPasswordProtected).mockResolvedValue(true)
vi.mocked(isPasswordProtected).mockResolvedValue(true)
vi.mocked(isStorefrontPasswordCorrect).mockResolvedValue(true)

// When
Expand All @@ -85,7 +85,7 @@ describe('ensureValidPassword', () => {

test('should prompt user for password if local storage password is no longer correct', async () => {
// Given
vi.mocked(isStorefrontPasswordProtected).mockResolvedValue(true)
vi.mocked(isPasswordProtected).mockResolvedValue(true)
vi.mocked(isStorefrontPasswordCorrect).mockResolvedValueOnce(false).mockResolvedValue(true)
vi.mocked(getStorefrontPassword).mockReturnValue('incorrectPassword')
vi.mocked(renderTextPrompt).mockResolvedValue('correctPassword')
Expand All @@ -101,7 +101,7 @@ describe('ensureValidPassword', () => {

test('should call ensureThemeStore with the store URL', async () => {
// Given
vi.mocked(isStorefrontPasswordProtected).mockResolvedValue(true)
vi.mocked(isPasswordProtected).mockResolvedValue(true)
vi.mocked(isStorefrontPasswordCorrect).mockResolvedValue(true)
vi.mocked(getThemeStore).mockReturnValue(undefined as any)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,105 +1,11 @@
import {
getStorefrontSessionCookies,
isStorefrontPasswordCorrect,
isStorefrontPasswordProtected,
ShopifyEssentialError,
} from './storefront-session.js'
import {getStorefrontSessionCookies, isStorefrontPasswordCorrect, ShopifyEssentialError} from './storefront-session.js'
import {describe, expect, test, vi} from 'vitest'
import {fetch} from '@shopify/cli-kit/node/http'
import {AbortError} from '@shopify/cli-kit/node/error'

vi.mock('@shopify/cli-kit/node/http')

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: 200, url: 'https://store.myshopify.com/password'}))

// When
const isProtected = await isStorefrontPasswordProtected('store.myshopify.com')

// Then
expect(isProtected).toBe(true)
expect(fetch).toBeCalledWith('https://store.myshopify.com', {
method: 'GET',
})
})

test('returns false when request is not redirected', async () => {
// Given
vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.com'}))

// When
const isProtected = await isStorefrontPasswordProtected('store.myshopify.com')

// Then
expect(isProtected).toBe(false)
expect(fetch).toBeCalledWith('https://store.myshopify.com', {
method: 'GET',
})
})

test('returns false when store redirects to a different domain', async () => {
// Given
vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.se'}))

// When
const isProtected = await isStorefrontPasswordProtected('store.myshopify.com')

// Then
expect(isProtected).toBe(false)
})

test('returns false when store redirects to a different URI', async () => {
// Given
vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.com/random'}))

// When
const isProtected = await isStorefrontPasswordProtected('store.myshopify.com')

// Then
expect(isProtected).toBe(false)
})

test('return true when store redirects to /<locale>/password', async () => {
// Given
vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.com/fr-CA/password'}))

// When
const isProtected = await isStorefrontPasswordProtected('store.myshopify.com')

// Then
expect(isProtected).toBe(true)
})

test('returns false if response is not a 302', async () => {
// Given
vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.com/random'}))

// When
const isProtected = await isStorefrontPasswordProtected('store.myshopify.com')

// 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', () => {
test('retrieves only _shopify_essential cookie when no password is provided', async () => {
// Given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,13 @@ import {defaultHeaders} from './storefront-utils.js'
import {fetch} from '@shopify/cli-kit/node/http'
import {AbortError} from '@shopify/cli-kit/node/error'
import {outputDebug} from '@shopify/cli-kit/node/output'
import {type AdminSession} from '@shopify/cli-kit/node/session'
import {passwordProtected} from '@shopify/cli-kit/node/themes/api'

export class ShopifyEssentialError extends Error {}

export async function isStorefrontPasswordProtected(storeURL: string): Promise<boolean> {
const response = await fetch(prependHttps(storeURL), {
method: 'GET',
})

const redirectLocation = new URL(response.url)
return redirectLocation.pathname.endsWith('/password')
export async function isPasswordProtected(session: AdminSession): Promise<boolean> {
return passwordProtected(session)
}

/**
Expand Down
Loading

0 comments on commit 1afd8f6

Please sign in to comment.