Skip to content

Commit 0167be7

Browse files
dthyressonjtoar
andauthored
fix: Improve GraphQL Schema Validation to allow Subscription types when 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]>
1 parent 2631a06 commit 0167be7

File tree

5 files changed

+290
-5
lines changed

5 files changed

+290
-5
lines changed
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`SDL with no reserved names used SDL is invalid because uses a reserved name as a type 1`] = `
4+
"The type named 'Float' is a reserved GraphQL name.
5+
Please rename it to something more specific, like: ApplicationFloat.
6+
"
7+
`;
8+
9+
exports[`SDL with no reserved names used because uses a reserved name as an input 1`] = `
10+
"The input type named 'Float' is a reserved GraphQL name.
11+
Please rename it to something more specific, like: ApplicationFloat.
12+
"
13+
`;
14+
15+
exports[`SDL with no reserved names used because uses a reserved name as an interface 1`] = `
16+
"The interface named 'Float' is a reserved GraphQL name.
17+
Please rename it to something more specific, like: ApplicationFloat.
18+
"
19+
`;

packages/internal/src/__tests__/validateSchemaForAuthDirectives.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { DocumentNode } from 'graphql'
88
import { getPaths } from '@redwoodjs/project-config'
99

1010
import {
11-
validateSchemaForDirectives,
11+
validateSchema,
1212
DIRECTIVE_REQUIRED_ERROR_MESSAGE,
1313
DIRECTIVE_INVALID_ROLE_TYPES_ERROR_MESSAGE,
1414
} from '../validateSchema'
@@ -43,7 +43,7 @@ const validateSdlFile = async (sdlFile: string) => {
4343

4444
// Merge in the rootSchema with JSON scalars, etc.
4545
const mergedDocumentNode = mergeTypeDefs([projectDocumentNodes])
46-
validateSchemaForDirectives(mergedDocumentNode)
46+
validateSchema(mergedDocumentNode)
4747
}
4848

