-
Notifications
You must be signed in to change notification settings - Fork 262
Recover on store info after a preview store has been claimed
#7986
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
496e69e
Recover on `store info` after a preview store has been claimed
amcaplan 5338329
Don't clear the stale preview session on `store info` re-auth
amcaplan 38f40b1
Simplify store-auth recovery next-steps text
amcaplan 5c5e221
Make PreviewStoreRequestError extend AbortError
amcaplan a46221a
Trim overlong comment in store info's preview-claim handling
amcaplan 79a2f3f
Don't re-list preview store scopes in the post-claim re-auth message
amcaplan e8df251
Fix the same scope-list-dumping bug in store execute
amcaplan 4713727
Reuse reauthScopesFor in store info's preview-claim path
amcaplan 9426d29
Simplify scope-placeholder logic and fix a related clearing gap
amcaplan dc77193
Drop dead scopes parameter from storeAuthCommandNextSteps
amcaplan fff33f2
Give the 3 stored-auth recovery cases distinct, consistent messages
amcaplan e56fd8d
Say 'to authenticate' (not 're-authenticate') when there was no prior…
amcaplan 268a8fb
Stop exporting UNKNOWN_SCOPES_PLACEHOLDER (knip: unused export)
amcaplan 004e9f0
Add dedicated unit tests for recovery.ts
amcaplan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@shopify/store': patch | ||
| --- | ||
|
|
||
| Fix `store info` failing with an unhelpful error on a preview store after it has been claimed; it now prompts to run `store auth` to re-authenticate |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
165 changes: 165 additions & 0 deletions
165
packages/store/src/cli/services/store/auth/recovery.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,165 @@ | ||
| import { | ||
| throwStoredStoreAuthError, | ||
| throwReauthenticateStoreAuthError, | ||
| throwStoredAuthInvalidError, | ||
| retryStoreAuthWithPermanentDomainError, | ||
| } from './recovery.js' | ||
| import {STORE_AUTH_APP_CLIENT_ID} from './config.js' | ||
| import {AbortError} from '@shopify/cli-kit/node/error' | ||
| import {describe, expect, test} from 'vitest' | ||
| import type {StoredStoreAppSession} from '@shopify/cli-kit/node/store-auth-session' | ||
|
|
||
| const SHOP = 'shop.myshopify.com' | ||
|
|
||
| function standardSession(overrides: Partial<StoredStoreAppSession> = {}): StoredStoreAppSession { | ||
| return { | ||
| store: SHOP, | ||
| clientId: STORE_AUTH_APP_CLIENT_ID, | ||
| userId: '42', | ||
| accessToken: 'token', | ||
| scopes: ['read_products', 'write_orders'], | ||
| acquiredAt: '2026-03-27T00:00:00.000Z', | ||
| ...overrides, | ||
| } | ||
| } | ||
|
|
||
| function previewSession(overrides: Partial<StoredStoreAppSession> = {}): StoredStoreAppSession { | ||
| return { | ||
| ...standardSession({ | ||
| userId: 'preview:placeholder-uuid', | ||
| // The full preapproved catalog is much larger in practice; a couple of entries are enough | ||
| // to prove the placeholder is used instead of these. | ||
| scopes: ['read_products', 'write_products', 'read_themes'], | ||
| }), | ||
| kind: 'preview', | ||
| preview: { | ||
| shopId: '123', | ||
| name: 'Lavender Candles', | ||
| createdAt: '2026-03-27T00:00:00.000Z', | ||
| }, | ||
| ...overrides, | ||
| } | ||
| } | ||
|
|
||
| describe('throwStoredStoreAuthError', () => { | ||
| test('reports no stored auth and prompts to authenticate (not re-authenticate) with a scopes placeholder', () => { | ||
| let captured: AbortError | undefined | ||
| try { | ||
| throwStoredStoreAuthError(SHOP) | ||
| // eslint-disable-next-line no-catch-all/no-catch-all | ||
| } catch (error) { | ||
| captured = error as AbortError | ||
| } | ||
|
|
||
| expect(captured).toMatchObject({ | ||
| message: `No stored app authentication found for ${SHOP}.`, | ||
| nextSteps: [ | ||
| ['Run', {command: `shopify store auth --store ${SHOP} --scopes <comma-separated-scopes>`}, 'to authenticate'], | ||
| ], | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| describe('throwReauthenticateStoreAuthError', () => { | ||
| test('suggests the real scopes for a standard session', () => { | ||
| let captured: AbortError | undefined | ||
| try { | ||
| throwReauthenticateStoreAuthError('Custom message.', standardSession()) | ||
| // eslint-disable-next-line no-catch-all/no-catch-all | ||
| } catch (error) { | ||
| captured = error as AbortError | ||
| } | ||
|
|
||
| expect(captured).toMatchObject({ | ||
| message: 'Custom message.', | ||
| nextSteps: [ | ||
| [ | ||
| 'Run', | ||
| {command: `shopify store auth --store ${SHOP} --scopes read_products,write_orders`}, | ||
| 'to re-authenticate', | ||
| ], | ||
| ], | ||
| }) | ||
| }) | ||
|
|
||
| test('suggests a scopes placeholder for a preview session instead of its preapproved catalog', () => { | ||
| let captured: AbortError | undefined | ||
| try { | ||
| throwReauthenticateStoreAuthError('Custom message.', previewSession()) | ||
| // eslint-disable-next-line no-catch-all/no-catch-all | ||
| } catch (error) { | ||
| captured = error as AbortError | ||
| } | ||
|
|
||
| expect(captured).toMatchObject({ | ||
| message: 'Custom message.', | ||
| nextSteps: [ | ||
| [ | ||
| 'Run', | ||
| {command: `shopify store auth --store ${SHOP} --scopes <comma-separated-scopes>`}, | ||
| 'to re-authenticate', | ||
| ], | ||
| ], | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| describe('throwStoredAuthInvalidError', () => { | ||
| test('uses the generic invalid-auth message and real scopes for a standard session', () => { | ||
| let captured: AbortError | undefined | ||
| try { | ||
| throwStoredAuthInvalidError(standardSession()) | ||
| // eslint-disable-next-line no-catch-all/no-catch-all | ||
| } catch (error) { | ||
| captured = error as AbortError | ||
| } | ||
|
|
||
| expect(captured).toMatchObject({ | ||
| message: `Stored app authentication for ${SHOP} is no longer valid.`, | ||
| nextSteps: [ | ||
| [ | ||
| 'Run', | ||
| {command: `shopify store auth --store ${SHOP} --scopes read_products,write_orders`}, | ||
| 'to re-authenticate', | ||
| ], | ||
| ], | ||
| }) | ||
| }) | ||
|
|
||
| test('flags a likely claim and suggests a scopes placeholder for a preview session', () => { | ||
| let captured: AbortError | undefined | ||
| try { | ||
| throwStoredAuthInvalidError(previewSession()) | ||
| // eslint-disable-next-line no-catch-all/no-catch-all | ||
| } catch (error) { | ||
| captured = error as AbortError | ||
| } | ||
|
|
||
| expect(captured).toMatchObject({ | ||
| message: `The preview store ${SHOP} has likely been claimed, so its stored authentication is no longer valid.`, | ||
| nextSteps: [ | ||
| [ | ||
| 'Run', | ||
| {command: `shopify store auth --store ${SHOP} --scopes <comma-separated-scopes>`}, | ||
| 'to re-authenticate', | ||
| ], | ||
| ], | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| describe('retryStoreAuthWithPermanentDomainError', () => { | ||
| test('returns (rather than throws) an AbortError pointing at the permanent domain with a scopes placeholder', () => { | ||
| const error = retryStoreAuthWithPermanentDomainError('permanent-shop.myshopify.com') | ||
|
|
||
| expect(error).toBeInstanceOf(AbortError) | ||
| expect(error).toMatchObject({ | ||
| message: 'OAuth callback store does not match the requested store.', | ||
| tryMessage: | ||
| 'Shopify returned permanent-shop.myshopify.com during authentication. Re-run using the permanent store domain:', | ||
| nextSteps: [ | ||
| [{command: 'shopify store auth --store permanent-shop.myshopify.com --scopes <comma-separated-scopes>'}], | ||
| ], | ||
| }) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,30 +1,62 @@ | ||
| import {AbortError} from '@shopify/cli-kit/node/error' | ||
| import type {StoredStoreAppSession} from '@shopify/cli-kit/node/store-auth-session' | ||
|
|
||
| const UNKNOWN_SCOPES_PLACEHOLDER = '<comma-separated-scopes>' | ||
|
|
||
| function storeAuthCommand(store: string, scopes: string): {command: string} { | ||
| return {command: `shopify store auth --store ${store} --scopes ${scopes}`} | ||
| } | ||
|
|
||
| function storeAuthCommandNextSteps(store: string, scopes: string) { | ||
| return [[storeAuthCommand(store, scopes)]] | ||
| function storeAuthCommandNextStepsWithUnknownScopes(store: string) { | ||
| return [[storeAuthCommand(store, UNKNOWN_SCOPES_PLACEHOLDER)]] | ||
| } | ||
|
|
||
| function storeAuthCommandNextStepsWithPurpose(store: string, scopes: string, purpose: string) { | ||
| return [['Run', storeAuthCommand(store, scopes), purpose]] | ||
| } | ||
|
|
||
| // Preview-store sessions are preapproved for a large, fixed scope catalog (often 30+ scopes). | ||
| // Suggesting the user re-request all of them encourages over-scoping, so they get the same | ||
| // placeholder as the "no stored auth" case and choose deliberately instead. | ||
| function reauthScopesFor(session: StoredStoreAppSession): string { | ||
| return session.kind === 'preview' ? UNKNOWN_SCOPES_PLACEHOLDER : session.scopes.join(',') | ||
| } | ||
|
|
||
| export function throwStoredStoreAuthError(store: string): never { | ||
| throw new AbortError( | ||
| `No stored app authentication found for ${store}.`, | ||
| 'To create stored auth for this store, run:', | ||
| storeAuthCommandNextSteps(store, '<comma-separated-scopes>'), | ||
| undefined, | ||
| storeAuthCommandNextStepsWithPurpose(store, UNKNOWN_SCOPES_PLACEHOLDER, 'to authenticate'), | ||
| ) | ||
| } | ||
|
|
||
| export function throwReauthenticateStoreAuthError(message: string, store: string, scopes: string): never { | ||
| throw new AbortError(message, 'To re-authenticate, run:', storeAuthCommandNextSteps(store, scopes)) | ||
| export function throwReauthenticateStoreAuthError(message: string, session: StoredStoreAppSession): never { | ||
| throw new AbortError( | ||
| message, | ||
| undefined, | ||
| storeAuthCommandNextStepsWithPurpose(session.store, reauthScopesFor(session), 'to re-authenticate'), | ||
| ) | ||
| } | ||
|
|
||
| // A preview store's local session has no way to know it was claimed through the browser claim | ||
| // flow; a 401/404 the first time the stale session is used again is the only signal. Surfacing | ||
| // that possibility is more useful than the generic "no longer valid" message a standard session | ||
| // gets, so every call site that detects an invalid stored session (regardless of which API it | ||
| // hit) should go through here instead of writing its own message. | ||
| export function throwStoredAuthInvalidError(session: StoredStoreAppSession): never { | ||
| const message = | ||
| session.kind === 'preview' | ||
| ? `The preview store ${session.store} has likely been claimed, so its stored authentication is no longer valid.` | ||
| : `Stored app authentication for ${session.store} is no longer valid.` | ||
|
|
||
| throwReauthenticateStoreAuthError(message, session) | ||
| } | ||
|
|
||
| export function retryStoreAuthWithPermanentDomainError(returnedStore: string): AbortError { | ||
| // eslint-disable-next-line @shopify/cli/no-error-factory-functions | ||
| return new AbortError( | ||
| 'OAuth callback store does not match the requested store.', | ||
| `Shopify returned ${returnedStore} during authentication. Re-run using the permanent store domain:`, | ||
| storeAuthCommandNextSteps(returnedStore, '<comma-separated-scopes>'), | ||
| storeAuthCommandNextStepsWithUnknownScopes(returnedStore), | ||
| ) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.