fix: Improve GraphQL Schema Validation to allow Subscription types wh…

…en not using Realtime and ensure schema does not use reserved names (#9005) See: #8988 1. In GraphQL there are many reserved names - `Subscription` being one of them. And Redwood's schema validation checks that a validator directive is applied to queries, mutations -- and subscriptions. However, we have cases where existing RW apps have used `Subscription` as a model and a type. This PR will only check for the directive on Subscriptions if Redwood Realtime is enabled. Note: a workaround is to keep your Prisma model named Subscription, but just rename the type to something like "CustomerSubscription" or "ProductSubscription" and rework your services to have resolver types that match your GraphQL schema. 2. This PR also will raise errors when reserved names are uses as types ,inputs or interfaces -- like if for example you had a Prisma model and a generated type like "Float" because maybe you had a fishing or sailing store and needed to have a collection of floats :) Note to @jtoar - this is a potentially breaking changes because some projects might be using reserved GraphQL names as types but really shouldn't to mitigate issues downstream. --------- Co-authored-by: Dominic Saadi <[email protected]>
redwoodjs · Aug 8, 2023 · 0167be7 · 0167be7
1 parent 2631a06
commit 0167be7
Show file tree

Hide file tree

Showing 5 changed files with 290 additions and 5 deletions.
diff --git a/packages/internal/src/__tests__/__snapshots__/validateSchemaForReservedNames.test.ts.snap b/packages/internal/src/__tests__/__snapshots__/validateSchemaForReservedNames.test.ts.snap
@@ -0,0 +1,19 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`SDL with no reserved names used SDL is invalid because uses a reserved name as a type 1`] = `
+"The type named 'Float' is a reserved GraphQL name.
+Please rename it to something more specific, like: ApplicationFloat.
+"
+`;
+
+exports[`SDL with no reserved names used because uses a reserved name as an input 1`] = `
+"The input type named 'Float' is a reserved GraphQL name.
+Please rename it to something more specific, like: ApplicationFloat.
+"
+`;
+
+exports[`SDL with no reserved names used because uses a reserved name as an interface 1`] = `
+"The interface named 'Float' is a reserved GraphQL name.
+Please rename it to something more specific, like: ApplicationFloat.
+"
+`;
diff --git a/packages/internal/src/__tests__/validateSchemaForAuthDirectives.test.ts b/packages/internal/src/__tests__/validateSchemaForAuthDirectives.test.ts
@@ -8,7 +8,7 @@ import { DocumentNode } from 'graphql'
 import { getPaths } from '@redwoodjs/project-config'
 
 import {
-  validateSchemaForDirectives,
+  validateSchema,
   DIRECTIVE_REQUIRED_ERROR_MESSAGE,
   DIRECTIVE_INVALID_ROLE_TYPES_ERROR_MESSAGE,
 } from '../validateSchema'
