Authorization

[mxstbr]

Authorization in GraphQL is (as noted by @helfer in this post) currently not a generally solved problem. Everybody has their own, custom solution that works for them but there's no general-purpose solution/library for it. (as far as I can tell)

I think graphql-tools is uniquely positioned to fill that hole in the community. Permissions are naturally bound to the schema of the requested data, so to me it feels like a natural fit for graphql-tools. (note that this is just an idea, wanted to kick off a discussion)

Imagine a simple type like this:

# I want posts to only be readable by logged-in users
type Post {
  id: ID
  content: String
  author: User
  notes: [String]  # I want these to only be accessible by the author
} 

extends type Query {
  post(id: ID!): Post
}

There's two types of permissions I want to enforce here. One of them I'm able to tell without running the resolvers first (only readable by the logged-in user), for the other I need the data from the database so they have to run after the resolvers. (only accessible by the author)

I could imagine an API like this working out well:

makeExecutableSchema({
  schema: schema,
  resolvers: resolverMap,
  // This is the new part:
  permissions: permissionMap,
})

Similar to resolvers this permissionMap would map to the schema:

const permissionMap = {
  Query: {
    post: () => { /* ... */ }
  },
  Post: {
    notes: () => { /* ... */ }
  }
}

Depending on if you return a truthy or a falsey value from these functions folks get access or a "Permission denied" error. The functions get the same information as resolvers. (i.e. obj, args and context)

This works for the pre-database case (only logged-in users should be able to access), since I could do something like this:

// The user comes from the context, and is passed in from the server
// Note: This is simplified, there's more complex checks to figure out if somebody is logged in of course
const isLoggedIn = (_, _, { user }) => user;

const permissionMap = {
  Query: {
    post: isLoggedIn
  },
  Post: isLoggedIn
}

Now the issue is post resolver permission functions. I don't have a good idea how to handle these right now, but maybe something like this could work where each "path" has two magic properties (.pre, .post) which run those functions before and after the resolver:

const isLoggedIn = (_, _, { user }) => user;
// Check if the user requesting the resource is the author of the post
const isAuthor = (resolverResult, _, { user }) => user.id === resolverResult.author;

const permissionMap = {
  Query: {
    post: isLoggedIn
  },
  Post: {
    '.pre': isLoggedIn,
    notes: {
      '.post': isAuthor,
    }
  }
}

What do you think? Any previous discussions I'm missing?

[mxstbr]

Another possible avenue (as suggested by @fubhy) rather than magic .pre/.post properties could be to simply pass a resolver executer, and let the user take care of running validations before/after:

const permissionMap = {
  Post: {
    notes: (resolverExecuter, args, context) => {
      if (!isLoggedIn(context.user)) return false;
      // Execute the resolver
      const data = resolverExecuter();
      return isAuthor(data);
    }
  }
}

resolverExecuter here is a closure around each resolver that takes care of passing the right variables to it and to return the unchanged data back to GraphQL. (otherwise the permissionMap could manipulate the data) It would also keep track of it's executed state, and if it's not been executed and the permission function returned true it'd execute the resolver itself, something to this effect:

let executed = false;
let data;

const resolverExecuter = () => {
  data = field.resolver();
  executed = true;
  return data;
}

if (field.permission(resolverExecuter, args, context)) {
  if (!executed) data = field.resolver();

  return data;
}

That way the developer has full control over when to execute which validations and which data they need, rather than relying on magic properties.

[helfer]

Hey @mxstbr, I think that's a really cool idea. I like the first syntax for defining it (permissionsMap), because it's much easier to read. You're right that something like .pre and .post will be needed, which would make it slightly less elegant. An alternative would be to have a permission be defined on a type itself along the lines of:

