diff --git a/packages/cli/src/cli/commands/store/auth.test.ts b/packages/cli/src/cli/commands/store/auth.test.ts index 85797e3f288..068ce77c141 100644 --- a/packages/cli/src/cli/commands/store/auth.test.ts +++ b/packages/cli/src/cli/commands/store/auth.test.ts @@ -15,22 +15,19 @@ describe('store auth command', () => { 'shop.myshopify.com', '--scopes', 'read_products,write_products', - '--client-secret-file', - './client-secret.txt', ]) expect(authenticateStoreWithApp).toHaveBeenCalledWith({ store: 'shop.myshopify.com', scopes: 'read_products,write_products', - clientSecretFile: expect.stringContaining('client-secret.txt'), - port: 3458, + port: 13387, }) }) test('defines the expected flags', () => { expect(StoreAuth.flags.store).toBeDefined() expect(StoreAuth.flags.scopes).toBeDefined() - expect(StoreAuth.flags['client-secret-file']).toBeDefined() expect(StoreAuth.flags.port).toBeDefined() + expect(StoreAuth.flags['client-secret-file']).toBeUndefined() }) }) diff --git a/packages/cli/src/cli/commands/store/auth.ts b/packages/cli/src/cli/commands/store/auth.ts index 9d09c663fe5..ed004650af2 100644 --- a/packages/cli/src/cli/commands/store/auth.ts +++ b/packages/cli/src/cli/commands/store/auth.ts @@ -1,7 +1,6 @@ import Command from '@shopify/cli-kit/node/base-command' import {globalFlags} from '@shopify/cli-kit/node/cli' import {normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn' -import {resolvePath} from '@shopify/cli-kit/node/path' import {Flags} from '@oclif/core' import {authenticateStoreWithApp} from '../../services/store/auth.js' import {DEFAULT_STORE_AUTH_PORT} from '../../services/store/config.js' @@ -9,14 +8,14 @@ import {DEFAULT_STORE_AUTH_PORT} from '../../services/store/config.js' export default class StoreAuth extends Command { static summary = 'Authenticate an app against a store for store commands.' - static descriptionWithMarkdown = `Starts a standard Shopify app OAuth flow against the specified store and stores an online access token for later use by \`shopify store execute\`. + static descriptionWithMarkdown = `Starts a PKCE OAuth flow against the specified store and stores an online access token for later use by \`shopify store execute\`. This flow authenticates the app on behalf of the current user. Re-run this command if the stored token is missing, expires, or no longer has the scopes you need.` static description = this.descriptionWithoutMarkdown() static examples = [ - '<%= config.bin %> <%= command.id %> --store shop.myshopify.com --scopes read_products,write_products --client-secret-file ./client-secret.txt', + '<%= config.bin %> <%= command.id %> --store shop.myshopify.com --scopes read_products,write_products', ] static flags = { @@ -33,12 +32,6 @@ This flow authenticates the app on behalf of the current user. Re-run this comma env: 'SHOPIFY_FLAG_SCOPES', required: true, }), - 'client-secret-file': Flags.string({ - description: 'Path to a file containing the app client secret used for the OAuth code exchange.', - env: 'SHOPIFY_FLAG_CLIENT_SECRET_FILE', - parse: async (input) => resolvePath(input), - required: true, - }), port: Flags.integer({ description: 'Local port to use for the OAuth callback server.', env: 'SHOPIFY_FLAG_PORT', @@ -52,7 +45,6 @@ This flow authenticates the app on behalf of the current user. Re-run this comma await authenticateStoreWithApp({ store: flags.store, scopes: flags.scopes, - clientSecretFile: flags['client-secret-file'], port: flags.port, }) } diff --git a/packages/cli/src/cli/services/store/auth.test.ts b/packages/cli/src/cli/services/store/auth.test.ts index 542a067cd14..7548e626dc8 100644 --- a/packages/cli/src/cli/services/store/auth.test.ts +++ b/packages/cli/src/cli/services/store/auth.test.ts @@ -1,20 +1,19 @@ import {createServer} from 'http' -import {createHmac} from 'crypto' import {describe, test, expect, vi, beforeEach, afterEach} from 'vitest' import { authenticateStoreWithApp, buildStoreAuthUrl, parseStoreAuthScopes, - verifyStoreAuthHmac, + generateCodeVerifier, + computeCodeChallenge, exchangeStoreAuthCodeForToken, waitForStoreAuthCode, } from './auth.js' import {setStoredStoreAppSession} from './session.js' -import {fileExists, readFile} from '@shopify/cli-kit/node/fs' +import {STORE_AUTH_APP_CLIENT_ID} from './config.js' import {fetch} from '@shopify/cli-kit/node/http' vi.mock('./session.js') -vi.mock('@shopify/cli-kit/node/fs') vi.mock('@shopify/cli-kit/node/http') vi.mock('@shopify/cli-kit/node/system', () => ({openURL: vi.fn().mockResolvedValue(true)})) vi.mock('@shopify/cli-kit/node/crypto', () => ({randomUUID: vi.fn().mockReturnValue('state-123')})) @@ -24,7 +23,7 @@ async function getAvailablePort(): Promise { const server = createServer() server.on('error', reject) - server.listen(0, 'localhost', () => { + server.listen(0, '127.0.0.1', () => { const address = server.address() if (!address || typeof address === 'string') { reject(new Error('Expected an ephemeral port.')) @@ -43,45 +42,54 @@ async function getAvailablePort(): Promise { }) } -function signedCallbackParams(options?: { +function callbackParams(options?: { code?: string shop?: string state?: string - timestamp?: string error?: string - clientSecret?: string }): URLSearchParams { const params = new URLSearchParams() params.set('shop', options?.shop ?? 'shop.myshopify.com') params.set('state', options?.state ?? 'state-123') - params.set('timestamp', options?.timestamp ?? '1711550000') if (options?.code) params.set('code', options.code) if (options?.error) params.set('error', options.error) if (!options?.code && !options?.error) params.set('code', 'abc123') - const message = [...params.entries()] - .sort(([a], [b]) => a.localeCompare(b)) - .map(([key, value]) => `${key}=${value}`) - .join('&') - - const hmac = createHmac('sha256', options?.clientSecret ?? 'secret-value').update(message).digest('hex') - params.set('hmac', hmac) - return params } describe('store auth service', () => { beforeEach(() => { vi.clearAllMocks() - vi.mocked(fileExists).mockResolvedValue(true) - vi.mocked(readFile).mockResolvedValue('secret-value' as any) }) afterEach(() => { vi.restoreAllMocks() }) + // --- PKCE crypto --- + + test('generateCodeVerifier produces a base64url string of 43 chars', () => { + const verifier = generateCodeVerifier() + expect(verifier).toMatch(/^[A-Za-z0-9_-]{43}$/) + }) + + test('generateCodeVerifier produces unique values', () => { + const a = generateCodeVerifier() + const b = generateCodeVerifier() + expect(a).not.toBe(b) + }) + + test('computeCodeChallenge produces a deterministic S256 hash', () => { + // RFC 7636 Appendix B test vector + const verifier = 'dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk' + const expected = 'E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM' + expect(computeCodeChallenge(verifier)).toBe(expected) + }) + + // --- Scope parsing --- + test('parseStoreAuthScopes splits and deduplicates scopes', () => { expect(parseStoreAuthScopes('read_products, write_products,read_products')).toEqual([ 'read_products', @@ -89,38 +97,39 @@ describe('store auth service', () => { ]) }) - test('buildStoreAuthUrl includes per-user grant options', () => { + // --- Authorize URL --- + + test('buildStoreAuthUrl includes PKCE params and response_type=code', () => { const url = new URL( buildStoreAuthUrl({ store: 'shop.myshopify.com', scopes: ['read_products', 'write_products'], state: 'state-123', - redirectUri: 'http://localhost:3458/auth/callback', + redirectUri: 'http://127.0.0.1:13387/auth/callback', + codeChallenge: 'test-challenge-value', }), ) expect(url.hostname).toBe('shop.myshopify.com') expect(url.pathname).toBe('/admin/oauth/authorize') - expect(url.searchParams.get('client_id')).toBe('4c6af92692662b9c95c8a47b1520aced') + expect(url.searchParams.get('client_id')).toBe(STORE_AUTH_APP_CLIENT_ID) expect(url.searchParams.get('scope')).toBe('read_products,write_products') expect(url.searchParams.get('state')).toBe('state-123') - expect(url.searchParams.get('redirect_uri')).toBe('http://localhost:3458/auth/callback') - expect(url.searchParams.getAll('grant_options[]')).toEqual(['per-user']) + expect(url.searchParams.get('redirect_uri')).toBe('http://127.0.0.1:13387/auth/callback') + expect(url.searchParams.get('response_type')).toBe('code') + expect(url.searchParams.get('code_challenge')).toBe('test-challenge-value') + expect(url.searchParams.get('code_challenge_method')).toBe('S256') + expect(url.searchParams.get('grant_options[]')).toBeNull() }) - test('verifyStoreAuthHmac validates Shopify callback signatures', () => { - const params = signedCallbackParams() - - expect(verifyStoreAuthHmac(params, 'secret-value')).toBe(true) - expect(verifyStoreAuthHmac(params, 'wrong-secret')).toBe(false) - }) + // --- Callback server --- test('waitForStoreAuthCode resolves after a valid callback', async () => { const port = await getAvailablePort() - const callbackParams = signedCallbackParams() + const params = callbackParams() const onListening = vi.fn(async () => { const response = await globalThis.fetch( - `http://localhost:${port}/auth/callback?${callbackParams.toString()}`, + `http://127.0.0.1:${port}/auth/callback?${params.toString()}`, ) expect(response.status).toBe(200) await response.text() @@ -130,7 +139,6 @@ describe('store auth service', () => { waitForStoreAuthCode({ store: 'shop.myshopify.com', state: 'state-123', - clientSecret: 'secret-value', port, timeoutMs: 1000, onListening, @@ -142,18 +150,17 @@ describe('store auth service', () => { test('waitForStoreAuthCode rejects when callback state does not match', async () => { const port = await getAvailablePort() - const callbackParams = signedCallbackParams({state: 'wrong-state'}) + const params = callbackParams({state: 'wrong-state'}) await expect( waitForStoreAuthCode({ store: 'shop.myshopify.com', state: 'state-123', - clientSecret: 'secret-value', port, timeoutMs: 1000, onListening: async () => { const response = await globalThis.fetch( - `http://localhost:${port}/auth/callback?${callbackParams.toString()}`, + `http://127.0.0.1:${port}/auth/callback?${params.toString()}`, ) expect(response.status).toBe(400) await response.text() @@ -167,14 +174,13 @@ describe('store auth service', () => { const server = createServer() await new Promise((resolve, reject) => { server.on('error', reject) - server.listen(port, 'localhost', () => resolve()) + server.listen(port, '127.0.0.1', () => resolve()) }) await expect( waitForStoreAuthCode({ store: 'shop.myshopify.com', state: 'state-123', - clientSecret: 'secret-value', port, timeoutMs: 1000, }), @@ -199,33 +205,56 @@ describe('store auth service', () => { waitForStoreAuthCode({ store: 'shop.myshopify.com', state: 'state-123', - clientSecret: 'secret-value', port, timeoutMs: 25, }), ).rejects.toThrow('Timed out waiting for OAuth callback.') }) - test('exchangeStoreAuthCodeForToken returns token response', async () => { + // --- Token exchange --- + + test('exchangeStoreAuthCodeForToken sends PKCE params and returns token response', async () => { vi.mocked(fetch).mockResolvedValue({ ok: true, - text: vi.fn().mockResolvedValue('{"access_token":"token","scope":"read_products","associated_user":{"id":42}}'), + text: vi.fn().mockResolvedValue(JSON.stringify({ + access_token: 'token', + scope: 'read_products', + expires_in: 86400, + refresh_token: 'refresh-token', + associated_user: {id: 42, email: 'test@example.com'}, + })), } as any) const response = await exchangeStoreAuthCodeForToken({ store: 'shop.myshopify.com', code: 'abc123', - clientSecret: 'secret-value', + codeVerifier: 'test-verifier', + redirectUri: 'http://127.0.0.1:13387/auth/callback', }) - expect(response).toEqual({ - access_token: 'token', - scope: 'read_products', - associated_user: {id: 42}, - }) + expect(response.access_token).toBe('token') + expect(response.refresh_token).toBe('refresh-token') + expect(response.expires_in).toBe(86400) + + expect(fetch).toHaveBeenCalledWith( + 'https://shop.myshopify.com/admin/oauth/access_token', + expect.objectContaining({ + method: 'POST', + body: expect.stringContaining('"code_verifier":"test-verifier"'), + }), + ) + + const sentBody = JSON.parse((fetch as any).mock.calls[0][1].body) + expect(sentBody.client_id).toBe(STORE_AUTH_APP_CLIENT_ID) + expect(sentBody.code).toBe('abc123') + expect(sentBody.code_verifier).toBe('test-verifier') + expect(sentBody.redirect_uri).toBe('http://127.0.0.1:13387/auth/callback') + expect(sentBody.client_secret).toBeUndefined() }) - test('authenticateStoreWithApp opens the browser after the listener is ready and stores the app session', async () => { + // --- Full orchestration --- + + test('authenticateStoreWithApp opens the browser and stores the session with refresh token', async () => { const openURL = vi.fn().mockResolvedValue(true) const waitForStoreAuthCodeMock = vi.fn().mockImplementation(async (options) => { await options.onListening?.() @@ -236,8 +265,7 @@ describe('store auth service', () => { { store: 'shop.myshopify.com', scopes: 'read_products', - clientSecretFile: '/tmp/client-secret.txt', - port: 3458, + port: 13387, }, { openURL, @@ -245,7 +273,9 @@ describe('store auth service', () => { exchangeStoreAuthCodeForToken: vi.fn().mockResolvedValue({ access_token: 'token', scope: 'read_products', - associated_user: {id: 42}, + expires_in: 86400, + refresh_token: 'refresh-token', + associated_user: {id: 42, email: 'test@example.com'}, }), renderInfo: vi.fn(), renderSuccess: vi.fn(), @@ -255,74 +285,80 @@ describe('store auth service', () => { expect(openURL).toHaveBeenCalledWith( expect.stringContaining('/admin/oauth/authorize?'), ) - expect(setStoredStoreAppSession).toHaveBeenCalledWith({ - store: 'shop.myshopify.com', - clientId: '4c6af92692662b9c95c8a47b1520aced', - userId: '42', - accessToken: 'token', - scopes: ['read_products'], - acquiredAt: expect.any(String), + + const storedSession = vi.mocked(setStoredStoreAppSession).mock.calls[0]![0] + expect(storedSession.store).toBe('shop.myshopify.com') + expect(storedSession.clientId).toBe(STORE_AUTH_APP_CLIENT_ID) + expect(storedSession.userId).toBe('42') + expect(storedSession.accessToken).toBe('token') + expect(storedSession.refreshToken).toBe('refresh-token') + expect(storedSession.scopes).toEqual(['read_products']) + expect(storedSession.expiresAt).toBeDefined() + expect(storedSession.associatedUser).toEqual({ + id: 42, + email: 'test@example.com', + firstName: undefined, + lastName: undefined, + accountOwner: undefined, }) }) - test('authenticateStoreWithApp rejects when Shopify grants fewer scopes than requested', async () => { + test('authenticateStoreWithApp stores only the granted scopes when fewer than requested', async () => { const waitForStoreAuthCodeMock = vi.fn().mockImplementation(async (options) => { await options.onListening?.() return 'abc123' }) - await expect( - authenticateStoreWithApp( - { - store: 'shop.myshopify.com', - scopes: 'read_products,write_products', - clientSecretFile: '/tmp/client-secret.txt', - port: 3458, - }, - { - openURL: vi.fn().mockResolvedValue(true), - waitForStoreAuthCode: waitForStoreAuthCodeMock, - exchangeStoreAuthCodeForToken: vi.fn().mockResolvedValue({ - access_token: 'token', - scope: 'read_products', - associated_user: {id: 42}, - }), - renderInfo: vi.fn(), - renderSuccess: vi.fn(), - }, - ), - ).rejects.toThrow('Shopify granted fewer scopes than were requested.') + await authenticateStoreWithApp( + { + store: 'shop.myshopify.com', + scopes: 'read_products,write_products', + port: 13387, + }, + { + openURL: vi.fn().mockResolvedValue(true), + waitForStoreAuthCode: waitForStoreAuthCodeMock, + exchangeStoreAuthCodeForToken: vi.fn().mockResolvedValue({ + access_token: 'token', + scope: 'read_products', + expires_in: 86400, + associated_user: {id: 42, email: 'test@example.com'}, + }), + renderInfo: vi.fn(), + renderSuccess: vi.fn(), + }, + ) - expect(setStoredStoreAppSession).not.toHaveBeenCalled() + const storedSession = vi.mocked(setStoredStoreAppSession).mock.calls[0]![0] + expect(storedSession.scopes).toEqual(['read_products']) }) - test('authenticateStoreWithApp rejects when Shopify omits granted scopes', async () => { + test('authenticateStoreWithApp falls back to requested scopes when response omits scope', async () => { const waitForStoreAuthCodeMock = vi.fn().mockImplementation(async (options) => { await options.onListening?.() return 'abc123' }) - await expect( - authenticateStoreWithApp( - { - store: 'shop.myshopify.com', - scopes: 'read_products', - clientSecretFile: '/tmp/client-secret.txt', - port: 3458, - }, - { - openURL: vi.fn().mockResolvedValue(true), - waitForStoreAuthCode: waitForStoreAuthCodeMock, - exchangeStoreAuthCodeForToken: vi.fn().mockResolvedValue({ - access_token: 'token', - associated_user: {id: 42}, - }), - renderInfo: vi.fn(), - renderSuccess: vi.fn(), - }, - ), - ).rejects.toThrow('Shopify did not return granted scopes for the online access token.') + await authenticateStoreWithApp( + { + store: 'shop.myshopify.com', + scopes: 'read_products', + port: 13387, + }, + { + openURL: vi.fn().mockResolvedValue(true), + waitForStoreAuthCode: waitForStoreAuthCodeMock, + exchangeStoreAuthCodeForToken: vi.fn().mockResolvedValue({ + access_token: 'token', + expires_in: 86400, + associated_user: {id: 42, email: 'test@example.com'}, + }), + renderInfo: vi.fn(), + renderSuccess: vi.fn(), + }, + ) - expect(setStoredStoreAppSession).not.toHaveBeenCalled() + const storedSession = vi.mocked(setStoredStoreAppSession).mock.calls[0]![0] + expect(storedSession.scopes).toEqual(['read_products']) }) }) diff --git a/packages/cli/src/cli/services/store/auth.ts b/packages/cli/src/cli/services/store/auth.ts index 630bfb274e7..c6282df16f8 100644 --- a/packages/cli/src/cli/services/store/auth.ts +++ b/packages/cli/src/cli/services/store/auth.ts @@ -1,28 +1,38 @@ -import {DEFAULT_STORE_AUTH_PORT, STORE_AUTH_APP_CLIENT_ID, STORE_AUTH_CALLBACK_PATH, storeAuthRedirectUri} from './config.js' +import {DEFAULT_STORE_AUTH_PORT, STORE_AUTH_APP_CLIENT_ID, STORE_AUTH_CALLBACK_PATH, storeAuthRedirectUri, maskToken} from './config.js' import {setStoredStoreAppSession} from './session.js' import {normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn' import {randomUUID} from '@shopify/cli-kit/node/crypto' import {AbortError} from '@shopify/cli-kit/node/error' -import {fileExists, readFile} from '@shopify/cli-kit/node/fs' import {fetch} from '@shopify/cli-kit/node/http' -import {outputContent, outputToken} from '@shopify/cli-kit/node/output' +import {outputContent, outputDebug, outputToken} from '@shopify/cli-kit/node/output' import {openURL} from '@shopify/cli-kit/node/system' import {renderInfo, renderSuccess} from '@shopify/cli-kit/node/ui' -import {createHmac, timingSafeEqual} from 'crypto' +import {createHash, randomBytes, timingSafeEqual} from 'crypto' import {createServer} from 'http' interface StoreAuthInput { store: string scopes: string - clientSecretFile: string port?: number } interface StoreTokenResponse { access_token: string + token_type?: string scope?: string + expires_in?: number + refresh_token?: string + refresh_token_expires_in?: number + associated_user_scope?: string associated_user?: { id: number + first_name?: string + last_name?: string + email?: string + account_owner?: boolean + locale?: string + collaborator?: boolean + email_verified?: boolean } } @@ -33,6 +43,7 @@ interface StoreAuthorizationContext { port: number redirectUri: string authorizationUrl: string + codeVerifier: string } interface StoreAuthBootstrap { @@ -44,12 +55,23 @@ interface StoreAuthBootstrap { export interface WaitForAuthCodeOptions { store: string state: string - clientSecret: string port: number timeoutMs?: number onListening?: () => void | Promise } +// --- PKCE helpers (RFC 7636) --- + +export function generateCodeVerifier(): string { + return randomBytes(32).toString('base64url') +} + +export function computeCodeChallenge(verifier: string): string { + return createHash('sha256').update(verifier).digest('base64url') +} + +// --- Scope parsing --- + export function parseStoreAuthScopes(input: string): string[] { const scopes = input .split(',') @@ -65,58 +87,48 @@ export function parseStoreAuthScopes(input: string): string[] { function resolveGrantedScopes(tokenResponse: StoreTokenResponse, requestedScopes: string[]): string[] { if (!tokenResponse.scope) { - throw new AbortError('Shopify did not return granted scopes for the online access token.') + outputDebug(outputContent`Token response did not include scope field, falling back to requested scopes`) + return requestedScopes } const grantedScopes = parseStoreAuthScopes(tokenResponse.scope) const missingScopes = requestedScopes.filter((scope) => !grantedScopes.includes(scope)) if (missingScopes.length > 0) { - throw new AbortError( - 'Shopify granted fewer scopes than were requested.', - `Missing scopes: ${missingScopes.join(', ')}. Update the app or store installation scopes, then re-run shopify store auth.`, + outputDebug( + outputContent`Shopify granted fewer scopes than requested. Missing: ${outputToken.raw(missingScopes.join(', '))}`, ) } return grantedScopes } +// --- Authorize URL --- + export function buildStoreAuthUrl(options: { store: string scopes: string[] state: string redirectUri: string + codeChallenge: string }): string { const params = new URLSearchParams() params.set('client_id', STORE_AUTH_APP_CLIENT_ID) params.set('scope', options.scopes.join(',')) params.set('redirect_uri', options.redirectUri) params.set('state', options.state) - params.append('grant_options[]', 'per-user') + params.set('response_type', 'code') + params.set('code_challenge', options.codeChallenge) + params.set('code_challenge_method', 'S256') return `https://${options.store}/admin/oauth/authorize?${params.toString()}` } -export function verifyStoreAuthHmac(params: URLSearchParams, clientSecret: string): boolean { - const providedHmac = params.get('hmac') - if (!providedHmac) return false - - const entries = [...params.entries()] - .filter(([key]) => key !== 'hmac' && key !== 'signature') - .sort(([a], [b]) => a.localeCompare(b)) - .map(([key, value]) => `${key}=${value}`) - - const message = entries.join('&') - const digest = createHmac('sha256', clientSecret).update(message).digest('hex') - if (digest.length !== providedHmac.length) return false - - return timingSafeEqual(Buffer.from(digest, 'utf8'), Buffer.from(providedHmac, 'utf8')) -} +// --- Localhost callback server --- export async function waitForStoreAuthCode({ store, state, - clientSecret, port, timeoutMs = 5 * 60 * 1000, onListening, @@ -132,7 +144,7 @@ export async function waitForStoreAuthCode({ }, timeoutMs) const server = createServer((req, res) => { - const requestUrl = new URL(req.url ?? '/', `http://localhost:${port}`) + const requestUrl = new URL(req.url ?? '/', `http://127.0.0.1:${port}`) if (requestUrl.pathname !== STORE_AUTH_CALLBACK_PATH) { res.statusCode = 404 @@ -145,26 +157,25 @@ export async function waitForStoreAuthCode({ const fail = (message: string) => { res.statusCode = 400 res.setHeader('Content-Type', 'text/html') - res.end(`

Authentication failed

${message}

`) + const safeMessage = message.replace(/&/g, '&').replace(//g, '>').replace(/"/g, '"') + res.end(`

Authentication failed

${safeMessage}

`) settleWithError(new AbortError(message)) } const returnedStore = searchParams.get('shop') + outputDebug(outputContent`Received OAuth callback for shop ${outputToken.raw(returnedStore ?? 'unknown')}`) + if (!returnedStore || normalizeStoreFqdn(returnedStore) !== normalizedStore) { fail('OAuth callback store does not match the requested store.') return } - if (searchParams.get('state') !== state) { + const returnedState = searchParams.get('state') + if (!returnedState || !constantTimeEqual(returnedState, state)) { fail('OAuth callback state does not match the original request.') return } - if (!verifyStoreAuthHmac(searchParams, clientSecret)) { - fail('OAuth callback could not be verified.') - return - } - const error = searchParams.get('error') if (error) { fail(`Shopify returned an OAuth error: ${error}`) @@ -177,6 +188,8 @@ export async function waitForStoreAuthCode({ return } + outputDebug(outputContent`Received authorization code ${outputToken.raw(maskToken(code))}`) + res.statusCode = 200 res.setHeader('Content-Type', 'text/html') res.end('

Authentication succeeded

You can close this window and return to the terminal.

') @@ -221,8 +234,9 @@ export async function waitForStoreAuthCode({ settleWithError(error) }) - server.listen(port, 'localhost', async () => { + server.listen(port, '127.0.0.1', async () => { isListening = true + outputDebug(outputContent`PKCE callback server listening on http://127.0.0.1:${outputToken.raw(String(port))}${outputToken.raw(STORE_AUTH_CALLBACK_PATH)}`) if (!onListening) return @@ -235,36 +249,60 @@ export async function waitForStoreAuthCode({ }) } +function constantTimeEqual(a: string, b: string): boolean { + if (a.length !== b.length) return false + return timingSafeEqual(Buffer.from(a, 'utf8'), Buffer.from(b, 'utf8')) +} + +// --- Token exchange (PKCE) --- + export async function exchangeStoreAuthCodeForToken(options: { store: string code: string - clientSecret: string + codeVerifier: string + redirectUri: string }): Promise { - const response = await fetch(`https://${options.store}/admin/oauth/access_token`, { + const endpoint = `https://${options.store}/admin/oauth/access_token` + + outputDebug(outputContent`Exchanging authorization code for token at ${outputToken.raw(endpoint)}`) + + const response = await fetch(endpoint, { method: 'POST', headers: {'Content-Type': 'application/json'}, body: JSON.stringify({ client_id: STORE_AUTH_APP_CLIENT_ID, - client_secret: options.clientSecret, code: options.code, + code_verifier: options.codeVerifier, + redirect_uri: options.redirectUri, }), }) const body = await response.text() + if (!response.ok) { + outputDebug(outputContent`Token exchange failed with HTTP ${outputToken.raw(String(response.status))}: ${outputToken.raw(body.slice(0, 300))}`) throw new AbortError( `Failed to exchange OAuth code for an access token (HTTP ${response.status}).`, body || response.statusText, ) } + let parsed: StoreTokenResponse try { - return JSON.parse(body) as StoreTokenResponse + parsed = JSON.parse(body) as StoreTokenResponse } catch { throw new AbortError('Received an invalid token response from Shopify.') } + + outputDebug( + outputContent`Token exchange succeeded: access_token=${outputToken.raw(maskToken(parsed.access_token))}, refresh_token=${outputToken.raw(parsed.refresh_token ? maskToken(parsed.refresh_token) : 'none')}, expires_in=${outputToken.raw(String(parsed.expires_in ?? 'unknown'))}s, user=${outputToken.raw(String(parsed.associated_user?.id ?? 'unknown'))} (${outputToken.raw(parsed.associated_user?.email ?? 'no email')})`, + ) + + return parsed } +// --- Orchestration --- + interface StoreAuthDependencies { openURL: typeof openURL waitForStoreAuthCode: typeof waitForStoreAuthCode @@ -281,36 +319,22 @@ const defaultStoreAuthDependencies: StoreAuthDependencies = { renderSuccess, } -// TODO: Replace this demo-only client-secret-file bootstrap with PKCE before shipping a production auth flow. -async function readClientSecretFromFile(path: string): Promise { - if (!(await fileExists(path))) { - throw new AbortError( - outputContent`Client secret file not found at ${outputToken.path(path)}. Please check the path and try again.`, - ) - } - - const contents = await readFile(path, {encoding: 'utf8'}) - const secret = contents.trim() - if (!secret) { - throw new AbortError( - outputContent`Client secret file at ${outputToken.path(path)} is empty. Please provide a valid client secret.`, - ) - } - - return secret -} - -async function createClientSecretFileBootstrap( +function createPkceBootstrap( input: StoreAuthInput, exchangeCodeForToken: typeof exchangeStoreAuthCodeForToken, -): Promise { +): StoreAuthBootstrap { const store = normalizeStoreFqdn(input.store) const scopes = parseStoreAuthScopes(input.scopes) const port = input.port ?? DEFAULT_STORE_AUTH_PORT - const clientSecret = await readClientSecretFromFile(input.clientSecretFile) const state = randomUUID() const redirectUri = storeAuthRedirectUri(port) - const authorizationUrl = buildStoreAuthUrl({store, scopes, state, redirectUri}) + const codeVerifier = generateCodeVerifier() + const codeChallenge = computeCodeChallenge(codeVerifier) + const authorizationUrl = buildStoreAuthUrl({store, scopes, state, redirectUri, codeChallenge}) + + outputDebug( + outputContent`Starting PKCE auth for ${outputToken.raw(store)} with scopes ${outputToken.raw(scopes.join(','))} (redirect_uri=${outputToken.raw(redirectUri)})`, + ) return { authorization: { @@ -320,14 +344,14 @@ async function createClientSecretFileBootstrap( port, redirectUri, authorizationUrl, + codeVerifier, }, waitForAuthCodeOptions: { store, state, - clientSecret, port, }, - exchangeCodeForToken: (code: string) => exchangeCodeForToken({store, code, clientSecret}), + exchangeCodeForToken: (code: string) => exchangeCodeForToken({store, code, codeVerifier, redirectUri}), } } @@ -335,7 +359,7 @@ export async function authenticateStoreWithApp( input: StoreAuthInput, dependencies: StoreAuthDependencies = defaultStoreAuthDependencies, ): Promise { - const bootstrap = await createClientSecretFileBootstrap(input, dependencies.exchangeStoreAuthCodeForToken) + const bootstrap = createPkceBootstrap(input, dependencies.exchangeStoreAuthCodeForToken) const { authorization: {store, scopes, redirectUri, authorizationUrl}, } = bootstrap @@ -363,19 +387,41 @@ export async function authenticateStoreWithApp( throw new AbortError('Shopify did not return associated user information for the online access token.') } + const now = Date.now() + const expiresAt = tokenResponse.expires_in ? new Date(now + tokenResponse.expires_in * 1000).toISOString() : undefined + setStoredStoreAppSession({ store, clientId: STORE_AUTH_APP_CLIENT_ID, userId, accessToken: tokenResponse.access_token, + refreshToken: tokenResponse.refresh_token, scopes: resolveGrantedScopes(tokenResponse, scopes), - acquiredAt: new Date().toISOString(), + acquiredAt: new Date(now).toISOString(), + expiresAt, + refreshTokenExpiresAt: tokenResponse.refresh_token_expires_in + ? new Date(now + tokenResponse.refresh_token_expires_in * 1000).toISOString() + : undefined, + associatedUser: tokenResponse.associated_user + ? { + id: tokenResponse.associated_user.id, + email: tokenResponse.associated_user.email, + firstName: tokenResponse.associated_user.first_name, + lastName: tokenResponse.associated_user.last_name, + accountOwner: tokenResponse.associated_user.account_owner, + } + : undefined, }) + outputDebug(outputContent`Session persisted for ${outputToken.raw(store)} (user ${outputToken.raw(userId)}, expires ${outputToken.raw(expiresAt ?? 'unknown')})`) + + const email = tokenResponse.associated_user?.email + const displayName = email ? ` as ${email}` : '' + dependencies.renderSuccess({ headline: 'Store authentication succeeded.', body: [ - `The app is now authenticated against ${store}.`, + `Authenticated${displayName} against ${store}.`, `Next step:`, {command: `shopify store execute --store ${store} --query 'query { shop { name id } }'`}, ], diff --git a/packages/cli/src/cli/services/store/config.ts b/packages/cli/src/cli/services/store/config.ts index bccdb6a05b4..94ce1f1ea5f 100644 --- a/packages/cli/src/cli/services/store/config.ts +++ b/packages/cli/src/cli/services/store/config.ts @@ -1,11 +1,16 @@ -export const STORE_AUTH_APP_CLIENT_ID = '4c6af92692662b9c95c8a47b1520aced' -export const DEFAULT_STORE_AUTH_PORT = 3458 +export const STORE_AUTH_APP_CLIENT_ID = '7e9cb568cfd431c538f36d1ad3f2b4f6' +export const DEFAULT_STORE_AUTH_PORT = 13387 export const STORE_AUTH_CALLBACK_PATH = '/auth/callback' export function storeAuthRedirectUri(port: number): string { - return `http://localhost:${port}${STORE_AUTH_CALLBACK_PATH}` + return `http://127.0.0.1:${port}${STORE_AUTH_CALLBACK_PATH}` } export function storeAuthSessionKey(store: string): string { return `${STORE_AUTH_APP_CLIENT_ID}::${store}` } + +export function maskToken(token: string): string { + if (token.length <= 10) return '***' + return `${token.slice(0, 10)}***` +} diff --git a/packages/cli/src/cli/services/store/execute.ts b/packages/cli/src/cli/services/store/execute.ts index 385b7e8428f..4cd90f74676 100644 --- a/packages/cli/src/cli/services/store/execute.ts +++ b/packages/cli/src/cli/services/store/execute.ts @@ -3,10 +3,13 @@ import {fetchApiVersions, adminUrl} from '@shopify/cli-kit/node/api/admin' import {graphqlRequest} from '@shopify/cli-kit/node/api/graphql' import {renderSingleTask, renderSuccess} from '@shopify/cli-kit/node/ui' import {AbortError} from '@shopify/cli-kit/node/error' -import {outputContent, outputResult, outputToken} from '@shopify/cli-kit/node/output' +import {outputContent, outputDebug, outputResult, outputToken} from '@shopify/cli-kit/node/output' import {fileExists, readFile, writeFile} from '@shopify/cli-kit/node/fs' +import {fetch} from '@shopify/cli-kit/node/http' import {parse} from 'graphql' -import {getStoredStoreAppSession} from './session.js' +import {getStoredStoreAppSession, setStoredStoreAppSession, clearStoredStoreAppSession, isSessionExpired} from './session.js' +import {STORE_AUTH_APP_CLIENT_ID, maskToken} from './config.js' +import type {StoredStoreAppSession} from './session.js' interface ExecuteStoreOperationInput { store: string @@ -161,16 +164,96 @@ async function writeOrOutputResult(result: unknown, outputFile?: string): Promis } } -function loadAuthenticatedStoreSession(store: string): AdminSession { - const session = getStoredStoreAppSession(store) +async function refreshStoreToken(session: StoredStoreAppSession): Promise { + if (!session.refreshToken) { + throw new AbortError( + `No refresh token stored for ${session.store}.`, + `Run ${outputToken.genericShellCommand(`shopify store auth --store ${session.store} --scopes ${session.scopes.join(',')}`).value} to re-authenticate.`, + ) + } + + const endpoint = `https://${session.store}/admin/oauth/access_token` + + outputDebug( + outputContent`Refreshing expired token for ${outputToken.raw(session.store)} (expired at ${outputToken.raw(session.expiresAt ?? 'unknown')}, refresh_token=${outputToken.raw(maskToken(session.refreshToken))})`, + ) + + const response = await fetch(endpoint, { + method: 'POST', + headers: {'Content-Type': 'application/json'}, + body: JSON.stringify({ + client_id: STORE_AUTH_APP_CLIENT_ID, + grant_type: 'refresh_token', + refresh_token: session.refreshToken, + }), + }) + + const body = await response.text() + + if (!response.ok) { + outputDebug(outputContent`Token refresh failed with HTTP ${outputToken.raw(String(response.status))}: ${outputToken.raw(body.slice(0, 300))}`) + clearStoredStoreAppSession(session.store) + throw new AbortError( + `Token refresh failed for ${session.store} (HTTP ${response.status}).`, + `Run ${outputToken.genericShellCommand(`shopify store auth --store ${session.store} --scopes ${session.scopes.join(',')}`).value} to re-authenticate.`, + ) + } + + let data: {access_token?: string; refresh_token?: string; expires_in?: number; refresh_token_expires_in?: number} + try { + data = JSON.parse(body) + } catch { + throw new AbortError('Received an invalid refresh response from Shopify.') + } + + if (!data.access_token) { + clearStoredStoreAppSession(session.store) + throw new AbortError( + `Token refresh returned an invalid response for ${session.store}.`, + `Run ${outputToken.genericShellCommand(`shopify store auth --store ${session.store} --scopes ${session.scopes.join(',')}`).value} to re-authenticate.`, + ) + } + + const now = Date.now() + const newExpiresAt = data.expires_in ? new Date(now + data.expires_in * 1000).toISOString() : session.expiresAt + + const refreshedSession: StoredStoreAppSession = { + ...session, + accessToken: data.access_token, + refreshToken: data.refresh_token ?? session.refreshToken, + expiresAt: newExpiresAt, + refreshTokenExpiresAt: data.refresh_token_expires_in + ? new Date(now + data.refresh_token_expires_in * 1000).toISOString() + : session.refreshTokenExpiresAt, + acquiredAt: new Date(now).toISOString(), + } + + outputDebug( + outputContent`Token refresh succeeded for ${outputToken.raw(session.store)}: ${outputToken.raw(maskToken(session.accessToken))} → ${outputToken.raw(maskToken(refreshedSession.accessToken))}, new expiry ${outputToken.raw(newExpiresAt ?? 'unknown')}`, + ) + + setStoredStoreAppSession(refreshedSession) + return refreshedSession +} + +async function loadAuthenticatedStoreSession(store: string): Promise { + let session = getStoredStoreAppSession(store) if (!session) { throw new AbortError( `No stored app authentication found for ${store}.`, - `Run ${outputToken.genericShellCommand(`shopify store auth --store ${store} --scopes --client-secret-file `).value} first.`, + `Run ${outputToken.genericShellCommand(`shopify store auth --store ${store} --scopes `).value} first.`, ) } + outputDebug( + outputContent`Loaded stored session for ${outputToken.raw(store)}: token=${outputToken.raw(maskToken(session.accessToken))}, expires=${outputToken.raw(session.expiresAt ?? 'unknown')}`, + ) + + if (isSessionExpired(session)) { + session = await refreshStoreToken(session) + } + return { token: session.accessToken, storeFqdn: session.store, @@ -201,7 +284,7 @@ export async function executeStoreOperation(input: ExecuteStoreOperationInput): const {adminSession, version} = await renderSingleTask({ title: outputContent`Authenticating`, task: async (): Promise<{adminSession: AdminSession; version: string}> => { - const adminSession = loadAuthenticatedStoreSession(store) + const adminSession = await loadAuthenticatedStoreSession(store) const version = await resolveApiVersion({adminSession, userSpecifiedVersion}) return {adminSession, version} }, @@ -227,9 +310,10 @@ export async function executeStoreOperation(input: ExecuteStoreOperationInput): await writeOrOutputResult(result, outputFile) } catch (error) { if (isGraphQLClientError(error) && error.response.status === 401) { + clearStoredStoreAppSession(store) throw new AbortError( `Stored app authentication for ${store} is no longer valid.`, - `Re-run ${outputToken.genericShellCommand(`shopify store auth --store ${store} --scopes --client-secret-file `).value}.`, + `Run ${outputToken.genericShellCommand(`shopify store auth --store ${store} --scopes `).value} to re-authenticate.`, ) } diff --git a/packages/cli/src/cli/services/store/session.test.ts b/packages/cli/src/cli/services/store/session.test.ts index 97d108d6abe..2e8ff06e4db 100644 --- a/packages/cli/src/cli/services/store/session.test.ts +++ b/packages/cli/src/cli/services/store/session.test.ts @@ -4,6 +4,7 @@ import { clearStoredStoreAppSession, getStoredStoreAppSession, setStoredStoreAppSession, + isSessionExpired, type StoredStoreAppSession, } from './session.js' @@ -26,7 +27,7 @@ function inMemoryStorage() { function buildSession(overrides: Partial = {}): StoredStoreAppSession { return { store: 'shop.myshopify.com', - clientId: '4c6af92692662b9c95c8a47b1520aced', + clientId: 'b16de5d7ba3e2e22279a38c22ef025a0', userId: '42', accessToken: 'token-1', scopes: ['read_products'], @@ -66,7 +67,7 @@ describe('store session storage', () => { test('returns undefined when the current user session is missing from the bucket', () => { const storage = inMemoryStorage() - storage.set('4c6af92692662b9c95c8a47b1520aced::shop.myshopify.com', { + storage.set('b16de5d7ba3e2e22279a38c22ef025a0::shop.myshopify.com', { currentUserId: '999', sessionsByUserId: { '42': buildSession(), @@ -76,3 +77,29 @@ describe('store session storage', () => { expect(getStoredStoreAppSession('shop.myshopify.com', storage as any)).toBeUndefined() }) }) + +describe('isSessionExpired', () => { + test('returns false when expiresAt is not set', () => { + expect(isSessionExpired(buildSession())).toBe(false) + }) + + test('returns false when token is still valid', () => { + const future = new Date(Date.now() + 60 * 60 * 1000).toISOString() + expect(isSessionExpired(buildSession({expiresAt: future}))).toBe(false) + }) + + test('returns true when token is expired', () => { + const past = new Date(Date.now() - 60 * 1000).toISOString() + expect(isSessionExpired(buildSession({expiresAt: past}))).toBe(true) + }) + + test('returns true within the 4-minute expiry margin', () => { + const almostExpired = new Date(Date.now() + 3 * 60 * 1000).toISOString() + expect(isSessionExpired(buildSession({expiresAt: almostExpired}))).toBe(true) + }) + + test('returns false just outside the 4-minute expiry margin', () => { + const safelyValid = new Date(Date.now() + 5 * 60 * 1000).toISOString() + expect(isSessionExpired(buildSession({expiresAt: safelyValid}))).toBe(false) + }) +}) diff --git a/packages/cli/src/cli/services/store/session.ts b/packages/cli/src/cli/services/store/session.ts index 38b49440e14..d8c8eb78915 100644 --- a/packages/cli/src/cli/services/store/session.ts +++ b/packages/cli/src/cli/services/store/session.ts @@ -6,8 +6,18 @@ export interface StoredStoreAppSession { clientId: string userId: string accessToken: string + refreshToken?: string scopes: string[] acquiredAt: string + expiresAt?: string + refreshTokenExpiresAt?: string + associatedUser?: { + id: number + email?: string + firstName?: string + lastName?: string + accountOwner?: boolean + } } interface StoredStoreAppSessionBucket { @@ -21,8 +31,7 @@ interface StoreSessionSchema { let _storeSessionStorage: LocalStorage | undefined -// TODO: Revisit store token persistence when the auth flow moves to PKCE. -// The current model is meant to keep the demo-oriented online-token flow coherent, not final. +// Per-store, per-user session storage for PKCE online tokens. function storeSessionStorage() { _storeSessionStorage ??= new LocalStorage({projectName: 'shopify-cli-store'}) return _storeSessionStorage @@ -62,3 +71,10 @@ export function clearStoredStoreAppSession( ): void { storage.delete(storeAuthSessionKey(store)) } + +const EXPIRY_MARGIN_MS = 4 * 60 * 1000 + +export function isSessionExpired(session: StoredStoreAppSession): boolean { + if (!session.expiresAt) return false + return new Date(session.expiresAt).getTime() - EXPIRY_MARGIN_MS < Date.now() +}