Graphql-shield: Graphql-shield doesn't work with multipart requests (Graphql-yoga Upload)

[RinatGumarov]

Bug report

• [x] I have checked other issues to make sure this is not a duplicate.

Describe the bug

I found unexpected behavior of graphql-shield while using it for mutations, that accept file of type Upload as parameter. I use it with apollo client, which transforms requests into multipart, if they have some file included.

To Reproduce

1. This is my GraphQL Schema.

scalar Upload

type Mutation {
  createUser(
    name: String!
    avatar: Upload
  ): User!
}

type Query {
  users: [User!]!
}

type User {
  id: ID!
  name: String!
  avatarUrl: String
}

1. This is the invoked query

createUser({
                variables: {
                  name: "John Doe",
                  avatar: new ReactNativeFile({
                          uri: values.image,
                          type: 'image/*',
                          name: `IMG_${new Date().toISOString()}`,
                  }),
                },
              })

1. I use these permissions

const allow = rule()(async (parent, args, ctx: Context, info) => {
  console.log("HERE")
  return true
})

const permissions = shield({
  Mutation: {
    createUser: allow,
  },
})

1. This is the error I see

Object {
  "data": null,
  "errors": Array [
    Object {
      "locations": Array [
        Object {
          "column": 3,
          "line": 2,
        },
      ],
      "message": "Not Authorised!",
      "path": Array [
        "createUser",
      ],
      "stack": "Error: Not Authorised!
    at normalizeOptions (/banshee/packages/backend/node_modules/graphql-shield/src/shield.ts:32:32)
    at Object.shield (/banshee/packages/backend/node_modules/graphql-shield/src/shield.ts:51:29)
    at Object.<anonymous> (/banshee/packages/backend/src/permissions.ts:245:28)
    at Module._compile (internal/modules/cjs/loader.js:774:30)
    at Module.m._compile (/banshee/packages/backend/node_modules/ts-node/src/index.ts:473:23)
    at Module._extensions..js (internal/modules/cjs/loader.js:785:10)
    at Object.require.extensions.<computed> [as .ts] (/banshee/packages/backend/node_modules/ts-node/src/index.ts:476:12)
    at Module.load (internal/modules/cjs/loader.js:641:32)
    at Function.Module._load (internal/modules/cjs/loader.js:556:12)
    at Module.require (internal/modules/cjs/loader.js:681:19)",
    },
  ],
}

Expected behavior

• "HERE" displayed in console.
• User created.

Actual behaviour

• "HERE" is not displayed in console.
• User not created, but permissions should not deny this mutation

Additional context

Also, I can successfully create user without avatar.

Versions:
backend:
"graphql-yoga": "1.18.0"
"graphql-shield": "^5.7.1"
frontend:
"apollo-upload-client": "10.0.0",
"graphql": "14.2.1",
"react-native": "https://github.com/expo/react-native/archive/sdk-33.0.0.tar.gz",

[open-collective-bot[bot]]

Hey @RinatGumarov :wave:,

Thank you for opening an issue. We will get back to you as soon as we can. Also, check out our Open Collective and consider backing us.

https://opencollective.com/graphql-shield

PS.: We offer priority support for all backers. Don't forget to add priority label when you start backing us :smile:

[RinatGumarov]

This error happens because type of avatar argument is promise and it could not be cached because "object-hash" does not have handler for promises. Maybe you can skip caching for promise arguments just for workaround?

[RinatGumarov]

For those, who have the same problem, I found workaround.
Add middleware that will resolve promises in args
e.g.
args.avatar = await args.avatar

[maticzav]

@RinatGumarov thank you for opening the issue. I think your suggestion is excellent. Maybe you could also use the new cacheFunction option which lets you skip/customise caching so that you don't rely on the default object-hash.

Let me know if there's anything else I can help you with 🙂

[bkstorm]

I think I have the same problem. The different thing is graphql-shield works with allow but doesn't work with 'other rules'.
My schema:

updateCmsUserProfile(data: CmsUserUpdateProfileInput!, where: CmsUserWhereUniqueInput!): CmsUser!

input CmsUserUpdateProfileInput {
  avatarUpload: Upload
  certificationUpload: Upload
  firstName: String!
  identityCardUpload: Upload
  lastName: String!
  phoneNumber: String!
}

Here is my rule:

export const isAuthenticated = rule()((parent, args, context) => {
  return Boolean(context.user)
})

When I set updateCmsUserProfile: allow, I can update user profile with avatar, certification, identitycard. But when I set updateCmsUserProfile: isAuthenticated, an error show up, here is the stack trace.

