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/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/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..b5f3e5795e5 --- /dev/null +++ b/packages/organizations/src/cli/services/business-platform.test.ts @@ -0,0 +1,125 @@ +import { + isOrganizationAccessError, + OrganizationStoreAccessError, + 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'} + +// 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.") + // 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', () => { + 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(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('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)) + + 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 notFound = handledClientError(404) + vi.mocked(businessPlatformOrganizationsRequestDoc).mockRejectedValue(notFound) + + 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 d52cf98ef21..763cc1c1a42 100644 --- a/packages/organizations/src/cli/services/business-platform.ts +++ b/packages/organizations/src/cli/services/business-platform.ts @@ -1,5 +1,11 @@ -import {type UnauthorizedHandler} from '@shopify/cli-kit/node/api/graphql' -import {ensureAuthenticatedBusinessPlatform} from '@shopify/cli-kit/node/session' +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' interface BusinessPlatformTokenRefreshHandlerOptions { noPrompt?: boolean @@ -15,3 +21,82 @@ 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. + * + * 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) 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: 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 ${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 ${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 ${name}.`, null, [ + "Check that you're logged in to the right account.", + 'Or ask an organization admin for access.', + ]) + } + } +} + +/** + * 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 options - The request, without the organization ID, which comes from `organization`. + * @returns The response of the query. + */ +export async function organizationStoresRequestDoc( + organization: OrganizationReference, + options: Omit, 'organizationId'>, +): Promise { + try { + 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.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..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} 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,10 +59,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 01385af7b63..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} from '@shopify/organizations' +export {businessPlatformTokenRefreshHandler, organizationStoresRequestDoc} 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..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} 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,12 +36,11 @@ 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 } @@ -123,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.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..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} 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,10 +43,9 @@ async function fetchOrganizationStores( ): Promise<{entries: StoreListEntry[]; hasMore: boolean}> { const unauthorizedHandler = businessPlatformTokenRefreshHandler() - const result = await businessPlatformOrganizationsRequestDoc({ + const result = await organizationStoresRequestDoc(organization, { query: ListAccessibleShops, token, - organizationId: organization.id, variables: {first: STORE_LIST_LIMIT, filters: shopFilters(storeType)}, unauthorizedHandler, }) 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, })