Support Route level hooks for grafserv/fastify/v4

[jcgsville]

Feature description

In the fastify grafserv server, there are several calls to app.route() to set up the relevant routes with fastify. I think it would be useful to be able to register fastify hooks to individual routes.

To do this, I think you'd need some server-specific settings in Grafserv options? I'm not sure if that's already a pattern somewhere, or whether that adds coupling where we want grafserv and the specific servers to remain decoupled.

I'm thinking the grafserv config would include something like

{
    // ... other grafserv config properties
    fastifyV4: {
        routeHooks: {
            '/graphql': {
                preHandler: () => {}
            }
        }
    }
}

Motivating example

I'm trying to find the cleanest way to add a hook to my postgraphile + fastify application that validates the access token present in request cookies and returns a 401 before the request starts executing grafast/postgraphile-related things.

Because grafserv's calls to app.route() are inaccessible to my code, I think my best option without this feature request would be to create a server-wide hook and check the path within the hook to determine whether the call is to /graphql or some other route. If it's a call to /graphql, then I do the validation.

Supporting development

I:

• [ ] am interested in building this feature myself
• [x] am interested in collaborating on building this feature
• [x] am willing to help testing this feature before it's released
• [x] am willing to write a test-driven test suite for this feature (before it exists)
• [x] am a Graphile sponsor ❤️
• [ ] have an active support or consultancy contract with Graphile

[benjie]

I'm not super familiar with Fastify, so I'd need a few people who use it to confirm that's the right approach. For now, you should be able to write your own fastify adaptor: just take a copy of the fastify adaptor that already exists, and then apply your changes to it:

https://github.com/graphile/crystal/blob/main/grafast/grafserv/src/servers/fastify/v4/index.ts

Hopefully everything it imports is available as an export from grafserv? :crossed_fingers:

There should be no other files you'd need to touch to add configuration options for fastify within grafserv, just use declaration merging:

interface FastifyV4GrafservOptions {
  // ...
}
declare global {
  namespace GraphileConfig {
    interface GrafservOptions {
      fastifyV4?: FastifyV4GrafservOptions
    }
  }
}

If you think lots of plugins will want to be able to hook into this then you'll want to use graphile-config's methods for handling plugins which I don't think are documented yet but you can find throughout the codebase. You probably want AsyncHooks to enable your plugins to do async things? Note addTo is already async, so waiting on hooks in there should be fine.

[jcgsville]

For future readers, I took a slightly different approach for now which I think it a bit less idiomatic, but works fine. I think it has the small cost of running the extra javascript of the hook on every route, but it's just a trivial if statement for routes other than the ones I want to authenticate

.addHook('onRequest', async (request, reply) => {
    if (request.routeOptions.url === '/graphql') {
        return authenticateRequest(request, reply)
    }
})

where authenticateRequest() contains my authn logic. Hope that helps others.

The addition of this to the fastify adaptor is simple enough that I believe I would be comfortable with taking this on and willing to put in the time to do so if you decide that it's a worthwhile change pending feedback from others who are more deeply familiar with fastify. Just lmk!

[benjie]

I’m happy adding things so long as they don’t have a noticeable performance impact for people not using the feature, and so long as the feature actually seems desirable and isn’t better served via an alternative solution. 👍

[benjie]

(But as I say; you can experiment with this external to core, even share the code so others can try it, and then we can merge it once general consensus supports it.)