From bcbaec8d548e8eb8c1b9014f506ba58acd487220 Mon Sep 17 00:00:00 2001 From: River Date: Fri, 23 Oct 2026 13:33:23 +0000 Subject: [PATCH 1/3] Explain a token's missing dev store access instead of dumping the 401 An app-only service account token running `store list`, `store create dev` or `store delete` crashed the CLI with the raw graphql-request error for Business Platform's 401: the full query text and variables, plus a stack trace, presented as a CLI bug. Two things were wrong. cli-kit's GraphQL client hands a 401 back raw so the caller's token refresh can run, but when the refreshed request was rejected too (or there was nothing to refresh) that raw error escaped, skipping the error handler every other failed response goes through. The client now maps a final 401 through that handler, unless the caller asked for raw errors with `handleErrors: false`, and the handler no longer tries to quote a GraphQL body a gateway's 401 doesn't have. On top of that, the store commands now say what the 401 means. The Organizations API refuses a token that holds no organization scope, which for a service account means it has no dev store access. A new `withOrganizationStoreAccess` helper in @shopify/organizations turns that refusal into an error naming the credential and where to fix it: the service account's Dev store management access under Settings > Users > Security, a service account token in place of a legacy automation token, or the user's own permission. `store list`, `store create dev` and `store delete` use it. Co-authored-by: Eddie Gao --- .../service-account-store-access-errors.md | 6 + .../cli-kit/src/private/node/api/graphql.ts | 3 + .../src/public/node/api/graphql.test.ts | 73 +++++++++++++ .../cli-kit/src/public/node/api/graphql.ts | 36 ++++-- .../cli/services/business-platform.test.ts | 103 ++++++++++++++++++ .../src/cli/services/business-platform.ts | 80 +++++++++++++- .../cli/services/dev/create-dev-store.test.ts | 16 ++- .../src/cli/services/dev/create-dev-store.ts | 30 ++--- packages/organizations/src/index.ts | 7 +- .../cli/services/store/business-platform.ts | 7 +- .../src/cli/services/store/delete/dev.test.ts | 16 ++- .../src/cli/services/store/delete/dev.ts | 24 ++-- .../cli/services/store/list/bp-source.test.ts | 15 ++- .../src/cli/services/store/list/bp-source.ts | 18 +-- 14 files changed, 389 insertions(+), 45 deletions(-) create mode 100644 .changeset/service-account-store-access-errors.md create mode 100644 packages/organizations/src/cli/services/business-platform.test.ts diff --git a/.changeset/service-account-store-access-errors.md b/.changeset/service-account-store-access-errors.md new file mode 100644 index 00000000000..1b06672e526 --- /dev/null +++ b/.changeset/service-account-store-access-errors.md @@ -0,0 +1,6 @@ +--- +'@shopify/cli-kit': patch +'@shopify/cli': patch +--- + +Explain which access is missing when a token can't manage dev stores, instead of printing a raw GraphQL 401 with the query and a stack trace. diff --git a/packages/cli-kit/src/private/node/api/graphql.ts b/packages/cli-kit/src/private/node/api/graphql.ts index f01ebd9ea08..60e8e05a62a 100644 --- a/packages/cli-kit/src/private/node/api/graphql.ts +++ b/packages/cli-kit/src/private/node/api/graphql.ts @@ -134,6 +134,9 @@ export function errorHandler(api: string): (error: unknown, requestId?: string) errorMessage = extractedMessages } else if (extractedMessages && status >= 500) { errorMessage = `The ${api} GraphQL API responded with HTTP status ${status}: ${extractedMessages}` + } else if (error.response.errors === undefined) { + // A 401 from an API gateway, for example, has no GraphQL body to quote. + errorMessage = `The ${api} GraphQL API responded unsuccessfully with the HTTP status ${status}.` } else { errorMessage = stringifyMessage(outputContent` The ${outputToken.raw(api)} GraphQL API responded unsuccessfully with${ diff --git a/packages/cli-kit/src/public/node/api/graphql.test.ts b/packages/cli-kit/src/public/node/api/graphql.test.ts index 965a16ec54b..bae1a079382 100644 --- a/packages/cli-kit/src/public/node/api/graphql.test.ts +++ b/packages/cli-kit/src/public/node/api/graphql.test.ts @@ -90,6 +90,15 @@ const handlers = [ }, ) }), + mockApi.query('FailsWithUnauthorizedAndNoBody', () => { + // A gateway's 401 has no GraphQL body, so the typed `HttpResponse.json` doesn't fit here. + return new HttpResponse(null, { + status: 401, + headers: { + 'x-request-id': 'unauthorized-request-id', + }, + }) + }), mockApi.query('FailsWithBadRequest', () => { return HttpResponse.json( { @@ -390,6 +399,70 @@ describe('graphqlRequest', () => { expect(requestCount).toBe(2) }) + describe('a 401 that survives the token refresh', () => { + const query = 'query FailsWithUnauthorizedAndNoBody { example(secret: $secret) }' + const variables = {secret: 'do-not-print-me'} + + test('reports the status without the request when no token refresh function is provided', async () => { + const res = graphqlRequest({query, api: 'mockApi', url: mockedAddress, token: mockToken, variables}) + + await expect(res).rejects.toMatchObject({ + statusCode: 401, + message: + 'The mockApi GraphQL API responded unsuccessfully with the HTTP status 401.\n\nRequest ID: unauthorized-request-id', + }) + await expect(res).rejects.not.toThrow(/do-not-print-me|example\(secret/) + }) + + test('reports the status without the request when the token cannot be refreshed', async () => { + const res = graphqlRequest({ + query, + api: 'mockApi', + url: mockedAddress, + token: mockToken, + variables, + unauthorizedHandler: {type: 'token_refresh', handler: async () => ({token: undefined})}, + }) + + await expect(res).rejects.toMatchObject({statusCode: 401}) + await expect(res).rejects.not.toThrow(/do-not-print-me|example\(secret/) + }) + + test('reports the status without the request when the refreshed token is rejected too', async () => { + const res = graphqlRequest({ + query, + api: 'mockApi', + url: mockedAddress, + token: mockToken, + variables, + unauthorizedHandler: {type: 'token_refresh', handler: async () => ({token: 'refreshed-token'})}, + }) + + let requestCount = 0 + server.events.on('request:start', () => { + requestCount++ + }) + + await expect(res).rejects.toMatchObject({statusCode: 401}) + await expect(res).rejects.not.toThrow(/do-not-print-me|example\(secret/) + expect(requestCount).toBe(2) + }) + + test('keeps the raw error when the caller handles errors itself', async () => { + const res = graphqlRequest({ + query, + api: 'mockApi', + url: mockedAddress, + token: mockToken, + variables, + responseOptions: {handleErrors: false}, + unauthorizedHandler: {type: 'token_refresh', handler: async () => ({token: 'refreshed-token'})}, + }) + + await expect(res).rejects.toMatchObject({response: {status: 401}}) + }) + }) + test('Fails without retry when bad request when unauthorized handler is provided', async () => { const res = graphqlRequest({ query: 'query FailsWithBadRequest { example }', diff --git a/packages/cli-kit/src/public/node/api/graphql.ts b/packages/cli-kit/src/public/node/api/graphql.ts index 137bfd1b365..fe5313482da 100644 --- a/packages/cli-kit/src/public/node/api/graphql.ts +++ b/packages/cli-kit/src/public/node/api/graphql.ts @@ -215,11 +215,18 @@ async function performGraphQLRequest(options: PerformGraphQLRequestOpti } : undefined + const handleErrors = responseOptions?.handleErrors !== false + const request = () => - retryAwareRequest( - {request: rawGraphQLRequest, url, ...behaviour}, - responseOptions?.handleErrors === false ? undefined : errorHandler(api), - ) + retryAwareRequest({request: rawGraphQLRequest, url, ...behaviour}, handleErrors ? errorHandler(api) : undefined) + + // `retryAwareRequest` hands a 401 back raw so the token can be refreshed and the request repeated. Once that is + // settled, the error goes through the same handler as every other failed response: a raw graphql-request error + // carries the whole request, query text included, and would otherwise reach the user as a CLI bug. + const unauthorizedError = (error: unknown): unknown => { + if (!handleErrors || !(error instanceof ClientError) || error.response.status !== 401) return error + return errorHandler(api)(error, requestIdFromClientError(error)) + } const executeWithTimer = () => runWithTimer('cmd_all_timing_network_ms')(async () => { @@ -229,12 +236,16 @@ async function performGraphQLRequest(options: PerformGraphQLRequestOpti } catch (error) { if (error instanceof ClientError && error.response.status === 401 && tokenRefreshUnauthorizedHandlerFunction) { if (await tokenRefreshUnauthorizedHandlerFunction()) { - response = await request() + try { + response = await request() + } catch (retryError) { + throw unauthorizedError(retryError) + } } else { - throw error + throw unauthorizedError(error) } } else { - throw error + throw unauthorizedError(error) } } @@ -274,6 +285,17 @@ async function performGraphQLRequest(options: PerformGraphQLRequestOpti return JSON.parse(result) as TResult } +function requestIdFromClientError(error: ClientError): string | undefined { + try { + // graphql-request types the response without headers, but it carries the fetch Response's headers. + const headers = (error.response as {headers?: {get?: (name: string) => string | null}}).headers + return headers?.get?.('x-request-id') ?? undefined + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + return undefined + } +} + function graphQLCachePrincipalHash(url: string, token: string | undefined): string { return sha256(JSON.stringify([url, token ?? ''])).toString('hex') } diff --git a/packages/organizations/src/cli/services/business-platform.test.ts b/packages/organizations/src/cli/services/business-platform.test.ts new file mode 100644 index 00000000000..ce0f8571879 --- /dev/null +++ b/packages/organizations/src/cli/services/business-platform.test.ts @@ -0,0 +1,103 @@ +import { + isOrganizationAccessError, + OrganizationStoreAccessError, + withOrganizationStoreAccess, +} from './business-platform.js' +import {describe, expect, test, vi, beforeEach} from 'vitest' +import {getAppAutomationToken} from '@shopify/cli-kit/node/environment' +import {isServiceAccountTokenSet} from '@shopify/cli-kit/node/session' +import {AbortError} from '@shopify/cli-kit/node/error' + +vi.mock('@shopify/cli-kit/node/environment') +vi.mock('@shopify/cli-kit/node/session') + +const organization = {id: '299069', businessName: 'A Better Looking Shop'} + +// The shape cli-kit gives a handled 4xx response: a `GraphQLClientError`, which carries `statusCode`. +function handledClientError(statusCode: number): Error { + return Object.assign(new Error(`The Business Platform GraphQL API responded with the HTTP status ${statusCode}.`), { + statusCode, + }) +} + +// The shape graphql-request throws when the caller handles errors itself. +function rawClientError(status: number): Error { + return Object.assign(new Error('GraphQL Error (Code: 401)'), {response: {status, errors: undefined}, request: {}}) +} + +beforeEach(() => { + vi.mocked(isServiceAccountTokenSet).mockReturnValue(false) + vi.mocked(getAppAutomationToken).mockReturnValue(undefined) +}) + +describe('isOrganizationAccessError', () => { + test('recognises a handled 401 and 403', () => { + expect(isOrganizationAccessError(handledClientError(401))).toBe(true) + expect(isOrganizationAccessError(handledClientError(403))).toBe(true) + }) + + test('recognises a raw graphql-request 401', () => { + expect(isOrganizationAccessError(rawClientError(401))).toBe(true) + }) + + test('ignores other statuses and other errors', () => { + expect(isOrganizationAccessError(handledClientError(404))).toBe(false) + expect(isOrganizationAccessError(handledClientError(500))).toBe(false) + expect(isOrganizationAccessError(new Error('boom'))).toBe(false) + expect(isOrganizationAccessError(undefined)).toBe(false) + }) +}) + +describe('OrganizationStoreAccessError', () => { + test('tells a service account which access it lacks and where to grant it', () => { + vi.mocked(isServiceAccountTokenSet).mockReturnValue(true) + + const error = new OrganizationStoreAccessError(organization) + + expect(error).toBeInstanceOf(AbortError) + expect(error.message).toBe("This service account can't manage dev stores in A Better Looking Shop.") + expect(JSON.stringify(error.tryMessage)).toContain('Dev store management') + expect(JSON.stringify(error.tryMessage)).toContain('Settings > Users > Security') + }) + + test('points a legacy automation token at service accounts', () => { + vi.mocked(getAppAutomationToken).mockReturnValue('legacy-token') + + const error = new OrganizationStoreAccessError(organization) + + expect(error.message).toBe("This automation token can't manage dev stores in A Better Looking Shop.") + expect(JSON.stringify(error.tryMessage)).toContain('SHOPIFY_SERVICE_ACCOUNT_TOKEN') + }) + + test('talks about permission for a logged-in user', () => { + const error = new OrganizationStoreAccessError(organization) + + expect(error.message).toBe("You don't have permission to manage dev stores in A Better Looking Shop.") + }) +}) + +describe('withOrganizationStoreAccess', () => { + test('returns the request result', async () => { + await expect(withOrganizationStoreAccess(organization, async () => 'ok')).resolves.toBe('ok') + }) + + test('replaces a refused request with the access error', async () => { + vi.mocked(isServiceAccountTokenSet).mockReturnValue(true) + + const result = withOrganizationStoreAccess(organization, async () => { + throw handledClientError(401) + }) + + await expect(result).rejects.toThrow("This service account can't manage dev stores in A Better Looking Shop.") + }) + + test('rethrows every other error unchanged', async () => { + const boom = new Error('boom') + + await expect( + withOrganizationStoreAccess(organization, async () => { + throw boom + }), + ).rejects.toBe(boom) + }) +}) diff --git a/packages/organizations/src/cli/services/business-platform.ts b/packages/organizations/src/cli/services/business-platform.ts index d52cf98ef21..b6396835231 100644 --- a/packages/organizations/src/cli/services/business-platform.ts +++ b/packages/organizations/src/cli/services/business-platform.ts @@ -1,5 +1,8 @@ +import {type Organization} from '../models/organization.js' import {type UnauthorizedHandler} from '@shopify/cli-kit/node/api/graphql' -import {ensureAuthenticatedBusinessPlatform} from '@shopify/cli-kit/node/session' +import {getAppAutomationToken} from '@shopify/cli-kit/node/environment' +import {AbortError} from '@shopify/cli-kit/node/error' +import {ensureAuthenticatedBusinessPlatform, isServiceAccountTokenSet} from '@shopify/cli-kit/node/session' interface BusinessPlatformTokenRefreshHandlerOptions { noPrompt?: boolean @@ -15,3 +18,78 @@ export function businessPlatformTokenRefreshHandler( }), } } + +/** + * Whether the Organizations API refused a request because the account can't act in the organization. + * + * Business Platform answers 401 when the token holds no organization scope at all (a service account without dev + * store access, or an app automation token) and 403 when the account lacks the permission. By the time the error + * reaches a command, cli-kit has wrapped it with a `statusCode`; a raw graphql-request error, which carries the + * status on its response, is recognised too. + * + * @param error - The error a Business Platform Organizations API request rejected with. + * @returns True when the account was refused access to the organization. + */ +export function isOrganizationAccessError(error: unknown): boolean { + if (typeof error !== 'object' || error === null) return false + const {statusCode, response} = error as {statusCode?: unknown; response?: {status?: unknown}} + const status = statusCode ?? response?.status + return status === 401 || status === 403 +} + +/** + * The error to show when the Organizations API refused access to an organization's stores. + * + * Names the credential that was refused: the service account, a legacy automation token, or the logged-in user. + */ +export class OrganizationStoreAccessError extends AbortError { + constructor(organization: Organization) { + const where = ['in the Shopify admin under', {subdued: 'Settings > Users > Security'}] + + if (isServiceAccountTokenSet()) { + super(`This service account can't manage dev stores in ${organization.businessName}.`, [ + 'Give it', + {bold: 'Dev store management'}, + 'access', + ...where, + {char: ','}, + 'or use a token from a service account that has it.', + ]) + } else if (getAppAutomationToken()) { + super(`This automation token can't manage dev stores in ${organization.businessName}.`, [ + 'Store commands need a service account token with', + {bold: 'Dev store management'}, + 'access. Create one', + ...where, + 'and set it in', + {command: 'SHOPIFY_SERVICE_ACCOUNT_TOKEN'}, + {char: '.'}, + ]) + } else { + super( + `You don't have permission to manage dev stores in ${organization.businessName}.`, + "Check that you're logged in to the right account, or ask an organization admin for access.", + ) + } + } +} + +/** + * Runs an Organizations API request for the organization, turning a refused request into an + * `OrganizationStoreAccessError` so the user learns which access is missing rather than seeing the HTTP status. + * + * @param organization - The organization the request addresses. + * @param request - The request to run. + * @returns The request's result. + */ +export async function withOrganizationStoreAccess( + organization: Organization, + request: () => Promise, +): Promise { + try { + return await request() + } catch (error) { + if (isOrganizationAccessError(error)) throw new OrganizationStoreAccessError(organization) + throw error + } +} diff --git a/packages/organizations/src/cli/services/dev/create-dev-store.test.ts b/packages/organizations/src/cli/services/dev/create-dev-store.test.ts index eb32b2ab673..4b2b503d3ce 100644 --- a/packages/organizations/src/cli/services/dev/create-dev-store.test.ts +++ b/packages/organizations/src/cli/services/dev/create-dev-store.test.ts @@ -3,7 +3,7 @@ import {createDevStore} from './create-dev-store.js' import {describe, expect, test, vi, beforeEach} from 'vitest' import {businessPlatformOrganizationsRequestDoc} from '@shopify/cli-kit/node/api/business-platform' -import {ensureAuthenticatedBusinessPlatform} from '@shopify/cli-kit/node/session' +import {ensureAuthenticatedBusinessPlatform, isServiceAccountTokenSet} from '@shopify/cli-kit/node/session' import {renderSingleTask, renderSuccess} from '@shopify/cli-kit/node/ui' import {outputResult} from '@shopify/cli-kit/node/output' import {sleep} from '@shopify/cli-kit/node/system' @@ -14,6 +14,7 @@ vi.mock('@shopify/cli-kit/node/api/business-platform', () => ({ vi.mock('@shopify/cli-kit/node/session', () => ({ ensureAuthenticatedBusinessPlatform: vi.fn(), + isServiceAccountTokenSet: vi.fn(), })) vi.mock('@shopify/cli-kit/node/ui', () => ({ @@ -77,6 +78,19 @@ describe('createDevStore', () => { expect(renderSuccess).not.toHaveBeenCalled() expect(outputResult).not.toHaveBeenCalled() }) + + test('explains the missing dev store access when the Organizations API refuses a service account', async () => { + vi.mocked(isServiceAccountTokenSet).mockReturnValue(true) + vi.mocked(businessPlatformOrganizationsRequestDoc).mockRejectedValueOnce( + Object.assign(new Error('The Business Platform GraphQL API responded unsuccessfully with the HTTP status 401.'), { + statusCode: 401, + }), + ) + + await expect( + createDevStore({name: 'test-store', organization: defaultOrg, plan: 'plus', json: false, summary: false}), + ).rejects.toThrow("This service account can't manage dev stores in Test Org.") + }) }) test.each([undefined, null, 'https://admin.shopify.com/store/test-store'])( diff --git a/packages/organizations/src/cli/services/dev/create-dev-store.ts b/packages/organizations/src/cli/services/dev/create-dev-store.ts index db9d5eaa238..8bbd14d3125 100644 --- a/packages/organizations/src/cli/services/dev/create-dev-store.ts +++ b/packages/organizations/src/cli/services/dev/create-dev-store.ts @@ -6,7 +6,7 @@ import { PollStoreCreationQuery, } from '../../api/graphql/business-platform-organizations/generated/poll_store_creation.js' import {Organization} from '../../models/organization.js' -import {businessPlatformTokenRefreshHandler} from '../business-platform.js' +import {businessPlatformTokenRefreshHandler, withOrganizationStoreAccess} from '../business-platform.js' import {businessPlatformOrganizationsRequestDoc} from '@shopify/cli-kit/node/api/business-platform' import {AbortError} from '@shopify/cli-kit/node/error' import {outputContent, outputResult} from '@shopify/cli-kit/node/output' @@ -60,19 +60,21 @@ export async function createDevStore(options: CreateDevStoreOptions): Promise + businessPlatformOrganizationsRequestDoc({ + query: CreateAppDevelopmentStore, + token, + organizationId: org.id, + variables: { + shopName: name, + priceLookupKey: DEV_STORE_PLANS[plan], + prepopulateTestData: options.withDemoData ?? false, + developerPreviewHandle: options.featurePreview, + country: options.country, + }, + unauthorizedHandler, + }), + ) const createAppDevelopmentStore = mutationResult.createAppDevelopmentStore if (!createAppDevelopmentStore) { diff --git a/packages/organizations/src/index.ts b/packages/organizations/src/index.ts index e87beacf904..87d73c8346f 100644 --- a/packages/organizations/src/index.ts +++ b/packages/organizations/src/index.ts @@ -3,7 +3,12 @@ export {selectOrg} from './cli/services/select.js' export {selectOrganizationPrompt} from './cli/prompts/organization.js' export type {Organization, OrganizationWithDetails, OrganizationStatus} from './cli/models/organization.js' export {organizationStatusValues} from './cli/models/organization.js' -export {businessPlatformTokenRefreshHandler} from './cli/services/business-platform.js' +export { + businessPlatformTokenRefreshHandler, + isOrganizationAccessError, + OrganizationStoreAccessError, + withOrganizationStoreAccess, +} from './cli/services/business-platform.js' export {createDevStore, devStorePlanHandles} from './cli/services/dev/create-dev-store.js' export type {CreateDevStoreOptions, DevStorePlan} from './cli/services/dev/create-dev-store.js' export {devStoreNamePrompt, devStorePlanPrompt, devStoreDemoDataPrompt} from './cli/prompts/dev.js' diff --git a/packages/store/src/cli/services/store/business-platform.ts b/packages/store/src/cli/services/store/business-platform.ts index 01385af7b63..1e7cf460cc9 100644 --- a/packages/store/src/cli/services/store/business-platform.ts +++ b/packages/store/src/cli/services/store/business-platform.ts @@ -1 +1,6 @@ -export {businessPlatformTokenRefreshHandler} from '@shopify/organizations' +export { + businessPlatformTokenRefreshHandler, + isOrganizationAccessError, + OrganizationStoreAccessError, + withOrganizationStoreAccess, +} from '@shopify/organizations' diff --git a/packages/store/src/cli/services/store/delete/dev.test.ts b/packages/store/src/cli/services/store/delete/dev.test.ts index 20eeacbbdc6..6001c6d699b 100644 --- a/packages/store/src/cli/services/store/delete/dev.test.ts +++ b/packages/store/src/cli/services/store/delete/dev.test.ts @@ -1,7 +1,7 @@ import {deleteDevStoreJsonOutputSchema} from './types.js' import {deleteDevStore, toOrganizationsShopifyShopId} from './dev.js' import {businessPlatformOrganizationsRequestDoc} from '@shopify/cli-kit/node/api/business-platform' -import {ensureAuthenticatedBusinessPlatform} from '@shopify/cli-kit/node/session' +import {ensureAuthenticatedBusinessPlatform, isServiceAccountTokenSet} from '@shopify/cli-kit/node/session' import {renderSingleTask, renderSuccess, renderWarning} from '@shopify/cli-kit/node/ui' import {outputResult} from '@shopify/cli-kit/node/output' import {sleep} from '@shopify/cli-kit/node/system' @@ -13,6 +13,7 @@ vi.mock('@shopify/cli-kit/node/api/business-platform', () => ({ vi.mock('@shopify/cli-kit/node/session', () => ({ ensureAuthenticatedBusinessPlatform: vi.fn(), + isServiceAccountTokenSet: vi.fn(), })) vi.mock('@shopify/cli-kit/node/ui', () => ({ @@ -269,6 +270,19 @@ describe('deleteDevStore', () => { ) expect(renderWarning).not.toHaveBeenCalled() }) + + test('explains the missing dev store access when the Organizations API refuses a service account', async () => { + vi.mocked(isServiceAccountTokenSet).mockReturnValue(true) + vi.mocked(businessPlatformOrganizationsRequestDoc).mockRejectedValue( + Object.assign(new Error('The Business Platform GraphQL API responded unsuccessfully with the HTTP status 401.'), { + statusCode: 401, + }), + ) + + await expect(deleteDevStore(defaultOptions)).rejects.toThrow( + "This service account can't manage dev stores in Test Org.", + ) + }) }) function mockBusinessPlatformRequests(options: {mutationResult?: unknown; pollShops?: PollShop[]} = {}) { diff --git a/packages/store/src/cli/services/store/delete/dev.ts b/packages/store/src/cli/services/store/delete/dev.ts index bc23bc90c03..ff705fc2eb1 100644 --- a/packages/store/src/cli/services/store/delete/dev.ts +++ b/packages/store/src/cli/services/store/delete/dev.ts @@ -1,5 +1,5 @@ import {deleteDevStoreJsonOutputSchema} from './types.js' -import {businessPlatformTokenRefreshHandler} from '../business-platform.js' +import {businessPlatformTokenRefreshHandler, withOrganizationStoreAccess} from '../business-platform.js' import {fetchOptionalOrganizationShop} from '../../../utilities/store-lookup/organization-shop.js' import {DeleteAppDevelopmentStore} from '../../../api/graphql/business-platform-organizations/generated/delete_app_development_store.js' import { @@ -37,15 +37,19 @@ export async function deleteDevStore(options: DeleteDevStoreOptions): Promise + fetchShopifyShopId({store, organizationId: org.id, token}), + ) + + const mutationResult = await withOrganizationStoreAccess(org, () => + businessPlatformOrganizationsRequestDoc({ + query: DeleteAppDevelopmentStore, + token, + organizationId: org.id, + variables: {storeFqdn: store}, + unauthorizedHandler, + }), + ) const deleteAppDevelopmentStore = mutationResult.deleteAppDevelopmentStore if (!deleteAppDevelopmentStore) { diff --git a/packages/store/src/cli/services/store/list/bp-source.test.ts b/packages/store/src/cli/services/store/list/bp-source.test.ts index 66dde45dae9..e888e1ad1fc 100644 --- a/packages/store/src/cli/services/store/list/bp-source.test.ts +++ b/packages/store/src/cli/services/store/list/bp-source.test.ts @@ -3,7 +3,7 @@ import {type ListAccessibleShopsQuery} from '../../../api/graphql/business-platf import {beforeEach, describe, expect, test, vi} from 'vitest' import {businessPlatformOrganizationsRequestDoc} from '@shopify/cli-kit/node/api/business-platform' import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output' -import {ensureAuthenticatedBusinessPlatform} from '@shopify/cli-kit/node/session' +import {ensureAuthenticatedBusinessPlatform, isServiceAccountTokenSet} from '@shopify/cli-kit/node/session' import {AbortError} from '@shopify/cli-kit/node/error' import {type Organization} from '@shopify/organizations' @@ -274,4 +274,17 @@ describe('listBusinessPlatformStores', () => { allowAutomationToken: true, }) }) + + test('explains the missing dev store access when the Organizations API refuses a service account', async () => { + vi.mocked(isServiceAccountTokenSet).mockReturnValue(true) + vi.mocked(businessPlatformOrganizationsRequestDoc).mockRejectedValue( + Object.assign(new Error('The Business Platform GraphQL API responded unsuccessfully with the HTTP status 401.'), { + statusCode: 401, + }), + ) + + await expect(listBusinessPlatformStores({token: 'bp-token', organization})).rejects.toThrow( + "This service account can't manage dev stores in Acme.", + ) + }) }) diff --git a/packages/store/src/cli/services/store/list/bp-source.ts b/packages/store/src/cli/services/store/list/bp-source.ts index ada4a7831f5..42dfd518d78 100644 --- a/packages/store/src/cli/services/store/list/bp-source.ts +++ b/packages/store/src/cli/services/store/list/bp-source.ts @@ -1,6 +1,6 @@ import {STORE_LIST_LIMIT} from './constants.js' import {type StoreListEntry} from './types.js' -import {businessPlatformTokenRefreshHandler} from '../business-platform.js' +import {businessPlatformTokenRefreshHandler, withOrganizationStoreAccess} from '../business-platform.js' import {planHandle} from '../plan.js' import {storeTypeFilterValue, storeTypeHandle, type StoreTypeFilter} from '../store-type.js' import { @@ -44,13 +44,15 @@ async function fetchOrganizationStores( ): Promise<{entries: StoreListEntry[]; hasMore: boolean}> { const unauthorizedHandler = businessPlatformTokenRefreshHandler() - const result = await businessPlatformOrganizationsRequestDoc({ - query: ListAccessibleShops, - token, - organizationId: organization.id, - variables: {first: STORE_LIST_LIMIT, filters: shopFilters(storeType)}, - unauthorizedHandler, - }) + const result = await withOrganizationStoreAccess(organization, () => + businessPlatformOrganizationsRequestDoc({ + query: ListAccessibleShops, + token, + organizationId: organization.id, + variables: {first: STORE_LIST_LIMIT, filters: shopFilters(storeType)}, + unauthorizedHandler, + }), + ) const accessibleShops = result.organization?.accessibleShops if (!accessibleShops) return {entries: [], hasMore: false} From ef7faa31cf5651dd15300b2bb4a784d3e656db66 Mon Sep 17 00:00:00 2001 From: River Date: Fri, 23 Oct 2026 14:04:25 +0000 Subject: [PATCH 2/3] Drop unused re-exports and keep app dev's 401 message Knip failed on the two names the store package re-exported from @shopify/organizations but never imports: isOrganizationAccessError and OrganizationStoreAccessError. The store commands only use withOrganizationStoreAccess. cli-kit now reports a 401 that survives the token refresh as an error with a statusCode instead of a raw graphql-request ClientError. app dev's upload step caught that shape with a bare `Error('Unauthorized')`, which would have replaced its "Auth session expired. Please run `shopify app dev` again." message. It now raises that same AbortError, and a test covers it. Co-authored-by: Eddie Gao --- .../dev-session/dev-session-process.test.ts | 22 +++++++++++++++++++ .../dev/processes/dev-session/dev-session.ts | 4 +++- .../cli/services/store/business-platform.ts | 7 +----- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts index 70ee061db60..d4efca19ace 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts @@ -16,6 +16,7 @@ import {getUploadURL, writeManifestToBundle} from '../../../bundle.js' import {formData} from '@shopify/cli-kit/node/http' import {describe, expect, test, vi, beforeEach, afterEach} from 'vitest' import {AbortController} from '@shopify/cli-kit/node/abort' +import {AbortError} from '@shopify/cli-kit/node/error' import {flushPromises} from '@shopify/cli-kit/node/promises' import * as outputContext from '@shopify/cli-kit/node/ui/components' import {readdir} from '@shopify/cli-kit/node/fs' @@ -168,6 +169,27 @@ describe('pushUpdatesForDevSession', () => { expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Test error')) }) + test('asks to restart app dev when the upload URL request is still unauthorized after a token refresh', async () => { + // Given: cli-kit reports a 401 that survives the token refresh with its status code. + vi.mocked(getUploadURL).mockRejectedValue( + Object.assign( + new AbortError('The App Management GraphQL API responded unsuccessfully with the HTTP status 401.'), + { + statusCode: 401, + }, + ), + ) + + // When + await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) + const [onStart] = appWatcher.listeners('ready') as ((event: unknown) => Promise)[] + + // Then + await expect(onStart!({app, extensionEvents: []})).rejects.toThrow( + new AbortError('Auth session expired. Please run `shopify app dev` again.'), + ) + }) + test('handles receiving an event before session is ready', async () => { // When await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts index 0a9dbc318f7..bcab3c1647f 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts @@ -334,8 +334,10 @@ export class DevSession { } // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (error: any) { + // cli-kit reports a 401 that survives the token refresh with its `statusCode`; a raw graphql-request error + // carries the status on its response. if (error.statusCode === 401) { - throw new Error('Unauthorized') + throw new AbortError('Auth session expired. Please run `shopify app dev` again.') } else if (error instanceof ClientError) { if (error.response.status === 401 || error.response.status === 403) { throw new AbortError('Auth session expired. Please run `shopify app dev` again.') diff --git a/packages/store/src/cli/services/store/business-platform.ts b/packages/store/src/cli/services/store/business-platform.ts index 1e7cf460cc9..9480934d78c 100644 --- a/packages/store/src/cli/services/store/business-platform.ts +++ b/packages/store/src/cli/services/store/business-platform.ts @@ -1,6 +1 @@ -export { - businessPlatformTokenRefreshHandler, - isOrganizationAccessError, - OrganizationStoreAccessError, - withOrganizationStoreAccess, -} from '@shopify/organizations' +export {businessPlatformTokenRefreshHandler, withOrganizationStoreAccess} from '@shopify/organizations' From 29322214d452c331cf54af69276716b3f32ab157 Mon Sep 17 00:00:00 2001 From: River Date: Fri, 23 Oct 2026 15:03:03 +0000 Subject: [PATCH 3/3] Route every store Organizations API request through one helper Review follow-up. The access guidance moves from tryMessage to nextSteps. `store create dev --json` and `store delete --json` only serialize message and nextSteps, so the guidance on which access to grant and where was lost from their JSON output. Command tests now cover both JSON paths. withOrganizationStoreAccess, which each call site had to remember to wrap around its request, is replaced by organizationStoresRequestDoc in @shopify/organizations. It sends a Business Platform Organizations API request for an organization and turns a refused one into OrganizationStoreAccessError. Every Organizations API request in the store and organizations packages now goes through it: store list, store create dev and its creation poll, store delete and its confirmation poll, and the shop lookup behind store info and store delete. A new request gets the same handling by using the helper. Co-authored-by: Eddie Gao --- .../cli/services/business-platform.test.ts | 58 ++++++++++----- .../src/cli/services/business-platform.ts | 73 ++++++++++--------- .../src/cli/services/dev/create-dev-store.ts | 33 ++++----- packages/organizations/src/index.ts | 4 +- .../src/cli/commands/store/create/dev.test.ts | 28 ++++++- .../src/cli/commands/store/delete.test.ts | 28 +++++++ .../cli/services/store/business-platform.ts | 2 +- .../src/cli/services/store/delete/dev.ts | 45 ++++++------ .../src/cli/services/store/info/index.test.ts | 14 +++- .../src/cli/services/store/info/index.ts | 7 +- .../src/cli/services/store/list/bp-source.ts | 18 ++--- .../store-lookup/organization-shop.ts | 12 ++- 12 files changed, 207 insertions(+), 115 deletions(-) diff --git a/packages/organizations/src/cli/services/business-platform.test.ts b/packages/organizations/src/cli/services/business-platform.test.ts index ce0f8571879..b5f3e5795e5 100644 --- a/packages/organizations/src/cli/services/business-platform.test.ts +++ b/packages/organizations/src/cli/services/business-platform.test.ts @@ -1,14 +1,16 @@ import { isOrganizationAccessError, OrganizationStoreAccessError, - withOrganizationStoreAccess, + organizationStoresRequestDoc, } from './business-platform.js' import {describe, expect, test, vi, beforeEach} from 'vitest' +import {businessPlatformOrganizationsRequestDoc} from '@shopify/cli-kit/node/api/business-platform' import {getAppAutomationToken} from '@shopify/cli-kit/node/environment' import {isServiceAccountTokenSet} from '@shopify/cli-kit/node/session' import {AbortError} from '@shopify/cli-kit/node/error' vi.mock('@shopify/cli-kit/node/environment') +vi.mock('@shopify/cli-kit/node/api/business-platform') vi.mock('@shopify/cli-kit/node/session') const organization = {id: '299069', businessName: 'A Better Looking Shop'} @@ -56,8 +58,12 @@ describe('OrganizationStoreAccessError', () => { expect(error).toBeInstanceOf(AbortError) expect(error.message).toBe("This service account can't manage dev stores in A Better Looking Shop.") - expect(JSON.stringify(error.tryMessage)).toContain('Dev store management') - expect(JSON.stringify(error.tryMessage)).toContain('Settings > Users > Security') + // The guidance lives in nextSteps, the field the store commands' --json output carries. + expect(error.tryMessage).toBeNull() + expect(error.nextSteps).toEqual([ + 'Give the service account Dev store management access in the Shopify admin under Settings > Users > Security.', + 'Or use a token from a service account that has that access.', + ]) }) test('points a legacy automation token at service accounts', () => { @@ -66,38 +72,54 @@ describe('OrganizationStoreAccessError', () => { const error = new OrganizationStoreAccessError(organization) expect(error.message).toBe("This automation token can't manage dev stores in A Better Looking Shop.") - expect(JSON.stringify(error.tryMessage)).toContain('SHOPIFY_SERVICE_ACCOUNT_TOKEN') + expect(error.nextSteps?.join(' ')).toContain('SHOPIFY_SERVICE_ACCOUNT_TOKEN') }) test('talks about permission for a logged-in user', () => { const error = new OrganizationStoreAccessError(organization) expect(error.message).toBe("You don't have permission to manage dev stores in A Better Looking Shop.") + expect(error.nextSteps).toEqual([ + "Check that you're logged in to the right account.", + 'Or ask an organization admin for access.', + ]) + }) + + test('falls back to the organization ID when the name is unknown', () => { + const error = new OrganizationStoreAccessError({id: '299069'}) + + expect(error.message).toBe("You don't have permission to manage dev stores in organization 299069.") }) }) -describe('withOrganizationStoreAccess', () => { - test('returns the request result', async () => { - await expect(withOrganizationStoreAccess(organization, async () => 'ok')).resolves.toBe('ok') +describe('organizationStoresRequestDoc', () => { + const request = { + query: {} as never, + token: 'token', + variables: {first: 1}, + unauthorizedHandler: {type: 'token_refresh' as const, handler: async () => ({token: 'refreshed'})}, + } + + test("sends the request to the organization's Organizations API and returns the result", async () => { + vi.mocked(businessPlatformOrganizationsRequestDoc).mockResolvedValue({ok: true}) + + await expect(organizationStoresRequestDoc(organization, request)).resolves.toEqual({ok: true}) + expect(businessPlatformOrganizationsRequestDoc).toHaveBeenCalledWith({...request, organizationId: '299069'}) }) test('replaces a refused request with the access error', async () => { vi.mocked(isServiceAccountTokenSet).mockReturnValue(true) + vi.mocked(businessPlatformOrganizationsRequestDoc).mockRejectedValue(handledClientError(401)) - const result = withOrganizationStoreAccess(organization, async () => { - throw handledClientError(401) - }) - - await expect(result).rejects.toThrow("This service account can't manage dev stores in A Better Looking Shop.") + await expect(organizationStoresRequestDoc(organization, request)).rejects.toThrow( + "This service account can't manage dev stores in A Better Looking Shop.", + ) }) test('rethrows every other error unchanged', async () => { - const boom = new Error('boom') + const notFound = handledClientError(404) + vi.mocked(businessPlatformOrganizationsRequestDoc).mockRejectedValue(notFound) - await expect( - withOrganizationStoreAccess(organization, async () => { - throw boom - }), - ).rejects.toBe(boom) + await expect(organizationStoresRequestDoc(organization, request)).rejects.toBe(notFound) }) }) diff --git a/packages/organizations/src/cli/services/business-platform.ts b/packages/organizations/src/cli/services/business-platform.ts index b6396835231..763cc1c1a42 100644 --- a/packages/organizations/src/cli/services/business-platform.ts +++ b/packages/organizations/src/cli/services/business-platform.ts @@ -1,5 +1,8 @@ -import {type Organization} from '../models/organization.js' -import {type UnauthorizedHandler} from '@shopify/cli-kit/node/api/graphql' +import { + businessPlatformOrganizationsRequestDoc, + type BusinessPlatformOrganizationsRequestOptions, +} from '@shopify/cli-kit/node/api/business-platform' +import {type GraphQLVariables, type UnauthorizedHandler} from '@shopify/cli-kit/node/api/graphql' import {getAppAutomationToken} from '@shopify/cli-kit/node/environment' import {AbortError} from '@shopify/cli-kit/node/error' import {ensureAuthenticatedBusinessPlatform, isServiceAccountTokenSet} from '@shopify/cli-kit/node/session' @@ -19,6 +22,14 @@ export function businessPlatformTokenRefreshHandler( } } +/** + * The organization a request addresses. The name is optional because some callers only know the ID. + */ +export interface OrganizationReference { + id: string + businessName?: string +} + /** * Whether the Organizations API refused a request because the account can't act in the organization. * @@ -40,54 +51,50 @@ export function isOrganizationAccessError(error: unknown): boolean { /** * The error to show when the Organizations API refused access to an organization's stores. * - * Names the credential that was refused: the service account, a legacy automation token, or the logged-in user. + * Names the credential that was refused (the service account, a legacy automation token, or the logged-in user) and + * puts the way to fix it in `nextSteps`, which both the terminal banner and the store commands' `--json` output show. */ export class OrganizationStoreAccessError extends AbortError { - constructor(organization: Organization) { - const where = ['in the Shopify admin under', {subdued: 'Settings > Users > Security'}] + constructor(organization: OrganizationReference) { + const name = organization.businessName ?? `organization ${organization.id}` + const where = 'in the Shopify admin under Settings > Users > Security' if (isServiceAccountTokenSet()) { - super(`This service account can't manage dev stores in ${organization.businessName}.`, [ - 'Give it', - {bold: 'Dev store management'}, - 'access', - ...where, - {char: ','}, - 'or use a token from a service account that has it.', + super(`This service account can't manage dev stores in ${name}.`, null, [ + `Give the service account Dev store management access ${where}.`, + 'Or use a token from a service account that has that access.', ]) } else if (getAppAutomationToken()) { - super(`This automation token can't manage dev stores in ${organization.businessName}.`, [ - 'Store commands need a service account token with', - {bold: 'Dev store management'}, - 'access. Create one', - ...where, - 'and set it in', - {command: 'SHOPIFY_SERVICE_ACCOUNT_TOKEN'}, - {char: '.'}, + super(`This automation token can't manage dev stores in ${name}.`, null, [ + `Create a service account with Dev store management access ${where}.`, + 'Set its token in the SHOPIFY_SERVICE_ACCOUNT_TOKEN environment variable.', ]) } else { - super( - `You don't have permission to manage dev stores in ${organization.businessName}.`, - "Check that you're logged in to the right account, or ask an organization admin for access.", - ) + super(`You don't have permission to manage dev stores in ${name}.`, null, [ + "Check that you're logged in to the right account.", + 'Or ask an organization admin for access.', + ]) } } } /** - * Runs an Organizations API request for the organization, turning a refused request into an - * `OrganizationStoreAccessError` so the user learns which access is missing rather than seeing the HTTP status. + * Runs a Business Platform Organizations API request against an organization's stores. + * + * Every Organizations API request the store commands make goes through here, so a token that can't act in the + * organization gets the same `OrganizationStoreAccessError` from every command, instead of each call site having to + * remember to translate the HTTP status. * * @param organization - The organization the request addresses. - * @param request - The request to run. - * @returns The request's result. + * @param options - The request, without the organization ID, which comes from `organization`. + * @returns The response of the query. */ -export async function withOrganizationStoreAccess( - organization: Organization, - request: () => Promise, -): Promise { +export async function organizationStoresRequestDoc( + organization: OrganizationReference, + options: Omit, 'organizationId'>, +): Promise { try { - return await request() + return await businessPlatformOrganizationsRequestDoc({...options, organizationId: organization.id}) } catch (error) { if (isOrganizationAccessError(error)) throw new OrganizationStoreAccessError(organization) throw error diff --git a/packages/organizations/src/cli/services/dev/create-dev-store.ts b/packages/organizations/src/cli/services/dev/create-dev-store.ts index 8bbd14d3125..123b6fe1e44 100644 --- a/packages/organizations/src/cli/services/dev/create-dev-store.ts +++ b/packages/organizations/src/cli/services/dev/create-dev-store.ts @@ -6,8 +6,7 @@ import { PollStoreCreationQuery, } from '../../api/graphql/business-platform-organizations/generated/poll_store_creation.js' import {Organization} from '../../models/organization.js' -import {businessPlatformTokenRefreshHandler, withOrganizationStoreAccess} from '../business-platform.js' -import {businessPlatformOrganizationsRequestDoc} from '@shopify/cli-kit/node/api/business-platform' +import {businessPlatformTokenRefreshHandler, organizationStoresRequestDoc} from '../business-platform.js' import {AbortError} from '@shopify/cli-kit/node/error' import {outputContent, outputResult} from '@shopify/cli-kit/node/output' import {sleep} from '@shopify/cli-kit/node/system' @@ -60,21 +59,18 @@ export async function createDevStore(options: CreateDevStoreOptions): Promise - businessPlatformOrganizationsRequestDoc({ - query: CreateAppDevelopmentStore, - token, - organizationId: org.id, - variables: { - shopName: name, - priceLookupKey: DEV_STORE_PLANS[plan], - prepopulateTestData: options.withDemoData ?? false, - developerPreviewHandle: options.featurePreview, - country: options.country, - }, - unauthorizedHandler, - }), - ) + const mutationResult = await organizationStoresRequestDoc(org, { + query: CreateAppDevelopmentStore, + token, + variables: { + shopName: name, + priceLookupKey: DEV_STORE_PLANS[plan], + prepopulateTestData: options.withDemoData ?? false, + developerPreviewHandle: options.featurePreview, + country: options.country, + }, + unauthorizedHandler, + }) const createAppDevelopmentStore = mutationResult.createAppDevelopmentStore if (!createAppDevelopmentStore) { @@ -101,10 +97,9 @@ export async function createDevStore(options: CreateDevStoreOptions): Promise { mockExit.mockRestore() }) + test('includes how to grant a service account dev store access in the --json error', async () => { + vi.stubEnv('SHOPIFY_SERVICE_ACCOUNT_TOKEN', 'service-account-token') + vi.mocked(createDevStore).mockRejectedValueOnce(new OrganizationStoreAccessError(defaultOrg)) + const mockExit = vi.spyOn(process, 'exit').mockImplementation((() => { + throw new Error('process.exit') + }) as never) + + await expect( + StoreCreateDev.run(['--name', 'my-test-store', '--plan', 'plus', '--organization-id', '12345', '--json']), + ).rejects.toThrow('process.exit') + + const parsed = JSON.parse(vi.mocked(outputResult).mock.calls[0]![0] as string) + expect(parsed).toEqual({ + error: true, + message: "This service account can't manage dev stores in Test Org.", + nextSteps: [ + 'Give the service account Dev store management access in the Shopify admin under Settings > Users > Security.', + 'Or use a token from a service account that has that access.', + ], + exitCode: 1, + }) + + mockExit.mockRestore() + vi.unstubAllEnvs() + }) + test('outputs one standard bug error for unexpected failures when --json is active', async () => { vi.mocked(createDevStore).mockRejectedValueOnce(new Error('unexpected')) diff --git a/packages/store/src/cli/commands/store/delete.test.ts b/packages/store/src/cli/commands/store/delete.test.ts index 9de6d6a7ccb..1c08bf0ffba 100644 --- a/packages/store/src/cli/commands/store/delete.test.ts +++ b/packages/store/src/cli/commands/store/delete.test.ts @@ -2,6 +2,7 @@ import StoreDelete from './delete.js' import {deleteDevStoreJsonOutputSchema} from '../../services/store/delete/types.js' import {deleteDevStore} from '../../services/store/delete/dev.js' import {resolveOrganizationForStore} from '../../utilities/store-lookup/organization.js' +import {OrganizationStoreAccessError} from '@shopify/organizations' import {AbortError} from '@shopify/cli-kit/node/error' import {outputResult} from '@shopify/cli-kit/node/output' import {isTTY, renderDangerousConfirmationPrompt} from '@shopify/cli-kit/node/ui' @@ -172,6 +173,33 @@ describe('store delete command', () => { mockExit.mockRestore() }) + test('includes how to grant a service account dev store access in the --json error', async () => { + vi.stubEnv('SHOPIFY_SERVICE_ACCOUNT_TOKEN', 'service-account-token') + vi.mocked(deleteDevStore).mockRejectedValueOnce(new OrganizationStoreAccessError(defaultOrg)) + const mockExit = vi.spyOn(process, 'exit').mockImplementation((() => { + throw new Error('process.exit') + }) as never) + + await expect( + StoreDelete.run(['--store', 'my-store.myshopify.com', '--organization-id', '12345', '--json', '--force']), + ).rejects.toThrow('process.exit') + + const parsed = JSON.parse(vi.mocked(outputResult).mock.calls[0]![0] as string) + expect(deleteDevStoreJsonOutputSchema.validate(parsed)).toEqual(parsed) + expect(parsed).toEqual({ + error: true, + message: "This service account can't manage dev stores in Test Org.", + nextSteps: [ + 'Give the service account Dev store management access in the Shopify admin under Settings > Users > Security.', + 'Or use a token from a service account that has that access.', + ], + exitCode: 1, + }) + + mockExit.mockRestore() + vi.unstubAllEnvs() + }) + test('outputs structured JSON error when --json is active and organization resolution throws AbortError', async () => { vi.mocked(resolveOrganizationForStore).mockRejectedValueOnce(new AbortError('Could not resolve organization')) const mockExit = vi.spyOn(process, 'exit').mockImplementation((() => { diff --git a/packages/store/src/cli/services/store/business-platform.ts b/packages/store/src/cli/services/store/business-platform.ts index 9480934d78c..b6149df99de 100644 --- a/packages/store/src/cli/services/store/business-platform.ts +++ b/packages/store/src/cli/services/store/business-platform.ts @@ -1 +1 @@ -export {businessPlatformTokenRefreshHandler, withOrganizationStoreAccess} from '@shopify/organizations' +export {businessPlatformTokenRefreshHandler, organizationStoresRequestDoc} from '@shopify/organizations' diff --git a/packages/store/src/cli/services/store/delete/dev.ts b/packages/store/src/cli/services/store/delete/dev.ts index ff705fc2eb1..c07a74252d8 100644 --- a/packages/store/src/cli/services/store/delete/dev.ts +++ b/packages/store/src/cli/services/store/delete/dev.ts @@ -1,5 +1,5 @@ import {deleteDevStoreJsonOutputSchema} from './types.js' -import {businessPlatformTokenRefreshHandler, withOrganizationStoreAccess} from '../business-platform.js' +import {businessPlatformTokenRefreshHandler, organizationStoresRequestDoc} from '../business-platform.js' import {fetchOptionalOrganizationShop} from '../../../utilities/store-lookup/organization-shop.js' import {DeleteAppDevelopmentStore} from '../../../api/graphql/business-platform-organizations/generated/delete_app_development_store.js' import { @@ -8,7 +8,6 @@ import { type OrganizationAccessibleShopQueryVariables, } from '../../../api/graphql/business-platform-organizations/generated/organization_accessible_shop.js' import {type Organization} from '@shopify/organizations' -import {businessPlatformOrganizationsRequestDoc} from '@shopify/cli-kit/node/api/business-platform' import {type UnauthorizedHandler} from '@shopify/cli-kit/node/api/graphql' import {ensureAuthenticatedBusinessPlatform} from '@shopify/cli-kit/node/session' import {renderSingleTask, renderSuccess, renderWarning} from '@shopify/cli-kit/node/ui' @@ -28,7 +27,7 @@ interface DeleteDevStoreOptions { interface StoreDeletionConfirmationOptions { shopifyShopId: string - organizationId: string + organization: Organization token: string unauthorizedHandler: UnauthorizedHandler } @@ -37,19 +36,14 @@ export async function deleteDevStore(options: DeleteDevStoreOptions): Promise - fetchShopifyShopId({store, organizationId: org.id, token}), - ) - - const mutationResult = await withOrganizationStoreAccess(org, () => - businessPlatformOrganizationsRequestDoc({ - query: DeleteAppDevelopmentStore, - token, - organizationId: org.id, - variables: {storeFqdn: store}, - unauthorizedHandler, - }), - ) + let shopifyShopId = await fetchShopifyShopId({store, organization: org, token}) + + const mutationResult = await organizationStoresRequestDoc(org, { + query: DeleteAppDevelopmentStore, + token, + variables: {storeFqdn: store}, + unauthorizedHandler, + }) const deleteAppDevelopmentStore = mutationResult.deleteAppDevelopmentStore if (!deleteAppDevelopmentStore) { @@ -65,12 +59,12 @@ export async function deleteDevStore(options: DeleteDevStoreOptions): Promise { - const shop = await fetchOptionalOrganizationShop(options) + const {store, organization, token} = options + const shop = await fetchOptionalOrganizationShop({ + store, + organizationId: organization.id, + organizationName: organization.businessName, + token, + }) return shop?.shopifyShopId } @@ -127,13 +127,12 @@ async function waitForStoreDeletionConfirmation(options: StoreDeletionConfirmati } // eslint-disable-next-line no-await-in-loop - const pollResult = await businessPlatformOrganizationsRequestDoc< + const pollResult = await organizationStoresRequestDoc< OrganizationAccessibleShopQuery, OrganizationAccessibleShopQueryVariables - >({ + >(options.organization, { query: OrganizationAccessibleShop, token: options.token, - organizationId: options.organizationId, variables: {id: organizationsShopifyShopId}, unauthorizedHandler: options.unauthorizedHandler, }) diff --git a/packages/store/src/cli/services/store/info/index.test.ts b/packages/store/src/cli/services/store/info/index.test.ts index 7680fa414d1..e47659e11ca 100644 --- a/packages/store/src/cli/services/store/info/index.test.ts +++ b/packages/store/src/cli/services/store/info/index.test.ts @@ -134,7 +134,12 @@ describe('getStoreInfo', () => { expect(getCurrentStoredStoreAppSession).toHaveBeenCalledWith(SHOP) expect(fetchDestinationsContext).toHaveBeenCalledWith({store: SHOP, noPrompt: false}) - expect(fetchOrganizationShop).toHaveBeenCalledWith({store: SHOP, organizationId: '149572536', noPrompt: false}) + expect(fetchOrganizationShop).toHaveBeenCalledWith({ + store: SHOP, + organizationId: '149572536', + organizationName: 'Acme Holdings', + noPrompt: false, + }) expect(loadStoredStoreSession).not.toHaveBeenCalled() expect(graphqlRequest).not.toHaveBeenCalled() expect(getPreviewStore).not.toHaveBeenCalled() @@ -337,7 +342,12 @@ describe('getStoreInfo', () => { const result = await getStoreInfo({store: SHOP}) expect(fetchDestinationsContext).toHaveBeenCalledWith({store: SHOP, noPrompt: true}) - expect(fetchOrganizationShop).toHaveBeenCalledWith({store: SHOP, organizationId: '149572536', noPrompt: true}) + expect(fetchOrganizationShop).toHaveBeenCalledWith({ + store: SHOP, + organizationId: '149572536', + organizationName: 'Acme Holdings', + noPrompt: true, + }) expect(loadStoredStoreSession).not.toHaveBeenCalled() expect(graphqlRequest).not.toHaveBeenCalled() expect(result).toEqual({ diff --git a/packages/store/src/cli/services/store/info/index.ts b/packages/store/src/cli/services/store/info/index.ts index 5a0f8a4a91a..90d15a2ef67 100644 --- a/packages/store/src/cli/services/store/info/index.ts +++ b/packages/store/src/cli/services/store/info/index.ts @@ -186,7 +186,12 @@ async function safeFetchOrganizationShop( return undefined } try { - return await fetchOrganizationShop({store, organizationId: ctx.owningOrg.id, noPrompt: options.noPrompt}) + return await fetchOrganizationShop({ + store, + organizationId: ctx.owningOrg.id, + organizationName: ctx.owningOrg.name, + noPrompt: options.noPrompt, + }) // eslint-disable-next-line no-catch-all/no-catch-all } catch (error) { outputDebug(`BP Organizations shop lookup failed: ${error instanceof Error ? error.message : String(error)}`) diff --git a/packages/store/src/cli/services/store/list/bp-source.ts b/packages/store/src/cli/services/store/list/bp-source.ts index 42dfd518d78..66d01d8e3ca 100644 --- a/packages/store/src/cli/services/store/list/bp-source.ts +++ b/packages/store/src/cli/services/store/list/bp-source.ts @@ -1,6 +1,6 @@ import {STORE_LIST_LIMIT} from './constants.js' import {type StoreListEntry} from './types.js' -import {businessPlatformTokenRefreshHandler, withOrganizationStoreAccess} from '../business-platform.js' +import {businessPlatformTokenRefreshHandler, organizationStoresRequestDoc} from '../business-platform.js' import {planHandle} from '../plan.js' import {storeTypeFilterValue, storeTypeHandle, type StoreTypeFilter} from '../store-type.js' import { @@ -8,7 +8,6 @@ import { type ListAccessibleShopsQuery, } from '../../../api/graphql/business-platform-organizations/generated/list_accessible_shops.js' import {type ShopFilterInput} from '../../../api/graphql/business-platform-organizations/generated/types.js' -import {businessPlatformOrganizationsRequestDoc} from '@shopify/cli-kit/node/api/business-platform' import {extractHost} from '@shopify/cli-kit/common/url' import {type Organization} from '@shopify/organizations' @@ -44,15 +43,12 @@ async function fetchOrganizationStores( ): Promise<{entries: StoreListEntry[]; hasMore: boolean}> { const unauthorizedHandler = businessPlatformTokenRefreshHandler() - const result = await withOrganizationStoreAccess(organization, () => - businessPlatformOrganizationsRequestDoc({ - query: ListAccessibleShops, - token, - organizationId: organization.id, - variables: {first: STORE_LIST_LIMIT, filters: shopFilters(storeType)}, - unauthorizedHandler, - }), - ) + const result = await organizationStoresRequestDoc(organization, { + query: ListAccessibleShops, + token, + variables: {first: STORE_LIST_LIMIT, filters: shopFilters(storeType)}, + unauthorizedHandler, + }) const accessibleShops = result.organization?.accessibleShops if (!accessibleShops) return {entries: [], hasMore: false} diff --git a/packages/store/src/cli/utilities/store-lookup/organization-shop.ts b/packages/store/src/cli/utilities/store-lookup/organization-shop.ts index 6bbc281feac..add9cfb2e7d 100644 --- a/packages/store/src/cli/utilities/store-lookup/organization-shop.ts +++ b/packages/store/src/cli/utilities/store-lookup/organization-shop.ts @@ -1,7 +1,9 @@ -import {businessPlatformTokenRefreshHandler} from '../../services/store/business-platform.js' +import { + businessPlatformTokenRefreshHandler, + organizationStoresRequestDoc, +} from '../../services/store/business-platform.js' import {StoreInfoShop} from '../../api/graphql/business-platform-organizations/generated/store-info-shop.js' import {AbortError} from '@shopify/cli-kit/node/error' -import {businessPlatformOrganizationsRequestDoc} from '@shopify/cli-kit/node/api/business-platform' import {ensureAuthenticatedBusinessPlatform} from '@shopify/cli-kit/node/session' import {extractHost} from '@shopify/cli-kit/common/url' import type { @@ -13,6 +15,8 @@ import type {OrganizationShopFields} from './types.js' interface FetchOrganizationShopOptions { store: string organizationId: string + // Names the organization if the token turns out to have no access to it. + organizationName?: string token?: string noPrompt?: boolean } @@ -38,10 +42,10 @@ export async function fetchOptionalOrganizationShop( (await ensureAuthenticatedBusinessPlatform([], {noPrompt: options.noPrompt, allowAutomationToken: true})) const unauthorizedHandler = businessPlatformTokenRefreshHandler({noPrompt: options.noPrompt}) - const response = await businessPlatformOrganizationsRequestDoc({ + const organization = {id: options.organizationId, businessName: options.organizationName} + const response = await organizationStoresRequestDoc(organization, { query: StoreInfoShop, token, - organizationId: options.organizationId, variables: {search: options.store}, unauthorizedHandler, })