Redesign @hasRole directive: make it as flexible as possible

[FluorescentHallucinogen]

1. Take array of enums instead of string as argument in @hasRole.

(Please also keep in mind that roles (permission subsets) can be non-overlapping.)

Current implementation:

directive @hasRole(role: String) on FIELD | FIELD_DEFINITION

type User
  name: String
  banned: Boolean @hasRole(role: "admin")
  canPost: Boolean @hasRole(role: "reviewer, admin")
}

Proposed implementation:

directive @hasRole(roles: [Role] = [UNKNOWN]) on FIELD | FIELD_DEFINITION

enum Role {
  ADMIN
  REVIEWER
  USER
  UNKNOWN
}

type User
  name: String
  banned: Boolean @hasRole(roles: [ADMIN])
  canPost: Boolean @hasRole(roles: [REVIEWER, ADMIN])
}

It seems that an array of strings would be more flexible than an array of enums (because you don't need to update enum in the GraphQL SDL to add a new role), but it less type-safe (you don't get a list of possible values e.g. for autocomplete).

2. Add support of other directive locations besides FIELD_DEFINITION to apply the directive to e.g. mutations and fields in the input types used in these mutations for more granular permissions.

Did I understand correctly that @hasRole directive can only be applied to FIELD_DEFINITION, while there are many places where directives (in general) can be applied (see the full list here: https://github.com/graphql/graphql-js/blob/master/src/language/directiveLocation.js)?

Example use case:

Everyone can query, create and update users (except canPost and banned fields), but only users with role ADMIN can delete users.
Only users with role REVIEWER or ADMIN can have access (query, create and update) canPost field.
Only users with role ADMIN can have access (query, create and update) banned field.
Only users with role MANAGER can update verified field.

Proposed implementation:

type User @hasRole(roles: [USER]) {
  name: String
  canPost: Boolean @hasRole(roles: [REVIEWER, ADMIN])
  banned: Boolean @hasRole(roles: [ADMIN])
  ...
}

...

type Mutation {
  createUser(data: UserCreateInput!): User!
  updateUser(data: UserUpdateInput!, where: UserWhereUniqueInput!): User!
  deleteUser(where: UserWhereUniqueInput!): User @hasRole(roles: [ADMIN])
  ...
}

...

input UserUpdateInput {
  phone: String
  verified: Boolean! @hasRole(roles: [MANAGER]) @default(value: false)
  ...
}

3. Add support of logical operations

Currently, @hasRole(role: "user, admin") checks if user has some (at least one any) of specified roles. What about all, none and other more complex cases? (Please also keep in mind that roles (permission subsets) can be non-overlapping.)

What about adding support of logic operations for nest rules (similar to https://github.com/maticzav/graphql-shield#and-or-not)? Something like (the syntax is subject to discussion):

@hasRole(roles: "or(ADMIN, and(OWNER, not(EDITOR)))")

[FluorescentHallucinogen]

Regarding the syntax for logical operations, I propose the syntax similar to the syntax that is used in Prisma: https://www.prisma.io/docs/prisma-graphql-api/reference/queries-qwe1/#combining-multiple-filters.

On the other hand, since

1. @hasRole(roles_in: [OWNER, ADMIN]) directive checks if user has some (at least one any) of specified roles (i.e. roles are combined by OR)

and

2. custom directives in GraphQL are combined by AND:

type User {
  verified: Boolean! @hasRole(roles_in: [OWNER, ADMIN]) @hasRole(roles_in: [MANAGER])
}

logical operations may not be needed.

The only downside is the need to use 2 directives alongside instead of 1.

[czystyl]

@FluorescentHallucinogen Great research, thanks for your effort!

The main idea of this directives was to create that Enum with roles and allow users to put custom logic into the directives.

About using a current directive implementation with query or mutation - it's working; I'm using it all the time in my side project.

I checked the graphql-shield project, and It's so good at flexibility. The logical operator is cool, and we can put custom login there what it's hard to do with directives - correct me If I'm wrong.

I was also thinking about support simple permissions tree, like:

Admin -> Manager -> User

It's mean that if we have an Admin role then automatically we getting the lower roles, so actually Admin === Admin + Manager + User - It could be helpful and limit the repetition.

I'd like to do redesign the API and adapt your thoughts. 👍

[FluorescentHallucinogen]

4. Add option to change the directive behavior.

In current implementation, if the query contains a field, to which the @hasRole directive is applied, and the user role doesn't match with the provided role, then directive will throw the error, and the entire query (all fields) will be rejected.

Proposed implementation:

type User {
  name: String!
  address: String
  email: String
  phone: String! @hasRole(roles_in: [OWNER, ADMIN], strategy: STRIP)
  ...
}

query {
  users {
    name
    address
    email
    phone
  }
}

should (if the user role doesn't match with the provided one in the directive) just strip the phone field, but return the other fields + errors. Something like:

{
  "data": {
    "users": [
      {
        "name": "Bob",
        "address": "",
        "email": "[email protected]",
        "phone": null
      },
      {
        "name": "Alice",
        "address": "",
        "email": "[email protected]",
        "phone": null
      }
    ]
  },
  "errors": [
    {
      "code": "ERR_GRAPHQL_PERMISSION_VALIDATION",
      "fieldNames": ["phone"],
      ...
    }
  ]
}

@hasRole(roles_in: [OWNER, ADMIN], strategy: REJECT)

should reject the entire query (all fields).

[FluorescentHallucinogen]

@czystyl

I was also thinking about support simple permissions tree, like:

Admin -> Manager -> User

It's mean that if we have an Admin role then automatically we getting the lower roles, so actually Admin === Admin + Manager + User - It could be helpful and limit the repetition.

This doesn't cover all use cases. E.g. in my app roles are non-overlapping. If the user has the EDITOR role, this doesn't mean that he also has the OWNER role (and vice versa).

[FluorescentHallucinogen]

The only downside is the need to use 2 directives alongside instead of 1.

I was wrong. According to https://facebook.github.io/graphql/June2018/#sec-Directives-Are-Unique-Per-Location, it's not possible to use more than one directive of the same name for the same location.

[amitava82]

How to use @isAuthenticated with mutation?

[czystyl]

@FluorescentHallucinogen But I'm talking about the separate directive.

@amitava82 Sure, it works ;)

[FluorescentHallucinogen]

@czystyl Is it possible to implement all my proposals using custom authenticateFunc and checkRoleFunc?

[czystyl]

@FluorescentHallucinogen Custom func allows you to define your own logic so probably you are able to do it.

[czystyl]

BTW would be great to migrate from string to Enum - @FluorescentHallucinogen Would you like to take care of it?

[FluorescentHallucinogen]

I've implemented a prototype of logical operations support using custom checkRoleFunc and third-party logical-expression-parser NPM package:

const authDirectives = require('graphql-directive-auth').default;
const logicalExpressionParser = require('logical-expression-parser');

function checkRoleCustomFunc(context, requiredRoles) {
  const userRole = context.auth.role;

  if (!userRole) {
    throw new Error(`Invalid token payload, missing role property inside!`);
  }

  const hasNeededRole = logicalExpressionParser.parse(requiredRoles, token => userRole.indexOf(token) > -1);

  if (!hasNeededRole) {
    throw new Error(
      `Must have role: ${requiredRoles}, you have role: ${userRole}`
    );
  }
}

const customAuth = authDirectives({
  checkRoleFunc: checkRoleCustomFunc
});

const schemaDirectives = {
  ...customAuth
};

It's possible to use something like this:

@hasRole(role: "EDITOR&(ADMIN|OWNER)")

[FluorescentHallucinogen]

@czystyl Any plans to fix #16?

[FluorescentHallucinogen]

@czystyl

BTW would be great to migrate from string to Enum

The only advantage of using enum over string that I see is type safety (including autocomplete). In current implementation that use string it's possible to make mistakes: typos, wrong case, nonexistent roles, etc.

On the other hand, string is more flexible than enum: no need to change enum in the GraphQL SDL to add a new role.

We have to decide which is better. :)

[FluorescentHallucinogen]

As for the logical-expression-parser NPM package, I haven't found a repo on GitHub. As for the syntax, I prefer Polish prefix notation + AND/OR/NOT instead of infix notation + &/|/!.

[FluorescentHallucinogen]

@czystyl

It seems all that is needed to do to migrate from string to enum is to change

directive @hasRole(role: String) on FIELD | FIELD_DEFINITION

to

directive @hasRole(role: Role) on FIELD | FIELD_DEFINITION

enum Role {
  USER
  OWNER
  EDITOR
  ADMIN
}

or

directive @hasRole(role: [Role]) on FIELD | FIELD_DEFINITION

enum Role {
  USER
  OWNER
  EDITOR
  ADMIN
}

Moreover, it's possible to set the default role

directive @hasRole(role: Role = USER) on FIELD | FIELD_DEFINITION

or

directive @hasRole(role: [Role] = [USER]) on FIELD | FIELD_DEFINITION

to use just @hasRole (without any argument) instead of @hasRole(role: USER) or @hasRole(role: [USER]).

The disadvantage of using enum over the string is that enum is incompatible with logical operations proposal. Please correct me if I'm wrong.

(Please also keep in mind that it's a best practice to use the plural for arrays, i.e. roles instead of role.)

[FluorescentHallucinogen]

As for proposal #4, I was wrong.

If a field’s resolve function throws an error, the error will be inserted into the response’s "errors" key and the field will resolve to null.

If a a null value returns in a resolver for a non-null field, the null result bubbles up to the nearest nullable parent.

[FluorescentHallucinogen]

BTW, it would be great if the @isAuthenticated and @hasRole directives could be applied to the OBJECT location (in addition to the FIELD_DEFINITION).

@czystyl Could you please implement OBJECT directive location support like in https://www.apollographql.com/docs/graphql-tools/schema-directives.html#Enforcing-access-permissions?

[czystyl]

You are very welcome to send a PR for that one.

[FluorescentHallucinogen]

@czystyl

You are very welcome to send a PR for that one.

I'll implement support of OBJECT directive location and make a pull request as soon as I have some free time. 😉

IMO, it would also be great to have support of ARGUMENT_DEFINITION and INPUT_FIELD_DEFINITION directive locations.

Currently, it's possible to restrict the whole mutation, e.g.:

type Mutation {
  createUser(name: String, email: String, age: Int, verified: Boolean): User!
  updateUser(id: ID!, name: String, email: String, age: Int, verified: Boolean): User! @hasRole(roles: [MANAGER])
  deleteUser(id: ID!): User @hasRole(roles: [ADMIN])
  ...
}

But what if I want to restrict only updating of the verified field (but allow updating the rest fileds) in the updateUser mutation, not the whole mutation (by applying the directive to the argument)? In this case the directive must support ARGUMENT_DEFINITION location. Something like:

type Mutation {
  updateUser(id: ID!, name: String, email: String, age: Int, verified: Boolean @hasRole(roles: [MANAGER])): User!
  ...
}

The mutation arguments may be extracted to separate input types. In this case the directive must support INPUT_FIELD_DEFINITION location. Something like:

type Mutation {
  updateUser(data: UserUpdateInput!, where: UserWhereUniqueInput!): User!
  ...
}

...

input UserUpdateInput {
  name: String
  email: String
  age: Int
  verified: Boolean @hasRole(roles: [MANAGER])
}

[czystyl]

Thank you so much. I appreciate it a lot 💪 If you need any help just let me know :)

[FluorescentHallucinogen]

Good news! I've implemented a @hasRole directive that:

• can be applied to FIELD_DEFINITION and OBJECT
• supports complex expressions using has_some and has_every rules and nested logical combinators OR, AND and NOT, e.g.:

title: String @hasRole(has_some: [ADMIN], NOT: [{ has_every: [MANAGER] }])

• supports arrays for user roles
• supports enum for user roles definition

PTAL at https://gist.github.com/FluorescentHallucinogen/75d12beb731f1a253c8f188919d5f5a7 😉

@czystyl Could you please integrate my code into your package? I'm new to TypeScript. 🙈

[czystyl]

Hi, @FluorescentHallucinogen great news! I'll try to have a look into that in the upcoming weekend. I'm very happy that you are so involved in this repository 💪

[FluorescentHallucinogen]

@czystyl Have you had a chance to look?

[czystyl]

Sorry for the late response. I try to do my best this weekend.

[czystyl]

@FluorescentHallucinogen wondering if you could open and PR with your gist and then I'll add the types there - then would be great to have you as a contributor. I'm a bit busy currently.