Fine grain access control support ?

[BerndWessels]

Hi Is there a way to restrict the generated schema to a single root type like User ?

Here is why: I have Users and Accounts.

const graphQlSchema = graphQlBuilder()
  .model(User)
  .model(Account)
  .build();

But I don't want API users to be able to query all accounts. I only want users to query accounts that they are allowed to see.

The way I see that could be done is to restrict the GraphQL schema to the User type only and not expose the Account type on root level. Then the only way to query accounts would be through the accounts property on the User type. This restricts the accounts one could query to that particular user.

There is still a flaw in that you can still query all users and so get to all accounts.

Is there a way to completely restrict the query to the user that makes the response ? (the user id comes with the response via some server authentication mechanism)

[timhuff]

I had to deal with this same issue in my usage. Almost caused me to ditch it. This is a solvable problem, though. You're going to want to utilize Objection's APIs for this purpose. When a user connects or makes a request, use their credentials to define their userId or whatever you want to use for access control. You'll need to define the rootValue so as to attach it to the resulting Objection queries' context (there might be other ways but using rootValue seems the most universally applicable one). Here's an example using apollo-server-koa:

const graphqlKoaServer = graphqlKoa(ctx => {
  // get their userId from maybe an auth token in the headers (inside ctx for koa) here
  return {
    schema, // this is the result of this library
    rootValue: {
      async onQuery(qb) {
        await qb.mergeContext({ userId })
      },
    },
  }
})

Now... word of warning here... be certain you know how context works. If you lose the context by making some arbitrary query, you might cause a security issue, depending on how you set things up. I'd suggest setting things up in a way such that losing context results in getting blocked out, rather than gaining access to everything. And if you're not sure if something has received the context, you can always .mergeContext({ userId }) on it to make sure.

So once you have your userId in your context, you can access it from pretty much anywhere by running qb.context(). Your userId will be on that object.

I would strongly recommend getting familiar with runBefore and runAfter hooks built into Objection's QueryBuilder, as well as how to override a specific model's query builder class to be one of your own design. runBefore lets you modify the query before it's ran and runAfter lets you modify the result of the query after it's ran.

[timhuff]

Oh, and I also pass auth-things in to my graphql context for the purpose of controlling mutations/subscriptions. So that code snippet up there has context: {userId} above rootValue. This lets you access it from resolvers like so:

