Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions packages/store/src/cli/services/store/auth/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
9 changes: 8 additions & 1 deletion packages/store/src/cli/services/store/auth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 18 additions & 0 deletions packages/store/src/cli/services/store/auth/result.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})
})
17 changes: 15 additions & 2 deletions packages/store/src/cli/services/store/auth/result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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.',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't print it, does this affect AI toolkit behavior?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No as the AI tooklit doesn't pass in the url as sensitive when going through the natural Oauth flow without the signup token

)
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('')
Expand Down
Loading