Merge pull request #8427 from Shopify/redact-store-auth-manual-url

Avoid printing sensitive store auth URLs

Author: craigmichaelmartin

packages/store/src/cli/services/store/auth/index.test.ts

@@ -304,10 +304,82 @@ 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)
   })
 
+  test('authenticateStoreWithApp marks manual auth URL as sensitive when signup JWT is present', async () => {
+    const openURL = vi.fn().mockResolvedValue(false)
+    const presenter = {
+      openingBrowser: vi.fn(),
+      manualAuthUrl: vi.fn(),
+      success: 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: vi.fn().mockResolvedValue({
+            access_token: 'token',
+            scope: 'read_products',
+            expires_in: 86400,
+            associated_user: {id: 42, email: 'test@example.com'},
+          }),
+          presenter,
+        },
+      ),
+    ).rejects.toThrow()
+
+    expect(presenter.manualAuthUrl).toHaveBeenCalledWith(expect.stringContaining('signup=signed.signup.jwt'), {
+      sensitive: true,
+    })
+  })
+
+  test('authenticateStoreWithApp fails immediately instead of waiting for a callback that cannot arrive', async () => {
+    const openURL = vi.fn().mockResolvedValue(false)
+    const presenter = {
+      openingBrowser: vi.fn(),
+      manualAuthUrl: vi.fn(),
+      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(

packages/store/src/cli/services/store/auth/index.ts

@@ -76,7 +76,14 @@ export async function authenticateStoreWithApp(
     ...bootstrap.waitForAuthCodeOptions,
     onListening: async () => {
       const opened = await resolvedDependencies.openURL(authorizationUrl)
-      if (!opened) resolvedDependencies.presenter.manualAuthUrl(authorizationUrl)
+      if (opened) return
+
+      const sensitive = Boolean(input.signup)
+      resolvedDependencies.presenter.manualAuthUrl(authorizationUrl, {sensitive})
+
+      // A withheld URL never reaches the browser, so the callback this server is waiting for cannot
+      // arrive. Returning here would leave the command idle until the timeout elapses.
+      if (sensitive) throw new AbortError("Authentication can't continue without a browser.")
     },
   })
   const tokenResponse = await bootstrap.exchangeCodeForToken(code)

packages/store/src/cli/services/store/auth/result.test.ts

@@ -103,4 +103,22 @@ describe('store auth presenter', () => {
     expect(streams.stdout()).toContain('"store": "shop.myshopify.com"')
     expect(streams.stdout()).not.toContain('Authenticated')
   })
+
+  test('does not print manual auth URL output when marked sensitive', () => {
+    const output = mockAndCaptureOutput()
+    const presenter = createStoreAuthPresenter('text')
+
+    presenter.manualAuthUrl('https://shop.myshopify.com/admin/oauth/authorize?client_id=test&secret=sensitive', {
+      sensitive: true,
+    })
+
+    expect(output.info()).toContain(
+      'Browser did not open automatically. The manual authorization URL contains sensitive credentials and was not printed.',
+    )
+    expect(output.info()).toContain(
+      'Run this command again in an environment where Shopify CLI can open a browser automatically.',
+    )
+    expect(output.info()).not.toContain('secret=sensitive')
+    expect(output.info()).not.toContain('https://shop.myshopify.com/admin/oauth/authorize')
+  })
 })

packages/store/src/cli/services/store/auth/result.ts

@@ -19,9 +19,13 @@ export interface StoreAuthResult {
 
 type StoreAuthOutputFormat = 'text' | 'json'
 
+interface ManualAuthUrlOptions {
+  sensitive?: boolean
+}
+
 export interface StoreAuthPresenter {
   openingBrowser: () => void
-  manualAuthUrl: (authorizationUrl: string) => void
+  manualAuthUrl: (authorizationUrl: string, options?: ManualAuthUrlOptions) => void
   success: (result: StoreAuthResult) => void
 }
 
@@ -47,7 +51,16 @@ function displayStoreAuthOpeningBrowser(): void {
   outputInfo('')
 }
 
-function displayStoreAuthManualAuthUrl(authorizationUrl: string): void {
+function displayStoreAuthManualAuthUrl(authorizationUrl: string, options: ManualAuthUrlOptions = {}): void {
+  if (options.sensitive) {
+    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
+  }
+
   outputInfo('Browser did not open automatically. Open this URL manually:')
   outputInfo(outputContent`${outputToken.link(authorizationUrl)}`)
   outputInfo('')