feature request: functional directives

[bionicles]

Hi,
I love the directive idea, but I'm not in love with OOP. We're just friends. Would it be possible to implement directives with functional style code?

Thanks in advance for any assistance,
Bion @ Bit Pharma

[yaacovCR]

Folding into #1306

[yaacovCR]

Moving to v5.1, reopening to move any relevant discussion here.

[yaacovCR]

See possible implementation suggestion here:

#1306 (comment)

[yaacovCR]

Copying here, as discussion should continue here:

But ideally, thinking more broadly, to allow directives specified on object fields to easily modify the input fields of the resolver arguments (see parallel approach https://github.com/profusion/apollo-validation-directives/blob/master/README.md#value-validation), we can think of a different architecture altogether that is more generic. The schema directive is meant to transform the schema, but perhaps a number of schema directives are meant to work together to modify a schema. A schema directive implementation should really be a function that takes a list of directive names (so users of schema directives can use whatever names they want, to prevent conflicts) and returns a function that takes a schema as an argument and returns a schema.

The implementation can rely on visitSchema or mapSchema to assess how the graphql objects are annotated and modify them accordingly.

An additional single exported helper would be useful to extract a map of the directives and their arguments from the AST

[bionicles]

To crystallize this into code examples, let’s consider how to make an @authorize directive to provide fine-grained field-level access control.

One problem I found was, the Class is complicated, so you don’t want to make a bunch of them, so you make one @auth directive with a bunch of arguments ab options and it explodes with feature creep.

For example you might want some fields to be public, other fields to be available to an author and admins or to members of a group. Likewise you might want to reject functions based on data state, like if someone tries to buy something and doesn’t have the money.

The main appeal for me of functional directives, would be to reduce the amount and complexity of the directive code, so I could make a lot of little helper directives instead of one class with a method for

TLDR: if it’s simpler to make directives as little arrow functions

const canAfford = ({ user, item }) => user.balance > item.price ? 1 : 0;

Is a lot easier to write, maintain, and share with friends and stakeholders, than:

import { SchemaDirectiveVisitor } from "graphql-tools";

class DeprecatedDirective extends SchemaDirectiveVisitor {
  public visitFieldDefinition(field: GraphQLField<any, any>) {
    field.isDeprecated = true;
    field.deprecationReason = this.args.reason;
  }

  public visitEnumValue(value: GraphQLEnumValue) {
    value.isDeprecated = true;
    value.deprecationReason = this.args.reason;
  }
}

Since directives are the no. 1 most powerful way to do advanced stuff with graphql, and we all want to do schema-first development anyway, this would be a huge difference for everyone because we could build a library of cool directives and share them (cue exponential growth for Apollo)

All roads to great GraphQL run through directives. If it’s exponentially easier to make the directives, then the developer experience will be exponentially better

[yaacovCR]

Not sure if the way I am thinking about this aligns with your goal. I think of schema directives as providing hints to schema transformation functions, which can be quite complex.

You may be interested as well in the older directiveResolvers https://github.com/apollographql/graphql-tools/blob/master/docs/source/directive-resolvers.md

These are pretty simple ways to chain resolver functions.

[bionicles]

Apollo users just want to put easily readable and maintainable security rules on data fields for fine-grained attribute based access control, link objects/fields with relationships, and the cleanest way to do it would be directives, but directives in Apollo Server right now are too complicated with too much OOP boilerplate, visual complexity, super long words. In fact, all of Apollo is way too complicated, but this part is particularly bad

That’s why I switched away from Apollo Server: the developer has to write a massive class just to put a one liner function like “allow authors to read their own posts”

Look at Hasura. Super easy to make graphql security rules. Then you come to Apollo, and you have to write 30 lines just to wrap the one line of code you care about.

Is it completely impossible to simplify directive implementation?

[yaacovCR]

Is it completely impossible to simplify directive implementation?

I hope not!

I think what you are looking for is functionality like the older directive resolvers, linked above, but with the ability to specify directive arguments.

But, I am not really sure...

I think the above discussion might be more fruitful if you would include what arguments you think should be provided to your functional directive and what you think it should return.

Are you thinking it should be provided the resolver type args like parent, args, context, info, and should return data, the functional directiveResolvers way?

Or are you thinking it should be provided a graphql field and return a new field?

Have you looked at the existing functional directiveResolvers option? Does that help you at all? If it helps somewhat, how does it fall short?

I don't work for Apollo, I contribute here in my spare time -- I am happy you have found in Hasura functionality that is working for you. Do you have a particular link to how that involves schema directives?

[yaacovCR]

I misspoke in one aspect, directiveResolvers already support arguments -- as per the docs, within your directiveResolver fn, you get access to the parent, the directive arguments, and the context. In the source, it appears that info is also passed.

It might be helpful actually to get access to the original args as well. That could be added pretty easily. One might think you can get that from info, but default arguments are not actually included, and nothing is parsed into internal values.

The implementation for directiveResolvers is now done in terms of the newer schemaResolvers and could be changes as follows:

  Object.keys(directiveResolvers).forEach((directiveName) => {
    schemaDirectives[directiveName] = class extends SchemaDirectiveVisitor {
      public visitFieldDefinition(field: GraphQLField<any, any>) {
        const resolver = directiveResolvers[directiveName];
        const originalResolver =
          field.resolve != null ? field.resolve : defaultFieldResolver;
        const directiveArgs = this.args;
        field.resolve = (...args) => {
//OLD:          const [source /* original args */, , context, info] = args;
          const [source,  fieldArgs, context, info] = args;
          return resolver(
            () =>
              new Promise((resolve, reject) => {
                const result = originalResolver.apply(field, args);
                if (result instanceof Error) {
                  reject(result);
                }
                resolve(result);
              }),
            source,
            fieldArgs, ///   <=== added this
            directiveArgs,
            context,
            info,
          );
        };
      }
    };
  });

  SchemaDirectiveVisitor.visitSchemaDirectives(schema, schemaDirectives);
}

