From bd60fc303891bc55abcef7fc77a3c43aed13a183 Mon Sep 17 00:00:00 2001
From: Marco Reni <reni.marco@gmail.com>
Date: Mon, 9 Dec 2024 14:02:13 +0100
Subject: [PATCH] feat: apq should persist queries only when inside an apq flow
---
docs/api/options.md | 2 ++
index.d.ts | 8 ++++++
lib/errors.js | 5 ++++
lib/persistedQueryDefaults.js | 4 ++-
lib/routes.js | 33 +++++++++++++---------
test/persisted.js | 53 +++++++++++++++++++++++++++++++++--
6 files changed, 88 insertions(+), 17 deletions(-)
diff --git a/docs/api/options.md b/docs/api/options.md
index 5e62e397..a00e705b 100644
--- a/docs/api/options.md
+++ b/docs/api/options.md
@@ -80,12 +80,14 @@
- `onlyPersisted`: Boolean. Flag to control whether to allow graphql queries other than persisted. When `true`, it'll make the server reject any queries that are not present in the `persistedQueries` option above. It will also disable any ide available (graphiql). Requires `persistedQueries` to be set, and overrides `persistedQueryProvider`.
- `persistedQueryProvider`
- `isPersistedQuery: (request: object) => boolean`: Return true if a given request matches the desired persisted query format.
+ - `isPersistedQueryRetry: (request: object) => boolean`: Return true if a given request matches the desired persisted query retry format.
- `getHash: (request: object) => string`: Return the hash from a given request, or falsy if this request format is not supported.
- `getQueryFromHash: async (hash: string) => string`: Return the query for a given hash.
- `getHashForQuery?: (query: string) => string`: Return the hash for a given query string. Do not provide if you want to skip saving new queries.
- `saveQuery?: async (hash: string, query: string) => void`: Save a query, given its hash.
- `notFoundError?: string`: An error message to return when `getQueryFromHash` returns no result. Defaults to `Bad Request`.
- `notSupportedError?: string`: An error message to return when a query matches `isPersistedQuery`, but returns no valid hash from `getHash`. Defaults to `Bad Request`.
+ - `mismatchError?: string`: An error message to return when the hash provided in the request does not match the calculated hash. Defaults to `Bad Request`.
- `allowBatchedQueries`: Boolean. Flag to control whether to allow batched queries. When `true`, the server supports recieving an array of queries and returns an array of results.
- `compilerOptions`: Object. Configurable options for the graphql-jit compiler. For more details check https://github.com/zalando-incubator/graphql-jit
diff --git a/index.d.ts b/index.d.ts
index 13396202..e898fd48 100644
--- a/index.d.ts
+++ b/index.d.ts
@@ -622,6 +622,10 @@ declare namespace mercurius {
* Return true if a given request matches the desired persisted query format.
*/
isPersistedQuery: (r: QueryRequest) => boolean;
+ /**
+ * Return true if a given request matches the desire persisted query retry format.
+ */
+ isPersistedQueryRetry: (r: QueryRequest) => boolean;
/**
* Return the hash from a given request, or falsy if this request format is not supported.
*/
@@ -646,6 +650,10 @@ declare namespace mercurius {
* An error message to return when a query matches isPersistedQuery, but fasly from getHash. Defaults to 'Bad Request'.
*/
notSupportedError?: string;
+ /**
+ * An error message to return when the hash provided in the request does not match the calculated hash. Defaults to 'Bad Request'.
+ */
+ mismatchError?: string;
}
/**
diff --git a/lib/errors.js b/lib/errors.js
index 34b9516f..4ac0d84e 100644
--- a/lib/errors.js
+++ b/lib/errors.js
@@ -147,6 +147,11 @@ const errors = {
'%s',
400
),
+ MER_ERR_GQL_PERSISTED_QUERY_MISMATCH: createError(
+ 'MER_ERR_GQL_PERSISTED_QUERY_MISMATCH',
+ '%s',
+ 400
+ ),
/**
* Subscription errors
*/
diff --git a/lib/persistedQueryDefaults.js b/lib/persistedQueryDefaults.js
index 1fcf1839..2fd7dd19 100644
--- a/lib/persistedQueryDefaults.js
+++ b/lib/persistedQueryDefaults.js
@@ -20,6 +20,7 @@ const persistedQueryDefaults = {
const cache = LRU(maxSize || 1024)
return ({
isPersistedQuery: (request) => !request.query && (request.extensions || {}).persistedQuery,
+ isPersistedQueryRetry: (request) => request.query && (request.extensions || {}).persistedQuery,
getHash: (request) => {
const { version, sha256Hash } = request.extensions.persistedQuery
return version === 1 ? sha256Hash : false
@@ -28,7 +29,8 @@ const persistedQueryDefaults = {
getHashForQuery: (query) => crypto.createHash('sha256').update(query, 'utf8').digest('hex'),
saveQuery: async (hash, query) => cache.set(hash, query),
notFoundError: 'PersistedQueryNotFound',
- notSupportedError: 'PersistedQueryNotSupported'
+ notSupportedError: 'PersistedQueryNotSupported',
+ mismatchError: 'provided sha does not match query'
})
}
}
diff --git a/lib/routes.js b/lib/routes.js
index 9d5d7985..0f731d90 100644
--- a/lib/routes.js
+++ b/lib/routes.js
@@ -9,6 +9,7 @@ const {
defaultErrorFormatter,
MER_ERR_GQL_PERSISTED_QUERY_NOT_FOUND,
MER_ERR_GQL_PERSISTED_QUERY_NOT_SUPPORTED,
+ MER_ERR_GQL_PERSISTED_QUERY_MISMATCH,
MER_ERR_GQL_VALIDATION,
toGraphQLError
} = require('./errors')
@@ -207,12 +208,14 @@ module.exports = async function (app, opts) {
// Load the persisted query settings
const {
isPersistedQuery,
+ isPersistedQueryRetry,
getHash,
getQueryFromHash,
getHashForQuery,
saveQuery,
notFoundError,
- notSupportedError
+ notSupportedError,
+ mismatchError,
} = persistedQueryProvider || {}
const normalizedRouteOptions = { ...additionalRouteOptions }
@@ -249,8 +252,7 @@ module.exports = async function (app, opts) {
const { operationName, variables } = body
// Verify if a query matches the persisted format
- const persisted = isPersistedQuery(body)
- if (persisted) {
+ if (isPersistedQuery(body)) {
// This is a peristed query, so we use the hash in the request
// to load the full query string.
@@ -276,16 +278,21 @@ module.exports = async function (app, opts) {
// Execute the query
const result = await executeQuery(query, variables, operationName, request, reply)
- // Only save queries which are not yet persisted
- if (!persisted && query) {
- // If provided the getHashForQuery, saveQuery settings we save this query
- const hash = getHashForQuery && getHashForQuery(query)
- if (hash) {
- try {
- await saveQuery(hash, query)
- } catch (err) {
- request.log.warn({ err, hash, query }, 'Failed to persist query')
- }
+ // Only save queries which are not yet persisted and if this is a persisted query retry
+ if (isPersistedQueryRetry && isPersistedQueryRetry(body) && query) {
+ // Extract the hash from the request
+ const hash = getHash && getHash(body)
+
+ const hashForQuery = getHashForQuery && getHashForQuery(query)
+ if (hash && hashForQuery !== hash) {
+ // The calculated hash does not match the provided one, tell the client
+ throw new MER_ERR_GQL_PERSISTED_QUERY_MISMATCH(mismatchError)
+ }
+
+ try {
+ await saveQuery(hashForQuery, query)
+ } catch (err) {
+ request.log.warn({ err, hash, query }, 'Failed to persist query')
}
}
diff --git a/test/persisted.js b/test/persisted.js
index ea4f4993..3ecc0c17 100644
--- a/test/persisted.js
+++ b/test/persisted.js
@@ -194,7 +194,13 @@ test('automatic POST new query, error on saveQuery is handled', async (t) => {
query: `
query AddQuery ($x: Int!, $y: Int!) {
add(x: $x, y: $y)
- }`
+ }`,
+ extensions: {
+ persistedQuery: {
+ version: 1,
+ sha256Hash: '14b859faf7e656329f24f7fdc7a33a3402dbd8b43f4f57364e15e096143927a9'
+ }
+ }
}
})
@@ -340,7 +346,7 @@ test('automatic POST invalid extension without persistedQueries and error', asyn
t.same(JSON.parse(res.body), { data: null, errors: [{ message: 'PersistedQueryNotSupported' }] })
})
-test('automatic POST persisted query after priming', async (t) => {
+test('avoid persisting POST query', async (t) => {
const app = Fastify()
const schema = `
@@ -389,7 +395,7 @@ test('automatic POST persisted query after priming', async (t) => {
}
})
- t.same(JSON.parse(res.body), { data: { add: 3 } })
+ t.same(JSON.parse(res.body), { data: null, errors: [{ message: 'PersistedQueryNotFound' }] })
})
test('automatic POST persisted query after priming, with extension set in both payloads', async (t) => {
@@ -450,6 +456,47 @@ test('automatic POST persisted query after priming, with extension set in both p
t.same(JSON.parse(res.body), { data: { add: 3 } })
})
+test('avoid persisting query if hashes mismatch', async (t) => {
+ const app = Fastify()
+
+ const schema = `
+ type Query {
+ add(x: Int, y: Int): Int
+ }
+ `
+
+ const resolvers = {
+ add: async ({ x, y }) => x + y
+ }
+
+ app.register(GQL, {
+ schema,
+ resolvers,
+ persistedQueryProvider: GQL.persistedQueryDefaults.automatic()
+ })
+
+ const res = await app.inject({
+ method: 'POST',
+ url: '/graphql',
+ body: {
+ operationName: 'AddQuery',
+ variables: { x: 1, y: 2 },
+ query: `
+ query AddQuery ($x: Int!, $y: Int!) {
+ add(x: $x, y: $y)
+ }`,
+ extensions: {
+ persistedQuery: {
+ version: 1,
+ sha256Hash: 'foobar'
+ }
+ }
+ }
+ })
+
+ t.same(JSON.parse(res.body), { data: null, errors: [{ message: 'provided sha does not match query' }] })
+})
+
// persistedQueryProvider
test('GET route with query, variables & persisted', async (t) => {