resolve: async (root, args, { userId }) => {

If you make queries inside the resolver, use mergeContext first, to ensure your hooks are provided the appropriate context with which to filter.

[BerndWessels]

Awesome, thanks. Will give it a try and report back here.

[timhuff]

Sounds good. I need to get around to outlining this in the README. Access control is pretty much a universal need and it's not clear if it's even possible with this library.

[BerndWessels]

@timhuff I am struggling a bit to find the right places for this.

There seems to be no way to add onBuild to model classes in general unless you do it right in the query() via .onBuild.

Therefore the only way I see is adding onQuery to the context given to graphql.

Would you be so kind to give an example on how you solved that?

Basically I want to make sure that my User with id=1 can only see accounts that are linked to that user.

This is how far I got so far:

Tables user with id, name account with id, name user_account with user_id, account_id, delegation_type

const graphql = require('graphql').graphql;
const graphQlBuilder = require('objection-graphql').builder;

const Knex = require('knex');

// Initialize knex.
const knex = Knex({
    client: 'sqlite3',
    useNullAsDefault: true,
    connection: {
        filename: 'db.sqlite3'
    }
});

knex.on( 'query', function( queryData ) {
    console.log( queryData.sql );
});

// Objection.js models.
const {User, Account} = require('./models');

// This is all you need to do to generate the schema.
const graphQlSchema = graphQlBuilder()
    .model(User)
    .model(Account)
    .build();

// Execute a GraphQL query.
graphql(graphQlSchema, `{
  users {
    id
    name
    accounts {
      id
      name
    }
    accounts {
      id
      name
    }
  }
}`, {knex, onQuery: (builder, context) => {
        builder.where('id', 1); // But this seems to only act on the Users, not on Accounts :(
    }}).then(result => {
    console.log(JSON.stringify(result));
});

UPDATE:

This gets the builders for each type it seems, but the innerJoin does not limit the accounts to user.id=1

// Execute a GraphQL query.
graphql(graphQlSchema, `{
  users {
    id
    name
    accounts {
      id
      name
    }
  }
}`, {
    knex, onQuery: (builder, context) => {
        builder.context({
            onBuild: anotherBuilder => {
                switch (anotherBuilder.modelClass()) {
                    case User: {
                        ////////// anotherBuilder.where('id', 1);
                        break;
                    }
                    case Account: {
                        anotherBuilder.innerJoin('user as userAccessControl', 'userAccessControl.id', 1);
                        break;
                    }
                }
            }
        });
    }
}).then(result => {
    console.log(JSON.stringify(result));
});

[timhuff]

Give me a few hours and I'll add an example for access control to the repo. I feel the usability of the library would greatly benefit from this anyway.

[timhuff]

@BerndWessels I'm not going to merge this into master until it's more throughly tested (don't want to make it too visible for now). You can see it here.

[timhuff]

A few quick notes: you'll typically want to prefer using runBefore, rather than filtering fetched results in runAfter, as that will be more performant. Also, you might want to generalize this a bit. I simplified my implementation a decent amount to make comprehension easier - mine utilizes a few polymorphism tricks to remove boilerplate from my API models.

[timhuff]

I decided to go ahead and put out the more advanced implementation after all. It's basically set up so that any model that extends from BaseModel can specify class functions that either modify a query or modify the results of a query based on user context. So if you want to make it so that users can only see projects that they're the owner of, you just add the following to the Project class:

  static async modifyApiQuery(qb, { userId }) {
    qb.where('ownerId', userId);
  }

[BerndWessels]

@timhuff Hi Thanks for the example. It makes things much clearer.

Unfortunately it shows that it can be pretty complex to add access control in all necessary places.

For my example

Tables user with id, name account with id, name user_account with user_id, account_id, delegation_type

const graphQlSchema = graphQlBuilder()
    .model(User)
    .model(Account)
    .build();

The generated schema allows querying users and accounts on the root query.

So this query is not working

{
    accounts {
      id
      name
    }
}

{"errors":[{"message":"select `account`.`id`, `account`.`name` from `account` where `user_id` = 1 - SQLITE_ERROR: no such column: user_id","locations":[{"line":2,"column":5}],"path":["accounts"]}],"data":{"accounts":null}}

class Account extends BaseModel {

    static async modifyApiQuery(qb, { userId }) {
        qb.where('user_id', userId);
    }

    // ...

because the accounts query could come from the root query accounts but also from the user accounts query.

And in this case the root account query does not do a join with the user table and therefore has no user_id. But if it were coming from a user query then the query would be a join and there would be a user_id. But even that is a risky assumption since user_id might have been aliased by the query generator.

Basically the modifyApiQuery need to understand the context of the generated query better to make the right kind of modification. But this could become very complex and prone to errors I fear.

Do you have any ideas how to improve that?

[timhuff]

That error indicates that user_id is not a column of account. For this, you'd want to do something like qb.where('owner_id', userId) (or whatever field of account indicates the owner). Regardless of where the query is coming from, the userId variable is coming from the auth token and it'll always be there (unless you permit unauthenticated users to make queries - for simplicity's sake, my auth requests go through REST). If you want to fork the repo and modify the example to reflect your situation, it'd be a lot easier for me to analyze the problem and demonstrate the solution.

[BerndWessels]

@timhuff Hi, I just updated my fork of your project:

https://github.com/BerndWessels/objection-graphql/tree/access-control

Im the access-control branch I have added the scenario I was describing earlier.

Basically you now have User Project User_Project link table

Now I can't figure out how to use modifyApiQuery and modifyApiResults in a way that User 1 can only ever see Projects that are linked to him via User_Person link table.

So for example a query like:

{
  projects {
    id
    title
  }
}

should still only return projects for User 1 in the same way that this does:

{
  users(id:1) {
    projects {
      id
      title
    }
  }
}

So pretty much I need to make sure that nothing ever get exposed that should not be visible for the given user - which would be most likely determined through link tables with permissions.

Hope that helps you to get a better idea and to experiment.

[timhuff]

I've provided a way to achieve that result in this pull request. I'm still experimenting with the best way to handle many-to-manys. One thing to note about this approach in general is it's not going to be ridiculously performant. But I feel the ability to concisely and easily achieve universal scoping on an auto-generated API is worth any lost performance. For now, I use this API as a general OOP API and utilize a secondary REST API in the event that I need something more flexible.

[timhuff]

@BerndWessels Another thing to note is that I don't actually just pass around userId on the context, as it doesn't give you much to work with and you end up having to query for additional information. I instead use it, before the query starts, to populate a few of the objects related to them that are most significant for determining their scope and permissions.

[BerndWessels]

Thanks @timhuff PR looks good and explains it quite nicely. I think ManyToMany relations are very common especially in more fine grain access control. I'll experiment a bit more with it to see how well it covers a lot of my cases. Cheers

[AaronNGray]

@timhuff @koskimas It would be lovely to be able to have a predicate access function that is checked by objection-graphql and defaults to true but can have a default defined in a base class that could then be set to false or an automated default that is then also overridable in child classes.

[CoveMB]

Hello, I am trying to implement access control as well but I am not sure about the performance of my implementation.

I would like to have strict data control in the app.

In this example there is a user model and an (auth) Token model, the user has many tokens and token belongs to a user.

Basically in my BaseModel I call all the lifecycle hooks to make sure all model have a accessControlGraphQLQuery function implemented and then call this method is the context has a isGraphQLQuery value set to true (I also pass it as user the authenticated user.

/* eslint-disable consistent-return */
const { Model, QueryBuilder } = require('objection');
const { ImplementationMissingError } = require('config/errors/errorTypes');
const { isFromGraphqlQuery } = require('./model.utils');

class BaseModel extends Model {

  static async validateGQLControlImplementation(hooksArguments) {

    if (isFromGraphqlQuery(hooksArguments)) {

      try {

        return this.accessControlGraphQLQuery(hooksArguments);

      } catch (error) {

        throw new ImplementationMissingError(`accessControlGraphQLQuery function for the ${this.name} model, ${error}`);

      }

    }

  }

  static async afterInsert(hooksArguments) {

    await super.beforeInsert(hooksArguments);

    return this.validateGQLControlImplementation(hooksArguments);

  }

  static async afterUpdate(hooksArguments) {

    await super.beforeInsert(hooksArguments);

    return this.validateGQLControlImplementation(hooksArguments);

  }

  static async afterDelete(hooksArguments) {

    await super.beforeInsert(hooksArguments);

    return this.validateGQLControlImplementation(hooksArguments);

  }

  static async afterFind(hooksArguments) {

    await super.beforeInsert(hooksArguments);

    return this.validateGQLControlImplementation(hooksArguments);

  }

  $beforeUpdate() {

    this.updated_at = new Date().toISOString();

  }

}

then in I have the following models

const BaseModel = require('models/BaseModel');
const TokenQueryBuilder = require('./token.queries');

class Token extends BaseModel {

  static get tableName() {

    return 'token';

  }

  static get QueryBuilder() {

    // This register the custom query builder
    return TokenQueryBuilder;

  }

  static async accessControlGraphQLQuery({ asFindQuery, context, items }) {

    const { user } = context;

    return asFindQuery()
      .clearContext()
      .where('token.user_id', user.id)
      .withGraphFetched('user');

  }

  static get jsonSchema() {

    return {
      type    : 'object',
      required: [ 'token' ],

      properties: {
        id    : { type: 'integer' },
        userId: { type: 'integer' },
        device: {
          type: 'string', minLength: 1, maxLength: 255
        },

        // expiration: { type: 'string' },
        token: {
          type: 'string', minLength: 1, maxLength: 255
        },
      }
    };

  }

  static get relationMappings() {

    // eslint-disable-next-line global-require
    const { User } = require('models');

    return {
      user: {
        relation: BaseModel.BelongsToOneRelation,

        modelClass: User,
        join      : {
          from: 'token.user_id',
          to  : 'user.id'
        }
      },
    };

  }

  // Modifiers are reusable query snippets that can be used in various places.
  static get modifiers() {

    return {
      orderByCreation(builder) {

        builder.orderBy('created_at');

      }
    };

  }

}

module.exports = { Token };

const BaseModel = require('models/BaseModel');
const validateUserInput = require('./user.validations');
const UserQueryBuilder = require('./user.queries');
const { capitalize } = require('utils');

// This plugin allow for automatic password hashing, if you want to allow an empty password you need to pass it allowEmptyPassword
const Password = require('objection-password')();

// This plugin allow for unique validation on model
const Unique = require('objection-unique')({
  fields     : [ 'email' ],
  identifiers: [ 'id' ]
});

class User extends Password(Unique(BaseModel)) {

  static get tableName() {

    return 'user';

  }

  static get QueryBuilder() {

    // This register the custom query builder
    return UserQueryBuilder;

  }

  static async accessControlGraphQLQuery({ asFindQuery, context, items }) {

    const { user } = context;

    return asFindQuery()
      .clearContext()
      .where('user.id', user.id);

  }

  static get jsonSchema() {

    return {
      type    : 'object',
      required: [ 'email', 'password' ],

      properties: {
        id   : { type: 'integer' },
        email: {
          type: 'string', minLength: 1, maxLength: 255
        },
        password: {
          type: 'string', minLength: 1, maxLength: 255
        },
        admin: {
          type: 'boolean'
        }
      }
    };

  }

  static get relationMappings() {

    const {
      Token
      // eslint-disable-next-line global-require
    } = require('models');

    return {
      tokens: {
        relation  : BaseModel.HasManyRelation,
        modelClass: Token,
        join      : {
          from: 'user.id',
          to  : 'token.user_id'
        }
      }
    };

  }

  // Omit fields for json response from model
  $formatJson(user) {

    super.$formatJson(user);

    delete user.password;
    delete user.admin;
    delete user.created_at;
    delete user.updated_at;

    return user;

  }

  // This hook triggers before an insert
  async $beforeInsert(queryContext) {

    // Validate before password hashing
    validateUserInput(this);

    if (this.country) {

      this.country = capitalize(this.country);

    }

    if (this.name) {

      this.name = capitalize(this.name);

    }

    // Super will take care of hashing password
    await super.$beforeInsert(queryContext);

  }

  // This hook triggers before an update
  async $beforeUpdate(opt, queryContext) {

    // Validate before password hashing
    validateUserInput(this);

    if (this.country) {

      this.country = capitalize(this.country);

    }

    if (this.name) {

      this.country = capitalize(this.name);

    }

    // Super will take care of hashing password
    await super.$beforeUpdate(opt, queryContext);

  }

  // Modifiers are reusable query snippets that can be used in various places.
  static get modifiers() {

    return {};

  }

}

module.exports = { User };

this is working okay but I have the intuition that it might have a performance issue.

I tried to use the beforeFind hooks but it was returning me all the tokens for example not only the one from the authenticated user.

Also with this implementation, it will be always withGraphFetched the user on the token even if I don't need it.

Any ideas ? :) @timhuff @BerndWessels maybe?

PS: I also get this error QueryBuilder#eager method is deprecated. You should use the withGraphFetched method instead. eager method will be removed in 3.0

[CoveMB]

Hello again I found an other solution that i think will be more performant.

I now pass a runBefore function in the query contexct passed in the rootValue as such :

exports.graphql = async ctx => graphqlHTTP({
  schema     : graphQlSchema,
  graphiql   : isDevelopment, // Will activate graphql only in development
  formatError: error => {

    // Throw error from context instead of return in it in the graphQl query result
    ctx.throw(error);

  },
  context  : ctx,
  pretty   : true,
  rootValue: {
    async onQuery(query) {

      query.context({
        runBefore(result, builder) {

          try {

            builder.modify('graphQLAccessControl', ctx.authenticated.user);

          } catch (error) {

            throw new ImplementationMissingError(`graphQLAccessControl modifier for one of the model queried by the graphQL query | ${error.stack}`);

          }

        },
      });

    }
  }
})(ctx);

then I just need to implement an graphQLAccessControl modifiers on all my queried models like this :

static get modifiers() {

    return {
      graphQLAccessControl(builder, user) {

        builder.where('id', user.id);

      }
    };

  }