Skip to content

Commit bdd4def

Browse files
authored
Merge pull request #7033 from Shopify/03-17-fix_require_authentication_key_for_graphiql_dev_server
fix: require authentication key for GraphiQL dev server
2 parents e28b8af + 04702b0 commit bdd4def

7 files changed

Lines changed: 153 additions & 11 deletions

File tree

packages/app/src/cli/commands/app/dev.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ export default class Dev extends AppLinkedCommand {
8383
'graphiql-key': Flags.string({
8484
hidden: true,
8585
description:
86-
'Key used to authenticate GraphiQL requests. Should be specified if exposing GraphiQL on a publicly accessible URL. By default, no key is required.',
86+
'Key used to authenticate GraphiQL requests. By default, a key is automatically derived from the app secret. Use this flag to override with a custom key.',
8787
env: 'SHOPIFY_FLAG_GRAPHIQL_KEY',
8888
}),
8989
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import {deriveGraphiQLKey, resolveGraphiQLKey} from './server.js'
2+
import {describe, expect, test} from 'vitest'
3+
4+
describe('deriveGraphiQLKey', () => {
5+
test('returns a 64-character hex string', () => {
6+
const key = deriveGraphiQLKey('secret', 'store.myshopify.com')
7+
expect(key).toMatch(/^[0-9a-f]{64}$/)
8+
})
9+
10+
test('is deterministic — same inputs produce the same key', () => {
11+
const key1 = deriveGraphiQLKey('secret', 'store.myshopify.com')
12+
const key2 = deriveGraphiQLKey('secret', 'store.myshopify.com')
13+
expect(key1).toBe(key2)
14+
})
15+
16+
test('different secrets produce different keys', () => {
17+
const key1 = deriveGraphiQLKey('secret-1', 'store.myshopify.com')
18+
const key2 = deriveGraphiQLKey('secret-2', 'store.myshopify.com')
19+
expect(key1).not.toBe(key2)
20+
})
21+
22+
test('different stores produce different keys', () => {
23+
const key1 = deriveGraphiQLKey('secret', 'store-a.myshopify.com')
24+
const key2 = deriveGraphiQLKey('secret', 'store-b.myshopify.com')
25+
expect(key1).not.toBe(key2)
26+
})
27+
})
28+
29+
describe('resolveGraphiQLKey', () => {
30+
test('uses provided key when non-empty', () => {
31+
const key = resolveGraphiQLKey('my-custom-key', 'secret', 'store.myshopify.com')
32+
expect(key).toBe('my-custom-key')
33+
})
34+
35+
test('derives key when provided key is undefined', () => {
36+
const key = resolveGraphiQLKey(undefined, 'secret', 'store.myshopify.com')
37+
expect(key).toBe(deriveGraphiQLKey('secret', 'store.myshopify.com'))
38+
})
39+
40+
test('derives key when provided key is empty string', () => {
41+
const key = resolveGraphiQLKey('', 'secret', 'store.myshopify.com')
42+
expect(key).toBe(deriveGraphiQLKey('secret', 'store.myshopify.com'))
43+
})
44+
45+
test('derives key when provided key is whitespace-only', () => {
46+
const key = resolveGraphiQLKey(' ', 'secret', 'store.myshopify.com')
47+
expect(key).toBe(deriveGraphiQLKey('secret', 'store.myshopify.com'))
48+
})
49+
})

packages/app/src/cli/services/dev/graphiql/server.ts

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,28 @@ import {adminUrl, supportedApiVersions} from '@shopify/cli-kit/node/api/admin'
1010
import {fetch} from '@shopify/cli-kit/node/http'
1111
import {renderLiquidTemplate} from '@shopify/cli-kit/node/liquid'
1212
import {outputDebug} from '@shopify/cli-kit/node/output'
13+
import {createHmac, timingSafeEqual} from 'crypto'
1314
import {Server} from 'http'
1415
import {Writable} from 'stream'
1516
import {createRequire} from 'module'
1617

18+
/**
19+
* Derives a deterministic GraphiQL authentication key from the app's API secret and store FQDN.
20+
* The key is stable across dev server restarts (so browser tabs survive restarts)
21+
* but is not guessable without the app secret.
22+
*/
23+
export function deriveGraphiQLKey(apiSecret: string, storeFqdn: string): string {
24+
return createHmac('sha256', apiSecret).update(`graphiql:${storeFqdn}`).digest('hex')
25+
}
26+
27+
/**
28+
* Resolves the GraphiQL authentication key. Uses the explicitly provided key
29+
* if non-empty, otherwise derives one deterministically from the app secret.
30+
*/
31+
export function resolveGraphiQLKey(providedKey: string | undefined, apiSecret: string, storeFqdn: string): string {
32+
return providedKey?.trim() || deriveGraphiQLKey(apiSecret, storeFqdn)
33+
}
34+
1735
const require = createRequire(import.meta.url)
1836

1937
class TokenRefreshError extends AbortError {
@@ -50,15 +68,21 @@ export function setupGraphiQLServer({
5068
appUrl,
5169
apiKey,
5270
apiSecret,
53-
key,
71+
key: providedKey,
5472
storeFqdn,
5573
}: SetupGraphiQLServerOptions): Server {
74+
// Always require an authentication key. If not explicitly provided, derive one
75+
// deterministically from apiSecret + storeFqdn so the key is stable across restarts
76+
// (browser tabs survive dev server restarts) but not guessable without the app secret.
77+
const key = resolveGraphiQLKey(providedKey, apiSecret, storeFqdn)
5678
outputDebug(`Setting up GraphiQL HTTP server on port ${port}...`, stdout)
5779
const app = express()
5880

5981
function failIfUnmatchedKey(str: string, res: express.Response): boolean {
60-
if (!key || str === key) return false
61-
res.status(404).send(`Invalid path ${res.req.originalUrl}`)
82+
const strBuffer = Buffer.from(str ?? '')
83+
const keyBuffer = Buffer.from(key)
84+
if (strBuffer.length === keyBuffer.length && timingSafeEqual(strBuffer, keyBuffer)) return false
85+
res.status(404).type('text/plain').send(`Invalid path ${res.req.originalUrl}`)
6286
return true
6387
}
6488

@@ -116,7 +140,8 @@ export function setupGraphiQLServer({
116140
)
117141
}
118142

119-
app.get('/graphiql/status', (_req, res) => {
143+
app.get('/graphiql/status', (req, res) => {
144+
if (failIfUnmatchedKey(req.query.key as string, res)) return
120145
fetchApiVersionsWithTokenRefresh()
121146
.then(() => res.send({status: 'OK', storeFqdn, appName, appUrl}))
122147
.catch(() => res.send({status: 'UNAUTHENTICATED'}))

packages/app/src/cli/services/dev/graphiql/templates/graphiql.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ interface GraphiQLTemplateOptions {
5151
apiVersions: string[]
5252
appName: string
5353
appUrl: string
54-
key?: string
54+
key: string
5555
storeFqdn: string
5656
}
5757

@@ -248,7 +248,7 @@ export function graphiqlTemplate({
248248
ReactDOM.render(
249249
React.createElement(GraphiQL, {
250250
fetcher: GraphiQL.createFetcher({
251-
url: '{{url}}/graphiql/graphql.json?key=${key ?? ''}&api_version=' + apiVersion,
251+
url: '{{url}}/graphiql/graphql.json?key=${encodeURIComponent(key)}&api_version=' + apiVersion,
252252
}),
253253
defaultEditorToolsVisibility: true,
254254
{% if query %}
@@ -320,7 +320,7 @@ export function graphiqlTemplate({
320320
321321
// Verify the current store/app connection
322322
setInterval(function() {
323-
fetch('{{ url }}/graphiql/status')
323+
fetch('{{ url }}/graphiql/status?key=${encodeURIComponent(key)}')
324324
.then(async function(response) {
325325
const {status, storeFqdn, appName, appUrl} = await response.json()
326326
appIsInstalled = status === 'OK'

packages/app/src/cli/services/dev/processes/setup-dev-processes.test.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {pushUpdatesForDraftableExtensions} from './draftable-extension.js'
88
import {pushUpdatesForDevSession} from './dev-session/dev-session-process.js'
99
import {runThemeAppExtensionsServer} from './theme-app-extension.js'
1010
import {launchAppWatcher} from './app-watcher-process.js'
11+
import {resolveGraphiQLKey} from '../graphiql/server.js'
1112
import {
1213
testAppAccessConfigExtension,
1314
testAppConfigExtensions,
@@ -312,6 +313,71 @@ describe('setup-dev-processes', () => {
312313
})
313314
})
314315

316+
test('auto-derives a graphiql key when none is provided', async () => {
317+
const developerPlatformClient: DeveloperPlatformClient = testDeveloperPlatformClient()
318+
const storeFqdn = 'store.myshopify.io'
319+
const storeId = '123456789'
320+
const remoteAppUpdated = true
321+
const graphiqlPort = 1234
322+
const commandOptions: DevConfig['commandOptions'] = {
323+
...appContextResult,
324+
directory: '',
325+
update: false,
326+
commandConfig: new Config({root: ''}),
327+
skipDependenciesInstallation: false,
328+
subscriptionProductUrl: '/products/999999',
329+
checkoutCartUrl: '/cart/999999:1',
330+
tunnel: {mode: 'auto'},
331+
}
332+
const network: DevConfig['network'] = {
333+
proxyUrl: 'https://example.com/proxy',
334+
proxyPort: 444,
335+
backendPort: 111,
336+
frontendPort: 222,
337+
currentUrls: {
338+
applicationUrl: 'https://example.com/application',
339+
redirectUrlWhitelist: ['https://example.com/redirect'],
340+
},
341+
}
342+
const localApp = testAppWithConfig({config: {}})
343+
vi.spyOn(loader, 'reloadApp').mockResolvedValue(localApp)
344+
345+
const remoteApp: DevConfig['remoteApp'] = {
346+
apiKey: 'api-key',
347+
apiSecretKeys: [{secret: 'api-secret'}],
348+
id: '1234',
349+
title: 'App',
350+
organizationId: '5678',
351+
grantedScopes: [],
352+
flags: [],
353+
developerPlatformClient,
354+
}
355+
356+
// No graphiqlKey provided — should auto-derive one
357+
const res = await setupDevProcesses({
358+
localApp,
359+
commandOptions,
360+
network,
361+
remoteApp,
362+
remoteAppUpdated,
363+
storeFqdn,
364+
storeId,
365+
developerPlatformClient,
366+
partnerUrlsUpdated: true,
367+
graphiqlPort,
368+
})
369+
370+
const expectedKey = resolveGraphiQLKey(undefined, 'api-secret', storeFqdn)
371+
372+
// The graphiql process should use the resolved key
373+
const graphiqlProcess = res.processes.find((process) => process.type === 'graphiql')
374+
expect(graphiqlProcess).toBeDefined()
375+
expect((graphiqlProcess!.options as {key: string}).key).toBe(expectedKey)
376+
377+
// The graphiql URL should include the resolved key
378+
expect(res.graphiqlUrl).toBe(`http://localhost:${graphiqlPort}/graphiql?key=${encodeURIComponent(expectedKey)}`)
379+
})
380+
315381
test('process list includes dev-session when useDevSession is true', async () => {
316382
const developerPlatformClient: DeveloperPlatformClient = testDeveloperPlatformClient({supportsDevSessions: true})
317383
const storeFqdn = 'store.myshopify.io'

packages/app/src/cli/services/dev/processes/setup-dev-processes.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {DevSessionProcess, setupDevSessionProcess} from './dev-session/dev-sessi
99
import {AppLogsSubscribeProcess, setupAppLogsPollingProcess} from './app-logs-polling.js'
1010
import {AppWatcherProcess, setupAppWatcherProcess} from './app-watcher-process.js'
1111
import {DevSessionStatusManager} from './dev-session/dev-session-status-manager.js'
12+
import {resolveGraphiQLKey} from '../graphiql/server.js'
1213
import {environmentVariableNames} from '../../../constants.js'
1314
import {AppLinkedInterface, getAppScopes, WebType} from '../../../models/app/app.js'
1415

@@ -119,8 +120,9 @@ export async function setupDevProcesses({
119120
const useDevConsole = is1PDev && anyPreviewableExtensions
120121
const previewURL = useDevConsole ? devConsoleURL : appPreviewUrl
121122

123+
const resolvedGraphiqlKey = resolveGraphiQLKey(graphiqlKey, apiSecret, storeFqdn)
122124
const graphiqlURL = shouldRenderGraphiQL
123-
? `http://localhost:${graphiqlPort}/graphiql${graphiqlKey ? `?key=${graphiqlKey}` : ''}`
125+
? `http://localhost:${graphiqlPort}/graphiql?key=${encodeURIComponent(resolvedGraphiqlKey)}`
124126
: undefined
125127

126128
const devSessionStatusManager = new DevSessionStatusManager({isReady: false, previewURL, graphiqlURL})
@@ -142,7 +144,7 @@ export async function setupDevProcesses({
142144
port: graphiqlPort,
143145
apiKey,
144146
apiSecret,
145-
key: graphiqlKey,
147+
key: resolvedGraphiqlKey,
146148
storeFqdn,
147149
})
148150
: undefined,

packages/cli/oclif.manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -848,7 +848,7 @@
848848
"type": "option"
849849
},
850850
"graphiql-key": {
851-
"description": "Key used to authenticate GraphiQL requests. Should be specified if exposing GraphiQL on a publicly accessible URL. By default, no key is required.",
851+
"description": "Key used to authenticate GraphiQL requests. By default, a key is automatically derived from the app secret. Use this flag to override with a custom key.",
852852
"env": "SHOPIFY_FLAG_GRAPHIQL_KEY",
853853
"hasDynamicHelp": false,
854854
"hidden": true,

0 commit comments

Comments
 (0)