Alternatively, you could just use the above as a blueprint for a schema directive.

[yaacovCR]

I suppose it might be nice (but more complicated) to also require passing arguments to the next function so that you could modify the source or arguments passed to the underlying resolver, akin to how graphql middleware works. That is also easily accomplished with some modification of the above.

[koresar]

Directives as functions would be much preferred than the visitor object. So simple!
Please, make this happen.

[yaacovCR]

@barbieri @alexstrat

I wonder if you would be so kind as to comment on #1463 in terms of whether you might find the approach more helpful in terms of your validation directive work.

Could it be improved even further for that use case?

[yaacovCR]

@bionicles @koresar and all tracking this issue, your thoughts are welcome as well!

Description of approach can be seen within the included changes to documentation.

[yaacovCR]

@bionicles The simpler directive resolvers have been rewritten with mapSchema as well, I am not sure whether you have further improvements in mind as you seemed more interested in directive resolvers. Please feel free to address that or comment on any of the above.

[koresar]

@yaacovCR I'm looking at the new docs of the #1463 PR. Maybe I missed it but my main concern is not answered there yet.

When I tried using the @constrained directive on an input String inside my Apollo2 project it dynamically created and leaked the ConstrainedString type (see this comment). The type was exposed to my GraphQL users. I don't want any new types in my schema.

I was trying to implement the @constrained directive myself but without having to create new types. It appeared to be literally impossible.

Question.
If I implement the @constrained directive myself using this new approach will I be forced to create any new leaking schema types?

[yaacovCR]

The validation directive included in the example does expose the new type.

