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

feat: filtered introspection queries #46

Merged
merged 35 commits into from
Nov 24, 2021
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
b0a34e2
wip: raw demo code
Eomm Nov 9, 2021
c6c7c04
wip
Eomm Nov 11, 2021
1d07402
Merge remote-tracking branch 'upstream/main' into filter-schema
Eomm Nov 11, 2021
526d42b
single hook
Eomm Nov 12, 2021
2224940
wip filtering inspection schema
Eomm Nov 12, 2021
2217b46
fix prune
Eomm Nov 15, 2021
30a3d2f
fix tests
Eomm Nov 15, 2021
cac0cfb
wip: test
Eomm Nov 15, 2021
adfab99
fix policy execution
Eomm Nov 15, 2021
94b46b7
full coverage
Eomm Nov 15, 2021
ff27b42
feat: namespace
Eomm Nov 16, 2021
bf97056
fix filter schema once
Eomm Nov 16, 2021
31ebe24
filter schema once
Eomm Nov 16, 2021
8cbfca8
add UNION tests
Eomm Nov 16, 2021
e1ed78d
refactor
Eomm Nov 16, 2021
0d9d733
check optimization
Eomm Nov 16, 2021
18fd4fd
fix coverage
Eomm Nov 16, 2021
dd0e7bd
fix info argument
Eomm Nov 18, 2021
f31db99
add docs
Eomm Nov 19, 2021
990d13e
rename option to filterSchema
Eomm Nov 22, 2021
1e75662
repeatable directive test
Eomm Nov 22, 2021
21f8392
add gateway test
Eomm Nov 22, 2021
1052b46
add tests
Eomm Nov 22, 2021
4da1bfe
add tests
Eomm Nov 22, 2021
9ace190
fix gateway refresh
Eomm Nov 23, 2021
fe33ea7
add types
Eomm Nov 23, 2021
9a768d3
improved docs
Eomm Nov 23, 2021
9fea7ab
fix todos
Eomm Nov 23, 2021
73a56e9
trigger ci
Eomm Nov 23, 2021
925f2fb
trigger ci - takes 2
Eomm Nov 23, 2021
34d8b6e
Update docs/api/options.md
Eomm Nov 23, 2021
d50f00c
docs feedback
Eomm Nov 24, 2021
c711390
update dep
Eomm Nov 24, 2021
96b72fe
add docs
Eomm Nov 24, 2021
28a7db4
Merge branch 'filter-schema' of https://github.com/Eomm/auth into fil…
Eomm Nov 24, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const fp = require('fastify-plugin')
const Auth = require('./lib/auth')
const { validateOpts } = require('./lib/validation')
const filterSchema = require('./lib/filter-schema')

