graphql-shield: Custom errors for rules and global fallback

Hi @maticzav! First of all thanks for the awesome library!

I have few questions/suggestions about CustomErrors:

  1. Is there a way to specify CustomError that will be returned from shield in case if permissions checks are failed?

Example: I have isAuthenticated rule

export default rule()((parent, args, { user }) => Boolean(user));

I want to return my custom 401 error like this

import { SevenBoom } from 'graphql-apollo-errors';

rule(null, { 
  customError: SevenBoom.unauthorized('You are not authenticated')
})((parent, args, { user }) => Boolean(user))
  1. Is there a way to specify global CustomError instead of https://github.com/maticzav/graphql-shield/blob/master/src/index.ts#L234 which will be used in case if there is no rule specific error?

Example:

import { SevenBoom } from 'graphql-apollo-errors';

shield({ /* Rules… */ }, {
  customError: SevenBoom.forbidden('You are not allowed to do this action.')
})

I can implement this and looks like it is not a breaking change.

Just want to clarify if there is no way to do this in current version of the library?

Let me know if you have any questions. Thanks!

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Comments: 23 (21 by maintainers)

Most upvoted comments

Is there no way to just map error messages to rules? That seems like the way to go.

I’m a few years late, but this seems to be a general problem with functions doing more than one thing. Rules should just test and return a boolean. That’s a separate concern from deciding which error to throw.

@maticzav by some reason I can not receive invite to Prisma Slack. Any chance you are using Prisma Spectrum chat? https://spectrum.chat/prisma

@maticzav I will join and ping you tomorrow then!

@maticzav I believe they are just alternatives to each other and it is just matter of preference how you would like to specify custom error.

In my opinion returning an error instead of passing it as param gives more flexibility because then rule may have many custom errors specified in case you need them for some complex one. And it is hard to imagine implemented with passing an error as param.

rule()((parent, args, ctx, info) => {
  if (args.something) {
    return new Error('Something');
  } else if (args.somethingElse) {
    return new Error('Something else');
  } else if (args.evenMore) {
    return new Error('Even more!');
  }	
  return true;
});

@maticzav thanks for the fast response!

  1. Not sure if we understand each other correctly on this one. I will try to explain more clear.

Let’s imagine I have 2 rules: isAuthenticated and isAdmin.

// isAuthenticated.js
import { shield, rule, and } from 'graphql-shield';

// User should be present in context
const isAuthenticated = rule()((parent, args, ctx) => Boolean(ctx.user));
// User role should be equal to 'admin'
const isAdmin = rule()((parent, args, ctx) => ctx.user.role === 'admin');

// I am combining this 2 rules together
const permissions = shield({
  Query: {
    getAllUsers: and(isAuthenticated, isAdmin)
  }
});

My goal to have this:

  1. If there is no user in the context(isAuthenticated is false) then throw 401 error.
  2. If the user is present but role is not an admin(isAdmin rule is false) then throw 403 error.

If I understood correctly now we always will have CustomError('Not Authorised!') if rules check was failed, right?

  1. Adding global customizable default error is fine, right?

Thanks for helping with this!

@kolybasov this is great. I have nothing to add to second question, let’s make a PR! 🎉

Right now, there are two ways to solving the first question. You could use allowExternalErrors which will allow all errors that happen during execution to pass through. The second one is by throwing CustomError in rule evaluation.

Hope this answers your question 🙂