fix: Refactor logic to enable/allow disable/not allow GraphiQL and In…

…trospection Configuration in Dev vs Prod (#9104) There was a regression in the logic enabling/allowing GraphiQL and GraphQL introspection was being checked when creating GraphQL Yoga. This PR reworks the logic, extracts it, and adds tests to prevent the logic regression from happening in the future. --------- Co-authored-by: Josh GM Walker <56300765+Josh-Walker-GM@users.noreply.github.com>
redwoodjs · Sep 2, 2023 · f5c26c5 · f5c26c5
1 parent a9c02e2
commit f5c26c5
Show file tree

Hide file tree

Showing 7 changed files with 387 additions and 26 deletions.
diff --git a/packages/graphql-server/src/__tests__/__snapshots__/graphiql.test.ts.snap b/packages/graphql-server/src/__tests__/__snapshots__/graphiql.test.ts.snap
@@ -0,0 +1,79 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`configureGraphiQLPlayground when in development should configure the GraphiQL Playground when allowGraphiQL is not provided 1`] = `
+{
+  "defaultQuery": "query Redwood {
+  redwood {
+    version
+  }
+}",
+  "headerEditorEnabled": true,
+  "headers": undefined,
+  "title": "Redwood GraphQL Playground",
+}
+`;
+
+exports[`configureGraphiQLPlayground when in development should configure the GraphiQL Playground when allowGraphiQL is null 1`] = `
+{
+  "defaultQuery": "query Redwood {
+  redwood {
+    version
+  }
+}",
+  "headerEditorEnabled": true,
+  "headers": undefined,
+  "title": "Redwood GraphQL Playground",
+}
+`;
+
+exports[`configureGraphiQLPlayground when in development should configure the GraphiQL Playground when allowGraphiQL is true 1`] = `
+{
+  "defaultQuery": "query Redwood {
+  redwood {
+    version
+  }
+}",
+  "headerEditorEnabled": true,
+  "headers": undefined,
+  "title": "Redwood GraphQL Playground",
+}
+`;
+
+exports[`configureGraphiQLPlayground when in development should configure the GraphiQL Playground when allowGraphiQL is undefined 1`] = `
+{
+  "defaultQuery": "query Redwood {
+  redwood {
+    version
+  }
+}",
+  "headerEditorEnabled": true,
+  "headers": undefined,
+  "title": "Redwood GraphQL Playground",
+}
+`;
+
+exports[`configureGraphiQLPlayground when in development should configure the GraphiQL Playground when no config is provided 1`] = `
+{
+  "defaultQuery": "query Redwood {
+  redwood {
+    version
+  }
+}",
+  "headerEditorEnabled": true,
+  "headers": "{"x-auth-comment": "See documentation: https://redwoodjs.com/docs/cli-commands#setup-graphiql-headers on how to auto generate auth headers"}",
+  "title": "Redwood GraphQL Playground",
+}
+`;
+
+exports[`configureGraphiQLPlayground when not in development environment should configure the GraphiQL Playground when allowGraphiQL is true 1`] = `
+{
+  "defaultQuery": "query Redwood {
+  redwood {
+    version
+  }
+}",
+  "headerEditorEnabled": true,
+  "headers": undefined,
+  "title": "Redwood GraphQL Playground",
+}
+`;
diff --git a/packages/graphql-server/src/__tests__/graphiql.test.ts b/packages/graphql-server/src/__tests__/graphiql.test.ts
@@ -0,0 +1,137 @@
+import { configureGraphiQLPlayground } from '../graphiql'
+
+describe('configureGraphiQLPlayground', () => {
+  describe('when not in development environment', () => {
+    const curNodeEnv = process.env.NODE_ENV
+
+    beforeAll(() => {
+      process.env.NODE_ENV = 'not-development'
+    })
+
+    afterAll(() => {
+      process.env.NODE_ENV = curNodeEnv
+      expect(process.env.NODE_ENV).toBe('test')
+    })
+
+    it('should return false when no config is provided', () => {
+      const result = configureGraphiQLPlayground({})
+
+      expect(result).toBe(false)
+    })
+
+    it('should configure the GraphiQL Playground when allowGraphiQL is true', () => {
+      const result = configureGraphiQLPlayground({
+        allowGraphiQL: true,
+        generateGraphiQLHeader: jest.fn(),
+      })
+
+      expect(result).not.toBe(false)
+      expect(result).toMatchSnapshot()
+    })
+
+    it('should return false when allowGraphiQL is false', () => {
+      const result = configureGraphiQLPlayground({
+        allowGraphiQL: false,
+        generateGraphiQLHeader: jest.fn(),
+      })
+
+      expect(result).toBe(false)
+    })
+
+    it('should return false when allowGraphiQL is not provided', () => {
+      const result = configureGraphiQLPlayground({
+        generateGraphiQLHeader: jest.fn(),
+      })
+
+      expect(result).toBe(false)
+    })
+
+    it('should return false when allowGraphiQL is undefined', () => {
+      const result = configureGraphiQLPlayground({
+        allowGraphiQL: undefined,
+        generateGraphiQLHeader: jest.fn(),
+      })
+
+      expect(result).toBe(false)
+    })
+
+    it('should return false when allowGraphiQL is null', () => {
+      const result = configureGraphiQLPlayground({
+        // @ts-expect-error - We don't explicitly allow null, but we will cover it in the tests anyway
+        allowGraphiQL: null,
+        generateGraphiQLHeader: jest.fn(),
+      })
+
+      expect(result).toBe(false)
+    })
+  })
+
+  describe('when in development', () => {
+    const curNodeEnv = process.env.NODE_ENV
+
+    beforeAll(() => {
+      process.env.NODE_ENV = 'development'
+    })
+
+    afterAll(() => {
+      process.env.NODE_ENV = curNodeEnv
+      expect(process.env.NODE_ENV).toBe('test')
+    })
+
+    it('should configure the GraphiQL Playground when no config is provided', () => {
+      const result = configureGraphiQLPlayground({})
+
+      expect(result).not.toBe(false)
+      expect(result).toMatchSnapshot()
+    })
+
+    it('should configure the GraphiQL Playground when allowGraphiQL is true', () => {
+      const result = configureGraphiQLPlayground({
+        allowGraphiQL: true,
+        generateGraphiQLHeader: jest.fn(),
+      })
+
+      expect(result).not.toBe(false)
+      expect(result).toMatchSnapshot()
+    })
+
+    it('should configure the GraphiQL Playground when allowGraphiQL is false', () => {
+      const result = configureGraphiQLPlayground({
+        allowGraphiQL: false,
+        generateGraphiQLHeader: jest.fn(),
+      })
+
+      expect(result).toBe(false)
+    })
+
+    it('should configure the GraphiQL Playground when allowGraphiQL is not provided', () => {
+      const result = configureGraphiQLPlayground({
+        generateGraphiQLHeader: jest.fn(),
+      })
+
+      expect(result).not.toBe(false)
+      expect(result).toMatchSnapshot()
+    })
+
+    it('should configure the GraphiQL Playground when allowGraphiQL is undefined', () => {
+      const result = configureGraphiQLPlayground({
+        allowGraphiQL: undefined,
+        generateGraphiQLHeader: jest.fn(),
+      })
+
+      expect(result).not.toBe(false)
+      expect(result).toMatchSnapshot()
+    })
+
+    it('should configure the GraphiQL Playground when allowGraphiQL is null', () => {
+      const result = configureGraphiQLPlayground({
+        // @ts-expect-error - We don't explicitly allow null, but we will cover it in the tests anyway
+        allowGraphiQL: null,
+        generateGraphiQLHeader: jest.fn(),
+      })
+
+      expect(result).not.toBe(false)
+      expect(result).toMatchSnapshot()
+    })
+  })
+})
diff --git a/packages/graphql-server/src/__tests__/introspection.test.ts b/packages/graphql-server/src/__tests__/introspection.test.ts
@@ -0,0 +1,107 @@
+import { configureGraphQLIntrospection } from '../introspection'
+
+describe('configureGraphQLIntrospection', () => {
+  describe('when not in development environment', () => {
+    const curNodeEnv = process.env.NODE_ENV
+
+    beforeAll(() => {
+      process.env.NODE_ENV = 'not-development'
+    })
+
+    afterAll(() => {
+      process.env.NODE_ENV = curNodeEnv
+      expect(process.env.NODE_ENV).toBe('test')
+    })
+
+    it('should not disable GraphQL Introspection when allowIntrospection is true', () => {
+      const { disableIntrospection } = configureGraphQLIntrospection({
+        allowIntrospection: true,
+      })
+
+      expect(disableIntrospection).toBe(false)
+    })
+
+    it('should disable GraphQL Introspection when allowIntrospection is false', () => {
+      const { disableIntrospection } = configureGraphQLIntrospection({
+        allowIntrospection: false,
+      })
+
+      expect(disableIntrospection).toBe(true)
+    })
+
+    it('should disable GraphQL Introspection when allowIntrospection is not provided', () => {
+      const { disableIntrospection } = configureGraphQLIntrospection({})
+
+      expect(disableIntrospection).toBe(true)
+    })
+
+    it('should disable GraphQL Introspection when allowIntrospection is undefined', () => {
+      const { disableIntrospection } = configureGraphQLIntrospection({
+        allowIntrospection: undefined,
+      })
+
+      expect(disableIntrospection).toBe(true)
+    })
+
+    it('should disable GraphQL Introspection when allowIntrospection is null', () => {
+      const { disableIntrospection } = configureGraphQLIntrospection({
+        // @ts-expect-error - We don't explicitly allow null, but we will cover it in the tests anyway
+        allowIntrospection: null,
+      })
+
+      expect(disableIntrospection).toBe(true)
+    })
+  })
+
+  describe('when in development', () => {
+    const curNodeEnv = process.env.NODE_ENV
+
+    beforeAll(() => {
+      process.env.NODE_ENV = 'development'
+    })
+
+    afterAll(() => {
+      process.env.NODE_ENV = curNodeEnv
+      expect(process.env.NODE_ENV).toBe('test')
+    })
+
+    it('should not disable GraphQL Introspection when allowIntrospection is true', () => {
+      const { disableIntrospection } = configureGraphQLIntrospection({
+        allowIntrospection: true,
+      })
+
+      expect(disableIntrospection).toBe(false)
+    })
+
+    it('should disable GraphQL Introspection when allowIntrospection is false', () => {
+      const { disableIntrospection } = configureGraphQLIntrospection({
+        allowIntrospection: false,
+      })
+
+      expect(disableIntrospection).toBe(true)
+    })
+
+    it('should not disable GraphQL Introspection when allowIntrospection is not provided', () => {
+      const { disableIntrospection } = configureGraphQLIntrospection({})
+
+      expect(disableIntrospection).toBe(false)
+    })
+
+    it('should not disable GraphQL Introspection when allowIntrospection is undefined', () => {
+      const { disableIntrospection } = configureGraphQLIntrospection({
+        allowIntrospection: undefined,
+      })
+
+      expect(disableIntrospection).toBe(false)
+    })
+
+    it('should not disable GraphQL Introspection when allowIntrospection is null', () => {
+      const { disableIntrospection } = configureGraphQLIntrospection({
+        // @ts-expect-error - We don't explicitly allow null, but we will cover it in the tests anyway
+        allowIntrospection: null,
+      })
+
+      expect(disableIntrospection).toBe(false)
+    })
+  })
+})
diff --git a/packages/graphql-server/src/createGraphQLYoga.ts b/packages/graphql-server/src/createGraphQLYoga.ts
@@ -6,6 +6,8 @@ import { Plugin, useReadinessCheck, createYoga } from 'graphql-yoga'
 
 import { mapRwCorsOptionsToYoga } from './cors'
 import { makeDirectivesForPlugin } from './directives/makeDirectives'
