-
Notifications
You must be signed in to change notification settings - Fork 275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Authorization #23
Authorization #23
Conversation
Good stuff. I like this approach. When documentation is written for authorization I think it would be a great idea to show an example of reusable
And then later I really like composing authChecks out of lots of smaller atomic functions and I think documentation examples which illustrate that would be powerful. |
We get used to resolvers, so another resolver to check user permission fits great.
{
// Mostly I use higher order resolver
addPost: createResolver({
roles: ['admin', 'manager'],
resolve: (root, args, ctx) => { }
}),
// But also checking inside resolver
editPostPost: createResolver({
roles: ['admin', 'manager'],
resolve: (root, args, ctx) => {
const post = ctx.repos.post.get(args.id);
if (ctx.user.role === 'manager' && post.authorId != ctx.user.id) {
throw new Error('Forbidden');
}
}
}),
}
Require |
As it stands at the moment can we use |
Yes |
Just a few thoughts (feel free to ignore them if not applicable):
(edit: after a better look I guess this is more about authorization than authentication, sorry! I'll still leave the links here just in case they're useful to anyone :) |
@tgriesser in regards to |
I thought everyone was on the "directive permissions" train until everyone ditched SDL for code-first solutions that don't support directives 😑 Anyway - I think the big problem with the Way back in 2017 someone proposed passing a |
Hey @SachaG, thanks for the links! Yeah, this is more around authorization - determining which mutations can be executed, which fields can be fetched/returned etc. The general stance by GraphQL is to push that to the business layer, though I think there's room for something slightly more opinionated/declarative. In terms of general authentication, I'm not sure if Nexus should take any sort of stance here since (at least as far as I've seen) that's typically something handled outside of the scope of GraphQL. Accounts.js looks really neat though, I'm going to look into it more! @andyk91 There is no real integration, you can currently just layer it over your schema as though you were using it with any other GraphQL schema. The reason I'm leaning against integrating with graphql-shield via a plugin or otherwise, is you'd lose some of the type-safety benefits nexus provides without re-inventing a lot of the same concepts that might be simpler expressed with something more integrated. Also I think the concepts of "caching" can be eliminated by pushing it to a more robust context object/data-fetching layer. |
@colinmcd94 that's not the case with the way it's setup in the example, the
I think a lot of these ideas are mirrored very nicely (albeit differently) in graphql-ruby with the concept of scoping and authorization both at the field-level and the type-level. I'm going to explore these concepts a bit more to see how they might look in an implementation here. |
[update: didn't see your response before submitting this]: I think considering the power of directives is a good way to consider potential scenarios you wouldn't have anticipated. You can attach a directive to an object type, object field, input type, input field, argument, interface, union, enum, and enum value. It's interesting to think about how a directive permission could be used on each of these data types. One scenario that isn't accounted for with the current approach: the ability to apply an authorization rule to entire model. This is possible in graphql-shield. Also: you mention here wanting to do validation/authorization of nested mutation input. Please do this! It would be a total game changer. Or at the very least, you should be able to mark a particular relation as "off limits" to nested mutations. (Sorry this is probably more relevant to the nexus-prisma plugin repo). |
Ah great! I hadn't looked into the implementation deeply enough. |
Yeah that's what we do in Vulcan.js. It ends up looking pretty similar to what you're proposing actually, except as a shorthand each field can also take an array of user group names that are allowed to perform an operation instead of the explicit function. Here's the relevant documentation if you're curious, although I'll admit the current implementation still feels a little clunky. It's just had to find one consistent API to handle things like reading documents, mutating them, and then individual field permissions as well.
I've also noticed this, but I wonder if the reason for that isn't simply the lack of good tools/boilerplates for GraphQL authentication, and if maybe that trend isn't going to change? |
This is just the beginning of the approach. This ticket is to discuss and implement these scenarios.
Yeah, might be better for discussion/feature requests over there - though possibly related, in the next major GraphQL release it looks like there will (hopefully) be validation in input types that will help with this. |
Thanks! I'll take a look / will take a look more into how Vulcan is doing schemas in general since it looks like you've gone with the code-first approach as well. Would love to hear any other thoughts on where you've found rough-edges with that approach, and whether nexus or a wrapper around it might ever be a good fit with what you're trying to do!
I don't know if it's good tools vs. just not the best tool for the job. I've found with authentication you have a lot more server redirects, errors, headers, things that you actually want non-200 http status codes for that feel a bit awkward to try to fit into the GraphQL model. |
Honestly that approach seems to work pretty well, but Vulcan is a bit special as its goal is to abstract away the GraphQL layer, at least when you're getting started. Probably the main downside is that code-first can be a bit more verbose compared to just adding one line to a GraphQL schema document. But I think the trade-off is worth it in the end.
That's a great point, I hadn't really thought about it like that. |
One struggle I have faced with GraphQL authorization in general is deciding when to delegate authorization to the appropriate resolver and when to utilize the authorization layer (be it the To illustrate one potential tricky use-case with lists, suppose I want to include a Briefly illustrated in SDL: type Query {
...
products(where: ProductsWhere): [Product]
}
type Product {
...
owner: User!
} As a first step, I can include an
I have not given the problem much thought, but I am curious as to (a) whether others have taken a different or better approach to similar impasses and (b) whether Nexus might be able to address this use-case to keep authorization nicely encapsulated! |
Coming to the discussion a bit late! I think the concept of I started trying to look for ways to hide fields from the introspection query, found that issue ardatan/graphql-tools#323 (which has never been solved), which lead me to ruby's visibility concept, which reminded me that you've mentioned it here. As stated in the ruby documentation, removing fields from your introspection schema based on auth has two main use-cases:
Most e-commerce related app would probably want that to hide all the CRUD operations to manage products/inventory/orders etc etc! |
Like a lot of people in the GraphQL community, me and my team have also faced some of the challenges that aren't clearly identified in the spec. The main points of this being the classic use cases that would fall under the express-style-middlewares. Authorization is one of the most important concepts when it comes to overall API design and therefor is a high priority detail in a GraphQL API as well. I personally have seen two general approaches here:
For situations where the first approach is more suitable I think there isn't much more GraphQL or any library or framework has to offer because the way authorization will be taken care of is determined by a down-stream service and the most common approach is to just find a way to bubble up the error. The second approach is where I fee like libraries and frameworks like Nexus could offer a great first-class API and make this experience better to solve. I've not yet used the ruby implementation but from reading their docs I very much like the approach they've taken and I think concepts like When it comes to plain authorization and basically checking if a user has permissions to execute a certain field or mutation we've mostly been using an approach that closely aligns with what @umidbekkarimov has explained. It mostly involves writing higher order helper functions that wrap multiple resolvers into one. I personally like this approach as it is fairly easy to grasp and explain to new team members and it follows the basic building blocks of a GraphQL implementation. I also think that with this approach for authorization we can safely assume that it's become best practice to put any auth/token/cookie reference onto your context by now and use this data to drive any authorization check so I think there are some opportunities here to create conventions and allow for easy access to these things via something like an authorize option (which has already landed).
I would argue to advice against this. Mostly because root types are not always the barrier into something that becomes authorized-only. I've seen and built plenty of schema's where the Query root type exposes a lot of what could be considered "public" data and only a limited portion of it was truly authorize-locked. I would prefer a solution that gives me quick and easy ways to add an authorization level on a field or mutation rather than forcing this onto the root types. Also it would get in the way of situations where I am just using GraphQL as a gateway into my underlying architecture. Scoping is something that I've personally had limited use cases for and this is usually solved by just having more verbose resolver implementation that do these checks by their implementation needs. However I wouldn't mind having an API that suggests an implementation like the ruby one, my main concern here are things like:
Visibility is possibly the most interesting to me and also one that I've been missing when it comes to implementing GraphQL in JavaScript. The only way I've been able to deal with visibility is by just setting up a second schema that doesn't expose the "secret" things. However I am very interested in an approach that would allow us to do this by just having it baked into the schema definition/code. An approach I've been exploring for this is to use schema transitions which are supported by Anyway, very much looking forward to what Nexus comes up with. I'm loving the focus on type safety and the code first approach as it bring a lot of clarity is larger code bases. To sum up:
|
@tgriesser Have you considered using the casbin library to manage access control? One could configure casbin and pass it into context. Subsequently, it would be accessible in the authorize hook to use like in the attached pseudo code. I think to make this work we need a Prisma ORM Adapter (see available adapters). Unlike graphql-shield, this solution offers object-level authorization like django-guardian. |
Wanted to share an existing solution we came up with for now. As we started using nexus we refactored and fine-tuned some of our existing helper functions to aid in this process. This solution mainly focuses on authorization and validation of input before executing the real resolver. Our solution looks like this: export const MutationDoStuff = mutationField("doStuff", {
type: "Stuff",
args: {
input: arg({
type: "DoStuffInput",
required: true
})
},
resolve: mutationResolver({
authorize: [
isAuthenticated,
// These are just type hints for TypeScript, still looking to improve this.
hasAccessToNode<"Mutation", "doStuff">(
({ input }) => input.id
)
],
validate: [validateDoStuff],
resolve: (root, { input }, { providers }) => {
const stuffID = fromGlobalId(input.id).id;
// ... resolve
}
})
}); The type OrAny<T> = T | any;
export type GetFieldName<TypeName extends keyof NexusGenFieldTypes> = Extract<
keyof NexusGenFieldTypes[TypeName],
string
>;
export type MiddlewareResolver<
TypeName extends keyof NexusGenFieldTypes,
FieldName extends GetFieldName<TypeName>
> = (
root: RootValue<TypeName>,
args: ArgsValue<TypeName, FieldName>,
context: GetGen<"context">,
info: GraphQLResolveInfo
) => Promise<void> | void; // We chose to throw errors if something fails, so these middlewares are just void functions.
type MutationResolverOptions<
TypeName extends string,
FieldName extends string
> = Readonly<{
authorize?: Array<MiddlewareResolver<OrAny<TypeName>, OrAny<TypeName>>>;
validate?: Array<MiddlewareResolver<OrAny<TypeName>, OrAny<TypeName>>>;
resolve: FieldResolver<TypeName, FieldName>;
}>;
const mutationResolver= <TypeName extends string, FieldName extends string>(
options: MutationResolverOptions<TypeName, FieldName>
): FieldResolver<TypeName, FieldName> => {
return async (root, args, context, info) => {
await Promise.all(
(options.authorize || []).map(authorize =>
authorize(root, args, context, info)
)
);
await Promise.all(
(options.validate || []).map(validate =>
validate(root, args, context, info)
)
);
return options.resolve(root, args, context, info) as any;
};
}; The isAuthenticated authorizer is a simple implemntation of a middleware resolver which looks like this (make middleware is just a simple wrapper): export const isAuthenticated = makeMiddleware<any, any>(
(root, args, { auth }) => {
if (_.isNil(auth)) {
throw new AuthenticationError("You are not authenticated");
}
}
); Also we use the global ID and Node specifications from Relay and have a general guard to check if a user has access to a given node (as seen in the example), the export const hasAccessToNode = <
TypeName extends keyof NexusGenFieldTypes,
FieldName extends GetFieldName<TypeName>
>(
nodeIdSelector: (args: ArgsValue<TypeName, FieldName>) => string
) =>
makeMiddleware<TypeName, FieldName>(async (root, args, context) => {
const nodeID = nodeIdSelector(args);
// This will throw if access is denied.
await checkAccessToNode(nodeID, context);
}); Also validate works the same way. It relies on the same Even though we don't need it, but the concept of I think ideally something similar would be available on the actual definition of the nexus definition so we don't need our helper anymore. |
Hi
It would be nice if authorize is also available when using model not only for field. If I can use this on model I do not have to know something about conversations for t.field I have to know is this a list, can this field be null... |
We should use Casbin. It has a lot of built-in helper functions to help guard the API: https://casbin.org/docs/en/function |
Not sure if given/obvious or already discussed between the lines somewhere, but I'd like for the |
@Nayni are you still using your posted solution? Or has something changed over time? |
I am sadly not working on the project that I mentioned anymore. However nothing much changed between the time I wrote that comment and before I moved on. I remember we cleaned up the typings a bit more and moved from a resolver-middleware to a simple boolean for checking logged in status (which basically just calls the middleware under the hood then). For new projects I still end up using approaches that are very much inspired with the one I suggested so the main ideas stay the same:
I've been thinking about trying to nail this down into a common utils package but I haven't gotten around to this yet neither have I fully figured all the edge cases out. |
@tgriesser can we close this now that we have a authorization plugin? |
Yep, I think we can close this for now. Might revisit object-level validation / field visibility at some point, but that can be a separate discussion. |
If people need something more complex I created https://github.com/Sytten/nexus-shield for that |
This PR is the beginning of an authorization strategy for GraphQL Nexus. Feedback welcome!
I added a field-level
authorize
method, but I'm not 100% convinced this should be the final solution here. Mainly wondering if there are things we can learn/adapt from graphql-ruby's approaches to authorization? Their concepts of "scoping", "authorization", "accessibility", and "visibility" are pretty interesting.Goals:
Questions:
authorize
property by default on "root type" fields (Query, Mutation, Subscription), with the ability to disable withforceRootAuthorize: false
on themakeSchema
config? Or maybe the other way around... opt in?