@@ -43,7 +43,7 @@ const validateSdlFile = async (sdlFile: string) => {
 
   // Merge in the rootSchema with JSON scalars, etc.
   const mergedDocumentNode = mergeTypeDefs([projectDocumentNodes])
-  validateSchemaForDirectives(mergedDocumentNode)
+  validateSchema(mergedDocumentNode)
 }
 
 describe('SDL uses auth directives', () => {

diff --git a/packages/internal/src/__tests__/validateSchemaForReservedNames.test.ts b/packages/internal/src/__tests__/validateSchemaForReservedNames.test.ts
@@ -0,0 +1,177 @@
+import path from 'path'
+
+import { DocumentNode } from 'graphql'
+import gql from 'graphql-tag'
+
+import { validateSchema } from '../validateSchema'
+
+const FIXTURE_PATH = path.resolve(
+  __dirname,
+  '../../../../__fixtures__/example-todo-main'
+)
+
+beforeAll(() => {
+  process.env.RWJS_CWD = FIXTURE_PATH
+})
+afterAll(() => {
+  delete process.env.RWJS_CWD
+})
+
+const validateSdlFile = async (document: DocumentNode) => {
+  validateSchema(document)
+}
+
+describe('SDL with no reserved names used', () => {
+  describe('SDL is valid', () => {
+    test('with proper type definition names', async () => {
+      const document = gql`
+        type Message {
+          from: String
+          body: String
+        }
+
+        type Query {
+          room(id: ID!): [Message!]! @skipAuth
+        }
+
+        input SendMessageInput {
+          roomId: ID!
+          from: String!
+          body: String!
+        }
+
+        type Mutation {
+          sendMessage(input: SendMessageInput!): Message! @skipAuth
+        }
+      `
+
+      await expect(validateSdlFile(document)).resolves.not.toThrowError()
+    })
+    test('with proper interface interface definition names', async () => {
+      const document = gql`
+        interface Node {
+          id: ID!
+        }
+
+        type Message implements Node {
+          id: ID!
+          from: String
+          body: String
+        }
+
+        type Query {
+          room(id: ID!): [Message!]! @skipAuth
+        }
+      `
+      await expect(validateSdlFile(document)).resolves.not.toThrowError()
+    })
+    test('with proper interface input type definition names', async () => {
+      const document = gql`
+        type Message {
+          from: String
+          body: String
+        }
+
+        type Query {
+          room(id: ID!): [Message!]! @skipAuth
+        }
+
+        input SendMessageInput {
+          roomId: ID!
+          from: String!
+          body: String!
+        }
+
+        type Mutation {
+          sendMessage(input: SendMessageInput!): Message! @skipAuth
+        }
+      `
+      await expect(validateSdlFile(document)).resolves.not.toThrowError()
+    })
+  })
+
+  describe('SDL is invalid', () => {
+    test('because uses a reserved name as a type', async () => {
+      const document = gql`
+        type Float {
+          from: String
+          body: String
+        }
+
+        type Query {
+          room(id: ID!): [Message!]! @skipAuth
+        }
+
+        input SendMessageInput {
+          roomId: ID!
+          from: String!
+          body: String!
+        }
+
+        type Mutation {
+          sendMessage(input: SendMessageInput!): Message! @skipAuth
+        }
+      `
+      await expect(
+        validateSdlFile(document)
+      ).rejects.toThrowErrorMatchingSnapshot()
+    })
+  })
+
+  test('because uses a reserved name as an input', async () => {
+    const document = gql`
+      type Message {
+        from: String
+        body: String
+      }
+
+      type Query {
+        room(id: ID!): [Message!]! @skipAuth
+      }
+
+      input Float {
+        roomId: ID!
+        from: String!
+        body: String!
+      }
+
+      type Mutation {
+        sendMessage(input: SendMessageInput!): Message! @skipAuth
+      }
+    `
+    await expect(
+      validateSdlFile(document)
+    ).rejects.toThrowErrorMatchingSnapshot()
+  })
+
+  test('because uses a reserved name as an interface', async () => {
+    const document = gql`
+      interface Float {
+        id: ID!
+      }
+
+      type Message implements Float {
+        id: ID!
+        from: String
+        body: String
+      }
+
+      type Query {
+        room(id: ID!): [Message!]! @skipAuth
+      }
+
+      input SendMessageInput {
+        roomId: ID!
+        from: String!
+        body: String!
+      }
+
+      type Mutation {
+        sendMessage(input: SendMessageInput!): Message! @skipAuth
+      }
+    `
+    await expect(
+      validateSdlFile(document)
+    ).rejects.toThrowErrorMatchingSnapshot()
+  })
+})
diff --git a/packages/internal/src/project.ts b/packages/internal/src/project.ts
@@ -29,3 +29,29 @@ export const getTsConfigs = () => {
     web: webTsConfig?.config ?? null,
   }
 }
+
+export const isTypeScriptProject = () => {
+  const paths = getPaths()
+  return (
+    fs.existsSync(path.join(paths.web.base, 'tsconfig.json')) ||
+    fs.existsSync(path.join(paths.api.base, 'tsconfig.json'))
+  )
+}
+
+export const isServerFileSetup = () => {
+  const serverFilePath = path.join(
+    getPaths().api.src,
+    `server.${isTypeScriptProject() ? 'ts' : 'js'}`
+  )
+
+  return fs.existsSync(serverFilePath)
+}
+
+export const isRealtimeSetup = () => {
+  const realtimePath = path.join(
+    getPaths().api.lib,
+    `realtime.${isTypeScriptProject() ? 'ts' : 'js'}`
+  )
+
+  return fs.existsSync(realtimePath)
+}
diff --git a/packages/internal/src/validateSchema.ts b/packages/internal/src/validateSchema.ts
@@ -6,20 +6,74 @@ import { DocumentNode, Kind, ObjectTypeDefinitionNode, visit } from 'graphql'
 import { rootSchema } from '@redwoodjs/graphql-server'
 import { getPaths } from '@redwoodjs/project-config'
 
+import { isServerFileSetup, isRealtimeSetup } from './project'
+
 export const DIRECTIVE_REQUIRED_ERROR_MESSAGE =
   'You must specify one of @requireAuth, @skipAuth or a custom directive'
 
 export const DIRECTIVE_INVALID_ROLE_TYPES_ERROR_MESSAGE =
   'Please check that the requireAuth roles is a string or an array of strings.'
-export function validateSchemaForDirectives(
+
+/**
+ * These are names that are commonly used in GraphQL schemas as scalars
+ * and would cause a conflict if used as a type name.
+ *
+ * Note: Query, Mutation, and Subscription are not included here because
+ * they are checked for separately.
+ */
+export const RESERVED_TYPES = [
+  'Int',
+  'Float',
+  'Boolean',
+  'String',
+  'DateTime',
+  'ID',
+  'uid',
+  'as',
+]
+
+export function validateSchema(
   schemaDocumentNode: DocumentNode,
-  typesToCheck: string[] = ['Query', 'Mutation', 'Subscription']
+  typesToCheck: string[] = ['Query', 'Mutation']
 ) {
   const validationOutput: string[] = []
+  const reservedNameValidationOutput: Record<string, any> = []
   const directiveRoleValidationOutput: Record<string, any> = []
 
+  // Is Subscriptions are enabled with Redwood Realtime, then enforce a rule
+  // that a Subscription type needs to have a authentication directive applied,
+  // just as Query and Mutation requires
+  if (isServerFileSetup() && isRealtimeSetup()) {
+    typesToCheck.push('Subscription')
+  }
+
   visit(schemaDocumentNode, {
+    InterfaceTypeDefinition(typeNode) {
+      // Warn that an interface definition in the SDL is using a reserved GraphQL type
+      if (RESERVED_TYPES.includes(typeNode.name.value)) {
+        reservedNameValidationOutput.push({
+          objectType: 'interface',
+          name: typeNode.name.value,
+        })
+      }
+    },
+    InputObjectTypeDefinition(typeNode) {
+      // Warn that an input definition in the SDL is using a reserved GraphQL type
+      if (RESERVED_TYPES.includes(typeNode.name.value)) {
+        reservedNameValidationOutput.push({
+          objectType: 'input type',
+          name: typeNode.name.value,
+        })
+      }
+    },
     ObjectTypeDefinition(typeNode) {
+      // Warn that a type definition in the SDL is using a reserved GraphQL type
+      if (RESERVED_TYPES.includes(typeNode.name.value)) {
+        reservedNameValidationOutput.push({
+          objectType: 'type',
+          name: typeNode.name.value,
+        })
+      }
       if (typesToCheck.includes(typeNode.name.value)) {
         for (const field of typeNode.fields ||
           ([] as ObjectTypeDefinitionNode[])) {
@@ -102,6 +156,15 @@ export function validateSchemaForDirectives(
       )} \n\nFor example: @requireAuth(roles: "admin") or @requireAuth(roles: ["admin", "editor"])`
     )
   }
+
+  if (reservedNameValidationOutput.length > 0) {
+    const reservedNameMsg = reservedNameValidationOutput.map(
+      (output: Record<string, any>) => {
+        return `The ${output.objectType} named '${output.name}' is a reserved GraphQL name.\nPlease rename it to something more specific, like: Application${output.name}.\n`
+      }
+    )
+    throw new TypeError(reservedNameMsg.join('\n'))
+  }
 }
 
 export const loadAndValidateSdls = async () => {
@@ -137,5 +200,5 @@ export const loadAndValidateSdls = async () => {
     projectDocumentNodes,
   ])
 
-  validateSchemaForDirectives(mergedDocumentNode)
+  validateSchema(mergedDocumentNode)
 }