neo4j / graphql

A GraphQL to Cypher query execution layer for Neo4j and JavaScript GraphQL implementations.
https://neo4j.com/docs/graphql-manual/current/
Apache License 2.0
497 stars 147 forks source link

Neo4jError: Variable this not defined when auth rules are defined #5515

Closed khoi-fish closed 1 week ago

khoi-fish commented 2 weeks ago

Describe the bug When a nested type contains auth rules, the underlying Cypher that's produced is invalid.

Minimal repo for reproduction: https://github.com/khoi-fish/neo4j-graphql-where-bug (used for a previous bug report, please ignore the naming)

Type definitions

type User {
  id: ID!
  name: String
  cabinets: [Cabinet!]! @relationship(type: "HAS_CABINET", direction: OUT)
}

extend type User @authorization(
  validate: [
    { operations: [CREATE, DELETE], where: { jwt: { roles_INCLUDES: "admin" } } },
    { operations: [READ, UPDATE], where: { node: { id: "$jwt.sub" } } },
  ],
  filter: [
    {
      where: {
        node: {
          id: "$jwt.sub"
        }
      }
    }
  ]
)

type Cabinet {
  id: ID! @id
  name: String!
  folders: [Folder!]! @relationship(type: "HAS_FOLDER", direction: OUT)
  categories: [Category!]! @relationship(type: "HAS_CATEGORY", direction: OUT)
  user: User! @relationship(type: "HAS_CABINET", direction: IN)
}

extend type Cabinet @authorization(
  filter: [
    {
      where: {
        node: {
          user: {
            id: "$jwt.sub"
          }
        }
      }
    }
  ]
)

type Category {
  id: ID! @id
  name: String!
  files: [File!]! @relationship(type: "HAS_FILE", direction: OUT)
  folder: [Folder!]! @relationship(type: "HAS_CATEGORY", properties: "ColorType", direction: IN)
  cabinet: Cabinet! @relationship(type: "HAS_CATEGORY", direction: IN)
}

extend type Category @authorization(
  filter: [{ where: { node: { cabinet: { user: { id: "$jwt.sub" } } } } }]
)

type Folder {
  id: ID! @id
  name: String!
  categories: [Category!]!
    @relationship(
      type: "HAS_CATEGORY"
      properties: "ColorType"
      direction: OUT
    )
  files: [File!]! @relationship(type: "HAS_FILE", direction: OUT)
  cabinet: Cabinet! @relationship(type: "HAS_CABINET", direction: IN)
}

extend type Folder
  @authorization(filter: [{ where: { node: { cabinet: { user: { id: "$jwt.sub" } } } } }])

type ColorType @relationshipProperties {
  color: String
}

type File {
  id: ID! @unique
  name: String!
  category: Category @relationship(type: "HAS_FILE", direction: IN)
  folder: Folder! @relationship(type: "HAS_FILE", direction: IN)
}

extend type File @authorization(
  filter: [
    { 
      where: {
        node: {
          folder: {
            cabinet: {
              user: {
                id: "$jwt.sub"
              }
            }
          }
        }
      }
    }
  ]
)

