diff --git a/packages/store/src/cli/commands/store/stripe-auth.test.ts b/packages/store/src/cli/commands/store/stripe-auth.test.ts index aef7e95485a..75512b1eb2c 100644 --- a/packages/store/src/cli/commands/store/stripe-auth.test.ts +++ b/packages/store/src/cli/commands/store/stripe-auth.test.ts @@ -1,6 +1,7 @@ import StoreStripeAuth, {readSignupJwtFromStdin} from './stripe-auth.js' import {authenticateStoreWithApp} from '../../services/store/auth/index.js' import {createStoreAuthPresenter} from '../../services/store/auth/result.js' +import {isStdinPiped} from '@shopify/cli-kit/node/system' import {describe, expect, test, vi} from 'vitest' import {Readable} from 'stream' @@ -9,6 +10,10 @@ vi.mock('../../services/store/attribution.js') vi.mock('../../services/store/auth/result.js', () => ({ createStoreAuthPresenter: vi.fn((format: 'text' | 'json') => ({format})), })) +vi.mock('@shopify/cli-kit/node/system', async (importOriginal) => ({ + ...(await importOriginal()), + isStdinPiped: vi.fn(), +})) describe('store stripe-auth command', () => { test('passes signup JWT through to the auth service', async () => { @@ -71,4 +76,25 @@ describe('store stripe-auth command', () => { test('rejects blank stdin signup JWTs', async () => { await expect(readSignupJwtFromStdin(Readable.from(['\n']))).rejects.toThrow('Missing signup JWT') }) + + test('reports the missing credential instead of waiting when stdin is an interactive terminal', async () => { + vi.mocked(isStdinPiped).mockReturnValue(false) + + await expect(readSignupJwtFromStdin()).rejects.toThrow('Missing signup JWT') + }) + + test('rejects a stdin signup JWT larger than the accepted size', async () => { + const oversized = 'a'.repeat(8 * 1024 + 1) + + await expect(readSignupJwtFromStdin(Readable.from([oversized]))).rejects.toThrow('too large') + }) + + test('does not authenticate when the signup flag is empty and no JWT is piped', async () => { + vi.mocked(isStdinPiped).mockReturnValue(false) + + await expect( + StoreStripeAuth.run(['--store', 'shop.myshopify.com', '--scopes', 'read_products', '--signup', '']), + ).rejects.toThrow() + expect(authenticateStoreWithApp).not.toHaveBeenCalled() + }) }) diff --git a/packages/store/src/cli/commands/store/stripe-auth.ts b/packages/store/src/cli/commands/store/stripe-auth.ts index cf33523c17f..1ce18e20e33 100644 --- a/packages/store/src/cli/commands/store/stripe-auth.ts +++ b/packages/store/src/cli/commands/store/stripe-auth.ts @@ -4,6 +4,7 @@ import StoreCommand from '../../utilities/store-command.js' import {storeFlags} from '../../flags.js' import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli' import {AbortError} from '@shopify/cli-kit/node/error' +import {isStdinPiped} from '@shopify/cli-kit/node/system' import {Flags} from '@oclif/core' export default class StoreStripeAuth extends StoreCommand { @@ -39,7 +40,7 @@ export default class StoreStripeAuth extends StoreCommand { public async run(): Promise { const {flags} = await this.parse(StoreStripeAuth) - const signup = flags.signup ?? (await readSignupJwtFromStdin()) + const signup = signupFlagValue(flags.signup) ?? (await readSignupJwtFromStdin()) await authenticateStoreWithApp( { @@ -54,21 +55,39 @@ export default class StoreStripeAuth extends StoreCommand { } } +const MAX_SIGNUP_JWT_BYTES = 8 * 1024 +const MISSING_SIGNUP_JWT = 'Missing signup JWT.' +const MISSING_SIGNUP_JWT_GUIDANCE = 'Pass --signup , set SHOPIFY_FLAG_SIGNUP, or pipe the JWT to stdin.' + +// A blank --signup is a credential that was never supplied rather than an empty one, so it falls +// through to stdin instead of starting an authorization without it. +function signupFlagValue(signup: string | undefined): string | undefined { + const trimmed = signup?.trim() + return trimmed === '' ? undefined : trimmed +} + export async function readSignupJwtFromStdin( stdin: NodeJS.ReadableStream & AsyncIterable = process.stdin, ): Promise { + // An interactive stdin never ends, so reading it would hang the command instead of reporting the + // credential that was never supplied. + if (stdin === process.stdin && !isStdinPiped()) { + throw new AbortError(MISSING_SIGNUP_JWT, MISSING_SIGNUP_JWT_GUIDANCE) + } + const chunks: Buffer[] = [] + let byteLength = 0 for await (const chunk of stdin) { - chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)) + const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk) + byteLength += buffer.length + if (byteLength > MAX_SIGNUP_JWT_BYTES) { + throw new AbortError('The signup JWT piped to stdin is too large.', 'Pipe only the JWT.') + } + chunks.push(buffer) } const signup = Buffer.concat(chunks).toString('utf8').trim() - if (!signup) { - throw new AbortError( - 'Missing signup JWT.', - 'Pass --signup , set SHOPIFY_FLAG_SIGNUP, or pipe the JWT to stdin.', - ) - } + if (!signup) throw new AbortError(MISSING_SIGNUP_JWT, MISSING_SIGNUP_JWT_GUIDANCE) return signup } diff --git a/packages/store/src/cli/services/store/auth/callback.test.ts b/packages/store/src/cli/services/store/auth/callback.test.ts index 133caca5102..3e357d27c5e 100644 --- a/packages/store/src/cli/services/store/auth/callback.test.ts +++ b/packages/store/src/cli/services/store/auth/callback.test.ts @@ -92,6 +92,100 @@ describe('store auth callback server', () => { ).resolves.toBe('abc123') }) + test('waitForStoreAuthCode answers 404 for a wrong nonce, a replay, and a non-GET handoff request', async () => { + const port = await getAvailablePort() + const params = callbackParams() + const authorizationUrl = 'https://shop.myshopify.com/admin/oauth/authorize?signup=signed.signup.jwt' + const handoffUrl = `http://127.0.0.1:${port}/auth/handoff?nonce=nonce-123` + const statuses: Record = {} + const bodies: string[] = [] + + const onListening = async () => { + const wrongNonce = await globalThis.fetch(`http://127.0.0.1:${port}/auth/handoff?nonce=wrong`, { + redirect: 'manual', + }) + statuses.wrongNonce = wrongNonce.status + bodies.push(await wrongNonce.text()) + + const missingNonce = await globalThis.fetch(`http://127.0.0.1:${port}/auth/handoff`, {redirect: 'manual'}) + statuses.missingNonce = missingNonce.status + bodies.push(await missingNonce.text()) + + const notGet = await globalThis.fetch(handoffUrl, {method: 'POST', redirect: 'manual'}) + statuses.notGet = notGet.status + bodies.push(await notGet.text()) + + const served = await globalThis.fetch(handoffUrl, {redirect: 'manual'}) + statuses.served = served.status + await served.text() + + const replay = await globalThis.fetch(handoffUrl, {redirect: 'manual'}) + statuses.replay = replay.status + bodies.push(await replay.text()) + + const callbackResponse = await globalThis.fetch(`http://127.0.0.1:${port}/auth/callback?${params.toString()}`) + await callbackResponse.text() + } + + await expect( + waitForStoreAuthCode({ + store: 'shop.myshopify.com', + state: 'state-123', + port, + timeoutMs: 1000, + authorizationRedirect: {nonce: 'nonce-123', authorizationUrl}, + onListening, + }), + ).resolves.toBe('abc123') + + expect(statuses).toEqual({wrongNonce: 404, missingNonce: 404, notGet: 404, served: 302, replay: 404}) + expect(bodies.join('')).not.toContain('signed.signup.jwt') + }) + + test('waitForStoreAuthCode does not spend the handoff on a speculative browser fetch', async () => { + const port = await getAvailablePort() + const params = callbackParams() + const authorizationUrl = 'https://shop.myshopify.com/admin/oauth/authorize?signup=signed.signup.jwt' + const handoffUrl = `http://127.0.0.1:${port}/auth/handoff?nonce=nonce-123` + let prefetchStatus = 0 + let prefetchBody = '' + let navigationStatus = 0 + let navigationLocation: string | null = null + + const onListening = async () => { + const prefetch = await globalThis.fetch(handoffUrl, { + headers: {'Sec-Purpose': 'prefetch;prerender'}, + redirect: 'manual', + }) + prefetchStatus = prefetch.status + prefetchBody = await prefetch.text() + + const navigation = await globalThis.fetch(handoffUrl, {redirect: 'manual'}) + navigationStatus = navigation.status + navigationLocation = navigation.headers.get('Location') + await navigation.text() + + const callbackResponse = await globalThis.fetch(`http://127.0.0.1:${port}/auth/callback?${params.toString()}`) + await callbackResponse.text() + } + + await expect( + waitForStoreAuthCode({ + store: 'shop.myshopify.com', + state: 'state-123', + port, + timeoutMs: 1000, + authorizationRedirect: {nonce: 'nonce-123', authorizationUrl}, + onListening, + }), + ).resolves.toBe('abc123') + + expect(prefetchStatus).toBe(404) + expect(prefetchBody).not.toContain('signed.signup.jwt') + expect(navigationStatus).toBe(302) + expect(navigationLocation).toBe(authorizationUrl) + }) + test('waitForStoreAuthCode rejects when callback state does not match', async () => { const port = await getAvailablePort() const params = callbackParams({state: 'wrong-state'}) diff --git a/packages/store/src/cli/services/store/auth/callback.ts b/packages/store/src/cli/services/store/auth/callback.ts index eacf4b95de0..3331a08149b 100644 --- a/packages/store/src/cli/services/store/auth/callback.ts +++ b/packages/store/src/cli/services/store/auth/callback.ts @@ -5,6 +5,12 @@ import {AbortError} from '@shopify/cli-kit/node/error' import {outputContent, outputDebug, outputToken} from '@shopify/cli-kit/node/output' import {timingSafeEqual} from 'crypto' import {createServer} from 'http' +import type {IncomingHttpHeaders} from 'http' + +export interface AuthorizationRedirect { + nonce: string + authorizationUrl: string +} export interface WaitForAuthCodeOptions { store: string @@ -12,10 +18,14 @@ export interface WaitForAuthCodeOptions { port: number timeoutMs?: number onListening?: () => void | Promise - authorizationRedirect?: { - nonce: string - authorizationUrl: string - } + authorizationRedirect?: AuthorizationRedirect +} + +// Browsers announce a speculative fetch so servers can decline side effects. Serving one would spend +// the single-use handoff before the navigation it is speculating about ever arrives. +function isSpeculativeRequest(headers: IncomingHttpHeaders): boolean { + const purpose = [headers['sec-purpose'], headers.purpose, headers['x-moz']].flat().join(' ') + return purpose.includes('prefetch') || purpose.includes('prerender') } function renderAuthCallbackPage(title: string, message: string): string { @@ -119,21 +129,26 @@ export async function waitForStoreAuthCode({ const server = createServer((req, res) => { const requestUrl = new URL(req.url ?? '/', `http://127.0.0.1:${port}`) - if (requestUrl.pathname === STORE_AUTH_HANDOFF_PATH && authorizationRedirect) { - const returnedNonce = requestUrl.searchParams.get('nonce') - if (!returnedNonce || !constantTimeEqual(returnedNonce, authorizationRedirect.nonce)) { - res.statusCode = 403 - res.setHeader('Cache-Control', 'no-store') - res.setHeader('Connection', 'close') - res.end('Forbidden') - return - } + const notFound = () => { + res.statusCode = 404 + res.setHeader('Cache-Control', 'no-store') + res.setHeader('Connection', 'close') + res.end('Not found') + } - if (authorizationRedirectUsed) { - res.statusCode = 410 - res.setHeader('Cache-Control', 'no-store') - res.setHeader('Connection', 'close') - res.end('Authorization handoff already used') + if (requestUrl.pathname === STORE_AUTH_HANDOFF_PATH) { + const returnedNonce = requestUrl.searchParams.get('nonce') + // Every rejection answers 404 so a local prober cannot tell a wrong nonce from a spent + // handoff, or either from a port with no store auth in flight. + const servable = + authorizationRedirect !== undefined && + !authorizationRedirectUsed && + req.method === 'GET' && + returnedNonce !== null && + constantTimeEqual(returnedNonce, authorizationRedirect.nonce) + + if (!servable || isSpeculativeRequest(req.headers)) { + notFound() return } @@ -148,8 +163,7 @@ export async function waitForStoreAuthCode({ } if (requestUrl.pathname !== STORE_AUTH_CALLBACK_PATH) { - res.statusCode = 404 - res.end('Not found') + notFound() return } diff --git a/packages/store/src/cli/services/store/auth/index.test.ts b/packages/store/src/cli/services/store/auth/index.test.ts index 63dad5722a1..24f15316b5d 100644 --- a/packages/store/src/cli/services/store/auth/index.test.ts +++ b/packages/store/src/cli/services/store/auth/index.test.ts @@ -280,7 +280,7 @@ describe('store auth service', () => { const openURL = vi.fn().mockResolvedValue(false) const presenter = { openingBrowser: vi.fn(), - manualAuthUrl: vi.fn(), + manualAuthUrl: vi.fn().mockReturnValue(true), success: vi.fn(), } const waitForStoreAuthCodeMock = vi.fn().mockImplementation(async (options) => { @@ -309,7 +309,6 @@ describe('store auth service', () => { expect(presenter.openingBrowser).toHaveBeenCalledOnce() expect(presenter.manualAuthUrl).toHaveBeenCalledWith( expect.stringContaining('https://shop.myshopify.com/admin/oauth/authorize?'), - {sensitive: false}, ) expect(presenter.success).toHaveBeenCalledWith(result) }) @@ -318,7 +317,7 @@ describe('store auth service', () => { const openURL = vi.fn().mockResolvedValue(false) const presenter = { openingBrowser: vi.fn(), - manualAuthUrl: vi.fn(), + manualAuthUrl: vi.fn().mockReturnValue(true), success: vi.fn(), } const waitForStoreAuthCodeMock = vi.fn().mockImplementation(async (options) => { @@ -347,11 +346,43 @@ describe('store auth service', () => { expect(presenter.manualAuthUrl).toHaveBeenCalledWith( expect.stringContaining('http://127.0.0.1:13387/auth/handoff?nonce='), - {sensitive: false}, ) expect(presenter.manualAuthUrl.mock.calls[0]![0]).not.toContain('signed.signup.jwt') }) + test('authenticateStoreWithApp fails immediately when the presenter withholds the authorization URL', async () => { + const openURL = vi.fn().mockResolvedValue(false) + const presenter = { + openingBrowser: vi.fn(), + manualAuthUrl: vi.fn().mockReturnValue(false), + success: vi.fn(), + } + const exchangeStoreAuthCodeForToken = vi.fn() + const waitForStoreAuthCodeMock = vi.fn().mockImplementation(async (options) => { + await options.onListening?.() + return 'abc123' + }) + + await expect( + authenticateStoreWithApp( + { + store: 'shop.myshopify.com', + scopes: 'read_products', + signup: 'signed.signup.jwt', + }, + { + openURL, + waitForStoreAuthCode: waitForStoreAuthCodeMock, + exchangeStoreAuthCodeForToken, + presenter, + }, + ), + ).rejects.toThrow("Authentication can't continue without a browser.") + + expect(exchangeStoreAuthCodeForToken).not.toHaveBeenCalled() + expect(presenter.success).not.toHaveBeenCalled() + }) + test('authenticateStoreWithApp records fqdn metadata before resolving existing scopes', async () => { await expect( authenticateStoreWithApp( diff --git a/packages/store/src/cli/services/store/auth/index.ts b/packages/store/src/cli/services/store/auth/index.ts index dbf6443f345..ea7e5962293 100644 --- a/packages/store/src/cli/services/store/auth/index.ts +++ b/packages/store/src/cli/services/store/auth/index.ts @@ -76,7 +76,12 @@ export async function authenticateStoreWithApp( ...bootstrap.waitForAuthCodeOptions, onListening: async () => { const opened = await resolvedDependencies.openURL(authorizationUrl) - if (!opened) resolvedDependencies.presenter.manualAuthUrl(authorizationUrl, {sensitive: false}) + if (opened) return + + // The callback server can only be reached by a browser that was given a URL, so waiting after a + // withheld one would idle until the timeout instead of reporting that authentication cannot proceed. + const surfaced = resolvedDependencies.presenter.manualAuthUrl(authorizationUrl) + if (!surfaced) throw new AbortError("Authentication can't continue without a browser.") }, }) const tokenResponse = await bootstrap.exchangeCodeForToken(code) diff --git a/packages/store/src/cli/services/store/auth/pkce.test.ts b/packages/store/src/cli/services/store/auth/pkce.test.ts index bb126f25d59..405e1c763ff 100644 --- a/packages/store/src/cli/services/store/auth/pkce.test.ts +++ b/packages/store/src/cli/services/store/auth/pkce.test.ts @@ -20,19 +20,28 @@ describe('store auth PKCE helpers', () => { expect(computeCodeChallenge(verifier)).toBe(expected) }) - test('buildStoreAuthUrl includes signup JWT when provided', () => { - const url = new URL( - buildStoreAuthUrl({ - store: 'shop.myshopify.com', - scopes: ['read_products'], - state: 'state-123', - redirectUri: 'http://127.0.0.1:13387/auth/callback', - codeChallenge: 'test-challenge-value', - signup: 'signed.signup.jwt', - }), - ) + test('buildStoreAuthUrl builds a store authorization URL that carries no signup credential', () => { + const url = buildStoreAuthUrl({ + store: 'shop.myshopify.com', + scopes: ['read_products'], + state: 'state-123', + redirectUri: 'http://127.0.0.1:13387/auth/callback', + codeChallenge: 'test-challenge-value', + }) + + expect(new URL(url).searchParams.has('signup')).toBe(false) + expect(url).not.toContain('signup') + }) + + test('createPkceBootstrap keeps the signup credential off the authorization context', () => { + const bootstrap = createPkceBootstrap({ + store: 'shop.myshopify.com', + scopes: ['read_products'], + signup: 'signed.signup.jwt', + exchangeCodeForToken: async () => ({access_token: 'token', scope: 'read_products'}), + }) - expect(url.searchParams.get('signup')).toBe('signed.signup.jwt') + expect(JSON.stringify(bootstrap.authorization)).not.toContain('signed.signup.jwt') }) test('createPkceBootstrap uses a loopback handoff URL when signup is provided', () => { diff --git a/packages/store/src/cli/services/store/auth/pkce.ts b/packages/store/src/cli/services/store/auth/pkce.ts index be97728fdf7..f1fd55372a4 100644 --- a/packages/store/src/cli/services/store/auth/pkce.ts +++ b/packages/store/src/cli/services/store/auth/pkce.ts @@ -3,7 +3,7 @@ import {randomUUID} from '@shopify/cli-kit/node/crypto' import {outputContent, outputDebug, outputToken} from '@shopify/cli-kit/node/output' import {createHash, randomBytes} from 'crypto' import type {StoreTokenResponse} from './token-client.js' -import type {WaitForAuthCodeOptions} from './callback.js' +import type {AuthorizationRedirect, WaitForAuthCodeOptions} from './callback.js' interface StoreAuthorizationContext { store: string @@ -13,7 +13,6 @@ interface StoreAuthorizationContext { redirectUri: string authorizationUrl: string codeVerifier: string - signup?: string } interface StoreAuthBootstrap { @@ -36,7 +35,6 @@ export function buildStoreAuthUrl(options: { state: string redirectUri: string codeChallenge: string - signup?: string }): string { const params = new URLSearchParams() params.set('client_id', STORE_AUTH_APP_CLIENT_ID) @@ -46,11 +44,17 @@ export function buildStoreAuthUrl(options: { params.set('response_type', 'code') params.set('code_challenge', options.codeChallenge) params.set('code_challenge_method', 'S256') - if (options.signup) params.set('signup', options.signup) return `https://${options.store}/admin/oauth/authorize?${params.toString()}` } +function buildAuthorizationRedirect(storeAuthorizationUrl: string, signup: string): AuthorizationRedirect { + const authorizationUrl = new URL(storeAuthorizationUrl) + authorizationUrl.searchParams.set('signup', signup) + + return {nonce: randomBytes(32).toString('base64url'), authorizationUrl: authorizationUrl.toString()} +} + export function createPkceBootstrap(options: { store: string scopes: string[] @@ -68,9 +72,11 @@ export function createPkceBootstrap(options: { const redirectUri = storeAuthRedirectUri(port) const codeVerifier = generateCodeVerifier() const codeChallenge = computeCodeChallenge(codeVerifier) - const sensitiveAuthorizationUrl = buildStoreAuthUrl({store, scopes, state, redirectUri, codeChallenge, signup}) - const handoffNonce = signup ? randomBytes(32).toString('base64url') : undefined - const authorizationUrl = handoffNonce ? storeAuthHandoffUri(port, handoffNonce) : sensitiveAuthorizationUrl + const storeAuthorizationUrl = buildStoreAuthUrl({store, scopes, state, redirectUri, codeChallenge}) + const authorizationRedirect = signup ? buildAuthorizationRedirect(storeAuthorizationUrl, signup) : undefined + const authorizationUrl = authorizationRedirect + ? storeAuthHandoffUri(port, authorizationRedirect.nonce) + : storeAuthorizationUrl outputDebug( outputContent`Starting PKCE auth for ${outputToken.raw(store)} with scopes ${outputToken.raw(scopes.join(','))} (redirect_uri=${outputToken.raw(redirectUri)})`, @@ -85,18 +91,12 @@ export function createPkceBootstrap(options: { redirectUri, authorizationUrl, codeVerifier, - signup, }, waitForAuthCodeOptions: { store, state, port, - authorizationRedirect: handoffNonce - ? { - nonce: handoffNonce, - authorizationUrl: sensitiveAuthorizationUrl, - } - : undefined, + authorizationRedirect, }, exchangeCodeForToken: (code: string) => exchangeCodeForToken({store, code, codeVerifier, redirectUri}), } diff --git a/packages/store/src/cli/services/store/auth/result.test.ts b/packages/store/src/cli/services/store/auth/result.test.ts index 0e820ea1fcb..92a98d2081f 100644 --- a/packages/store/src/cli/services/store/auth/result.test.ts +++ b/packages/store/src/cli/services/store/auth/result.test.ts @@ -108,9 +108,12 @@ describe('store auth presenter', () => { const output = mockAndCaptureOutput() const presenter = createStoreAuthPresenter('text') - presenter.manualAuthUrl('https://shop.myshopify.com/admin/oauth/authorize?client_id=test&secret=sensitive', { - sensitive: true, - }) + const surfaced = presenter.manualAuthUrl( + 'https://shop.myshopify.com/admin/oauth/authorize?client_id=test&secret=sensitive', + {sensitive: true}, + ) + + expect(surfaced).toBe(false) expect(output.info()).toContain( 'Browser did not open automatically. The manual authorization URL contains sensitive credentials and was not printed.', @@ -121,4 +124,47 @@ describe('store auth presenter', () => { expect(output.info()).not.toContain('secret=sensitive') expect(output.info()).not.toContain('https://shop.myshopify.com/admin/oauth/authorize') }) + + test('withholds a manual auth URL carrying a signup credential even when the caller does not mark it sensitive', () => { + const output = mockAndCaptureOutput() + const presenter = createStoreAuthPresenter('text') + + const surfaced = presenter.manualAuthUrl( + 'https://shop.myshopify.com/admin/oauth/authorize?client_id=test&signup=signed.signup.jwt', + ) + + expect(surfaced).toBe(false) + + expect(output.info()).toContain( + 'Browser did not open automatically. The manual authorization URL contains sensitive credentials and was not printed.', + ) + expect(output.info()).not.toContain('signed.signup.jwt') + expect(output.info()).not.toContain('signup=') + }) + + test('withholds a manual auth URL it cannot parse', () => { + const output = mockAndCaptureOutput() + const presenter = createStoreAuthPresenter('text') + + const surfaced = presenter.manualAuthUrl('not-a-url?signup=signed.signup.jwt') + + expect(surfaced).toBe(false) + + expect(output.info()).toContain( + 'Browser did not open automatically. The manual authorization URL contains sensitive credentials and was not printed.', + ) + expect(output.info()).not.toContain('signed.signup.jwt') + }) + + test('prints a loopback handoff URL that carries no credential', () => { + const output = mockAndCaptureOutput() + const presenter = createStoreAuthPresenter('text') + + const surfaced = presenter.manualAuthUrl('http://127.0.0.1:13387/auth/handoff?nonce=abc123') + + expect(surfaced).toBe(true) + + expect(output.info()).toContain('Browser did not open automatically. Open this URL manually:') + expect(output.info()).toContain('http://127.0.0.1:13387/auth/handoff?nonce=abc123') + }) }) diff --git a/packages/store/src/cli/services/store/auth/result.ts b/packages/store/src/cli/services/store/auth/result.ts index db83df4a277..59d20acb1d8 100644 --- a/packages/store/src/cli/services/store/auth/result.ts +++ b/packages/store/src/cli/services/store/auth/result.ts @@ -25,7 +25,7 @@ interface ManualAuthUrlOptions { export interface StoreAuthPresenter { openingBrowser: () => void - manualAuthUrl: (authorizationUrl: string, options?: ManualAuthUrlOptions) => void + manualAuthUrl: (authorizationUrl: string, options?: ManualAuthUrlOptions) => boolean success: (result: StoreAuthResult) => void } @@ -51,19 +51,28 @@ function displayStoreAuthOpeningBrowser(): void { outputInfo('') } -function displayStoreAuthManualAuthUrl(authorizationUrl: string, options: ManualAuthUrlOptions = {}): void { - if (options.sensitive) { +// Callers mark a URL sensitive when they know why it is; this catches the signup credential even when +// they forget, and fails closed on anything it cannot parse well enough to clear. +function carriesSignupCredential(authorizationUrl: string): boolean { + if (!URL.canParse(authorizationUrl)) return true + return new URL(authorizationUrl).searchParams.has('signup') +} + +function displayStoreAuthManualAuthUrl(authorizationUrl: string, options: ManualAuthUrlOptions = {}): boolean { + if (options.sensitive || carriesSignupCredential(authorizationUrl)) { outputInfo( 'Browser did not open automatically. The manual authorization URL contains sensitive credentials and was not printed.', ) outputInfo('Run this command again in an environment where Shopify CLI can open a browser automatically.') outputInfo('') - return + return false } outputInfo('Browser did not open automatically. Open this URL manually:') outputInfo(outputContent`${outputToken.link(authorizationUrl)}`) outputInfo('') + + return true } function displayStoreAuthResult(result: StoreAuthResult, format: StoreAuthOutputFormat = 'text'): void {