{
  Query: {
    post: isLoggedIn, // just an example
  }, {
    Post: {
      __self: and(isLoggedIn, or(isAuthor, isPublished)),
      notes: isAuthor,
  },
}

The __self permissions would be applied to every resolver that returns an object of that type, and it would get that object and the context as input, but not the args.
The non-__self permissions would get the usual data that the resolver gets.

I think this could look really neat, and would be great to have in graphql-tools. Do you have time to help us make it happen?

Another approach would be to actually push permissions down to the model layer (which is what FB does internally). We're thinking about a next version of graphql-tools that would organize schema and resolvers a little differently and lets you define the models along with it. I think @thebigredgeek has been thinking about stuff like this, so he might have some thoughts to share here.

Also, checking permissions is just a form of validation. Some validations (as you pointed out) can be done statically, some can only be done during execution. It would be really neat if the static ones could be run before the query even begins execution. I recently had a conversation with @JeffRMoore about this, so it would be really interesting to have his take here as well. I think it could be relatively easy to create a validation rule that checks "static" permissions before executing if we mark them somehow.

[mxstbr]

I like the first syntax for defining it (permissionsMap), because it's much easier to read.

The second syntax is still the same, except rather than having .pre/.post you have a function that lets you do your thing!

__self sounds like a great idea though, I like it.

Do you have time to help us make it happen?

I'm exactly 0 familiar with the codebase, but I'd be happy to dig in seeing as I need to add permissions to my server. 😉 Let's wait and see what @thebigredgeek and @JeffRMoore have to say!

[stubailo]

Yeah those two have been thinking about this a lot!

[southpolesteve]

I saw @mxstbr tweet about this thread and wanted to chime in with how we do auth on the bustle graphQL API. I extracted the code below but this is the general setup:

• Add a currentUser object to the context that is passed to graphQL. The setup and auth check for this occurs outside the scope of graphQL (We use a token based auth system)
• Extend the field object to include a authorize key. This expects a function. The func signature is the same as resolve
• Walk the built schema and wrap any resolvers in the authorize function

import {
  defaultFieldResolver,
  GraphQLID,
  GraphQLNonNull,
  GraphQLError,
  GraphQLObjectType,
  GraphQLString,
  GraphQLSchema
} from 'graphql'

const User = new GraphQLObjectType({
  name: 'User',
  fields: {
    id: {
      type: new GraphQLNonNull(GraphQLID)
    },
    email: {
      authorize: (root, args, context) => {
        if (root.id !== context.currentUser.id) {
          throw new GraphQLError('You are only authorized to access field "email" on your own user')
        }
      },
      type: new GraphQLNonNull(GraphQLString)
    }
  }
})

// Walk every field and swaps out the resolvers if an `authorize` key is present
function authorize (schema) {
  Object.keys(schema._typeMap).forEach((typName) => {
    const typ = schema._typeMap[typName]
    if (!typ._fields) {
      return
    }
    Object.keys(typ._fields).forEach((fieldName) => {
      const field = typ._fields[fieldName]
      if (field.authorize) {
        typ._fields[fieldName] = wrapResolver(field)
      }
    })
  })
  return schema
}

// wrap resolver with authorize function
function wrapResolver (field) {
  const {
    resolve: oldResolver = defaultFieldResolver,
    authorize,
    description: oldDescription
  } = field

  const resolve = async function (root, args, context, info) {
    await authorize(...arguments)
    return oldResolver(...arguments)
  }

  const description = oldDescription || 'This field requires authorization.'
  return { ...field, resolve, description, authorize, isAuthorized: true }
}

const query = new GraphQLObjectType({
  name: 'Query',
  fields: {
    user: {
      type: User,
      args: {
        id: { type: new GraphQLNonNull(GraphQLID) }
      },
      resolve: (root, args, context) => context.db.find('user', args.id)
    }
  }
})

export default authorize(new GraphQLSchema({ query }))

[mxstbr]

Thanks of chiming in, that's remarkably similar to what I'm thinking! 👌

Seeing that example of using root to avoid having to run authorization checks before and after, maybe we don't even need .pre/.post or anything like that?

const isLoggedIn = (_, _, { user }) => user;
// Check if the user requesting the resource is the author of the post
const isAuthor = (root, _, { user }) => user.id === root.author;

const permissionMap = {
  Query: {
    post: isLoggedIn
  },
  Post: {
    __self: isLoggedIn,
    notes: isAuthor
  }
}

[thebigredgeek]

What are the benefits to using a permissions map compared to, say, composable resolvers that simply throw errors if state preconditions aren't met? This is what I am already doing and it works really well. I don't know if additional API surface area is necessary to solve this challenge

[mxstbr]

Sure, one could do that (it's what @fubhy is doing right now and suggested I do) but I found it makes the whole codebase more complex, unnecessarily so.

You can't write resolvers inline anymore, you have to understand how to compose functions (you and I do, but why do the juniors in your team have to?), permission functions are this limbo that kind of live with resolvers but kind of don't etc. (in my opinion, of course. Happy to be proven otherwise!)

It also means you have to define totally unnecessary resolvers for scalar types, as post.notes might just be a string that doesn't need a custom resolver, but now I have to write a custom resolver because I can't handle permissions otherwise:

const resolverMap = {
  Post: {
    // post.notes is just a String, but now I need to define a custom resolver just to add permissions even though GraphQL could get this scalar type perfectly fine without the custom resolver
    notes: (root, args, context) => isAuthor(root, args, context) && root.notes,
  },
};

Resolvers are a mapping from a path (post.notes) to a database query (db.getNotes(postId)) and, kind of like controllers in a MVC system, I've found that keeping them as small as possible helps keeping the codebase easy to grok. (types: output to/input from the user, resolvers: mapping of that input/output to db queries, models: database handling)

Now, one could argue that this means permissions should be pushed down to the model layer (skinny controller, fat model in traditional MVC-speak) and handled there, but that's unwieldy because you have to pass the context and root of the resolver down to the model for every. single. query. in every. single. resolver:

const Post = require('../models/Post');

const resolverMap = {
  Post: {
    // One would have to pass { root, context } to every query 👇 in every resolver
    notes: (root, args, context) => Post.getNotes(args.postId, { root, context }),
  },
};

That (again) adds a lot of complexity when working in the codebase because one has to remember which queries get passed that and which don't, and that you have to pass them all the time.

That's my logic and why I think permissions should live on a separate layer from resolvers and models. I don't know if permissionMap is the best solution, maybe there's a better one, but I feel like there needs to be a different way of doing this.

[smolinari]

but that's unwieldy because you have to pass the context and root of the resolver down to the model for every. single. query. in every. single. resolver:

Why would the root be needed, when resolving the data? I mean, I can only imagine the user (user context) being needed, similar to what Dan Schafer points out in his talk about how Facebook goes about authorization.

Scott

[Aurelsicoko]

Like @southpolesteve, I saw @mxstbr tweet about this thread. I think the solution given by @southpolesteve is the way to go. However, this is too much dedicated to Authorization. This new layer should be more generic and be able to execute an array of functions whatever their purpose.

To give you an example, on a mutation, sometimes you have to format the incoming data or check if the email is available or not. In this case, you have to execute functions which are not specific to Authorization and we have to execute these functions before the query/mutation resolver.

IMO, we have two levels:

• Before the query or mutation resolver
• Before the field resolver

I kept the exact same example but I replaced the authorize key by a new one called policies and I'm passing an array of functions which are executed in series before the resolver. Maybe, we should have the exact same thing for executing an array of functions after the resolver, and this solution becomes closest to the pre/post one of @mxstbr.

Note: I haven't tested this code but it's just to give you an idea of how we could implement it.

import {
  defaultFieldResolver,
  GraphQLID,
  GraphQLNonNull,
  GraphQLError,
  GraphQLObjectType,
  GraphQLString,
  GraphQLSchema
} from 'graphql';

import {
  isAuthenticated,
  isOwner
} from './policies';

const User = new GraphQLObjectType({
  name: 'User',
  fields: {
    id: {
      type: new GraphQLNonNull(GraphQLID)
    },
    email: {
      policies: [isOwner],
      type: new GraphQLNonNull(GraphQLString)
    }
  }
})

function policies (schema) {
  Object.keys(schema._typeMap).forEach((typName) => {
    const typ = schema._typeMap[typName]
    if (!typ._fields) {
      return
    }
    // Be able to execute functions before the query/mutation resolver.
    if (typ.policies) {
      typ.resolve = wrapResolver(type);
    }
    
    Object.keys(typ._fields).forEach((fieldName) => {
      const field = typ._fields[fieldName]
      if (field.policies) {
        // Be able to execute functions before the field resolver.
        typ._fields[fieldName] = wrapResolver(field);
      }
    })
  })
  return schema
}

// wrap resolver with authorize function
function wrapResolver (field) {
  const {
    resolve: oldResolver = defaultFieldResolver,
    policies
  } = field

  const resolve = async function (root, args, context, info) {
    // Execute the array of functions (policies) in series.
    for (let i = 0; i < policies.length; ++i) {
      await policies[i](...arguments);
    } 
  
    return oldResolver(...arguments)
  }

  return { ...field, resolve, policies}
}

const query = new GraphQLObjectType({
  name: 'Query',
  fields: {
    user: {
      type: User,
      args: {
        id: { type: new GraphQLNonNull(GraphQLID) }
      },
      policies: [isAuthenticated],
      resolve: (root, args, context) => context.db.find('user', args.id)
    }
  }
})

export default policies(new GraphQLSchema({ query }))

The policy (function) could look like this:

// ./policies/isAuthenticated.js
export default (root, args, context, info) => {
  if (root.id !== context.currentUser.id) {
    throw new GraphQLError('You are only authorized to access field "email" on your own user')
  }
}

[mxstbr]

Why would the root be needed, when resolving the data?

What about only the author of a post being able to read the notes? (see the OP) Can't do that based on the context and the args alone, you need to have the data for that.

This new layer should be more generic and be able to execute an array of functions whatever their purpose.

I like that idea, rather than being specific to permissions it could be a generic validation layer, no matter if format or permissions or anything else! The API could still stand as discussed above, except renaming it to validationMap?

[smolinari]

What about only the author of a post being able to read the notes? (see the OP) Can't do that based on the context and the args alone, you need to have the data for that.

Hmm... I would have thought the data would be outside the resolver, like Facebook does with DataLoader.

Scott

[mxstbr]

Sure, that might be the case, but I feel like if we have the data why not pass it in and let people decide how to handle it? Also, not everybody might be using DataLoader etc. (it's also a pretty fine detail of a pretty big API 😉)

[Aurelsicoko]

I like that idea, rather than being specific to permissions it could be a generic validation layer, no matter if format or permissions or anything else! The API could still stand as discussed above, except renaming it to validationMap?

Yes, it makes sense to use a more generic name. validationMap seems great to me!

Based on my personal experience, it was a nightmare to pass the context into the DataLoader because it only accepts one argument... Despite this, I agree with @mxstbr if we have it, we should probably pass the data in the context and let people decide how to handle it.

[smolinari]

Pulling back and looking at this from an overarching 1000 mile up in the sky view, I'm of the opinion that the authorization should be on top of the business logic and not in the API. So, this goes along with what @helfer said in his first post with:

Another approach would be to actually push permissions down to the model layer

My main reasoning for this is based on the assumption that at some point in the evolution of GraphQL (and I'll be money Facebook is doing it now), I believe the creation of the API will be mostly an automated process. Having permissions mixed in the API layer would make that automation programming a lot more difficult.

This graphic is best practice according to the makers (and how I see it too):

Also, in this great article from @helfer, he also says this:

1. Permission checks are best implemented on nodes
2. Permission checks are best separated from data fetching logic to make sure they are applied consistently, regardless of how the data is fetched.

I'd also actually improve number 2.

2. Permission checks are best separated from data fetching and API logic to make sure they are applied consistently, regardless of how the data is fetched or requested.

Which basically means, the authorization could be available for any type of API (as the image shows too) and not be concerned with data fetching concerns.

Such "business details" as authorization is opinionation and again, should be avoided in any API. Sure, it means no standard for authorization within (Apollo's) GraphQL implementation. But, that is, again in my opinion, a good thing.

What should first be agreed upon is where authorization should take place or where it shouldn't. I say, it should definitely not be within the GraphQL API layer.

That is just my (last) 2 cents. 😄

Scott