0: "Error: Not Authorised!"
1: "    at normalizeOptions (/Users/hoangnv/workspace/projects/reviewty/cms-api/node_modules/graphql-shield/src/shield.ts:32:32)"
2: "    at Object.shield (/Users/hoangnv/workspace/projects/reviewty/cms-api/node_modules/graphql-shield/src/shield.ts:51:29)"
3: "    at Object.<anonymous> (/Users/hoangnv/workspace/projects/reviewty/cms-api/src/permissions/index.ts:14:28)"
4: "    at Module._compile (internal/modules/cjs/loader.js:774:30)"
5: "    at Module._compile (/Users/hoangnv/workspace/projects/reviewty/cms-api/node_modules/source-map-support/source-map-support.js:521:25)"
6: "    at Module.m._compile (/private/var/folders/46/tn0m_h7d0_gc2zv9lg76gs140000gn/T/ts-node-dev-hook-24405334158666148.js:56:25)"
7: "    at Module._extensions..js (internal/modules/cjs/loader.js:785:10)"
8: "    at require.extensions.<computed> (/private/var/folders/46/tn0m_h7d0_gc2zv9lg76gs140000gn/T/ts-node-dev-hook-24405334158666148.js:58:14)"
9: "    at Object.nodeDevHook [as .ts] (/Users/hoangnv/workspace/projects/reviewty/cms-api/node_modules/ts-node-dev/lib/hook.js:61:7)"
10: "    at Module.load (internal/modules/cjs/loader.js:641:32)"
11: "    at Function.Module._load (internal/modules/cjs/loader.js:556:12)"
12: "    at Module.require (internal/modules/cjs/loader.js:681:19)"
13: "    at require (internal/modules/cjs/helpers.js:16:16)"
14: "    at Object.<anonymous> (/Users/hoangnv/workspace/projects/reviewty/cms-api/src/index.ts:15:1)"
15: "    at Module._compile (internal/modules/cjs/loader.js:774:30)"
16: "    at Module._compile (/Users/hoangnv/workspace/projects/reviewty/cms-api/node_modules/source-map-support/source-map-support.js:521:25)"

I'm using graphql-shield 5.7.1. I tried to update to 6.0.2 but it doesn't work. Then I downgrade to 5.3.1, it works :D.
If you guy need more information or a repository to reproduce this error, just let me know.

[maticzav]

@bkstorm a reproduction repository would be great. 🙂

[bkstorm]

@bkstorm a reproduction repository would be great. 🙂

Okay, I will create a repository.

[bkstorm]

@maticzav I just create a repository: https://github.com/bkstorm/apollo-upload-graphql-shield
Please read README.md for more information.

[sapkra]

@maticzav Any update on that? Would be nice to have this fixed soon.

[maticzav]

@bkstorm thank you for the reproduction repo. Is the problem you are experiencing connected with @RinatGumarov's comment - hash function prevents you from using Shield with the multipart request, or something different?

Could you try changing the cache option to contextual, or providing custom (probably empty) hash function in the rule for the upload filed?

[sapkra]

Adding { cache: "contextual" } to the related rules is fixing the problem but it should work by default.

[maticzav]

@sapkra, we changed the default option to strict to prevent any unwanted security leaks. I believe that was the right decision. Furthermore, I agree that it be best if it would work by default; however, considering that the alternative case is potential data leak, I find the additional work to be beneficial.

I suggest we add a section to the docs or create a rule which would allow the upload.

[sapkra]

Havn't looked in the source code yet but will it not be possible to support the upload with strict caching?

[maticzav]

@sapkra I think not since the problem arises because upload field arguments aren't cacheable (I.e. They are async).

[sapkra]

@maticzav I've created an PR which fixes the problem for me. Can you have a look?

[tylerjbainbridge]

I'm having this same issue-

...
Mutation: {
      '*': requireAuth,
      googleSignIn: allow,
    },
...

always fails for my uploadFile mutation.

[maticzav]

6.0.6 version changed the default caching option from strict to no_cache. I believe this should fix the majority of cases mentioned in the thread as we no longer rely on the hash function.

For anyone still experiencing the problem, I propose the following solution; either, use allow rule specifically on the Mutation/<upload> field, or create a new rule, which is a modification of allow rule and use custom hashing function which implements promise caching or set cache option to contextual or no_cache.

```ts

const uploadRule = rule({ cache: "no_cache" })(() => true)
const uploadRule = rule({ cache: "contextual" })(() => true)
const uploadRule = rule({
cache: (obj) => { /* custom fn / return "key" }
})(() => {
/ My rule implementation */
return true
})