+import { configureGraphiQLPlayground } from './graphiql'
+import { configureGraphQLIntrospection } from './introspection'
 import { makeMergedSchema } from './makeMergedSchema'
 import {
   useArmor,
@@ -52,6 +54,8 @@ export const createGraphQLYoga = ({
   let redwoodDirectivePlugins = [] as Plugin[]
   const logger = loggerConfig.logger
 
+  const isDevEnv = process.env.NODE_ENV === 'development'
+
   try {
     // @NOTE: Directives are optional
     const projectDirectives = makeDirectivesForPlugin(directives)
@@ -94,31 +98,9 @@ export const createGraphQLYoga = ({
     // so the order here matters
     const plugins: Array<Plugin<any>> = []
 
-    const isDevEnv = process.env.NODE_ENV === 'development'
-    const disableIntrospection =
-      (allowIntrospection === null && !isDevEnv) || allowIntrospection === false
-    const disableGraphQL =
-      (allowGraphiQL === null && !isDevEnv) || allowGraphiQL === false
-
-    const defaultQuery = `query Redwood {
-    redwood {
-    version
-    }
-  }`
-
-    // TODO: Once Studio is not experimental, can remove these generateGraphiQLHeaders
-    const authHeader = `{"x-auth-comment": "See documentation: https://redwoodjs.com/docs/cli-commands#setup-graphiql-headers on how to auto generate auth headers"}`
-
-    const graphiql = !disableGraphQL
-      ? {
-          title: 'Redwood GraphQL Playground',
-          headers: generateGraphiQLHeader
-            ? generateGraphiQLHeader()
-            : authHeader,
-          defaultQuery,
-          headerEditorEnabled: true,
-        }
-      : false
+    const { disableIntrospection } = configureGraphQLIntrospection({
+      allowIntrospection,
+    })
 
     if (disableIntrospection) {
       plugins.push(useDisableIntrospection())
@@ -225,7 +207,10 @@ export const createGraphQLYoga = ({
       logging: logger,
       healthCheckEndpoint: graphiQLEndpoint + '/health',
       graphqlEndpoint: graphiQLEndpoint,
-      graphiql,
+      graphiql: configureGraphiQLPlayground({
+        allowGraphiQL,
+        generateGraphiQLHeader,
+      }),
       cors: (request: Request) => {
         const requestOrigin = request.headers.get('origin')
         return mapRwCorsOptionsToYoga(cors, requestOrigin)