diff --git a/.changeset/name-skipped-store-auth-session.md b/.changeset/name-skipped-store-auth-session.md new file mode 100644 index 00000000000..322b533865f --- /dev/null +++ b/.changeset/name-skipped-store-auth-session.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': patch +--- + +Name the skipped stored store auth session when theme device authentication fails. The reason now appears in the error's next steps, together with a recovery step where one applies. diff --git a/packages/theme/src/cli/utilities/theme-command.test.ts b/packages/theme/src/cli/utilities/theme-command.test.ts index 6434fb972c7..701c25b08e0 100644 --- a/packages/theme/src/cli/utilities/theme-command.test.ts +++ b/packages/theme/src/cli/utilities/theme-command.test.ts @@ -2,6 +2,7 @@ import ThemeCommand, {RequiredFlags} from './theme-command.js' import {ensureThemeStore} from './theme-store.js' import {describe, vi, expect, test, beforeEach} from 'vitest' import {Config, Flags} from '@oclif/core' +import {AbortError} from '@shopify/cli-kit/node/error' import {AdminSession, ensureAuthenticatedThemes} from '@shopify/cli-kit/node/session' import { getCurrentStoredStoreAppSession, @@ -376,6 +377,192 @@ describe('ThemeCommand', () => { }) }) + test('reuses an unscoped preview store session for a command that declares store auth scopes', async () => { + vi.mocked(getCurrentStoredStoreAppSession).mockReturnValue({ + store: 'test-store.myshopify.com', + clientId: 'store-auth-client-id', + userId: 'preview:123', + accessToken: 'shpat_preview_token', + scopes: [], + acquiredAt: '2026-06-08T11:00:00.000Z', + kind: 'preview', + preview: {shopId: '1', name: 'Preview Store', createdAt: '2026-06-08T11:00:00.000Z'}, + }) + + await CommandConfig.load() + const command = new TestScopedThemeCommand([], CommandConfig) + + await command.run() + + expect(ensureAuthenticatedThemes).not.toHaveBeenCalled() + expect(command.commandCalls[0]).toMatchObject({ + session: {token: 'shpat_preview_token', storeFqdn: 'test-store.myshopify.com'}, + }) + }) + + test('attaches the skipped store auth session reason to a device authentication error', async () => { + vi.mocked(getCurrentStoredStoreAppSession).mockReturnValue({ + store: 'test-store.myshopify.com', + clientId: 'store-auth-client-id', + userId: 'preview:123', + accessToken: 'shpat_preview_token', + scopes: ['read_themes'], + acquiredAt: '2026-06-08T11:00:00.000Z', + }) + const abortError = new AbortError('Failed to authenticate.') + vi.mocked(ensureAuthenticatedThemes).mockRejectedValueOnce(abortError) + + await CommandConfig.load() + const command = new TestThemeCommand([], CommandConfig) + + const error = await command.run().catch((thrown: unknown) => thrown) + + expect(error).toBe(abortError) + expect((error as AbortError).nextSteps).toContainEqual([ + 'The CLI found a stored store auth session for test-store.myshopify.com, but did not use it: it is a standard session and this command only reuses preview store sessions.', + ]) + expect((error as AbortError).nextSteps).toContainEqual(['Pass a Theme Access password with `--password`.']) + expect((error as AbortError).nextSteps).not.toContainEqual([ + 'Pass a Theme Access password with `--password`, or run `theme pull` or `theme push`, which reuse standard store auth sessions.', + ]) + }) + + test('omits the store auth advice when an expired preview store session is skipped', async () => { + vi.mocked(getCurrentStoredStoreAppSession).mockReturnValue({ + store: 'test-store.myshopify.com', + clientId: 'store-auth-client-id', + userId: 'preview:123', + accessToken: 'shpat_preview_token', + scopes: [], + expiresAt: '2020-01-01T00:00:00.000Z', + acquiredAt: '2026-06-08T11:00:00.000Z', + kind: 'preview', + preview: {shopId: '1', name: 'Preview Store', createdAt: '2026-06-08T11:00:00.000Z'}, + }) + vi.mocked(ensureAuthenticatedThemes).mockRejectedValueOnce(new AbortError('Failed to authenticate.')) + + await CommandConfig.load() + const command = new TestThemeCommand([], CommandConfig) + + const error = await command.run().catch((thrown: unknown) => thrown) + + expect(error).toBeInstanceOf(AbortError) + expect((error as AbortError).nextSteps).toContainEqual([ + 'The CLI found a stored store auth session for test-store.myshopify.com, but did not use it: it expired at 2020-01-01T00:00:00.000Z.', + ]) + expect((error as AbortError).nextSteps).not.toContainEqual([ + 'Run `shopify store auth --store test-store.myshopify.com` to store a fresh session.', + ]) + }) + + test('propagates a device authentication error unchanged when no stored session was skipped', async () => { + vi.mocked(ensureAuthenticatedThemes).mockRejectedValueOnce(new AbortError('Failed to authenticate.')) + + await CommandConfig.load() + const command = new TestThemeCommand([], CommandConfig) + + const error = await command.run().catch((thrown: unknown) => thrown) + + expect(error).toBeInstanceOf(AbortError) + expect((error as AbortError).nextSteps).toBeUndefined() + }) + + test('attaches the missing-scopes reason and scope advice to a device authentication error', async () => { + vi.mocked(getCurrentStoredStoreAppSession).mockReturnValue({ + store: 'test-store.myshopify.com', + clientId: 'store-auth-client-id', + userId: 'store-auth-user', + accessToken: 'shpat_standard_token', + scopes: ['read_products'], + acquiredAt: '2026-06-08T11:00:00.000Z', + }) + vi.mocked(ensureAuthenticatedThemes).mockRejectedValueOnce(new AbortError('Failed to authenticate.')) + + await CommandConfig.load() + const command = new TestScopedThemeCommand([], CommandConfig) + + const error = await command.run().catch((thrown: unknown) => thrown) + + expect(error).toBeInstanceOf(AbortError) + expect((error as AbortError).nextSteps).toContainEqual([ + 'The CLI found a stored store auth session for test-store.myshopify.com, but did not use it: it is missing required scopes (has: read_products; needs: read_themes).', + ]) + expect((error as AbortError).nextSteps).toContainEqual([ + 'Run `shopify store auth --store test-store.myshopify.com --scopes read_themes` to grant the required scopes.', + ]) + }) + + test('attaches the store auth advice when an expired standard session is skipped', async () => { + vi.mocked(getCurrentStoredStoreAppSession).mockReturnValue({ + store: 'test-store.myshopify.com', + clientId: 'store-auth-client-id', + userId: 'store-auth-user', + accessToken: 'shpat_standard_token', + scopes: ['read_themes'], + expiresAt: '2020-01-01T00:00:00.000Z', + acquiredAt: '2026-06-08T11:00:00.000Z', + }) + vi.mocked(ensureAuthenticatedThemes).mockRejectedValueOnce(new AbortError('Failed to authenticate.')) + + await CommandConfig.load() + const command = new TestThemeCommand([], CommandConfig) + + const error = await command.run().catch((thrown: unknown) => thrown) + + expect((error as AbortError).nextSteps).toContainEqual([ + 'The CLI found a stored store auth session for test-store.myshopify.com, but did not use it: it expired at 2020-01-01T00:00:00.000Z.', + ]) + expect((error as AbortError).nextSteps).toContainEqual([ + 'Run `shopify store auth --store test-store.myshopify.com` to store a fresh session.', + ]) + }) + + test('names the stored expiry when the expiry timestamp is invalid', async () => { + vi.mocked(getCurrentStoredStoreAppSession).mockReturnValue({ + store: 'test-store.myshopify.com', + clientId: 'store-auth-client-id', + userId: 'store-auth-user', + accessToken: 'shpat_standard_token', + scopes: ['read_themes'], + expiresAt: 'not-a-date', + acquiredAt: '2026-06-08T11:00:00.000Z', + }) + vi.mocked(ensureAuthenticatedThemes).mockRejectedValueOnce(new AbortError('Failed to authenticate.')) + + await CommandConfig.load() + const command = new TestThemeCommand([], CommandConfig) + + const error = await command.run().catch((thrown: unknown) => thrown) + + expect((error as AbortError).nextSteps).toContainEqual([ + 'The CLI found a stored store auth session for test-store.myshopify.com, but did not use it: it expired at not-a-date.', + ]) + expect((error as AbortError).nextSteps).toContainEqual([ + 'Run `shopify store auth --store test-store.myshopify.com` to store a fresh session.', + ]) + }) + + test('propagates a non-AbortError unchanged when a stored session was skipped', async () => { + vi.mocked(getCurrentStoredStoreAppSession).mockReturnValue({ + store: 'test-store.myshopify.com', + clientId: 'store-auth-client-id', + userId: 'store-auth-user', + accessToken: 'shpat_standard_token', + scopes: ['read_themes'], + acquiredAt: '2026-06-08T11:00:00.000Z', + }) + const failure = new Error('Network failed.') + vi.mocked(ensureAuthenticatedThemes).mockRejectedValueOnce(failure) + + await CommandConfig.load() + const command = new TestThemeCommand([], CommandConfig) + + const error = await command.run().catch((thrown: unknown) => thrown) + + expect(error).toBe(failure) + expect((error as {nextSteps?: unknown}).nextSteps).toBeUndefined() + }) + test('treats a matching write scope in the stored session as satisfying a required read scope', async () => { vi.mocked(getCurrentStoredStoreAppSession).mockReturnValue({ store: 'test-store.myshopify.com', @@ -1233,6 +1420,43 @@ describe('ThemeCommand', () => { expect(ensureAuthenticatedThemes).not.toHaveBeenCalled() }) + test('multiple environment commands reuse the pre-pass result instead of re-reading a skipped session', async () => { + vi.mocked(loadEnvironment) + .mockResolvedValueOnce({store: 'store1.myshopify.com'}) + .mockResolvedValueOnce({store: 'store2.myshopify.com', password: 'password2'}) + vi.mocked(listCurrentStoredStoreAppSessions).mockReturnValue([ + { + store: 'store1.myshopify.com', + clientId: 'store-auth-client-id', + userId: 'store-auth-user', + accessToken: 'shpat_standard_token', + scopes: ['read_themes'], + acquiredAt: '2026-06-08T11:00:00.000Z', + }, + ]) + vi.mocked(renderConfirmationPrompt).mockResolvedValue(true) + vi.mocked(renderConcurrent).mockImplementation(async ({processes}) => { + for (const process of processes) { + // eslint-disable-next-line no-await-in-loop + await process.action({} as Writable, {} as Writable, {} as any) + } + }) + vi.mocked(ensureThemeStore).mockImplementation((options: any) => options.store) + + await CommandConfig.load() + const command = new TestThemeCommand( + ['--environment', 'preview', '--environment', 'another-preview'], + CommandConfig, + ) + + await command.run() + + expect(listCurrentStoredStoreAppSessions).toHaveBeenCalledOnce() + expect(getCurrentStoredStoreAppSession).not.toHaveBeenCalled() + expect(ensureAuthenticatedThemes).toHaveBeenCalledWith('store1.myshopify.com', undefined) + expect(ensureAuthenticatedThemes).toHaveBeenCalledWith('store2.myshopify.com', 'password2') + }) + test('commands will only create a session object if the password flag is supported', async () => { // Given vi.mocked(loadEnvironment) diff --git a/packages/theme/src/cli/utilities/theme-command.ts b/packages/theme/src/cli/utilities/theme-command.ts index ef062e15d9c..248b94dfad7 100644 --- a/packages/theme/src/cli/utilities/theme-command.ts +++ b/packages/theme/src/cli/utilities/theme-command.ts @@ -37,7 +37,7 @@ interface ValidEnvironment { environment: EnvironmentName flags: FlagValues requiresAuth: boolean - storeAuthSession?: AdminSession + storeAuthResult?: StoreAuthSessionResult } type EnvironmentName = string /** @@ -56,6 +56,16 @@ type EnvironmentName = string */ export type RequiredFlags = (string | string[])[] | null +interface SkippedStoreAuthSession { + reason: string + advice?: string +} + +interface StoreAuthSessionResult { + session?: AdminSession + skipped?: SkippedStoreAuthSession +} + export default abstract class ThemeCommand extends Command { static baseFlags = authAliasFlag @@ -209,25 +219,25 @@ export default abstract class ThemeCommand extends Command { const storeAuthSessionsByStore = requiresAuth ? this.storeAuthSessionsForTheme(Array.from(environmentMap.values()).map(({validationFlags}) => validationFlags)) - : new Map() + : new Map() const entriesWithStoreAuthSessions = Array.from(environmentMap.entries()).map( ([environmentName, {flags, validationFlags}]) => ({ environmentName, flags, validationFlags, - storeAuthSession: this.storeAuthSessionFromCache(validationFlags, storeAuthSessionsByStore), + storeAuthResult: this.storeAuthSessionFromCache(validationFlags, storeAuthSessionsByStore), }), ) - for (const {environmentName, flags, validationFlags, storeAuthSession} of entriesWithStoreAuthSessions) { - const validationResult = this.validConfig(validationFlags, requiredFlags, environmentName, storeAuthSession) + for (const {environmentName, flags, validationFlags, storeAuthResult} of entriesWithStoreAuthSessions) { + const validationResult = this.validConfig(validationFlags, requiredFlags, environmentName, storeAuthResult) if (validationResult !== true) { const missingFlagsText = validationResult.join(', ') invalid.push({environment: environmentName, reason: `Missing flags: ${missingFlagsText}`}) continue } - valid.push({environment: environmentName, flags, requiresAuth, storeAuthSession}) + valid.push({environment: environmentName, flags, requiresAuth, storeAuthResult}) } return {valid, invalid} @@ -303,13 +313,13 @@ export default abstract class ThemeCommand extends Command { for (const runGroup of runGroups) { // eslint-disable-next-line no-await-in-loop await renderConcurrent({ - processes: runGroup.map(({environment, flags, requiresAuth, storeAuthSession}) => ({ + processes: runGroup.map(({environment, flags, requiresAuth, storeAuthResult}) => ({ prefix: environment, action: async (stdout: Writable, stderr: Writable, _signal) => { try { const store = flags.store as string await useThemeStoreContext(store, async () => { - const session = requiresAuth ? await this.createSession(flags, storeAuthSession) : undefined + const session = requiresAuth ? await this.createSession(flags, storeAuthResult) : undefined const commandName = this.constructor.name.toLowerCase() recordEvent(`theme-command:${commandName}:multi-env:authenticated`) @@ -359,31 +369,42 @@ export default abstract class ThemeCommand extends Command { * @param flags - The environment flags containing store and password * @returns The unauthenticated session object */ - private async createSession(flags: FlagValues, storeAuthSession?: AdminSession) { + private async createSession(flags: FlagValues, storeAuthResult?: StoreAuthSessionResult) { const store = ensureThemeStore({store: flags.store as string | undefined}) const password = flags.password as string | undefined - const session = password - ? await ensureAuthenticatedThemes(store, password) - : (storeAuthSession ?? - (await this.storeAuthSessionForTheme({store})) ?? - (await ensureAuthenticatedThemes(store, password))) - - return session + if (password) return ensureAuthenticatedThemes(store, password) + + const result = storeAuthResult ?? (await this.storeAuthSessionForTheme({store})) + if (result.session) return result.session + + try { + return await ensureAuthenticatedThemes(store, password) + } catch (error) { + const skipped = result.skipped + if (!(error instanceof AbortError) || !skipped) throw error + + error.nextSteps = [ + ...(error.nextSteps ?? []), + [`The CLI found a stored store auth session for ${store}, but did not use it: ${skipped.reason}`], + ...(skipped.advice === undefined ? [] : [[skipped.advice]]), + ] + throw error + } } - private async storeAuthSessionForTheme(flags: FlagValues): Promise { + private async storeAuthSessionForTheme(flags: FlagValues): Promise { const store = typeof flags.store === 'string' ? flags.store : undefined const password = flags.password - if (!store || password) return undefined + if (!store || password) return {} const storeFqdn = normalizeStoreFqdn(store) const storedSession = getCurrentStoredStoreAppSession(storeFqdn) - if (!storedSession) return undefined + if (!storedSession) return {} return this.adminSessionFromStoreAuthSession(storedSession, storeFqdn, this.storeAuthScopes()) } - private storeAuthSessionsForTheme(flagsList: FlagValues[]): Map { + private storeAuthSessionsForTheme(flagsList: FlagValues[]): Map { const requiredScopes = this.storeAuthScopes() const stores = new Set( @@ -399,17 +420,16 @@ export default abstract class ThemeCommand extends Command { const storeFqdn = normalizeStoreFqdn(storedSession.store) if (!stores.has(storeFqdn)) return undefined - const session = this.adminSessionFromStoreAuthSession(storedSession, storeFqdn, requiredScopes) - return session ? ([storeFqdn, session] as const) : undefined + return [storeFqdn, this.adminSessionFromStoreAuthSession(storedSession, storeFqdn, requiredScopes)] as const }) - .filter((entry): entry is readonly [string, AdminSession] => entry !== undefined), + .filter((entry): entry is readonly [string, StoreAuthSessionResult] => entry !== undefined), ) } private storeAuthSessionFromCache( flags: FlagValues, - storeAuthSessionsByStore: Map, - ): AdminSession | undefined { + storeAuthSessionsByStore: Map, + ): StoreAuthSessionResult | undefined { const store = typeof flags.store === 'string' ? flags.store : undefined const password = flags.password if (!store || password) return undefined @@ -421,30 +441,41 @@ export default abstract class ThemeCommand extends Command { storedSession: StoredStoreAppSession, storeFqdn: string, requiredScopes: string[] | undefined, - ): AdminSession | undefined { + ): StoreAuthSessionResult { + const isPreviewSession = storedSession.kind === 'preview' + if (isSessionExpired(storedSession)) { - outputDebug( - `Ignoring stored store auth session for ${storeFqdn}: it expired at ${storedSession.expiresAt ?? 'unknown'}.`, - ) - return undefined + const reason = `it expired at ${storedSession.expiresAt ?? 'an unknown time'}.` + outputDebug(`Ignoring stored store auth session for ${storeFqdn}: ${reason}`) + return { + skipped: { + reason, + // A preview store has no account to log in as, so `store auth` cannot replace its session. + advice: isPreviewSession + ? undefined + : `Run \`shopify store auth --store ${storeFqdn}\` to store a fresh session.`, + }, + } } - const isPreviewSession = storedSession.kind === 'preview' if (!isPreviewSession) { if (!requiredScopes) { - outputDebug( - `Ignoring stored store auth session for ${storeFqdn}: it is a standard session and this command only reuses preview store sessions.`, - ) - return undefined + const reason = 'it is a standard session and this command only reuses preview store sessions.' + outputDebug(`Ignoring stored store auth session for ${storeFqdn}: ${reason}`) + return {skipped: {reason, advice: 'Pass a Theme Access password with `--password`.'}} } if (!this.hasRequiredStoreAuthScopes(storedSession.scopes, requiredScopes)) { - outputDebug( - `Ignoring stored store auth session for ${storeFqdn}: it is missing required scopes (has: ${storedSession.scopes.join( - ', ', - )}; needs: ${requiredScopes.join(', ')}).`, - ) - return undefined + const reason = `it is missing required scopes (has: ${storedSession.scopes.join( + ', ', + )}; needs: ${requiredScopes.join(', ')}).` + outputDebug(`Ignoring stored store auth session for ${storeFqdn}: ${reason}`) + return { + skipped: { + reason, + advice: `Run \`shopify store auth --store ${storeFqdn} --scopes ${requiredScopes.join(',')}\` to grant the required scopes.`, + }, + } } } @@ -453,8 +484,10 @@ export default abstract class ThemeCommand extends Command { setLastSeenUserId(storedSession.userId) return { - token: storedSession.accessToken, - storeFqdn, + session: { + token: storedSession.accessToken, + storeFqdn, + }, } } @@ -487,13 +520,13 @@ export default abstract class ThemeCommand extends Command { environmentFlags: FlagValues, requiredFlags: Exclude, environmentName: string, - storeAuthSession?: AdminSession, + storeAuthResult?: StoreAuthSessionResult, ): string[] | true { const missingFlags = requiredFlags .filter((flag) => Array.isArray(flag) - ? !flag.some((flag) => this.hasRequiredFlag(environmentFlags, flag, storeAuthSession)) - : !this.hasRequiredFlag(environmentFlags, flag, storeAuthSession), + ? !flag.some((flag) => this.hasRequiredFlag(environmentFlags, flag, storeAuthResult)) + : !this.hasRequiredFlag(environmentFlags, flag, storeAuthResult), ) .map((flag) => (Array.isArray(flag) ? flag.join(' or ') : flag)) @@ -510,8 +543,12 @@ export default abstract class ThemeCommand extends Command { return true } - private hasRequiredFlag(environmentFlags: FlagValues, flag: string, storeAuthSession?: AdminSession): boolean { - if (flag === 'password' && storeAuthSession) return true + private hasRequiredFlag( + environmentFlags: FlagValues, + flag: string, + storeAuthResult?: StoreAuthSessionResult, + ): boolean { + if (flag === 'password' && storeAuthResult?.session) return true return Boolean(environmentFlags[flag]) }