diff --git a/.changeset/admin-as-app-normalize-store.md b/.changeset/admin-as-app-normalize-store.md new file mode 100644 index 00000000000..0a9e31ded4a --- /dev/null +++ b/.changeset/admin-as-app-normalize-store.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': patch +--- + +Normalize and validate the store domain in `ensureAuthenticatedAdminAsApp` before sending the app's client secret. diff --git a/packages/app/src/cli/services/app-security-commands.test.ts b/packages/app/src/cli/services/app-security-commands.test.ts index 3412894b8a4..fe5e00a5f34 100644 --- a/packages/app/src/cli/services/app-security-commands.test.ts +++ b/packages/app/src/cli/services/app-security-commands.test.ts @@ -508,6 +508,8 @@ describe('formatAppSecurityCommand', () => { test.skipIf(process.platform !== 'win32')( 'the PowerShell record command reads a findings file without a byte order mark as UTF-8 in Windows PowerShell 5.1', + // Native PowerShell startup needs its own allowance on shared Windows runners. + {timeout: 30000}, async () => { await inTemporaryDirectory(async (directory) => { const findingsPath = joinPath(directory, 'findings.json') @@ -531,9 +533,10 @@ describe('formatAppSecurityCommand', () => { const result = spawnSync( 'powershell.exe', ['-NoProfile', '-NonInteractive', '-EncodedCommand', Buffer.from(script, 'utf16le').toString('base64')], - {encoding: 'utf8', windowsHide: true}, + {encoding: 'utf8', windowsHide: true, timeout: 25000}, ) + expect(result.error).toBeUndefined() expect(result.status, result.stderr).toBe(0) expect(JSON.parse(await readFile(receivedPath))).toStrictEqual({file: 'app/café.ts'}) }) diff --git a/packages/cli-kit/src/public/node/session.test.ts b/packages/cli-kit/src/public/node/session.test.ts index 5236d53a49d..5aa72a0a4e2 100644 --- a/packages/cli-kit/src/public/node/session.test.ts +++ b/packages/cli-kit/src/public/node/session.test.ts @@ -512,4 +512,53 @@ describe('ensureAuthenticatedAdminAsApp', () => { // Then await expect(got).rejects.toThrow('Failed to get access token for app client123 on store mystore.myshopify.com') }) + + test('normalizes the store before sending the client secret', async () => { + // Given + vi.mocked(shopifyFetch).mockResolvedValueOnce({ + status: 200, + text: async () => JSON.stringify({access_token: 'app_access_token'}), + } as any) + + // When + const got = await ensureAuthenticatedAdminAsApp('mystore', 'client123', 'secret456') + + // Then + expect(shopifyFetch).toHaveBeenCalledWith( + 'https://mystore.myshopify.com/admin/oauth/access_token', + expect.anything(), + 'slow-request', + ) + expect(got).toEqual({token: 'app_access_token', storeFqdn: 'mystore.myshopify.com'}) + }) + + test('only sends the client secret to a Shopify store domain', async () => { + // Given + vi.mocked(shopifyFetch).mockResolvedValueOnce({ + status: 200, + text: async () => JSON.stringify({access_token: 'app_access_token'}), + } as any) + + // When + await ensureAuthenticatedAdminAsApp('evil.attacker.test', 'client123', 'secret456') + + // Then + expect(shopifyFetch).toHaveBeenCalledWith( + 'https://evil.attacker.test.myshopify.com/admin/oauth/access_token', + expect.anything(), + 'slow-request', + ) + }) + + test.each(['evil.attacker.test:8443', 'mystore.myshopify.com:8443', 'user@mystore.myshopify.com'])( + 'rejects %s without sending the client secret', + async (store) => { + // When + const got = ensureAuthenticatedAdminAsApp(store, 'client123', 'secret456') + + // Then + await expect(got).rejects.toThrow(`Invalid store value: ${store}`) + expect(shopifyFetch).not.toHaveBeenCalled() + }, + ) }) diff --git a/packages/cli-kit/src/public/node/session.ts b/packages/cli-kit/src/public/node/session.ts index 5ec9a3288b0..ef93c864a6e 100644 --- a/packages/cli-kit/src/public/node/session.ts +++ b/packages/cli-kit/src/public/node/session.ts @@ -2,6 +2,7 @@ import {shopifyFetch} from './http.js' import {nonRandomUUID} from './crypto.js' import {getAppAutomationToken} from './environment.js' import {AbortError, BugError} from './error.js' +import {normalizeStoreFqdn} from './context/fqdn.js' import {outputContent, outputToken, outputDebug} from './output.js' import * as sessionStore from '../../private/node/session/store.js' import {automationTokenVariable} from '../../private/node/session/automation-token.js' @@ -391,7 +392,8 @@ export function logout(): Promise { * * See `ensureAuthenticatedAdmin` for access on behalf of a user. * - * @param storeFqdn - Store fqdn to request auth for. + * @param storeFqdn - Store handle or domain, normalized with `normalizeStoreFqdn`. Unsupported URL components are + * rejected before the client secret is sent. * @param clientId - Client ID of the app. * @param clientSecret - Client secret of the app. * @returns The access token for the Admin API. @@ -401,13 +403,14 @@ export async function ensureAuthenticatedAdminAsApp( clientId: string, clientSecret: string, ): Promise { + const normalizedStoreFqdn = normalizeStoreFqdn(storeFqdn) const bodyData = { client_id: clientId, client_secret: clientSecret, grant_type: 'client_credentials', } const tokenResponse = await shopifyFetch( - `https://${storeFqdn}/admin/oauth/access_token`, + `https://${normalizedStoreFqdn}/admin/oauth/access_token`, { method: 'POST', headers: { @@ -424,17 +427,17 @@ export async function ensureAuthenticatedAdminAsApp( if (body.includes('app_not_installed')) { throw new AbortError( outputContent`App is not installed on ${outputToken.green( - storeFqdn, + normalizedStoreFqdn, )}. Try running ${outputToken.genericShellCommand(`shopify app dev`)} to connect your app to the shop.`, ) } throw new AbortError( - `Failed to get access token for app ${clientId} on store ${storeFqdn}: ${tokenResponse.statusText}`, + `Failed to get access token for app ${clientId} on store ${normalizedStoreFqdn}: ${tokenResponse.statusText}`, ) } try { const tokenJson = JSON.parse(body) as {access_token: string} - return {token: tokenJson.access_token, storeFqdn} + return {token: tokenJson.access_token, storeFqdn: normalizedStoreFqdn} } catch (error) { if (error instanceof SyntaxError) { throw new AbortError(