Skip to content

Commit

Permalink
fix: Improve GraphQL Schema Validation to allow Subscription types wh…
Browse files Browse the repository at this point in the history
…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 <dominiceliassaadi@gmail.com>
  • Loading branch information
dthyresson and jtoar authored Aug 8, 2023
1 parent 2631a06 commit 0167be7
Show file tree
Hide file tree
Showing 5 changed files with 290 additions and 5 deletions.
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',
'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)
}

0 comments on commit 0167be7

Please sign in to comment.