(Note this can also be found under src/types/index.ts

To Reproduce

https://github.com/khoi-fish/neo4j-graphql-where-bug

How to run dev mode

  1. Create a .env file with the following contents
    ENVIRONMENT=local
    NEO4J_URL=****
    NEO4J_USER=****
    NEO4J_PASS=****

    All variables can be found under this project's Settings -> Secrets and variables -> Actions -> "Variables" tab

  2. Be on Node 20
  3. Run npm install
  4. Run npm run dev

    a. (optional) Run npm run dev:debug to get Cypher output

Reproducing the error

  1. Go to localhost:4000/graphql
  2. Run the following mutation:

    mutation DeleteCategory($categoryId: ID!) {
      deleteCategories(where: { id: $categoryId }) {
        __typename
        nodesDeleted
        relationshipsDeleted
      }
    }
    
    // Variables
    {
      "categoryId": "category-video"
    }
  3. The response should return an error "Variable this not defined (line 9, column 8 ..."

Note that the delete operation works when auth rules are not defined.

Corresponding Cypher output:

MATCH (this:Category)
MATCH (this)<-[:HAS_CATEGORY]-(this0:Cabinet)
OPTIONAL MATCH (this0)<-[:HAS_CABINET]-(this1:User)
WITH *, count(this1) AS userCount
WITH *
WHERE (userCount <> 0 AND ($jwt.sub IS NOT NULL AND this1.id = $jwt.sub))
RETURN count(this0) = 1 AS var2
WITH *
WHERE (this.id = $param1 AND ($isAuthenticated = true AND var2 = true))
DETACH DELETE this

cypher params: {
  jwt: {
    iat: 1724442925,
    exp: 9724445955,
    sub: '4252a2b0-9492-4bef-8a94-fa20c97e7f71',
    tId: 'public',
    sessionHandle: 'a9021a8e-0d39-4639-8cb4-276e1724ba69',
    'st-ev': { v: true, t: 1724442925149 },
    'st-role': { v: [Array], t: 1724172602771 },
    'st-perm': { v: [], t: 1724172602871 }
  },
  param1: 'category-video',
  isAuthenticated: true
}

Expected behavior

The category should be deleted and there should be no errors

Screenshot

image

System (please complete the following information):

Additional context Add any other context about the problem here.

khoi-fish commented 2 weeks ago

Same repo and reproduction steps as https://github.com/neo4j/graphql/issues/5497 but just a different mutation. Please let me know if anyone has issues reproducing the bug. Also, I will try to keep the AuraDB instance up and running this time lol

neo4j-team-graphql commented 1 week ago

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

mjfwebb commented 1 week ago

Minimal reproduction for testing:

No data required.

Type definitions:

type JWT @jwt {
    roles: [String!]!
}

type User
    @authorization(
        validate: [
            { operations: [CREATE, DELETE], where: { jwt: { roles_INCLUDES: "admin" } } }
            { operations: [READ, UPDATE], where: { node: { id: "$jwt.sub" } } }
        ]
        filter: [{ where: { node: { id: "$jwt.sub" } } }]
    ) {
    id: ID!
    cabinets: [Cabinet!]! @relationship(type: "HAS_CABINET", direction: OUT)
}

type Cabinet @authorization(filter: [{ where: { node: { user: { id: "$jwt.sub" } } } }]) {
    id: ID! @id
    categories: [Category!]! @relationship(type: "HAS_CATEGORY", direction: OUT)
    user: User! @relationship(type: "HAS_CABINET", direction: IN)
}

type Category @authorization(filter: [{ where: { node: { cabinet: { user: { id: "$jwt.sub" } } } } }]) {
    id: ID! @id
    files: [File!]! @relationship(type: "HAS_FILE", direction: OUT)
    cabinet: Cabinet! @relationship(type: "HAS_CATEGORY", direction: IN)
}

type File {
    id: ID! @unique
    category: Category @relationship(type: "HAS_FILE", direction: IN)
}

Mutation:

mutation {
  deleteCategories(where: { id: "category-video" }) {
    __typename
    nodesDeleted
    relationshipsDeleted
  }
}

Error output:

{
  "errors": [
    {
      "message": "Variable `this` not defined (line 9, column 8 (offset: 281))\n\"WHERE (this.id = $param1 AND ($isAuthenticated = true AND var2 = true))\"\n        ^",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "deleteCategories"
      ],
      "extensions": {
        "code": "INTERNAL_SERVER_ERROR",
        "stacktrace": [
          "Neo4jError: Variable `this` not defined (line 9, column 8 (offset: 281))",
          "\"WHERE (this.id = $param1 AND ($isAuthenticated = true AND var2 = true))\"",
          "        ^",
          "",
          "    at captureStacktrace (/graphql/node_modules/neo4j-driver-core/lib/result.js:624:17)",
          "    at new Result (/graphql/node_modules/neo4j-driver-core/lib/result.js:112:23)",
          "    at newCompletedResult (/graphql/node_modules/neo4j-driver-core/lib/transaction.js:528:12)",
          "    at Object.run (/graphql/node_modules/neo4j-driver-core/lib/transaction.js:360:20)",
          "    at TransactionPromise.Transaction.run (/graphql/node_modules/neo4j-driver-core/lib/transaction.js:181:34)",
          "    at ManagedTransaction.run (/graphql/node_modules/neo4j-driver-core/lib/transaction-managed.js:54:21)",
          "    at Executor.transactionRun (/graphql/packages/graphql/src/classes/Executor.ts:297:28)",
          "    at /graphql/packages/graphql/src/classes/Executor.ts:274:33",
          "    at TransactionExecutor._safeExecuteTransactionWork (/graphql/node_modules/neo4j-driver-core/lib/internal/transaction-executor.js:211:26)",
          "    at TransactionExecutor.<anonymous> (/graphql/node_modules/neo4j-driver-core/lib/internal/transaction-executor.js:198:46)"
        ]
      }
    }
  ],
  "data": null
}
angrykoala commented 1 week ago

Hi @khoi-fish

Thanks for your report. I'm terribly sorry that this keeps cropping up in different places. Bear with us as we keep patching these :pray:. Your reports are being really helpful on finding and testing these cases

khoi-fish commented 1 week ago

No worries at all! Happy to help out 🙌