4949
describe('SDL uses auth directives', () => {
Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
import path from 'path'
2+
3+
import { DocumentNode } from 'graphql'
4+
import gql from 'graphql-tag'
5+
6+
import { validateSchema } from '../validateSchema'
7+
8+
const FIXTURE_PATH = path.resolve(
9+
__dirname,
10+
'../../../../__fixtures__/example-todo-main'
11+
)
12+
13+
beforeAll(() => {
14+
process.env.RWJS_CWD = FIXTURE_PATH
15+
})
16+
afterAll(() => {
17+
delete process.env.RWJS_CWD
18+
})
19+
20+
const validateSdlFile = async (document: DocumentNode) => {
21+
validateSchema(document)
22+
}
23+
24+
describe('SDL with no reserved names used', () => {
25+
describe('SDL is valid', () => {
26+
test('with proper type definition names', async () => {
27+
const document = gql`
28+
type Message {
29+
from: String
30+
body: String
31+
}
32+
33+
type Query {
34+
room(id: ID!): [Message!]! @skipAuth
35+
}
36+
37+
input SendMessageInput {
38+
roomId: ID!
39+
from: String!
40+
body: String!
41+
}
42+
43+
type Mutation {
44+
sendMessage(input: SendMessageInput!): Message! @skipAuth
45+
}
46+
`
47+
48+
await expect(validateSdlFile(document)).resolves.not.toThrowError()
49+
})
50+
test('with proper interface interface definition names', async () => {
51+
const document = gql`
52+
interface Node {
53+
id: ID!
54+
}
55+
56+
type Message implements Node {
57+
id: ID!
58+
from: String
59+
body: String
60+
}
61+
62+
type Query {
63+
room(id: ID!): [Message!]! @skipAuth
64+
}
65+
`
66+
await expect(validateSdlFile(document)).resolves.not.toThrowError()
67+
})
68+
test('with proper interface input type definition names', async () => {
69+
const document = gql`
70+
type Message {
71+
from: String
72+
body: String
73+
}
74+
75+
type Query {
76+
room(id: ID!): [Message!]! @skipAuth
77+
}
78+
79+
input SendMessageInput {
80+
roomId: ID!
81+
from: String!
82+
body: String!
83+
}
84+
85+
type Mutation {
86+
sendMessage(input: SendMessageInput!): Message! @skipAuth
87+
}
88+
`
89+
await expect(validateSdlFile(document)).resolves.not.toThrowError()
90+
})
91+
})
92+
93+
describe('SDL is invalid', () => {
94+
test('because uses a reserved name as a type', async () => {
95+
const document = gql`
96+
type Float {
97+
from: String
98+
body: String
99+
}
100+
101+
type Query {
102+
room(id: ID!): [Message!]! @skipAuth
103+
}
104+
105+
input SendMessageInput {
106+
roomId: ID!
107+
from: String!
108+
body: String!
109+
}
110+
111+
type Mutation {
112+
sendMessage(input: SendMessageInput!): Message! @skipAuth
113+
}
114+
`
115+
await expect(
116+
validateSdlFile(document)
117+
).rejects.toThrowErrorMatchingSnapshot()
118+
})
119+
})
120+
121+
test('because uses a reserved name as an input', async () => {
122+
const document = gql`
123+
type Message {
124+
from: String
125+
body: String
126+
}
127+
128+
type Query {
129+
room(id: ID!): [Message!]! @skipAuth
130+
}
131+
132+
input Float {
133+
roomId: ID!
134+
from: String!
135+
body: String!
136+
}
137+
138+
type Mutation {
139+
sendMessage(input: SendMessageInput!): Message! @skipAuth
140+
}
141+
`
142+
await expect(
143+
validateSdlFile(document)
144+
).rejects.toThrowErrorMatchingSnapshot()
145+
})
146+
147+
test('because uses a reserved name as an interface', async () => {
148+
const document = gql`
149+
interface Float {
150+
id: ID!
151+
}
152+
153+
type Message implements Float {
154+
id: ID!
155+
from: String
156+
body: String
157+
}
158+
159+
type Query {
160+
room(id: ID!): [Message!]! @skipAuth
161+
}
162+
163+
input SendMessageInput {
164+
roomId: ID!
165+
from: String!
166+
body: String!
167+
}
168+
169+
type Mutation {
170+
sendMessage(input: SendMessageInput!): Message! @skipAuth
171+
}
172+
`
173+
await expect(
174+
validateSdlFile(document)
175+
).rejects.toThrowErrorMatchingSnapshot()
176+
})
177+
})

packages/internal/src/project.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,29 @@ export const getTsConfigs = () => {
2929
web: webTsConfig?.config ?? null,
3030
}
3131
}
32+
33+
export const isTypeScriptProject = () => {
34+
const paths = getPaths()
35+
return (
36+
fs.existsSync(path.join(paths.web.base, 'tsconfig.json')) ||
37+
fs.existsSync(path.join(paths.api.base, 'tsconfig.json'))
38+
)
39+
}
40+
41+
export const isServerFileSetup = () => {
42+
const serverFilePath = path.join(
43+
getPaths().api.src,
44+
`server.${isTypeScriptProject() ? 'ts' : 'js'}`
45+
)
46+
47+
return fs.existsSync(serverFilePath)
48+
}
49+
50+
export const isRealtimeSetup = () => {
51+
const realtimePath = path.join(
52+
getPaths().api.lib,
53+
`realtime.${isTypeScriptProject() ? 'ts' : 'js'}`
54+
)
55+
56+
return fs.existsSync(realtimePath)
57+
}

packages/internal/src/validateSchema.ts

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,74 @@ import { DocumentNode, Kind, ObjectTypeDefinitionNode, visit } from 'graphql'
66
import { rootSchema } from '@redwoodjs/graphql-server'
77
import { getPaths } from '@redwoodjs/project-config'
88

9+
import { isServerFileSetup, isRealtimeSetup } from './project'
10+
911
export const DIRECTIVE_REQUIRED_ERROR_MESSAGE =
1012
'You must specify one of @requireAuth, @skipAuth or a custom directive'
1113

1214
export const DIRECTIVE_INVALID_ROLE_TYPES_ERROR_MESSAGE =
1315
'Please check that the requireAuth roles is a string or an array of strings.'
14-
export function validateSchemaForDirectives(
16+
17+
/**
18+
* These are names that are commonly used in GraphQL schemas as scalars
19+
* and would cause a conflict if used as a type name.
20+
*
21+
* Note: Query, Mutation, and Subscription are not included here because
22+
* they are checked for separately.
23+
*/
24+
export const RESERVED_TYPES = [
25+
'Int',
26+
'Float',
27+
'Boolean',
28+
'String',
29+
'DateTime',
30+
'ID',
31+
'uid',
32+
'as',
33+
]
34+
35+
export function validateSchema(
1536
schemaDocumentNode: DocumentNode,
16-
typesToCheck: string[] = ['Query', 'Mutation', 'Subscription']
37+
typesToCheck: string[] = ['Query', 'Mutation']
1738
) {
1839
const validationOutput: string[] = []
40+
const reservedNameValidationOutput: Record<string, any> = []
1941
const directiveRoleValidationOutput: Record<string, any> = []
2042

43+
// Is Subscriptions are enabled with Redwood Realtime, then enforce a rule
44+
// that a Subscription type needs to have a authentication directive applied,
45+
// just as Query and Mutation requires
46+
if (isServerFileSetup() && isRealtimeSetup()) {
47+
typesToCheck.push('Subscription')
48+
}
49+
2150
visit(schemaDocumentNode, {
51+
InterfaceTypeDefinition(typeNode) {
52+
// Warn that an interface definition in the SDL is using a reserved GraphQL type
53+
if (RESERVED_TYPES.includes(typeNode.name.value)) {
54+
reservedNameValidationOutput.push({
55+
objectType: 'interface',
56+
name: typeNode.name.value,
57+
})
58+
}
59+
},
60+
InputObjectTypeDefinition(typeNode) {
61+
// Warn that an input definition in the SDL is using a reserved GraphQL type
62+
if (RESERVED_TYPES.includes(typeNode.name.value)) {
63+
reservedNameValidationOutput.push({
64+
objectType: 'input type',
65+
name: typeNode.name.value,
66+
})
67+
}
68+
},
2269
ObjectTypeDefinition(typeNode) {
70+
// Warn that a type definition in the SDL is using a reserved GraphQL type
71+
if (RESERVED_TYPES.includes(typeNode.name.value)) {
72+
reservedNameValidationOutput.push({
73+
objectType: 'type',
74+
name: typeNode.name.value,
75+
})
76+
}
2377
if (typesToCheck.includes(typeNode.name.value)) {
2478
for (const field of typeNode.fields ||
2579
([] as ObjectTypeDefinitionNode[])) {
@@ -102,6 +156,15 @@ export function validateSchemaForDirectives(
102156
)} \n\nFor example: @requireAuth(roles: "admin") or @requireAuth(roles: ["admin", "editor"])`
103157
)
104158
}
159+
160+
if (reservedNameValidationOutput.length > 0) {
161+
const reservedNameMsg = reservedNameValidationOutput.map(
162+
(output: Record<string, any>) => {
163+
return `The ${output.objectType} named '${output.name}' is a reserved GraphQL name.\nPlease rename it to something more specific, like: Application${output.name}.\n`
164+
}
165+
)
166+
throw new TypeError(reservedNameMsg.join('\n'))
167+
}
105168
}
106169

107170
export const loadAndValidateSdls = async () => {
@@ -137,5 +200,5 @@ export const loadAndValidateSdls = async () => {
137200
projectDocumentNodes,
138201
])
139202

140-
validateSchemaForDirectives(mergedDocumentNode)
203+
validateSchema(mergedDocumentNode)
141204
}

0 commit comments

Comments
 (0)