Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Improve GraphQL Schema Validation to allow Subscription types when not using Realtime and ensure schema does not use reserved names #9005

Merged
merged 10 commits into from
Aug 8, 2023
Original file line number Diff line number Diff line change
@@ -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.
"
`;
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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', () => {
Expand Down
177 changes: 177 additions & 0 deletions packages/internal/src/__tests__/validateSchemaForReservedNames.test.ts
Original file line number Diff line number Diff line change
@@ -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()
})
})
26 changes: 26 additions & 0 deletions packages/internal/src/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
69 changes: 66 additions & 3 deletions packages/internal/src/validateSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
jtoar marked this conversation as resolved.
Show resolved Hide resolved
'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[])) {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -137,5 +200,5 @@ export const loadAndValidateSdls = async () => {
projectDocumentNodes,
])

validateSchemaForDirectives(mergedDocumentNode)
validateSchema(mergedDocumentNode)
}
Loading