diff --git a/.github/workflows/shopify-cli.yml b/.github/workflows/shopify-cli.yml index f04f62436ec..4a78d1a25a8 100644 --- a/.github/workflows/shopify-cli.yml +++ b/.github/workflows/shopify-cli.yml @@ -40,7 +40,7 @@ jobs: uses: ruby/setup-ruby@v1 with: ruby-version: ${{ env.RUBY_VERSION }} - - name: Install Bundler for Windows + - name: Install Bundler run: gem install bundler -v ${{ env.BUNDLER_VERSION }} - name: Set Node.js uses: actions/setup-node@master @@ -130,6 +130,8 @@ jobs: uses: ruby/setup-ruby@v1 with: ruby-version: ${{ env.RUBY_VERSION }} + - name: Install Bundler + run: gem install bundler -v ${{ env.BUNDLER_VERSION }} - name: Set Node.js on ubuntu environments uses: actions/setup-node@master with: diff --git a/packages/cli-kit/src/api/admin.test.ts b/packages/cli-kit/src/api/admin.test.ts index bf4bb963130..fa278df7232 100644 --- a/packages/cli-kit/src/api/admin.test.ts +++ b/packages/cli-kit/src/api/admin.test.ts @@ -1,25 +1,27 @@ import * as admin from './admin.js' import {buildHeaders} from './common.js' import {AdminSession} from '../session.js' -import {test, vi, expect, describe} from 'vitest' -import {request as graphqlRequest} from 'graphql-request' - -vi.mock('graphql-request', async () => { - const {gql} = await vi.importActual('graphql-request') - return { - request: vi.fn(), - gql, - } -}) +import {graphqlClient} from '../http/graphql.js' +import {test, vi, expect, describe, beforeEach} from 'vitest' +import {GraphQLClient} from 'graphql-request' +vi.mock('../http/graphql.js') vi.mock('./common.js', async () => { - const common: any = await vi.importActual('./common.js') + const module: any = await vi.importActual('./common.js') return { - ...common, + ...module, buildHeaders: vi.fn(), } }) +let client: GraphQLClient +beforeEach(() => { + client = { + request: vi.fn(), + } as any + vi.mocked(graphqlClient).mockResolvedValue(client) +}) + const mockedResult = { publicApiVersions: [ { @@ -43,37 +45,32 @@ const Session: AdminSession = {token, storeFqdn: 'store'} describe('admin-api', () => { test('calls the graphql client twice: get api version and then execute the request', async () => { // Given - vi.mocked(graphqlRequest).mockResolvedValue(mockedResult) + vi.mocked(client.request).mockResolvedValue(mockedResult) // When await admin.request('query', Session, {}) // Then - expect(graphqlRequest).toHaveBeenCalledTimes(2) + expect(client.request).toHaveBeenCalledTimes(2) }) test('request is called with correct parameters', async () => { // Given // eslint-disable-next-line @typescript-eslint/naming-convention const headers = {'custom-header': token} - vi.mocked(graphqlRequest).mockResolvedValue(mockedResult) + vi.mocked(client.request).mockResolvedValue(mockedResult) vi.mocked(buildHeaders).mockResolvedValue(headers) // When await admin.request('query', Session, {variables: 'variables'}) // Then - expect(graphqlRequest).toHaveBeenLastCalledWith( - 'https://store/admin/api/2022-01/graphql.json', - 'query', - {variables: 'variables'}, - headers, - ) + expect(client.request).toHaveBeenLastCalledWith('query', {variables: 'variables'}) }) test('buildHeaders is called with user token', async () => { // Given - vi.mocked(graphqlRequest).mockResolvedValue(mockedResult) + vi.mocked(client.request).mockResolvedValue(mockedResult) // When await admin.request('query', Session, {}) diff --git a/packages/cli-kit/src/api/admin.ts b/packages/cli-kit/src/api/admin.ts index 5e77ba79da4..dcd4e541035 100644 --- a/packages/cli-kit/src/api/admin.ts +++ b/packages/cli-kit/src/api/admin.ts @@ -2,7 +2,8 @@ import {buildHeaders, debugLogRequest, handlingErrors} from './common.js' import {AdminSession} from '../session.js' import {debug, content, token as outputToken} from '../output.js' import {Bug, Abort} from '../error.js' -import {request as graphqlRequest, gql, RequestDocument, Variables} from 'graphql-request' +import {graphqlClient} from '../http/graphql.js' +import {gql, RequestDocument, Variables} from 'graphql-request' const UnauthorizedAccessError = (store: string) => { const adminLink = outputToken.link(`URL`, `https://${store}/admin`) @@ -25,8 +26,13 @@ export async function request(query: RequestDocument, session: AdminSession, const version = await fetchApiVersion(session) const url = adminUrl(session.storeFqdn, version) const headers = await buildHeaders(session.token) + const client = await graphqlClient({ + headers, + url, + service: 'shopify', + }) debugLogRequest(api, query, variables, headers) - const response = await graphqlRequest(url, query, variables, headers) + const response = await client.request(query, variables) return response }) } @@ -35,16 +41,18 @@ async function fetchApiVersion(session: AdminSession): Promise { const url = adminUrl(session.storeFqdn, 'unstable') const query = apiVersionQuery() const headers = await buildHeaders(session.token) - + const client = await graphqlClient({url, headers, service: 'shopify'}) debug(` Sending Admin GraphQL request to URL ${url} with query: ${query} `) - const data = await graphqlRequest<{ - publicApiVersions: {handle: string; supported: boolean}[] - }>(url, query, {}, headers).catch((err) => { - throw err.response.status === 403 ? UnauthorizedAccessError(session.storeFqdn) : UnknownError() - }) + const data = await client + .request<{ + publicApiVersions: {handle: string; supported: boolean}[] + }>(query, {}) + .catch((err) => { + throw err.response.status === 403 ? UnauthorizedAccessError(session.storeFqdn) : UnknownError() + }) return data.publicApiVersions .filter((item) => item.supported) diff --git a/packages/cli-kit/src/api/identity.ts b/packages/cli-kit/src/api/identity.ts index 86ebea8b8cd..f6e34f4642e 100644 --- a/packages/cli-kit/src/api/identity.ts +++ b/packages/cli-kit/src/api/identity.ts @@ -1,6 +1,6 @@ import {identity} from '../environment/fqdn.js' import {debug} from '../output.js' -import {fetch} from '../http.js' +import {shopifyFetch} from '../http.js' export async function validateIdentityToken(token: string) { try { @@ -13,7 +13,7 @@ export async function validateIdentityToken(token: string) { } debug(`Sending Identity Introspection request to URL: ${instrospectionURL}`) - const response = await fetch(instrospectionURL, options) + const response = await shopifyFetch('shopify', instrospectionURL, options) // eslint-disable-next-line @typescript-eslint/no-explicit-any const json: any = await response.json() @@ -27,7 +27,7 @@ export async function validateIdentityToken(token: string) { } async function getInstrospectionEndpoint(): Promise { - const response = await fetch(`https://${await identity()}/.well-known/openid-configuration.json`) + const response = await shopifyFetch('identity', `https://${await identity()}/.well-known/openid-configuration.json`) // eslint-disable-next-line @typescript-eslint/no-explicit-any const json: any = await response.json() return json.introspection_endpoint diff --git a/packages/cli-kit/src/api/partners.test.ts b/packages/cli-kit/src/api/partners.test.ts index db12a94295b..1b98e46392c 100644 --- a/packages/cli-kit/src/api/partners.test.ts +++ b/packages/cli-kit/src/api/partners.test.ts @@ -1,63 +1,64 @@ import * as partnersApi from './partners.js' import {buildHeaders} from './common.js' import {partners} from '../environment/fqdn.js' -import {test, vi, expect, describe, it} from 'vitest' -import {ClientError, request as graphqlRequest} from 'graphql-request' - -vi.mock('graphql-request', async () => { - const {gql, ClientError} = await vi.importActual('graphql-request') - return { - request: vi.fn(), - gql, - ClientError, - } -}) +import {graphqlClient} from '../http/graphql.js' +import {test, vi, expect, describe, beforeEach} from 'vitest' +import {GraphQLClient, ClientError} from 'graphql-request' +vi.mock('../http/graphql.js') vi.mock('./common.js', async () => { - const common: any = await vi.importActual('./common.js') + const module: any = await vi.importActual('./common.js') return { - ...common, + ...module, buildHeaders: vi.fn(), } }) -vi.mock('../environment/fqdn') +vi.mock('../environment/fqdn.js') const mockedResult = 'OK' const partnersFQDN = 'partners.shopify.com' const url = 'https://partners.shopify.com/api/cli/graphql' const mockedToken = 'token' +let client: GraphQLClient +beforeEach(() => { + client = { + request: vi.fn(), + } as any + vi.mocked(graphqlClient).mockResolvedValue(client) +}) + describe('partners-api', () => { test('calls the graphql client once', async () => { // Given - vi.mocked(graphqlRequest).mockResolvedValue(mockedResult) + vi.mocked(client.request).mockResolvedValue(mockedResult) vi.mocked(partners).mockResolvedValue(partnersFQDN) // When await partnersApi.request('query', mockedToken, {some: 'variables'}) // Then - expect(graphqlRequest).toHaveBeenCalledOnce() + expect(client.request).toHaveBeenCalledOnce() }) test('request is called with correct parameters', async () => { // Given // eslint-disable-next-line @typescript-eslint/naming-convention const headers = {'custom-header': mockedToken} - vi.mocked(graphqlRequest).mockResolvedValue(mockedResult) - vi.mocked(buildHeaders).mockResolvedValue(headers) + vi.mocked(client.request).mockResolvedValue(mockedResult) + vi.mocked(client.request).mockResolvedValue(headers) vi.mocked(partners).mockResolvedValue(partnersFQDN) // When await partnersApi.request('query', mockedToken, {variables: 'variables'}) // Then - expect(graphqlRequest).toHaveBeenLastCalledWith(url, 'query', {variables: 'variables'}, headers) + expect(client.request).toHaveBeenLastCalledWith('query', {variables: 'variables'}) }) test('buildHeaders is called with user token', async () => { // Given - vi.mocked(graphqlRequest).mockResolvedValue(mockedResult) + vi.mocked(client.request).mockResolvedValue(mockedResult) vi.mocked(partners).mockResolvedValue(partnersFQDN) // When @@ -69,9 +70,9 @@ describe('partners-api', () => { }) describe('checkIfTokenIsRevoked', () => { - it('returns true if error is 401', async () => { + test('returns true if error is 401', async () => { const graphQLError = new ClientError({status: 401}, {query: ''}) - vi.mocked(graphqlRequest).mockRejectedValueOnce(graphQLError) + vi.mocked(client.request).mockRejectedValueOnce(graphQLError) vi.mocked(partners).mockResolvedValue(partnersFQDN) const got = await partnersApi.checkIfTokenIsRevoked(mockedToken) @@ -79,9 +80,9 @@ describe('checkIfTokenIsRevoked', () => { expect(got).toBe(true) }) - it('returns false if error is not 401', async () => { + test('returns false if error is not 401', async () => { const graphQLError = new ClientError({status: 404}, {query: ''}) - vi.mocked(graphqlRequest).mockRejectedValueOnce(graphQLError) + vi.mocked(client.request).mockRejectedValueOnce(graphQLError) vi.mocked(partners).mockResolvedValue(partnersFQDN) const got = await partnersApi.checkIfTokenIsRevoked(mockedToken) @@ -89,8 +90,8 @@ describe('checkIfTokenIsRevoked', () => { expect(got).toBe(false) }) - it('returns false if there is no error', async () => { - vi.mocked(graphqlRequest).mockResolvedValue(mockedResult) + test('returns false if there is no error', async () => { + vi.mocked(client.request).mockResolvedValue(mockedResult) vi.mocked(partners).mockResolvedValue(partnersFQDN) const got = await partnersApi.checkIfTokenIsRevoked(mockedToken) diff --git a/packages/cli-kit/src/api/partners.ts b/packages/cli-kit/src/api/partners.ts index 2f14a1a35fc..6ed0d4c403b 100644 --- a/packages/cli-kit/src/api/partners.ts +++ b/packages/cli-kit/src/api/partners.ts @@ -1,7 +1,8 @@ import {buildHeaders, debugLogRequest, handlingErrors} from './common.js' import {ScriptServiceProxyQuery} from './graphql/index.js' import {partners as partnersFqdn} from '../environment/fqdn.js' -import {request as graphqlRequest, Variables, RequestDocument, ClientError, gql} from 'graphql-request' +import {graphqlClient} from '../http/graphql.js' +import {Variables, ClientError, gql, RequestDocument} from 'graphql-request' export async function request(query: RequestDocument, token: string, variables?: Variables): Promise { const api = 'Partners' @@ -10,7 +11,12 @@ export async function request(query: RequestDocument, token: string, variable const url = `https://${fqdn}/api/cli/graphql` const headers = await buildHeaders(token) debugLogRequest(api, query, variables, headers) - const response = await graphqlRequest(url, query, variables, headers) + const client = await graphqlClient({ + headers, + service: 'partners', + url, + }) + const response = await client.request(query, variables) return response }) } @@ -30,13 +36,16 @@ export async function checkIfTokenIsRevoked(token: string): Promise { } } ` - const fqdn = await partnersFqdn() const url = `https://${fqdn}/api/cli/graphql` const headers = await buildHeaders(token) - + const client = await graphqlClient({ + headers, + url, + service: 'partners', + }) try { - await graphqlRequest(url, query, {}, headers) + await client.request(query, {}) return false // eslint-disable-next-line no-catch-all/no-catch-all } catch (error) { diff --git a/packages/cli-kit/src/environment/service.ts b/packages/cli-kit/src/environment/service.ts index 3242fd99e11..e7313c6924b 100644 --- a/packages/cli-kit/src/environment/service.ts +++ b/packages/cli-kit/src/environment/service.ts @@ -1,6 +1,27 @@ -import {Environment} from '../network/service.js' +import { + partners as partnersEnvironment, + shopify as shopifyEnvironment, + identity as identityEnvironment, +} from './service.js' +import {Environment, Service} from '../network/service.js' import constants from '../constants.js' +export async function environmentForService(service: Service): Promise { + let environment: Environment + switch (service) { + case 'identity': + environment = await identityEnvironment() + break + case 'partners': + environment = await partnersEnvironment() + break + case 'shopify': + environment = await shopifyEnvironment() + break + } + return environment +} + /** * Given an environment variable that represents the environment to use for a given serve, * it returns the environment as a enum; diff --git a/packages/cli-kit/src/http.ts b/packages/cli-kit/src/http.ts index 0093d073eee..5a65c83f750 100644 --- a/packages/cli-kit/src/http.ts +++ b/packages/cli-kit/src/http.ts @@ -1,2 +1,33 @@ +import {Service} from './network/service.js' +import {environmentForService} from './environment/service.js' +import https from 'https' + export {default as fetch} from './http/fetch.js' +export {shopifyFetch} from './http/fetch.js' export {default as formData} from './http/formdata.js' + +/** + * This utility function returns the https.Agent to use for a given service. The agent + * includes the right configuration based on the service's environment. For example, + * if the service is running in a Spin environment, the attribute "rejectUnauthorized" is + * set to false + */ +export async function httpsAgent(service: Service) { + return new https.Agent({rejectUnauthorized: await shouldRejectUnauthorizedRequests(service)}) +} + +/** + * Spin stores the CA certificate in the keychain and it should be used when sending HTTP + * requests to Spin instances. However, Node doesn't read certificates from the Keychain + * by default, which leads to Shopifolks running into issues that they workaround by setting the + * NODE_TLS_REJECT_UNAUTHORIZED=0 environment variable, which applies to all the HTTP + * requests sent from the CLI (context: https://github.com/nodejs/node/issues/39657) + * This utility function allows controlling the behavior in a per-service level by returning + * the value of for the "rejectUnauthorized" attribute that's used in the https agent. + * + * @returns {Promise} A promise that resolves with a boolean indicating whether + * unauthorized requests should be rejected or not. + */ +export async function shouldRejectUnauthorizedRequests(service: Service): Promise { + return (await environmentForService(service)) !== 'spin' +} diff --git a/packages/cli-kit/src/http/fetch.ts b/packages/cli-kit/src/http/fetch.ts index 021b95eeaf2..ce1522134ed 100644 --- a/packages/cli-kit/src/http/fetch.ts +++ b/packages/cli-kit/src/http/fetch.ts @@ -1,3 +1,6 @@ +import {Service} from '../network/service.js' + +import {httpsAgent} from '../http.js' import nodeFetch from 'node-fetch' import type {RequestInfo, RequestInit} from 'node-fetch' @@ -13,9 +16,17 @@ type Response = ReturnType * @param init {RequestInit} An object containing any custom settings that you want to apply to the request * @returns A promise that resolves with the response. */ -async function fetch(url: RequestInfo, init?: RequestInit): Response { +export default async function fetch(url: RequestInfo, init?: RequestInit): Response { const response = await nodeFetch(url, init) return response } -export default fetch +/** + * A fetch function to use with Shopify services. The function ensures the right + * TLS configuragion is used based on the environment in which the service is running + * (e.g. spin) + */ +export async function shopifyFetch(service: Service, url: RequestInfo, init?: RequestInit): Response { + const response = await nodeFetch(url, {...init, agent: await httpsAgent(service)}) + return response +} diff --git a/packages/cli-kit/src/http/graphql.ts b/packages/cli-kit/src/http/graphql.ts new file mode 100644 index 00000000000..2569eac4859 --- /dev/null +++ b/packages/cli-kit/src/http/graphql.ts @@ -0,0 +1,19 @@ +import {Service} from '../network/service.js' +import {httpsAgent} from '../http.js' +import {GraphQLClient} from 'graphql-request' + +interface GraphqlClientOptions { + url: string + service: Service + headers: {[key: string]: string} +} + +/** + * Creates a GraphQLClient instance with the right HTTPs agent baed on the service + * the client will interact with. + */ +export async function graphqlClient(options: GraphqlClientOptions) { + const clientOptions = {agent: await httpsAgent(options.service), headers: options.headers} + const client = new GraphQLClient(options.url, clientOptions) + return client +} diff --git a/packages/cli-kit/src/network/service.ts b/packages/cli-kit/src/network/service.ts index c9e034468f9..bc3340c3d92 100644 --- a/packages/cli-kit/src/network/service.ts +++ b/packages/cli-kit/src/network/service.ts @@ -3,7 +3,7 @@ * @readonly * @enum {number} */ -export type Service = 'shopify' | 'admin' | 'identity' +export type Service = 'shopify' | 'partners' | 'identity' /** * Enum that represents the environment to use for a given service. diff --git a/packages/cli-kit/src/session/exchange.test.ts b/packages/cli-kit/src/session/exchange.test.ts index 319de59659b..b5ab55b469a 100644 --- a/packages/cli-kit/src/session/exchange.test.ts +++ b/packages/cli-kit/src/session/exchange.test.ts @@ -3,7 +3,7 @@ import {exchangeAccessForApplicationTokens, exchangeCodeForAccessToken, InvalidGrantError} from './exchange.js' import {applicationId, clientId} from './identity.js' import {IdentityToken} from './schema.js' -import {fetch} from '../http.js' +import {shopifyFetch} from '../http.js' import {identity} from '../environment/fqdn.js' import {describe, it, expect, vi, afterAll, beforeEach} from 'vitest' import {Response} from 'node-fetch' @@ -46,13 +46,14 @@ describe('exchange code for identity token', () => { it('obtains an identity token from a authorization code', async () => { // Given const response = new Response(JSON.stringify(data)) - vi.mocked(fetch).mockResolvedValue(response) + vi.mocked(shopifyFetch).mockResolvedValue(response) // When const got = await exchangeCodeForAccessToken(code) // Then - expect(fetch).toBeCalledWith( + expect(shopifyFetch).toBeCalledWith( + 'identity', 'https://fqdn.com/oauth/token?grant_type=authorization_code&code=code&redirect_uri=http%3A%2F%2F127.0.0.1%3A3456&client_id=clientId&code_verifier=verifier', {method: 'POST'}, ) @@ -66,7 +67,7 @@ describe('exchange code for identity token', () => { error_description: 'The grant is invalid', } const response = new Response(JSON.stringify(responseBody), {status: 500}) - vi.mocked(fetch).mockResolvedValue(response) + vi.mocked(shopifyFetch).mockResolvedValue(response) // When const got = exchangeCodeForAccessToken(code) @@ -84,7 +85,7 @@ describe('exchange identity token for application tokens', () => { const response = new Response(JSON.stringify(data)) // Need to do it 3 times because a Response can only be used once - vi.mocked(fetch) + vi.mocked(shopifyFetch) .mockResolvedValue(response) .mockResolvedValueOnce(response.clone()) .mockResolvedValueOnce(response.clone()) @@ -118,7 +119,7 @@ describe('exchange identity token for application tokens', () => { const response = new Response(JSON.stringify(data)) // Need to do it 3 times because a Response can only be used once - vi.mocked(fetch) + vi.mocked(shopifyFetch) .mockResolvedValue(response) .mockResolvedValueOnce(response.clone()) .mockResolvedValueOnce(response.clone()) diff --git a/packages/cli-kit/src/session/exchange.ts b/packages/cli-kit/src/session/exchange.ts index f8e4b2cf4bf..d6fa22fa2d4 100644 --- a/packages/cli-kit/src/session/exchange.ts +++ b/packages/cli-kit/src/session/exchange.ts @@ -4,8 +4,8 @@ import {CodeAuthResult} from './authorize.js' import * as secureStore from './store.js' import {Abort} from '../error.js' import {API} from '../network/api.js' -import {fetch} from '../http.js' import {identity as identityFqdn} from '../environment/fqdn.js' +import {shopifyFetch} from '../http.js' export class InvalidGrantError extends Error {} @@ -137,7 +137,7 @@ async function tokenRequest(params: {[key: string]: string}): Promise { const fqdn = await identityFqdn() const url = new URL(`https://${fqdn}/oauth/token`) url.search = new URLSearchParams(Object.entries(params)).toString() - const res = await fetch(url.href, {method: 'POST'}) + const res = await shopifyFetch('identity', url.href, {method: 'POST'}) // eslint-disable-next-line @typescript-eslint/no-explicit-any const payload: any = await res.json() if (!res.ok) {