To perform validation that differs depending on where the scalar type is employed, I believe one cannot use simple scalar parse functions, You could instead use output field resolver wrapping, an approach employed by @barbieri @alexstrat within their respective packages (see #789 and #842 (comment))

Their work -- I believe -- was complicated muchly by the class-based schema directive approach, which essentially only exposed hooks for modification on the annotated graphql object, rather on the entirety of the schema. I believe the functional approach adopted here would make their lives much easier, as a first pass of mapSchema could simply scan for graphql objects tagged with the appropriate directives, and a second scan could then allow them to perform the modification where necessary.

But I am eagerly awaiting their comments, because I am sure (just see my confusion within #789) that I am missing something with regard to this somewhat complicated topic.

Because of the complexity -- or just because some of it is clearly going over my head -- for now, the implementation of the resolver validation approach will be in userland rather than in graphql-tools proper. But, I hope that the functional approach within graphql-tools will be helpful to them and to the larger community.

I await further comments to find out for sure!

[alexstrat]

@yaacovCR you summed up exactly the concern IMO and indeed #1463 makes things easier.

The key feature is the fact that schemaTransforms, compared to schemaDirectives, can visit not only directive-targeted nodes, but all nodes. That's what makes implementations of argument validation by field-resolver-wrapping look like a standard use-case of graphql-tools and not a hack.

The OOP API per-se was not a blocker, but the functional API seems less verbose which makes things probably easier to read.

Though, my opinion is that, the documentation, for the validation example, should now present the field-resolver-wrapping technique rather than the type/scalar hijacking technique — that has some disadvantages.

A basic use-case, like validation of field and only arguments (not input), is, with the new API, rather simple to write. More advanced use-cases (deep validation of arguments, input object, input field..) can be deferred to userland libraries.

[yaacovCR]

happy to accept PR to docs with new example to whatever depths of complexity you think appropriate, with links to appropriate packages completing the workflow

we should consider in general linking to popular external schemaDirective classes and potentially any new external schemaTransform libraries

[alexstrat]

That being said, regarding graphql-field-arguments-coercion, I now realize that it relies on attaching a non-standard property coerce on type definitions via schema visiting. But patching types outside of GraphQL standard type config is not possible with schema transformation.

Anyway, I was not fan of the technique of patching type and it's probably possible to do differently and compatible with schema transformation.

OK noted for the doc! I might work on this when I'll adapt graphql-field-arguments-coercion to schema transformation.

[yaacovCR]

Thanks @alexstrat

We are doing our best to not decorate graphql objects with custom fields or at least put them under extensions property as per upstream graphql-js

I believe #1468 should help preserve backwards compatibility to some extent

[bionicles]

I can dig through some old code and supply an example of what I was doing before: it was an @auth directive which allowed user to specify a list of functions as arguments.

The functions would take arguments:

request
context (especially the user)
data target

and return True or False (accept or reject) the request, or it could return “CRUD” for Create Read Update Delete

Basically, it lets you build a library of simple one liner security rules

It would be cool if we could quickly turn one liners like this into security directives to decorate fields

When user makes a request, you run all the rules and concatenate their decisions. Then you check to see if the request type is contained within the decisions. if an anonymous user requests to update an object which only devs can update, request is denied

const canAfford = ({user, target}) => user.balance >= target.price; 

const authorCRUD = ({user, target}) => user.id === target.authorId ? “CRUD” : “”;

const devCRUD = ({user}) => user.roles.includes(“dev”) ? “CRUD” : “”

const publicC = () => “C”

// then we can secure data like this:
email string @authorCRUD @publicC

const readACL = ({user,target}) => target.acl.includes(user.id) ? “R” : “”;
``` I

[yaacovCR]

See https://github.com/ardatan/graphql-tools/blob/move-graphql-toolkit/website/docs/directive-resolvers.md
Which is implemented as https://github.com/ardatan/graphql-tools/blob/move-graphql-toolkit/website/docs/schema-directives.md#what-about-directiveresolvers

Basically I think this is possible with old and new API

[yaacovCR]

Am happy to try to help with your particular use case, either on this issue or offline.

[bionicles]

Here’s what I had before, for your reading pleasure. Almost surely possible to optimize this and particularly to remove the “eval” (although it only evaluates rules and not user supplied code)

Consider it MIT license

// authorize.js
import {
  pipe,
  prop,
  juxt,
  map,
  split,
  flip,
  includes,
  useWith,
  __,
  path,
  anyPass,
  ifElse,
  trim
} from "ramda";
import { SchemaDirectiveVisitor } from "apollo-server-express";
import { defaultFieldResolver } from "graphql";

const getDotPath = r => useWith(path(__, r), [split(".")]);
const makeRuleFn = rule => {
  const [conditionStr, opStr] = map(trim, split("?", rule));
  const hasOp = flip(includes)(split("", opStr));
  const evalString = `; ${conditionStr}`;
  return r => {
    const p = getDotPath(r);
    return eval(evalString) && hasOp(r);
  };
};
const makeCheckRulesFn = ruleStrings => {
  const ruleFns = map(makeRuleFn, ruleStrings);
  return pipe(juxt(ruleFns), anyPass);
};
const reject = root => {
  throw Error(`unauthorized op: ${prop("op", root)}`);
};

// apollographql.com/docs/apollo-server/schema/creating-directives/
export class AuthDirective extends SchemaDirectiveVisitor {
  visitFieldDefinition(field) {
    const { resolve = defaultFieldResolver } = field;
    const { rules } = this.args;
    const checkRules = makeCheckRulesFn(rules);
    // apollographql.com/docs/graphql-tools/resolvers/#resolver-function-signature
    field.resolve = async (parent, args, context) => {
      const item = context.redis.hgetall(args.id);
      const root = { parent, ...context, args, item };
      return ifElse(checkRules, resolve, reject)(root);
    };
  }
}

// type-defs.js
import { gql } from "apollo-server";

// define rule conditions
const AUTHOR = "p('item.author.id') === p('me.id')";
const DEV = "includes('devs', p('me.groups'))";
// define rules
const MAYBE_PUBLIC_READ = "!p('item.isPrivate') ? R";
// const AUTHOR_CRUD = `${AUTHOR} ? CRUD`;
const AUTHOR_READ = `${AUTHOR} ? R`;
// const DEV_CRUD = `${DEV} ? CRUD`;
const DEV_READ = `${DEV} ? R`;
// define rule sets
const FEEDBACK_RULES = `["1 ? C", "${DEV_READ}", "${AUTHOR_READ}", "${MAYBE_PUBLIC_READ}"]`;
const PERSON_RULES = `["1 ? C", "${DEV_READ}", "${AUTHOR_READ}"]`;
const DENY = `["0 ? CRUD"]`;
// define schema
export const typeDefs = gql`
  directive @auth(rules: [String]) on FIELD_DEFINITION
  type Person {
    id: ID! @auth(rules: ${DENY})
    name: String! @auth(rules: ${PERSON_RULES})
    email: String! @auth(rules: ${PERSON_RULES})
    groups: [String] @auth(rules: ${PERSON_RULES})
    signIns: [String] @auth(rules: ${PERSON_RULES})
    feedback: [Feedback] @auth(rules: ${PERSON_RULES})
  }
  type Feedback {
    id: ID! @auth(rules: ${DENY})
    author: Person @auth(rules: ${DENY}) 
    satisfied: Boolean! @auth(rules: ${FEEDBACK_RULES})
    score: Int! @auth(rules: ${FEEDBACK_RULES})
    feedback: String! @auth(rules: ${FEEDBACK_RULES})
    isPrivate: Boolean! @auth(rules: ${FEEDBACK_RULES})
  }
  type Mutation {
    createFeedback(
      authorId: ID
      satisfied: Boolean!
      score: Int!
      feedback: String!
      isPrivate: Boolean!
    ): Feedback
    me: Person
  }
  type Query {
    hello: String!,
    listFeedback: [Feedback]
  }
`;

[bionicles]

Now with this change we can avoid writing “@auth(rules=“ 50 billion times

“p” is just a hack to do functional dot-based pathing on nested objects

“r” is the object with user, target, and request type

[yaacovCR]

Closed by #1419.