Skip to content

Commit

Permalink
fix(fastify): Ensure global context is unique and scoped to an indivi…
Browse files Browse the repository at this point in the history
…dual request lifetime (#8569)

* Add hook to setup the async store

* Switched to use async local store for the context of all requests.

Fixed the failing tests.

* Remove legacy code

* Fixed mockRedwoodDirective to correctly handle the fake request scope.

Minor changes to the global context file too.

* Add context isolating hook to the api fastify plugin too.

This is likely inefficient to have it defined in both plugins unconditionally but right now I think it ensures context safety for all API requests.

* lint
  • Loading branch information
Josh-Walker-GM authored Jun 24, 2023
1 parent d851609 commit c5921bc
Show file tree
Hide file tree
Showing 11 changed files with 190 additions and 339 deletions.
9 changes: 9 additions & 0 deletions packages/fastify/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ import fastifyUrlData from '@fastify/url-data'
import type { FastifyInstance, HookHandlerDoneFunction } from 'fastify'
import fastifyRawBody from 'fastify-raw-body'

import type { GlobalContext } from '@redwoodjs/graphql-server'
import { getAsyncStoreInstance } from '@redwoodjs/graphql-server'

import { loadFastifyConfig } from './config'
import { lambdaRequestHandler, loadFunctionsFromDist } from './lambda'
import type { RedwoodFastifyAPIOptions } from './types'
Expand All @@ -14,6 +17,12 @@ export async function redwoodFastifyAPI(
fastify.register(fastifyUrlData)
await fastify.register(fastifyRawBody)

// TODO: This should be refactored to only be defined once and it might not live here
// Ensure that each request has a unique global context
fastify.addHook('onRequest', (_req, _reply, done) => {
getAsyncStoreInstance().run(new Map<string, GlobalContext>(), done)
})

fastify.addContentTypeParser(
['application/x-www-form-urlencoded', 'multipart/form-data'],
{ parseAs: 'string' },
Expand Down
27 changes: 14 additions & 13 deletions packages/fastify/src/graphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,14 @@ import fastifyUrlData from '@fastify/url-data'
import type { FastifyInstance, HookHandlerDoneFunction } from 'fastify'
import fastifyRawBody from 'fastify-raw-body'

import { createGraphQLYoga } from '@redwoodjs/graphql-server'
import type { GraphQLYogaOptions } from '@redwoodjs/graphql-server'
import type {
GraphQLYogaOptions,
GlobalContext,
} from '@redwoodjs/graphql-server'
import {
createGraphQLYoga,
getAsyncStoreInstance,
} from '@redwoodjs/graphql-server'

/**
* Transform a Fastify Request to an event compatible with the RedwoodGraphQLContext's event
Expand All @@ -14,17 +20,6 @@ import { lambdaEventForFastifyRequest as transformToRedwoodGraphQLContextEvent }
/**
* Redwood GraphQL Server Fastify plugin based on GraphQL Yoga
*
* Important: Need to set DISABLE_CONTEXT_ISOLATION = 1 in environment variables
* so that global context is populated correctly and features such as authentication
* works properly.
*
* It is critical to set shouldUseLocalStorageContext correctly so that the `setContext` function
* in the `useRedwoodPopulateContext` plugin sets the global context correctly with any
* extended GraphQL context as is done with `useRedwoodAuthContext` that sets
* the `currentUser` in the context when used to authenticate a user.
*
* See: packages/graphql-server/src/globalContext.ts
*
* @param {FastifyInstance} fastify Encapsulated Fastify Instance
* @param {GraphQLYogaOptions} options GraphQLYogaOptions options used to configure the GraphQL Yoga Server
*/
Expand All @@ -42,6 +37,12 @@ export async function redwoodFastifyGraphQLServer(
try {
const { yoga } = createGraphQLYoga(options)

// TODO: This should be refactored to only be defined once and it might not live here
// Ensure that each request has a unique global context
fastify.addHook('onRequest', (_req, _reply, done) => {
getAsyncStoreInstance().run(new Map<string, GlobalContext>(), done)
})

fastify.route({
url: yoga.graphqlEndpoint,
method: ['GET', 'POST', 'OPTIONS'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { APIGatewayProxyEvent, Context } from 'aws-lambda'
import { createLogger } from '@redwoodjs/api/logger'

import { createGraphQLHandler } from '../../functions/graphql'
import { context } from '../../globalContext'

jest.mock('../../makeMergedSchema', () => {
const { makeExecutableSchema } = require('@graphql-tools/schema')
Expand All @@ -17,9 +18,11 @@ jest.mock('../../makeMergedSchema', () => {
}
type User {
id: ID!
firstName: String!
lastName: String!
id: ID!
token: String!
roles: [String!]!
}
`,
resolvers: {
Expand All @@ -29,9 +32,11 @@ jest.mock('../../makeMergedSchema', () => {
const currentUser = globalContext.currentUser

return {
id: currentUser.userId,
firstName: 'Ba',
lastName: 'Zinga',
id: currentUser.userId,
token: currentUser.token,
roles: currentUser.roles,
}
},
},
Expand Down Expand Up @@ -117,20 +122,6 @@ const mockLambdaEvent = ({
}

describe('createGraphQLHandler', () => {
beforeAll(() => {
process.env.DISABLE_CONTEXT_ISOLATION = '1'
})

afterAll(() => {
process.env.DISABLE_CONTEXT_ISOLATION = '0'
})

afterEach(() => {
// Clean up after test cases
const globalContext = require('../../globalContext').context
delete globalContext.currentUser
})

const adminAuthDecoder: Decoder = async (token, type) => {
if (type !== 'admin-auth') {
return null
Expand Down Expand Up @@ -178,20 +169,18 @@ describe('createGraphQLHandler', () => {
'auth-provider': 'admin-auth',
authorization: 'Bearer auth-test-token-admin',
},
body: JSON.stringify({ query: '{ me { id, firstName, lastName } }' }),
body: JSON.stringify({
query: '{ me { id, firstName, lastName, token, roles } }',
}),
httpMethod: 'POST',
})

const response = await handler(mockedEvent, {} as Context)

const body = JSON.parse(response.body)
const globalContext = require('../../globalContext').context
const currentUser = globalContext.currentUser

expect(body.data.me.id).toEqual('admin-one')
expect(currentUser.userId).toEqual('admin-one')
expect(currentUser.token).toEqual('auth test token admin')
expect(currentUser.roles).toEqual(['admin'])
expect(body.data.me.token).toEqual('auth test token admin')
expect(body.data.me.roles).toEqual(['admin'])
expect(response.statusCode).toBe(200)
})

Expand All @@ -212,20 +201,19 @@ describe('createGraphQLHandler', () => {
'auth-provider': 'admin-auth',
authorization: 'Bearer auth-test-token-admin',
},
body: JSON.stringify({ query: '{ me { id, firstName, lastName } }' }),
body: JSON.stringify({
query: '{ me { id, firstName, lastName, token, roles } }',
}),
httpMethod: 'POST',
})

const response = await handler(mockedEvent, {} as Context)

const body = JSON.parse(response.body)
const globalContext = require('../../globalContext').context
const currentUser = globalContext.currentUser

expect(body.data.me.id).toEqual('admin-one')
expect(currentUser.userId).toEqual('admin-one')
expect(currentUser.token).toEqual('auth test token admin')
expect(currentUser.roles).toEqual(['admin'])
expect(body.data.me.token).toEqual('auth test token admin')
expect(body.data.me.roles).toEqual(['admin'])
expect(response.statusCode).toBe(200)
})

Expand All @@ -246,20 +234,19 @@ describe('createGraphQLHandler', () => {
'auth-provider': 'customer-auth',
authorization: 'Bearer auth-test-token-customer',
},
body: JSON.stringify({ query: '{ me { id, firstName, lastName } }' }),
body: JSON.stringify({
query: '{ me { id, firstName, lastName, token, roles } }',
}),
httpMethod: 'POST',
})

const response = await handler(mockedEvent, {} as Context)

const body = JSON.parse(response.body)
const globalContext = require('../../globalContext').context
const currentUser = globalContext.currentUser

expect(body.data.me.id).toEqual('customer-one')
expect(currentUser.userId).toEqual('customer-one')
expect(currentUser.token).toEqual('auth test token customer')
expect(currentUser.roles).toEqual(['user'])
expect(body.data.me.token).toEqual('auth test token customer')
expect(body.data.me.roles).toEqual(['user'])
expect(response.statusCode).toBe(200)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ import {
} from '../../globalContext'

describe('Global context with context isolation', () => {
beforeAll(() => {
process.env.DISABLE_CONTEXT_ISOLATION = '0'
})

it('Should work when assigning directly into context', async () => {
const asyncStore = getAsyncStoreInstance()

Expand Down Expand Up @@ -36,4 +32,21 @@ describe('Global context with context isolation', () => {
// Check that context was isolated
expect(globalContext.anotherValue).not.toBe('kittens')
})

it('setContext replaces global context', async () => {
const asyncStore = getAsyncStoreInstance()

asyncStore.run(new Map(), () => {
// This is the actual test
globalContext.myNewValue = 'bazinga'
setContext({ anotherValue: 'kittens' })

expect(globalContext.myNewValue).toBeUndefined()
expect(globalContext.anotherValue).toBe('kittens')
})

// Check that context was isolated
expect(globalContext.myNewValue).toBeUndefined()
expect(globalContext.anotherValue).toBeUndefined()
})
})
141 changes: 0 additions & 141 deletions packages/graphql-server/src/functions/__tests__/graphql.test.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -146,20 +146,6 @@ const getCurrentUserWithError = async (
}

describe('useRequireAuth', () => {
beforeAll(() => {
process.env.DISABLE_CONTEXT_ISOLATION = '1'
})

afterAll(() => {
process.env.DISABLE_CONTEXT_ISOLATION = '0'
})

afterEach(() => {
// Clean up after test cases
const globalContext = require('../../globalContext').context
delete globalContext.currentUser
})

it('Updates context with output of current user', async () => {
// @MARK
// Because we use context inside useRequireAuth, we only want to import this function
Expand Down
Loading

0 comments on commit c5921bc

Please sign in to comment.