Urigo / graphql-modules

Enterprise Grade Tooling For Your GraphQL Server
https://graphql-modules.com/
MIT License
1.3k stars 114 forks source link

Overriding Middlewares #1775

Open spencermefford opened 3 years ago

spencermefford commented 3 years ago

I'm loving the middlewares concept of GraphQL Modules!

I am struggling with something though, and wonder if anyone can help. I have a set of 20 or so queries in a module that all need to use an authorization middleware, but 2 of those queries need no middleware at all. I was hoping I could set the middleware for * and then override the 2 that don't need it like this:

export default createModule({
  id: 'my-module',
  dirname: __dirname,
  typeDefs,
  resolvers,
  middlewares: {
    '*': {
      '*': [authorization],
    },
    Query: {
      noAuthQuery: [], // Override these with no middleware
    }
  },
});

However, that approach does not work since middlewares aren't coded to merge over one another. I could write out each "with auth" query and set them all to the middleware one by one, but that doesn't scale well and we're likely to forget one. I could also create a new module just for the 2 "without auth" queries, but that seems heavy since they all rely on the same types. Is there any other way I could accomplish my goal?

Thanks!

dotansimha commented 3 years ago

Hi @spencermefford ! Currently, we are only adding middleware and not overriding. @kamilkisiela thoughts?

AlissonRS commented 3 years ago

@dotansimha @kamilkisiela

I was thinking this could be a new Lifecycle Hook (e.g OnBeforeResolve) that would be called after all middlewares, but before the resolver.

If you agree, I'm keen on making a PR for it.

AlissonRS commented 3 years ago

There are two solutions I can see for this without introducing breaking changes.

1 - Add a new option in the module config to set whether middlewares should run from top > bottom or the other way around. By default it's top > bottom (as it's right now, so existing codebases wouldn't be affected), but other people would have the ability to specify a different order strategy.

2 - Add a new Lifecycle Hook OnBeforeResolve (like we have OnDestroy), this would run after all middlewares, but before the actual resolver. I've seen this been referred to as "filters" in other frameworks, so you have things like "OnExecuting", "OnExecuted" and so on. This would provide us a last chance to run some custom logic before all middlewares have been executed. If one would use a provider to store the result of various middlewares (attached to current context), this Lifecycle Hook would allow you to further inspect these results and apply some additional logic.

AlissonRS commented 3 years ago

@spencermefford as a side note, I was able to get this behavior by doing this:

1 - Add your auth middleware (will apply to the whole schema) 2 - Add a skipAuth schema directive like below:

Add this in your schema (typeDefs):

directive @skipAuth on FIELD_DEFINITION

yourField: String! @skipAuth

Refer to this link.

Then create a class with your directive logic. Here we will just append a skipAuth field to the context:

export class SkipAuthDirective extends SchemaDirectiveVisitor {
    public visitFieldDefinition(field: GraphQLField<any, any>) {
        const originalResolver = field.resolve;

        if (originalResolver) {
            field.resolve = (source: {}, args: {}, context, info) => {
                context.skipAuth = true; // append this, we will use this in the auth middleware later
                return originalResolver!(source, args, context, info);
            };
        }
    }
}

Create this function to configure your new directive:

export function configureDirectives(app: Application) {
    SchemaDirectiveVisitor.visitSchemaDirectives(app.schema, {
        skipAuth: SkipAuthDirective,
               // you can add more directives here
    });
}

Make sure you call this new function after you create the application with your modules:

export const app = createApplication({
    modules: [yourModule],
});

configureDirectives(app);

Now in your middleware, check if that skipAuth is there, otherwise apply your existing logic:

export function authorization({ context }: any, next: Next) {
    if (context.skipAuth) {
        return next(); // ignore auth
    }
       // otherwise, apply usual authorization logic
}

I got inspired by this solution after reading #1293

spencermefford commented 3 years ago

@AlissonRS Thank you, that's a very clever solution! I think that will solve our needs for now. In the future, it would be cool to see the middleware code become a bit more configurable like mentioned above, but I'm happy with this for now. Thank you so much!