Order of fields on a type matter for @authorization validation

[afreakk]

Describe the bug
In certain scenarios, the order of fields on a type matter for @authorization validation.

To Reproduce

1. Clone the following repo https://github.com/afreakk/n4j-issue

2. yarn install

3. ./test.sh watch-test

4. Test should now fail with:

   ...snipped...
   -               "id": Any<String>,
   +           "extensions": Object {
   +             "code": "INTERNAL_SERVER_ERROR",
   +             "stacktrace": Array [
   +               "Neo4jGraphQLForbiddenError: Forbidden",
   ...snipped...
   

5. Now if you change the following type in schema.graphql:

   type VehicleCard
       @authorization(
           validate: [{ where: { node: { tenant: { admins: { userId: "$jwt.id" } } } } }]
       ) {
       id: ID! @id
       garages: [Garage!]! @relationship(type: "VALID_GARAGES", direction: OUT)
       tenant: Tenant! @relationship(type: "VEHICLECARD_OWNER", direction: OUT) # <---  this line
   }
   

   By moving the pointed to line up so the type will look like this:

   type VehicleCard
       @authorization(
           validate: [{ where: { node: { tenant: { admins: { userId: "$jwt.id" } } } } }]
       ) {
       id: ID! @id
       tenant: Tenant! @relationship(type: "VEHICLECARD_OWNER", direction: OUT) # <---  this line
       garages: [Garage!]! @relationship(type: "VALID_GARAGES", direction: OUT)
   }
   

6. Tests will now pass...
   (you can force save index.test.js to force rerun of tests)

Expected behavior
Order of fields should not have an impact on @authorization.

System (please complete the following information):

• OS: NixOs
• Version: @neo4j/[email protected]
• Node.js version: v18.17.1

[neo4j-team-graphql]

Many thanks for raising this bug report @afreakk. 🐛 We will now attempt to reproduce the bug based on the steps you have provided.

Please ensure that you've provided the necessary information for a minimal reproduction, including but not limited to:

• Type definitions
• Resolvers
• Query and/or Mutation (or multiple) needed to reproduce

If you have a support agreement with Neo4j, please link this GitHub issue to a new or existing Zendesk ticket.

Thanks again! 🙏

[neo4j-team-graphql]

Many thanks for raising this bug report @afreakk. 🐛 We will now attempt to reproduce the bug based on the steps you have provided.

Please ensure that you've provided the necessary information for a minimal reproduction, including but not limited to:

• Type definitions
• Resolvers
• Query and/or Mutation (or multiple) needed to reproduce

If you have a support agreement with Neo4j, please link this GitHub issue to a new or existing Zendesk ticket.

Thanks again! 🙏

[neo4j-team-graphql]

We've been able to confirm this bug using the steps to reproduce that you provided - many thanks @afreakk! 🙏 We will now prioritise the bug and address it appropriately.

[neo4j-team-graphql]

This bug report has been assigned high priority to fix. If you wish to contribute a fix, please branch from master and submit your PR with the base set to master. Thanks!

[darrellwarde]

Hey @afreakk, I've been taking a look at this the past days. This is primarily down to the nature of authorization rules are checked directly before and after each operation, rather than before and after the entire query or mutation. The fix here is to change this architecture to the latter, which as you can maybe guess, is a very large bug fix!

We don't consider there to be a security issue here, so we are going to take this to the drawing board to think about how to achieve this over the coming weeks and months.