nexus-plugin-shield icon indicating copy to clipboard operation
nexus-plugin-shield copied to clipboard

Type-checking resolvers

Open SpaceK33z opened this issue 4 years ago • 6 comments

Nexus has type-checking everywhere, but this plugin deviates from that in that it doesn't check if the resolvers actually exist. While building my first Nexus app I made the mistake for example to put the frontPage resolver in Mutation instead of Query and only found out much later. Would be really handy if this could give an error.

const permissions = shield({
  rules: {
    Query: {
      frontPage: not(isAuthenticated),
      asdf: isAuthenticated(), // <--- I want this to give a typecheck error if it does not exist
    },
    Mutation: {
      //
    },
  }
});

I was also thinking how cool it would be if instead of having one big "permissions" files that duplicates all the resolver names etc, you could add the permission like this;

schema.extendType({
  type: 'Query',
  definition(t) {
    t.field('frontPage', {
      permission: not(isAuthenticated), // <----
      resolve() {}
      // ...
    });
  }
});

Thoughts?

SpaceK33z avatar Jul 03 '20 09:07 SpaceK33z

Hi @SpaceK33z,

There is another plugin done by @Sytten: https://github.com/Sytten/nexus-shield This is a well done @nexus/schema plugin but cannot be used directly by the framework.

We are thinking about how to merge those two projects. Maybe this one should just be a framework wrapper around the @Sytten's schema plugin.

Pro:

  • remove the dependency with original graphql-shield and graphql-middleware
  • add stronger Typings

Cons:

  • Loosing full compatibility with original graphql-shield rules
  • permissions has to be defined per field (may be a 'pro')

lvauvillier avatar Jul 04 '20 15:07 lvauvillier

I am also planning to support the framework at some point when it is a bit more stable. I am starting to think we might want to keep it as two plugins, but create an happy path for transition. So developers could do graphql-shield -> nexus-plugin-graphql-shield (this project) -> nexus-plugin-shield (my project).

Sytten avatar Jul 04 '20 21:07 Sytten

After thinking more about how to merge, this cannot be just a wrapper. We have to keep an option to define rules in a separate "big" permission file. This will allow to add permissions to cruds auto generated by prisma plugin.

I think the plan can be:

  • expose rule operators (and, or, etc...) from your project
  • integration at schema level
  • keep the current middleware logic to allow apply rules using a global definition object defined in plugin settings.

This will simplify the transition.

I'll give a try tomorrow.

lvauvillier avatar Jul 04 '20 22:07 lvauvillier

@SpaceK33z, check new version. I just add some type checks.

lvauvillier avatar Jul 05 '20 19:07 lvauvillier

Thanks @Sytten and @lvauvillier! Good to hear your thoughts about merging.

@lvauvillier the new version works very well! Since the permissions are in a separate file, I need to manually add the Settings type to the permissions object;

import { Settings } from 'nexus-plugin-shield/dist/settings';

export const permissions: Settings = {
  rules:  {
    Query: {
      frontPage: allow,
      asdf: allow // <-- if I don't add the `Settings` type above, this will not give an error
    }
  }
};

Not a problem per se, just an observation :).

SpaceK33z avatar Jul 06 '20 09:07 SpaceK33z

You can keep it in a separate file without importing the Settings type:

// permissions.ts
import { shield, allow } from "nexus-plugin-shield";
export const permissions = shield({
  rules: {
    Query: {
      frontPage: allow,
      asdf: allow
    }
  }
});

// app.ts
import { permissions } from "./permissions";
use(permissions);

lvauvillier avatar Jul 06 '20 10:07 lvauvillier