feat: apq should persist queries only when inside an apq flow (#1124)

Author: marcoreni

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

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;
   }
 
   /**

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
    */

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'
     })
   }
 }

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')
       }
     }
 

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) => {