Middleware not called for GraphQL request in 7.4.2 (upgrade from 7.0.1)

[manju-reddys]

Bug Report

Upgrade from 7.0.1 to 7.4.2

Current behavior

The middleware registered to all requests are not invoked for graphql request and so the user inject into the request is not available to graphql queries.

Input Code

# main.ts
app.setGlobalPrefix('/api/v3');

// app.module.ts
// set GraphQLModule to use useGlobalPrefix
GraphQLModule.forRoot({
            debug: Boolean(process.env.GRAPHQL_ENABLE_DEBUG),
            playground: Boolean(process.env.GRAPHQL_ENABLE_PLAYGROUND) || true,
            autoSchemaFile: 'dist/apps/server/schema.gql',
            include: [DbModule, AppGraphModule],
            buildSchemaOptions: {
                dateScalarMode: 'timestamp'
            },
            uploads: false,
            useGlobalPrefix: true,
            definitions: {
                path: join(process.cwd(), 'libs/models/graphql/schema.definitions.ts'),
                skipResolverArgs: true
}

// Registered middleware

export class OpenidIdntityModule implements NestModule {
    configure(consumer: MiddlewareConsumer) {
        consumer
            .apply(PreRequestHandleMiddleware)
                .forRoutes('(.*)')            
            .apply(CallbackAuthenticationMiddleware)
                .forRoutes({
                    path: 'callback', method: RequestMethod.GET
                })
            .apply(CookieSessionValidationAndRefreshMiddleware)
                .forRoutes('(.*)');
    }
}

Expected behavior

GraphQL request must first flow through middleware where auth checks will be done and user will be injected for down steam consumption.

Below code works fine in 7.0.1 but doesn't in 7.4.2

export const Preferences = createParamDecorator(
    (data: any, ctx: ExecutionContext): string[] => {        
        const gEctx = GqlExecutionContext.create(ctx);
        const user = gEctx.getContext().req.user as User; --> Req / raw either of them should available => undefined
        if (!Utils.isEmpty(user?.preferences)) {
            const p2pPref = user.preferences.find(p => p.startsWith('p2p_exchangers'));
            if (p2pPref && p2pPref.split('__').length > 1) {
                const banks = p2pPref.split('__')[1]
                    .split('-')
                    .filter(b => b)
                    .map(
                        b => b.split('_')
                            .filter(n => n)
                            .join(' ')
                            .trim()
                    );
                return banks;
            }
        }
        return [];
    }
)

Possible Solution

Environment

Nest version: 7.4.2

 
For Tooling issues:
- Node version: 14.x
- Platform:  Mac

[jmcdo29]

Please provide a minimum reproduction.

[ayattis]

Repo: https://github.com/manju-reddys/nest_7.4.2_gql, Please see the readme for more info

[manju-reddys]

Repo: https://github.com/manju-reddys/nest_7.4.2_gql, Please see the readme for more info

[jmcdo29]

I get the feeling this is happening due to how Fastify v3 handles middleware vs how Fastify v2 used to handle it. As no changes were made to the GQL specific code, it's hard to say where the break really is. The ApolloServer is essentially registered as a middleware that does it's own handling, via line 218 of the graphql.module.ts file, so it could be an issue of middleware order, but I'm not sure how that could get fixed.

Out of curiosity, is there a reason you're relying so heavily on middleware over something like enhancers (guards and interceptors to be specific)?

[manju-reddys]

@jmcdo29 I'm using middleware to protect every request to api/* auth (oAuth2, check token, refresh if near to expire etc...). Guards we need to add to each controller which is not optimal.

[jmcdo29]

@manju-reddys why couldn't you make the guard global and have the guard check if a certain metadata is present (something like 'OAUTH_GUARD_SKIP') and if so, skip the guard by short circuiting to a true?

[manju-reddys]

That's good point but I also wants to inject user and extra security headers upon token verification which will be consumed by downstream, can we do that as well? I haven't given thought about that.

[jmcdo29]

Absolutely. Objects in JavaScript are passed by reference, meaning if you modify it in one place, you can get the same modification elsewhere. This is how passport initially handles adding req.user to the request, and how you can do things like adding extra headers, properties, or pretty much anything else to the request.

[manju-reddys]

thank you for the tips, I will give it a try sometime this week and if that works I will drop the middlewares.

[manju-reddys]

And also how do I inject the user to GrraphQL request?

[jmcdo29]

You can add it to the request object and get the request object via GqlExecutionContext.create(context).request.user or if you're inside of a resolver and using @Context() ctx you can do ctx.request.user (using request as that's what apollo-server-fastify recommends, if you map it to req then replace with req)

[manju-reddys]

Unfortunately the problem is that, the request and/or req is undefined in 7.4.2 which were present in 7.0.1

 (data: any, ctx: ExecutionContext): string[] => {        
        const gEctx = GqlExecutionContext.create(ctx);
        const user = gEctx.getContext().req.user as User; --> Req / raw either of them should available => but undefined in 7.4.2
        if (!Utils.isEmpty(user?.preferences)) {

[jmcdo29]

Do you have the request added to the contrxt as described by the Apollo server docs? For fastify the setting is context: ({ request, reply }) => ({ request, reply })

[manju-reddys]

Would you mind pointing me to the right documentation or code reference how I should add context in NestJs? Closest one I found is apollographql/apollo-server#3156 I didn't try yet.

[jmcdo29]

https://docs.nestjs.com/techniques/authentication#graphql

[manju-reddys]

Do you have the request added to the contrxt as described by the Apollo server docs? For fastify the setting is context: ({ request, reply }) => ({ request, reply })

@jacob87o2 sorry if I wasn't clear, I want to understand how to set the context so the request, reply will be available in the GraphQL context which is missing in 7.4.2

[jmcdo29]

The link I posted has this snippet, which is essentially what you're looking for:

GraphQLModule.forRoot({
  context: ({ req }) => ({ req }),
});

Mix that with this API doc and you should have what you need.

[kamilmysliwiec]

Request related issues have been fixed.