From 3bc66ee18c71b8ee4040ac901b29823917dd9b60 Mon Sep 17 00:00:00 2001 From: Lucy Xiang Date: Tue, 22 Oct 2024 15:21:59 -0400 Subject: [PATCH 1/2] Add GQL support for theme access tokens --- packages/cli-kit/src/private/node/api.ts | 14 ++++++- .../cli-kit/src/private/node/api/graphql.ts | 10 ++--- .../src/private/node/api/headers.test.ts | 39 ++++++++++--------- .../cli-kit/src/private/node/api/headers.ts | 2 +- .../cli-kit/src/public/node/api/admin.test.ts | 29 ++++++++++++++ packages/cli-kit/src/public/node/api/admin.ts | 34 +++++++++++++--- .../src/public/node/api/graphql.test.ts | 1 + .../cli-kit/src/public/node/api/graphql.ts | 2 +- 8 files changed, 99 insertions(+), 32 deletions(-) diff --git a/packages/cli-kit/src/private/node/api.ts b/packages/cli-kit/src/private/node/api.ts index d4ab5d91cd..50286565a9 100644 --- a/packages/cli-kit/src/private/node/api.ts +++ b/packages/cli-kit/src/private/node/api.ts @@ -70,7 +70,7 @@ async function makeVerboseRequest( } const sanitizedHeaders = sanitizedHeadersOutput(responseHeaders) - if (err.response.errors?.some((error) => error.extensions.code === '429') || err.response.status === 429) { + if (errorsIncludeStatus429(err)) { let delayMs: number | undefined try { @@ -120,6 +120,18 @@ async function makeVerboseRequest( } } +function errorsIncludeStatus429(error: ClientError): boolean { + if (error.response.status === 429) { + return true + } + + // More so checking if type of error.response.errors is not GraphQLError[] + if (typeof error.response.errors === 'string') { + return false + } + return error.response.errors?.some((error) => error.extensions?.code === '429') ?? false +} + export async function simpleRequestWithDebugLog( {request, url}: RequestOptions, errorHandler?: (error: unknown, requestId: string | undefined) => unknown, diff --git a/packages/cli-kit/src/private/node/api/graphql.ts b/packages/cli-kit/src/private/node/api/graphql.ts index c9f77fddba..d61c82c7ec 100644 --- a/packages/cli-kit/src/private/node/api/graphql.ts +++ b/packages/cli-kit/src/private/node/api/graphql.ts @@ -1,12 +1,12 @@ -/* eslint-disable @typescript-eslint/no-base-to-string */ import {GraphQLClientError, sanitizedHeadersOutput} from './headers.js' import {stringifyMessage, outputContent, outputToken, outputDebug} from '../../../public/node/output.js' import {AbortError} from '../../../public/node/error.js' -import {ClientError, RequestDocument, Variables} from 'graphql-request' +import {ClientError, Variables} from 'graphql-request' export function debugLogRequestInfo( api: string, - query: RequestDocument, + query: string, + url: string, variables?: Variables, headers: {[key: string]: string} = {}, ) { @@ -14,8 +14,8 @@ export function debugLogRequestInfo( ${outputToken.raw(query.toString().trim())} ${variables ? `\nWith variables:\n${sanitizeVariables(variables)}\n` : ''} With request headers: -${sanitizedHeadersOutput(headers)} -`) +${sanitizedHeadersOutput(headers)}\n +to ${url}`) } function sanitizeVariables(variables: Variables): string { diff --git a/packages/cli-kit/src/private/node/api/headers.test.ts b/packages/cli-kit/src/private/node/api/headers.test.ts index 8bf60ab3c5..b67b5e10ed 100644 --- a/packages/cli-kit/src/private/node/api/headers.test.ts +++ b/packages/cli-kit/src/private/node/api/headers.test.ts @@ -52,25 +52,28 @@ describe('common API methods', () => { }) }) - test.each(['shpat', 'shpua', 'shpca'])(`when custom app token starts with %s, do not prepend 'Bearer'`, (prefix) => { - // Given - vi.mocked(randomUUID).mockReturnValue('random-uuid') - vi.mocked(firstPartyDev).mockReturnValue(false) - const token = `${prefix}_my_token` - // When - const headers = buildHeaders(token) + test.each(['shpat', 'shpua', 'shpca', 'shptka'])( + `when custom app token starts with %s, do not prepend 'Bearer'`, + (prefix) => { + // Given + vi.mocked(randomUUID).mockReturnValue('random-uuid') + vi.mocked(firstPartyDev).mockReturnValue(false) + const token = `${prefix}_my_token` + // When + const headers = buildHeaders(token) - // Then - const version = CLI_KIT_VERSION - expect(headers).toEqual({ - 'Content-Type': 'application/json', - 'Keep-Alive': 'timeout=30', - 'X-Shopify-Access-Token': token, - 'User-Agent': `Shopify CLI; v=${version}`, - authorization: token, - 'Sec-CH-UA-PLATFORM': process.platform, - }) - }) + // Then + const version = CLI_KIT_VERSION + expect(headers).toEqual({ + 'Content-Type': 'application/json', + 'Keep-Alive': 'timeout=30', + 'X-Shopify-Access-Token': token, + 'User-Agent': `Shopify CLI; v=${version}`, + authorization: token, + 'Sec-CH-UA-PLATFORM': process.platform, + }) + }, + ) test('sanitizedHeadersOutput removes the headers that include the token', () => { // Given diff --git a/packages/cli-kit/src/private/node/api/headers.ts b/packages/cli-kit/src/private/node/api/headers.ts index 17781fdefb..7d5ac55ee9 100644 --- a/packages/cli-kit/src/private/node/api/headers.ts +++ b/packages/cli-kit/src/private/node/api/headers.ts @@ -56,7 +56,7 @@ export function buildHeaders(token?: string): {[key: string]: string} { ...(firstPartyDev() && {'X-Shopify-Cli-Employee': '1'}), } if (token) { - const authString = token.match(/^shp(at|ua|ca)/) ? token : `Bearer ${token}` + const authString = token.match(/^shp(at|ua|ca|tka)/) ? token : `Bearer ${token}` headers.authorization = authString headers['X-Shopify-Access-Token'] = authString diff --git a/packages/cli-kit/src/public/node/api/admin.test.ts b/packages/cli-kit/src/public/node/api/admin.test.ts index f3e6ab2618..f1d72e44ff 100644 --- a/packages/cli-kit/src/public/node/api/admin.test.ts +++ b/packages/cli-kit/src/public/node/api/admin.test.ts @@ -57,10 +57,39 @@ describe('admin-graphql-api', () => { query: 'query', api: 'Admin', url: 'https://store.myshopify.com/admin/api/2022-01/graphql.json', + addedHeaders: {}, token, variables: {variables: 'variables'}, }) }) + + test('request is called with correct parameters when it is a theme access session', async () => { + // Given + const themeAccessToken = 'shptka_token' + const themeAccessSession = { + ...Session, + token: themeAccessToken, + } + + vi.mocked(graphqlRequest).mockResolvedValue(mockedResult) + vi.mocked(graphqlRequestDoc).mockResolvedValue(mockedResult) + + // When + await admin.adminRequest('query', themeAccessSession, {variables: 'variables'}) + + // Then + expect(graphqlRequest).toHaveBeenLastCalledWith({ + query: 'query', + api: 'Admin', + addedHeaders: { + 'X-Shopify-Access-Token': 'shptka_token', + 'X-Shopify-Shop': 'store', + }, + url: `https://${defaultThemeKitAccessDomain}/cli/admin/api/2022-01/graphql.json`, + token: themeAccessToken, + variables: {variables: 'variables'}, + }) + }) }) describe('admin-rest-api', () => { diff --git a/packages/cli-kit/src/public/node/api/admin.ts b/packages/cli-kit/src/public/node/api/admin.ts index 8244d4724c..a08eb43ce3 100644 --- a/packages/cli-kit/src/public/node/api/admin.ts +++ b/packages/cli-kit/src/public/node/api/admin.ts @@ -2,10 +2,16 @@ import {graphqlRequest, graphqlRequestDoc, GraphQLResponseOptions, GraphQLVariab import {AdminSession} from '../session.js' import {outputContent, outputToken} from '../../../public/node/output.js' import {BugError, AbortError} from '../error.js' -import {restRequestBody, restRequestHeaders, restRequestUrl} from '../../../private/node/api/rest.js' +import { + restRequestBody, + restRequestHeaders, + restRequestUrl, + isThemeAccessSession, +} from '../../../private/node/api/rest.js' import {fetch} from '../http.js' import {PublicApiVersions} from '../../../cli/api/graphql/admin/generated/public_api_versions.js' import {normalizeStoreFqdn} from '../context/fqdn.js' +import {defaultThemeKitAccessDomain, environmentVariables} from '../../../private/node/constants.js' import {ClientError, Variables} from 'graphql-request' import {TypedDocumentNode} from '@graphql-typed-document-node/core' @@ -21,8 +27,9 @@ export async function adminRequest(query: string, session: AdminSession, vari const api = 'Admin' const version = await fetchLatestSupportedApiVersion(session) const store = await normalizeStoreFqdn(session.storeFqdn) - const url = adminUrl(store, version) - return graphqlRequest({query, api, url, token: session.token, variables}) + const url = adminUrl(store, version, session) + const addedHeaders = headers(session) + return graphqlRequest({query, api, addedHeaders, url, token: session.token, variables}) } /** @@ -47,15 +54,23 @@ export async function adminRequestDoc( apiVersion = await fetchLatestSupportedApiVersion(session) } const store = await normalizeStoreFqdn(session.storeFqdn) + const addedHeaders = headers(session) const opts = { - url: adminUrl(store, apiVersion), + url: adminUrl(store, apiVersion, session), api: 'Admin', token: session.token, + addedHeaders, } const result = graphqlRequestDoc({...opts, query, variables, responseOptions}) return result } +function headers(session: AdminSession): {[header: string]: string} { + return isThemeAccessSession(session) + ? {'X-Shopify-Shop': session.storeFqdn, 'X-Shopify-Access-Token': session.token} + : {} +} + /** * GraphQL query to retrieve the latest supported API version. * @@ -121,11 +136,18 @@ async function fetchApiVersions(session: AdminSession): Promise { * * @param store - Store FQDN. * @param version - API version. + * @param session - User session. * @returns - Admin API URL. */ -export function adminUrl(store: string, version: string | undefined): string { +export function adminUrl(store: string, version: string | undefined, session?: AdminSession): string { const realVersion = version ?? 'unstable' - return `https://${store}/admin/api/${realVersion}/graphql.json` + const themeKitAccessDomain = process.env[environmentVariables.themeKitAccessDomain] ?? defaultThemeKitAccessDomain + + const url = + session && isThemeAccessSession(session) + ? `https://${themeKitAccessDomain}/cli/admin/api/${realVersion}/graphql.json` + : `https://${store}/admin/api/${realVersion}/graphql.json` + return url } interface ApiVersion { 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 4ef7b35671..4e5e4d2f17 100644 --- a/packages/cli-kit/src/public/node/api/graphql.test.ts +++ b/packages/cli-kit/src/public/node/api/graphql.test.ts @@ -101,6 +101,7 @@ describe('graphqlRequestDoc', () => { `query QueryName { example }`, + 'mockedAddress', mockVariables, expect.anything(), ) diff --git a/packages/cli-kit/src/public/node/api/graphql.ts b/packages/cli-kit/src/public/node/api/graphql.ts index 6b7998e69d..bb6f788d8d 100644 --- a/packages/cli-kit/src/public/node/api/graphql.ts +++ b/packages/cli-kit/src/public/node/api/graphql.ts @@ -55,7 +55,7 @@ async function performGraphQLRequest(options: PerformGraphQLRequestOpti ...buildHeaders(token), } - debugLogRequestInfo(api, queryAsString, variables, headers) + debugLogRequestInfo(api, queryAsString, url, variables, headers) const clientOptions = {agent: await httpsAgent(), headers} const client = new GraphQLClient(url, clientOptions) From f747f67f482d1565800716f63b5ffbe34848150e Mon Sep 17 00:00:00 2001 From: Lucy Xiang Date: Thu, 24 Oct 2024 11:19:29 -0400 Subject: [PATCH 2/2] Address comments --- packages/cli-kit/src/private/node/api.ts | 3 ++- packages/cli-kit/src/private/node/api/graphql.ts | 3 ++- packages/cli-kit/src/private/node/api/rest.ts | 4 +--- packages/cli-kit/src/private/node/constants.ts | 3 +++ packages/cli-kit/src/public/node/api/admin.ts | 9 ++++----- 5 files changed, 12 insertions(+), 10 deletions(-) diff --git a/packages/cli-kit/src/private/node/api.ts b/packages/cli-kit/src/private/node/api.ts index 50286565a9..2327fcafb7 100644 --- a/packages/cli-kit/src/private/node/api.ts +++ b/packages/cli-kit/src/private/node/api.ts @@ -125,7 +125,8 @@ function errorsIncludeStatus429(error: ClientError): boolean { return true } - // More so checking if type of error.response.errors is not GraphQLError[] + // GraphQL returns a 401 with a string error message when auth fails + // Therefore error.response.errros can be a string or GraphQLError[] if (typeof error.response.errors === 'string') { return false } diff --git a/packages/cli-kit/src/private/node/api/graphql.ts b/packages/cli-kit/src/private/node/api/graphql.ts index d61c82c7ec..7975e00baa 100644 --- a/packages/cli-kit/src/private/node/api/graphql.ts +++ b/packages/cli-kit/src/private/node/api/graphql.ts @@ -1,4 +1,5 @@ import {GraphQLClientError, sanitizedHeadersOutput} from './headers.js' +import {sanitizeURL} from './urls.js' import {stringifyMessage, outputContent, outputToken, outputDebug} from '../../../public/node/output.js' import {AbortError} from '../../../public/node/error.js' import {ClientError, Variables} from 'graphql-request' @@ -15,7 +16,7 @@ export function debugLogRequestInfo( ${variables ? `\nWith variables:\n${sanitizeVariables(variables)}\n` : ''} With request headers: ${sanitizedHeadersOutput(headers)}\n -to ${url}`) +to ${sanitizeURL(url)}`) } function sanitizeVariables(variables: Variables): string { diff --git a/packages/cli-kit/src/private/node/api/rest.ts b/packages/cli-kit/src/private/node/api/rest.ts index a2312dae7b..cdc3e2b87f 100644 --- a/packages/cli-kit/src/private/node/api/rest.ts +++ b/packages/cli-kit/src/private/node/api/rest.ts @@ -1,5 +1,5 @@ import {buildHeaders} from './headers.js' -import {defaultThemeKitAccessDomain, environmentVariables} from '../constants.js' +import {themeKitAccessDomain} from '../constants.js' import {AdminSession} from '@shopify/cli-kit/node/session' export function restRequestBody(requestBody?: T) { @@ -14,9 +14,7 @@ export function restRequestUrl( apiVersion: string, path: string, searchParams: {[name: string]: string} = {}, - env = process.env, ) { - const themeKitAccessDomain = env[environmentVariables.themeKitAccessDomain] || defaultThemeKitAccessDomain const url = new URL( isThemeAccessSession(session) ? `https://${themeKitAccessDomain}/cli/admin/api/${apiVersion}${path}.json` diff --git a/packages/cli-kit/src/private/node/constants.ts b/packages/cli-kit/src/private/node/constants.ts index c917cc948b..6337664367 100644 --- a/packages/cli-kit/src/private/node/constants.ts +++ b/packages/cli-kit/src/private/node/constants.ts @@ -80,3 +80,6 @@ export const sessionConstants = { export const bugsnagApiKey = '9e1e6889176fd0c795d5c659225e0fae' export const reportingRateLimit = {limit: 300, timeout: {days: 1}} + +export const themeKitAccessDomain = + process.env[environmentVariables.themeKitAccessDomain] ?? defaultThemeKitAccessDomain diff --git a/packages/cli-kit/src/public/node/api/admin.ts b/packages/cli-kit/src/public/node/api/admin.ts index a08eb43ce3..1312fe5e6d 100644 --- a/packages/cli-kit/src/public/node/api/admin.ts +++ b/packages/cli-kit/src/public/node/api/admin.ts @@ -11,7 +11,7 @@ import { import {fetch} from '../http.js' import {PublicApiVersions} from '../../../cli/api/graphql/admin/generated/public_api_versions.js' import {normalizeStoreFqdn} from '../context/fqdn.js' -import {defaultThemeKitAccessDomain, environmentVariables} from '../../../private/node/constants.js' +import {themeKitAccessDomain} from '../../../private/node/constants.js' import {ClientError, Variables} from 'graphql-request' import {TypedDocumentNode} from '@graphql-typed-document-node/core' @@ -28,7 +28,7 @@ export async function adminRequest(query: string, session: AdminSession, vari const version = await fetchLatestSupportedApiVersion(session) const store = await normalizeStoreFqdn(session.storeFqdn) const url = adminUrl(store, version, session) - const addedHeaders = headers(session) + const addedHeaders = themeAccessHeaders(session) return graphqlRequest({query, api, addedHeaders, url, token: session.token, variables}) } @@ -54,7 +54,7 @@ export async function adminRequestDoc( apiVersion = await fetchLatestSupportedApiVersion(session) } const store = await normalizeStoreFqdn(session.storeFqdn) - const addedHeaders = headers(session) + const addedHeaders = themeAccessHeaders(session) const opts = { url: adminUrl(store, apiVersion, session), api: 'Admin', @@ -65,7 +65,7 @@ export async function adminRequestDoc( return result } -function headers(session: AdminSession): {[header: string]: string} { +function themeAccessHeaders(session: AdminSession): {[header: string]: string} { return isThemeAccessSession(session) ? {'X-Shopify-Shop': session.storeFqdn, 'X-Shopify-Access-Token': session.token} : {} @@ -141,7 +141,6 @@ async function fetchApiVersions(session: AdminSession): Promise { */ export function adminUrl(store: string, version: string | undefined, session?: AdminSession): string { const realVersion = version ?? 'unstable' - const themeKitAccessDomain = process.env[environmentVariables.themeKitAccessDomain] ?? defaultThemeKitAccessDomain const url = session && isThemeAccessSession(session)