const plugin = fp(
async function (app, opts) {
Expand All @@ -26,6 +27,10 @@ const plugin = fp(
if (typeof opts.authContext !== 'undefined') {
app.graphql.addHook('preExecution', auth.authContextHook.bind(auth))
}

if (opts.namespace && opts.authDirective) {
filterSchema(app, authSchema, opts)
Eomm marked this conversation as resolved.
Show resolved Hide resolved
}
},
{
name: 'mercurius-auth',
Expand Down
163 changes: 163 additions & 0 deletions lib/filter-schema.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
'use strict'

const {
wrapSchema,
PruneSchema,
FilterTypes,
TransformObjectFields
} = require('@graphql-tools/wrap')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

graphql-tools is not a depdency. Could we avoid it? Or just add it as a dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added it to the package.json

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How feasible would it be to not include this dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have checked it and it requires a lot of GraphQL knowledge about the spec and the graphqljs implementation to manage all the use cases.

I'm monitoring this issue: graphql/graphql-js#113
Or it could be best to include this feature in the official graphql implementation.

At the moment the issue is 5 years old but still active (people want it)


const kDirectiveNamespace = Symbol('mercurius-auth.namespace')

const { GraphQLObjectType } = require('graphql')

module.exports = function filterIntrospectionSchema (app, policy, { namespace, applyPolicy: policyFunction }) {
if (!app[kDirectiveNamespace]) {
app[kDirectiveNamespace] = {}

// the filter hook must be the last one to be executed (after all the authContextHook ones)
app.ready(err => {
/* istanbul ignore next */
jonnydgreen marked this conversation as resolved.
Show resolved Hide resolved
if (err) throw err
app.graphql.addHook('preExecution', filterGraphQLSchemaHook(namespace).bind(app))
})
}

if (app[kDirectiveNamespace][namespace]) {
app[kDirectiveNamespace][namespace].push({
policy,
policyFunction
})
} else {
app[kDirectiveNamespace][namespace] = [{
policy,
policyFunction
}]
}
}

function filterGraphQLSchemaHook (namespace) {
return async function filterHook (schema, document, context) {
if (!isIntrospection(document)) {
return
}

const filteredSchema = await filterSchema(schema,
this[kDirectiveNamespace][namespace],
context)

return {
schema: filteredSchema
}
}
}

function isIntrospection (document) {
for (const queryType of document.definitions) {
if (queryType.operation !== 'query') {
// if there is a mutation or subscription, we can skip the introspection check
break
}

// if there is an introspection operation, we must filter the schema
if (queryType.selectionSet.selections.some(sel => (
sel.name.value === '__schema' ||
sel.name.value === '__type'
))) {
return true
}
}
return false
}

async function filterSchema (graphQLSchema, policies, context) {
const filterDirectiveMap = {}

// each `policies` item is a directive
let skipFiltering = true
for (const { policy, policyFunction } of policies) {
// each `policy` contains all the GraphQL OBJECT and FIELDS that are affected by the directive
for (const [typeName, typePolicy] of Object.entries(policy)) {
// different `policies` item can affect the same GraphQL OBJECT
if (filterDirectiveMap[typeName] === undefined) {
filterDirectiveMap[typeName] = {}
} else if (filterDirectiveMap[typeName] === false) {
// if the object has already been filtered, we can skip the field processing
continue
}

const schemaType = graphQLSchema.getType(typeName)
const schemaTypeFields = typeof schemaType.getFields === 'function'
? schemaType.getFields()
: {}
for (const [fieldName, fieldPolicy] of Object.entries(typePolicy)) {
// each `fieldName` is a single GraphQL item associated with the directive

if (filterDirectiveMap[typeName] === false || filterDirectiveMap[typeName][fieldName] === false) {
// if we have already decided to filter out this field
// it does not need to be checked again
continue
jonnydgreen marked this conversation as resolved.
Show resolved Hide resolved
}

let canShowDirectiveField = true
const isObjectPolicy = fieldName === '__typePolicy'
try {
// https://github.com/graphql/graphql-js/blob/main/src/type/definition.ts#L974
const info = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to create the GraphQLResolveInfo that contains information about the operation's execution state, including the field name.

I did not cover all the cases, but it will be slightly different from the info argument received as input by the resolver function.

Any suggestion to recreate it is highly appreciated

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the slow response, I've been very busy this week - I will have a look this weekend if that's okay!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple of functions exported from the graphql package that might be of interest here:

  • buildExecutionContext
  • buildResolveInfo (requires an execution context)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not find a way to build the info object actually.

Would you mind defining a subset info object in the first place?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you anticipate this being problematic for the type definition of the GraphQLResolveInfo type?

Depending on the data included, I think this is okay for the initial release of this feature if it's not possible - we'd need to:

  • Make this clear in the documentation

What was blocking you on this out of interest?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me make an example:
if you assign the @auth directive to a type MyObject and you will set this type as output for two different Query, the info object depends on the query that is being executed - the resolver changes.

Instead, during the filtering, the applyPolicy function is not being executed as a resolver wrapper, but it is executed on the MyObject type without any other context.

For this reason I was unable to build the same argument

fieldName,
fieldNodes: schemaType.astNode.fields,
returnType: isObjectPolicy ? '' : schemaTypeFields[fieldName].type, // TODO
jonnydgreen marked this conversation as resolved.
Show resolved Hide resolved
parentType: schemaType,
schema: graphQLSchema,
fragments: {},
rootValue: {},
operation: { kind: 'OperationDefinition', operation: 'query' },
variableValues: {}
}
// The undefined parameters are: https://graphql.org/learn/execution/#root-fields-resolvers
// - parent: it is not possible know it since the resolver is not executed yet
// - args: it is not expected that the introspection query will have arguments for the directives policies
canShowDirectiveField = await policyFunction(fieldPolicy, undefined, undefined, context, info)
Eomm marked this conversation as resolved.
Show resolved Hide resolved
if (canShowDirectiveField instanceof Error || !canShowDirectiveField) {
canShowDirectiveField = false
}
} catch (error) {
canShowDirectiveField = false
}

skipFiltering = skipFiltering && canShowDirectiveField
if (canShowDirectiveField === false && isObjectPolicy) {
// the directive is assigned to a GraphQL OBJECT so we need to filter out all the fields
filterDirectiveMap[typeName] = canShowDirectiveField
} else {
filterDirectiveMap[typeName][fieldName] = canShowDirectiveField
}
}
}
}

if (skipFiltering) {
return graphQLSchema
}
return wrapSchema({
schema: graphQLSchema,
transforms: [
new FilterTypes(type => {
// should we filter out this whole type?
return filterDirectiveMap[type.name] !== false
}),
new TransformObjectFields((typeName, fieldName, fieldConfig) => {
if (filterDirectiveMap[typeName] && filterDirectiveMap[typeName][fieldName] === false) {
return null // omit the field
}
return undefined // unchanged
}),
new PruneSchema({
skipPruning (type) {
// skip pruning if the type is the Query or Mutation object
return type instanceof GraphQLObjectType && (type.name === 'Query' || type.name === 'Mutation')
}
})
]
})
}
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"autocannon": "^7.0.5",
"concurrently": "^6.1.0",
"fastify": "^3.0.2",
"mercurius": "^8.0.0",
"mercurius": "github:mercurius-js/mercurius#master",
Eomm marked this conversation as resolved.
Show resolved Hide resolved
"pre-commit": "^1.2.2",
"snazzy": "^9.0.0",
"standard": "^16.0.3",
Expand All @@ -48,6 +48,7 @@
"wait-on": "^6.0.0"
},
"dependencies": {
"@graphql-tools/wrap": "^8.3.2",
"fastify-error": "^0.3.0",
"fastify-plugin": "^3.0.0",
"graphql": "^15.4.0"
Expand Down